New module for generating remmina file #2
Conversation
Still lacks a lot of stuff I have elsewhere. Slowly migrating it over. Currently features: - ssh templates (missing proxy support) - host_vars plugin exposing hostname variables in branching A lot of missing, most importantly roles/playbook layouts; will be coming shortly.
Assorted random stuff I've been pondering doing for a while.
Hopefully we can remove this stuff as part of talking directly to jenkins. Hopefully.
almost there. need to generalise how different ssh users might have to use "become".
For some reason I don't inherit the variable properly. It looks like the group_vars file isn't included since other commands I put in there isn't read either
- Introduce support for jump hosts - Create a simple filter plugin to replace dicts - Update readme/todo
..only used while testing filter plugins
avoiding pythons non-deterministic dictionaries
For some reason, libuv failed with a missing m4 dependency.
Build fixes for libuv.
- add hosts that were missing - use consistent versioning for ubuntu1204 - add missing gaps in systemd setup
also, force sudo over su at requireio
..this is needed for debian7 as well. The shipped monit is old enough to not support process watching, so stick with pidfile.
work in progress of installing tap2junit which is needed to convert tap output to junit xml, suitable for reading in jenkins.
using only hardware (or the "smart" way) won't always give us the ansible vcpu stuff, which we use for `JOBS`.
did some mistakes by not committing often enough. These will likely be refactored once the branch will merge. Pretty readable changes though.
- avoid installing tap2junit for release hosts - add release hosts to worker host match - add new smartos15 worker - few notes to readme; todo is growing!
|
That's awesome! @joaocgreis can you also help review this? |
There was a problem hiding this comment.
@piccoloaiutante thanks for your effort in this, it's great to see this taking shape! And sorry for my delay reviewing, I wanted to give a careful review and actually try to run it. I hope you find this helpful.
Even if this is a moderate file, there is a lot to grasp. I might have missed your reason for some of the options you've taken. If so, please feel free to discuss! I'm trying to make this as simple as possible for someone new to just run.
It looks like you're testing on OSX. I'm on Linux so some things may differ, let me know if I've taken wrong assumptions. Would be great to have this working on both!
I like the file format you've defined for the secrets. @jbergstroem this bypasses the inventory file completely, and it makes sense to me. Ports should be randomized and secret as much as possible, and I see no reason to make the IPs public either. I understand that for Unix the inventory file is useful to run ansible, but Windows relies heavily on host_vars. When the actual windows playbook gets ported here, it would be good to translate the secrets into host_vars, or find a way to use them directly.
| module = AnsibleModule( | ||
| argument_spec=dict( | ||
| path=dict(required=True, type='str'), | ||
| serverfile=dict(required=True, type='str'), |
There was a problem hiding this comment.
path could be not required, default to $HOME/.remmina/ (you seem to be testing this on OSX, I'm not familiar with remmina there but the default could be per os?). path could be called remminapath, to make it obvious (it makes sense to me that the .remmina files are written directly to there, to make them usable right away).
You're reading serverfile from inside path below, but it should be an absolute location, so it can point to the file in secrets, wherever the secrets repo is cloned. serverfile could be the only required argument.
| argument_spec=dict( | ||
| path=dict(required=True, type='str'), | ||
| serverfile=dict(required=True, type='str'), | ||
| secret=dict(required=True, type='str'), |
There was a problem hiding this comment.
secret is not needed, it can be read from path/remmina.pref and look for a line that starts with secret=. If the file does not exist, just exit with error that remmina should be run once before this.
| secret=dict(required=True, type='str'), | ||
| passphrase=dict(required=False, type='str'), | ||
| gpgdir=dict(required=False, type='str'), | ||
| gpgbinary=dict(required=False, type='str') |
There was a problem hiding this comment.
I don't like to write my password anywhere but to dotgpg/gpg directly, it can very easily be left in bash history or log files. I tried to run this without specifying a passphrase, but got a TypeError: 'NoneType' object is not iterable. Perhaps that was just ansible getting in the way of the prompt, if that was the case then serverfile should just point to an already unencrypted file, and require the user to run dotgpg/gpg directly before this.
Can you perhaps use dotgpg instead, would it work? Requiring dotgpg to be in the path seems reasonable to me, you can just check if dotgpg -h can be run and error out if not. Plus, it would require no arguments. Instructions at: nodejs#577 (comment) . Note: to install gem on Ubuntu:
sudo apt-get install ruby-full ruby git-core &&
sudo gem install rubygems-update &&
sudo update_rubygems
| @@ -0,0 +1,115 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
The file should have the exec bit set: chmod +x remmina_config.py
|
|
||
| import base64 | ||
| from Crypto.Cipher import DES3 | ||
| import gnupg |
There was a problem hiding this comment.
Add a note that pip install pycrypto gnupg is needed, perhaps in the README.md and surround this in a try block that prints a helpful error?
| outfile.write("server={0}:{1}\n".format(ip.strip(), port.strip())) | ||
| outfile.write("username={0}\n".format(username.strip())) | ||
| outfile.write("password={0}\n".format(generate_password(password, secret))) | ||
| outfile.write("colordepth=15") |
| (username, ip, password, port) = server_list[host] | ||
| try: | ||
| filename = "{0}.remmina".format(host.strip()) | ||
| outfile = open(os.path.join(path,filename), "w") |
There was a problem hiding this comment.
Don't overwrite the file if it already exists because users can add other options using the remmina interface (shared folders, etc). Can you just print a warning if it exists?
| server_list = yaml.load(decrypted_server_list) | ||
| remmina_files = [] | ||
| for host in server_list: | ||
| (username, ip, password, port) = server_list[host] |
There was a problem hiding this comment.
This line is not working as intended, server_list[host] is good, but the variables don't get assigned. I get lines like server=ip:port in the resulting files. (Python 2.7.12)
| gpgdir='/Users/michele/Github/remmina/.gpg' \ | ||
| passphrase='1234567890' \ | ||
| secret='MTIzNDU2Nzg5MDEyMzQ1Njc4OTAxMjM0MTIzNDU2Nzg=' \ | ||
| gpgbinary='/usr/local/bin/gpg'" |
There was a problem hiding this comment.
Can you also add a simple playbook to run this, like https://github.com/jbergstroem/build/blob/feature/refactor-the-world/ansible/playbooks/write-ssh-config.yml ? The location of the script file could be passed using --extra-vars.
Also, can you add a line in https://github.com/jbergstroem/build/blob/feature/refactor-the-world/ansible/README.md#getting-things-done ?
|
Thanks for the feedback Joao.
I also use I will review work within a day or two! Exciting. |
|
@jbergstroem yes, the @piccoloaiutante can you rename |
|
@joaocgreis ok, thanks for clearing that out. I'll have a look and see if I can provide any suggestions. |
|
@joaocgreis thanks for your in deep review. I'll go through it in the next days and I'll apply your feedbacks. |
jbergstroem
force-pushed
the
feature/refactor-the-world
branch
from
8fa2a89
to
8bc3a5f
Compare
This is a a new ansible module for generating
.remminafiles starting from a yml file. The yml file should have a structure like this:This ansible module is expecting 5 params:
ymlfile is storedymlfilenameThe execution of this module will create one
.remminafile per server entry in server list file: each one of this file will be named as the server name (liketest-example-win10-x64-1.remmina).