Skip to content

Conversation

dfangl
Copy link
Member

@dfangl dfangl commented May 11, 2023

Motivation

#8253 introduced a regression for some methods the key arn in the KeyId field instead of the ID.

The API documentation for (at least one) of the operation clearly states it will return the ARN in that field: https://docs.aws.amazon.com/kms/latest/APIReference/API_GenerateDataKey.html#API_GenerateDataKey_ResponseSyntax

Changes

  • Add snapshot validated tests for the generate_data_key* methods
  • Fix the return value

This is mainly done because the aws_encryption_sdk depends on the format being an ARN, which breaks our -ext pipeline.

Future work

@dfangl dfangl requested a review from sannya-singal May 11, 2023 12:45
@dfangl dfangl added the semver: patch Non-breaking changes which can be included in patch releases label May 11, 2023
@github-actions
Copy link

LocalStack Community integration with Pro

2 037 tests   1 756 ✔️  1h 22m 2s ⏱️
       2 suites     281 💤
       2 files           0

Results for commit e8d0ca4.

@coveralls
Copy link

Coverage Status

Coverage: 82.285% (-0.008%) from 82.293% when pulling e8d0ca4 on fix/kms-generate-keyid into 815c399 on master.

@sannya-singal
Copy link
Contributor

Thanks for tacking this @dfangl! 🎉

LGTM! Only a minor nit with the transformer.

@dfangl dfangl merged commit 539334c into master May 11, 2023
@dfangl dfangl deleted the fix/kms-generate-keyid branch May 11, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants