From 01a66dbd1dd34e1a67000f97ff67b371d8d92973 Mon Sep 17 00:00:00 2001 From: "Charel Baum (external expert on behalf of DB InfraGO AG)" Date: Thu, 4 Jul 2024 09:03:03 +0200 Subject: [PATCH 1/2] feat(docdb): allow (major) version upgrades + observe engineVersion Signed-off-by: Charel Baum (external expert on behalf of DB InfraGO AG) --- apis/docdb/generator-config.yaml | 13 + apis/docdb/v1alpha1/custom_types.go | 2 + apis/docdb/v1alpha1/zz_db_cluster.go | 8 + apis/docdb/v1alpha1/zz_generated.deepcopy.go | 10 + .../docdb.aws.crossplane.io_dbclusters.yaml | 12 + pkg/controller/docdb/dbcluster/setup.go | 68 +++- pkg/controller/docdb/dbcluster/setup_test.go | 379 +++++++++++++++++- .../docdb/dbcluster/zz_conversions.go | 3 + pkg/controller/docdb/utils/engine_version.go | 85 ++++ .../docdb/utils/engine_version_test.go | 83 ++++ 10 files changed, 656 insertions(+), 7 deletions(-) create mode 100644 pkg/controller/docdb/utils/engine_version.go create mode 100644 pkg/controller/docdb/utils/engine_version_test.go diff --git a/apis/docdb/generator-config.yaml b/apis/docdb/generator-config.yaml index 412a19cf6e..90a950f45e 100644 --- a/apis/docdb/generator-config.yaml +++ b/apis/docdb/generator-config.yaml @@ -1,3 +1,16 @@ +resources: + DBCluster: + fields: + AllowMajorVersionUpgrade: + from: + operation: ModifyDBCluster + path: AllowMajorVersionUpgrade + EngineVersion: + is_read_only: true + from: + operation: DescribeDBClusters + path: DBClusters.EngineVersion + ignore: field_paths: - DescribeDBClusterParameterGroupsInput.DBClusterParameterGroupName diff --git a/apis/docdb/v1alpha1/custom_types.go b/apis/docdb/v1alpha1/custom_types.go index cffe296c63..903ac5c1b5 100644 --- a/apis/docdb/v1alpha1/custom_types.go +++ b/apis/docdb/v1alpha1/custom_types.go @@ -31,6 +31,8 @@ const ( DocDBInstanceStateDeleting = "deleting" // The instance is being modified. DocDBInstanceStateModifying = "modifying" + // The instance is being upgraded. + DocDBInstanceStateUpgrading = "upgrading" // The instance has failed and Amazon RDS can't recover it. Perform a point-in-time restore to the latest restorable time of the instance to recover the data. DocDBInstanceStateFailed = "failed" ) diff --git a/apis/docdb/v1alpha1/zz_db_cluster.go b/apis/docdb/v1alpha1/zz_db_cluster.go index 665a53be3a..5f96687518 100644 --- a/apis/docdb/v1alpha1/zz_db_cluster.go +++ b/apis/docdb/v1alpha1/zz_db_cluster.go @@ -29,6 +29,12 @@ type DBClusterParameters struct { // Region is which region the DBCluster will be created. // +kubebuilder:validation:Required Region string `json:"region"` + // A value that indicates whether major version upgrades are allowed. + // + // Constraints: You must allow major version upgrades when specifying a value + // for the EngineVersion parameter that is a different major version than the + // DB cluster's current version. + AllowMajorVersionUpgrade *bool `json:"allowMajorVersionUpgrade,omitempty"` // A list of Amazon EC2 Availability Zones that instances in the cluster can // be created in. AvailabilityZones []*string `json:"availabilityZones,omitempty"` @@ -188,6 +194,8 @@ type DBClusterObservation struct { EnabledCloudwatchLogsExports []*string `json:"enabledCloudwatchLogsExports,omitempty"` // Specifies the connection endpoint for the primary instance of the cluster. Endpoint *string `json:"endpoint,omitempty"` + // Indicates the database engine version. + EngineVersion *string `json:"engineVersion,omitempty"` // Specifies the ID that Amazon Route 53 assigns when you create a hosted zone. HostedZoneID *string `json:"hostedZoneID,omitempty"` // Specifies the latest time to which a database can be restored with point-in-time diff --git a/apis/docdb/v1alpha1/zz_generated.deepcopy.go b/apis/docdb/v1alpha1/zz_generated.deepcopy.go index 9e84d5b635..4ae69928fe 100644 --- a/apis/docdb/v1alpha1/zz_generated.deepcopy.go +++ b/apis/docdb/v1alpha1/zz_generated.deepcopy.go @@ -514,6 +514,11 @@ func (in *DBClusterObservation) DeepCopyInto(out *DBClusterObservation) { *out = new(string) **out = **in } + if in.EngineVersion != nil { + in, out := &in.EngineVersion, &out.EngineVersion + *out = new(string) + **out = **in + } if in.HostedZoneID != nil { in, out := &in.HostedZoneID, &out.HostedZoneID *out = new(string) @@ -775,6 +780,11 @@ func (in *DBClusterParameterGroup_SDK) DeepCopy() *DBClusterParameterGroup_SDK { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DBClusterParameters) DeepCopyInto(out *DBClusterParameters) { *out = *in + if in.AllowMajorVersionUpgrade != nil { + in, out := &in.AllowMajorVersionUpgrade, &out.AllowMajorVersionUpgrade + *out = new(bool) + **out = **in + } if in.AvailabilityZones != nil { in, out := &in.AvailabilityZones, &out.AvailabilityZones *out = make([]*string, len(*in)) diff --git a/package/crds/docdb.aws.crossplane.io_dbclusters.yaml b/package/crds/docdb.aws.crossplane.io_dbclusters.yaml index dd96f9d203..591b0c20e1 100644 --- a/package/crds/docdb.aws.crossplane.io_dbclusters.yaml +++ b/package/crds/docdb.aws.crossplane.io_dbclusters.yaml @@ -73,6 +73,15 @@ spec: forProvider: description: DBClusterParameters defines the desired state of DBCluster properties: + allowMajorVersionUpgrade: + description: |- + A value that indicates whether major version upgrades are allowed. + + + Constraints: You must allow major version upgrades when specifying a value + for the EngineVersion parameter that is a different major version than the + DB cluster's current version. + type: boolean applyImmediately: description: |- A value that specifies whether the changes in this request and any pending @@ -984,6 +993,9 @@ spec: description: Specifies the connection endpoint for the primary instance of the cluster. type: string + engineVersion: + description: Indicates the database engine version. + type: string hostedZoneID: description: Specifies the ID that Amazon Route 53 assigns when you create a hosted zone. diff --git a/pkg/controller/docdb/dbcluster/setup.go b/pkg/controller/docdb/dbcluster/setup.go index b4d231a8bc..cb8e2660f2 100644 --- a/pkg/controller/docdb/dbcluster/setup.go +++ b/pkg/controller/docdb/dbcluster/setup.go @@ -31,6 +31,7 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/password" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" + cpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -40,8 +41,10 @@ import ( svcapitypes "github.com/crossplane-contrib/provider-aws/apis/docdb/v1alpha1" "github.com/crossplane-contrib/provider-aws/apis/v1alpha1" + "github.com/crossplane-contrib/provider-aws/pkg/controller/docdb/utils" svcutils "github.com/crossplane-contrib/provider-aws/pkg/controller/docdb/utils" "github.com/crossplane-contrib/provider-aws/pkg/features" + errorutils "github.com/crossplane-contrib/provider-aws/pkg/utils/errors" "github.com/crossplane-contrib/provider-aws/pkg/utils/pointer" custommanaged "github.com/crossplane-contrib/provider-aws/pkg/utils/reconciler/managed" ) @@ -121,6 +124,9 @@ func (e *hooks) postObserve(ctx context.Context, cr *svcapitypes.DBCluster, resp return managed.ExternalObservation{}, err } + cluster := resp.DBClusters[0] + cr.Status.AtProvider.EngineVersion = cluster.EngineVersion + obs.ConnectionDetails = getConnectionDetails(cr) if !meta.WasDeleted(cr) { @@ -177,6 +183,10 @@ func lateInitialize(cr *svcapitypes.DBClusterParameters, resp *svcsdk.DescribeDB func (e *hooks) isUpToDate(ctx context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.DescribeDBClustersOutput) (bool, string, error) { cluster := resp.DBClusters[0] + if pointer.StringValue(cluster.Status) == svcapitypes.DocDBInstanceStateModifying || pointer.StringValue(cluster.Status) == svcapitypes.DocDBInstanceStateUpgrading { + return true, "", nil + } + _, pwChanged, err := e.getPasswordFromRef(ctx, cr.Spec.ForProvider.MasterUserPasswordSecretRef, cr.Spec.WriteConnectionSecretToReference) if err != nil || pwChanged { return false, "", err @@ -187,6 +197,7 @@ func (e *hooks) isUpToDate(ctx context.Context, cr *svcapitypes.DBCluster, resp pointer.StringValue(cr.Spec.ForProvider.DBClusterParameterGroupName) != pointer.StringValue(cluster.DBClusterParameterGroup), pointer.BoolValue(cr.Spec.ForProvider.DeletionProtection) != pointer.BoolValue(cluster.DeletionProtection), !areSameElements(cr.Spec.ForProvider.EnableCloudwatchLogsExports, cluster.EnabledCloudwatchLogsExports), + !isEngineVersionUpToDate(cr, resp), pointer.Int64Value(cr.Spec.ForProvider.Port) != pointer.Int64Value(cluster.Port), pointer.StringValue(cr.Spec.ForProvider.PreferredBackupWindow) != pointer.StringValue(cluster.PreferredBackupWindow), pointer.StringValue(cr.Spec.ForProvider.PreferredMaintenanceWindow) != pointer.StringValue(cluster.PreferredMaintenanceWindow): @@ -211,14 +222,53 @@ func (e *hooks) preUpdate(ctx context.Context, cr *svcapitypes.DBCluster, obj *s if pwchanged { obj.MasterUserPassword = aws.String(pw) } + + // ModifyDBCluster() returns error, when trying to upgrade major (minor is fine) EngineVersion: + // "Cannot change VPC security group while doing a major version upgrade." + // therefore EngineVersion update is entirely done separately in postUpdate + obj.EngineVersion = nil + // In case of a custom DBClusterParameterGroup, AWS requires for a major version update that + // EngineVersion and DBClusterParameterGroupName are in the same ModifyDBCluster()-call + obj.DBClusterParameterGroupName = nil + return nil } -func (e *hooks) postUpdate(_ context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.ModifyDBClusterOutput, upd managed.ExternalUpdate, err error) (managed.ExternalUpdate, error) { +func (e *hooks) postUpdate(ctx context.Context, cr *svcapitypes.DBCluster, resp *svcsdk.ModifyDBClusterOutput, upd managed.ExternalUpdate, err error) (managed.ExternalUpdate, error) { if err != nil { return managed.ExternalUpdate{}, err } + input := GenerateDescribeDBClustersInput(cr) + // GenerateDescribeDBClustersInput returns an empty DescribeDBClustersInput + // and the function is generated by ack-generate, so we manually need to set the + // DBClusterIdentifier + input.DBClusterIdentifier = pointer.ToOrNilIfZeroValue(meta.GetExternalName(cr)) + out, err := e.client.DescribeDBClustersWithContext(ctx, input) + if err != nil { + return managed.ExternalUpdate{}, errorutils.Wrap(cpresource.Ignore(IsNotFound, err), errDescribe) + } + + needsEngineVersionUpdate := !isEngineVersionUpToDate(cr, out) + needsDBClusterParamGroupUpdate := pointer.StringValue(cr.Spec.ForProvider.DBClusterParameterGroupName) != pointer.StringValue(out.DBClusters[0].DBClusterParameterGroup) + needsPostUpdate := needsEngineVersionUpdate || needsDBClusterParamGroupUpdate + + if needsPostUpdate { + modifyInput := &svcsdk.ModifyDBClusterInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(meta.GetExternalName(cr)), + ApplyImmediately: cr.Spec.ForProvider.ApplyImmediately, + DBClusterParameterGroupName: cr.Spec.ForProvider.DBClusterParameterGroupName, + } + if needsEngineVersionUpdate { + modifyInput.EngineVersion = cr.Spec.ForProvider.EngineVersion + modifyInput.AllowMajorVersionUpgrade = cr.Spec.ForProvider.AllowMajorVersionUpgrade + } + + if _, err = e.client.ModifyDBClusterWithContext(ctx, modifyInput); err != nil { + return managed.ExternalUpdate{}, err + } + } + return upd, svcutils.UpdateTagsForResource(e.client, cr.Spec.ForProvider.Tags, resp.DBCluster.DBClusterArn) } @@ -442,6 +492,22 @@ func areSameElements(a1, a2 []*string) bool { return true } +func isEngineVersionUpToDate(cr *svcapitypes.DBCluster, out *svcsdk.DescribeDBClustersOutput) bool { + // If EngineVersion is not set, AWS sets a default value, + // so we do not try to update in this case + if cr.Spec.ForProvider.EngineVersion != nil { + if out.DBClusters[0].EngineVersion == nil { + return false + } + + // Upgrade is only necessary if the spec version is higher. + // Downgrades are not possible in pointer. + c := utils.CompareEngineVersions(*cr.Spec.ForProvider.EngineVersion, *out.DBClusters[0].EngineVersion) + return c <= 0 + } + return true +} + func (e *hooks) getPasswordFromRef(ctx context.Context, in *xpv1.SecretKeySelector, out *xpv1.SecretReference) (newPwd string, changed bool, err error) { if in == nil { return "", false, nil diff --git a/pkg/controller/docdb/dbcluster/setup_test.go b/pkg/controller/docdb/dbcluster/setup_test.go index 18aefb1318..2f1fea1772 100644 --- a/pkg/controller/docdb/dbcluster/setup_test.go +++ b/pkg/controller/docdb/dbcluster/setup_test.go @@ -55,7 +55,8 @@ var ( testCloudWatchLog = "some-log" testOtherCloudWatchLog = "some-other-log" testEngine = "some-engine" - testEngineVersion = "some-engine-version" + testEngineVersion = "4.0.0" + testOtherEngineVersion = "5.0.0" testKMSKeyID = "some-key" testMasterUserName = "some-user" testMasterUserPassword = "some-pw" @@ -211,18 +212,31 @@ func withEngineVersion(value string) docDBModifier { } } +func withStatusEngineVersion(value string) docDBModifier { + return func(o *svcapitypes.DBCluster) { + o.Status.AtProvider.EngineVersion = pointer.ToOrNilIfZeroValue(value) + + } +} + +func withAllowMajorVersionUpgrade(value bool) docDBModifier { + return func(o *svcapitypes.DBCluster) { + o.Spec.ForProvider.AllowMajorVersionUpgrade = pointer.ToOrNilIfZeroValue(value) + } +} + func withMasterUserName(value string) docDBModifier { return func(o *svcapitypes.DBCluster) { o.Spec.ForProvider.MasterUsername = pointer.ToOrNilIfZeroValue(value) } } -func withMasterPasswordSecretRef(namesapce, name, key string) docDBModifier { +func withMasterPasswordSecretRef(namespace, name, key string) docDBModifier { return func(o *svcapitypes.DBCluster) { o.Spec.ForProvider.MasterUserPasswordSecretRef = &xpv1.SecretKeySelector{ SecretReference: xpv1.SecretReference{ Name: name, - Namespace: o.Namespace, + Namespace: namespace, }, Key: key, } @@ -1224,6 +1238,114 @@ func TestObserve(t *testing.T) { }, }, }, + "AvailableState_and_changed_EngineVersion_should_not_be_UpToDate": { + args: args{ + docdb: &fake.MockDocDBClient{ + MockDescribeDBClustersWithContext: func(c context.Context, ddi *docdb.DescribeDBClustersInput, o []request.Option) (*docdb.DescribeDBClustersOutput, error) { + return &docdb.DescribeDBClustersOutput{ + DBClusters: []*docdb.DBCluster{ + { + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + Status: pointer.ToOrNilIfZeroValue(svcapitypes.DocDBInstanceStateAvailable), + EngineVersion: pointer.ToOrNilIfZeroValue(testEngineVersion), + }, + }, + }, nil + }, + }, + cr: instance( + withDBClusterIdentifier(testDBClusterIdentifier), + withExternalName(testDBClusterIdentifier), + withEngineVersion(testOtherEngineVersion), + ), + }, + want: want{ + cr: instance( + withDBClusterIdentifier(testDBClusterIdentifier), + withExternalName(testDBClusterIdentifier), + withConditions(xpv1.Available()), + withStatus(svcapitypes.DocDBInstanceStateAvailable), + withEngineVersion(testOtherEngineVersion), + withStatusEngineVersion(testEngineVersion), + withVpcSecurityGroupIds(), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: false, + ResourceLateInitialized: true, + ConnectionDetails: generateConnectionDetails("", "", "", "", 0), + }, + docdb: fake.MockDocDBClientCall{ + DescribeDBClustersWithContext: []*fake.CallDescribeDBClustersWithContext{ + { + Ctx: context.Background(), + I: &docdb.DescribeDBClustersInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, + }, + }, + }, + "AvailableState_and_same_EngineVersion_should_be_UpToDate": { + args: args{ + docdb: &fake.MockDocDBClient{ + MockDescribeDBClustersWithContext: func(c context.Context, ddi *docdb.DescribeDBClustersInput, o []request.Option) (*docdb.DescribeDBClustersOutput, error) { + return &docdb.DescribeDBClustersOutput{ + DBClusters: []*docdb.DBCluster{ + { + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + Status: pointer.ToOrNilIfZeroValue(svcapitypes.DocDBInstanceStateAvailable), + EngineVersion: pointer.ToOrNilIfZeroValue(testEngineVersion), + }, + }, + }, nil + }, + MockListTagsForResource: func(ltfri *docdb.ListTagsForResourceInput) (*docdb.ListTagsForResourceOutput, error) { + return &docdb.ListTagsForResourceOutput{ + TagList: []*docdb.Tag{}, + }, nil + }, + }, + cr: instance( + withDBClusterIdentifier(testDBClusterIdentifier), + withExternalName(testDBClusterIdentifier), + withEngineVersion(testEngineVersion), + ), + }, + want: want{ + cr: instance( + withDBClusterIdentifier(testDBClusterIdentifier), + withExternalName(testDBClusterIdentifier), + withConditions(xpv1.Available()), + withStatus(svcapitypes.DocDBInstanceStateAvailable), + withEngineVersion(testEngineVersion), + withStatusEngineVersion(testEngineVersion), + withVpcSecurityGroupIds(), + ), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: true, + ResourceLateInitialized: true, + ConnectionDetails: generateConnectionDetails("", "", "", "", 0), + }, + docdb: fake.MockDocDBClientCall{ + DescribeDBClustersWithContext: []*fake.CallDescribeDBClustersWithContext{ + { + Ctx: context.Background(), + I: &docdb.DescribeDBClustersInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, + ListTagsForResource: []*fake.CallListTagsForResource{ + { + I: &docdb.ListTagsForResourceInput{}, + }, + }, + }, + }, + }, "AvailableState_and_changed_Port_should_not_be_UpToDate": { args: args{ docdb: &fake.MockDocDBClient{ @@ -2697,7 +2819,7 @@ func TestUpdate(t *testing.T) { args want }{ - "SuccessfulUpdate": { + "SuccessfulRegularUpdate": { args: args{ docdb: &fake.MockDocDBClient{ MockModifyDBClusterWithContext: func(c context.Context, mdpgi *docdb.ModifyDBClusterInput, o []request.Option) (*docdb.ModifyDBClusterOutput, error) { @@ -2708,6 +2830,17 @@ func TestUpdate(t *testing.T) { }, }, nil }, + MockDescribeDBClustersWithContext: func(ctx context.Context, ddi *docdb.DescribeDBClustersInput, o []request.Option) (*docdb.DescribeDBClustersOutput, error) { + return &docdb.DescribeDBClustersOutput{ + DBClusters: []*docdb.DBCluster{ + { + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + DBClusterParameterGroup: pointer.ToOrNilIfZeroValue(testDBClusterParameterGroupName), + EngineVersion: pointer.ToOrNilIfZeroValue(testEngineVersion), + }, + }, + }, nil + }, MockListTagsForResource: func(ltfri *docdb.ListTagsForResourceInput) (*docdb.ListTagsForResourceOutput, error) { return &docdb.ListTagsForResourceOutput{ TagList: []*docdb.Tag{ @@ -2792,9 +2925,174 @@ func TestUpdate(t *testing.T) { I: &docdb.ModifyDBClusterInput{ DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), BackupRetentionPeriod: pointer.ToIntAsInt64(testBackupRetentionPeriod), - DBClusterParameterGroupName: pointer.ToOrNilIfZeroValue(testDBClusterParameterGroupName), + DBClusterParameterGroupName: nil, + DeletionProtection: pointer.ToOrNilIfZeroValue(true), + EngineVersion: nil, + Port: pointer.ToIntAsInt64(testPort), + PreferredBackupWindow: pointer.ToOrNilIfZeroValue(testPreferredBackupWindow), + PreferredMaintenanceWindow: pointer.ToOrNilIfZeroValue(testPreferredMaintenanceWindow), + VpcSecurityGroupIds: toStringPtrArray( + testVpcSecurityGroup, + testOtherVpcSecurityGroup, + ), + CloudwatchLogsExportConfiguration: &docdb.CloudwatchLogsExportConfiguration{ + DisableLogTypes: []*string{}, + EnableLogTypes: []*string{}, + }, + }, + }, + }, + DescribeDBClustersWithContext: []*fake.CallDescribeDBClustersWithContext{ + { + Ctx: context.Background(), + I: &docdb.DescribeDBClustersInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, + ListTagsForResource: []*fake.CallListTagsForResource{ + { + I: &docdb.ListTagsForResourceInput{ + ResourceName: pointer.ToOrNilIfZeroValue(testDBClusterArn), + }, + }, + }, + AddTagsToResource: []*fake.CallAddTagsToResource{ + { + I: &docdb.AddTagsToResourceInput{ + ResourceName: pointer.ToOrNilIfZeroValue(testDBClusterArn), + Tags: []*docdb.Tag{ + {Key: pointer.ToOrNilIfZeroValue(testTagKey), Value: pointer.ToOrNilIfZeroValue(testTagValue)}, + {Key: pointer.ToOrNilIfZeroValue(testOtherTagKey), Value: pointer.ToOrNilIfZeroValue(testOtherTagValue)}, + }, + }, + }, + }, + RemoveTagsFromResource: []*fake.CallRemoveTagsFromResource{ + { + I: &docdb.RemoveTagsFromResourceInput{ + ResourceName: pointer.ToOrNilIfZeroValue(testDBClusterArn), + TagKeys: []*string{ + pointer.ToOrNilIfZeroValue(testOtherOtherTagKey), + }, + }, + }, + }, + }, + }, + }, + "Successful_EngineVersion_DBClusterParameterGroupName_Update": { + args: args{ + docdb: &fake.MockDocDBClient{ + MockModifyDBClusterWithContext: func(c context.Context, mdpgi *docdb.ModifyDBClusterInput, o []request.Option) (*docdb.ModifyDBClusterOutput, error) { + return &docdb.ModifyDBClusterOutput{ + DBCluster: &docdb.DBCluster{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + DBClusterArn: pointer.ToOrNilIfZeroValue(testDBClusterArn), + }, + }, nil + }, + MockDescribeDBClustersWithContext: func(ctx context.Context, ddi *docdb.DescribeDBClustersInput, o []request.Option) (*docdb.DescribeDBClustersOutput, error) { + return &docdb.DescribeDBClustersOutput{ + DBClusters: []*docdb.DBCluster{ + { + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + DBClusterParameterGroup: pointer.ToOrNilIfZeroValue(testDBClusterParameterGroupName), + EngineVersion: pointer.ToOrNilIfZeroValue(testEngineVersion), + }, + }, + }, nil + }, + MockListTagsForResource: func(ltfri *docdb.ListTagsForResourceInput) (*docdb.ListTagsForResourceOutput, error) { + return &docdb.ListTagsForResourceOutput{ + TagList: []*docdb.Tag{ + { + Key: pointer.ToOrNilIfZeroValue(testOtherOtherTagKey), + Value: pointer.ToOrNilIfZeroValue(testOtherOtherTagValue), + }, + }, + }, nil + }, + MockAddTagsToResource: func(attri *docdb.AddTagsToResourceInput) (*docdb.AddTagsToResourceOutput, error) { + return &docdb.AddTagsToResourceOutput{}, nil + }, + MockRemoveTagsFromResource: func(rtfri *docdb.RemoveTagsFromResourceInput) (*docdb.RemoveTagsFromResourceOutput, error) { + return &docdb.RemoveTagsFromResourceOutput{}, nil + }, + }, + cr: instance( + withDBClusterIdentifier(testDBClusterIdentifier), + withExternalName(testDBClusterIdentifier), + withAvailabilityZones( + testAvailabilityZone, + testOtherAvailabilityZone, + ), + withBackupRetentionPeriod(testBackupRetentionPeriod), + withDBClusterParameterGroupName(testOtherDBClusterParameterGroupName), + withDBSubnetGroup(testDBSubnetGroupName), + withDeletionProtection(true), + withEngine(testEngine), + withEngineVersion(testOtherEngineVersion), + withAllowMajorVersionUpgrade(true), + withKmsKeyID(testKMSKeyID), + withMasterUserName(testMasterUserName), + withPort(testPort), + withPreSignedURL(testPresignedURL), + withPreferredBackupWindow(testPreferredBackupWindow), + withPreferredMaintenanceWindow(testPreferredMaintenanceWindow), + withStorageEncrypted(true), + withTags( + &svcapitypes.Tag{Key: pointer.ToOrNilIfZeroValue(testTagKey), Value: pointer.ToOrNilIfZeroValue(testTagValue)}, + &svcapitypes.Tag{Key: pointer.ToOrNilIfZeroValue(testOtherTagKey), Value: pointer.ToOrNilIfZeroValue(testOtherTagValue)}, + ), + withVpcSecurityGroupIds( + testVpcSecurityGroup, + testOtherVpcSecurityGroup, + ), + ), + }, + want: want{ + cr: instance( + withDBClusterIdentifier(testDBClusterIdentifier), + withExternalName(testDBClusterIdentifier), + withAvailabilityZones( + testAvailabilityZone, + testOtherAvailabilityZone, + ), + withBackupRetentionPeriod(testBackupRetentionPeriod), + withDBClusterParameterGroupName(testOtherDBClusterParameterGroupName), + withDBSubnetGroup(testDBSubnetGroupName), + withDeletionProtection(true), + withEngine(testEngine), + withEngineVersion(testOtherEngineVersion), + withAllowMajorVersionUpgrade(true), + withKmsKeyID(testKMSKeyID), + withMasterUserName(testMasterUserName), + withPort(testPort), + withPreSignedURL(testPresignedURL), + withPreferredBackupWindow(testPreferredBackupWindow), + withPreferredMaintenanceWindow(testPreferredMaintenanceWindow), + withStorageEncrypted(true), + withTags( + &svcapitypes.Tag{Key: pointer.ToOrNilIfZeroValue(testTagKey), Value: pointer.ToOrNilIfZeroValue(testTagValue)}, + &svcapitypes.Tag{Key: pointer.ToOrNilIfZeroValue(testOtherTagKey), Value: pointer.ToOrNilIfZeroValue(testOtherTagValue)}, + ), + withVpcSecurityGroupIds( + testVpcSecurityGroup, + testOtherVpcSecurityGroup, + ), + ), + docdb: fake.MockDocDBClientCall{ + ModifyDBClusterWithContext: []*fake.CallModifyDBClusterWithContext{ + { + Ctx: context.Background(), + I: &docdb.ModifyDBClusterInput{ + AllowMajorVersionUpgrade: pointer.ToOrNilIfZeroValue(true), + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + BackupRetentionPeriod: pointer.ToIntAsInt64(testBackupRetentionPeriod), + DBClusterParameterGroupName: nil, DeletionProtection: pointer.ToOrNilIfZeroValue(true), - EngineVersion: pointer.ToOrNilIfZeroValue(testEngineVersion), + EngineVersion: nil, Port: pointer.ToIntAsInt64(testPort), PreferredBackupWindow: pointer.ToOrNilIfZeroValue(testPreferredBackupWindow), PreferredMaintenanceWindow: pointer.ToOrNilIfZeroValue(testPreferredMaintenanceWindow), @@ -2808,7 +3106,25 @@ func TestUpdate(t *testing.T) { }, }, }, + { // Modify call for engineVersion/dbClusterParameterGroupName in postUpdate + Ctx: context.Background(), + I: &docdb.ModifyDBClusterInput{ + AllowMajorVersionUpgrade: pointer.ToOrNilIfZeroValue(true), + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + DBClusterParameterGroupName: pointer.ToOrNilIfZeroValue(testOtherDBClusterParameterGroupName), + EngineVersion: pointer.ToOrNilIfZeroValue(testOtherEngineVersion), + }, + }, }, + DescribeDBClustersWithContext: []*fake.CallDescribeDBClustersWithContext{ + { + Ctx: context.Background(), + I: &docdb.DescribeDBClustersInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, + ListTagsForResource: []*fake.CallListTagsForResource{ { I: &docdb.ListTagsForResourceInput{ @@ -2851,6 +3167,15 @@ func TestUpdate(t *testing.T) { }, }, nil }, + MockDescribeDBClustersWithContext: func(ctx context.Context, ddi *docdb.DescribeDBClustersInput, o []request.Option) (*docdb.DescribeDBClustersOutput, error) { + return &docdb.DescribeDBClustersOutput{ + DBClusters: []*docdb.DBCluster{ + { + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, nil + }, MockListTagsForResource: func(ltfri *docdb.ListTagsForResourceInput) (*docdb.ListTagsForResourceOutput, error) { return &docdb.ListTagsForResourceOutput{ TagList: []*docdb.Tag{}, @@ -2891,6 +3216,14 @@ func TestUpdate(t *testing.T) { }, }, }, + DescribeDBClustersWithContext: []*fake.CallDescribeDBClustersWithContext{ + { + Ctx: context.Background(), + I: &docdb.DescribeDBClustersInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, ListTagsForResource: []*fake.CallListTagsForResource{ { I: &docdb.ListTagsForResourceInput{ @@ -2912,6 +3245,15 @@ func TestUpdate(t *testing.T) { }, }, nil }, + MockDescribeDBClustersWithContext: func(ctx context.Context, ddi *docdb.DescribeDBClustersInput, o []request.Option) (*docdb.DescribeDBClustersOutput, error) { + return &docdb.DescribeDBClustersOutput{ + DBClusters: []*docdb.DBCluster{ + { + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, nil + }, MockListTagsForResource: func(ltfri *docdb.ListTagsForResourceInput) (*docdb.ListTagsForResourceOutput, error) { return &docdb.ListTagsForResourceOutput{ TagList: []*docdb.Tag{}, @@ -2951,6 +3293,14 @@ func TestUpdate(t *testing.T) { }, }, }, + DescribeDBClustersWithContext: []*fake.CallDescribeDBClustersWithContext{ + { + Ctx: context.Background(), + I: &docdb.DescribeDBClustersInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, ListTagsForResource: []*fake.CallListTagsForResource{ { I: &docdb.ListTagsForResourceInput{ @@ -2972,6 +3322,15 @@ func TestUpdate(t *testing.T) { }, }, nil }, + MockDescribeDBClustersWithContext: func(ctx context.Context, ddi *docdb.DescribeDBClustersInput, o []request.Option) (*docdb.DescribeDBClustersOutput, error) { + return &docdb.DescribeDBClustersOutput{ + DBClusters: []*docdb.DBCluster{ + { + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, nil + }, MockListTagsForResource: func(ltfri *docdb.ListTagsForResourceInput) (*docdb.ListTagsForResourceOutput, error) { return &docdb.ListTagsForResourceOutput{ TagList: []*docdb.Tag{}, @@ -3017,6 +3376,14 @@ func TestUpdate(t *testing.T) { }, }, }, + DescribeDBClustersWithContext: []*fake.CallDescribeDBClustersWithContext{ + { + Ctx: context.Background(), + I: &docdb.DescribeDBClustersInput{ + DBClusterIdentifier: pointer.ToOrNilIfZeroValue(testDBClusterIdentifier), + }, + }, + }, ListTagsForResource: []*fake.CallListTagsForResource{ { I: &docdb.ListTagsForResourceInput{ diff --git a/pkg/controller/docdb/dbcluster/zz_conversions.go b/pkg/controller/docdb/dbcluster/zz_conversions.go index eedd42ecca..627b0d4938 100644 --- a/pkg/controller/docdb/dbcluster/zz_conversions.go +++ b/pkg/controller/docdb/dbcluster/zz_conversions.go @@ -371,6 +371,9 @@ func GenerateCreateDBClusterInput(cr *svcapitypes.DBCluster) *svcsdk.CreateDBClu func GenerateModifyDBClusterInput(cr *svcapitypes.DBCluster) *svcsdk.ModifyDBClusterInput { res := &svcsdk.ModifyDBClusterInput{} + if cr.Spec.ForProvider.AllowMajorVersionUpgrade != nil { + res.SetAllowMajorVersionUpgrade(*cr.Spec.ForProvider.AllowMajorVersionUpgrade) + } if cr.Spec.ForProvider.BackupRetentionPeriod != nil { res.SetBackupRetentionPeriod(*cr.Spec.ForProvider.BackupRetentionPeriod) } diff --git a/pkg/controller/docdb/utils/engine_version.go b/pkg/controller/docdb/utils/engine_version.go new file mode 100644 index 0000000000..9458d727fc --- /dev/null +++ b/pkg/controller/docdb/utils/engine_version.go @@ -0,0 +1,85 @@ +package utils + +import ( + "strconv" + "strings" +) + +// EngineVersion represents an AWS DocDB engine version. +type EngineVersion []any + +// ParseEngineVersion from a raw string. +func ParseEngineVersion(raw string) EngineVersion { + split := strings.Split(raw, ".") + + v := make(EngineVersion, len(split)) + for i, s := range split { + d, err := strconv.Atoi(s) + if err != nil { + v[i] = s + } else { + v[i] = d + } + } + return v +} + +const ( + compareIsHigher = 1 + compareIsEqual = 0 + compareIsLower = -1 +) + +// Compare returns a positive value if v is represents a higher version number +// than other. A negative value is returned if other is higher than v. +// It returns 0 if both are considered equal. +func (v EngineVersion) Compare(other EngineVersion) int { + if other == nil { + return compareIsHigher + } + + for i := 0; i < len(v); i++ { + a := v.get(i) + b := other.get(i) + c := compareVersionComponents(a, b) + if c != 0 { + return c + } + } + return compareIsEqual +} + +func compareVersionComponents(a, b any) int { + if a == b { + return compareIsEqual + } + if b == nil { + return compareIsHigher + } + aI, aIsInt := a.(int) + bI, bIsInt := b.(int) + if aIsInt { + if bIsInt { + return aI - bI + } + return compareIsHigher + } + if bIsInt { + return compareIsLower + } + return compareIsEqual // We cannot decide if both are strings. +} + +func (v EngineVersion) get(i int) any { + if i >= 0 && i < len(v) { + return v[i] + } + return nil +} + +// CompareEngineVersions is a shortcut to compare two engine versions. +func CompareEngineVersions(a, b string) int { + av := ParseEngineVersion(a) + bv := ParseEngineVersion(b) + return av.Compare(bv) +} diff --git a/pkg/controller/docdb/utils/engine_version_test.go b/pkg/controller/docdb/utils/engine_version_test.go new file mode 100644 index 0000000000..b9bb674918 --- /dev/null +++ b/pkg/controller/docdb/utils/engine_version_test.go @@ -0,0 +1,83 @@ +package utils + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestEngineVersionIsHigherOrEqual(t *testing.T) { + type args struct { + spec string + current string + } + + type want struct { + result int + } + + cases := map[string]struct { + args + want + }{ + "DocDBIsEqual": { + args: args{ + spec: "5.0.0", + current: "5.0.0", + }, + want: want{ + result: 0, + }, + }, + "DocDBIsEqual2": { + args: args{ + spec: "5.0", + current: "5.0.0", + }, + want: want{ + result: 0, + }, + }, + "DocDBIsHigher": { + args: args{ + spec: "5.0", + current: "4.0.0", + }, + want: want{ + result: 1, + }, + }, + "DocDBIsLower": { + args: args{ + spec: "4.0.0", + current: "5.0.0", + }, + want: want{ + result: -1, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + specV := ParseEngineVersion(tc.args.spec) + curV := ParseEngineVersion(tc.args.current) + + res := specV.Compare(curV) + resSign := sign(res) + if diff := cmp.Diff(tc.want.result, resSign); diff != "" { + t.Errorf("r: -want, +got:\n%q\n%q\n%s", tc.args.spec, tc.args.current, diff) + } + }) + } +} + +func sign(x int) int { + if x < 0 { + return -1 + } + if x > 0 { + return 1 + } + return 0 +} From 39e248653bf70a1aae6d61f16cfd990803d762cf Mon Sep 17 00:00:00 2001 From: "Maximilian Blatt (external expert on behalf of DB Netz)" Date: Wed, 24 Jul 2024 14:18:55 +0200 Subject: [PATCH 2/2] chore: Use uptest v0.11.0 v0.12.0 is not yet available. Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1d3f68ddf8..bb97dc4854 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ GOLANGCILINT_VERSION = 1.54.2 UP_VERSION = v0.27.0 UP_CHANNEL = stable -UPTEST_VERSION = v0.12.0 +UPTEST_VERSION = v0.11.0 -include build/makelib/k8s_tools.mk