Compare tags using common tag type #399
Conversation
| @@ -333,12 +340,8 @@ func TestCompareResource_Lambda_Function(t *testing.T) { | |||
| delta.Add("Spec.Runtime", a.ko.Spec.Runtime, b.ko.Spec.Runtime) | |||
| } | |||
| } | |||
| if ackcompare.HasNilDifference(a.ko.Spec.Tags, b.ko.Spec.Tags) { | |||
| if !ackcompare.MapStringStringEqual(ToACKTags(a.ko.Spec.Tags), ToACKTags(b.ko.Spec.Tags)) { | |||
There was a problem hiding this comment.
In what package namespace is ToACKTags?
There was a problem hiding this comment.
It's part of the generated code within each resource https://github.com/aws-controllers-k8s/ec2-controller/blob/main/pkg/resource/vpc/tags.go
There was a problem hiding this comment.
Oh, duh, yeah I forgot about that. I thought for a second it was in runtime's pkg/tags
Extends aws-controllers-k8s/code-generator#399 Description of changes: Adds a `GetTagsDifference` helper method that can be used to determine which tags should be added and removed based on what they are going "from" and "to" - typically needed for services with `TagResource` and `UntagResource` operations. For example: ```go aTags := ToACKTags(a.ko.Spec.Tags) bTags := ToACKTags(b.ko.Spec.Tags) added, _, removed := ackcompare.GetTagsDifference(aTags, bTags) // optionally, convert back to service Tag type toAdd := FromACKTags(added) toRemove := FromACKTags(removed) ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: A-Hilaly, jaypipes, RedbackThomson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -25,3 +25,6 @@ resources: | |||
| in: | |||
| - 1 | |||
| - 2 | |||
| CodeSigningConfig: | |||
| tags: | |||
| ignore: true | |||
There was a problem hiding this comment.
For my own edification: why is this necessary here for Lambda.CodeSigningConfig but not for other applicable test cases like MemoryDB.User?
There was a problem hiding this comment.
CodeSigningConfig doesn't support tags in its create call - https://docs.aws.amazon.com/sdk-for-go/api/service/lambda/#CreateCodeSigningConfigInput - and for the purposes of these tests we aren't adding the custom field to support it
Implements aws-controllers-k8s/community#1658
Description of changes:
Instead of comparing tag fields using the standard
reflect.DeepEquals, this PR updates the code-generator to convert the tags to the commonacktags.Tagtype and then compare those as maps. This ensures that changes in ordering do not result in false positive deltas.Controllers that rely on custom tag delta functions also typically use a function to determine which tags are being added and which are being removed. Using the same trick as this PR, a method can be added to the common runtime to deduce those - resulting in far less duplicated and verbose custom hook code. Those changes are not included in this PR, though.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.