Remove unused cloudformation parameter from image copier stack#1801
Remove unused cloudformation parameter from image copier stack#1801philmcmahon wants to merge 4 commits intomainfrom
Conversation
cdk/lib/image-copier-lambda.ts
Outdated
| const kmsKeyArn = | ||
| kmsKeyArnParameter.valueAsString.length > 0 | ||
| ? kmsKeyArnParameter.valueAsString | ||
| : Fn.importValue('amigo-imagecopier-key'); |
There was a problem hiding this comment.
I don't think this ternary will work as the value of kmsKeyArnParameter is known at runtime, however this check is evaluated at compile time.
I think we could drop Fn.importValue('amigo-imagecopier-key') entirely TBH. Do we know what creates the image copier key? Could it also create a SSM Parameter on a known path, and that becomes the default of the CFN Parameter in this stack?
There was a problem hiding this comment.
Ah yes sorry, that is very obvious in the snapshots that it just just replaced one thing with another. The image copier key is created in another stack within this repo, I could definitely update it to do as you suggest
There was a problem hiding this comment.
(will do that then submit for review again)
There was a problem hiding this comment.
I've ended up just removing the parameter entirely
philmcmahon
changed the title
philmcmahon
force-pushed
the
pm-support-kms-key-override
branch
from
a73a172 to
0be0a81
Compare
…udformation parameter" This reverts commit be35361.
philmcmahon
force-pushed
the
pm-support-kms-key-override
branch
from
0be0a81 to
2fb6d48
Compare
3 participants
What does this change?
Somewhere along the road of the CDK migration (I'm assuming) we stopped respecting the KmsKeyArn cloudformation parameter in the image copier CDK. I initially set about to get it working again, but, because I was scared of pushing out a change that would impact teams who had unknowingly set that parameter, I've instead removed it entirely.
How to test
This should be a noop, I've tested it on the investigations account