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

fix error in module iam_server_certificate for _compare_cert #2104

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andrewjroth
Copy link

SUMMARY

While using the module iam_server_certificate, I discovered that the module does not properly compare an existing certificate with the passed value. This happens because the str.replace method does not replace the variable in-place, but returns the string after replacement.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

iam_server_certificate

ADDITIONAL INFORMATION

To reproduce the error, run the module or include it in a playbook. Assuming the module's arguments remain the same, the first run of the playbook will succeed. The second run will fail with the message "Modifying the certificate body is not supported by AWS". The expected result should be a success, not changed.

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/6b1b7d4f56dc4a6998d15667479dc96e

ansible-galaxy-importer FAILURE in 4m 35s (non-voting)
✔️ build-ansible-collection SUCCESS in 12m 39s
✔️ ansible-test-splitter SUCCESS in 4m 57s
✔️ integration-community.aws-1 SUCCESS in 5m 20s
Skipped 21 jobs

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.

Hi @andrewjroth,

Thanks for taking the time to submit this PR. Two things,

  1. To help avoid regressions, please could you add an integration test to either this PR or Add Support for Updating in iam_server_certificate by Deleting and Creating #2105 (tests/integration/targets/iam_server_certificate)
  2. please add a changelog entry https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to

@andrewjroth
Copy link
Author

andrewjroth commented Jun 17, 2024

@tremble Looking at the existing integration tests, I feel that they might already cover this.

Before making any changes, when attempting to run the integration tasks locally, I get an error...

I run:

$ ansible-test integration --docker

... and get ...

ERROR! couldn't resolve module/action 'community.crypto.openssl_privatekey'. This often indicates a misspelling, missing collection, or incorrect module path.

The error appears to be in '/root/ansible_collections/community/aws/tests/output/.tmp/integration/acm_certificate-tfx8pg2l-ÅÑŚÌβŁÈ/tests/integration/targets/acm_certificate/tasks/main.yml': line 51, column 5, but may
be elsewhere in the file depending on the exact syntax problem.

This is using commit 9c66d1e4 -- Bump release for main branch to 9.0.0-dev0 (#2091)

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

Successfully merging this pull request may close these issues.

2 participants