From 21cc81a0bab701c16a31830e75156af83bc2d195 Mon Sep 17 00:00:00 2001 From: "Charel Baum (external expert on behalf of DB Netz AG)" Date: Thu, 7 Dec 2023 16:40:54 +0100 Subject: [PATCH] fix(database): move tag cache to controller Signed-off-by: Charel Baum (external expert on behalf of DB Netz AG) --- pkg/clients/database/rds.go | 28 +++++++++---------- pkg/clients/database/rds_test.go | 2 +- .../database/rdsinstance/rdsinstance.go | 17 +++++++---- .../database/rdsinstance/rdsinstance_test.go | 6 ++-- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/pkg/clients/database/rds.go b/pkg/clients/database/rds.go index ba34c79e01..45fecd369d 100644 --- a/pkg/clients/database/rds.go +++ b/pkg/clients/database/rds.go @@ -49,11 +49,6 @@ const ( errGetPasswordSecretFailed = "cannot get password secret" ) -type Cache struct { - AddTags []rdstypes.Tag - RemoveTags []string -} - // Client defines RDS RDSClient operations type Client interface { CreateDBInstance(context.Context, *rds.CreateDBInstanceInput, ...func(*rds.Options)) (*rds.CreateDBInstanceOutput, error) @@ -699,15 +694,18 @@ func lateInitializeOptionGroupName(inOptionGroupName *string, members []rdstypes } // IsUpToDate checks whether there is a change in any of the modifiable fields. -func IsUpToDate(ctx context.Context, kube client.Client, r *v1beta1.RDSInstance, db rdstypes.DBInstance) (bool, string, Cache, error) { //nolint:gocyclo - cacheRds := Cache{} +func IsUpToDate(ctx context.Context, kube client.Client, r *v1beta1.RDSInstance, db rdstypes.DBInstance) (bool, string, []rdstypes.Tag, []string, error) { //nolint:gocyclo + + addTags := []rdstypes.Tag{} + removeTags := []string{} + _, pwdChanged, err := GetPassword(ctx, kube, r.Spec.ForProvider.MasterPasswordSecretRef, r.Spec.WriteConnectionSecretToReference) if err != nil { - return false, "", cacheRds, err + return false, "", addTags, removeTags, err } patch, err := CreatePatch(&db, &r.Spec.ForProvider) if err != nil { - return false, "", cacheRds, err + return false, "", addTags, removeTags, err } diff := cmp.Diff(&v1beta1.RDSInstanceParameters{}, patch, cmpopts.EquateEmpty(), cmpopts.IgnoreTypes(&xpv1.Reference{}, &xpv1.Selector{}, []xpv1.Reference{}), @@ -728,8 +726,8 @@ func IsUpToDate(ctx context.Context, kube client.Client, r *v1beta1.RDSInstance, cmpopts.IgnoreFields(v1beta1.RDSInstanceParameters{}, "CloudwatchLogsExportConfiguration"), ) - cacheRds.AddTags, cacheRds.RemoveTags = diffTags(r.Spec.ForProvider.Tags, db.TagList) - tagsChanged := len(cacheRds.AddTags) != 0 || len(cacheRds.RemoveTags) != 0 + addTags, removeTags = DiffTags(r.Spec.ForProvider.Tags, db.TagList) + tagsChanged := len(addTags) != 0 || len(removeTags) != 0 engineVersionChanged := !isEngineVersionUpToDate(r, db) @@ -743,16 +741,16 @@ func IsUpToDate(ctx context.Context, kube client.Client, r *v1beta1.RDSInstance, } if diff == "" && !pwdChanged && !engineVersionChanged && !optionGroupChanged && !cloudwatchLogsExportChanged && !tagsChanged { - return true, "", cacheRds, nil + return true, "", addTags, removeTags, nil } diff = "Found observed difference in rds\n" + diff if tagsChanged { - diff += fmt.Sprintf("\nadd %d tag(s) and remove %d tag(s)", len(cacheRds.AddTags), len(cacheRds.RemoveTags)) + diff += fmt.Sprintf("\nadd %d tag(s) and remove %d tag(s)", len(addTags), len(removeTags)) } - return false, diff, cacheRds, nil + return false, diff, addTags, removeTags, nil } func isEngineVersionUpToDate(cr *v1beta1.RDSInstance, db rdstypes.DBInstance) bool { @@ -893,7 +891,7 @@ func areSameElements(a1, a2 []string) bool { } // DiffTags between spec and current -func diffTags(spec []v1beta1.Tag, current []rdstypes.Tag) (addTags []rdstypes.Tag, removeTags []string) { +func DiffTags(spec []v1beta1.Tag, current []rdstypes.Tag) (addTags []rdstypes.Tag, removeTags []string) { currentMap := make(map[string]string, len(current)) for _, t := range current { currentMap[pointer.StringValue(t.Key)] = pointer.StringValue(t.Value) diff --git a/pkg/clients/database/rds_test.go b/pkg/clients/database/rds_test.go index a8f64d483d..dd23a83d1b 100644 --- a/pkg/clients/database/rds_test.go +++ b/pkg/clients/database/rds_test.go @@ -532,7 +532,7 @@ func TestIsUpToDate(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { ctx := context.Background() - got, _, _, _ := IsUpToDate(ctx, tc.args.kube, &tc.args.r, tc.args.db) + got, _, _, _, _ := IsUpToDate(ctx, tc.args.kube, &tc.args.r, tc.args.db) if diff := cmp.Diff(tc.want, got); diff != "" { t.Errorf("r: -want, +got:\n%s", diff) } diff --git a/pkg/controller/database/rdsinstance/rdsinstance.go b/pkg/controller/database/rdsinstance/rdsinstance.go index 522664b90e..82777131fe 100644 --- a/pkg/controller/database/rdsinstance/rdsinstance.go +++ b/pkg/controller/database/rdsinstance/rdsinstance.go @@ -22,6 +22,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" awsrds "github.com/aws/aws-sdk-go-v2/service/rds" + rdstypes "github.com/aws/aws-sdk-go-v2/service/rds/types" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/connection" "github.com/crossplane/crossplane-runtime/pkg/controller" @@ -112,14 +113,19 @@ func (c *connector) Connect(ctx context.Context, mg resource.Managed) (managed.E if err != nil { return nil, err } - return &external{c.newClientFn(cfg), c.kube, rds.Cache{}}, nil + return &external{c.newClientFn(cfg), c.kube, Cache{}}, nil +} + +type Cache struct { + AddTags []rdstypes.Tag + RemoveTags []string } type external struct { client rds.Client kube client.Client - cache rds.Cache + cache Cache } func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { @@ -155,11 +161,10 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex cr.Status.SetConditions(xpv1.Unavailable()) } - var cachecache rds.Cache - - upToDate, diff, cachecache, err := rds.IsUpToDate(ctx, e.kube, cr, instance) + var upToDate bool + var diff string - e.cache = cachecache + upToDate, diff, e.cache.AddTags, e.cache.RemoveTags, err = rds.IsUpToDate(ctx, e.kube, cr, instance) if err != nil { return managed.ExternalObservation{}, errorutils.Wrap(err, errUpToDateFailed) diff --git a/pkg/controller/database/rdsinstance/rdsinstance_test.go b/pkg/controller/database/rdsinstance/rdsinstance_test.go index e83bb8fe63..fce4b36a91 100644 --- a/pkg/controller/database/rdsinstance/rdsinstance_test.go +++ b/pkg/controller/database/rdsinstance/rdsinstance_test.go @@ -100,7 +100,7 @@ func FromTimePtr(t *time.Time) *metav1.Time { type args struct { rds rds.Client kube client.Client - cache rds.Cache + cache Cache cr *v1beta1.RDSInstance } @@ -194,8 +194,8 @@ func instance(m ...rdsModifier) *v1beta1.RDSInstance { return cr } -func cache(addTagsMap map[string]string, removeTagsMap map[string]string) rds.Cache { - c := rds.Cache{} +func cache(addTagsMap map[string]string, removeTagsMap map[string]string) Cache { + c := Cache{} for k, v := range addTagsMap { c.AddTags = append(c.AddTags, awsrdstypes.Tag{Key: pointer.ToOrNilIfZeroValue(k), Value: pointer.ToOrNilIfZeroValue(v)}) }