Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Submit to ansible-module-extras #3

Open
angystardust opened this issue Jul 19, 2016 · 17 comments
Open

Submit to ansible-module-extras #3

angystardust opened this issue Jul 19, 2016 · 17 comments
Assignees
Labels

Comments

@angystardust
Copy link

Hi @lqueryvg
first of all, I want to thank you for your great module. It's far better than using the buggy and ugly expires parameter of the user module; even better than shelling out using the command or the shell module in order to run the chage binary.
For those reasons, I'm asking you to submit your module for inclusion in the official ansible-module-extras repository so everyone can profit from using your module.

@lqueryvg lqueryvg self-assigned this Jul 20, 2016
@lqueryvg
Copy link
Owner

@angystardust, thanks for the kind words.
I shall investigate how to get this included.

@lqueryvg
Copy link
Owner

lqueryvg commented Jul 22, 2016

Before trying to get this added as an Ansible extras module, I'd like to get to the bottom of the date format issues that you (perhaps unintentionally) highlighted.

In order to support Idempotence, my module needs to try to predict whether a task would make a change and only execute chage when needed. This means that it needs to convert a date string into number of days and compare this with what's currently in /etc/shadow.

Having looked at the source code for chage, it turns out there's a few thousand lines of code (from GNU gettime() function) which takes date strings in lots of different formats. For example, strings like:

Fourth Oct 17
next wednesday
now +5 days

I don't think it's appropriate to try to replicate all of that code in an Ansible Python module.

But I could implement a subset by pre-defining a bunch of strptime() formats and looping through them until I get a parse success.

I'm keen to know, what are the most common date strings used by people and what would be useful from a playbook ?

@angystardust
Copy link
Author

Wow...i didn't know about all that different input formats. It reminds me the syntax of the date command.
By the way I think that we should support the AAAA-MM-GG format which is the most used one.
Would also be cool to support the "now + 5 days" format but maybe it's better to not implement this in the chage module code.
I was searching for a "date" jinja filter to convert the different formats in the AAAA-MM-GG but it seems that it doesn't exist. Would be very very useful to do things like this:

chage: user=test sp_expire="{{ 'now + 5 days' | date }}"

@lqueryvg
Copy link
Owner

I thought about the now +5 days idea. It's a strange one. Like Cinderella: it would stay idempotent only until midnight, after which it would change /etc/shadow again. It's probably therefore only useful for ad-hoc playbooks. Nevertheless I could support it with a simple +n syntax.

The filter approach looks good - but you'd probably need to write a custom filter for that.

@lqueryvg
Copy link
Owner

Started a discussion on the development group.

https://mail.google.com/mail/u/0/#inbox/1563bf2c423ac2fc

@mavit
Copy link

mavit commented Aug 3, 2016

To me, it seems sensible to keep the module simple. If anyone needs to do more complex transformations, they can use/write a filter plugin. If it was me, I'd support only ISO YYYY-MM-DD format, and integer days since 1970 (since it's the native format of /etc/shadow).

By the way, I notice that YAML itself supports times and dates, and these automatically get parsed in playbooks but don't, at present, seem to be passable to modules:

- hosts: localhost
  tasks:
    - debug:
        msg: 2016-01-01

Produces:

TASK [debug] *******************************************************************
 [WARNING]: Failure using method (v2_runner_on_ok) in callback plugin
(<ansible.plugins.callback.default.CallbackModule object at 0x7f890a585550>):
datetime.date(2016, 1, 1) is not JSON serializable

@lqueryvg
Copy link
Owner

I have to be honest and say that the number of hoops I'd have to jump through to get this module included in core Ansible seems enormous when compared to the simplicity of people just including this module as-is.

So far, this, plus lack of time has been putting me off implementing this request. However, I haven't ruled it out yet...

@jamescassell
Copy link
Contributor

Another issue is that ansible-galaxy does not include the library folder, so just adding a dependency does not work. I'm using ansible-galaxy with "scm: git"

@lqueryvg
Copy link
Owner

Hi @jamescassell - can you tell me how to reproduce the problem and I'll take a look...
Thanks,

@lqueryvg
Copy link
Owner

@jamescassell - this worked for me...

$ ls
requirements.yml
$ cat requirements.yml
- src: [email protected]:lqueryvg/ansible-role-chage.git
  scm: git
$ mkdir roles
$ ansible-galaxy install  -p roles -r requirements.yml
- extracting ansible-role-chage to /home/john/tmp/tmp/roles/ansible-role-chage
- ansible-role-chage was installed successfully
$ find .
.
./requirements.yml
./roles
./roles/ansible-role-chage
./roles/ansible-role-chage/.gitignore
./roles/ansible-role-chage/.travis.yml
./roles/ansible-role-chage/LICENSE
./roles/ansible-role-chage/README.md
./roles/ansible-role-chage/library
./roles/ansible-role-chage/library/chage      <<<< HERE
./roles/ansible-role-chage/meta
./roles/ansible-role-chage/meta/main.yml
./roles/ansible-role-chage/meta/.galaxy_install_info
./roles/ansible-role-chage/tests
./roles/ansible-role-chage/tests/Dockerfile.centos5
./roles/ansible-role-chage/tests/Dockerfile.ubuntu14.04
./roles/ansible-role-chage/tests/Makefile
./roles/ansible-role-chage/tests/ansible.cfg
./roles/ansible-role-chage/tests/hosts
./roles/ansible-role-chage/tests/library
./roles/ansible-role-chage/tests/test_playbook.yml
$

FWIW...

$ ansible-galaxy --version
ansible-galaxy 2.4.2.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/john/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible-galaxy
  python version = 2.7.5 (default, Nov 20 2015, 02:00:19) [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)]
$

@jamescassell
Copy link
Contributor

jamescassell commented Jun 14, 2018

$ ansible-galaxy --version
ansible-galaxy 2.5.4
  config file = /home/user/ansible/ansible.cfg
  configured module search path = [u'/home/user/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible-galaxy
  python version = 2.7.5 (default, Apr 11 2018, 07:36:10) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

@lqueryvg
Copy link
Owner

@jamescassell I have tried upgrading ansible. Albeit a sllightly higher version than you. This still works for me.

If you want me to look at this, please tell me exactly how to reproduce the problem.
Also, if it's a real problem, I'd like to move it to a new issue.

$ ansible-galaxy --version
ansible-galaxy 2.5.5
  config file = None
  configured module search path = [u'/Users/john/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/john/.pyenv/versions/2.7.14/envs/ansible/lib/python2.7/site-packages/ansible
  executable location = /Users/john/.pyenv/versions/ansible/bin/ansible-galaxy
  python version = 2.7.14 (default, May 20 2018, 15:20:12) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)]
$

$ ansible-galaxy install   -r requirements.yml
- extracting ansible-role-chage to /Users/john/.ansible/roles/ansible-role-chage
- ansible-role-chage was installed successfully
$ find /Users/john/.ansible/roles/ansible-role-chage
/Users/john/.ansible/roles/ansible-role-chage
/Users/john/.ansible/roles/ansible-role-chage/.gitignore
/Users/john/.ansible/roles/ansible-role-chage/.travis.yml
/Users/john/.ansible/roles/ansible-role-chage/library
/Users/john/.ansible/roles/ansible-role-chage/library/chage
/Users/john/.ansible/roles/ansible-role-chage/LICENSE
/Users/john/.ansible/roles/ansible-role-chage/meta
/Users/john/.ansible/roles/ansible-role-chage/meta/.galaxy_install_info
/Users/john/.ansible/roles/ansible-role-chage/meta/main.yml
/Users/john/.ansible/roles/ansible-role-chage/README.md
/Users/john/.ansible/roles/ansible-role-chage/tests
/Users/john/.ansible/roles/ansible-role-chage/tests/ansible.cfg
/Users/john/.ansible/roles/ansible-role-chage/tests/Dockerfile.centos5
/Users/john/.ansible/roles/ansible-role-chage/tests/Dockerfile.ubuntu14.04
/Users/john/.ansible/roles/ansible-role-chage/tests/hosts
/Users/john/.ansible/roles/ansible-role-chage/tests/library
/Users/john/.ansible/roles/ansible-role-chage/tests/Makefile
/Users/john/.ansible/roles/ansible-role-chage/tests/test_playbook.yml
$

@jamescassell
Copy link
Contributor

Sorry, I'm an idiot. I put the wrong "src:" path and it was grabbing a different role.

@lqueryvg
Copy link
Owner

All's well that ends well.

@angystardust
Copy link
Author

@lqueryvg now that extras repo is definitely closed, you should make a PR to the main ansible repo.
I think many people need your module so having it by default on an ansible installation would be great

@jamescassell
Copy link
Contributor

I've pointed several folks to this repo... would be very nice to have it included in ansible proper. I second the opinion of only supporting a very basic date format at first. If someone wants an extended version, that person can submit a PR implementing the additional functionality. It'd be a big win to have even just the basic functionality upstream.

@itcultus
Copy link

I love this module.
For me it would be ideal to add it in the extras or as a community collection (in case it's simpler to add it there instead of the core).

For the time being, I just download the file and place it in my "library" folder defined in the ansible.cfg file.

Thank you for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants