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

Adding aws_acm_export_certificates #58689

Open
wants to merge 8 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@diodonfrost
Copy link

commented Jul 3, 2019

SUMMARY

PR to introduce aws_acm_export_certificates module.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

aws_acm_export_certificates

ADDITIONAL INFORMATION

This module allow to export private certificates from aws certificates manager via the Ansible framework.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

@diodonfrost, just so you are aware we have a dedicated Working Group for aws.
You can find other people interested in this in #ansible-aws on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_acm_export_certificates.py:138:0: missing-final-newline Final newline missing

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_acm_export_certificates.py:138:11: W292 no newline at end of file

click here for bot help

@ansibot ansibot removed the ci_verified label Jul 3, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

@BobBoldin @Constantin007 @Constantin07 @Deepakkothandan @Etherdaemon @Java1Guy @Madhura-CSI @MichaelBaydoun @Sodki @Zeekin @adq @akazakov @alachaum @amir343 @anryko @bekelchik @brandond @captainkerk @chenl87 @defionscode @defunctio @dennisconrad @dkhenry @fiunchinho @fivethreeo @flowerysong @garethr @gobins @gunzy83 @gurumaia @hsingh @hyperized @iiibrad @infectsoldier @j-carl @jarv @jimbydamonk @jmenga @joelthompson @jonhadfield @jonmer85 @joshsouza @jsdalton @jsmartin @kaczynskid @leedm777 @linuxdynasty @loia @lwade @michaeljs1990 @minichate @mjschultz @mmochan @nand0p @naslanidis @nathanwebsterdotme @nerzhul @nickball @orthanc @ozbillwang @piontas @pjodouin @prasadkatti @psykotox @ptux @pwnall @raags @rafaeldriutti @rickmendes @roadmapper @rrey @ruimoreira @ryansydnor @scicoin-project @scottanderson42 @sdubrul @shepdelacreme @silviud @slapula @stefanhorning @steynovich @tastychutney @tgerla @timmahoney @tomislacker @tsiganenok @viper233 @whiter @willricardo @wilvk @wimnat @yaakov-github @zacblazic @zbal @zimbatm

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@willthames
Copy link
Contributor

left a comment

Still a few minor tweaks but overall this is looking very good.

@willthames

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

This will also require a test suite (see test/integration/targets for existing ones) - it doesn't have to do much, just enough that we can validate that the module actually works. You may need to use the command module with the aws cli to create a certificate as we don't seem to have a module for creating acm certificates.

@diodonfrost

This comment has been minimized.

Copy link
Author

commented Jul 11, 2019

AWS acm command export-certificate only work with private certificates

Private certificates can only be requested by AWS Certificate Manager Private Certificate Authority

I can make a test suite but you need a ACM PCA for it to work.

exception=traceback.format_exc(),
**camel_dict_to_snake_dict(error.response))

module.exit_json(certificates=camel_dict_to_snake_dict(response))

This comment has been minimized.

Copy link
@willthames

willthames Jul 12, 2019

Contributor

This doesn't match docs and should still be singular certificate

This comment has been minimized.

Copy link
@willthames

willthames Jul 12, 2019

Contributor

Might also be better to just module.exit_json(**camel_dict_to_snake_dict(response))

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 12, 2019

Author
     try:
         response = connection.export_certificate(**kwargs)
     except (ClientError, ParamValidationError) as error:
-        module.fail_json(msg="Couldn't obtain private certificates",
-                         exception=traceback.format_exc(),
-                         **camel_dict_to_snake_dict(error.response))
+        module.fail_json_aws(error, msg="Couldn't obtain private certificate")

-    module.exit_json(certificates=camel_dict_to_snake_dict(response))
+    module.exit_json(**camel_dict_to_snake_dict(response))

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 17, 2019

Author

refactor module.exit_json



def main():
argument_spec = ec2_argument_spec()

This comment has been minimized.

Copy link
@willthames

willthames Jul 12, 2019

Contributor

not needed with AnsibleAWSModule

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 12, 2019

Author
 def main():
-    argument_spec = ec2_argument_spec()
-    argument_spec.update(
-        dict(
-            certificate_arn=dict(required=True),
-            passphrase=dict(required=True)
-        )
+    argument_spec = dict(
+        certificate_arn=dict(required=True),
+        passphrase=dict(required=True)
     )
-
-    module = AnsibleAWSModule(argument_spec=argument_spec)
+
+    module = AnsibleAWSModule(argument_spec=argument_spec, supports_check_mode=False,)
     connection = module.client('acm')

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 17, 2019

Author

remove argument_spec = ec2_argument_spec()

try:
response = connection.export_certificate(**kwargs)
except (ClientError, ParamValidationError) as error:
module.fail_json(msg="Couldn't obtain private certificates",

This comment has been minimized.

Copy link
@willthames

willthames Jul 12, 2019

Contributor

Use module.fail_json_aws(error, msg="Couldn't obtain private certificates")

# Exporting a private certificate and private key (more details: https://docs.aws.amazon.com/acm/latest/userguide/gs-acm-export-private.html#export-cli)
- name: obtain specific privates certificates
aws_acm_export_certificates:

This comment has been minimized.

Copy link
@willthames

willthames Jul 12, 2019

Contributor

Rename the module to be singular as it only exports one certificate

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 12, 2019

Author
DOCUMENTATION = '''
 ---
-module: aws_acm_export_certificates
+module: aws_acm_export_certificate
 short_description: Exports a private certificate issued by a private certificate authority (CA) for use anywhere
 description:
     - Exports a private certificate issued by a private certificate authority (CA) for use anywhere
@@ -42,22 +42,26 @@ EXAMPLES = '''
 # Note: These examples do not set authentication details, see the AWS Guide for details.
 # Exporting a private certificate and private key (more details: https://docs.aws.amazon.com/acm/latest/userguide/gs-acm-export-private.html#export-cli)

-- name: obtain specific privates certificates
-  aws_acm_export_certificates:
+- name: obtain specific privates certificate
+  aws_acm_export_certificate:
     certificate_arn: "arn:aws:acm:us-east-1:123456789012:certificate/12345678-1234-1234-1234-123456789012"
     passphrase: secretpassword
-  register: acm_certificates
+  register: acm_certificate

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 17, 2019

Author

rename module to aws_acm_export_certificate

'''

RETURN = '''
export:

This comment has been minimized.

Copy link
@willthames

willthames Jul 12, 2019

Contributor

As suggested below, maybe make certificate, certificate_chain and private_key top level return values

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 12, 2019

Author
RETURN = '''
-export:
-    description: Export the certificate, the certificate chain, and the encrypted private key associated with the public key embedded in the certificate
+certificate:
+    Description: The base64 PEM-encoded certificate
     returned: always
-    type: dict
-    sample:
-      certificate: XXXXXXXXXXXXXXXXXXXX
-      certificate_chain: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
-      private_key: XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
+    type: string
+certificate_chain:
+    Description: The base64 PEM-encoded certificate chain. This does not include the certificate that you are exporting
+    returned: always
+    type: string
+private_key:
+    Description: The encrypted private key associated with the public key in the certificate. The key is output in PKCS #8 format and is base64 PEM-encoded
+    returned: always
+    type: string

This comment has been minimized.

Copy link
@diodonfrost

diodonfrost Jul 17, 2019

Author

return certificate, certificate_chain and private_key

diodonfrost added some commits Jul 12, 2019

fix doc and issues
-   Use singular certificate name
-   Refactoring `module.exit_json`
-   Removing `argument_spec`
-   Fix doc
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_acm_export_certificate.py:113:0: trailing-newlines Trailing newlines

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_acm_export_certificate.py:113:1: W391 blank line at end of file

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_acm_export_certificate.py:112:0: E109 Next to last line should be: if __name__ == "__main__":

click here for bot help

@ansibot ansibot added the ci_verified label Jul 12, 2019

@ansibot ansibot removed the ci_verified label Jul 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.