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.

Sign up for GitHub

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

Jump to bottom

docker_secret isn't changing a secret when the data changes #30

Closed
kaysond opened this issue Nov 6, 2020 · 11 comments
Closed

docker_secret isn't changing a secret when the data changes #30

kaysond opened this issue Nov 6, 2020 · 11 comments

Comments

@kaysond
Copy link
Contributor

kaysond commented Nov 6, 2020

SUMMARY

docker_secret succeeds and indicates changed: false even though I've run the module with a data argument that is different from the current secret

ISSUE TYPE
  • Bug Report
COMPONENT NAME

community.general.docker_secret

ANSIBLE VERSION
ansible 2.9.6
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/administrator/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.8.5 (default, Jul 28 2020, 12:59:40) [GCC 9.3.0]
CONFIGURATION
none
OS / ENVIRONMENT

Ubuntu 20.04

STEPS TO REPRODUCE

Unfortunately, I've been unable to reproduce this in a generic way, but my current setup exhibits the problem.
I have no containers or services running:

administrator@testserver:/opt/traefik-forward-auth$ docker ps
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES
administrator@testserver:/opt/traefik-forward-auth$ docker service ls
ID                  NAME                MODE                REPLICAS            IMAGE               PORTS

I have an existing secret:

administrator@testserver:/opt/traefik-forward-auth$ docker secret inspect traefik_forward_auth_secrets
[
    {
        "ID": "v2d61pz8f7457znwxlqob1mjd",
        "Version": {
            "Index": 3772
        },
        "CreatedAt": "2020-11-06T20:08:03.774468503Z",
        "UpdatedAt": "2020-11-06T20:08:03.774468503Z",
        "Spec": {
            "Name": "traefik_forward_auth_secrets",
            "Labels": {}
        }
    }
]

If I then run the module twice, each time with different data, at least one of them should succeed and change the secret:

administrator@testserver:/opt/traefik-forward-auth$ ansible -m community.general.docker_secret -a "name=traefik_forward_auth_secrets data=data1" localhost
localhost | SUCCESS => {
    "changed": false,
    "secret_id": "v2d61pz8f7457znwxlqob1mjd"
}
administrator@testserver:/opt/traefik-forward-auth$ ansible -m community.general.docker_secret -a "name=traefik_forward_auth_secrets data=data2" localhost
localhost | SUCCESS => {
    "changed": false,
    "secret_id": "v2d61pz8f7457znwxlqob1mjd"
}
EXPECTED RESULTS

The secret data should change

ACTUAL RESULTS

The secret data does not change, but the module reports success and unchanged

Running with extra verbosity:

ansible 2.9.6
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/administrator/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.8.5 (default, Jul 28 2020, 12:59:40) [GCC 9.3.0]
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
host_list declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
script declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
auto declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
Parsed /etc/ansible/hosts inventory source with ini plugin
Loading callback plugin minimal of type stdout, v2.0 from /usr/lib/python3/dist-packages/ansible/plugins/callback/minimal.py
META: ran handlers
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: administrator
<127.0.0.1> EXEC /bin/sh -c 'echo ~administrator && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/administrator/.ansible/tmp/ansible-tmp-1604703216.2871368-195791058474888 `" && echo ansible-tmp-1604703216.2871368-195791058474888="` echo /home/administrator/.ansible/tmp/ansible-tmp-1604703216.2871368-195791058474888 `" ) && sleep 0'
Using module_utils file ansible_collections/community/general
Using module_utils file ansible_collections/community
Using module_utils file ansible_collections
Using module_utils file ansible_collections/community/general/plugins/module_utils
Using module_utils file ansible_collections/community/general/plugins
Using module_utils file ansible_collections/community/general/plugins/module_utils/docker/common
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/_text.py
Using module_utils file ansible_collections/community/general/plugins/module_utils/docker
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/basic.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/parsing/convert_bool.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/six/__init__.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/__init__.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/_collections_compat.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/parsing/__init__.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/file.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/text/formatters.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/validation.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/sys_info.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/parameters.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/process.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/text/converters.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/text/__init__.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/_utils.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/pycompat24.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/_json_compat.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/common/collections.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/distro/__init__.py
Using module_utils file /usr/lib/python3/dist-packages/ansible/module_utils/distro/_distro.py
Using module file /home/administrator/.ansible/collections/ansible_collections/community/general/plugins/modules/docker_secret.py
<127.0.0.1> PUT /home/administrator/.ansible/tmp/ansible-local-2779062rjq36an5/tmpmh2jfgwc TO /home/administrator/.ansible/tmp/ansible-tmp-1604703216.2871368-195791058474888/AnsiballZ_docker_secret.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/administrator/.ansible/tmp/ansible-tmp-1604703216.2871368-195791058474888/ /home/administrator/.ansible/tmp/ansible-tmp-1604703216.2871368-195791058474888/AnsiballZ_docker_secret.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python3 /home/administrator/.ansible/tmp/ansible-tmp-1604703216.2871368-195791058474888/AnsiballZ_docker_secret.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /home/administrator/.ansible/tmp/ansible-tmp-1604703216.2871368-195791058474888/ > /dev/null 2>&1 && sleep 0'
localhost | SUCCESS => {
    "changed": false,
    "invocation": {
        "module_args": {
            "api_version": "auto",
            "ca_cert": null,
            "client_cert": null,
            "client_key": null,
            "data": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "data_is_b64": false,
            "debug": false,
            "docker_host": "unix://var/run/docker.sock",
            "force": false,
            "labels": null,
            "name": "traefik_forward_auth_secrets",
            "ssl_version": null,
            "state": "present",
            "timeout": 60,
            "tls": false,
            "tls_hostname": "localhost",
            "validate_certs": false
        }
    },
    "secret_id": "v2d61pz8f7457znwxlqob1mjd"
}
META: ran handlers
META: ran handlers
@ansibullbot

This comment has been minimized.

Sign in to view
@kaysond kaysond closed this as completed Nov 6, 2020
@kaysond kaysond reopened this Nov 6, 2020
@kaysond
Copy link
Contributor Author

kaysond commented Nov 6, 2020

!component

@kaysond
Copy link
Contributor Author

kaysond commented Nov 6, 2020

Upon further investigation, it seems the issue may be that the secret lacks a label called ansible_key. The rest of my secrets have that.

@felixfontein
Copy link
Collaborator

Did you set the old value with the module as well?

The problem is that it is not possible to retrieve the secret, so the module has to use ansible_key to store a hash for being able to check if it changed.

@felixfontein felixfontein transferred this issue from ansible-collections/community.general Nov 7, 2020
@kaysond
Copy link
Contributor Author

kaysond commented Nov 7, 2020 •
edited
Loading

Did you set the old value with the module as well?

I thought I did, but the lack of the label would indicate otherwise.

The problem is that it is not possible to retrieve the secret, so the module has to use ansible_key to store a hash for being able to check if it changed.

Is this really the intended behavior? It seems extremely dangerous.

Correct me if I'm wrong: for every other Ansible module, if it succeeds and indicates changed: false, it means the module completed its operation without any problems and the host was already in the desired state. (changed: true would mean the module completed without any problems and the host was not previously in the desired state, but now is).

If force is not set, and the module is unable to determine whether the secret has changed or not, I think the module should fail. Otherwise a user has no idea that the host is not in the desired state once the module completes.

For example, if the secret needs to be changed but it is attached to the service, the module will fail whether force is set or not, because it can't remove the existing secret (this is because Docker returns an error, not through the module's behavior). The above situation should yield the same result.

@felixfontein
Copy link
Collaborator

The module does exactly do as documented, please read it's documentation: https://docs.ansible.com/ansible/latest/collections/community/general/docker_secret_module.html#synopsis

If force is not set, and the module is unable to determine whether the secret has changed or not, I think the module should fail. Otherwise a user has no idea that the host is not in the desired state once the module completes.

The author of the module obviously thought differently. Both sides have their pros and cons. I guess it would be possible to add an option which - when explicitly enabled - would yield the behavior you want, but changing the current behavior would be a breaking change and we try to avoid them.

@kaysond
Copy link
Contributor Author

kaysond commented Nov 7, 2020

The module does exactly do as documented, please read it's documentation: https://docs.ansible.com/ansible/latest/collections/community/general/docker_secret_module.html#synopsis

Thank you. I had looked at that page but missed the bit about the label.

The author of the module obviously thought differently. Both sides have their pros and cons. I guess it would be possible to add an option which - when explicitly enabled - would yield the behavior you want, but changing the current behavior would be a breaking change and we try to avoid them.

What about issuing a warning in that scenario? Doesn't change the behavior but at least makes the user aware that the host state may not be correct.

@felixfontein
Copy link
Collaborator

I think a warning would be ok. Feel free to create a PR (in this repo) for that!

kaysond added a commit to kaysond/community.docker that referenced this issue Nov 8, 2020
@kaysond
add a warning when ansible_label is not found on a secret. addresses a…
ef17ccb 
…nsible-collections#30
@kaysond kaysond mentioned this issue Nov 8, 2020
Merged
@kaysond
Copy link
Contributor Author

kaysond commented Nov 8, 2020

Done. Are the docker modules being moved out of community.general and into this collection? Should I be using community.docker going forward?

@felixfontein
Copy link
Collaborator

Thanks! About your questions: yes and yes+no. The details of the move are described here: ansible-collections/overview#117 (reply in thread)

If you are using Ansible 2.9, you need to switch to community.docker. If you are using ansible-base 2.10+, you don't have to change anything, except to make sure that community.docker is also installed when using community.general >= 2.0.0 (will be released in January 2021) or ansible-base 2.11+ (spring 2021). If you are using Ansible 2.10+, you don't need to change anything, as community.docker will be contained before the redirects happen (the redirects will appear in Ansible 2.11).

For ansible-base 2.10+ and Ansible 2.10.x, you only need to do someting if you want to use community.docker now (which has all the deprecations applied that were scheduled for community.general 2.0.0).

In any case, bugfixes and maybe also some features added in this repo will be backported to community.general, which means that your new warning will also show up in community.general.

kaysond added a commit to kaysond/community.docker that referenced this issue Nov 9, 2020
@kaysond
add a warning when ansible_label is not found on a secret. addresses a…
0de1d39 
…nsible-collections#30
felixfontein added a commit that referenced this issue Nov 9, 2020
@kaysond @felixfontein
Add a warning when ansible_key label is not found on a secret. (#31)
22ba76b 
* add a warning when ansible_label is not found on a secret. addresses #30

* Update changelogs/fragments/31-docker-secret.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
@kaysond
Copy link
Contributor Author

kaysond commented Nov 9, 2020

Fixed in #31

@kaysond kaysond closed this as completed Nov 9, 2020
@kaysond kaysond mentioned this issue Nov 27, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kaysond @felixfontein @ansibullbot