I use this a lot, even though I could template the key files. Feedback
welcome. Code at https://github.com/bradobro/ansible/blob/sshkey/library/sshkey.
Sshkey Module
I like the idea, and this is something I’d very much like to have.
I think the playbook syntax looks pretty dense though, so I think we can iterate on it a bit.
One idea for an improvement (unrelated and not blocking this) is we make some custom Jinja2 function so we can do this:
name: add ssh key for $item
action: sshkey -a “user=$item key={{ readfile(‘/path/on/server’) }} state=present”
with_items:
- charlie
- eddie
One other thing is more of a fix, we use “state”, not “ensure”, to be consistent with the rest of Ansible.
The code is also quite different from the other modules, and I would like them to be more or less consistent, which makes things easier for people to maintain.
I have nothing against Python classes, but they aren’t really needed here, so I think the path to starting a core module would be to make them look more or less the same.
The idea here is not rightness, but consistency for maintaince purposes.
If different modules implement their own ways of parsing, returning, thinking, it’s going to quickly become confusing when we want to change things or work on them.
Can you take a shot at homogenizing the module structure and fixing the “ensure” thing to be “state” ? I can probably look at the Jinja2 function.
–Michael
FWIW, Jinja2 may already allow access to the python os.file, in which case we would not need a custom function.
I will have to look at what this takes. That seems to make more sense but we can still put in a wrapper if need be.
Anyway, see comments re: module structure.
Michael,
Thanks for the feedback. Work has me travelling and speaking next
week, so I might not get to tweak things until I clear my desk the
week after.
I think the playbook syntax looks pretty dense though, so I think we can iterate on it a bit.
One idea for an improvement (unrelated and not blocking this) is we make some custom Jinja2 function so we can do this:
name: add ssh key for $item
action: sshkey -a "user=$item key={{ readfile('/path/on/server') }} state=present"
with_items:
- charlie
- eddie
That's a start. Seems to me with some discussion we might be able to
do even better.
The current syntax is very similar to puppet, which I don't like, but
handle by keeping all the classes with sshkeys in one file I edit
rarely (only when I make my team change their key pairs). I know I'd
better have linewrap off when I open it.
Seems better to me to have a team's sshkeys in one file, instead of a
bunch of separate files. I thought about just including a vars with
- charlies_pubkey = ghghgh
- eddies_pubkey = ghghghkd
....
Problem is it doesn't work well with lightweight iterations:
"key={{$items_key}}" doesn't work, it's ugly, and making it work
starts down that dark path of looking like a programming language.
Another annoyance I have with puppet (and all this is probably just
taste, but I rant on in case there's a better way here) is that I
always install public keys when I'm making users, and always when I
make users because of our security policy. I also don't like ugly
hashes in user classes. It would be nice to have a file with all the
ugly:
charlie: password=uglystuff key=uglierstuff
But accessing that starts looking very un-simple in a playbook. As it
is, my logins file, used mostly on dev servers, looks like
- name: make login brad
action: user name=brad ...
only_if: "'brado' in '$users'"
- name: setup pubkey brad
action: sshkey user=brad ...
only_if: "'brado' in '$users'"
One other thing is more of a fix, we use "state", not "ensure", to be consistent with the rest of Ansible.
Will do.
The code is also quite different from the other modules, and I would like them to be more or less consistent, which makes things easier for people to maintain.
I have nothing against Python classes, but they aren't really needed here, so I think the path to starting a core module would be to make them look more or less the same.
The idea here is not rightness, but consistency for maintaince purposes.If different modules implement their own ways of parsing, returning, thinking, it's going to quickly become confusing when we want to change things or work on them.
I can appreciate that. I value trying to match styles in a group
project. It was my first module, classes help me think through how
things work, I don't like the 'global' statement, but I'll look over
some of the other modules again and see what I can do.
Can you take a shot at homogenizing the module structure and fixing the "ensure" thing to be "state" ?
will do. I have to bring up some new development infrastructure in a 3
weeks, and I hope to do with with ansible.
Problem is it doesn’t work well with lightweight iterations:
“key={{$items_key}}” doesn’t work, it’s ugly, and making it work
starts down that dark path of looking like a programming language.
I might be able to do something like this.
user=$item key=[[/srv/sshkeys/$item]]
The code is also quite different from the other modules, and I would like them to be more or less consistent, which makes things easier for people to maintain.
I have nothing against Python classes, but they aren’t really needed here, so I think the path to starting a core module would be to make them look more or less the same.
The idea here is not rightness, but consistency for maintaince purposes.If different modules implement their own ways of parsing, returning, thinking, it’s going to quickly become confusing when we want to change things or work on them.
I can appreciate that. I value trying to match styles in a group
project. It was my first module, classes help me think through how
things work, I don’t like the ‘global’ statement, but I’ll look over
some of the other modules again and see what I can do.
I’m generally an OO kind of guy, so it pains me to ask for that, but I think it’s best in the long run.
It’s also the easiest for a non-Python developer wanting to write a module in ANOTHER language to know how to port
it to his or her favorite language. It doesn’t imply there is a class API you have to stick to or anything like that. Modules are basically really really dumb constructs. There are idempotent, but they are still scripts.
I don’t think we used “global” anywhere.
Can you take a shot at homogenizing the module structure and fixing the “ensure” thing to be “state” ?
will do. I have to bring up some new development infrastructure in a 3
weeks, and I hope to do with with ansible.
Great!