Addition of ECS_Certificate Module

[ctrufan]

SUMMARY

Addition of a module to submit certificate requests to the Entrust Certificate Services API.

Supports various entrust specific parameters, such as specifying client ID or tracking information such as additional emails or custom fields.

If the certificate in 'path' is present but not an entrust certificate, it will be treated as not valid - expectation is that this allows for transitioning from self-signed/etc to Entrust issued certificates.

If the certificate present in path but is an Entrust certificate, whether or not a new one is requested is based on the days remaining and the value of 'force', similarly to how both openssl_certificate and acme_certificate behave.

The "tracking_id" parameter is the value to uniquely identify certificates in the ECS system, and is something customers would be familiar with if they needed to use it.

The ECS API supports two operations on existing certificates, 'RENEW' and 'REISSUE'. If the tracking_id parameter is used, this module can be used to 'adopt' an existing ECS certificate for management under Ansible - specifying a tracking_id means that it will download that certificate to path if it is a valid certificate, and if it is within remaining_days of expiration or the 'force' command is used, it can be reissued or renewed. This design decision was made to make it easier to migrate existing infrastructure to ansible, as many customers have internal setups that already keep track of the tracking_id of existing certificates that they could pass to an ansible playbook.

ISSUE TYPE

• New Module Pull Request

COMPONENT NAME

crypto
ecs_certificate
entrust/api

ADDITIONAL INFORMATION

Integration Tests

Tests run against VMs set up with Python 2.7 and Python 3.6. The reason the attached playbook doesn't run as 'localhost' was specifically to test behavior on different endpoint environments.

Attached are the results of the following integration tests running against our QA environment. Once merged these tests will run nightly against both ansible/devel and any baseline releases that include this module, against both our "next release" of ECS and the "current release". If needed I can try to adapt them (e.g. remove any sensitive values) for inclusion in ansible code base, though I'll note that customers and other ansible developers cannot access our internal QA environment, they can only submit against our actual CA, so testing would need to involve owned domains and actual ECS accounts, and unless the account was configured otherwise, would issue genuine publicly trusted certificates.

I am going to clean up the integration tests additionally (migrate the 'fail' calls into distinct pytest methods that check data against our database as well).

Integration Tests:

• Have ECS generate a signed certificate for us using a local file path for the specification
• Verify that ECS will not mark 'changed' if calling a still valid file
• Verify that ECS will force a new request even if current is still valid, and REISSUE works with no CSR.
• Test that ECS will download an existing older cert if tracking_id specified, but will not perform action if still valid.
• Perform an actual renew based on tracking ID.
• Request a certificate with all various parameters
• Request a certificate with client ID other than 1
  ecs_certificate_playbook.txt
  ecs_certificate_output.txt

[ansibot]

@ctrufan this PR contains the following merge commits:

• 2fd707c

Please rebase your branch to remove these commits.

click here for bot help

[ansibot]

cc @MarkusTeufelberger @Shaps @Spredzy @Xyon @felixfontein @puiterwijk @resmo
click here for bot help

[ansibot]

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

.github/BOTMETA.yml:0:0: expected str for dictionary value @ data['files']['$module_utils/ecs/']. Got None

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 1 error:

lib/ansible/plugins/doc_fragments/ecs_credential.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

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

lib/ansible/plugins/doc_fragments/ecs_credential.py:0:0: missing: __metaclass__ = type

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

lib/ansible/plugins/doc_fragments/ecs_credential.py:0:0: should not have a shebang

The test ansible-test sanity --test validate-modules [explain] failed with 5 errors:

lib/ansible/modules/crypto/entrust/ecs_certificate.py:0:0: E325 Argument 'number1' in argument_spec found in custom_fields defines type as 'int' but documentation defines type as 'float'
lib/ansible/modules/crypto/entrust/ecs_certificate.py:0:0: E325 Argument 'number2' in argument_spec found in custom_fields defines type as 'int' but documentation defines type as 'float'
lib/ansible/modules/crypto/entrust/ecs_certificate.py:0:0: E325 Argument 'number3' in argument_spec found in custom_fields defines type as 'int' but documentation defines type as 'float'
lib/ansible/modules/crypto/entrust/ecs_certificate.py:0:0: E325 Argument 'number4' in argument_spec found in custom_fields defines type as 'int' but documentation defines type as 'float'
lib/ansible/modules/crypto/entrust/ecs_certificate.py:0:0: E325 Argument 'number5' in argument_spec found in custom_fields defines type as 'int' but documentation defines type as 'float'

click here for bot help

[ansibot]

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 1 error:

lib/ansible/plugins/doc_fragments/ecs_credential.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

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

lib/ansible/plugins/doc_fragments/ecs_credential.py:0:0: missing: __metaclass__ = type

click here for bot help

[ansibot]

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

lib/ansible/plugins/doc_fragments/ecs_credential.py:9:1: E302 expected 2 blank lines, found 1

click here for bot help

[felixfontein]

I didn't look at the implementation, just at the documentation so far.

BTW, all new modules should have tests of some kind (either unit tests, or integration tests, even if they are marked as unsupported, i.e. they are not run via CI).

[ansibot]

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/crypto/entrust/ecs_certificate.py:532:0: anomalous-backslash-in-string Anomalous backslash in string: '\Z'. String constant might be missing an r prefix.
lib/ansible/modules/crypto/entrust/ecs_certificate.py:534:0: anomalous-backslash-in-string Anomalous backslash in string: '\Z'. String constant might be missing an r prefix.

The test ansible-test sanity --test import --python 3.8 [explain] failed with 2 errors:

lib/ansible/modules/crypto/entrust/ecs_certificate.py:532:0: SyntaxWarning: invalid escape sequence \Z
lib/ansible/modules/crypto/entrust/ecs_certificate.py:534:0: SyntaxWarning: invalid escape sequence \Z

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/crypto/entrust/ecs_certificate.py:532:91: W605 invalid escape sequence '\Z'
lib/ansible/modules/crypto/entrust/ecs_certificate.py:534:108: W605 invalid escape sequence '\Z'

click here for bot help

[ctrufan]

@felixfontein
I've been using the "mark resolved" to internally mark when I fixed something, but it occurred to me maybe that's typically something the reviewer does to indicate that their comments are addressed? My apologies if I've been a bit backwards on the process :).

[felixfontein]

@felixfontein
I've been using the "mark resolved" to internally mark when I fixed something, but it occurred to me maybe that's typically something the reviewer does to indicate that their comments are addressed? My apologies if I've been a bit backwards on the process :).

No worries, that's totally fine! (Well, at least as long as you're very sure the comment is really addressed. If you're not sure, don't resolve it.)

[felixfontein]

Found a few nits, besides that, I think everything is fine!

The only thing that is missing are tests (integration tests marked as unsupported are also fine).

If you can add them today, we should be able to get this new module in for Ansible 2.9.

[ctrufan]

For running the integration tests, I couldn't for the life of me get it to pick up on an integration_config.yml file - since I figured it was likely a config issue and not something that would require code changes, I ran tests with:

cat integration_config.yml >> test/integration/targets/ecs_certificate/vars/main.yml
ansible-test integration unsupported/ecs_certificate | tee ecs_certificate_test_output.txt

Contents of integration_config are:

entrust_api_user: CENSORED
entrust_api_key: CENSORED
entrust_api_ip_address: CENSORED
entrust_cloud_ip_address: CENSORED
entrust_api_client_cert_path: /var/integration-testing/publicCert.pem
entrust_api_client_cert_key_path: /var/integration-testing/privateKey.pem
cacerts_bundle_path_local: /var/integration-testing/cacerts

Attached is the file output:
ecs_certificate_test_output.txt

[felixfontein]

In case the tests modify the system, you want to add --docker ubuntu1804 or something similar to the ansible-test integration command line (https://docs.ansible.com/ansible/latest/dev_guide/testing_integration.html#container-images). Then it won't screw up your system, but only a ephemeral container :)

[felixfontein]

Looks good. Just a few minor optional points. Also, tests for check mode would be nice. But I'm totally fine with adding them in a later PR.

[ctrufan]

In case the tests modify the system, you want to add --docker ubuntu1804 or something similar to the ansible-test integration command line (https://docs.ansible.com/ansible/latest/dev_guide/testing_integration.html#container-images). Then it won't screw up your system, but only a ephemeral container :)

I was running in a VM already, so there were no worries there. This does bring up an issue with the integration tests though - these ones can't run with --docker, because they need access to the cert/key files, and since the entire playbook runs from within the docker container, there's no way to get them to it (our internal molecule tests solve this by copying from the host to the docker VMs, as well as doing API calls delegated to localhost).

Unless I'm missing something in the docker behavior, there's no way to push data from the host to the container, so I might have to refactor the variable to take in the full PEM string of cert/key, then write them to disk.

[felixfontein]

Ah, in that case, don't ;-)

[felixfontein]

@ctrufan thanks for contributing this module!

[ctrufan]

@felixfontein

Thank you for the reviews and input! Very appreciated, especially getting it in before 2.8.