Username Management for EOS with oc-system model #119
Conversation
- Added in translator for the users - Added tests for parser/translator - Added in the dummy template for the whole oc-system model
| # luckily the ordering is consistent regardless how it | ||
| # was entered(in eos at least), privilege then role | ||
| # | ||
| role: |
There was a problem hiding this comment.
I was wondering if we should do something like $privilege:$role:
_process:
- path: "privilege"
regexp: "(?P<value>\\d+) secret"
value: "{{ value }}:network-operator"
- path: "privilege"
regexp: "(?P<value>(?P<privilege>\\d+) role (?P<role>\\S+)) secret"
value: "{{ privilege }}:{{ role }}"
I just checked and looks like "network-operator" is the default role so I guess it would be better to be consistent and always include it. Thoughts?
There was a problem hiding this comment.
I like that idea. Unfortunately I can't get this to work as expected with the mock data I have. Here is what I have now
role:
_process:
- path: "privilege"
regexp: "(?P<value>\\d+) nopassword"
value: "{{value}}:network-operator"
- path: "privilege"
regexp: "(?P<value>\\d+) secret \\S+"
value: "{{value}}:network-operator"
- path: "privilege"
regexp: "(?P<value>(?P<privilege>\\d+) role (?P<role>\\S+)) secret \\S+"
value: "{{ privilege }}:{{ role }}"
I get this traceback
yang.parse_config(device=d)
File "/Users/tim/napalm-yang/napalm_yang/base.py", line 241, in parse_config
parser.parse()
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 87, in parse
self._parse(self._yang_name, self.model, self.mapping[self._yang_name])
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 93, in _parse
self._parse_container(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 131, in _parse_container
self._parse(k, v, mapping[v._yang_name])
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 93, in _parse
self._parse_container(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 131, in _parse_container
self._parse(k, v, mapping[v._yang_name])
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 93, in _parse
self._parse_container(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 131, in _parse_container
self._parse(k, v, mapping[v._yang_name])
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 93, in _parse
self._parse_container(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 131, in _parse_container
self._parse(k, v, mapping[v._yang_name])
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 95, in _parse
self._parse_list(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 173, in _parse_list
self._parse(key, obj, element_mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 93, in _parse
self._parse_container(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 131, in _parse_container
self._parse(k, v, mapping[v._yang_name])
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 93, in _parse
self._parse_container(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 131, in _parse_container
self._parse(k, v, mapping[v._yang_name])
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 97, in _parse
self._parse_leaf(attribute, model, mapping)
File "/Users/tim/napalm-yang/napalm_yang/parser.py", line 185, in _parse_leaf
value = self.parser.parse_leaf(attribute, mapping["_process"], self.bookmarks)
File "/Users/tim/napalm-yang/napalm_yang/parsers/base.py", line 151, in parse_leaf
result = self._parse_leaf_default(attribute, m, data)
File "/Users/tim/napalm-yang/napalm_yang/parsers/text_tree.py", line 113, in _parse_leaf_default
return super()._parse_leaf_default(attribute, mapping, data)
File "/Users/tim/napalm-yang/napalm_yang/parsers/jsonp.py", line 101, in _parse_leaf_default
d = self._parse_post_process_filter(mapping["value"], value=d)
File "/Users/tim/napalm-yang/napalm_yang/parsers/base.py", line 183, in _parse_post_process_filter
return helpers.template(post_process_filter, extra_vars=self.extra_vars, **kwargs)
File "/Users/tim/napalm-yang/napalm_yang/helpers.py", line 137, in template
return template.render(**kwargs)
File "/usr/local/lib/python2.7/site-packages/jinja2/environment.py", line 1008, in render
return self.environment.handle_exception(exc_info, True)
File "/usr/local/lib/python2.7/site-packages/jinja2/environment.py", line 780, in handle_exception
reraise(exc_type, exc_value, tb)
File "<template>", line 1, in top-level template code
jinja2.exceptions.UndefinedError: 'privilege' is undefined
I tired to do extra_vars.privilege, since that how it is done with the address prefix in the ipv4.yaml EOS mapping, but it fails as well but because with this error
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'privilege'
Right before the template.render command in helpers I launched a debugger. It looks like the
string = {{ privilege }}:{{ role }}
kwargs = {'user_key': 'test3', 'parent_key': 'test3', 'value': '15 role network-operator secret sha512 $6$Vd6.7k2FybfsTKKp$S.AHfdwicaWEoA41sPd6ZXOOdruJMrKJh70WNfiX/eZKH1oYBtFz9VbrPlYNDkhM/pi54gcYKH2hviy/xrUav.', 'extra_vars': {'user': {}, 'parent': {}}}
The other thing we'll need to do is check for the following line to set the default role
aaa authorization policy local default-role network-admin and set the default role accordingly.
There was a problem hiding this comment.
No worries, will try to find some time tomorrow and give it a try myself. Might require some massaging in the core.
|
@dbarrosop Do you have an examples for how this would work? I've update my fork with your suggestion below, but it isn't working. |
|
let me look into this tomorrow, I saw while helping with the junos:isis model that there might be a bug in how the |
|
ok, I think I finally got this right, see: #122 Note that I removed the |
Store the privilege and role in the oc-system role leaf and break out in translator
|
@dbarrosop Thank you for this, I've merge that branch it and works perfectly. I've updated the parser and translator. One more thing I would like to get working that I'm not sure the best way to do it is to change the default role. 7The default role is set with a line like this |
|
So I am not sure it'd work today but I think the best way of solving this is by adding rules to containers where you capture data to be used later on as "extra_vars". I will take a look when I find the time this week. |
Added a rule directive that specifies a default config setting that will then store that data in extra_vars so it can be used latter.
|
So I took a stab at doing what you suggested. It works, but I'm not sure how robust my solution is. Basically I've added new a rule directive that causes some config to be parsed in an otherwise unused _process section. |
napalm_yang/parsers/jsonp.py
Outdated
| key = mapping.get('key', 'config_default') | ||
| config_default = self.resolve_path( | ||
| data[0], mapping["config_default"], mapping.get("default")) | ||
| return "", {key: config_default['#text']} |
There was a problem hiding this comment.
Looks quite good and it's similar to what I was thinking myself. Just two differences to what I was thinking myself. From an API I'd do:
- path: "aaa.authorization.policy.local.default-role"
save: "default_role"
And then your implementation would look more like:
elif "save" in mapping:
return None, {mapping["save"]: d}
I also think we should return None here. That'd mean that the parser will keep processing rules (not sure if it will save the extra_vars though but we can fix that if it doesn't). In that case you could save multiple defaults in the same object or even save defaults right in the same leaf as it's needed. For instance:
my_container:
_process:
- path: blah.blah
save: key1
- path: bleh.bleh
save: key2
my_leaf:
- pre: "{{ etra_vars.key1 }}-{{ extra_vars.key2 }}"
or
my_container:
my_leaf:
- path: blah.blah
save: key1
- path: bleh.bleh
save: key2
- pre: "{{ etra_vars.key1 }}-{{ extra_vars.key2 }}"
There was a problem hiding this comment.
I couldn't get it to work exactly the way you have it above, and please let me know if im just going further down the wrong road. But if I return None the whole thing stops processing and I don't see how it would continue to process the other save directives. So for now I left it returning "" but I added something to the parsers.base.parse_container that would allow you to set multiple extra_vars, so we can do what you have posted about. My latest commit I'm able to grab another part of the config and I tacked it on to the end of the role just as a demo.
Now I haven't found a good example for testing the save directive in a leaf yet, since in the username role example we have here the data available to it are things under the path username.?name.privilege . But I'll test the some more later.
|
Nice job! Good to see someone is managing to decipher all that madness :) |
Changed the directive rules as per dbarrosop's comments, from config_default and key to path and save. Tweaked parsers.base.parse_container to allow for multiple extra_vars to be returned. If the mapping has multiple save directives, only the first was being taken since the mapping loop breaked after the extra_vars were returned. Other minor changes * Added parsing for eos root password * default to ipdb when debugging if installed. I need color in my life
|
Can you take a look to #125? Basically introduces the |
|
Oh, I just saw commit c6f8e7f which is quite similar. |
|
I merged #125 into my branch and it works as expected. I'll finish writing the parser integration tests and then push everything later tonight. Thanks for the help! And expect a nxos PR for the same user stuff soon. |
Take all the changes from napalm-automation#125 to get the extra data and remove my attempts. Update the mapping files to use the save_as directive and finish writing the profile tests. fix auto merge issue, jsonp.py should only take code from extract_data branch Add tests for the eos username parsing remove src dir that was accidentally committed specify ipdb.set_trace() correctly
|
I added the tests for the parsers, so I think this is ready to get merged but let me know if you want me to tweak anything. |
napalm_yang/parsers/base.py
Outdated
| import pdb | ||
| pdb.set_trace() | ||
| continue | ||
| try: |
There was a problem hiding this comment.
I don't mind this change but I'd rather do at the top of the file:
try:
import ipdb
pdb = ipdb
except ImportError:
import pdb
and then just leave in here pdb.set_trace()
There was a problem hiding this comment.
either that or a helper function that does what you wrote. Probably the helper function is actually better to avoid loading this if it's not necessary. I mostly want to avoid duplication of code.
|
Nice job! I took a quick look and it looks good. Just had a tiny comment that should be easy to fix. |
|
Don't merge this yet. I assumed that by passing a md5 hash or a SHA512 hash eos would know which hash I'm using, but I just sets the password to the hash. |
|
I fixed the password issue, let me know if there is anything else to tweak. |
|
Sorry for the delay, I think this is good now so I am merging it. |
I've adding in the functionality parse/translate user from EOS device with oc-system model
A few special notes on the username parsing. Let me know if you want me to change how any of this works. I put some of these notes in the yaml files as well, if that is ok.
The oc-system model has the idea of roles, but not of privilege level, but Arista has both. So I've lumped them together. If there is a privilege level set, only the privilege number will be set as the role. But if a role is also defined the role will be
D role NAMEwhereDis the privilege level number andNAMEis the role name.Also on the Aristas you can either have an MD5 hashed password or a sha512, but EOS detects the hash as either so you don't have to specify the type of hash when translating the code out. I originally pulled in either the number 5 or sha512 into the encrypted-password leaf with spaces but that isn't really needed.