From c8700ba53cfd24afcdbd8eb71804f971d8407d42 Mon Sep 17 00:00:00 2001 From: "Charel Baum (external expert on behalf of DB InfraGO AG)" Date: Wed, 22 Jan 2025 16:30:49 +0100 Subject: [PATCH] fix(rdsinstance.database): do not send empty modify-calls Signed-off-by: Charel Baum (external expert on behalf of DB InfraGO AG) (cherry picked from commit 9f9c44a7b02cbb47f167c4e97cd708ecbfb6cb91) --- pkg/clients/database/rds.go | 4 ++++ pkg/clients/database/rds_test.go | 4 ---- .../database/rdsinstance/rdsinstance.go | 18 ++++++++++++++++-- .../database/rdsinstance/rdsinstance_test.go | 6 ++++-- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/pkg/clients/database/rds.go b/pkg/clients/database/rds.go index 7b7f8b5ace..c21fd40411 100644 --- a/pkg/clients/database/rds.go +++ b/pkg/clients/database/rds.go @@ -887,6 +887,10 @@ func generateCloudWatchExportConfiguration(spec, current []string) *rdstypes.Clo } } + if len(toEnable) == 0 && len(toDisable) == 0 { + return nil + } + return &rdstypes.CloudwatchLogsExportConfiguration{ EnableLogTypes: toEnable, DisableLogTypes: toDisable, diff --git a/pkg/clients/database/rds_test.go b/pkg/clients/database/rds_test.go index 972d96e8ed..530b717a5b 100644 --- a/pkg/clients/database/rds_test.go +++ b/pkg/clients/database/rds_test.go @@ -1368,10 +1368,6 @@ func TestGenerateModifyDBInstanceInput(t *testing.T) { params: v1beta1.RDSInstanceParameters{}, want: rds.ModifyDBInstanceInput{ DBInstanceIdentifier: &emptyName, - CloudwatchLogsExportConfiguration: &rdstypes.CloudwatchLogsExportConfiguration{ - DisableLogTypes: []string{}, - EnableLogTypes: []string{}, - }, }, }, } diff --git a/pkg/controller/database/rdsinstance/rdsinstance.go b/pkg/controller/database/rdsinstance/rdsinstance.go index 0b3307f237..21f2d2bb43 100644 --- a/pkg/controller/database/rdsinstance/rdsinstance.go +++ b/pkg/controller/database/rdsinstance/rdsinstance.go @@ -31,6 +31,8 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/password" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/pkg/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -297,8 +299,20 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext modify.StorageThroughput = nil } - if _, err = e.client.ModifyDBInstance(ctx, modify); err != nil { - return managed.ExternalUpdate{}, errorutils.Wrap(err, errModifyFailed) + // We only want to request a modification if we actually got more than ID and modify flags. + // Note: When ApplyImmediately is set to false, AWS rejects those empty calls with + // "cannot modify RDS instance: api error InalidParameterCombination: No modifications were requested" + // (but when set to true, AWS doesn't block them...) + diff := cmp.Diff(&awsrds.ModifyDBInstanceInput{}, modify, cmpopts.EquateEmpty(), + cmpopts.IgnoreUnexported(awsrds.ModifyDBInstanceInput{}), + cmpopts.IgnoreFields(awsrds.ModifyDBInstanceInput{}, "DBInstanceIdentifier"), + cmpopts.IgnoreFields(awsrds.ModifyDBInstanceInput{}, "AllowMajorVersionUpgrade"), + cmpopts.IgnoreFields(awsrds.ModifyDBInstanceInput{}, "ApplyImmediately"), + ) + if diff != "" { + if _, err = e.client.ModifyDBInstance(ctx, modify); err != nil { + return managed.ExternalUpdate{}, errorutils.Wrap(err, errModifyFailed) + } } // Update tags if necessary diff --git a/pkg/controller/database/rdsinstance/rdsinstance_test.go b/pkg/controller/database/rdsinstance/rdsinstance_test.go index 5becf4cd30..ca7af425fe 100644 --- a/pkg/controller/database/rdsinstance/rdsinstance_test.go +++ b/pkg/controller/database/rdsinstance/rdsinstance_test.go @@ -857,10 +857,12 @@ func TestUpdate(t *testing.T) { }, nil }, }, - cr: instance(), + cr: instance( + withEngineVersion(&engineVersion)), }, want: want{ - cr: instance(), + cr: instance( + withEngineVersion(&engineVersion)), err: errorutils.Wrap(errBoom, errModifyFailed), }, },