[codegen] adding best practice to update tags#327
[codegen] adding best practice to update tags#327ammokhov wants to merge 1 commit intoaws-cloudformation:masterfrom
Conversation
|
I can see the why we'd want to do this for backwards compatibility of existing resources, but do we really want to enforce this on newly introduced resources? Also, Create handler will have the same issue and if we go the same route, there could be issues with tag based access control. I'd like to hear some others' opinions as well. |
| }) | ||
|
|
||
| // If your resource supports tags, then the following pattern is required to handle resource level tags | ||
| // STEP 5 [update resource level tags progress chain] |
There was a problem hiding this comment.
I'm confused. Is the Step 4 and 5 are the same?
There was a problem hiding this comment.
step 4 has different translate request method - uses only stack tags:
(model) -> Translator.translateToStackTagUpdateRequest(request.getPreviousResourceTags(), request.getDesiredResourceTags())Step 5 has normal request translator - involves previous and current model
.translateToServiceRequest((model) -> Translator.translateToResourceTagUpdateRequest(previousModel, model))
| * Each BaseHandlerException maps to a specific error code, and you should map service exceptions as closely as possible | ||
| * to more specific error codes | ||
| */ | ||
| throw new CfnGeneralServiceException(ResourceModel.TYPE_NAME, e); |
There was a problem hiding this comment.
stack level tags are not always intentional attachment by the customer so we dont really wanna enforce additional permissions when stack has not been changed on update
Yea, I don't think this will be solving the problem of existing resources adding tagging in a backwards incompatible manner. This code will have already been deleted for resources that are adding tagging later. We would also need to do it for create as well to cover replacement update operations |
|
I agree this additional step is confusing to understand and I don't think it's a best practice for resource especially for any new resource and we don't want to encourage to ignore the stack level tag update. Adding a new comments to mention to handler stack level tag properly would be better than this. We should leverage other tools to prevent this happen instead of here. |
4 participants
Issue #, if available:
Description of changes:
adding best practice to the codegen on how to handle tag update:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.