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

aws_acm - check mode #477

Merged
merged 5 commits into from
Mar 18, 2021
Merged

Conversation

mdavis-xyz
Copy link
Contributor

SUMMARY

Fixes #451

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

aws_acm

ADDITIONAL INFORMATION

I've hard-coded the domain and ARN to return for some check_mode tasks.
I don't know if I need to bother dynamically extracting the domain from the local cert file.
I also don't know if ARNs are predictable for ACM certs.

I have not tried running these integration tests locally because the tests won't run. ansible-test tells me I need to configure stuff. I did, but ansible-test still won't run.

╭╴matthew:~/Documents/Projects/software/ansible/collections/ansible_collections/community/aws
╰╴ $ ansible-test integration --docker centos8 -v aws_acm
Falling back to tests in "tests/integration/targets/" because "roles/test/" was not found.
WARNING: Excluding tests marked "unstable" which require --allow-unstable or prefixing with "unstable/": aws_acm
WARNING: Excluding tests marked "cloud/aws" which require config (see "/home/matthew/.local/lib/python3.8/site-packages/ansible_test/config/cloud-config-aws.ini.template"): aws_acm
WARNING: All targets skipped.

So I'm just hoping that the CI bots can run the tests for me.

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_triage plugins plugin (any type) python3 tests tests needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 14, 2021
@mdavis-xyz mdavis-xyz force-pushed the aws_acm_check_mode branch 5 times, most recently from 9a5b9d8 to 32baad6 Compare March 14, 2021 05:49
@tremble tremble changed the title Aws acm check mode aws_acm - check mode Mar 14, 2021
plugins/modules/aws_acm.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"msg": "The conditional check '(list_tag.certificates | length) == 0' failed. The error was: error while evaluating conditional ((list_tag.certificates | length) == 0): 'dict object' has no attribute 'certificates'"

Looks like the tests need a tweak

@mdavis-xyz
Copy link
Contributor Author

I cannot figure out how to fix the failing tests, because I cannot figure out how to run integration tests locally.

The only documentation I found was for community.general. When I ran the tests like that it told me I needed to specify my AWS credentials in a config file. I did that, but this error won't go away.

How do I resolve this?

╭╴matthew:~/Documents/Projects/software/ansible/collections/ansible_collections/community/aws
╰╴ $ ansible-test integration --docker centos8 -v aws_acm
Falling back to tests in "tests/integration/targets/" because "roles/test/" was not found.
WARNING: Excluding tests marked "unstable" which require --allow-unstable or prefixing with "unstable/": aws_acm
WARNING: Excluding tests marked "cloud/aws" which require config (see "/home/matthew/.local/lib/python3.8/site-packages/ansible_test/config/cloud-config-aws.ini.template"): aws_acm
WARNING: All targets skipped.

@tremble
Copy link
Contributor

tremble commented Mar 16, 2021

Two changes needed:

  1. add the --allow-unstable flag to the command
  2. add a cloud config file (telling ansible-test what credentials to use to connect to AWS) needs to be at tests/integration/cloud-config-aws.ini

I use the following (credentials redacted):

$ cat ansible_collections/community/aws/tests/integration/cloud-config-aws.ini 
[default]
aws_access_key: AKIAXXXX-REDACTED-XXXXX
aws_secret_key: XXXXXX-REDACTED-XXXXXXXX
#security_token: @SECURITY_TOKEN
aws_region: us-east-1

ec2_access_key: {{ aws_access_key }}
ec2_secret_key: {{ aws_secret_key }}
ec2_region: {{ aws_region }}

@tremble
Copy link
Contributor

tremble commented Mar 16, 2021

The README.md for this collection also includes some resources around getting the environment setup:
https://github.com/ansible-collections/community.aws#more-information-about-contributing

@jillr's blog post: https://www.ansible.com/blog/getting-started-with-aws-ansible-module-development includes a walk through.

@mdavis-xyz
Copy link
Contributor Author

Ah I see. There's a bug in ansible-test. It told me to save that config file in the wrong location.

@mdavis-xyz
Copy link
Contributor Author

My integration tests are failing locally with a different error to CI. It seems the community.crypto.openssl_privatekey module is not installed.

Why do I get a different error locally vs in CI? If I'm running tests in docker, shouldn't the dependencies be the same?

ansible-test integration --docker centos8 -v aws_acm
TASK [aws_acm : include_tasks] *************************************************                                                       
fatal: [testhost]: FAILED! => {"reason": "couldn't resolve module/action 'community.crypto.openssl_privatekey'. This often indicates a m
isspelling, missing collection, or incorrect module path.\n\nThe error appears to be in '/root/ansible/ansible_collections/community/aws/tests/output/.tmp/integration/aws_acm-2zn07z0p-ÅÑŚÌβŁÈ/tests/integration/targets/aws_acm/tasks/full_acm_test.yml': line 59, column 5, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n      - remote_tmp_dir is defined\n  - name: Generate private key for local certs\n    ^ here\n"}                                                               
[WARNING]: Failure using method (v2_runner_on_failed) in callback plugin                          
(<ansible.plugins.callback.junit.CallbackModule object at 0x7f52772d4518>): /ro
ot/ansible/ansible_collections/community/aws/tests/output/.tmp/integration/aws_                                                         
acm-2zn07z0p-ÅÑŚÌβŁÈ/tests/integration/targets/aws_acm/tasks/main.yml:32:
testhost: aws_acm : include_tasks _raw_params=full_acm_test.yml: duplicate host
callback: testhost

@tremble
Copy link
Contributor

tremble commented Mar 16, 2021

Some of the tests need additional modules (including the acm tests) the quickest way is to clone the community.crypto repo next to the community.aws repo. There are some extra wrappers in the main CI account which ensure the dependencies are pulled in from galaxy: https://github.com/ansible-collections/community.aws/blob/main/tests/requirements.yml these haven't been built into ansible-test yet.

@mdavis-xyz
Copy link
Contributor Author

mdavis-xyz commented Mar 17, 2021

Ok, now I'm getting this error:

TASK [aws_acm : list certs] ****************************************************
fatal: [testhost]: FAILED! => {"msg": "Could not find imported module support code for aws_acm_info.  Looked for either AnsibleAWSModule.py or core.py"}

When running from software/ansible/collections/ansible_collections/community/aws with COLLECTIONS_PATHS set to software/ansible/collections

Note that @jillr 's blog post contains instructions which don't work for me.

The instructions say:

ansible-test integration ec2_group

So I tried with:

ansible-test integration aws_acm

which gives:

ERROR: Rename "/home/matthew/.aws" or use the --docker or --remote option to isolate tests.

@tremble
Copy link
Contributor

tremble commented Mar 17, 2021

That looks like it failed to find the amazon.aws collection. (community.aws depends on amazon.aws)

@mdavis-xyz
Copy link
Contributor Author

Isn't aws_acm_info in the community.aws collection, not amazon.aws?

https://github.com/ansible-collections/community.aws/blob/main/plugins/modules/aws_acm_info.py

@tremble
Copy link
Contributor

tremble commented Mar 17, 2021

It is, however the "module_utils" code that it relies on (the core.py and AnsibleAwsModule it referred to) lives in amazon.aws

@tremble tremble merged commit ae19e71 into ansible-collections:main Mar 18, 2021
@tremble
Copy link
Contributor

tremble commented Mar 18, 2021

@mdavis-xyz Thanks for working on this, I'm sorry it's been a bit of a battle to get the test-suite up and running.

The test failures look like they were flakes, re-running them they pass, and the tests are still flagged as 'unstable'. We've seen some APIs return inconsistent results and I suspect that's what we saw here.

@mdavis-xyz
Copy link
Contributor Author

I actually had a few bug fixes I hadn't pushed yet. I think the module itself is fine, but there were bugs in the test. (e.g. I actually ended up testing check mode in aws_acm_info, which doesn't seem to work as expected.) And the rest can be fixed with retry backoff logic.

Should I open another pull request once I've got them working?

@tremble
Copy link
Contributor

tremble commented Mar 18, 2021 via email

danquixote pushed a commit to danquixote/community.aws that referenced this pull request May 16, 2021
* add integration tests for acm with check mode
* add check mode to acm module
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* add integration tests for acm with check mode
* add check mode to acm module
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* add integration tests for acm with check mode
* add check mode to acm module
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
* add integration tests for acm with check mode
* add check mode to acm module
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
…-collections#601)

ec2_vpc_route_table: Add IPv6 support in ec2_vpc_route_table

SUMMARY

Allow usage of IPv6 CIDR in ec2_vpc_route_table for creating routes.

Fixes ansible-collections#477
ISSUE TYPE


Feature Pull Request

COMPONENT NAME

ec2_vpc_route_table

Reviewed-by: Jill R <None>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug integration tests/integration module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) python3 tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ACM module has no "check" mode
4 participants