ansible-collections / amazon.aws

Ansible Collection for Amazon AWS
GNU General Public License v3.0
309 stars 341 forks source link

ec2_key makes changes in check mode #1367

Closed pgrenaud closed 1 year ago

pgrenaud commented 1 year ago

Summary

When running an ec2_key task in check mode, I've noticed that it creates new key pairs in AWS when it definitely should not.

Issue Type

Bug Report

Component Name

ec2_key

Ansible Version

ansible [core 2.14.0]
  config file = /Users/pgrenaud/.ansible.cfg
  configured module search path = ['/Users/pgrenaud/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.10/site-packages/ansible
  ansible collection location = /Users/pgrenaud/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.10.9 (main, Dec 15 2022, 18:18:30) [Clang 14.0.0 (clang-1400.0.29.202)] (/usr/local/opt/python@3.10/bin/python3.10)
  jinja version = 3.1.2
  libyaml = True

Collection Versions

# /usr/local/lib/python3.10/site-packages/ansible_collections
Collection                    Version
----------------------------- -------
amazon.aws                    5.1.0  
ansible.netcommon             4.1.0  
ansible.posix                 1.4.0  
ansible.utils                 2.7.0  
ansible.windows               1.12.0 
arista.eos                    6.0.0  
awx.awx                       21.8.0 
azure.azcollection            1.14.0 
check_point.mgmt              4.0.0  
chocolatey.chocolatey         1.3.1  
cisco.aci                     2.3.0  
cisco.asa                     4.0.0  
cisco.dnac                    6.6.0  
cisco.intersight              1.0.20 
cisco.ios                     4.0.0  
cisco.iosxr                   4.0.2  
cisco.ise                     2.5.9  
cisco.meraki                  2.11.0 
cisco.mso                     2.1.0  
cisco.nso                     1.0.3  
cisco.nxos                    4.0.0  
cisco.ucs                     1.8.0  
cloud.common                  2.1.2  
cloudscale_ch.cloud           2.2.2  
community.aws                 5.0.0  
community.azure               2.0.0  
community.ciscosmb            1.0.5  
community.crypto              2.8.1  
community.digitalocean        1.22.0 
community.dns                 2.4.1  
community.docker              3.2.1  
community.fortios             1.0.0  
community.general             6.0.1  
community.google              1.0.0  
community.grafana             1.5.3  
community.hashi_vault         4.0.0  
community.hrobot              1.6.0  
community.libvirt             1.2.0  
community.mongodb             1.4.2  
community.mysql               3.5.1  
community.network             5.0.0  
community.okd                 2.2.0  
community.postgresql          2.3.0  
community.proxysql            1.4.0  
community.rabbitmq            1.2.3  
community.routeros            2.3.1  
community.sap                 1.0.0  
community.sap_libs            1.3.0  
community.skydive             1.0.0  
community.sops                1.4.1  
community.vmware              3.1.0  
community.windows             1.11.1 
community.zabbix              1.8.0  
containers.podman             1.9.4  
cyberark.conjur               1.2.0  
cyberark.pas                  1.0.14 
dellemc.enterprise_sonic      2.0.0  
dellemc.openmanage            6.3.0  
dellemc.os10                  1.1.1  
dellemc.os6                   1.0.7  
dellemc.os9                   1.0.4  
f5networks.f5_modules         1.20.0 
fortinet.fortimanager         2.1.7  
fortinet.fortios              2.1.7  
frr.frr                       2.0.0  
gluster.gluster               1.0.2  
google.cloud                  1.0.2  
hetzner.hcloud                1.8.2  
hpe.nimble                    1.1.4  
ibm.qradar                    2.1.0  
ibm.spectrum_virtualize       1.10.0 
infinidat.infinibox           1.3.7  
infoblox.nios_modules         1.4.0  
inspur.ispim                  1.2.0  
inspur.sm                     2.3.0  
junipernetworks.junos         4.0.0  
kubernetes.core               2.3.2  
lowlydba.sqlserver            1.0.4  
mellanox.onyx                 1.0.0  
netapp.aws                    21.7.0 
netapp.azure                  21.10.0
netapp.cloudmanager           21.21.0
netapp.elementsw              21.7.0 
netapp.ontap                  22.0.1 
netapp.storagegrid            21.11.1
netapp.um_info                21.8.0 
netapp_eseries.santricity     1.3.1  
netbox.netbox                 3.8.1  
ngine_io.cloudstack           2.2.4  
ngine_io.exoscale             1.0.0  
ngine_io.vultr                1.1.2  
openstack.cloud               1.10.0 
openvswitch.openvswitch       2.1.0  
ovirt.ovirt                   2.3.1  
purestorage.flasharray        1.14.0 
purestorage.flashblade        1.10.0 
purestorage.fusion            1.1.1  
sensu.sensu_go                1.13.1 
splunk.es                     2.1.0  
t_systems_mms.icinga_director 1.31.4 
theforeman.foreman            3.7.0  
vmware.vmware_rest            2.2.0  
vultr.cloud                   1.3.1  
vyos.vyos                     4.0.0  
wti.remote                    1.0.4  

AWS SDK versions

Name: boto
Version: 2.49.0
Summary: Amazon Web Services Library
Home-page: https://github.com/boto/boto/
Author: Mitch Garnaat
Author-email: mitch@garnaat.com
License: MIT
Location: /usr/local/lib/python3.10/site-packages
Requires: 
Required-by: 
---
Name: boto3
Version: 1.24.71
Summary: The AWS SDK for Python
Home-page: https://github.com/boto/boto3
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /usr/local/lib/python3.10/site-packages
Requires: botocore, jmespath, s3transfer
Required-by: 
---
Name: botocore
Version: 1.27.71
Summary: Low-level, data-driven core of boto 3.
Home-page: https://github.com/boto/botocore
Author: Amazon Web Services
Author-email: 
License: Apache License 2.0
Location: /usr/local/lib/python3.10/site-packages
Requires: jmespath, python-dateutil, urllib3
Required-by: boto3, s3transfer

Configuration

ANSIBLE_NOCOWS(/Users/pgrenaud/.ansible.cfg) = True
ANSIBLE_PIPELINING(/Users/pgrenaud/.ansible.cfg) = True
CONFIG_FILE() = /Users/pgrenaud/.ansible.cfg
DEFAULT_FORKS(/Users/pgrenaud/.ansible.cfg) = 25
DEFAULT_GATHERING(/Users/pgrenaud/.ansible.cfg) = smart
DEFAULT_HOST_LIST(/Users/pgrenaud/.ansible.cfg) = ['REDACTED, 'REDACTED']
DEFAULT_MANAGED_STR(/Users/pgrenaud/.ansible.cfg) = WARNING! THIS FILE IS MANAGED BY ANSIBLE. DO NOT EDIT!
INTERPRETER_PYTHON(/Users/pgrenaud/.ansible.cfg) = auto_silent
RETRY_FILES_ENABLED(/Users/pgrenaud/.ansible.cfg) = False
TRANSFORM_INVALID_GROUP_CHARS(/Users/pgrenaud/.ansible.cfg) = ignore

OS / Environment

No response

Steps to Reproduce

Run the following play once to create the key pair (ansible-playbook play.yml), then a second time in check mode (ansible-playbook play.yml --check):

- name: set ssh key pub
  ansible.builtin.set_fact:
    ssh_key_pub: '{{ lookup("file", "~/.ssh/id_rsa.pub") }}'
  run_once: true
  delegate_to: localhost

- name: set ssh key name
  ansible.builtin.set_fact:
    ssh_key_name: '{{ (ssh_key_pub | split)[2] }}'
  run_once: true
  delegate_to: localhost

- name: create key pair
  amazon.aws.ec2_key:
    name: '{{ ssh_key_name }}'
    key_material: '{{ ssh_key_pub }}'
  run_once: true
  delegate_to: localhost

Expected Results

I expect Ansible to report if the key pair would be or not created when running in check mode and not actually doing something.

Actual Results

Even though Ansible does not report any changes, a new key pair is created every time when running in check mode if the key pair already exists (nothing actually happens if the key pair does not exists). When running the play 5 times, I get the following result in the AWS console:

Screenshot 2023-02-13 at 11 52 28

Code of Conduct

ansibullbot commented 1 year ago

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 1 year ago

cc @jillr @prasadkatti @s-hertel @tremble @zbal click here for bot help

abikouo commented 1 year ago

@pgrenaud, the check_mode is working as expected for this module. Please note that in your first run, the key pair is created, therefore the 2nd run in check_mode report key is updated as this already exists. Note that If you run only once the playbook in check_mode ansible will report key pair created but you won't see the key pair in amazon.

Could you please clean your environment, run the playbook in check_mode, and validate that the key pair is not created even if ansible reports it is? Thanks

pgrenaud commented 1 year ago

@abikouo I was able to reproduce the issue many times, cleaning everything in-between tries.

If the key pair does not exist, ec2_key won't actually do anything when the playbook is run in regular mode even if changes are reported. This is expected behaviour.

If the key pair already exists, ec2_key will actually create a new key pair each time the playbook is run in check mode even if no changes are reported. This is unexpected behaviour, as no changes should be perform in check mode. See the attached screenshot above for the result in the AWS console.

abikouo commented 1 year ago

@pgrenaud seems that each time you are running using --check parameter the ssh_key_name is randomly generated, could you please run once again using -v option (ansible-playbook play.yaml --check -v) multiple times and share traces ?

gravesm commented 1 year ago

I'm able to reproduce this. The simplest reproducer is just a single task:

- amazon.aws.ec2_key:
    name: test-key
    key_material: "{{ lookup('file', '/path/to/key') }}"

The key does need to exist first. Running this in check mode before test-key exists won't create a new key, but running it in check mode when test-key exists will.

pgrenaud commented 1 year ago

@abikouo We don't learn much from running in verbose. Even running with ansible-playbook play.yml --check -vvv doesn't tell much. I ran it 3 times and I got the exact same output (compared with diff to make sure). Here it is:

ansible-playbook [core 2.14.2]
  config file = /Users/pgrenaud/REDACTED/ansible.cfg
  configured module search path = ['/Users/pgrenaud/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.11/site-packages/ansible
  ansible collection location = /Users/pgrenaud/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible-playbook
  python version = 3.11.2 (main, Feb 13 2023, 03:46:18) [Clang 14.0.0 (clang-1400.0.29.202)] (/usr/local/opt/python@3.11/bin/python3.11)
  jinja version = 3.1.2
  libyaml = True
Using /Users/pgrenaud/REDACTED/ansible.cfg as config file
host_list declined parsing /Users/pgrenaud/REDACTED/inventory.ini as it did not pass its verify_file() method
script declined parsing /Users/pgrenaud/REDACTED/inventory.ini as it did not pass its verify_file() method
auto declined parsing /Users/pgrenaud/REDACTED/inventory.ini as it did not pass its verify_file() method
yaml declined parsing /Users/pgrenaud/REDACTED/inventory.ini as it did not pass its verify_file() method
Parsed /Users/pgrenaud/REDACTED/inventory.ini inventory source with ini plugin
host_list declined parsing /Users/pgrenaud/REDACTED/inventory-local.ini as it did not pass its verify_file() method
script declined parsing /Users/pgrenaud/REDACTED/inventory-local.ini as it did not pass its verify_file() method
auto declined parsing /Users/pgrenaud/REDACTED/inventory-local.ini as it did not pass its verify_file() method
yaml declined parsing /Users/pgrenaud/REDACTED/inventory-local.ini as it did not pass its verify_file() method
Parsed /Users/pgrenaud/REDACTED/inventory-local.ini inventory source with ini plugin
Skipping callback 'default', as we already have a stdout callback.
Skipping callback 'minimal', as we already have a stdout callback.
Skipping callback 'oneline', as we already have a stdout callback.

PLAYBOOK: play.yml *************************************************************
1 plays in play.yml

PLAY [ec2] *********************************************************************

TASK [set ssh key pub] *********************************************************
task path: /Users/pgrenaud/REDACTED/play.yml:7
ok: [localhost -> localhost] => {
    "ansible_facts": {
        "ssh_key_pub": "ssh-rsa REDACTED pgrenaud@pgrenaud-2021"
    },
    "changed": false
}

TASK [set ssh key name] ********************************************************
task path: /Users/pgrenaud/REDACTED/play.yml:13
ok: [localhost -> localhost] => {
    "ansible_facts": {
        "ssh_key_name": "pgrenaud@pgrenaud-2021"
    },
    "changed": false
}

TASK [create key pair] *********************************************************
task path: /Users/pgrenaud/REDACTED/play.yml:19
Using module file /usr/local/lib/python3.11/site-packages/ansible_collections/amazon/aws/plugins/modules/ec2_key.py
Pipelining is enabled.
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: pgrenaud
<localhost> EXEC /bin/sh -c '/usr/local/bin/python3 && sleep 0'
ok: [localhost -> localhost] => {
    "changed": false,
    "invocation": {
        "module_args": {
            "access_key": "REDACTED",
            "aws_ca_bundle": null,
            "aws_config": null,
            "debug_botocore_endpoint_logs": false,
            "endpoint_url": null,
            "force": true,
            "key_material": "ssh-rsa REDACTED pgrenaud@pgrenaud-2021",
            "key_type": null,
            "name": "pgrenaud@pgrenaud-2021",
            "profile": null,
            "purge_tags": true,
            "region": "ca-central-1",
            "secret_key": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "session_token": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "state": "present",
            "tags": null,
            "validate_certs": true
        }
    },
    "key": {
        "fingerprint": "ee:12:9c:69:81:0a:8e:0d:91:b2:92:95:e8:e3:01:15",
        "id": "key-082de2518417d1795",
        "name": "pgrenaud@pgrenaud-2021",
        "tags": {},
        "type": "rsa"
    },
    "msg": "key pair already exists"
}

PLAY RECAP *********************************************************************
localhost           : ok=3    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

It does say at the end that the key pair already exists and refer to key key-082de2518417d1795 which is the key that created first when running in normal mode (not check mode).

pgrenaud commented 1 year ago

@gravesm Yes, exactly!

abikouo commented 1 year ago

@gravesm @pgrenaud thanks for the feedback, I will try to reproduce it on my side and fix the issue. I will notify you when the pull request is ready

tremble commented 1 year ago

@abikouo FYI, looking at the code I suspect this was fixed by #1288 but I don't think the code's been backported.

abikouo commented 1 year ago

@pgrenaud I can confirm that the issue has been fixed by #1288 (thanks @tremble ) I will backport to this branch, in the meantime you can use the main branch the have latest features and fixes

tremble commented 1 year ago

Relevant fixes have been landed in main/stable-5 and will be available with the next release