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

Add support for requesting public and private ACM certificate #869

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

Conversation

sebastien-rosset
Copy link
Contributor

@sebastien-rosset sebastien-rosset commented Jan 15, 2022

SUMMARY
  1. Add new certificate_request parameter (and sub-options) to obtain a public or private cert from ACM.
  2. When certificate_request parameter is specified:
    1. Evaluate whether to create a new certificate request, renew or do nothing.
    2. Support public and private certs.
    3. Certificate request is submitted to ACM.
  3. Task output includes validation records, such as DNS CNAME records.
  4. Add wait and wait_timeout parameters. If wait is specified, wait until the validation records are generated, then return them.

Fixes #868

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

aws_acm

ADDITIONAL INFORMATION
  1. Currently the aws_acm module only supports imported certificates. This PR is adding support for requesting a certificate.
  2. This PR should come after Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests #870. To leverage the refactor work that was done in Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests #870, I have rebased this PR from the acm-tags branch.
  3. The integration tests validate various scenarios for public certificates.
  4. Integration tests do not currently include validation of private certificates. This would require having another ansible module that can create/delete private certificate authorities. I.e. a pre-requisite for issuing private ACM certs is to have a private certificate authority. I would like to postpone these tests until such a module exist.

@sebastien-rosset sebastien-rosset changed the title Add support for requesting ACM certificate Add support for requesting public and private ACM certificate Jan 20, 2022
@sebastien-rosset sebastien-rosset marked this pull request as ready for review January 20, 2022 02:42
@ansibullbot
Copy link

@sebastien-rosset this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request merge_commit This PR contains at least one merge commit. Please resolve! module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_triage labels Jan 27, 2022
@ansibullbot ansibullbot added the new_contributor Help guide this first time contributor label Jan 27, 2022
@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review integration tests/integration plugins plugin (any type) tests tests and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Jan 28, 2022
@ansibullbot ansibullbot removed the new_contributor Help guide this first time contributor label Jan 30, 2022
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Mar 31, 2022
… integration tests (#870)

Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests

SUMMARY

This PR adds support for configuring arbitrary tags when importing a certificate using the aws_acm module. Previously, it was only possible to set the 'Name' tag.
Additionally, this PR fixes issues with the aws_acm integration tests.  The integration tests were using deprecated tasks or attributes, such as openssl_certificate.

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

aws_acm
ADDITIONAL INFORMATION


Changes to the aws_acm.py module:

Add new tags and purge_tags attributes.
The certificate_arn attribute is now allowed when state='present'. A playbook should be allowed to modify an existing certificate entry by providing the ARN. For example, a play may want to add, modify, remove tags on an existing certificate.
The aws_acm module returns the updated tags. See example below.
Refactor aws_acm.py to improve code reuse and make it possible to set arbitrary tags. This should also help to 1) improve readability. 2) prepare for #869 which I am planning to work on next.

Backwards-compatibility is retained, even though it might make sense to normalize some of the attributes.
Example return value:
"certificate": {
            "arn": "arn:aws:acm:us-west-1:account:certificate/f85abf9d-4bda-4dcc-98c3-770664a68243",
            "domain_name": "acm1.949058644.ansible.com",
            "tags": {
                "Application": "search",
                "Environment": "development",
                "Name": "ansible-test-78006277-398b5796f999_949058644_1"
            }
        }

Integration tests:

The openssl_certificate task is deprecated. Migrate to x509_certificate.
The signature_algorithms attribute is no longer supported by the new x509_certificate task. Using selfsigned_digest instead.
The integration tests for the aws_acm module pass locally.
I see ansible/ansible#67788 has been closed, but tests/integration/targets/aws_acm/aliases still has unstable. I am not sure what to do about it. I was able to run the tests in my local workspace after making the above changes.

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Sebastien Rosset <None>
Reviewed-by: Mark Woolley <[email protected]>
Reviewed-by: Alina Buzachis <None>
patchback bot pushed a commit that referenced this pull request Mar 31, 2022
… integration tests (#870)

Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests

SUMMARY

This PR adds support for configuring arbitrary tags when importing a certificate using the aws_acm module. Previously, it was only possible to set the 'Name' tag.
Additionally, this PR fixes issues with the aws_acm integration tests.  The integration tests were using deprecated tasks or attributes, such as openssl_certificate.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

aws_acm
ADDITIONAL INFORMATION

Changes to the aws_acm.py module:

Add new tags and purge_tags attributes.
The certificate_arn attribute is now allowed when state='present'. A playbook should be allowed to modify an existing certificate entry by providing the ARN. For example, a play may want to add, modify, remove tags on an existing certificate.
The aws_acm module returns the updated tags. See example below.
Refactor aws_acm.py to improve code reuse and make it possible to set arbitrary tags. This should also help to 1) improve readability. 2) prepare for #869 which I am planning to work on next.

Backwards-compatibility is retained, even though it might make sense to normalize some of the attributes.
Example return value:
"certificate": {
            "arn": "arn:aws:acm:us-west-1:account:certificate/f85abf9d-4bda-4dcc-98c3-770664a68243",
            "domain_name": "acm1.949058644.ansible.com",
            "tags": {
                "Application": "search",
                "Environment": "development",
                "Name": "ansible-test-78006277-398b5796f999_949058644_1"
            }
        }

Integration tests:

The openssl_certificate task is deprecated. Migrate to x509_certificate.
The signature_algorithms attribute is no longer supported by the new x509_certificate task. Using selfsigned_digest instead.
The integration tests for the aws_acm module pass locally.
I see ansible/ansible#67788 has been closed, but tests/integration/targets/aws_acm/aliases still has unstable. I am not sure what to do about it. I was able to run the tests in my local workspace after making the above changes.

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Sebastien Rosset <None>
Reviewed-by: Mark Woolley <[email protected]>
Reviewed-by: Alina Buzachis <None>
(cherry picked from commit 29d37be)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Mar 31, 2022
… integration tests (#870) (#1044)

[PR #870/29d37bed backport][stable-3] Add support for tagging certificates. Fix deprecated tasks in aws_acm integration tests

This is a backport of PR #870 as merged into main (29d37be).
SUMMARY

This PR adds support for configuring arbitrary tags when importing a certificate using the aws_acm module. Previously, it was only possible to set the 'Name' tag.
Additionally, this PR fixes issues with the aws_acm integration tests.  The integration tests were using deprecated tasks or attributes, such as openssl_certificate.

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

aws_acm
ADDITIONAL INFORMATION


Changes to the aws_acm.py module:

Add new tags and purge_tags attributes.
The certificate_arn attribute is now allowed when state='present'. A playbook should be allowed to modify an existing certificate entry by providing the ARN. For example, a play may want to add, modify, remove tags on an existing certificate.
The aws_acm module returns the updated tags. See example below.
Refactor aws_acm.py to improve code reuse and make it possible to set arbitrary tags. This should also help to 1) improve readability. 2) prepare for #869 which I am planning to work on next.

Backwards-compatibility is retained, even though it might make sense to normalize some of the attributes.
Example return value:
"certificate": {
            "arn": "arn:aws:acm:us-west-1:account:certificate/f85abf9d-4bda-4dcc-98c3-770664a68243",
            "domain_name": "acm1.949058644.ansible.com",
            "tags": {
                "Application": "search",
                "Environment": "development",
                "Name": "ansible-test-78006277-398b5796f999_949058644_1"
            }
        }

Integration tests:

The openssl_certificate task is deprecated. Migrate to x509_certificate.
The signature_algorithms attribute is no longer supported by the new x509_certificate task. Using selfsigned_digest instead.
The integration tests for the aws_acm module pass locally.
I see ansible/ansible#67788 has been closed, but tests/integration/targets/aws_acm/aliases still has unstable. I am not sure what to do about it. I was able to run the tests in my local workspace after making the above changes.

Reviewed-by: Alina Buzachis <None>
@softwarefactory-project-zuul
Copy link
Contributor

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 31, 2022
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…nsible-collections#869)

Rename aws_s3 to s3_object (and deprecate bucket creation/deleting)

SUMMARY
The aws_s3 module (as it's known today) is primarily for managing objects within S3.  While it provides minimal support for creating S3 buckets, the feature set is very limited.  Support for the advanced bucket management features is provided via the s3_bucket modules (such as managing encryption settings).
Because the name aws_s3 often puts the module at the top of the list of modules, well away from the s3_bucket module, it can be difficult for folks to discover the s3_bucket module leading them to assume that we simply have no support for the more complex s3_bucket management features.
As such, I suggest renaming the module to s3_object to make the intended scope more obvious and to improve the discoverability of s3_bucket.  At this time I do not recommend setting a deprecation date for the alias, the cost of an alias is minimal and we've had a lot of churn recently.
Additionally, deprecates the duplicated (but very limited) bucket creation/deletion functionality of aws_s3/s3_object
ISSUE TYPE

Feature Pull Request

COMPONENT NAME

aws_s3
(s3_object)

ADDITIONAL INFORMATION
See for example ansible-collections#866 where there was an attempt to create duplicate functionality.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Jill R <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request integration tests/integration module module needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_acm should support requesting certificate from AWS certificate manager
2 participants