fix(database): Ignore AvailabilityZone if MultiAZ is set

Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <maximilian.blatt-extern@deutschebahn.com>
crossplane-contrib · Oct 13, 2023 · e592356 · e592356
1 parent 3a860f1
commit e592356
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 6 deletions.
diff --git a/apis/database/v1beta1/rdsinstance_types.go b/apis/database/v1beta1/rdsinstance_types.go
@@ -276,8 +276,9 @@ type RDSInstanceParameters struct {
 	// Default: A random, system-chosen Availability Zone in the endpoint's AWS
 	// Region.
 	// Example: us-east-1d
-	// Constraint: The AvailabilityZone parameter can't be specified if the MultiAZ
-	// parameter is set to true. The specified Availability Zone must be in the
+	// Constraint: The AvailabilityZone parameter is ignored if the MultiAZ
+	// is set to true.
+	// The specified Availability Zone must be in the
 	// same AWS Region as the current endpoint.
 	// +immutable
 	// +optional

diff --git a/package/crds/database.aws.crossplane.io_rdsinstances.yaml b/package/crds/database.aws.crossplane.io_rdsinstances.yaml
@@ -146,9 +146,9 @@ spec:
                       and Availability Zones, see Regions and Availability Zones (http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Concepts.RegionsAndAvailabilityZones.html).
                       Default: A random, system-chosen Availability Zone in the endpoint''s
                       AWS Region. Example: us-east-1d Constraint: The AvailabilityZone
-                      parameter can''t be specified if the MultiAZ parameter is set
-                      to true. The specified Availability Zone must be in the same
-                      AWS Region as the current endpoint.'
+                      parameter is ignored if the MultiAZ is set to true. The specified
+                      Availability Zone must be in the same AWS Region as the current
+                      endpoint.'
                     type: string
                   backupRetentionPeriod:
                     description: 'BackupRetentionPeriod is the number of days for

diff --git a/pkg/clients/database/rds.go b/pkg/clients/database/rds.go
@@ -318,7 +318,8 @@ func GenerateRestoreRDSInstanceToPointInTimeInput(name string, p *v1beta1.RDSIns
 // CreatePatch creates a *v1beta1.RDSInstanceParameters that has only the changed
 // values between the target *v1beta1.RDSInstanceParameters and the current
 // *rds.DBInstance
-func CreatePatch(in *rdstypes.DBInstance, target *v1beta1.RDSInstanceParameters) (*v1beta1.RDSInstanceParameters, error) {
+func CreatePatch(in *rdstypes.DBInstance, spec *v1beta1.RDSInstanceParameters) (*v1beta1.RDSInstanceParameters, error) { //nolint:gocyclo
+	target := spec.DeepCopy()
 	currentParams := &v1beta1.RDSInstanceParameters{}
 	LateInitialize(currentParams, in)
 
@@ -329,6 +330,15 @@ func CreatePatch(in *rdstypes.DBInstance, target *v1beta1.RDSInstanceParameters)
 		})
 	}
 
+	// AvailabilityZone parameters is not allowed for MultiAZ deployments.
+	// So set this to nil if that is the case to avoid unnecessary diffs.
+	if ptr.Deref(target.MultiAZ, false) {
+		target.AvailabilityZone = nil
+	}
+	if ptr.Deref(currentParams.MultiAZ, false) {
+		currentParams.AvailabilityZone = nil
+	}
+
 	// Don't attempt to scale down storage if autoscaling is enabled,
 	// and the current storage is larger than what was once
 	// requested. We still want to allow the user to manually scale

diff --git a/pkg/clients/database/rds_test.go b/pkg/clients/database/rds_test.go
@@ -128,16 +128,19 @@ func TestCreatePatch(t *testing.T) {
 					AllocatedStorage: allocatedStorage,
 					CharacterSetName: &characterSetName,
 					DBName:           &dbName,
+					AvailabilityZone: ptr.To("az1"),
 				},
 				p: &v1beta1.RDSInstanceParameters{
 					AllocatedStorage: awsclient.IntAddress(awsclient.Int64(30)),
 					CharacterSetName: &characterSetName,
 					DBName:           &dbName,
+					AvailabilityZone: ptr.To("az2"),
 				},
 			},
 			want: want{
 				patch: &v1beta1.RDSInstanceParameters{
 					AllocatedStorage: awsclient.IntAddress(awsclient.Int64(30)),
+					AvailabilityZone: ptr.To("az2"),
 				},
 			},
 		},
@@ -210,6 +213,21 @@ func TestCreatePatch(t *testing.T) {
 				patch: &v1beta1.RDSInstanceParameters{},
 			},
 		},
+		"IgnoreDifferentAvailabilityZoneForMultiAZ": {
+			args: args{
+				db: &rdstypes.DBInstance{
+					AvailabilityZone: ptr.To("az1"),
+					MultiAZ:          true,
+				},
+				p: &v1beta1.RDSInstanceParameters{
+					AvailabilityZone: ptr.To("az2"),
+					MultiAZ:          ptr.To(true),
+				},
+			},
+			want: want{
+				patch: &v1beta1.RDSInstanceParameters{},
+			},
+		},
 	}
 
 	for name, tc := range cases {
@@ -473,6 +491,23 @@ func TestIsUpToDate(t *testing.T) {
 			},
 			want: true,
 		},
+		"NoUpdateForDifferentAvailabilityZoneWhenMultiAZ": {
+			args: args{
+				db: rdstypes.DBInstance{
+					AvailabilityZone: ptr.To("az1"),
+					MultiAZ:          true,
+				},
+				r: v1beta1.RDSInstance{
+					Spec: v1beta1.RDSInstanceSpec{
+						ForProvider: v1beta1.RDSInstanceParameters{
+							AvailabilityZone: ptr.To("az2"),
+							MultiAZ:          ptr.To(true),
+						},
+					},
+				},
+			},
+			want: true,
+		},
 	}
 
 	for name, tc := range cases {