ACM module has no "check" mode #451

dale-c-anderson · 2021-03-02T20:34:53Z

SUMMARY

When running a playbook in check mode, changes are affected.

ISSUE TYPE

Bug Report

COMPONENT NAME

aws_acm

ANSIBLE VERSION

$ aws --version
aws-cli/1.19.17 Python/3.6.9 Linux/4.15.0-136-generic botocore/1.20.16

CONFIGURATION

$ ansible-config dump --only-changed
ANSIBLE_PIPELINING(./myproject/ansible.cfg) = True
ANSIBLE_SSH_CONTROL_PATH(./myproject/ansible.cfg) = ~/.ssh/ansible-ssh-%%C
COLLECTIONS_PATHS(./myproject/ansible.cfg) = ['./myproject/collections']
DEFAULT_CALLBACK_PLUGIN_PATH(./myproject/ansible.cfg) = ['./myproject/callbacks']
DEFAULT_FORKS(./myproject/ansible.cfg) = 20
DEFAULT_ROLES_PATH(./myproject/ansible.cfg) = ['./myproject/roles']
INTERPRETER_PYTHON(./myproject/ansible.cfg) = /usr/bin/python3
INVENTORY_IGNORE_PATTERNS(./myproject/ansible.cfg) = ['ssh-config']
RETRY_FILES_ENABLED(./myproject/ansible.cfg) = False

OS / ENVIRONMENT

$ uname -a
Linux mybox 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

STEPS TO REPRODUCE

Full playbook is at https://gist.github.com/dale-c-anderson/31c6212e8a9e86f4a829e2bdee40a6f8

The relevant portion is reproduced here:

- hosts: localhost
  run_once: true
  become: false
  connection: local
  vars_files:
    - "{{ playbook_dir }}/path_definitions.yml"
  tags:
    - install_certs
  tasks:
    - name: Import SSL cert to AWS Certificate Manager
      community.aws.aws_acm:
        state: present
        region: ca-central-1
        certificate: "{{ lookup('file', ssl_public_certs_dir + '/dummy.crt') }}"
        private_key: "{{ lookup('file', ssl_private_keys_dir + '/dummy.key') }}" # Key has already been encrypted.
        name_tag: dummy.example.com

Ran the above with

ansible-playbook --diff --check --tags install_certs

EXPECTED RESULTS

Certificate import should only be simulated.

ACTUAL RESULTS

Certificate was actually imported.

$ ansible-playbook playbooks/ssl-999-bug-report.yml --diff --check --tags install_certs

[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'
[WARNING]: Skipping plugin (/home/danderson/repos/git.acromedia.com/teams/telus/project-lion/telus-lion-drupal-infrastructure/callbacks/human-log.py) as it seems to be invalid: name 'reload' is not
defined

PLAY [localhost] ********************************************************************************************************************************************************************************************

PLAY [localhost] ********************************************************************************************************************************************************************************************

TASK [Gathering Facts] **************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Import SSL cert to AWS Certificate Manager] ***********************************************************************************************************************************************************
changed: [localhost]

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

The text was updated successfully, but these errors were encountered:

ansibullbot · 2021-03-02T20:41:35Z

Files identified in the description:
None

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 · 2021-03-02T23:04:37Z

Files identified in the description:

[plugins/modules/aws_acm.py](https://github.com/['ansible-collections/amazon.aws', 'ansible-collections/community.aws', 'ansible-collections/community.vmware']/blob/main/plugins/modules/aws_acm.py)

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 · 2021-03-02T23:04:38Z

cc @jillr @matt-telstra @s-hertel @tremble @wimnat
click here for bot help

matt-telstra · 2021-03-02T23:41:19Z

Ah, woops. I never use check mode myself, so I forgot to put this in.

What's check mode supposed to do? Just check the input parameters match the module spec? Or should it check that it can find IAM credentials?

Some boto calls allow you to do a "dry_run" to check that you've got sufficient IAM permissions, but I expect that this particular call does not.

dale-c-anderson · 2021-03-03T00:02:21Z

@matt-telstra See https://docs.ansible.com/ansible/latest/user_guide/playbooks_checkmode.html

Yes, check mode and dry run are the same thing. If the boto call doesn't support dry run directly, the module would probably need simulate it in some other way. Having a module that affects changes when it's not supposed to will result in a never-ending litany of bug reports.

matt-telstra · 2021-03-03T00:35:02Z

For testing in my own projects (outside of Ansible), I use moto. I suppose I could just turn on moto and do the checks that way?
Although that would start with a clean slate for every task. So deleting a cert would throw an error because the cert doesn't exist. So I'd have to handle that case.

I'm not sure whether other Ansible AWS modules use moto. That would add a dependency to the module. Is that worth it?

matt-telstra · 2021-03-03T02:25:56Z

It looks like some modules only check the input. Others say supports_check_mode=False (example), although I'm not sure what that does.

@jillr what do AWS modules normally do in check mode? Just validate the input? Or is there a helper function I can call to check IAM credentials? Would we expect an AWS task to fail in check mode if no IAM credentials can be found?

matt-telstra · 2021-03-03T06:58:29Z

Is this the kind of thing we need?

matt-telstra@0df3af8

(I haven't created a PR yet, because I haven't written the tests for this, or tested it manually, yet)

tremble · 2021-03-03T07:47:57Z

The idea of "check_mode" is generally to tell you if changes would be made. The most common way to do this is just to exit with changed=True or return True from a function (depending on the structure of the module) just before you'd apply a change. Generally read-only (get, list or describe) calls should still happen. Please note, the Boto/AWS check mode only tests permissions, not if a change would occur, and is especially useless on read-only operations.

mdavis-xyz · 2021-03-13T00:49:54Z

Can someone confirm that these integration tests cover the functionality that needs to be added?

mdavis-xyz@2c9e2c3

tremble · 2021-03-13T06:30:24Z

@mdavis-xyz Those tests look right at first glance.

Normally we write the tests using a separate assert statement rather than "failed_when", this makes the logic a little simpler to flow since you can write it in the positive rather than the negative

- name: check that cert was not really uploaded during check_mode
  assert:
    that:
    - upload_check is changed
    - list_after_check_mode_upload.certificates | length == 0

… default value for instance_type (ansible-collections#587) ec2_instance: Fix launch template condition, handle launch template - default value for instance_type SUMMARY The launch_template option in ec2_instance has a broken condition. Also the launch_template option defaults the instance_type to t2.micro if not specified and ignores the instance_type specified in the launch_template as said in the issue ansible-collections#451. Fixes ansible-collections#451 ansible-collections#462 ISSUE TYPE Bugfix Pull Request COMPONENT NAME ec2_instance ADDITIONAL INFORMATION The change does not break existing functionality as tested by the integration test run locally. Related to the condition fix in community.aws: ansible-collections#111 Ran the following playbook to verify the change. # create a launch template called "test-launch-template" - name: create test launch template community.aws.ec2_launch_template: template_name: test-launch-template image_id: ami-002068ed284xxxxxx instance_type: t3a.small network_interfaces: - associate_public_ip_address: no delete_on_termination: yes device_index: 0 groups: - sg-xxxxxxxxxxxxxxxxxx subnet_id: subnet-xxxxxxxxxxxxxxxxxx region: us-east-2 block_device_mappings: - device_name: /dev/sdb ebs: volume_size: 5 volume_type: gp3 delete_on_termination: true encrypted: yes - device_name: /dev/sdc ebs: volume_size: 2 volume_type: gp2 delete_on_termination: true encrypted: no tags: ssome: tags # launch a ec2 instance using launch template created earlier - launches t3a.small instance as expected - name: test launch template usage ec2_instance: wait: yes name: "test-instance-mk-t3a.small" launch_template: name: test-launch-template vpc_subnet_id: subnet-xxxxxxxxxxxxxxxxxx # launch ec2 instance using launch template created earlier - override instance type to be launch to t3.xlarge - name: test launch template usage - override instance type ec2_instance: wait: yes name: "test-instance-mk-t3.xlarge" instance_type: t3.xlarge launch_template: name: test-launch-template vpc_subnet_id: subnet-xxxxxxxxxxxxxxxxxx Reviewed-by: Jill R <None> Reviewed-by: Markus Bergholz <[email protected]> Reviewed-by: Emanuele Leopardi <None> Reviewed-by: Alina Buzachis <None> Reviewed-by: None <None>

ansibullbot added bug This issue/PR relates to a bug needs_triage python3 labels Mar 2, 2021

ansibullbot added module module plugins plugin (any type) labels Mar 2, 2021

ansibullbot removed the needs_triage label Mar 2, 2021

mdavis-xyz mentioned this issue Mar 14, 2021

aws_acm - check mode #477

Merged

tremble closed this as completed in #477 Mar 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ACM module has no "check" mode #451

ACM module has no "check" mode #451

dale-c-anderson commented Mar 2, 2021 •

edited

Loading

ansibullbot commented Mar 2, 2021

ansibullbot commented Mar 2, 2021

ansibullbot commented Mar 2, 2021

matt-telstra commented Mar 2, 2021

dale-c-anderson commented Mar 3, 2021 •

edited

Loading

matt-telstra commented Mar 3, 2021

matt-telstra commented Mar 3, 2021

matt-telstra commented Mar 3, 2021

tremble commented Mar 3, 2021 via email

mdavis-xyz commented Mar 13, 2021

tremble commented Mar 13, 2021

ACM module has no "check" mode #451

ACM module has no "check" mode #451

Comments

dale-c-anderson commented Mar 2, 2021 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

CONFIGURATION

OS / ENVIRONMENT

STEPS TO REPRODUCE

EXPECTED RESULTS

ACTUAL RESULTS

ansibullbot commented Mar 2, 2021

ansibullbot commented Mar 2, 2021

ansibullbot commented Mar 2, 2021

matt-telstra commented Mar 2, 2021

dale-c-anderson commented Mar 3, 2021 • edited Loading

matt-telstra commented Mar 3, 2021

matt-telstra commented Mar 3, 2021

matt-telstra commented Mar 3, 2021

tremble commented Mar 3, 2021 via email

mdavis-xyz commented Mar 13, 2021

tremble commented Mar 13, 2021

dale-c-anderson commented Mar 2, 2021 •

edited

Loading

dale-c-anderson commented Mar 3, 2021 •

edited

Loading