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 for aws_kms_info with external/custom key store keys #311

Merged
merged 3 commits into from
Nov 28, 2020

Conversation

PandaWill
Copy link
Contributor

SUMMARY

aws_kms_info raises an exception when used with a KMS key where the source of key material is external.

botocore.errorfactory.UnsupportedOperationException: An error occurred (UnsupportedOperationException) when calling the GetKeyRotationStatus operation: arn:aws:kms:<key_id> origin is EXTERNAL which is not valid for this operation.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

aws_kms_info

ADDITIONAL INFORMATION

The documentation states that these key types are not supported, https://docs.aws.amazon.com/kms/latest/developerguide/rotate-keys.html#rotate-keys-how-it-works

Unsupported CMK types. Automatic key rotation is not supported on the following types of CMKs, but you can rotate these CMKs manually.

  • Asymmetric CMKs
  • CMKs in custom key stores
  • CMKs that have imported key material

We don't have to handle the case for asymmetric keys because the KMS end-point just returns false. But we do have to handle the external/custom key store key cases:

$ aws kms get-key-rotation-status --key-id <asymmetric_key_id>
{
    "KeyRotationEnabled": false
}

$ aws kms get-key-rotation-status --key-id <external_key_id>
An error occurred (UnsupportedOperationException) when calling the GetKeyRotationStatus operation: arn:aws:kms:<key_id> origin is EXTERNAL which is not valid for this operation.

Note I haven't added a regression test because aws_kms cannot create KMS keys with external key material.

@tremble
Copy link
Contributor

tremble commented Nov 27, 2020

Hi @PandaWill,

Thanks for your contribution, the change looks solid and I've successfully run it through the integration tests to double check that the standard behaviour isn't broken. I agree that we can't easily add an integration test for this specific case, however please could you add a changelog entry: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to

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.

@PandaWill
Copy link
Contributor Author

I've added the change-log fragment - I hope it's looks okay.

Thanks for the speedy review.

@tremble
Copy link
Contributor

tremble commented Nov 28, 2020

Thanks for your submission. For the future, an alternative would have been to catch the UnsupportedOperationException error.

@tremble tremble merged commit 44ad137 into ansible-collections:main Nov 28, 2020
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
…lections#311)

* Fix for aws_kms_info with external/custom key store keys
* Added changelog fragment
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
…lections#311)

* Fix for aws_kms_info with external/custom key store keys
* Added changelog fragment
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
…lections#311)

* Fix for aws_kms_info with external/custom key store keys
* Added changelog fragment
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
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