Skip to content
This repository has been archived by the owner on Mar 7, 2018. It is now read-only.

Set & delete users template #112

Merged
merged 2 commits into from
Jan 13, 2017

Conversation

mirceaulinic
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented Dec 30, 2016

Coverage Status

Coverage remained the same at 79.587% when pulling bb95552 on mirceaulinic:USERS-TPL into 3aee29e on napalm-automation:develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.587% when pulling bb95552 on mirceaulinic:USERS-TPL into 3aee29e on napalm-automation:develop.

@itdependsnetworks
Copy link
Contributor

At least when using ansible's implementation of jinja, it will fail if the key doesn't exist. I would suggest the following:

{%- for user_name, user_details in users.items() %}
{%- if user_details.get('password') %}
username {{ user_name }} secret 5 {{ user_details.password }}
{%- else %}
username {{ user_name }} nopassword
{%- endif %}
{%- if user_details.get('level') %}
username {{ user_name }} privilege {{ user_details.level }}
{%- endif %}
{%- if user_details.get('sshkeys') %}
{%- for sshkey in user_details.sshkeys %}
username {{ user_name }} sshkey {{ sshkey }}
{%- endfor %}
{%- endif %}
{%- endfor %}

@dbarrosop
Copy link
Member

I agree with @itdependsnetworks. We should check the variables are present before trying to configure them. Newest EOS release support users without password, only with keys. And there might be people using passwords only instead of keys.

@@ -0,0 +1,16 @@
{%- for user_name, user_details in users.iteritems() %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it enough to do something like:

{%- for user_name, user_details in users_to_delete %}
default username {{ user_name}}
{%- endfor %}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does:

default username {{ user_name}}

do more specifically?

@mirceaulinic
Copy link
Member Author

We should check the variables are present before trying to configure them. Newest EOS release support users without password, only with keys. And there might be people using passwords only instead of keys.

Can't agree more - that's why I have:

{%- if user_details.sshkeys %}

Which works in pure python.. but it seems to fail in other obscure implementations.

But I agree on the additional level of safety.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.587% when pulling 3a40477 on mirceaulinic:USERS-TPL into 3aee29e on napalm-automation:develop.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.587% when pulling 3a40477 on mirceaulinic:USERS-TPL into 3aee29e on napalm-automation:develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.587% when pulling 3a40477 on mirceaulinic:USERS-TPL into 3aee29e on napalm-automation:develop.

@dbarrosop dbarrosop assigned dbarrosop and unassigned dbarrosop Jan 13, 2017
@mirceaulinic mirceaulinic merged commit 5ba5212 into napalm-automation:develop Jan 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants