From be99cf6fdd2b41cd39aec30cf2e0d597d15c0cdc Mon Sep 17 00:00:00 2001 From: Riyad Ilyasov Date: Fri, 6 Jun 2025 14:57:04 +0200 Subject: [PATCH 1/3] Added tag ignore logic --- pkg/controller/rds/dbinstance/setup.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/controller/rds/dbinstance/setup.go b/pkg/controller/rds/dbinstance/setup.go index 027ebf29ad..ea45bc42ab 100644 --- a/pkg/controller/rds/dbinstance/setup.go +++ b/pkg/controller/rds/dbinstance/setup.go @@ -534,7 +534,20 @@ func (e *custom) isUpToDate(ctx context.Context, cr *svcapitypes.DBInstance, out cmpopts.IgnoreFields(svcapitypes.CustomDBInstanceParameters{}, "DeleteAutomatedBackups"), ) - e.cache.addTags, e.cache.removeTags = utils.DiffTags(cr.Spec.ForProvider.Tags, db.TagList) + var observedTags []*svcsdk.Tag + if db.TagList != nil { + for i, tag := range db.TagList { + // ignore system tags + if strings.HasPrefix(*tag.Key, "aws:") || strings.HasPrefix(*tag.Key, "c7n:") { + continue + } + observedTags[i] = &svcsdk.Tag{ + Key: tag.Key, + Value: tag.Value, + } + } + } + e.cache.addTags, e.cache.removeTags = utils.DiffTags(cr.Spec.ForProvider.Tags, observedTags) tagsChanged := len(e.cache.addTags) != 0 || len(e.cache.removeTags) != 0 if diff == "" && !maintenanceWindowChanged && !backupWindowChanged && !iopsChanged && !storageThroughputChanged && !versionChanged && !vpcSGsChanged && !dbParameterGroupChanged && !optionGroupChanged && !tagsChanged { From e09120e0b1e49a80329df764ac91e60d9b6a6e4d Mon Sep 17 00:00:00 2001 From: Riyad Ilyasov Date: Mon, 9 Jun 2025 09:02:57 +0200 Subject: [PATCH 2/3] Changed API --- apis/rds/v1alpha1/custom_types.go | 5 ++ apis/rds/v1alpha1/zz_generated.deepcopy.go | 5 ++ .../rds.aws.crossplane.io_dbinstances.yaml | 7 +++ .../rds/dbinstance/ignore_tags_test.go | 52 +++++++++++++++++++ pkg/controller/rds/dbinstance/setup.go | 10 ++-- pkg/controller/rds/utils/tags.go | 11 ++++ 6 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 pkg/controller/rds/dbinstance/ignore_tags_test.go diff --git a/apis/rds/v1alpha1/custom_types.go b/apis/rds/v1alpha1/custom_types.go index 12736d0e09..9e37b902c4 100644 --- a/apis/rds/v1alpha1/custom_types.go +++ b/apis/rds/v1alpha1/custom_types.go @@ -741,6 +741,11 @@ type CustomDBInstanceParameters struct { // deleted. // +optional DeleteAutomatedBackups *bool `json:"deleteAutomatedBackups,omitempty"` + + // TagIgnorePrefixes tells the reconciler to pretend tags with these + // prefixes don't exist during diff/updates. + // +optional + TagIgnorePrefixes []string `json:"tagIgnorePrefixes,omitempty"` } // CustomDBInstanceObservation includes the custom status fields of DBInstance. diff --git a/apis/rds/v1alpha1/zz_generated.deepcopy.go b/apis/rds/v1alpha1/zz_generated.deepcopy.go index b4e061d18f..b459804463 100644 --- a/apis/rds/v1alpha1/zz_generated.deepcopy.go +++ b/apis/rds/v1alpha1/zz_generated.deepcopy.go @@ -728,6 +728,11 @@ func (in *CustomDBInstanceParameters) DeepCopyInto(out *CustomDBInstanceParamete *out = new(bool) **out = **in } + if in.TagIgnorePrefixes != nil { + in, out := &in.TagIgnorePrefixes, &out.TagIgnorePrefixes + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomDBInstanceParameters. diff --git a/package/crds/rds.aws.crossplane.io_dbinstances.yaml b/package/crds/rds.aws.crossplane.io_dbinstances.yaml index 0fcbee3742..7f826320df 100644 --- a/package/crds/rds.aws.crossplane.io_dbinstances.yaml +++ b/package/crds/rds.aws.crossplane.io_dbinstances.yaml @@ -1770,6 +1770,13 @@ spec: Default: io1, if the Iops parameter is specified. Otherwise, gp2. type: string + tagIgnorePrefixes: + description: |- + TagIgnorePrefixes tells the reconciler to pretend tags with these + prefixes don't exist during diff/updates. + items: + type: string + type: array tags: description: Tags to assign to the DB instance. items: diff --git a/pkg/controller/rds/dbinstance/ignore_tags_test.go b/pkg/controller/rds/dbinstance/ignore_tags_test.go new file mode 100644 index 0000000000..b81480dcbd --- /dev/null +++ b/pkg/controller/rds/dbinstance/ignore_tags_test.go @@ -0,0 +1,52 @@ +// file: pkg/controller/rds/dbinstance/ignore_tags_test.go +package dbinstance + +import ( + "testing" + + svcsdk "github.com/aws/aws-sdk-go/service/rds" + svcapitypes "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" + "github.com/crossplane-contrib/provider-aws/pkg/controller/rds/utils" + "github.com/crossplane-contrib/provider-aws/pkg/utils/pointer" +) + +// TestIgnorePrefixesAreRespected verifies that aws:, c7n:, and any user-supplied +// prefixes are ignored when we diff tags. +func TestIgnorePrefixesAreRespected(t *testing.T) { + // Tags declared in the CR. + spec := []*svcapitypes.Tag{ + {Key: pointer.ToOrNilIfZeroValue("owner"), Value: pointer.ToOrNilIfZeroValue("cp")}, + } + + // Tags returned by AWS for the DB instance. + raw := []*svcsdk.Tag{ + {Key: pointer.ToOrNilIfZeroValue("owner"), Value: pointer.ToOrNilIfZeroValue("cp")}, + {Key: pointer.ToOrNilIfZeroValue("aws:createdBy"), Value: pointer.ToOrNilIfZeroValue("rds")}, + {Key: pointer.ToOrNilIfZeroValue("c7n:Finding"), Value: pointer.ToOrNilIfZeroValue("foo")}, + {Key: pointer.ToOrNilIfZeroValue("foo:bar"), Value: pointer.ToOrNilIfZeroValue("baz")}, // extra user-ignored prefix + } + + // Pretend the user added an extra ignore prefix via YAML. + customPrefixes := []string{"foo:"} + + // Build the final ignore slice the same way the controller does. + ignore := append([]string{"aws:", "c7n:"}, customPrefixes...) + + // Strip ignored tags, keeping only the ones we care about. + var observed []*svcsdk.Tag + for _, tag := range raw { + if utils.ShouldIgnore(pointer.StringValue(tag.Key), ignore) { + continue + } + observed = append(observed, &svcsdk.Tag{ + Key: tag.Key, + Value: tag.Value, + }) + } + + add, remove := utils.DiffTags(spec, observed) + + if len(add) != 0 || len(remove) != 0 { + t.Fatalf("ignored tags leaked into diff; add=%v remove=%v", add, remove) + } +} diff --git a/pkg/controller/rds/dbinstance/setup.go b/pkg/controller/rds/dbinstance/setup.go index ea45bc42ab..93f205add0 100644 --- a/pkg/controller/rds/dbinstance/setup.go +++ b/pkg/controller/rds/dbinstance/setup.go @@ -534,17 +534,17 @@ func (e *custom) isUpToDate(ctx context.Context, cr *svcapitypes.DBInstance, out cmpopts.IgnoreFields(svcapitypes.CustomDBInstanceParameters{}, "DeleteAutomatedBackups"), ) + ignore := append([]string{"aws:", "c7n:"}, cr.Spec.ForProvider.TagIgnorePrefixes...) var observedTags []*svcsdk.Tag if db.TagList != nil { - for i, tag := range db.TagList { - // ignore system tags - if strings.HasPrefix(*tag.Key, "aws:") || strings.HasPrefix(*tag.Key, "c7n:") { + for _, tag := range db.TagList { // index discarded with _ + if utils.ShouldIgnore(pointer.StringValue(tag.Key), ignore) { continue } - observedTags[i] = &svcsdk.Tag{ + observedTags = append(observedTags, &svcsdk.Tag{ Key: tag.Key, Value: tag.Value, - } + }) } } e.cache.addTags, e.cache.removeTags = utils.DiffTags(cr.Spec.ForProvider.Tags, observedTags) diff --git a/pkg/controller/rds/utils/tags.go b/pkg/controller/rds/utils/tags.go index f3da92849a..c0e7160b3a 100644 --- a/pkg/controller/rds/utils/tags.go +++ b/pkg/controller/rds/utils/tags.go @@ -19,6 +19,7 @@ package utils import ( "context" "sort" + "strings" svcsdk "github.com/aws/aws-sdk-go/service/rds" "github.com/aws/aws-sdk-go/service/rds/rdsiface" @@ -35,6 +36,16 @@ const ( errCreateTags = "cannot create tags" ) +// ShouldIgnore returns true if `key` starts with any supplied prefix. +func ShouldIgnore(key string, prefixes []string) bool { + for _, p := range prefixes { + if strings.HasPrefix(key, p) { + return true + } + } + return false +} + // AreTagsUpToDate for spec and resourceName func AreTagsUpToDate(ctx context.Context, client rdsiface.RDSAPI, spec []*svcapitypes.Tag, resourceName *string) (bool, []*svcsdk.Tag, []*string, error) { current, err := ListTagsForResource(ctx, client, resourceName) From 3a84033b82bad540a2da255a90460a14911fb4fb Mon Sep 17 00:00:00 2001 From: Riyad Ilyasov Date: Wed, 2 Jul 2025 14:26:38 +0200 Subject: [PATCH 3/3] changes base on the commetns and deletion of the test file --- .../rds.aws.crossplane.io_dbinstances.yaml | 4 +- .../rds/dbinstance/ignore_tags_test.go | 52 ------------------- pkg/controller/rds/dbinstance/setup.go | 2 +- 3 files changed, 2 insertions(+), 56 deletions(-) delete mode 100644 pkg/controller/rds/dbinstance/ignore_tags_test.go diff --git a/package/crds/rds.aws.crossplane.io_dbinstances.yaml b/package/crds/rds.aws.crossplane.io_dbinstances.yaml index 7f826320df..6638bd70f1 100644 --- a/package/crds/rds.aws.crossplane.io_dbinstances.yaml +++ b/package/crds/rds.aws.crossplane.io_dbinstances.yaml @@ -1771,9 +1771,7 @@ spec: Default: io1, if the Iops parameter is specified. Otherwise, gp2. type: string tagIgnorePrefixes: - description: |- - TagIgnorePrefixes tells the reconciler to pretend tags with these - prefixes don't exist during diff/updates. + description: List of tag prefixes to be ignored during reconciliation. items: type: string type: array diff --git a/pkg/controller/rds/dbinstance/ignore_tags_test.go b/pkg/controller/rds/dbinstance/ignore_tags_test.go deleted file mode 100644 index b81480dcbd..0000000000 --- a/pkg/controller/rds/dbinstance/ignore_tags_test.go +++ /dev/null @@ -1,52 +0,0 @@ -// file: pkg/controller/rds/dbinstance/ignore_tags_test.go -package dbinstance - -import ( - "testing" - - svcsdk "github.com/aws/aws-sdk-go/service/rds" - svcapitypes "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" - "github.com/crossplane-contrib/provider-aws/pkg/controller/rds/utils" - "github.com/crossplane-contrib/provider-aws/pkg/utils/pointer" -) - -// TestIgnorePrefixesAreRespected verifies that aws:, c7n:, and any user-supplied -// prefixes are ignored when we diff tags. -func TestIgnorePrefixesAreRespected(t *testing.T) { - // Tags declared in the CR. - spec := []*svcapitypes.Tag{ - {Key: pointer.ToOrNilIfZeroValue("owner"), Value: pointer.ToOrNilIfZeroValue("cp")}, - } - - // Tags returned by AWS for the DB instance. - raw := []*svcsdk.Tag{ - {Key: pointer.ToOrNilIfZeroValue("owner"), Value: pointer.ToOrNilIfZeroValue("cp")}, - {Key: pointer.ToOrNilIfZeroValue("aws:createdBy"), Value: pointer.ToOrNilIfZeroValue("rds")}, - {Key: pointer.ToOrNilIfZeroValue("c7n:Finding"), Value: pointer.ToOrNilIfZeroValue("foo")}, - {Key: pointer.ToOrNilIfZeroValue("foo:bar"), Value: pointer.ToOrNilIfZeroValue("baz")}, // extra user-ignored prefix - } - - // Pretend the user added an extra ignore prefix via YAML. - customPrefixes := []string{"foo:"} - - // Build the final ignore slice the same way the controller does. - ignore := append([]string{"aws:", "c7n:"}, customPrefixes...) - - // Strip ignored tags, keeping only the ones we care about. - var observed []*svcsdk.Tag - for _, tag := range raw { - if utils.ShouldIgnore(pointer.StringValue(tag.Key), ignore) { - continue - } - observed = append(observed, &svcsdk.Tag{ - Key: tag.Key, - Value: tag.Value, - }) - } - - add, remove := utils.DiffTags(spec, observed) - - if len(add) != 0 || len(remove) != 0 { - t.Fatalf("ignored tags leaked into diff; add=%v remove=%v", add, remove) - } -} diff --git a/pkg/controller/rds/dbinstance/setup.go b/pkg/controller/rds/dbinstance/setup.go index 93f205add0..e1f6bf7ac0 100644 --- a/pkg/controller/rds/dbinstance/setup.go +++ b/pkg/controller/rds/dbinstance/setup.go @@ -534,7 +534,7 @@ func (e *custom) isUpToDate(ctx context.Context, cr *svcapitypes.DBInstance, out cmpopts.IgnoreFields(svcapitypes.CustomDBInstanceParameters{}, "DeleteAutomatedBackups"), ) - ignore := append([]string{"aws:", "c7n:"}, cr.Spec.ForProvider.TagIgnorePrefixes...) + ignore := append([]string{"aws:"}, cr.Spec.ForProvider.TagIgnorePrefixes...) var observedTags []*svcsdk.Tag if db.TagList != nil { for _, tag := range db.TagList { // index discarded with _