From a1755997b9b21c9a12abcdb9d1bf722a5b7f105c Mon Sep 17 00:00:00 2001 From: bfabricio Date: Tue, 18 Feb 2025 08:13:16 -0300 Subject: [PATCH 01/22] feat: add new providers package --- pkg/providers/aws.go | 26 +++++++++++++++ pkg/providers/cloudnative_pg.go | 26 +++++++++++++++ pkg/providers/gcp.go | 26 +++++++++++++++ pkg/providers/providers.go | 57 +++++++++++++++++++++++++++++++++ 4 files changed, 135 insertions(+) create mode 100644 pkg/providers/aws.go create mode 100644 pkg/providers/cloudnative_pg.go create mode 100644 pkg/providers/gcp.go create mode 100644 pkg/providers/providers.go diff --git a/pkg/providers/aws.go b/pkg/providers/aws.go new file mode 100644 index 0000000..c60bb45 --- /dev/null +++ b/pkg/providers/aws.go @@ -0,0 +1,26 @@ +package providers + +import ( + "context" + "github.com/spf13/viper" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type AWSProvider struct { +} + +func NewAWSProvider(k8sClient client.Client, config *viper.Viper) *AWSProvider { + return &AWSProvider{} +} + +func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) error { + return nil +} + +func (p *AWSProvider) DeleteDatabase(ctx context.Context, name string) error { + return nil +} + +func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { + return nil, nil +} diff --git a/pkg/providers/cloudnative_pg.go b/pkg/providers/cloudnative_pg.go new file mode 100644 index 0000000..f969b43 --- /dev/null +++ b/pkg/providers/cloudnative_pg.go @@ -0,0 +1,26 @@ +package providers + +import ( + "context" + "github.com/spf13/viper" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type CloudNativePGProvider struct { +} + +func NewCloudNativePGProvider(k8sClient client.Client, config *viper.Viper) *CloudNativePGProvider { + return &CloudNativePGProvider{} +} + +func (p *CloudNativePGProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) error { + return nil +} + +func (p *CloudNativePGProvider) DeleteDatabase(ctx context.Context, name string) error { + return nil +} + +func (p *CloudNativePGProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { + return nil, nil +} diff --git a/pkg/providers/gcp.go b/pkg/providers/gcp.go new file mode 100644 index 0000000..ead372e --- /dev/null +++ b/pkg/providers/gcp.go @@ -0,0 +1,26 @@ +package providers + +import ( + "context" + "github.com/spf13/viper" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type GCPProvider struct { +} + +func NewGCPProvider(k8sClient client.Client, config *viper.Viper) *GCPProvider { + return &GCPProvider{} +} + +func (p *GCPProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) error { + return nil +} + +func (p *GCPProvider) DeleteDatabase(ctx context.Context, name string) error { + return nil +} + +func (p *GCPProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { + return nil, nil +} diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go new file mode 100644 index 0000000..ce26f56 --- /dev/null +++ b/pkg/providers/providers.go @@ -0,0 +1,57 @@ +package providers + +import ( + "context" + "fmt" + v1 "github.com/infobloxopen/db-controller/api/v1" + "github.com/infobloxopen/db-controller/pkg/hostparams" + "github.com/spf13/viper" + + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// DatabaseSpec defines the required parameters to provision a database using any provider. +type DatabaseSpec struct { + ProviderResourceName string + DbType v1.DatabaseType + SharedDBHost bool + MasterConnInfo v1.DatabaseClaimConnectionInfo + TempSecret string + HostParams hostparams.HostParams + EnableReplicationRole bool + EnableSuperUser bool + EnablePerfInsight bool + EnableCloudwatchLogsExport []*string + BackupRetentionDays int64 + CACertificateIdentifier string + + Tags v1.Tag + Labels map[string]string + PreferredMaintenanceWindow *string + BackupPolicy string +} + +// Provider is an interface that abstracts provider-specific logic. +type Provider interface { + // CreateDatabase provisions a new database instance. + CreateDatabase(ctx context.Context, spec DatabaseSpec) error + // DeleteDatabase deprovisions an existing database instance. + DeleteDatabase(ctx context.Context, name string) error + // GetDatabase retrieves the current status of a database instance. + GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) +} + +// NewProvider returns a concrete provider implementation based on the given provider name. +func NewProvider(config *viper.Viper, k8sClient client.Client) (Provider, error) { + cloud := config.GetString("cloud") + switch cloud { + case "aws": + return NewAWSProvider(k8sClient, config), nil + case "gcp": + return NewGCPProvider(k8sClient, config), nil + case "cloudnative-pg": + return nil, nil + default: + return nil, fmt.Errorf("unsupported provider for cloud: %s", cloud) + } +} From f93542154016a18c89fe35421f69f5534ea666ab Mon Sep 17 00:00:00 2001 From: bfabricio Date: Thu, 27 Feb 2025 08:03:44 -0300 Subject: [PATCH 02/22] feat: add aws crossplane provider --- claim.yaml | 0 pkg/databaseclaim/databaseclaim.go | 9 +- pkg/databaseclaim/requestinfo.go | 37 + pkg/providers/aws.go | 26 - pkg/providers/cloudnative_pg.go | 2 +- pkg/providers/crossplane_aws.go | 770 ++++++++++++++++++++ pkg/providers/crossplane_aws_test.go | 157 ++++ pkg/providers/{gcp.go => crossplane_gcp.go} | 8 +- pkg/providers/providers.go | 28 +- pkg/providers/tags.go | 58 ++ pkg/providers/utils.go | 97 +++ 11 files changed, 1147 insertions(+), 45 deletions(-) create mode 100644 claim.yaml delete mode 100644 pkg/providers/aws.go create mode 100644 pkg/providers/crossplane_aws.go create mode 100644 pkg/providers/crossplane_aws_test.go rename pkg/providers/{gcp.go => crossplane_gcp.go} (66%) create mode 100644 pkg/providers/tags.go create mode 100644 pkg/providers/utils.go diff --git a/claim.yaml b/claim.yaml new file mode 100644 index 0000000..e69de29 diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index 7ce2dfd..9b0297a 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -4,6 +4,7 @@ import ( "context" "fmt" crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" + "github.com/infobloxopen/db-controller/pkg/providers" crossplanegcp "github.com/upbound/provider-gcp/apis/alloydb/v1beta2" "strings" "time" @@ -79,6 +80,7 @@ type DatabaseClaimReconciler struct { Config *DatabaseClaimConfig kctl *kctlutils.Client statusManager *StatusManager + provider providers.Provider } // New returns a configured databaseclaim reconciler @@ -88,6 +90,7 @@ func New(cli client.Client, cfg *DatabaseClaimConfig) *DatabaseClaimReconciler { Config: cfg, kctl: kctlutils.New(cli, cfg.Viper.GetString("SERVICE_NAMESPACE")), statusManager: NewStatusManager(cli, cfg.Viper), + provider: providers.NewProvider(cfg.Viper, cli, cfg.Namespace), } } @@ -189,7 +192,8 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques } if basefun.GetCloud(r.Config.Viper) == "aws" { // our finalizer is present, so lets handle any external dependency - if err := r.deleteExternalResourcesAWS(ctx, &reqInfo, &dbClaim); err != nil { + spec := NewDatabaseSpecFromRequestInfo(&reqInfo, &dbClaim, r.getMode(ctx, &reqInfo, &dbClaim), r.Config.Viper) + if err := r.provider.DeleteDatabase(ctx, spec); err != nil { // if fail to delete the external dependency here, return with error // so that it can be retried return ctrl.Result{}, err @@ -514,7 +518,8 @@ func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, reqInfo *r isReady := false var err error if cloud == "aws" { - isReady, err = r.manageCloudHostAWS(ctx, reqInfo, dbClaim, operationalMode) + spec := NewDatabaseSpecFromRequestInfo(reqInfo, dbClaim, operationalMode, r.Config.Viper) + isReady, err = r.provider.CreateDatabase(ctx, spec) if err != nil { logr.Error(err, "manage_cloud_host_AWS") return ctrl.Result{}, err diff --git a/pkg/databaseclaim/requestinfo.go b/pkg/databaseclaim/requestinfo.go index 455f7fe..b8672c7 100644 --- a/pkg/databaseclaim/requestinfo.go +++ b/pkg/databaseclaim/requestinfo.go @@ -3,6 +3,7 @@ package databaseclaim import ( "context" "fmt" + "github.com/infobloxopen/db-controller/pkg/providers" v1 "github.com/infobloxopen/db-controller/api/v1" basefun "github.com/infobloxopen/db-controller/pkg/basefunctions" @@ -94,3 +95,39 @@ func NewRequestInfo(ctx context.Context, cfg *viper.Viper, dbClaim *v1.DatabaseC return ri, nil } + +func NewDatabaseSpecFromRequestInfo(ri *requestInfo, claim *v1.DatabaseClaim, mode ModeEnum, cfg *viper.Viper) providers.DatabaseSpec { + var snapshotID *string + if mode == M_UseNewDB && claim.Spec.RestoreFrom != "" { + snapshotID = &claim.Spec.RestoreFrom + } + + var prefix string + suffix := "-" + ri.HostParams.Hash() + + if basefun.GetDBIdentifierPrefix(cfg) != "" { + prefix = basefun.GetDBIdentifierPrefix(cfg) + "-" + } + + return providers.DatabaseSpec{ + ResourceName: prefix + claim.Name + suffix, + HostParams: ri.HostParams, + DbType: ri.DbType, + SharedDBHost: ri.SharedDBHost, + MasterConnInfo: ri.MasterConnInfo, + TempSecret: ri.TempSecret, + EnableReplicationRole: ri.EnableReplicationRole, + EnableSuperUser: ri.EnableSuperUser, + EnablePerfInsight: ri.EnablePerfInsight, + EnableCloudwatchLogsExport: ri.EnableCloudwatchLogsExport, + BackupRetentionDays: ri.BackupRetentionDays, + CACertificateIdentifier: &ri.CACertificateIdentifier, + Tags: providers.ConvertToProviderTags(claim.Spec.Tags, func(tag v1.Tag) (string, string) { + return tag.Key, tag.Value + }), + Labels: claim.Labels, + PreferredMaintenanceWindow: claim.Spec.PreferredMaintenanceWindow, + BackupPolicy: claim.Spec.BackupPolicy, // this is added as a TAG + SnapshotID: snapshotID, + } +} diff --git a/pkg/providers/aws.go b/pkg/providers/aws.go deleted file mode 100644 index c60bb45..0000000 --- a/pkg/providers/aws.go +++ /dev/null @@ -1,26 +0,0 @@ -package providers - -import ( - "context" - "github.com/spf13/viper" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -type AWSProvider struct { -} - -func NewAWSProvider(k8sClient client.Client, config *viper.Viper) *AWSProvider { - return &AWSProvider{} -} - -func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) error { - return nil -} - -func (p *AWSProvider) DeleteDatabase(ctx context.Context, name string) error { - return nil -} - -func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { - return nil, nil -} diff --git a/pkg/providers/cloudnative_pg.go b/pkg/providers/cloudnative_pg.go index f969b43..d18260a 100644 --- a/pkg/providers/cloudnative_pg.go +++ b/pkg/providers/cloudnative_pg.go @@ -17,7 +17,7 @@ func (p *CloudNativePGProvider) CreateDatabase(ctx context.Context, spec Databas return nil } -func (p *CloudNativePGProvider) DeleteDatabase(ctx context.Context, name string) error { +func (p *CloudNativePGProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) error { return nil } diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go new file mode 100644 index 0000000..452f285 --- /dev/null +++ b/pkg/providers/crossplane_aws.go @@ -0,0 +1,770 @@ +package providers + +import ( + "context" + "fmt" + crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + v1 "github.com/infobloxopen/db-controller/api/v1" + basefun "github.com/infobloxopen/db-controller/pkg/basefunctions" + "github.com/spf13/viper" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +const ( + MasterPasswordSuffix = "-master" + MasterPasswordSecretKey = "password" + SnapshotSource = "Snapshot" +) + +type AWSProvider struct { + k8sClient client.Client + config *viper.Viper + serviceNS string +} + +func newAWSProvider(k8sClient client.Client, config *viper.Viper, serviceNS string) Provider { + return &AWSProvider{ + k8sClient: k8sClient, + config: config, + serviceNS: serviceNS, + } +} + +func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) { + switch spec.DbType { + case v1.AuroraPostgres: + err := p.createAuroraDB(ctx, spec) + if err != nil { + return false, err + } + instanceReady, err := p.isDBInstanceReady(ctx, spec.ResourceName) + if err != nil { + return false, err + } + clusterReady, err := p.isDBClusterReady(ctx, spec.ResourceName) + if err != nil { + return false, err + } + if basefun.GetMultiAZEnabled(p.config) { + instance2Ready, err := p.isDBInstanceReady(ctx, spec.ResourceName+"-2") + if err != nil { + return false, err + } + return instanceReady && instance2Ready && clusterReady, nil + } + + return instanceReady && clusterReady, nil + case v1.Postgres: + err := p.createPostgres(ctx, spec) + if err != nil { + return false, err + } + instanceReady, err := p.isDBInstanceReady(ctx, spec.ResourceName) + if err != nil { + return false, err + } + return instanceReady, nil + } + return false, fmt.Errorf("%w: %q must be one of %s", v1.ErrInvalidDBType, spec.DbType, []v1.DatabaseType{v1.Postgres, v1.AuroraPostgres}) +} + +func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) error { + deletionPolicy := client.PropagationPolicy(metav1.DeletePropagationBackground) + paramGroupName := getParameterGroupName(spec.ResourceName, spec.HostParams.DBVersion, spec.DbType) + // Delete DBClusterParameterGroup if it exists + dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} + clusterParamGroupKey := client.ObjectKey{Name: paramGroupName} + if err := p.k8sClient.Get(ctx, clusterParamGroupKey, dbClusterParamGroup); err == nil { + if err := p.k8sClient.Delete(ctx, dbClusterParamGroup, deletionPolicy); err != nil { + return err + } + } + + // Delete DBParameterGroup if it exists + dbParameterGroup := &crossplaneaws.DBParameterGroup{} + paramGroupKey := client.ObjectKey{Name: paramGroupName} + if err := p.k8sClient.Get(ctx, paramGroupKey, dbParameterGroup); err == nil { + if err := p.k8sClient.Delete(ctx, dbParameterGroup, deletionPolicy); err != nil { + return err + } + } + + // Delete DBCluster if it exists + dbCluster := &crossplaneaws.DBCluster{} + clusterKey := client.ObjectKey{Name: spec.ResourceName} + if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err == nil { + if err := p.k8sClient.Delete(ctx, dbCluster, deletionPolicy); err != nil { + return err + } + } + + // Delete DBInstance if it exists + instanceKey := client.ObjectKey{Name: spec.ResourceName} + dbInstance := &crossplaneaws.DBInstance{} + if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err == nil { + if err := p.k8sClient.Delete(ctx, dbInstance, deletionPolicy); err != nil { + return err + } + } + + // Delete secondary DBInstance if it exists + dbInstance2 := &crossplaneaws.DBInstance{} + instance2Key := client.ObjectKey{Name: spec.ResourceName + "-2"} + if err := p.k8sClient.Get(ctx, instance2Key, dbInstance2); err == nil { + if err := p.k8sClient.Delete(ctx, dbInstance2, deletionPolicy); err != nil { + return err + } + } + + return nil +} + +func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { + return nil, nil +} + +func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) error { + // Handle database parameter group + paramGroupName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + dbParamGroup := &crossplaneaws.DBParameterGroup{} + + if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { + if errors.IsNotFound(err) { + if err := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { + return fmt.Errorf("failed to create DB parameter group: %w", err) + } + } else { + return fmt.Errorf("failed to get DB parameter group: %w", err) + } + } + + // Handle database instance + dbInstance := &crossplaneaws.DBInstance{} + instanceKey := client.ObjectKey{Name: params.ResourceName} + + if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err != nil { + if errors.IsNotFound(err) { + dbInstance = p.postgresDBInstance(params) + + // Create master password secret before creating the DB instance + passwordErr := ManageMasterPassword(ctx, dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef, p.k8sClient) + if passwordErr != nil { + return fmt.Errorf("failed to manage master password: %w", passwordErr) + } + + if err := p.k8sClient.Create(ctx, dbInstance); err != nil { + return fmt.Errorf("failed to create DB instance: %w", err) + } + } else { + return fmt.Errorf("failed to get DB instance: %w", err) + } + } + + if !dbInstance.ObjectMeta.DeletionTimestamp.IsZero() { + return fmt.Errorf("can not create Cloud DB instance %s it is being deleted", params.ResourceName) + } + + err := p.updateDBInstance(ctx, params, dbInstance) + if err != nil { + return err + } + + return nil +} + +func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) error { + // Handle database instance parameter group + paramGroupName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + dbParamGroup := &crossplaneaws.DBParameterGroup{} + if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { + if errors.IsNotFound(err) { + if err := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { + return fmt.Errorf("failed to create DB parameter group: %w", err) + } + } else { + return fmt.Errorf("failed to get DB parameter group: %w", err) + } + } + + // Handle database cluster parameter group + dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} + if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbClusterParamGroup); err != nil { + if errors.IsNotFound(err) { + if err := p.k8sClient.Create(ctx, p.auroraClusterParamGroup(params)); err != nil { + return fmt.Errorf("failed to create DB parameter group: %w", err) + } + } else { + return fmt.Errorf("failed to get DB parameter group: %w", err) + } + } + + // Handle cluster + dbCluster := &crossplaneaws.DBCluster{} + clusterKey := client.ObjectKey{Name: params.ResourceName} + if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err != nil { + if errors.IsNotFound(err) { + dbCluster = p.auroraDBCluster(params) + // Create master password secret before creating the DB instance + passwordErr := ManageMasterPassword(ctx, dbCluster.Spec.ForProvider.CustomDBClusterParameters.MasterUserPasswordSecretRef, p.k8sClient) + if passwordErr != nil { + return fmt.Errorf("failed to manage master password: %w", passwordErr) + } + + if err := p.k8sClient.Create(ctx, dbCluster); err != nil { + return fmt.Errorf("failed to create DB instance: %w", err) + } + } else { + return fmt.Errorf("failed to get DB instance: %w", err) + } + } + + // Handle database instance + primaryDbInstance := &crossplaneaws.DBInstance{} + primaryInstanceKey := client.ObjectKey{Name: params.ResourceName} + if err := p.k8sClient.Get(ctx, primaryInstanceKey, primaryDbInstance); err != nil { + if errors.IsNotFound(err) { + primaryDbInstance = p.auroraDBInstance(params, false) + if err := p.k8sClient.Create(ctx, primaryDbInstance); err != nil { + return fmt.Errorf("failed to create DB instance: %w", err) + } + } else { + return fmt.Errorf("failed to get DB instance: %w", err) + } + } + + if !primaryDbInstance.ObjectMeta.DeletionTimestamp.IsZero() || !dbCluster.ObjectMeta.DeletionTimestamp.IsZero() { + return fmt.Errorf("can not create Cloud DB instance %s it is being deleted", params.ResourceName) + } + + err := p.auroraUpdateDBCluster(ctx, params, dbCluster) + if err != nil { + return err + } + + err = p.updateDBInstance(ctx, params, primaryDbInstance) + if err != nil { + return err + } + + if basefun.GetMultiAZEnabled(p.config) { + secondaryDbInstance := &crossplaneaws.DBInstance{} + secondaryInstanceKey := client.ObjectKey{Name: params.ResourceName + "-2"} + if err := p.k8sClient.Get(ctx, secondaryInstanceKey, secondaryDbInstance); err != nil { + if errors.IsNotFound(err) { + secondaryDbInstance = p.auroraDBInstance(params, true) + if err := p.k8sClient.Create(ctx, secondaryDbInstance); err != nil { + return fmt.Errorf("failed to create DB instance: %w", err) + } + } else { + return fmt.Errorf("failed to get DB instance: %w", err) + } + } + + if !secondaryDbInstance.ObjectMeta.DeletionTimestamp.IsZero() { + return fmt.Errorf("can not create Cloud DB instance %s it is being deleted", params.ResourceName) + } + + err := p.updateDBInstance(ctx, params, secondaryDbInstance) + if err != nil { + return err + } + } + + return nil +} + +func (p *AWSProvider) postgresDBInstance(params DatabaseSpec) *crossplaneaws.DBInstance { + ms64 := int64(params.HostParams.MinStorageGB) + multiAZ := basefun.GetMultiAZEnabled(p.config) + + var maxStorageVal *int64 + if params.HostParams.MaxStorageGB == 0 { + maxStorageVal = nil + } else { + maxStorageVal = ¶ms.HostParams.MaxStorageGB + } + + var restoreFrom *crossplaneaws.RestoreDBInstanceBackupConfiguration + if params.SnapshotID != nil { + restoreFrom = &crossplaneaws.RestoreDBInstanceBackupConfiguration{ + Snapshot: &crossplaneaws.SnapshotRestoreBackupConfiguration{ + SnapshotIdentifier: params.SnapshotID, + }, + Source: ptr.To(SnapshotSource), + } + } + + p.configureDBTags(¶ms) + + return &crossplaneaws.DBInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: params.ResourceName, + Labels: params.Labels, + }, + Spec: crossplaneaws.DBInstanceSpec{ + ForProvider: crossplaneaws.DBInstanceParameters{ + CACertificateIdentifier: params.CACertificateIdentifier, + Region: basefun.GetRegion(p.config), + CustomDBInstanceParameters: crossplaneaws.CustomDBInstanceParameters{ + ApplyImmediately: ptr.To(true), + SkipFinalSnapshot: params.HostParams.SkipFinalSnapshotBeforeDeletion, + VPCSecurityGroupIDRefs: []xpv1.Reference{ + {Name: basefun.GetVpcSecurityGroupIDRefs(p.config)}, + }, + DBSubnetGroupNameRef: &xpv1.Reference{ + Name: basefun.GetDbSubnetGroupNameRef(p.config), + }, + AutogeneratePassword: true, + MasterUserPasswordSecretRef: &xpv1.SecretKeySelector{ + SecretReference: xpv1.SecretReference{ + Name: params.ResourceName + MasterPasswordSuffix, + Namespace: p.serviceNS, + }, + Key: MasterPasswordSecretKey, + }, + EngineVersion: GetEngineVersion(params.HostParams, p.config), + RestoreFrom: restoreFrom, + DBParameterGroupNameRef: &xpv1.Reference{ + Name: getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType), + }, + }, + Engine: ¶ms.HostParams.Type, + MultiAZ: &multiAZ, + DBInstanceClass: ¶ms.HostParams.InstanceClass, + AllocatedStorage: &ms64, + MaxAllocatedStorage: maxStorageVal, + MasterUsername: ¶ms.HostParams.MasterUsername, + PubliclyAccessible: ¶ms.HostParams.PubliclyAccessible, + EnableIAMDatabaseAuthentication: ¶ms.HostParams.EnableIAMDatabaseAuthentication, + EnablePerformanceInsights: ¶ms.EnablePerfInsight, + EnableCloudwatchLogsExports: params.EnableCloudwatchLogsExport, + BackupRetentionPeriod: ¶ms.BackupRetentionDays, + StorageEncrypted: ptr.To(true), + StorageType: ¶ms.HostParams.StorageType, + Port: ¶ms.HostParams.Port, + PreferredMaintenanceWindow: params.PreferredMaintenanceWindow, + Tags: ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { + return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} + }), + }, + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: params.ResourceName, + Namespace: p.serviceNS, + }, + ProviderConfigReference: &xpv1.Reference{ + Name: basefun.GetProviderConfig(p.config), + }, + DeletionPolicy: params.HostParams.DeletionPolicy, + }, + }, + } +} + +func (p *AWSProvider) postgresDBParameterGroup(params DatabaseSpec) *crossplaneaws.DBParameterGroup { + logical := "rds.logical_replication" + one := "1" + immediate := "immediate" + reboot := "pending-reboot" + forceSsl := "rds.force_ssl" + transactionTimeout := "idle_in_transaction_session_timeout" + transactionTimeoutValue := "300000" + pgName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + sharedLib := "shared_preload_libraries" + sharedLibValue := "pg_stat_statements,pg_cron" + cron := "cron.database_name" + cronValue := params.MasterConnInfo.DatabaseName + desc := "custom PG for " + pgName + + return &crossplaneaws.DBParameterGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: pgName, + }, + Spec: crossplaneaws.DBParameterGroupSpec{ + ForProvider: crossplaneaws.DBParameterGroupParameters{ + Region: basefun.GetRegion(p.config), + Description: &desc, + CustomDBParameterGroupParameters: crossplaneaws.CustomDBParameterGroupParameters{ + DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ + Engine: params.HostParams.Type, + EngineVersion: GetEngineVersion(params.HostParams, p.config), + }, + Parameters: []crossplaneaws.CustomParameter{ + {ParameterName: &logical, + ParameterValue: &one, + ApplyMethod: &reboot, + }, + {ParameterName: &forceSsl, + ParameterValue: &one, + ApplyMethod: &immediate, + }, + {ParameterName: &transactionTimeout, + ParameterValue: &transactionTimeoutValue, + ApplyMethod: &immediate, + }, + {ParameterName: &sharedLib, + ParameterValue: &sharedLibValue, + ApplyMethod: &reboot, + }, + {ParameterName: &cron, + ParameterValue: &cronValue, + ApplyMethod: &reboot, + }, + }, + }, + }, + ResourceSpec: xpv1.ResourceSpec{ + ProviderConfigReference: &xpv1.Reference{ + Name: basefun.GetProviderConfig(p.config), + }, + DeletionPolicy: params.HostParams.DeletionPolicy, + }, + }, + } +} + +func (p *AWSProvider) updateDBInstance(ctx context.Context, params DatabaseSpec, dbInstance *crossplaneaws.DBInstance) error { + logr := log.FromContext(ctx) + + // Create a patch snapshot from current DBInstance + patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) + + if params.DbType == v1.Postgres { + multiAZ := basefun.GetMultiAZEnabled(p.config) + ms64 := int64(params.HostParams.MinStorageGB) + dbInstance.Spec.ForProvider.AllocatedStorage = &ms64 + + var maxStorageVal *int64 + if params.HostParams.MaxStorageGB == 0 { + maxStorageVal = nil + } else { + maxStorageVal = ¶ms.HostParams.MaxStorageGB + } + dbInstance.Spec.ForProvider.MaxAllocatedStorage = maxStorageVal + dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports = params.EnableCloudwatchLogsExport + dbInstance.Spec.ForProvider.MultiAZ = &multiAZ + } + enablePerfInsight := params.EnablePerfInsight + dbInstance.Spec.ForProvider.EnablePerformanceInsights = &enablePerfInsight + dbInstance.Spec.DeletionPolicy = params.HostParams.DeletionPolicy + dbInstance.Spec.ForProvider.CACertificateIdentifier = params.CACertificateIdentifier + if params.DbType == v1.AuroraPostgres { + dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports = nil + } + // Compute a json patch based on the changed DBInstance + dbInstancePatchData, err := patchDBInstance.Data(dbInstance) + if err != nil { + return err + } + // an empty json patch will be {}, we can assert that no update is required if len == 2 + // we could also just apply the empty patch if additional call to apiserver isn't an issue + if len(dbInstancePatchData) <= 2 { + return nil + } + + if params.PreferredMaintenanceWindow != nil { + dbInstance.Spec.ForProvider.PreferredMaintenanceWindow = params.PreferredMaintenanceWindow + } + + logr.Info("updating crossplane DBInstance resource", "DBInstance", dbInstance.Name) + err = p.k8sClient.Patch(ctx, dbInstance, patchDBInstance) + if err != nil { + return err + } + + return nil +} + +func (p *AWSProvider) auroraDBInstance(params DatabaseSpec, isSecondInstance bool) *crossplaneaws.DBInstance { + dbHostname := params.ResourceName + if isSecondInstance { + dbHostname = dbHostname + "-2" + } + + p.configureDBTags(¶ms) + + return &crossplaneaws.DBInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: dbHostname, + Labels: params.Labels, + }, + Spec: crossplaneaws.DBInstanceSpec{ + ForProvider: crossplaneaws.DBInstanceParameters{ + CACertificateIdentifier: params.CACertificateIdentifier, + Region: basefun.GetRegion(p.config), + CustomDBInstanceParameters: crossplaneaws.CustomDBInstanceParameters{ + ApplyImmediately: ptr.To(true), + SkipFinalSnapshot: params.HostParams.SkipFinalSnapshotBeforeDeletion, + EngineVersion: GetEngineVersion(params.HostParams, p.config), + DBParameterGroupNameRef: &xpv1.Reference{ + Name: getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType), + }, + }, + Engine: ¶ms.HostParams.Type, + DBInstanceClass: ¶ms.HostParams.InstanceClass, + PubliclyAccessible: ¶ms.HostParams.PubliclyAccessible, + DBClusterIdentifier: ¶ms.ResourceName, + EnablePerformanceInsights: ¶ms.EnablePerfInsight, + EnableCloudwatchLogsExports: nil, + PreferredMaintenanceWindow: params.PreferredMaintenanceWindow, + Tags: ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { + return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} + }), + }, + ResourceSpec: xpv1.ResourceSpec{ + ProviderConfigReference: &xpv1.Reference{ + Name: basefun.GetProviderConfig(p.config), + }, + DeletionPolicy: params.HostParams.DeletionPolicy, + }, + }, + } + +} + +func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBCluster { + var auroraBackupRetentionPeriod *int64 + if params.BackupRetentionDays != 0 { + auroraBackupRetentionPeriod = ¶ms.BackupRetentionDays + } else { + auroraBackupRetentionPeriod = nil + } + + p.configureDBTags(¶ms) + + return &crossplaneaws.DBCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: params.ResourceName, + }, + Spec: crossplaneaws.DBClusterSpec{ + ForProvider: crossplaneaws.DBClusterParameters{ + Region: basefun.GetRegion(p.config), + BackupRetentionPeriod: auroraBackupRetentionPeriod, + CustomDBClusterParameters: crossplaneaws.CustomDBClusterParameters{ + SkipFinalSnapshot: params.HostParams.SkipFinalSnapshotBeforeDeletion, + VPCSecurityGroupIDRefs: []xpv1.Reference{ + {Name: basefun.GetVpcSecurityGroupIDRefs(p.config)}, + }, + DBSubnetGroupNameRef: &xpv1.Reference{ + Name: basefun.GetDbSubnetGroupNameRef(p.config), + }, + AutogeneratePassword: true, + MasterUserPasswordSecretRef: &xpv1.SecretKeySelector{ + SecretReference: xpv1.SecretReference{ + Name: params.ResourceName + MasterPasswordSuffix, + Namespace: p.serviceNS, + }, + Key: MasterPasswordSuffix, + }, + DBClusterParameterGroupNameRef: &xpv1.Reference{ + Name: getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType), + }, + EngineVersion: GetEngineVersion(params.HostParams, p.config), + }, + Engine: ¶ms.HostParams.Type, + Tags: ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { + return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} + }), + MasterUsername: ¶ms.HostParams.MasterUsername, + EnableIAMDatabaseAuthentication: ¶ms.HostParams.EnableIAMDatabaseAuthentication, + StorageEncrypted: ptr.To(true), + StorageType: ¶ms.HostParams.StorageType, + Port: ¶ms.HostParams.Port, + EnableCloudwatchLogsExports: params.EnableCloudwatchLogsExport, + IOPS: nil, + PreferredMaintenanceWindow: params.PreferredMaintenanceWindow, + }, + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: params.ResourceName, + Namespace: p.serviceNS, + }, + ProviderConfigReference: &xpv1.Reference{ + Name: basefun.GetProviderConfig(p.config), + }, + DeletionPolicy: params.HostParams.DeletionPolicy, + }, + }, + } +} + +func (p *AWSProvider) auroraClusterParamGroup(params DatabaseSpec) *crossplaneaws.DBClusterParameterGroup { + logical := "rds.logical_replication" + one := "1" + immediate := "immediate" + reboot := "pending-reboot" + forceSsl := "rds.force_ssl" + transactionTimeout := "idle_in_transaction_session_timeout" + transactionTimeoutValue := "300000" + pgName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + sharedLib := "shared_preload_libraries" + sharedLibValue := "pg_stat_statements,pg_cron" + cron := "cron.database_name" + cronValue := params.MasterConnInfo.DatabaseName + desc := "custom PG for " + pgName + + return &crossplaneaws.DBClusterParameterGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: pgName, + }, + Spec: crossplaneaws.DBClusterParameterGroupSpec{ + ForProvider: crossplaneaws.DBClusterParameterGroupParameters{ + Region: basefun.GetRegion(p.config), + Description: &desc, + CustomDBClusterParameterGroupParameters: crossplaneaws.CustomDBClusterParameterGroupParameters{ + DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ + Engine: params.HostParams.Type, + EngineVersion: GetEngineVersion(params.HostParams, p.config), + }, + Parameters: []crossplaneaws.CustomParameter{ + {ParameterName: &logical, + ParameterValue: &one, + ApplyMethod: &reboot, + }, + {ParameterName: &forceSsl, + ParameterValue: &one, + ApplyMethod: &immediate, + }, + {ParameterName: &transactionTimeout, + ParameterValue: &transactionTimeoutValue, + ApplyMethod: &immediate, + }, + {ParameterName: &sharedLib, + ParameterValue: &sharedLibValue, + ApplyMethod: &reboot, + }, + {ParameterName: &cron, + ParameterValue: &cronValue, + ApplyMethod: &reboot, + }, + }, + }, + }, + ResourceSpec: xpv1.ResourceSpec{ + ProviderConfigReference: &xpv1.Reference{ + Name: basefun.GetProviderConfig(p.config), + }, + DeletionPolicy: params.HostParams.DeletionPolicy, + }, + }, + } +} + +func (p *AWSProvider) auroraInstanceParamGroup(params DatabaseSpec) *crossplaneaws.DBParameterGroup { + immediate := "immediate" + reboot := "pending-reboot" + transactionTimeout := "idle_in_transaction_session_timeout" + transactionTimeoutValue := "300000" + pgName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + sharedLib := "shared_preload_libraries" + sharedLibValue := "pg_stat_statements,pg_cron" + cron := "cron.database_name" + cronValue := params.MasterConnInfo.DatabaseName + desc := "custom PG for " + pgName + + return &crossplaneaws.DBParameterGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: pgName, + }, + Spec: crossplaneaws.DBParameterGroupSpec{ + ForProvider: crossplaneaws.DBParameterGroupParameters{ + Region: basefun.GetRegion(p.config), + Description: &desc, + CustomDBParameterGroupParameters: crossplaneaws.CustomDBParameterGroupParameters{ + DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ + Engine: params.HostParams.Type, + EngineVersion: GetEngineVersion(params.HostParams, p.config), + }, + Parameters: []crossplaneaws.CustomParameter{ + {ParameterName: &transactionTimeout, + ParameterValue: &transactionTimeoutValue, + ApplyMethod: &immediate, + }, + {ParameterName: &sharedLib, + ParameterValue: &sharedLibValue, + ApplyMethod: &reboot, + }, + {ParameterName: &cron, + ParameterValue: &cronValue, + ApplyMethod: &reboot, + }, + }, + }, + }, + ResourceSpec: xpv1.ResourceSpec{ + ProviderConfigReference: &xpv1.Reference{ + Name: basefun.GetProviderConfig(p.config), + }, + DeletionPolicy: params.HostParams.DeletionPolicy, + }, + }, + } +} + +func (p *AWSProvider) auroraUpdateDBCluster(ctx context.Context, params DatabaseSpec, dbCluster *crossplaneaws.DBCluster) error { + logr := log.FromContext(ctx) + + // Create a patch snapshot from current DBCluster + patchDBCluster := client.MergeFrom(dbCluster.DeepCopy()) + p.configureDBTags(¶ms) + // Update DBCluster + dbCluster.Spec.ForProvider.Tags = ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { + return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} + }) + if params.BackupRetentionDays != 0 { + dbCluster.Spec.ForProvider.BackupRetentionPeriod = ¶ms.BackupRetentionDays + } + dbCluster.Spec.ForProvider.StorageType = ¶ms.HostParams.StorageType + dbCluster.Spec.DeletionPolicy = params.HostParams.DeletionPolicy + + // Compute a json patch based on the changed RDSInstance + dbClusterPatchData, err := patchDBCluster.Data(dbCluster) + if err != nil { + return err + } + // an empty json patch will be {}, we can assert that no update is required if len == 2 + // we could also just apply the empty patch if additional call to apiserver isn't an issue + if len(dbClusterPatchData) <= 2 { + return nil + } + + logr.Info("updating crossplane DBCluster resource", "DBCluster", dbCluster.Name) + err = p.k8sClient.Patch(ctx, dbCluster, patchDBCluster) + if err != nil { + return err + } + + return nil +} + +func (p *AWSProvider) configureDBTags(params *DatabaseSpec) { + backupPolicy := params.BackupPolicy + if backupPolicy == "" { + backupPolicy = basefun.GetDefaultBackupPolicy(p.config) + } + + params.Tags = MergeTags(params.Tags, []ProviderTag{OperationalTAGActive, {Key: BackupPolicyKey, Value: backupPolicy}}) +} + +func (p *AWSProvider) isDBInstanceReady(ctx context.Context, instanceName string) (bool, error) { + dbInstance := &crossplaneaws.DBInstance{} + instanceKey := client.ObjectKey{Name: instanceName} + if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err != nil { + return false, err + } + return isReady(dbInstance.Status.Conditions) +} + +func (p *AWSProvider) isDBClusterReady(ctx context.Context, clusterName string) (bool, error) { + dbCluster := &crossplaneaws.DBCluster{} + clusterKey := client.ObjectKey{Name: clusterName} + if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err != nil { + return false, err + } + return isReady(dbCluster.Status.Conditions) +} diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go new file mode 100644 index 0000000..4049759 --- /dev/null +++ b/pkg/providers/crossplane_aws_test.go @@ -0,0 +1,157 @@ +package providers + +import ( + "context" + crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" + v1 "github.com/infobloxopen/db-controller/api/v1" + "github.com/infobloxopen/db-controller/pkg/config" + "github.com/infobloxopen/db-controller/pkg/hostparams" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/spf13/viper" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" + "path/filepath" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "testing" + + k8sRuntime "k8s.io/apimachinery/pkg/runtime" + + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var k8sClient client.Client +var controllerConfig *viper.Viper + +func TestAWSProvider(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "AWS Provider Test Suite") +} + +var _ = BeforeSuite(func() { + By("bootstrapping test environment") + sch := k8sRuntime.NewScheme() + err := scheme.AddToScheme(sch) + Expect(err).NotTo(HaveOccurred()) + err = crossplaneaws.AddToScheme(sch) + Expect(err).NotTo(HaveOccurred()) + k8sClient = fake.NewClientBuilder().WithScheme(sch).Build() + + By("setting up the database controller") + configPath, err := filepath.Abs(filepath.Join("..", "..", "cmd", "config", "config.yaml")) + Expect(err).NotTo(HaveOccurred()) + controllerConfig = config.NewConfig(configPath) +}) + +var _ = AfterSuite(func() {}) + +var _ = Describe("AWSProvider createPostgres", func() { + var ( + provider *AWSProvider + ctx context.Context + ) + + BeforeEach(func() { + ctx = context.TODO() + provider = &AWSProvider{ + k8sClient: k8sClient, + config: controllerConfig, + serviceNS: "db-controller", + } + }) + + AfterEach(func() { + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBInstance{})).To(Succeed()) + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBParameterGroup{})).To(Succeed()) + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBCluster{})).To(Succeed()) + }) + + It("Should create postgres DBInstance with correct parameters", func() { + spec := DatabaseSpec{ + ResourceName: "env-app-name-db-1d9fb876", + HostParams: hostparams.HostParams{ + Shape: "db.t3.medium", + MinStorageGB: 20, + DBVersion: "15.7", + }, + DbType: "postgres", + SharedDBHost: false, + MasterConnInfo: v1.DatabaseClaimConnectionInfo{ + Username: "testadmin", + Password: "test-password-123", + Port: "5432", + }, + TempSecret: "temp-secret-abc123", + EnableReplicationRole: true, + EnableSuperUser: false, + EnablePerfInsight: true, + EnableCloudwatchLogsExport: []*string{ptr.To("postgresql"), ptr.To("upgrade")}, + BackupRetentionDays: 7, + CACertificateIdentifier: ptr.To("rds-ca-2019"), + Tags: []ProviderTag{ + {Key: "environment", Value: "test"}, + {Key: "managed-by", Value: "controller-test"}, + }, + Labels: map[string]string{ + "app": "test-app", + "environment": "test", + "team": "database", + }, + PreferredMaintenanceWindow: ptr.To("sun:02:00-sun:03:00"), + BackupPolicy: "daily", + SnapshotID: nil, + } + + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + paramGroup := &crossplaneaws.DBInstance{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, paramGroup) + Expect(err).ToNot(HaveOccurred()) + }) + + It("Should create Aurora cluster and instances with correct parameters", func() { + spec := DatabaseSpec{ + ResourceName: "env-app-name-db-1d9fb877", + HostParams: hostparams.HostParams{ + Shape: "db.t3.medium", + MinStorageGB: 20, + DBVersion: "15.7", + }, + DbType: "aurora-postgresql", + SharedDBHost: false, + MasterConnInfo: v1.DatabaseClaimConnectionInfo{ + Username: "testadmin", + Password: "test-password-123", + Port: "5432", + }, + TempSecret: "temp-secret-abc123", + EnableReplicationRole: true, + EnableSuperUser: false, + EnablePerfInsight: true, + EnableCloudwatchLogsExport: []*string{ptr.To("postgresql"), ptr.To("upgrade")}, + BackupRetentionDays: 7, + CACertificateIdentifier: ptr.To("rds-ca-2019"), + Tags: []ProviderTag{ + {Key: "environment", Value: "test"}, + {Key: "managed-by", Value: "controller-test"}, + }, + Labels: map[string]string{ + "app": "test-app", + "environment": "test", + "team": "database", + }, + PreferredMaintenanceWindow: ptr.To("sun:02:00-sun:03:00"), + BackupPolicy: "daily", + SnapshotID: nil, + } + + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + paramGroup := &crossplaneaws.DBInstance{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb877"}, paramGroup) + Expect(err).ToNot(HaveOccurred()) + }) +}) diff --git a/pkg/providers/gcp.go b/pkg/providers/crossplane_gcp.go similarity index 66% rename from pkg/providers/gcp.go rename to pkg/providers/crossplane_gcp.go index ead372e..708071b 100644 --- a/pkg/providers/gcp.go +++ b/pkg/providers/crossplane_gcp.go @@ -9,15 +9,15 @@ import ( type GCPProvider struct { } -func NewGCPProvider(k8sClient client.Client, config *viper.Viper) *GCPProvider { +func newGCPProvider(k8sClient client.Client, config *viper.Viper) Provider { return &GCPProvider{} } -func (p *GCPProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) error { - return nil +func (p *GCPProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) { + return false, nil } -func (p *GCPProvider) DeleteDatabase(ctx context.Context, name string) error { +func (p *GCPProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) error { return nil } diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go index ce26f56..16eb605 100644 --- a/pkg/providers/providers.go +++ b/pkg/providers/providers.go @@ -12,46 +12,50 @@ import ( // DatabaseSpec defines the required parameters to provision a database using any provider. type DatabaseSpec struct { - ProviderResourceName string + ResourceName string + HostParams hostparams.HostParams DbType v1.DatabaseType SharedDBHost bool MasterConnInfo v1.DatabaseClaimConnectionInfo TempSecret string - HostParams hostparams.HostParams EnableReplicationRole bool EnableSuperUser bool EnablePerfInsight bool EnableCloudwatchLogsExport []*string BackupRetentionDays int64 - CACertificateIdentifier string + CACertificateIdentifier *string - Tags v1.Tag + Tags []ProviderTag Labels map[string]string PreferredMaintenanceWindow *string BackupPolicy string + + SnapshotID *string } // Provider is an interface that abstracts provider-specific logic. type Provider interface { - // CreateDatabase provisions a new database instance. - CreateDatabase(ctx context.Context, spec DatabaseSpec) error + // CreateDatabase ensures a database instance is provisioned, updated, and ready. + // If the instance does not exist, it provisions a new one. + // Returns true if the database is fully ready, along with any encountered error. + CreateDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) // DeleteDatabase deprovisions an existing database instance. - DeleteDatabase(ctx context.Context, name string) error + DeleteDatabase(ctx context.Context, spec DatabaseSpec) error // GetDatabase retrieves the current status of a database instance. GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) } // NewProvider returns a concrete provider implementation based on the given provider name. -func NewProvider(config *viper.Viper, k8sClient client.Client) (Provider, error) { +func NewProvider(config *viper.Viper, k8sClient client.Client, serviceNS string) Provider { cloud := config.GetString("cloud") switch cloud { case "aws": - return NewAWSProvider(k8sClient, config), nil + return newAWSProvider(k8sClient, config, serviceNS) case "gcp": - return NewGCPProvider(k8sClient, config), nil + return newGCPProvider(k8sClient, config) case "cloudnative-pg": - return nil, nil + return nil default: - return nil, fmt.Errorf("unsupported provider for cloud: %s", cloud) + panic(fmt.Sprintf("Unsupported provider %s", cloud)) } } diff --git a/pkg/providers/tags.go b/pkg/providers/tags.go new file mode 100644 index 0000000..666a169 --- /dev/null +++ b/pkg/providers/tags.go @@ -0,0 +1,58 @@ +package providers + +const ( + BackupPolicyKey = "Backup" +) + +var ( + OperationalTAGActive = ProviderTag{Key: "operational-status", Value: "active"} + OperationalTAGInactive = ProviderTag{Key: "operational-status", Value: "inactive"} +) + +// ProviderTag represents a key-value tag format. +type ProviderTag struct { + Key string + Value string +} + +// ConvertToProviderTags converts any slice of struct tags to ProviderTag format. +func ConvertToProviderTags[T any](tags []T, extractFunc func(T) (string, string)) []ProviderTag { + providerTags := make([]ProviderTag, 0, len(tags)) + for _, tag := range tags { + key, value := extractFunc(tag) + providerTags = append(providerTags, ProviderTag{Key: key, Value: value}) + } + return providerTags +} + +// ConvertFromProviderTags converts a slice of ProviderTag to another tag type. +func ConvertFromProviderTags[T any](providerTags []ProviderTag, transformFunc func(ProviderTag) T) []T { + convertedTags := make([]T, 0, len(providerTags)) + for _, tag := range providerTags { + convertedTags = append(convertedTags, transformFunc(tag)) + } + return convertedTags +} + +// ConvertMapToProviderTags converts a map[string]string to a slice of ProviderTag. +func ConvertMapToProviderTags(tags map[string]string) []ProviderTag { + providerTags := make([]ProviderTag, 0, len(tags)) + for k, v := range tags { + providerTags = append(providerTags, ProviderTag{Key: k, Value: v}) + } + return providerTags +} + +// MergeTags merges two slices of ProviderTag, ensuring unique keys (newTags override existingTags). +func MergeTags(existingTags, newTags []ProviderTag) []ProviderTag { + tagMap := make(map[string]string) + + for _, tag := range existingTags { + tagMap[tag.Key] = tag.Value + } + for _, tag := range newTags { + tagMap[tag.Key] = tag.Value + } + + return ConvertMapToProviderTags(tagMap) +} diff --git a/pkg/providers/utils.go b/pkg/providers/utils.go new file mode 100644 index 0000000..97ff6b8 --- /dev/null +++ b/pkg/providers/utils.go @@ -0,0 +1,97 @@ +package providers + +import ( + "context" + "fmt" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" + v1 "github.com/infobloxopen/db-controller/api/v1" + basefun "github.com/infobloxopen/db-controller/pkg/basefunctions" + "github.com/infobloxopen/db-controller/pkg/hostparams" + gopassword "github.com/sethvargo/go-password/password" + "github.com/spf13/viper" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "strings" +) + +// ManageMasterPassword ensures the master password secret exists. +// It creates a new one if it doesn't exist. +func ManageMasterPassword(ctx context.Context, secret *xpv1.SecretKeySelector, k8Client client.Client) error { + password, err := generateMasterPassword() + if err != nil { + return fmt.Errorf("failed to generate master password: %w", err) + } + + masterSecret := &corev1.Secret{} + secretKey := client.ObjectKey{ + Name: secret.SecretReference.Name, + Namespace: secret.SecretReference.Namespace, + } + + if err := k8Client.Get(ctx, secretKey, masterSecret); err != nil { + if !errors.IsNotFound(err) { + return fmt.Errorf("failed to get master secret: %w", err) + } + + masterSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: secret.SecretReference.Namespace, + Name: secret.SecretReference.Name, + }, + Data: map[string][]byte{ + secret.Key: []byte(password), + }, + } + return k8Client.Create(ctx, masterSecret) + } + + return nil +} + +func generateMasterPassword() (string, error) { + var pass string + var err error + minPasswordLength := 30 + + pass, err = gopassword.Generate(minPasswordLength, 3, 0, false, true) + if err != nil { + return "", err + } + return pass, nil +} + +func GetEngineVersion(params hostparams.HostParams, config *viper.Viper) *string { + defaultMajorVersion := "" + if params.IsDefaultVersion { + defaultMajorVersion = basefun.GetDefaultMajorVersion(config) + } else { + defaultMajorVersion = params.DBVersion + } + return &defaultMajorVersion +} + +func getParameterGroupName(providerResourceName, dbVersion string, dbType v1.DatabaseType) string { + switch dbType { + case v1.Postgres: + return providerResourceName + "-" + (strings.Split(dbVersion, "."))[0] + case v1.AuroraPostgres: + return providerResourceName + "-a-" + (strings.Split(dbVersion, "."))[0] + default: + return providerResourceName + "-" + (strings.Split(dbVersion, "."))[0] + } +} + +func isReady(cond []xpv1.Condition) (bool, error) { + // Check if the cluster is marked as ready in its status conditions + for _, condition := range cond { + if condition.Type == xpv1.TypeReady && condition.Status == corev1.ConditionTrue { + return true, nil + } + if condition.Reason == xpv1.ReasonReconcileError { + return false, fmt.Errorf("crossplane resource is in a bad state: %s", condition.Message) + } + } + return false, nil +} From b943fd14078ddb97e5daaae3c636b3c5de04bbde Mon Sep 17 00:00:00 2001 From: bfabricio Date: Thu, 27 Feb 2025 11:01:07 -0300 Subject: [PATCH 03/22] refactor: improve error handling --- pkg/providers/crossplane_aws.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 452f285..16c39fa 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -12,7 +12,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" ) const ( @@ -125,7 +124,7 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) err } func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { - return nil, nil + panic("not implemented") } func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) error { @@ -203,7 +202,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e } } - // Handle cluster + // Handle aurora database cluster creation dbCluster := &crossplaneaws.DBCluster{} clusterKey := client.ObjectKey{Name: params.ResourceName} if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err != nil { @@ -223,7 +222,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e } } - // Handle database instance + // Handle primary database instance primaryDbInstance := &crossplaneaws.DBInstance{} primaryInstanceKey := client.ObjectKey{Name: params.ResourceName} if err := p.k8sClient.Get(ctx, primaryInstanceKey, primaryDbInstance); err != nil { @@ -251,6 +250,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e return err } + // Handle secondary database instance if basefun.GetMultiAZEnabled(p.config) { secondaryDbInstance := &crossplaneaws.DBInstance{} secondaryInstanceKey := client.ObjectKey{Name: params.ResourceName + "-2"} @@ -429,8 +429,6 @@ func (p *AWSProvider) postgresDBParameterGroup(params DatabaseSpec) *crossplanea } func (p *AWSProvider) updateDBInstance(ctx context.Context, params DatabaseSpec, dbInstance *crossplaneaws.DBInstance) error { - logr := log.FromContext(ctx) - // Create a patch snapshot from current DBInstance patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) @@ -471,7 +469,6 @@ func (p *AWSProvider) updateDBInstance(ctx context.Context, params DatabaseSpec, dbInstance.Spec.ForProvider.PreferredMaintenanceWindow = params.PreferredMaintenanceWindow } - logr.Info("updating crossplane DBInstance resource", "DBInstance", dbInstance.Name) err = p.k8sClient.Patch(ctx, dbInstance, patchDBInstance) if err != nil { return err @@ -707,8 +704,6 @@ func (p *AWSProvider) auroraInstanceParamGroup(params DatabaseSpec) *crossplanea } func (p *AWSProvider) auroraUpdateDBCluster(ctx context.Context, params DatabaseSpec, dbCluster *crossplaneaws.DBCluster) error { - logr := log.FromContext(ctx) - // Create a patch snapshot from current DBCluster patchDBCluster := client.MergeFrom(dbCluster.DeepCopy()) p.configureDBTags(¶ms) @@ -733,7 +728,6 @@ func (p *AWSProvider) auroraUpdateDBCluster(ctx context.Context, params Database return nil } - logr.Info("updating crossplane DBCluster resource", "DBCluster", dbCluster.Name) err = p.k8sClient.Patch(ctx, dbCluster, patchDBCluster) if err != nil { return err From 4fce3431bfac0fdbb0d801bf654374afae11419c Mon Sep 17 00:00:00 2001 From: bfabricio Date: Thu, 27 Feb 2025 11:02:56 -0300 Subject: [PATCH 04/22] fix: wrong import sorting --- claim.yaml | 0 pkg/databaseclaim/databaseclaim.go | 2 -- 2 files changed, 2 deletions(-) delete mode 100644 claim.yaml diff --git a/claim.yaml b/claim.yaml deleted file mode 100644 index e69de29..0000000 diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index b4e6572..2609407 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -9,13 +9,11 @@ import ( "strings" "time" - crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/go-logr/logr" _ "github.com/lib/pq" gopassword "github.com/sethvargo/go-password/password" "github.com/spf13/viper" - crossplanegcp "github.com/upbound/provider-gcp/apis/alloydb/v1beta2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" From 168ca17d1422ad3e3ed299ccf3a9b98f1dd35834 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Thu, 27 Feb 2025 20:05:09 -0300 Subject: [PATCH 05/22] refactor: improve error handling --- pkg/providers/crossplane_aws.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 16c39fa..7559b9d 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -12,6 +12,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" ) const ( @@ -128,12 +129,14 @@ func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSp } func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) error { + logger := log.FromContext(ctx) // Handle database parameter group paramGroupName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) dbParamGroup := &crossplaneaws.DBParameterGroup{} if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { if errors.IsNotFound(err) { + logger.Info("Creating Postgres Instance parameter group", "paramGroupName", paramGroupName) if err := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { return fmt.Errorf("failed to create DB parameter group: %w", err) } @@ -156,6 +159,7 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e return fmt.Errorf("failed to manage master password: %w", passwordErr) } + logger.Info("Creating Postgres db instance", "dbInstance", params.ResourceName) if err := p.k8sClient.Create(ctx, dbInstance); err != nil { return fmt.Errorf("failed to create DB instance: %w", err) } @@ -170,18 +174,20 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e err := p.updateDBInstance(ctx, params, dbInstance) if err != nil { - return err + return fmt.Errorf("failed to update DB instance: %w", err) } return nil } func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) error { + logger := log.FromContext(ctx) // Handle database instance parameter group paramGroupName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) dbParamGroup := &crossplaneaws.DBParameterGroup{} if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { if errors.IsNotFound(err) { + logger.Info("Creating Aurora DB Instance parameter group", "paramGroupName", paramGroupName) if err := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { return fmt.Errorf("failed to create DB parameter group: %w", err) } @@ -194,6 +200,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbClusterParamGroup); err != nil { if errors.IsNotFound(err) { + logger.Info("Creating Aurora DB Cluster parameter group", "paramGroupName", paramGroupName) if err := p.k8sClient.Create(ctx, p.auroraClusterParamGroup(params)); err != nil { return fmt.Errorf("failed to create DB parameter group: %w", err) } @@ -213,7 +220,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e if passwordErr != nil { return fmt.Errorf("failed to manage master password: %w", passwordErr) } - + logger.Info("Creating Aurora DB Cluster", "name", params.ResourceName) if err := p.k8sClient.Create(ctx, dbCluster); err != nil { return fmt.Errorf("failed to create DB instance: %w", err) } @@ -228,6 +235,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e if err := p.k8sClient.Get(ctx, primaryInstanceKey, primaryDbInstance); err != nil { if errors.IsNotFound(err) { primaryDbInstance = p.auroraDBInstance(params, false) + logger.Info("Creating Aurora DB Instance", "name", params.ResourceName) if err := p.k8sClient.Create(ctx, primaryDbInstance); err != nil { return fmt.Errorf("failed to create DB instance: %w", err) } @@ -242,12 +250,12 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e err := p.auroraUpdateDBCluster(ctx, params, dbCluster) if err != nil { - return err + return fmt.Errorf("failed to update DB cluster: %v", err) } err = p.updateDBInstance(ctx, params, primaryDbInstance) if err != nil { - return err + return fmt.Errorf("failed to update primary DB instance: %v", err) } // Handle secondary database instance @@ -257,6 +265,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e if err := p.k8sClient.Get(ctx, secondaryInstanceKey, secondaryDbInstance); err != nil { if errors.IsNotFound(err) { secondaryDbInstance = p.auroraDBInstance(params, true) + logger.Info("Creating Aurora DB Secondary Instance", "name", params.ResourceName+"-2") if err := p.k8sClient.Create(ctx, secondaryDbInstance); err != nil { return fmt.Errorf("failed to create DB instance: %w", err) } @@ -271,7 +280,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e err := p.updateDBInstance(ctx, params, secondaryDbInstance) if err != nil { - return err + return fmt.Errorf("failed to update secondary DB instance: %v", err) } } From 2ee8efed84a4c3e01a9234409f5ee2c64d2df66e Mon Sep 17 00:00:00 2001 From: bfabricio Date: Fri, 28 Feb 2025 09:16:07 -0300 Subject: [PATCH 06/22] fix: add not foud handler for is resource ready --- pkg/providers/crossplane_aws.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 7559b9d..e4edde8 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -758,6 +758,9 @@ func (p *AWSProvider) isDBInstanceReady(ctx context.Context, instanceName string dbInstance := &crossplaneaws.DBInstance{} instanceKey := client.ObjectKey{Name: instanceName} if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err != nil { + if errors.IsNotFound(err) { + return false, nil + } return false, err } return isReady(dbInstance.Status.Conditions) From 343eb8f4fc452db4b2ad35fc15c983c457974373 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Mon, 3 Mar 2025 09:14:20 -0300 Subject: [PATCH 07/22] test: unit test create rds postgres instance --- pkg/databaseclaim/databaseclaim.go | 49 +- pkg/providers/crossplane_aws.go | 125 ++++- pkg/providers/crossplane_aws_test.go | 715 ++++++++++++++++++++++++--- pkg/providers/crossplane_gcp.go | 4 +- pkg/providers/providers.go | 5 +- pkg/providers/tags.go | 16 + pkg/providers/utils_test.go | 47 ++ 7 files changed, 849 insertions(+), 112 deletions(-) create mode 100644 pkg/providers/utils_test.go diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index 2609407..34734dc 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -193,7 +193,7 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques if basefun.GetCloud(r.Config.Viper) == "aws" { // our finalizer is present, so lets handle any external dependency spec := NewDatabaseSpecFromRequestInfo(&reqInfo, &dbClaim, r.getMode(ctx, &reqInfo, &dbClaim), r.Config.Viper) - if err := r.provider.DeleteDatabase(ctx, spec); err != nil { + if _, err := r.provider.DeleteDatabase(ctx, spec); err != nil { // if fail to delete the external dependency here, return with error // so that it can be retried return ctrl.Result{}, err @@ -251,52 +251,26 @@ func (r *DatabaseClaimReconciler) createMetricsDeployment(ctx context.Context, d } func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, dbClaim *v1.DatabaseClaim) (ctrl.Result, error) { - logr := log.FromContext(ctx).WithValues("databaseclaim", dbClaim.Namespace+"/"+dbClaim.Name) - logr.Info("post migration is in progress") - // get name of DBInstance from connectionInfo dbInstanceName := strings.Split(dbClaim.Status.OldDB.ConnectionInfo.Host, ".")[0] - var dbParamGroupName string - // get name of DBParamGroup from connectionInfo - if dbClaim.Status.OldDB.Type == v1.AuroraPostgres { - dbParamGroupName = dbInstanceName + "-a-" + (strings.Split(dbClaim.Status.OldDB.DBVersion, "."))[0] - } else { - dbParamGroupName = dbInstanceName + "-" + (strings.Split(dbClaim.Status.OldDB.DBVersion, "."))[0] + deleted, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName}) + if err != nil { + return ctrl.Result{}, err } - TagsVerified, err := r.manageOperationalTagging(ctx, logr, dbInstanceName, dbParamGroupName) - - // Even though we get error in updating tags, we log the error - // and go ahead with deleting resources - if err != nil || TagsVerified { - + if time.Since(dbClaim.Status.OldDB.PostMigrationActionStartedAt.Time).Minutes() > 10 { + _, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName, TagInactive: false}) if err != nil { - logr.Error(err, "Failed updating or verifying operational tags") - } - - if err = r.deleteCloudDatabaseAWS(dbInstanceName, ctx); err != nil { - logr.Error(err, "Could not delete crossplane DBInstance/DBCLluster") - } - if err = r.deleteParameterGroupAWS(ctx, dbParamGroupName); err != nil { - logr.Error(err, "Could not delete crossplane DBParamGroup/DBClusterParamGroup") + return ctrl.Result{}, err } - dbClaim.Status.OldDB = v1.StatusForOldDB{} - } else if time.Since(dbClaim.Status.OldDB.PostMigrationActionStartedAt.Time).Minutes() > 10 { - // Lets keep the state of old as it is for defined time to wait and verify tags before actually deleting resources - logr.Info("defined wait time is over to verify operational tags on AWS resources. Moving ahead to delete associated crossplane resources anyway") - - if err = r.deleteCloudDatabaseAWS(dbInstanceName, ctx); err != nil { - logr.Error(err, "Could not delete crossplane DBInstance/DBCLluster") - } - if err = r.deleteParameterGroupAWS(ctx, dbParamGroupName); err != nil { - logr.Error(err, "Could not delete crossplane DBParamGroup/DBClusterParamGroup") - } + } - dbClaim.Status.OldDB = v1.StatusForOldDB{} + if !deleted { + return ctrl.Result{RequeueAfter: time.Minute}, nil } if err := r.statusManager.ClearError(ctx, dbClaim); err != nil { @@ -307,7 +281,8 @@ func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, d if !dbClaim.ObjectMeta.DeletionTimestamp.IsZero() { return ctrl.Result{Requeue: true}, nil } - return ctrl.Result{RequeueAfter: time.Minute}, nil + + return ctrl.Result{}, err } // Create, migrate or upgrade database diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index e4edde8..8cece3d 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -73,7 +73,16 @@ func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bo return false, fmt.Errorf("%w: %q must be one of %s", v1.ErrInvalidDBType, spec.DbType, []v1.DatabaseType{v1.Postgres, v1.AuroraPostgres}) } -func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) error { +// DeleteDatabase attempt to delete all crossplane resources related to the provided spec +// optionally +func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) { + if spec.TagInactive { + // it takes some time to propagate the tag to the underlying aws resource + if tagged, err := p.addInactiveOperationalTag(ctx, spec); err != nil || !tagged { + return tagged, err + } + } + deletionPolicy := client.PropagationPolicy(metav1.DeletePropagationBackground) paramGroupName := getParameterGroupName(spec.ResourceName, spec.HostParams.DBVersion, spec.DbType) // Delete DBClusterParameterGroup if it exists @@ -81,7 +90,7 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) err clusterParamGroupKey := client.ObjectKey{Name: paramGroupName} if err := p.k8sClient.Get(ctx, clusterParamGroupKey, dbClusterParamGroup); err == nil { if err := p.k8sClient.Delete(ctx, dbClusterParamGroup, deletionPolicy); err != nil { - return err + return false, err } } @@ -90,7 +99,7 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) err paramGroupKey := client.ObjectKey{Name: paramGroupName} if err := p.k8sClient.Get(ctx, paramGroupKey, dbParameterGroup); err == nil { if err := p.k8sClient.Delete(ctx, dbParameterGroup, deletionPolicy); err != nil { - return err + return false, err } } @@ -99,7 +108,7 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) err clusterKey := client.ObjectKey{Name: spec.ResourceName} if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err == nil { if err := p.k8sClient.Delete(ctx, dbCluster, deletionPolicy); err != nil { - return err + return false, err } } @@ -108,7 +117,7 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) err dbInstance := &crossplaneaws.DBInstance{} if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err == nil { if err := p.k8sClient.Delete(ctx, dbInstance, deletionPolicy); err != nil { - return err + return false, err } } @@ -117,11 +126,11 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) err instance2Key := client.ObjectKey{Name: spec.ResourceName + "-2"} if err := p.k8sClient.Get(ctx, instance2Key, dbInstance2); err == nil { if err := p.k8sClient.Delete(ctx, dbInstance2, deletionPolicy); err != nil { - return err + return false, err } } - return nil + return true, nil } func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { @@ -248,7 +257,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e return fmt.Errorf("can not create Cloud DB instance %s it is being deleted", params.ResourceName) } - err := p.auroraUpdateDBCluster(ctx, params, dbCluster) + err := p.updateAuroraDBCluster(ctx, params, dbCluster) if err != nil { return fmt.Errorf("failed to update DB cluster: %v", err) } @@ -712,7 +721,7 @@ func (p *AWSProvider) auroraInstanceParamGroup(params DatabaseSpec) *crossplanea } } -func (p *AWSProvider) auroraUpdateDBCluster(ctx context.Context, params DatabaseSpec, dbCluster *crossplaneaws.DBCluster) error { +func (p *AWSProvider) updateAuroraDBCluster(ctx context.Context, params DatabaseSpec, dbCluster *crossplaneaws.DBCluster) error { // Create a patch snapshot from current DBCluster patchDBCluster := client.MergeFrom(dbCluster.DeepCopy()) p.configureDBTags(¶ms) @@ -770,7 +779,105 @@ func (p *AWSProvider) isDBClusterReady(ctx context.Context, clusterName string) dbCluster := &crossplaneaws.DBCluster{} clusterKey := client.ObjectKey{Name: clusterName} if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err != nil { + if errors.IsNotFound(err) { + return false, nil + } return false, err } return isReady(dbCluster.Status.Conditions) } + +func (p *AWSProvider) markClusterAsInactive(ctx context.Context, name string) (bool, error) { + dbCluster := &crossplaneaws.DBCluster{} + err := p.k8sClient.Get(ctx, client.ObjectKey{ + Name: name, + }, dbCluster) + if err != nil { + return false, err + } + + if isInactiveAtProvider(dbCluster.Status.AtProvider.TagList) { + return true, nil + } + + patchDBInstance := client.MergeFrom(dbCluster.DeepCopy()) + dbCluster.Spec.ForProvider.Tags = changeToInactive(dbCluster.Spec.ForProvider.Tags) + + err = p.k8sClient.Patch(ctx, dbCluster, patchDBInstance) + if err != nil { + return false, err + } + return false, nil +} + +func (p *AWSProvider) markInstanceAsInactive(ctx context.Context, name string) (bool, error) { + dbInstance := &crossplaneaws.DBInstance{} + err := p.k8sClient.Get(ctx, client.ObjectKey{ + Name: name, + }, dbInstance) + if err != nil { + return false, err + } + + if isInactiveAtProvider(dbInstance.Status.AtProvider.TagList) { + return true, nil + } + + patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) + dbInstance.Spec.ForProvider.Tags = changeToInactive(dbInstance.Spec.ForProvider.Tags) + + err = p.k8sClient.Patch(ctx, dbInstance, patchDBInstance) + if err != nil { + return false, err + } + return false, nil +} + +func isInactiveAtProvider(tags []*crossplaneaws.Tag) bool { + for _, tag := range tags { + if *tag.Key == OperationalTAGInactive.Key && *tag.Value == OperationalTAGInactive.Value { + return true + } + } + return false +} + +func changeToInactive(tags []*crossplaneaws.Tag) []*crossplaneaws.Tag { + found := false + for _, tag := range tags { + if *tag.Key == OperationalTAGInactive.Key && *tag.Value == OperationalTAGInactive.Value { + found = true + break + } + if *tag.Key == OperationalTAGInactive.Key && *tag.Value == OperationalTAGActive.Value { + found = true + *tag.Value = OperationalTAGInactive.Value + } + } + if !found { + tags = append(tags, &crossplaneaws.Tag{Key: &OperationalTAGInactive.Key, Value: &OperationalTAGInactive.Value}) + } + return tags +} + +// addInactiveOperationalTag marks the instance with operational-status: inactive tag, +// it returns ErrTagNotPropagated if the tag is not yet propagated to the cloud resource +func (p *AWSProvider) addInactiveOperationalTag(ctx context.Context, spec DatabaseSpec) (bool, error) { + for _, tag := range spec.Tags { + if tag.Key == OperationalTAGInactive.Key || tag.Value == OperationalTAGInactive.Value { + return true, nil + } + } + + if tagged, err := p.markInstanceAsInactive(ctx, spec.ResourceName); err != nil || !tagged { + return false, err + } + + if spec.DbType == v1.AuroraPostgres { + if tagged, err := p.markClusterAsInactive(ctx, spec.ResourceName); err != nil || !tagged { + return false, err + } + } + + return true, nil +} diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index 4049759..64e56af 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -3,15 +3,21 @@ package providers import ( "context" crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" v1 "github.com/infobloxopen/db-controller/api/v1" + basefun "github.com/infobloxopen/db-controller/pkg/basefunctions" "github.com/infobloxopen/db-controller/pkg/config" "github.com/infobloxopen/db-controller/pkg/hostparams" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/spf13/viper" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" "path/filepath" + "reflect" "sigs.k8s.io/controller-runtime/pkg/client/fake" "testing" @@ -46,34 +52,29 @@ var _ = BeforeSuite(func() { var _ = AfterSuite(func() {}) -var _ = Describe("AWSProvider createPostgres", func() { +var _ = Describe("AWSProvider create Postgres database", func() { var ( provider *AWSProvider ctx context.Context + spec DatabaseSpec ) BeforeEach(func() { ctx = context.TODO() - provider = &AWSProvider{ - k8sClient: k8sClient, - config: controllerConfig, - serviceNS: "db-controller", - } - }) - - AfterEach(func() { - Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBInstance{})).To(Succeed()) - Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBParameterGroup{})).To(Succeed()) - Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBCluster{})).To(Succeed()) - }) - - It("Should create postgres DBInstance with correct parameters", func() { - spec := DatabaseSpec{ + spec = DatabaseSpec{ ResourceName: "env-app-name-db-1d9fb876", HostParams: hostparams.HostParams{ - Shape: "db.t3.medium", - MinStorageGB: 20, - DBVersion: "15.7", + Shape: "db.t3.medium", + MinStorageGB: 10, + MaxStorageGB: 20, + DBVersion: "15.7", + SkipFinalSnapshotBeforeDeletion: true, + MasterUsername: "root", + EnableIAMDatabaseAuthentication: true, + StorageType: "storage-type", + DeletionPolicy: xpv1.DeletionOrphan, + PubliclyAccessible: true, + InstanceClass: "db.t3.medium", }, DbType: "postgres", SharedDBHost: false, @@ -102,56 +103,646 @@ var _ = Describe("AWSProvider createPostgres", func() { BackupPolicy: "daily", SnapshotID: nil, } + provider = &AWSProvider{ + k8sClient: k8sClient, + config: controllerConfig, + serviceNS: "db-controller", + } + }) + + AfterEach(func() { + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBInstance{})).To(Succeed()) + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBParameterGroup{})).To(Succeed()) + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBCluster{})).To(Succeed()) + }) + + Describe("create postgres database", func() { + When("create function is called with correct parameters", func() { + It("should properly create crossplane resources", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + // Validate DBParameterGroup + paramGroup := &crossplaneaws.DBParameterGroup{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876-15"}, paramGroup) + }).Should(Succeed()) + + Expect(paramGroup.Spec.ForProvider.Parameters).To(ContainElements( + crossplaneaws.CustomParameter{ + ParameterName: ptr.To("idle_in_transaction_session_timeout"), + ParameterValue: ptr.To("300000"), + ApplyMethod: ptr.To("immediate"), + }, + crossplaneaws.CustomParameter{ + ParameterName: ptr.To("shared_preload_libraries"), + ParameterValue: ptr.To("pg_stat_statements,pg_cron"), + ApplyMethod: ptr.To("pending-reboot"), + }, + crossplaneaws.CustomParameter{ + ParameterName: ptr.To("cron.database_name"), + ParameterValue: ptr.To(spec.MasterConnInfo.DatabaseName), + ApplyMethod: ptr.To("pending-reboot"), + }, + )) + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + }).Should(Succeed()) + + Expect(dbInstance.Spec.ForProvider.Engine).To(Equal(ptr.To(spec.HostParams.Type))) + Expect(dbInstance.Spec.ForProvider.EngineVersion).To(Equal(GetEngineVersion(spec.HostParams, provider.config))) + Expect(dbInstance.Spec.ForProvider.DBInstanceClass).To(Equal(ptr.To(spec.HostParams.InstanceClass))) + Expect(dbInstance.Spec.ForProvider.AllocatedStorage).To(Equal(ptr.To(int64(spec.HostParams.MinStorageGB)))) + Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-15")) + Expect(dbInstance.Spec.ForProvider.CACertificateIdentifier).To(Equal(spec.CACertificateIdentifier)) + Expect(dbInstance.Spec.ForProvider.MultiAZ).To(Equal(ptr.To(basefun.GetMultiAZEnabled(provider.config)))) + Expect(dbInstance.Spec.ForProvider.MasterUsername).To(Equal(ptr.To(spec.HostParams.MasterUsername))) + Expect(dbInstance.Spec.ForProvider.PubliclyAccessible).To(Equal(ptr.To(spec.HostParams.PubliclyAccessible))) + Expect(dbInstance.Spec.ForProvider.EnableIAMDatabaseAuthentication).To(Equal(ptr.To(spec.HostParams.EnableIAMDatabaseAuthentication))) + Expect(dbInstance.Spec.ForProvider.EnablePerformanceInsights).To(Equal(ptr.To(spec.EnablePerfInsight))) + Expect(dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports).To(Equal(spec.EnableCloudwatchLogsExport)) + Expect(dbInstance.Spec.ForProvider.BackupRetentionPeriod).To(Equal(ptr.To(spec.BackupRetentionDays))) + Expect(dbInstance.Spec.ForProvider.StorageEncrypted).To(Equal(ptr.To(true))) + Expect(dbInstance.Spec.ForProvider.StorageType).To(Equal(ptr.To(spec.HostParams.StorageType))) + Expect(dbInstance.Spec.ForProvider.Port).To(Equal(ptr.To(spec.HostParams.Port))) + Expect(dbInstance.Spec.ForProvider.PreferredMaintenanceWindow).To(Equal(spec.PreferredMaintenanceWindow)) + Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-15")) + + // master password + Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Name).To(Equal(spec.ResourceName + MasterPasswordSuffix)) + Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Namespace).To(Equal(provider.serviceNS)) + Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Name).To(Equal(spec.ResourceName)) + Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Namespace).To(Equal(provider.serviceNS)) + + // Validate VPC & Security Groups + Expect(dbInstance.Spec.ForProvider.VPCSecurityGroupIDRefs).To(ContainElement(xpv1.Reference{ + Name: basefun.GetVpcSecurityGroupIDRefs(provider.config), + })) + Expect(dbInstance.Spec.ForProvider.DBSubnetGroupNameRef.Name).To(Equal(basefun.GetDbSubnetGroupNameRef(provider.config))) + + // Validate Provider Config & Deletion Policy + Expect(dbInstance.Spec.ResourceSpec.ProviderConfigReference.Name).To(Equal(basefun.GetProviderConfig(provider.config))) + Expect(dbInstance.Spec.ResourceSpec.DeletionPolicy).To(Equal(spec.HostParams.DeletionPolicy)) + + }) + + It("should creates master password secret for the new database instance", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + // Define the expected secret key + secretKey := types.NamespacedName{ + Name: spec.ResourceName + MasterPasswordSuffix, + Namespace: provider.serviceNS, + } - _, err := provider.CreateDatabase(ctx, spec) - Expect(err).ToNot(HaveOccurred()) + // Validate that the secret is created + masterSecret := &corev1.Secret{} + Eventually(func() error { + return k8sClient.Get(ctx, secretKey, masterSecret) + }).Should(Succeed()) - paramGroup := &crossplaneaws.DBInstance{} - err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, paramGroup) - Expect(err).ToNot(HaveOccurred()) + // Validate the secret contains the expected key + Expect(masterSecret.Data).To(HaveKey(MasterPasswordSecretKey)) + Expect(masterSecret.Data[MasterPasswordSecretKey]).ToNot(BeEmpty()) + }) + + It("should return true when database is already provisioned", func() { + isReady, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + }).Should(Succeed()) + + updatedInstance := dbInstance.DeepCopy() + updatedInstance.Status.Conditions = []xpv1.Condition{ + { + Type: xpv1.TypeReady, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + }, + } + + // Manually trigger an Update (not Status().Update()) + Expect(k8sClient.Update(ctx, updatedInstance)).To(Succeed()) + + // Call CreateDatabase again and check if it returns true + isReady, err = provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + Expect(isReady).To(BeTrue()) // Now, the database should be marked as ready + }) + + It("should it propagates the spec provider tags to database instance, add operational active tag, and add backup tag", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + }).Should(Succeed()) + + expectedTags := spec.Tags + expectedTags = append(expectedTags, OperationalTAGActive) + expectedTags = append(expectedTags, ProviderTag{Key: BackupPolicyKey, Value: spec.BackupPolicy}) + // Validate Tags + Expect(dbInstance.Spec.ForProvider.Tags).To(ConsistOf(ConvertFromProviderTags(expectedTags, func(tag ProviderTag) *crossplaneaws.Tag { + return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} + }))) + }) + + It("should configures RestoreDBInstanceBackupConfiguration when snapshot id is passed", func() { + newSpec := spec + newSpec.SnapshotID = ptr.To("snapshot-id") + + _, err := provider.CreateDatabase(ctx, newSpec) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + }).Should(Succeed()) + + Expect(dbInstance.Spec.ForProvider.RestoreFrom).ToNot(BeNil()) + Expect(dbInstance.Spec.ForProvider.RestoreFrom.Snapshot.SnapshotIdentifier).To(Equal(newSpec.SnapshotID)) + Expect(dbInstance.Spec.ForProvider.RestoreFrom.Source).To(Equal(ptr.To(SnapshotSource))) + }) + + It("should propagate passed parameter provider labels to database instance", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + }).Should(Succeed()) + + Expect(dbInstance.Labels).To(Equal(spec.Labels)) + }) + }) }) - It("Should create Aurora cluster and instances with correct parameters", func() { - spec := DatabaseSpec{ - ResourceName: "env-app-name-db-1d9fb877", - HostParams: hostparams.HostParams{ - Shape: "db.t3.medium", - MinStorageGB: 20, - DBVersion: "15.7", + Describe("delete database interface usage", func() { + When("delete func is called with correct spec and no operational tagging", func() { + It("should delete postgres the database right away without adding operational tagging", func() { + By("creating a new database") + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + deleted, err := provider.DeleteDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + Expect(deleted).To(BeTrue()) + + paramGroup := &crossplaneaws.DBParameterGroup{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876-15"}, paramGroup) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + + dbInstance := &crossplaneaws.DBInstance{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + }) + }) + When("delete func is called with correct spec and instructed to add inactive operational tagging", func() { + It("should delete postgres database with operational tagging only when tag is propagated to aws", func() { + By("creating a new database") + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + deleteSpec := spec + deleteSpec.TagInactive = true + deleted, err := provider.DeleteDatabase(ctx, deleteSpec) + Expect(err).ToNot(HaveOccurred()) + Expect(deleted).To(BeFalse()) + + paramGroup := &crossplaneaws.DBParameterGroup{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876-15"}, paramGroup) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + Expect(err).ToNot(HaveOccurred()) + + Expect(dbInstance.Spec.ForProvider.Tags).To(ContainElement(&crossplaneaws.Tag{ + Key: ptr.To("operational-status"), + Value: ptr.To("inactive"), + })) + + By("simulating the operational tagging propagated to aws") + patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) + dbInstance.Status.AtProvider.TagList = append( + dbInstance.Status.AtProvider.TagList, + &crossplaneaws.Tag{Key: &OperationalTAGInactive.Key, Value: &OperationalTAGInactive.Value}) + err = k8sClient.Patch(ctx, dbInstance, patchDBInstance) + Expect(err).ToNot(HaveOccurred()) + + deleted, err = provider.DeleteDatabase(ctx, deleteSpec) + Expect(err).ToNot(HaveOccurred()) + Expect(deleted).To(BeTrue()) + }) + }) + }) +}) + +func createFakeClientWithObjects(objects ...k8sRuntime.Object) client.Client { + sch := k8sRuntime.NewScheme() + _ = scheme.AddToScheme(sch) + _ = crossplaneaws.AddToScheme(sch) + + return fake.NewClientBuilder(). + WithScheme(sch). + WithRuntimeObjects(objects...). + Build() +} + +func TestIsDBInstanceReady(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + instanceName string + instanceConditions []xpv1.Condition + expectedReady bool + expectedError bool + }{ + { + name: "Instance not found", + instanceName: "non-existent-instance", + expectedReady: false, + expectedError: false, + }, + { + name: "Instance not ready (Creating)", + instanceName: "test-instance", + instanceConditions: []xpv1.Condition{ + xpv1.Creating(), }, - DbType: "aurora-postgresql", - SharedDBHost: false, - MasterConnInfo: v1.DatabaseClaimConnectionInfo{ - Username: "testadmin", - Password: "test-password-123", - Port: "5432", + expectedReady: false, + expectedError: false, + }, + { + name: "Instance not ready (Failed)", + instanceName: "test-instance", + instanceConditions: []xpv1.Condition{ + { + Type: xpv1.TypeReady, + Status: corev1.ConditionFalse, + Reason: xpv1.ReasonReconcileError, + }, }, - TempSecret: "temp-secret-abc123", - EnableReplicationRole: true, - EnableSuperUser: false, - EnablePerfInsight: true, - EnableCloudwatchLogsExport: []*string{ptr.To("postgresql"), ptr.To("upgrade")}, - BackupRetentionDays: 7, - CACertificateIdentifier: ptr.To("rds-ca-2019"), - Tags: []ProviderTag{ - {Key: "environment", Value: "test"}, - {Key: "managed-by", Value: "controller-test"}, + expectedReady: false, + expectedError: true, + }, + { + name: "Instance ready", + instanceName: "test-instance", + instanceConditions: []xpv1.Condition{ + xpv1.Available(), }, - Labels: map[string]string{ - "app": "test-app", - "environment": "test", - "team": "database", + expectedReady: true, + expectedError: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var fakeClient client.Client + + if test.instanceConditions != nil { + instance := &crossplaneaws.DBInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: test.instanceName, + }, + } + instance.SetConditions(test.instanceConditions...) + fakeClient = createFakeClientWithObjects(instance) + } else { + // Create an empty fake client if instance is not found + fakeClient = createFakeClientWithObjects() + } + + provider := &AWSProvider{ + k8sClient: fakeClient, + } + + ready, err := provider.isDBInstanceReady(ctx, test.instanceName) + + if test.expectedError { + if err == nil { + t.Errorf("[%s] expected error but got nil", test.name) + } + } else { + if err != nil { + t.Errorf("[%s] unexpected error: %v", test.name, err) + } + if ready != test.expectedReady { + t.Errorf("[%s] expected ready: %v, got: %v", test.name, test.expectedReady, ready) + } + } + }) + } +} + +func TestIsDBClusterReady(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + clusterName string + clusterConditions []xpv1.Condition + expectedReady bool + expectedError bool + }{ + { + name: "Cluster not found", + clusterName: "non-existent-cluster", + expectedReady: false, + expectedError: false, + }, + { + name: "Cluster not ready (Creating)", + clusterName: "test-cluster", + clusterConditions: []xpv1.Condition{ + xpv1.Creating(), }, - PreferredMaintenanceWindow: ptr.To("sun:02:00-sun:03:00"), - BackupPolicy: "daily", - SnapshotID: nil, - } + expectedReady: false, + expectedError: false, + }, + { + name: "Cluster not ready (Failed)", + clusterName: "test-cluster", + clusterConditions: []xpv1.Condition{ + { + Type: xpv1.TypeReady, + Status: corev1.ConditionFalse, + Reason: xpv1.ReasonReconcileError, + }, + }, + expectedReady: false, + expectedError: true, + }, + { + name: "Cluster ready", + clusterName: "test-cluster", + clusterConditions: []xpv1.Condition{ + xpv1.Available(), + }, + expectedReady: true, + expectedError: false, + }, + } - _, err := provider.CreateDatabase(ctx, spec) - Expect(err).ToNot(HaveOccurred()) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var fakeClient client.Client - paramGroup := &crossplaneaws.DBInstance{} - err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb877"}, paramGroup) - Expect(err).ToNot(HaveOccurred()) - }) -}) + if test.clusterConditions != nil { + cluster := &crossplaneaws.DBCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: test.clusterName, + }, + } + cluster.SetConditions(test.clusterConditions...) + fakeClient = createFakeClientWithObjects(cluster) + } else { + // Create an empty fake client if cluster is not found + fakeClient = createFakeClientWithObjects() + } + + provider := &AWSProvider{ + k8sClient: fakeClient, + } + + ready, err := provider.isDBClusterReady(ctx, test.clusterName) + + if test.expectedError { + if err == nil { + t.Errorf("[%s] expected error but got nil", test.name) + } + } else { + if err != nil { + t.Errorf("[%s] unexpected error: %v", test.name, err) + } + if ready != test.expectedReady { + t.Errorf("[%s] expected ready: %v, got: %v", test.name, test.expectedReady, ready) + } + } + }) + } +} + +func TestConfigureDBTags(t *testing.T) { + tests := []struct { + name string + inputSpec *DatabaseSpec + expectedTags []ProviderTag + backupPolicy string + }{ + { + name: "No backup policy provided, use default", + inputSpec: &DatabaseSpec{ + Tags: []ProviderTag{ + {Key: "env", Value: "production"}, + }, + }, + expectedTags: []ProviderTag{ + {Key: "env", Value: "production"}, + OperationalTAGActive, + {Key: BackupPolicyKey, Value: "default-policy"}, + }, + backupPolicy: "default-policy", + }, + { + name: "Backup policy provided", + inputSpec: &DatabaseSpec{ + BackupPolicy: "custom-policy", + Tags: []ProviderTag{ + {Key: "team", Value: "devops"}, + }, + }, + expectedTags: []ProviderTag{ + {Key: "team", Value: "devops"}, + OperationalTAGActive, + {Key: BackupPolicyKey, Value: "custom-policy"}, + }, + backupPolicy: "custom-policy", + }, + { + name: "Empty tags, only required tags applied", + inputSpec: &DatabaseSpec{ + BackupPolicy: "retention-30-days", + }, + expectedTags: []ProviderTag{ + OperationalTAGActive, + {Key: BackupPolicyKey, Value: "retention-30-days"}, + }, + backupPolicy: "retention-30-days", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mockConfig := viper.New() + mockConfig.Set("defaultBackupPolicyValue", test.backupPolicy) + provider := &AWSProvider{config: mockConfig} + + provider.configureDBTags(test.inputSpec) + + if !CompareTags(test.inputSpec.Tags, test.expectedTags) { + t.Errorf("[%s] expected tags: %+v, got: %+v", + test.name, test.expectedTags, test.inputSpec.Tags) + } + }) + } +} + +func TestIsInactiveAtProvider(t *testing.T) { + tests := []struct { + name string + tags []*crossplaneaws.Tag + expected bool + }{ + { + name: "Tag present", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + }, + expected: true, + }, + { + name: "Tag key matches but value differs", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("active")}, + }, + expected: false, + }, + { + name: "Tag value matches but key differs", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("status"), Value: ptr.To("inactive")}, + }, + expected: false, + }, + { + name: "Empty tag list", + tags: []*crossplaneaws.Tag{}, + expected: false, + }, + { + name: "Nil tag list", + tags: nil, + expected: false, + }, + { + name: "Multiple tags, with inactive tag present", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("random-key"), Value: ptr.To("random-value")}, + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + {Key: ptr.To("another-key"), Value: ptr.To("another-value")}, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isInactiveAtProvider(tt.tags) + if result != tt.expected { + t.Errorf("[%s] expected isInactiveAtProvider %v, got %v", tt.name, tt.expected, result) + } + }) + } +} + +func TestChangeToInactive(t *testing.T) { + tests := []struct { + name string + tags []*crossplaneaws.Tag + expected []*crossplaneaws.Tag + }{ + { + name: "Tag already inactive", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + }, + expected: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + }, + }, + { + name: "Change active to inactive", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("active")}, + }, + expected: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + }, + }, + { + name: "Add inactive tag when no operational-status key exists", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("some-key"), Value: ptr.To("some-value")}, + }, + expected: []*crossplaneaws.Tag{ + {Key: ptr.To("some-key"), Value: ptr.To("some-value")}, + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + }, + }, + { + name: "Empty tag list, add inactive tag", + tags: []*crossplaneaws.Tag{}, + expected: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + }, + }, + { + name: "Nil tag list, add inactive tag", + tags: nil, + expected: []*crossplaneaws.Tag{ + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, + }, + }, + { + name: "Multiple tags, active tag present and changed", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("random-key"), Value: ptr.To("random-value")}, + {Key: ptr.To("operational-status"), Value: ptr.To("active")}, + {Key: ptr.To("another-key"), Value: ptr.To("another-value")}, + }, + expected: []*crossplaneaws.Tag{ + {Key: ptr.To("random-key"), Value: ptr.To("random-value")}, + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, // Active changed to inactive + {Key: ptr.To("another-key"), Value: ptr.To("another-value")}, + }, + }, + { + name: "Multiple tags, no operational-status tag, should add inactive", + tags: []*crossplaneaws.Tag{ + {Key: ptr.To("random-key"), Value: ptr.To("random-value")}, + {Key: ptr.To("another-key"), Value: ptr.To("another-value")}, + }, + expected: []*crossplaneaws.Tag{ + {Key: ptr.To("random-key"), Value: ptr.To("random-value")}, + {Key: ptr.To("another-key"), Value: ptr.To("another-value")}, + {Key: ptr.To("operational-status"), Value: ptr.To("inactive")}, // Inactive tag added + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := changeToInactive(tt.tags) + + for i, expectedTag := range tt.expected { + if !reflect.DeepEqual(result[i], expectedTag) { + t.Errorf("Unexpected tag at index %d: expected %v, actual %v", i, expectedTag, result[i]) + } + } + }) + } +} diff --git a/pkg/providers/crossplane_gcp.go b/pkg/providers/crossplane_gcp.go index 708071b..418198e 100644 --- a/pkg/providers/crossplane_gcp.go +++ b/pkg/providers/crossplane_gcp.go @@ -17,8 +17,8 @@ func (p *GCPProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bo return false, nil } -func (p *GCPProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) error { - return nil +func (p *GCPProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) { + return false, nil } func (p *GCPProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) { diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go index 16eb605..649e150 100644 --- a/pkg/providers/providers.go +++ b/pkg/providers/providers.go @@ -30,7 +30,8 @@ type DatabaseSpec struct { PreferredMaintenanceWindow *string BackupPolicy string - SnapshotID *string + SnapshotID *string + TagInactive bool } // Provider is an interface that abstracts provider-specific logic. @@ -40,7 +41,7 @@ type Provider interface { // Returns true if the database is fully ready, along with any encountered error. CreateDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) // DeleteDatabase deprovisions an existing database instance. - DeleteDatabase(ctx context.Context, spec DatabaseSpec) error + DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) // GetDatabase retrieves the current status of a database instance. GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) } diff --git a/pkg/providers/tags.go b/pkg/providers/tags.go index 666a169..a5fc922 100644 --- a/pkg/providers/tags.go +++ b/pkg/providers/tags.go @@ -56,3 +56,19 @@ func MergeTags(existingTags, newTags []ProviderTag) []ProviderTag { return ConvertMapToProviderTags(tagMap) } + +func CompareTags(actual, expected []ProviderTag) bool { + if len(actual) != len(expected) { + return false + } + tagMap := make(map[string]string) + for _, tag := range actual { + tagMap[tag.Key] = tag.Value + } + for _, tag := range expected { + if val, exists := tagMap[tag.Key]; !exists || val != tag.Value { + return false + } + } + return true +} diff --git a/pkg/providers/utils_test.go b/pkg/providers/utils_test.go new file mode 100644 index 0000000..fbbc7a1 --- /dev/null +++ b/pkg/providers/utils_test.go @@ -0,0 +1,47 @@ +package providers + +import ( + v1 "github.com/infobloxopen/db-controller/api/v1" + "testing" +) + +func TestGetParameterGroupName(t *testing.T) { + tests := []struct { + name string + providerResourceName string + dbVersion string + dbType v1.DatabaseType + expectedParameterName string + }{ + { + name: "Postgres DB", + providerResourceName: "env-app-name-db-1d9fb87", + dbVersion: "14.5", + dbType: v1.Postgres, + expectedParameterName: "env-app-name-db-1d9fb87-14", + }, + { + name: "Aurora Postgres DB", + providerResourceName: "env-app-name-db-1d9fb87", + dbVersion: "13.3", + dbType: v1.AuroraPostgres, + expectedParameterName: "env-app-name-db-1d9fb87-a-13", + }, + { + name: "Malformed version, should still extract major version", + providerResourceName: "env-app-name-db-1d9fb87", + dbVersion: "9", + dbType: v1.Postgres, + expectedParameterName: "env-app-name-db-1d9fb87-9", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := getParameterGroupName(test.providerResourceName, test.dbVersion, test.dbType) + if result != test.expectedParameterName { + t.Errorf("[%s] expected: %s, got: %s", test.name, test.expectedParameterName, result) + } + }) + } +} From 10362924e5a8559f2bc46f9b332749013a0f8665 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Mon, 3 Mar 2025 09:23:37 -0300 Subject: [PATCH 08/22] fix: add cloud provider to unit test config --- internal/controller/databaseclaim_controller_tagging_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/controller/databaseclaim_controller_tagging_test.go b/internal/controller/databaseclaim_controller_tagging_test.go index 2021b15..f765441 100644 --- a/internal/controller/databaseclaim_controller_tagging_test.go +++ b/internal/controller/databaseclaim_controller_tagging_test.go @@ -164,6 +164,7 @@ var _ = Describe("Tagging", Ordered, func() { }, } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", true) + mockReconciler.Config.Viper.Set("cloud", "aws") mockReconciler.Setup() // providing names of non-existing resources below @@ -221,6 +222,7 @@ var _ = Describe("Tagging", Ordered, func() { }, } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", false) + mockReconciler.Config.Viper.Set("cloud", "aws") mockReconciler.Setup() check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), logger, name, name) @@ -278,6 +280,7 @@ var _ = Describe("Tagging", Ordered, func() { }, } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", true) + mockReconciler.Config.Viper.Set("cloud", "aws") mockReconciler.Setup() check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), logger, name, name) @@ -318,6 +321,7 @@ var _ = Describe("Tagging", Ordered, func() { }, } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", true) + mockReconciler.Config.Viper.Set("cloud", "aws") mockReconciler.Setup() By("adding tags beforehand to .status.AtProvier.TagList. As in reality, if tags gets successfully added. It will reflect at the said path") From 1e66982724355817012db21557e59afb9b11ce4e Mon Sep 17 00:00:00 2001 From: bfabricio Date: Fri, 7 Mar 2025 12:00:22 -0300 Subject: [PATCH 09/22] fix: incorrect master secret key --- pkg/providers/crossplane_aws.go | 51 +++++++++++++--------------- pkg/providers/crossplane_aws_test.go | 6 ++++ 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 8cece3d..d42cfe5 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -70,7 +70,7 @@ func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bo } return instanceReady, nil } - return false, fmt.Errorf("%w: %q must be one of %s", v1.ErrInvalidDBType, spec.DbType, []v1.DatabaseType{v1.Postgres, v1.AuroraPostgres}) + return false, fmt.Errorf("%w: %s must be one of %s", v1.ErrInvalidDBType, spec.DbType, []v1.DatabaseType{v1.Postgres, v1.AuroraPostgres}) } // DeleteDatabase attempt to delete all crossplane resources related to the provided spec @@ -139,51 +139,48 @@ func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSp func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) error { logger := log.FromContext(ctx) + // Handle database parameter group paramGroupName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) dbParamGroup := &crossplaneaws.DBParameterGroup{} - if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { - if errors.IsNotFound(err) { - logger.Info("Creating Postgres Instance parameter group", "paramGroupName", paramGroupName) - if err := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { - return fmt.Errorf("failed to create DB parameter group: %w", err) - } - } else { - return fmt.Errorf("failed to get DB parameter group: %w", err) + err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup) + if errors.IsNotFound(err) { + logger.Info("Creating Postgres Instance parameter group", "paramGroupName", paramGroupName) + if createErr := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); createErr != nil { + return fmt.Errorf("failed to create DB parameter group: %w", createErr) } + } else if err != nil { + return fmt.Errorf("failed to get DB parameter group: %w", err) } // Handle database instance dbInstance := &crossplaneaws.DBInstance{} instanceKey := client.ObjectKey{Name: params.ResourceName} - if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err != nil { - if errors.IsNotFound(err) { - dbInstance = p.postgresDBInstance(params) + err = p.k8sClient.Get(ctx, instanceKey, dbInstance) + if errors.IsNotFound(err) { + dbInstance = p.postgresDBInstance(params) - // Create master password secret before creating the DB instance - passwordErr := ManageMasterPassword(ctx, dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef, p.k8sClient) - if passwordErr != nil { - return fmt.Errorf("failed to manage master password: %w", passwordErr) - } + // Create master password secret before creating the DB instance + if passwordErr := ManageMasterPassword(ctx, dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef, p.k8sClient); passwordErr != nil { + return fmt.Errorf("failed to manage master password for DB instance %s: %w", params.ResourceName, passwordErr) + } - logger.Info("Creating Postgres db instance", "dbInstance", params.ResourceName) - if err := p.k8sClient.Create(ctx, dbInstance); err != nil { - return fmt.Errorf("failed to create DB instance: %w", err) - } - } else { - return fmt.Errorf("failed to get DB instance: %w", err) + logger.Info("Creating Postgres db instance", "dbInstance", params.ResourceName) + if createErr := p.k8sClient.Create(ctx, dbInstance); createErr != nil { + return fmt.Errorf("failed to create DB instance %s: %w", params.ResourceName, createErr) } + } else if err != nil { + return fmt.Errorf("failed to get DB instance %s: %w", params.ResourceName, err) } if !dbInstance.ObjectMeta.DeletionTimestamp.IsZero() { return fmt.Errorf("can not create Cloud DB instance %s it is being deleted", params.ResourceName) } - err := p.updateDBInstance(ctx, params, dbInstance) - if err != nil { - return fmt.Errorf("failed to update DB instance: %w", err) + if updateErr := p.updateDBInstance(ctx, params, dbInstance); updateErr != nil { + return fmt.Errorf("failed to update DB instance: %w", updateErr) } return nil @@ -574,7 +571,7 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus Name: params.ResourceName + MasterPasswordSuffix, Namespace: p.serviceNS, }, - Key: MasterPasswordSuffix, + Key: MasterPasswordSecretKey, }, DBClusterParameterGroupNameRef: &xpv1.Reference{ Name: getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType), diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index 64e56af..4305058 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -286,6 +286,11 @@ var _ = Describe("AWSProvider create Postgres database", func() { }) }) + Describe("update postgres database", func() { + When("when crossplane cr preexists the creation call we need to update with provided parameters", func() { + }) + }) + Describe("delete database interface usage", func() { When("delete func is called with correct spec and no operational tagging", func() { It("should delete postgres the database right away without adding operational tagging", func() { @@ -308,6 +313,7 @@ var _ = Describe("AWSProvider create Postgres database", func() { Expect(errors.IsNotFound(err)).To(BeTrue()) }) }) + When("delete func is called with correct spec and instructed to add inactive operational tagging", func() { It("should delete postgres database with operational tagging only when tag is propagated to aws", func() { By("creating a new database") From 14cf7cf440a6c5aacafbf1cf2357d7bdee3ddfbe Mon Sep 17 00:00:00 2001 From: bfabricio Date: Mon, 10 Mar 2025 12:26:47 -0300 Subject: [PATCH 10/22] refactor: increase reconsiler separation of concern --- pkg/databaseclaim/requestinfo.go | 24 +- pkg/providers/crossplane_aws.go | 127 ++++---- pkg/providers/crossplane_aws_test.go | 465 ++++++++++++++++++++++++--- pkg/providers/providers.go | 35 +- pkg/providers/utils.go | 22 +- pkg/providers/utils_test.go | 11 +- 6 files changed, 543 insertions(+), 141 deletions(-) diff --git a/pkg/databaseclaim/requestinfo.go b/pkg/databaseclaim/requestinfo.go index b8672c7..8542ddb 100644 --- a/pkg/databaseclaim/requestinfo.go +++ b/pkg/databaseclaim/requestinfo.go @@ -110,14 +110,22 @@ func NewDatabaseSpecFromRequestInfo(ri *requestInfo, claim *v1.DatabaseClaim, mo } return providers.DatabaseSpec{ - ResourceName: prefix + claim.Name + suffix, - HostParams: ri.HostParams, - DbType: ri.DbType, - SharedDBHost: ri.SharedDBHost, - MasterConnInfo: ri.MasterConnInfo, - TempSecret: ri.TempSecret, - EnableReplicationRole: ri.EnableReplicationRole, - EnableSuperUser: ri.EnableSuperUser, + ResourceName: prefix + claim.Name + suffix, + DatabaseName: ri.MasterConnInfo.DatabaseName, + DbType: ri.HostParams.Type, + Port: ri.HostParams.Port, + MinStorageGB: ri.HostParams.MinStorageGB, + MaxStorageGB: ri.HostParams.MaxStorageGB, + DBVersion: ri.HostParams.DBVersion, + MasterUsername: ri.HostParams.MasterUsername, + InstanceClass: ri.HostParams.InstanceClass, + StorageType: ri.HostParams.StorageType, + SkipFinalSnapshotBeforeDeletion: ri.HostParams.SkipFinalSnapshotBeforeDeletion, + PubliclyAccessible: ri.HostParams.PubliclyAccessible, + EnableIAMDatabaseAuthentication: ri.HostParams.EnableIAMDatabaseAuthentication, + DeletionPolicy: ri.HostParams.DeletionPolicy, + IsDefaultVersion: ri.HostParams.IsDefaultVersion, + EnablePerfInsight: ri.EnablePerfInsight, EnableCloudwatchLogsExport: ri.EnableCloudwatchLogsExport, BackupRetentionDays: ri.BackupRetentionDays, diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index d42cfe5..ee48f69 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -19,6 +19,8 @@ const ( MasterPasswordSuffix = "-master" MasterPasswordSecretKey = "password" SnapshotSource = "Snapshot" + AwsPostgres = "postgres" + AwsAuroraPostgres = "aurora-postgresql" ) type AWSProvider struct { @@ -36,8 +38,11 @@ func newAWSProvider(k8sClient client.Client, config *viper.Viper, serviceNS stri } func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) { + logger := log.FromContext(ctx) + logger.Info("provisioning crossplane aws database", "DatabaseSpec", spec) + switch spec.DbType { - case v1.AuroraPostgres: + case AwsAuroraPostgres: err := p.createAuroraDB(ctx, spec) if err != nil { return false, err @@ -50,16 +55,17 @@ func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bo if err != nil { return false, err } + logger.Info("checking provisioned instance readiness", "instanceReady", instanceReady, "clusterReady", clusterReady) if basefun.GetMultiAZEnabled(p.config) { instance2Ready, err := p.isDBInstanceReady(ctx, spec.ResourceName+"-2") if err != nil { return false, err } + logger.Info("checking provisioned instance readiness", "instance2Ready", instance2Ready) return instanceReady && instance2Ready && clusterReady, nil } - return instanceReady && clusterReady, nil - case v1.Postgres: + case AwsPostgres: err := p.createPostgres(ctx, spec) if err != nil { return false, err @@ -68,6 +74,7 @@ func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bo if err != nil { return false, err } + logger.Info("checking provisioned instance readiness", "instanceReady", instanceReady) return instanceReady, nil } return false, fmt.Errorf("%w: %s must be one of %s", v1.ErrInvalidDBType, spec.DbType, []v1.DatabaseType{v1.Postgres, v1.AuroraPostgres}) @@ -84,7 +91,7 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bo } deletionPolicy := client.PropagationPolicy(metav1.DeletePropagationBackground) - paramGroupName := getParameterGroupName(spec.ResourceName, spec.HostParams.DBVersion, spec.DbType) + paramGroupName := getParameterGroupName(spec) // Delete DBClusterParameterGroup if it exists dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} clusterParamGroupKey := client.ObjectKey{Name: paramGroupName} @@ -141,7 +148,7 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e logger := log.FromContext(ctx) // Handle database parameter group - paramGroupName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + paramGroupName := getParameterGroupName(params) dbParamGroup := &crossplaneaws.DBParameterGroup{} err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup) @@ -189,7 +196,7 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) error { logger := log.FromContext(ctx) // Handle database instance parameter group - paramGroupName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + paramGroupName := getParameterGroupName(params) dbParamGroup := &crossplaneaws.DBParameterGroup{} if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { if errors.IsNotFound(err) { @@ -294,14 +301,14 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e } func (p *AWSProvider) postgresDBInstance(params DatabaseSpec) *crossplaneaws.DBInstance { - ms64 := int64(params.HostParams.MinStorageGB) + ms64 := int64(params.MinStorageGB) multiAZ := basefun.GetMultiAZEnabled(p.config) var maxStorageVal *int64 - if params.HostParams.MaxStorageGB == 0 { + if params.MaxStorageGB == 0 { maxStorageVal = nil } else { - maxStorageVal = ¶ms.HostParams.MaxStorageGB + maxStorageVal = ¶ms.MaxStorageGB } var restoreFrom *crossplaneaws.RestoreDBInstanceBackupConfiguration @@ -327,7 +334,7 @@ func (p *AWSProvider) postgresDBInstance(params DatabaseSpec) *crossplaneaws.DBI Region: basefun.GetRegion(p.config), CustomDBInstanceParameters: crossplaneaws.CustomDBInstanceParameters{ ApplyImmediately: ptr.To(true), - SkipFinalSnapshot: params.HostParams.SkipFinalSnapshotBeforeDeletion, + SkipFinalSnapshot: params.SkipFinalSnapshotBeforeDeletion, VPCSecurityGroupIDRefs: []xpv1.Reference{ {Name: basefun.GetVpcSecurityGroupIDRefs(p.config)}, }, @@ -342,26 +349,26 @@ func (p *AWSProvider) postgresDBInstance(params DatabaseSpec) *crossplaneaws.DBI }, Key: MasterPasswordSecretKey, }, - EngineVersion: GetEngineVersion(params.HostParams, p.config), + EngineVersion: GetEngineVersion(params, p.config), RestoreFrom: restoreFrom, DBParameterGroupNameRef: &xpv1.Reference{ - Name: getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType), + Name: getParameterGroupName(params), }, }, - Engine: ¶ms.HostParams.Type, + Engine: ¶ms.DbType, MultiAZ: &multiAZ, - DBInstanceClass: ¶ms.HostParams.InstanceClass, + DBInstanceClass: ¶ms.InstanceClass, AllocatedStorage: &ms64, MaxAllocatedStorage: maxStorageVal, - MasterUsername: ¶ms.HostParams.MasterUsername, - PubliclyAccessible: ¶ms.HostParams.PubliclyAccessible, - EnableIAMDatabaseAuthentication: ¶ms.HostParams.EnableIAMDatabaseAuthentication, + MasterUsername: ¶ms.MasterUsername, + PubliclyAccessible: ¶ms.PubliclyAccessible, + EnableIAMDatabaseAuthentication: ¶ms.EnableIAMDatabaseAuthentication, EnablePerformanceInsights: ¶ms.EnablePerfInsight, EnableCloudwatchLogsExports: params.EnableCloudwatchLogsExport, BackupRetentionPeriod: ¶ms.BackupRetentionDays, StorageEncrypted: ptr.To(true), - StorageType: ¶ms.HostParams.StorageType, - Port: ¶ms.HostParams.Port, + StorageType: ¶ms.StorageType, + Port: ¶ms.Port, PreferredMaintenanceWindow: params.PreferredMaintenanceWindow, Tags: ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} @@ -375,7 +382,7 @@ func (p *AWSProvider) postgresDBInstance(params DatabaseSpec) *crossplaneaws.DBI ProviderConfigReference: &xpv1.Reference{ Name: basefun.GetProviderConfig(p.config), }, - DeletionPolicy: params.HostParams.DeletionPolicy, + DeletionPolicy: params.DeletionPolicy, }, }, } @@ -389,11 +396,11 @@ func (p *AWSProvider) postgresDBParameterGroup(params DatabaseSpec) *crossplanea forceSsl := "rds.force_ssl" transactionTimeout := "idle_in_transaction_session_timeout" transactionTimeoutValue := "300000" - pgName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + pgName := getParameterGroupName(params) sharedLib := "shared_preload_libraries" sharedLibValue := "pg_stat_statements,pg_cron" cron := "cron.database_name" - cronValue := params.MasterConnInfo.DatabaseName + cronValue := params.DatabaseName desc := "custom PG for " + pgName return &crossplaneaws.DBParameterGroup{ @@ -406,8 +413,8 @@ func (p *AWSProvider) postgresDBParameterGroup(params DatabaseSpec) *crossplanea Description: &desc, CustomDBParameterGroupParameters: crossplaneaws.CustomDBParameterGroupParameters{ DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ - Engine: params.HostParams.Type, - EngineVersion: GetEngineVersion(params.HostParams, p.config), + Engine: params.DbType, + EngineVersion: GetEngineVersion(params, p.config), }, Parameters: []crossplaneaws.CustomParameter{ {ParameterName: &logical, @@ -437,7 +444,7 @@ func (p *AWSProvider) postgresDBParameterGroup(params DatabaseSpec) *crossplanea ProviderConfigReference: &xpv1.Reference{ Name: basefun.GetProviderConfig(p.config), }, - DeletionPolicy: params.HostParams.DeletionPolicy, + DeletionPolicy: params.DeletionPolicy, }, }, } @@ -447,16 +454,16 @@ func (p *AWSProvider) updateDBInstance(ctx context.Context, params DatabaseSpec, // Create a patch snapshot from current DBInstance patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) - if params.DbType == v1.Postgres { + if params.DbType == AwsPostgres { multiAZ := basefun.GetMultiAZEnabled(p.config) - ms64 := int64(params.HostParams.MinStorageGB) + ms64 := int64(params.MinStorageGB) dbInstance.Spec.ForProvider.AllocatedStorage = &ms64 var maxStorageVal *int64 - if params.HostParams.MaxStorageGB == 0 { + if params.MaxStorageGB == 0 { maxStorageVal = nil } else { - maxStorageVal = ¶ms.HostParams.MaxStorageGB + maxStorageVal = ¶ms.MaxStorageGB } dbInstance.Spec.ForProvider.MaxAllocatedStorage = maxStorageVal dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports = params.EnableCloudwatchLogsExport @@ -464,9 +471,9 @@ func (p *AWSProvider) updateDBInstance(ctx context.Context, params DatabaseSpec, } enablePerfInsight := params.EnablePerfInsight dbInstance.Spec.ForProvider.EnablePerformanceInsights = &enablePerfInsight - dbInstance.Spec.DeletionPolicy = params.HostParams.DeletionPolicy + dbInstance.Spec.DeletionPolicy = params.DeletionPolicy dbInstance.Spec.ForProvider.CACertificateIdentifier = params.CACertificateIdentifier - if params.DbType == v1.AuroraPostgres { + if params.DbType == AwsAuroraPostgres { dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports = nil } // Compute a json patch based on the changed DBInstance @@ -511,15 +518,15 @@ func (p *AWSProvider) auroraDBInstance(params DatabaseSpec, isSecondInstance boo Region: basefun.GetRegion(p.config), CustomDBInstanceParameters: crossplaneaws.CustomDBInstanceParameters{ ApplyImmediately: ptr.To(true), - SkipFinalSnapshot: params.HostParams.SkipFinalSnapshotBeforeDeletion, - EngineVersion: GetEngineVersion(params.HostParams, p.config), + SkipFinalSnapshot: params.SkipFinalSnapshotBeforeDeletion, + EngineVersion: GetEngineVersion(params, p.config), DBParameterGroupNameRef: &xpv1.Reference{ - Name: getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType), + Name: getParameterGroupName(params), }, }, - Engine: ¶ms.HostParams.Type, - DBInstanceClass: ¶ms.HostParams.InstanceClass, - PubliclyAccessible: ¶ms.HostParams.PubliclyAccessible, + Engine: ¶ms.DbType, + DBInstanceClass: ¶ms.InstanceClass, + PubliclyAccessible: ¶ms.PubliclyAccessible, DBClusterIdentifier: ¶ms.ResourceName, EnablePerformanceInsights: ¶ms.EnablePerfInsight, EnableCloudwatchLogsExports: nil, @@ -532,7 +539,7 @@ func (p *AWSProvider) auroraDBInstance(params DatabaseSpec, isSecondInstance boo ProviderConfigReference: &xpv1.Reference{ Name: basefun.GetProviderConfig(p.config), }, - DeletionPolicy: params.HostParams.DeletionPolicy, + DeletionPolicy: params.DeletionPolicy, }, }, } @@ -558,7 +565,7 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus Region: basefun.GetRegion(p.config), BackupRetentionPeriod: auroraBackupRetentionPeriod, CustomDBClusterParameters: crossplaneaws.CustomDBClusterParameters{ - SkipFinalSnapshot: params.HostParams.SkipFinalSnapshotBeforeDeletion, + SkipFinalSnapshot: params.SkipFinalSnapshotBeforeDeletion, VPCSecurityGroupIDRefs: []xpv1.Reference{ {Name: basefun.GetVpcSecurityGroupIDRefs(p.config)}, }, @@ -574,19 +581,19 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus Key: MasterPasswordSecretKey, }, DBClusterParameterGroupNameRef: &xpv1.Reference{ - Name: getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType), + Name: getParameterGroupName(params), }, - EngineVersion: GetEngineVersion(params.HostParams, p.config), + EngineVersion: GetEngineVersion(params, p.config), }, - Engine: ¶ms.HostParams.Type, + Engine: ¶ms.DBVersion, Tags: ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} }), - MasterUsername: ¶ms.HostParams.MasterUsername, - EnableIAMDatabaseAuthentication: ¶ms.HostParams.EnableIAMDatabaseAuthentication, + MasterUsername: ¶ms.MasterUsername, + EnableIAMDatabaseAuthentication: ¶ms.EnableIAMDatabaseAuthentication, StorageEncrypted: ptr.To(true), - StorageType: ¶ms.HostParams.StorageType, - Port: ¶ms.HostParams.Port, + StorageType: ¶ms.StorageType, + Port: ¶ms.Port, EnableCloudwatchLogsExports: params.EnableCloudwatchLogsExport, IOPS: nil, PreferredMaintenanceWindow: params.PreferredMaintenanceWindow, @@ -599,7 +606,7 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus ProviderConfigReference: &xpv1.Reference{ Name: basefun.GetProviderConfig(p.config), }, - DeletionPolicy: params.HostParams.DeletionPolicy, + DeletionPolicy: params.DeletionPolicy, }, }, } @@ -613,11 +620,11 @@ func (p *AWSProvider) auroraClusterParamGroup(params DatabaseSpec) *crossplaneaw forceSsl := "rds.force_ssl" transactionTimeout := "idle_in_transaction_session_timeout" transactionTimeoutValue := "300000" - pgName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + pgName := getParameterGroupName(params) sharedLib := "shared_preload_libraries" sharedLibValue := "pg_stat_statements,pg_cron" cron := "cron.database_name" - cronValue := params.MasterConnInfo.DatabaseName + cronValue := params.DatabaseName desc := "custom PG for " + pgName return &crossplaneaws.DBClusterParameterGroup{ @@ -630,8 +637,8 @@ func (p *AWSProvider) auroraClusterParamGroup(params DatabaseSpec) *crossplaneaw Description: &desc, CustomDBClusterParameterGroupParameters: crossplaneaws.CustomDBClusterParameterGroupParameters{ DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ - Engine: params.HostParams.Type, - EngineVersion: GetEngineVersion(params.HostParams, p.config), + Engine: params.DbType, + EngineVersion: GetEngineVersion(params, p.config), }, Parameters: []crossplaneaws.CustomParameter{ {ParameterName: &logical, @@ -661,7 +668,7 @@ func (p *AWSProvider) auroraClusterParamGroup(params DatabaseSpec) *crossplaneaw ProviderConfigReference: &xpv1.Reference{ Name: basefun.GetProviderConfig(p.config), }, - DeletionPolicy: params.HostParams.DeletionPolicy, + DeletionPolicy: params.DeletionPolicy, }, }, } @@ -672,11 +679,11 @@ func (p *AWSProvider) auroraInstanceParamGroup(params DatabaseSpec) *crossplanea reboot := "pending-reboot" transactionTimeout := "idle_in_transaction_session_timeout" transactionTimeoutValue := "300000" - pgName := getParameterGroupName(params.ResourceName, params.HostParams.DBVersion, params.DbType) + pgName := getParameterGroupName(params) sharedLib := "shared_preload_libraries" sharedLibValue := "pg_stat_statements,pg_cron" cron := "cron.database_name" - cronValue := params.MasterConnInfo.DatabaseName + cronValue := params.DatabaseName desc := "custom PG for " + pgName return &crossplaneaws.DBParameterGroup{ @@ -689,8 +696,8 @@ func (p *AWSProvider) auroraInstanceParamGroup(params DatabaseSpec) *crossplanea Description: &desc, CustomDBParameterGroupParameters: crossplaneaws.CustomDBParameterGroupParameters{ DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ - Engine: params.HostParams.Type, - EngineVersion: GetEngineVersion(params.HostParams, p.config), + Engine: params.DbType, + EngineVersion: GetEngineVersion(params, p.config), }, Parameters: []crossplaneaws.CustomParameter{ {ParameterName: &transactionTimeout, @@ -712,7 +719,7 @@ func (p *AWSProvider) auroraInstanceParamGroup(params DatabaseSpec) *crossplanea ProviderConfigReference: &xpv1.Reference{ Name: basefun.GetProviderConfig(p.config), }, - DeletionPolicy: params.HostParams.DeletionPolicy, + DeletionPolicy: params.DeletionPolicy, }, }, } @@ -729,8 +736,8 @@ func (p *AWSProvider) updateAuroraDBCluster(ctx context.Context, params Database if params.BackupRetentionDays != 0 { dbCluster.Spec.ForProvider.BackupRetentionPeriod = ¶ms.BackupRetentionDays } - dbCluster.Spec.ForProvider.StorageType = ¶ms.HostParams.StorageType - dbCluster.Spec.DeletionPolicy = params.HostParams.DeletionPolicy + dbCluster.Spec.ForProvider.StorageType = ¶ms.StorageType + dbCluster.Spec.DeletionPolicy = params.DeletionPolicy // Compute a json patch based on the changed RDSInstance dbClusterPatchData, err := patchDBCluster.Data(dbCluster) @@ -870,7 +877,7 @@ func (p *AWSProvider) addInactiveOperationalTag(ctx context.Context, spec Databa return false, err } - if spec.DbType == v1.AuroraPostgres { + if spec.DbType == AwsAuroraPostgres { if tagged, err := p.markClusterAsInactive(ctx, spec.ResourceName); err != nil || !tagged { return false, err } diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index 4305058..332752b 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -4,10 +4,8 @@ import ( "context" crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - v1 "github.com/infobloxopen/db-controller/api/v1" basefun "github.com/infobloxopen/db-controller/pkg/basefunctions" "github.com/infobloxopen/db-controller/pkg/config" - "github.com/infobloxopen/db-controller/pkg/hostparams" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/spf13/viper" @@ -62,34 +60,23 @@ var _ = Describe("AWSProvider create Postgres database", func() { BeforeEach(func() { ctx = context.TODO() spec = DatabaseSpec{ - ResourceName: "env-app-name-db-1d9fb876", - HostParams: hostparams.HostParams{ - Shape: "db.t3.medium", - MinStorageGB: 10, - MaxStorageGB: 20, - DBVersion: "15.7", - SkipFinalSnapshotBeforeDeletion: true, - MasterUsername: "root", - EnableIAMDatabaseAuthentication: true, - StorageType: "storage-type", - DeletionPolicy: xpv1.DeletionOrphan, - PubliclyAccessible: true, - InstanceClass: "db.t3.medium", - }, - DbType: "postgres", - SharedDBHost: false, - MasterConnInfo: v1.DatabaseClaimConnectionInfo{ - Username: "testadmin", - Password: "test-password-123", - Port: "5432", - }, - TempSecret: "temp-secret-abc123", - EnableReplicationRole: true, - EnableSuperUser: false, - EnablePerfInsight: true, - EnableCloudwatchLogsExport: []*string{ptr.To("postgresql"), ptr.To("upgrade")}, - BackupRetentionDays: 7, - CACertificateIdentifier: ptr.To("rds-ca-2019"), + ResourceName: "env-app-name-db-1d9fb876", + DatabaseName: "app-name-db", + MinStorageGB: 10, + MaxStorageGB: 20, + DBVersion: "15.7", + SkipFinalSnapshotBeforeDeletion: true, + MasterUsername: "root", + EnableIAMDatabaseAuthentication: true, + StorageType: "storage-type", + DeletionPolicy: xpv1.DeletionOrphan, + PubliclyAccessible: true, + InstanceClass: "db.t3.medium", + DbType: "postgres", + EnablePerfInsight: true, + EnableCloudwatchLogsExport: []*string{ptr.To("postgresql"), ptr.To("upgrade")}, + BackupRetentionDays: 7, + CACertificateIdentifier: ptr.To("rds-ca-2019"), Tags: []ProviderTag{ {Key: "environment", Value: "test"}, {Key: "managed-by", Value: "controller-test"}, @@ -141,7 +128,7 @@ var _ = Describe("AWSProvider create Postgres database", func() { }, crossplaneaws.CustomParameter{ ParameterName: ptr.To("cron.database_name"), - ParameterValue: ptr.To(spec.MasterConnInfo.DatabaseName), + ParameterValue: ptr.To(spec.DatabaseName), ApplyMethod: ptr.To("pending-reboot"), }, )) @@ -151,22 +138,22 @@ var _ = Describe("AWSProvider create Postgres database", func() { return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) }).Should(Succeed()) - Expect(dbInstance.Spec.ForProvider.Engine).To(Equal(ptr.To(spec.HostParams.Type))) - Expect(dbInstance.Spec.ForProvider.EngineVersion).To(Equal(GetEngineVersion(spec.HostParams, provider.config))) - Expect(dbInstance.Spec.ForProvider.DBInstanceClass).To(Equal(ptr.To(spec.HostParams.InstanceClass))) - Expect(dbInstance.Spec.ForProvider.AllocatedStorage).To(Equal(ptr.To(int64(spec.HostParams.MinStorageGB)))) + Expect(dbInstance.Spec.ForProvider.Engine).To(Equal(ptr.To(spec.DbType))) + Expect(dbInstance.Spec.ForProvider.EngineVersion).To(Equal(GetEngineVersion(spec, provider.config))) + Expect(dbInstance.Spec.ForProvider.DBInstanceClass).To(Equal(ptr.To(spec.InstanceClass))) + Expect(dbInstance.Spec.ForProvider.AllocatedStorage).To(Equal(ptr.To(int64(spec.MinStorageGB)))) Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-15")) Expect(dbInstance.Spec.ForProvider.CACertificateIdentifier).To(Equal(spec.CACertificateIdentifier)) Expect(dbInstance.Spec.ForProvider.MultiAZ).To(Equal(ptr.To(basefun.GetMultiAZEnabled(provider.config)))) - Expect(dbInstance.Spec.ForProvider.MasterUsername).To(Equal(ptr.To(spec.HostParams.MasterUsername))) - Expect(dbInstance.Spec.ForProvider.PubliclyAccessible).To(Equal(ptr.To(spec.HostParams.PubliclyAccessible))) - Expect(dbInstance.Spec.ForProvider.EnableIAMDatabaseAuthentication).To(Equal(ptr.To(spec.HostParams.EnableIAMDatabaseAuthentication))) + Expect(dbInstance.Spec.ForProvider.MasterUsername).To(Equal(ptr.To(spec.MasterUsername))) + Expect(dbInstance.Spec.ForProvider.PubliclyAccessible).To(Equal(ptr.To(spec.PubliclyAccessible))) + Expect(dbInstance.Spec.ForProvider.EnableIAMDatabaseAuthentication).To(Equal(ptr.To(spec.EnableIAMDatabaseAuthentication))) Expect(dbInstance.Spec.ForProvider.EnablePerformanceInsights).To(Equal(ptr.To(spec.EnablePerfInsight))) Expect(dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports).To(Equal(spec.EnableCloudwatchLogsExport)) Expect(dbInstance.Spec.ForProvider.BackupRetentionPeriod).To(Equal(ptr.To(spec.BackupRetentionDays))) Expect(dbInstance.Spec.ForProvider.StorageEncrypted).To(Equal(ptr.To(true))) - Expect(dbInstance.Spec.ForProvider.StorageType).To(Equal(ptr.To(spec.HostParams.StorageType))) - Expect(dbInstance.Spec.ForProvider.Port).To(Equal(ptr.To(spec.HostParams.Port))) + Expect(dbInstance.Spec.ForProvider.StorageType).To(Equal(ptr.To(spec.StorageType))) + Expect(dbInstance.Spec.ForProvider.Port).To(Equal(ptr.To(spec.Port))) Expect(dbInstance.Spec.ForProvider.PreferredMaintenanceWindow).To(Equal(spec.PreferredMaintenanceWindow)) Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-15")) @@ -184,7 +171,7 @@ var _ = Describe("AWSProvider create Postgres database", func() { // Validate Provider Config & Deletion Policy Expect(dbInstance.Spec.ResourceSpec.ProviderConfigReference.Name).To(Equal(basefun.GetProviderConfig(provider.config))) - Expect(dbInstance.Spec.ResourceSpec.DeletionPolicy).To(Equal(spec.HostParams.DeletionPolicy)) + Expect(dbInstance.Spec.ResourceSpec.DeletionPolicy).To(Equal(spec.DeletionPolicy)) }) @@ -288,6 +275,66 @@ var _ = Describe("AWSProvider create Postgres database", func() { Describe("update postgres database", func() { When("when crossplane cr preexists the creation call we need to update with provided parameters", func() { + It("should propagate the new spec fields to crossplane CR", func() { + isReady, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + }).Should(Succeed()) + + updatedSpec := spec + updatedSpec.MaxStorageGB = 30 + updatedSpec.MinStorageGB = 20 + updatedSpec.EnableCloudwatchLogsExport = []*string{ptr.To("not"), ptr.To("upgrade")} + updatedSpec.EnablePerfInsight = false + updatedSpec.DeletionPolicy = xpv1.DeletionDelete + + isReady, err = provider.CreateDatabase(ctx, updatedSpec) + Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned + + dbInstance = &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + }).Should(Succeed()) + + Expect(dbInstance.Spec.ForProvider.AllocatedStorage).To(Equal(ptr.To(int64(updatedSpec.MinStorageGB)))) + Expect(dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports).To(Equal(updatedSpec.EnableCloudwatchLogsExport)) + Expect(dbInstance.Spec.ForProvider.EnablePerformanceInsights).To(Equal(ptr.To(updatedSpec.EnablePerfInsight))) + Expect(dbInstance.Spec.ResourceSpec.DeletionPolicy).To(Equal(updatedSpec.DeletionPolicy)) + Expect(dbInstance.Spec.ForProvider.Engine).To(Equal(ptr.To(updatedSpec.DbType))) + Expect(dbInstance.Spec.ForProvider.EngineVersion).To(Equal(GetEngineVersion(updatedSpec, provider.config))) + Expect(dbInstance.Spec.ForProvider.DBInstanceClass).To(Equal(ptr.To(updatedSpec.InstanceClass))) + Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-15")) + Expect(dbInstance.Spec.ForProvider.CACertificateIdentifier).To(Equal(updatedSpec.CACertificateIdentifier)) + Expect(dbInstance.Spec.ForProvider.MultiAZ).To(Equal(ptr.To(basefun.GetMultiAZEnabled(provider.config)))) + Expect(dbInstance.Spec.ForProvider.MasterUsername).To(Equal(ptr.To(updatedSpec.MasterUsername))) + Expect(dbInstance.Spec.ForProvider.PubliclyAccessible).To(Equal(ptr.To(updatedSpec.PubliclyAccessible))) + Expect(dbInstance.Spec.ForProvider.EnableIAMDatabaseAuthentication).To(Equal(ptr.To(updatedSpec.EnableIAMDatabaseAuthentication))) + Expect(dbInstance.Spec.ForProvider.BackupRetentionPeriod).To(Equal(ptr.To(updatedSpec.BackupRetentionDays))) + Expect(dbInstance.Spec.ForProvider.StorageEncrypted).To(Equal(ptr.To(true))) + Expect(dbInstance.Spec.ForProvider.StorageType).To(Equal(ptr.To(updatedSpec.StorageType))) + Expect(dbInstance.Spec.ForProvider.Port).To(Equal(ptr.To(updatedSpec.Port))) + Expect(dbInstance.Spec.ForProvider.PreferredMaintenanceWindow).To(Equal(updatedSpec.PreferredMaintenanceWindow)) + Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-15")) + + // master password + Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Name).To(Equal(updatedSpec.ResourceName + MasterPasswordSuffix)) + Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Namespace).To(Equal(provider.serviceNS)) + Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Name).To(Equal(updatedSpec.ResourceName)) + Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Namespace).To(Equal(provider.serviceNS)) + + // Validate VPC & Security Groups + Expect(dbInstance.Spec.ForProvider.VPCSecurityGroupIDRefs).To(ContainElement(xpv1.Reference{ + Name: basefun.GetVpcSecurityGroupIDRefs(provider.config), + })) + Expect(dbInstance.Spec.ForProvider.DBSubnetGroupNameRef.Name).To(Equal(basefun.GetDbSubnetGroupNameRef(provider.config))) + + // Validate Provider Config & Deletion Policy + Expect(dbInstance.Spec.ResourceSpec.ProviderConfigReference.Name).To(Equal(basefun.GetProviderConfig(provider.config))) + }) }) }) @@ -752,3 +799,339 @@ func TestChangeToInactive(t *testing.T) { }) } } + +func TestAuroraDBInstance(t *testing.T) { + tests := []struct { + name string + params DatabaseSpec + isSecondInstance bool + expectedResult *crossplaneaws.DBInstance + }{ + { + name: "Creates primary Aurora DB instance with correct configuration", + params: DatabaseSpec{ + ResourceName: "test-db-cluster", + DbType: AwsAuroraPostgres, + InstanceClass: "db.r5.large", + PubliclyAccessible: true, + EnablePerfInsight: true, + PreferredMaintenanceWindow: ptr.To("sun:05:00-sun:06:00"), + SkipFinalSnapshotBeforeDeletion: true, + CACertificateIdentifier: ptr.To("rds-ca-2019"), + DeletionPolicy: xpv1.DeletionDelete, + Labels: map[string]string{"env": "test"}, + Tags: []ProviderTag{{Key: "Environment", Value: "Test"}}, + }, + isSecondInstance: false, + expectedResult: &crossplaneaws.DBInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-db-cluster", + Labels: map[string]string{"env": "test"}, + }, + Spec: crossplaneaws.DBInstanceSpec{ + ForProvider: crossplaneaws.DBInstanceParameters{ + CACertificateIdentifier: ptr.To("rds-ca-2019"), + Region: "us-west-2", + CustomDBInstanceParameters: crossplaneaws.CustomDBInstanceParameters{ + ApplyImmediately: ptr.To(true), + SkipFinalSnapshot: true, + EngineVersion: ptr.To("15"), + DBParameterGroupNameRef: &xpv1.Reference{ + Name: "param-group-test-db-cluster", + }, + }, + Engine: ptr.To("aurora-postgresql"), + DBInstanceClass: ptr.To("db.r5.large"), + PubliclyAccessible: ptr.To(true), + DBClusterIdentifier: ptr.To("test-db-cluster"), + EnablePerformanceInsights: ptr.To(true), + EnableCloudwatchLogsExports: nil, + PreferredMaintenanceWindow: ptr.To("sun:05:00-sun:06:00"), + Tags: []*crossplaneaws.Tag{ + {Key: ptr.To("Environment"), Value: ptr.To("Test")}, + }, + }, + ResourceSpec: xpv1.ResourceSpec{ + ProviderConfigReference: &xpv1.Reference{ + Name: "aws-provider", + }, + DeletionPolicy: xpv1.DeletionDelete, + }, + }, + }, + }, + { + name: "Creates secondary Aurora DB instance with -2 suffix", + params: DatabaseSpec{ + ResourceName: "test-db-cluster", + DbType: AwsAuroraPostgres, + InstanceClass: "db.r5.xlarge", + PubliclyAccessible: false, + EnablePerfInsight: false, + PreferredMaintenanceWindow: ptr.To("mon:03:00-mon:04:00"), + SkipFinalSnapshotBeforeDeletion: false, + CACertificateIdentifier: ptr.To("rds-ca-2019"), + DeletionPolicy: xpv1.DeletionOrphan, + Labels: map[string]string{"env": "prod"}, + Tags: []ProviderTag{{Key: "Environment", Value: "Production"}}, + }, + isSecondInstance: true, + expectedResult: &crossplaneaws.DBInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-db-cluster-2", + Labels: map[string]string{"env": "prod"}, + }, + Spec: crossplaneaws.DBInstanceSpec{ + ForProvider: crossplaneaws.DBInstanceParameters{ + CACertificateIdentifier: ptr.To("rds-ca-2019"), + Region: "us-west-2", + CustomDBInstanceParameters: crossplaneaws.CustomDBInstanceParameters{ + ApplyImmediately: ptr.To(true), + SkipFinalSnapshot: false, + EngineVersion: ptr.To("13.4"), + DBParameterGroupNameRef: &xpv1.Reference{ + Name: "param-group-test-db-cluster", + }, + }, + Engine: ptr.To("aurora-postgresql"), + DBInstanceClass: ptr.To("db.r5.xlarge"), + PubliclyAccessible: ptr.To(false), + DBClusterIdentifier: ptr.To("test-db-cluster"), + EnablePerformanceInsights: ptr.To(false), + EnableCloudwatchLogsExports: nil, + PreferredMaintenanceWindow: ptr.To("mon:03:00-mon:04:00"), + Tags: []*crossplaneaws.Tag{ + {Key: ptr.To("Environment"), Value: ptr.To("Production")}, + }, + }, + ResourceSpec: xpv1.ResourceSpec{ + ProviderConfigReference: &xpv1.Reference{ + Name: "aws-provider", + }, + DeletionPolicy: xpv1.DeletionOrphan, + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + mockConfig := viper.New() + mockConfig.Set("providerConfig", "aws-provider") + mockConfig.Set("region", "us-west-2") + provider := &AWSProvider{config: mockConfig} + + result := provider.auroraDBInstance(tc.params, tc.isSecondInstance) + Expect(result.ObjectMeta.Name).To(Equal(tc.expectedResult.ObjectMeta.Name)) + Expect(result.ObjectMeta.Labels).To(Equal(tc.expectedResult.ObjectMeta.Labels)) + Expect(result.Spec.ForProvider.Engine).To(Equal(tc.expectedResult.Spec.ForProvider.Engine)) + Expect(result.Spec.ForProvider.DBInstanceClass).To(Equal(tc.expectedResult.Spec.ForProvider.DBInstanceClass)) + Expect(result.Spec.ForProvider.PubliclyAccessible).To(Equal(tc.expectedResult.Spec.ForProvider.PubliclyAccessible)) + Expect(result.Spec.ForProvider.DBClusterIdentifier).To(Equal(tc.expectedResult.Spec.ForProvider.DBClusterIdentifier)) + Expect(result.Spec.ForProvider.EnablePerformanceInsights).To(Equal(tc.expectedResult.Spec.ForProvider.EnablePerformanceInsights)) + Expect(result.Spec.ForProvider.PreferredMaintenanceWindow).To(Equal(tc.expectedResult.Spec.ForProvider.PreferredMaintenanceWindow)) + Expect(result.Spec.ForProvider.CACertificateIdentifier).To(Equal(tc.expectedResult.Spec.ForProvider.CACertificateIdentifier)) + Expect(result.Spec.ForProvider.ApplyImmediately).To(Equal(tc.expectedResult.Spec.ForProvider.ApplyImmediately)) + Expect(result.Spec.ForProvider.SkipFinalSnapshot).To(Equal(tc.expectedResult.Spec.ForProvider.SkipFinalSnapshot)) + Expect(result.Spec.DeletionPolicy).To(Equal(tc.expectedResult.Spec.DeletionPolicy)) + Expect(result.Spec.ProviderConfigReference.Name).To(Equal(tc.expectedResult.Spec.ProviderConfigReference.Name)) + Expect(result.Spec.ForProvider.Region).To(Equal("us-west-2")) + + if len(tc.params.Tags) > 0 { + Expect(len(result.Spec.ForProvider.Tags)).To(BeNumerically(">", 0)) + } + }) + } +} + +func TestAuroraDBCluster(t *testing.T) { + tests := []struct { + name string + params DatabaseSpec + expectedResult *crossplaneaws.DBCluster + }{ + { + name: "Creates Aurora DB Cluster with backup retention days", + params: DatabaseSpec{ + ResourceName: "test-db-cluster", + DBVersion: "aurora-mysql", + BackupRetentionDays: 7, + SkipFinalSnapshotBeforeDeletion: true, + MasterUsername: "admin", + EnableIAMDatabaseAuthentication: true, + StorageType: "aurora", + Port: 3306, + EnableCloudwatchLogsExport: []*string{ptr.To("audit"), ptr.To("error")}, + PreferredMaintenanceWindow: ptr.To("sun:05:00-sun:06:00"), + DeletionPolicy: xpv1.DeletionDelete, + Tags: []ProviderTag{{Key: "Environment", Value: "Test"}}, + }, + expectedResult: &crossplaneaws.DBCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-db-cluster", + }, + Spec: crossplaneaws.DBClusterSpec{ + ForProvider: crossplaneaws.DBClusterParameters{ + Region: "us-west-2", + BackupRetentionPeriod: ptr.To(int64(7)), + CustomDBClusterParameters: crossplaneaws.CustomDBClusterParameters{ + SkipFinalSnapshot: true, + VPCSecurityGroupIDRefs: []xpv1.Reference{ + {Name: "security-group-id"}, + }, + DBSubnetGroupNameRef: &xpv1.Reference{ + Name: "subnet-group", + }, + AutogeneratePassword: true, + MasterUserPasswordSecretRef: &xpv1.SecretKeySelector{ + SecretReference: xpv1.SecretReference{ + Name: "test-db-cluster-master-password", + Namespace: "default", + }, + Key: "password", + }, + DBClusterParameterGroupNameRef: &xpv1.Reference{ + Name: "param-group-test-db-cluster", + }, + EngineVersion: ptr.To("aurora-postgresql"), + }, + Engine: ptr.To("aurora-mysql"), + Tags: []*crossplaneaws.Tag{ + {Key: ptr.To("Environment"), Value: ptr.To("Test")}, + }, + MasterUsername: ptr.To("admin"), + EnableIAMDatabaseAuthentication: ptr.To(true), + StorageEncrypted: ptr.To(true), + StorageType: ptr.To("aurora"), + Port: ptr.To(int64(3306)), + EnableCloudwatchLogsExports: []*string{ptr.To("audit"), ptr.To("error")}, + IOPS: nil, + PreferredMaintenanceWindow: ptr.To("sun:05:00-sun:06:00"), + }, + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "test-db-cluster", + Namespace: "default", + }, + ProviderConfigReference: &xpv1.Reference{ + Name: "aws-provider", + }, + DeletionPolicy: xpv1.DeletionDelete, + }, + }, + }, + }, + { + name: "Creates Aurora DB Cluster without backup retention days", + params: DatabaseSpec{ + ResourceName: "prod-db-cluster", + DBVersion: "aurora-postgresql", + BackupRetentionDays: 0, + SkipFinalSnapshotBeforeDeletion: false, + MasterUsername: "postgres", + EnableIAMDatabaseAuthentication: false, + StorageType: "aurora-iopt1", + Port: 5432, + EnableCloudwatchLogsExport: []*string{ptr.To("postgresql")}, + PreferredMaintenanceWindow: ptr.To("mon:03:00-mon:04:00"), + DeletionPolicy: xpv1.DeletionOrphan, + Tags: []ProviderTag{{Key: "Environment", Value: "Production"}}, + }, + expectedResult: &crossplaneaws.DBCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "prod-db-cluster", + }, + Spec: crossplaneaws.DBClusterSpec{ + ForProvider: crossplaneaws.DBClusterParameters{ + Region: "us-west-2", + BackupRetentionPeriod: nil, + CustomDBClusterParameters: crossplaneaws.CustomDBClusterParameters{ + SkipFinalSnapshot: false, + VPCSecurityGroupIDRefs: []xpv1.Reference{ + {Name: "security-group-id"}, + }, + DBSubnetGroupNameRef: &xpv1.Reference{ + Name: "subnet-group", + }, + AutogeneratePassword: true, + MasterUserPasswordSecretRef: &xpv1.SecretKeySelector{ + SecretReference: xpv1.SecretReference{ + Name: "prod-db-cluster-master-password", + Namespace: "default", + }, + Key: "password", + }, + DBClusterParameterGroupNameRef: &xpv1.Reference{ + Name: "param-group-prod-db-cluster", + }, + EngineVersion: ptr.To("13.4"), + }, + Engine: ptr.To("aurora-postgresql"), + Tags: []*crossplaneaws.Tag{ + {Key: ptr.To("Environment"), Value: ptr.To("Production")}, + }, + MasterUsername: ptr.To("postgres"), + EnableIAMDatabaseAuthentication: ptr.To(false), + StorageEncrypted: ptr.To(true), + StorageType: ptr.To("aurora-iopt1"), + Port: ptr.To(int64(5432)), + EnableCloudwatchLogsExports: []*string{ptr.To("postgresql")}, + IOPS: nil, + PreferredMaintenanceWindow: ptr.To("mon:03:00-mon:04:00"), + }, + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "prod-db-cluster", + Namespace: "default", + }, + ProviderConfigReference: &xpv1.Reference{ + Name: "aws-provider", + }, + DeletionPolicy: xpv1.DeletionOrphan, + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + RegisterTestingT(t) + mockConfig := viper.New() + mockConfig.Set("providerConfig", "aws-provider") + mockConfig.Set("region", "us-west-2") + provider := &AWSProvider{config: mockConfig} + + result := provider.auroraDBCluster(tc.params) + + Expect(result.ObjectMeta.Name).To(Equal(tc.expectedResult.ObjectMeta.Name)) + Expect(result.Spec.ForProvider.Engine).To(Equal(tc.expectedResult.Spec.ForProvider.Engine)) + Expect(result.Spec.ForProvider.BackupRetentionPeriod).To(Equal(tc.expectedResult.Spec.ForProvider.BackupRetentionPeriod)) + Expect(result.Spec.ForProvider.MasterUsername).To(Equal(tc.expectedResult.Spec.ForProvider.MasterUsername)) + Expect(result.Spec.ForProvider.EnableIAMDatabaseAuthentication).To(Equal(tc.expectedResult.Spec.ForProvider.EnableIAMDatabaseAuthentication)) + Expect(result.Spec.ForProvider.StorageEncrypted).To(Equal(tc.expectedResult.Spec.ForProvider.StorageEncrypted)) + Expect(result.Spec.ForProvider.StorageType).To(Equal(tc.expectedResult.Spec.ForProvider.StorageType)) + Expect(result.Spec.ForProvider.Port).To(Equal(tc.expectedResult.Spec.ForProvider.Port)) + Expect(result.Spec.ForProvider.EnableCloudwatchLogsExports).To(Equal(tc.expectedResult.Spec.ForProvider.EnableCloudwatchLogsExports)) + Expect(result.Spec.ForProvider.PreferredMaintenanceWindow).To(Equal(tc.expectedResult.Spec.ForProvider.PreferredMaintenanceWindow)) + + Expect(result.Spec.ForProvider.SkipFinalSnapshot).To(Equal(tc.expectedResult.Spec.ForProvider.SkipFinalSnapshot)) + Expect(result.Spec.ForProvider.AutogeneratePassword).To(Equal(tc.expectedResult.Spec.ForProvider.AutogeneratePassword)) + + Expect(result.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Name).To(Equal(tc.params.ResourceName + MasterPasswordSuffix)) + Expect(result.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Namespace).To(Equal(provider.serviceNS)) + Expect(result.Spec.ForProvider.MasterUserPasswordSecretRef.Key).To(Equal(MasterPasswordSecretKey)) + + Expect(result.Spec.DeletionPolicy).To(Equal(tc.expectedResult.Spec.DeletionPolicy)) + Expect(result.Spec.ProviderConfigReference.Name).To(Equal(tc.expectedResult.Spec.ProviderConfigReference.Name)) + Expect(result.Spec.WriteConnectionSecretToReference.Name).To(Equal(tc.params.ResourceName)) + Expect(result.Spec.WriteConnectionSecretToReference.Namespace).To(Equal(provider.serviceNS)) + + if len(tc.params.Tags) > 0 { + Expect(len(result.Spec.ForProvider.Tags)).To(BeNumerically(">", 0)) + } + }) + } +} diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go index 649e150..5c64e19 100644 --- a/pkg/providers/providers.go +++ b/pkg/providers/providers.go @@ -3,8 +3,7 @@ package providers import ( "context" "fmt" - v1 "github.com/infobloxopen/db-controller/api/v1" - "github.com/infobloxopen/db-controller/pkg/hostparams" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/spf13/viper" "sigs.k8s.io/controller-runtime/pkg/client" @@ -12,18 +11,26 @@ import ( // DatabaseSpec defines the required parameters to provision a database using any provider. type DatabaseSpec struct { - ResourceName string - HostParams hostparams.HostParams - DbType v1.DatabaseType - SharedDBHost bool - MasterConnInfo v1.DatabaseClaimConnectionInfo - TempSecret string - EnableReplicationRole bool - EnableSuperUser bool - EnablePerfInsight bool - EnableCloudwatchLogsExport []*string - BackupRetentionDays int64 - CACertificateIdentifier *string + ResourceName string + DatabaseName string + DbType string + Port int64 + MinStorageGB int + MaxStorageGB int64 + DBVersion string + + MasterUsername string + InstanceClass string + StorageType string + SkipFinalSnapshotBeforeDeletion bool + PubliclyAccessible bool + EnableIAMDatabaseAuthentication bool + DeletionPolicy xpv1.DeletionPolicy + IsDefaultVersion bool + EnablePerfInsight bool + EnableCloudwatchLogsExport []*string + BackupRetentionDays int64 + CACertificateIdentifier *string Tags []ProviderTag Labels map[string]string diff --git a/pkg/providers/utils.go b/pkg/providers/utils.go index 97ff6b8..07c5505 100644 --- a/pkg/providers/utils.go +++ b/pkg/providers/utils.go @@ -4,9 +4,7 @@ import ( "context" "fmt" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" - v1 "github.com/infobloxopen/db-controller/api/v1" basefun "github.com/infobloxopen/db-controller/pkg/basefunctions" - "github.com/infobloxopen/db-controller/pkg/hostparams" gopassword "github.com/sethvargo/go-password/password" "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" @@ -62,24 +60,24 @@ func generateMasterPassword() (string, error) { return pass, nil } -func GetEngineVersion(params hostparams.HostParams, config *viper.Viper) *string { +func GetEngineVersion(spec DatabaseSpec, config *viper.Viper) *string { defaultMajorVersion := "" - if params.IsDefaultVersion { + if spec.IsDefaultVersion { defaultMajorVersion = basefun.GetDefaultMajorVersion(config) } else { - defaultMajorVersion = params.DBVersion + defaultMajorVersion = spec.DBVersion } return &defaultMajorVersion } -func getParameterGroupName(providerResourceName, dbVersion string, dbType v1.DatabaseType) string { - switch dbType { - case v1.Postgres: - return providerResourceName + "-" + (strings.Split(dbVersion, "."))[0] - case v1.AuroraPostgres: - return providerResourceName + "-a-" + (strings.Split(dbVersion, "."))[0] +func getParameterGroupName(spec DatabaseSpec) string { + switch spec.DbType { + case AwsPostgres: + return spec.ResourceName + "-" + (strings.Split(spec.DBVersion, "."))[0] + case AwsAuroraPostgres: + return spec.ResourceName + "-a-" + (strings.Split(spec.DBVersion, "."))[0] default: - return providerResourceName + "-" + (strings.Split(dbVersion, "."))[0] + return spec.ResourceName + "-" + (strings.Split(spec.DBVersion, "."))[0] } } diff --git a/pkg/providers/utils_test.go b/pkg/providers/utils_test.go index fbbc7a1..0063cc3 100644 --- a/pkg/providers/utils_test.go +++ b/pkg/providers/utils_test.go @@ -1,7 +1,6 @@ package providers import ( - v1 "github.com/infobloxopen/db-controller/api/v1" "testing" ) @@ -10,35 +9,35 @@ func TestGetParameterGroupName(t *testing.T) { name string providerResourceName string dbVersion string - dbType v1.DatabaseType + dbType string expectedParameterName string }{ { name: "Postgres DB", providerResourceName: "env-app-name-db-1d9fb87", dbVersion: "14.5", - dbType: v1.Postgres, + dbType: AwsPostgres, expectedParameterName: "env-app-name-db-1d9fb87-14", }, { name: "Aurora Postgres DB", providerResourceName: "env-app-name-db-1d9fb87", dbVersion: "13.3", - dbType: v1.AuroraPostgres, + dbType: AwsAuroraPostgres, expectedParameterName: "env-app-name-db-1d9fb87-a-13", }, { name: "Malformed version, should still extract major version", providerResourceName: "env-app-name-db-1d9fb87", dbVersion: "9", - dbType: v1.Postgres, + dbType: AwsPostgres, expectedParameterName: "env-app-name-db-1d9fb87-9", }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - result := getParameterGroupName(test.providerResourceName, test.dbVersion, test.dbType) + result := getParameterGroupName(DatabaseSpec{ResourceName: test.providerResourceName, DBVersion: test.dbVersion, DbType: test.dbType}) if result != test.expectedParameterName { t.Errorf("[%s] expected: %s, got: %s", test.name, test.expectedParameterName, result) } From 340a859d6f434ee7f9535bc127f315e9df3e0474 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Sat, 15 Mar 2025 17:51:57 -0300 Subject: [PATCH 11/22] fix: wrong paramenter group for postgres rds --- pkg/providers/crossplane_aws.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index ee48f69..6962073 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -154,7 +154,7 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup) if errors.IsNotFound(err) { logger.Info("Creating Postgres Instance parameter group", "paramGroupName", paramGroupName) - if createErr := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); createErr != nil { + if createErr := p.k8sClient.Create(ctx, p.postgresDBInstance(params)); createErr != nil { return fmt.Errorf("failed to create DB parameter group: %w", createErr) } } else if err != nil { From 12d88908fcc2f538bf4c4a178121b80bde1bfca7 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Sat, 15 Mar 2025 17:57:57 -0300 Subject: [PATCH 12/22] refactor: embed k8s client into aws provider --- pkg/providers/crossplane_aws.go | 75 ++++++++++++++-------------- pkg/providers/crossplane_aws_test.go | 6 +-- 2 files changed, 41 insertions(+), 40 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 6962073..f6dc2cb 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -24,14 +24,15 @@ const ( ) type AWSProvider struct { - k8sClient client.Client + client.Client + config *viper.Viper serviceNS string } -func newAWSProvider(k8sClient client.Client, config *viper.Viper, serviceNS string) Provider { +func newAWSProvider(Client client.Client, config *viper.Viper, serviceNS string) Provider { return &AWSProvider{ - k8sClient: k8sClient, + Client: Client, config: config, serviceNS: serviceNS, } @@ -95,8 +96,8 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bo // Delete DBClusterParameterGroup if it exists dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} clusterParamGroupKey := client.ObjectKey{Name: paramGroupName} - if err := p.k8sClient.Get(ctx, clusterParamGroupKey, dbClusterParamGroup); err == nil { - if err := p.k8sClient.Delete(ctx, dbClusterParamGroup, deletionPolicy); err != nil { + if err := p.Client.Get(ctx, clusterParamGroupKey, dbClusterParamGroup); err == nil { + if err := p.Client.Delete(ctx, dbClusterParamGroup, deletionPolicy); err != nil { return false, err } } @@ -104,8 +105,8 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bo // Delete DBParameterGroup if it exists dbParameterGroup := &crossplaneaws.DBParameterGroup{} paramGroupKey := client.ObjectKey{Name: paramGroupName} - if err := p.k8sClient.Get(ctx, paramGroupKey, dbParameterGroup); err == nil { - if err := p.k8sClient.Delete(ctx, dbParameterGroup, deletionPolicy); err != nil { + if err := p.Client.Get(ctx, paramGroupKey, dbParameterGroup); err == nil { + if err := p.Client.Delete(ctx, dbParameterGroup, deletionPolicy); err != nil { return false, err } } @@ -113,8 +114,8 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bo // Delete DBCluster if it exists dbCluster := &crossplaneaws.DBCluster{} clusterKey := client.ObjectKey{Name: spec.ResourceName} - if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err == nil { - if err := p.k8sClient.Delete(ctx, dbCluster, deletionPolicy); err != nil { + if err := p.Client.Get(ctx, clusterKey, dbCluster); err == nil { + if err := p.Client.Delete(ctx, dbCluster, deletionPolicy); err != nil { return false, err } } @@ -122,8 +123,8 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bo // Delete DBInstance if it exists instanceKey := client.ObjectKey{Name: spec.ResourceName} dbInstance := &crossplaneaws.DBInstance{} - if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err == nil { - if err := p.k8sClient.Delete(ctx, dbInstance, deletionPolicy); err != nil { + if err := p.Client.Get(ctx, instanceKey, dbInstance); err == nil { + if err := p.Client.Delete(ctx, dbInstance, deletionPolicy); err != nil { return false, err } } @@ -131,8 +132,8 @@ func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bo // Delete secondary DBInstance if it exists dbInstance2 := &crossplaneaws.DBInstance{} instance2Key := client.ObjectKey{Name: spec.ResourceName + "-2"} - if err := p.k8sClient.Get(ctx, instance2Key, dbInstance2); err == nil { - if err := p.k8sClient.Delete(ctx, dbInstance2, deletionPolicy); err != nil { + if err := p.Client.Get(ctx, instance2Key, dbInstance2); err == nil { + if err := p.Client.Delete(ctx, dbInstance2, deletionPolicy); err != nil { return false, err } } @@ -151,10 +152,10 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e paramGroupName := getParameterGroupName(params) dbParamGroup := &crossplaneaws.DBParameterGroup{} - err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup) + err := p.Client.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup) if errors.IsNotFound(err) { logger.Info("Creating Postgres Instance parameter group", "paramGroupName", paramGroupName) - if createErr := p.k8sClient.Create(ctx, p.postgresDBInstance(params)); createErr != nil { + if createErr := p.Client.Create(ctx, p.postgresDBParameterGroup(params)); createErr != nil { return fmt.Errorf("failed to create DB parameter group: %w", createErr) } } else if err != nil { @@ -165,17 +166,17 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e dbInstance := &crossplaneaws.DBInstance{} instanceKey := client.ObjectKey{Name: params.ResourceName} - err = p.k8sClient.Get(ctx, instanceKey, dbInstance) + err = p.Client.Get(ctx, instanceKey, dbInstance) if errors.IsNotFound(err) { dbInstance = p.postgresDBInstance(params) // Create master password secret before creating the DB instance - if passwordErr := ManageMasterPassword(ctx, dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef, p.k8sClient); passwordErr != nil { + if passwordErr := ManageMasterPassword(ctx, dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef, p.Client); passwordErr != nil { return fmt.Errorf("failed to manage master password for DB instance %s: %w", params.ResourceName, passwordErr) } logger.Info("Creating Postgres db instance", "dbInstance", params.ResourceName) - if createErr := p.k8sClient.Create(ctx, dbInstance); createErr != nil { + if createErr := p.Client.Create(ctx, dbInstance); createErr != nil { return fmt.Errorf("failed to create DB instance %s: %w", params.ResourceName, createErr) } } else if err != nil { @@ -198,10 +199,10 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e // Handle database instance parameter group paramGroupName := getParameterGroupName(params) dbParamGroup := &crossplaneaws.DBParameterGroup{} - if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { + if err := p.Client.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { if errors.IsNotFound(err) { logger.Info("Creating Aurora DB Instance parameter group", "paramGroupName", paramGroupName) - if err := p.k8sClient.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { + if err := p.Client.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { return fmt.Errorf("failed to create DB parameter group: %w", err) } } else { @@ -211,10 +212,10 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e // Handle database cluster parameter group dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} - if err := p.k8sClient.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbClusterParamGroup); err != nil { + if err := p.Client.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbClusterParamGroup); err != nil { if errors.IsNotFound(err) { logger.Info("Creating Aurora DB Cluster parameter group", "paramGroupName", paramGroupName) - if err := p.k8sClient.Create(ctx, p.auroraClusterParamGroup(params)); err != nil { + if err := p.Client.Create(ctx, p.auroraClusterParamGroup(params)); err != nil { return fmt.Errorf("failed to create DB parameter group: %w", err) } } else { @@ -225,16 +226,16 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e // Handle aurora database cluster creation dbCluster := &crossplaneaws.DBCluster{} clusterKey := client.ObjectKey{Name: params.ResourceName} - if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err != nil { + if err := p.Client.Get(ctx, clusterKey, dbCluster); err != nil { if errors.IsNotFound(err) { dbCluster = p.auroraDBCluster(params) // Create master password secret before creating the DB instance - passwordErr := ManageMasterPassword(ctx, dbCluster.Spec.ForProvider.CustomDBClusterParameters.MasterUserPasswordSecretRef, p.k8sClient) + passwordErr := ManageMasterPassword(ctx, dbCluster.Spec.ForProvider.CustomDBClusterParameters.MasterUserPasswordSecretRef, p.Client) if passwordErr != nil { return fmt.Errorf("failed to manage master password: %w", passwordErr) } logger.Info("Creating Aurora DB Cluster", "name", params.ResourceName) - if err := p.k8sClient.Create(ctx, dbCluster); err != nil { + if err := p.Client.Create(ctx, dbCluster); err != nil { return fmt.Errorf("failed to create DB instance: %w", err) } } else { @@ -245,11 +246,11 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e // Handle primary database instance primaryDbInstance := &crossplaneaws.DBInstance{} primaryInstanceKey := client.ObjectKey{Name: params.ResourceName} - if err := p.k8sClient.Get(ctx, primaryInstanceKey, primaryDbInstance); err != nil { + if err := p.Client.Get(ctx, primaryInstanceKey, primaryDbInstance); err != nil { if errors.IsNotFound(err) { primaryDbInstance = p.auroraDBInstance(params, false) logger.Info("Creating Aurora DB Instance", "name", params.ResourceName) - if err := p.k8sClient.Create(ctx, primaryDbInstance); err != nil { + if err := p.Client.Create(ctx, primaryDbInstance); err != nil { return fmt.Errorf("failed to create DB instance: %w", err) } } else { @@ -275,11 +276,11 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e if basefun.GetMultiAZEnabled(p.config) { secondaryDbInstance := &crossplaneaws.DBInstance{} secondaryInstanceKey := client.ObjectKey{Name: params.ResourceName + "-2"} - if err := p.k8sClient.Get(ctx, secondaryInstanceKey, secondaryDbInstance); err != nil { + if err := p.Client.Get(ctx, secondaryInstanceKey, secondaryDbInstance); err != nil { if errors.IsNotFound(err) { secondaryDbInstance = p.auroraDBInstance(params, true) logger.Info("Creating Aurora DB Secondary Instance", "name", params.ResourceName+"-2") - if err := p.k8sClient.Create(ctx, secondaryDbInstance); err != nil { + if err := p.Client.Create(ctx, secondaryDbInstance); err != nil { return fmt.Errorf("failed to create DB instance: %w", err) } } else { @@ -491,7 +492,7 @@ func (p *AWSProvider) updateDBInstance(ctx context.Context, params DatabaseSpec, dbInstance.Spec.ForProvider.PreferredMaintenanceWindow = params.PreferredMaintenanceWindow } - err = p.k8sClient.Patch(ctx, dbInstance, patchDBInstance) + err = p.Client.Patch(ctx, dbInstance, patchDBInstance) if err != nil { return err } @@ -750,7 +751,7 @@ func (p *AWSProvider) updateAuroraDBCluster(ctx context.Context, params Database return nil } - err = p.k8sClient.Patch(ctx, dbCluster, patchDBCluster) + err = p.Client.Patch(ctx, dbCluster, patchDBCluster) if err != nil { return err } @@ -770,7 +771,7 @@ func (p *AWSProvider) configureDBTags(params *DatabaseSpec) { func (p *AWSProvider) isDBInstanceReady(ctx context.Context, instanceName string) (bool, error) { dbInstance := &crossplaneaws.DBInstance{} instanceKey := client.ObjectKey{Name: instanceName} - if err := p.k8sClient.Get(ctx, instanceKey, dbInstance); err != nil { + if err := p.Client.Get(ctx, instanceKey, dbInstance); err != nil { if errors.IsNotFound(err) { return false, nil } @@ -782,7 +783,7 @@ func (p *AWSProvider) isDBInstanceReady(ctx context.Context, instanceName string func (p *AWSProvider) isDBClusterReady(ctx context.Context, clusterName string) (bool, error) { dbCluster := &crossplaneaws.DBCluster{} clusterKey := client.ObjectKey{Name: clusterName} - if err := p.k8sClient.Get(ctx, clusterKey, dbCluster); err != nil { + if err := p.Client.Get(ctx, clusterKey, dbCluster); err != nil { if errors.IsNotFound(err) { return false, nil } @@ -793,7 +794,7 @@ func (p *AWSProvider) isDBClusterReady(ctx context.Context, clusterName string) func (p *AWSProvider) markClusterAsInactive(ctx context.Context, name string) (bool, error) { dbCluster := &crossplaneaws.DBCluster{} - err := p.k8sClient.Get(ctx, client.ObjectKey{ + err := p.Client.Get(ctx, client.ObjectKey{ Name: name, }, dbCluster) if err != nil { @@ -807,7 +808,7 @@ func (p *AWSProvider) markClusterAsInactive(ctx context.Context, name string) (b patchDBInstance := client.MergeFrom(dbCluster.DeepCopy()) dbCluster.Spec.ForProvider.Tags = changeToInactive(dbCluster.Spec.ForProvider.Tags) - err = p.k8sClient.Patch(ctx, dbCluster, patchDBInstance) + err = p.Client.Patch(ctx, dbCluster, patchDBInstance) if err != nil { return false, err } @@ -816,7 +817,7 @@ func (p *AWSProvider) markClusterAsInactive(ctx context.Context, name string) (b func (p *AWSProvider) markInstanceAsInactive(ctx context.Context, name string) (bool, error) { dbInstance := &crossplaneaws.DBInstance{} - err := p.k8sClient.Get(ctx, client.ObjectKey{ + err := p.Client.Get(ctx, client.ObjectKey{ Name: name, }, dbInstance) if err != nil { @@ -830,7 +831,7 @@ func (p *AWSProvider) markInstanceAsInactive(ctx context.Context, name string) ( patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) dbInstance.Spec.ForProvider.Tags = changeToInactive(dbInstance.Spec.ForProvider.Tags) - err = p.k8sClient.Patch(ctx, dbInstance, patchDBInstance) + err = p.Client.Patch(ctx, dbInstance, patchDBInstance) if err != nil { return false, err } diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index 332752b..a738d8f 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -91,7 +91,7 @@ var _ = Describe("AWSProvider create Postgres database", func() { SnapshotID: nil, } provider = &AWSProvider{ - k8sClient: k8sClient, + Client: k8sClient, config: controllerConfig, serviceNS: "db-controller", } @@ -480,7 +480,7 @@ func TestIsDBInstanceReady(t *testing.T) { } provider := &AWSProvider{ - k8sClient: fakeClient, + Client: fakeClient, } ready, err := provider.isDBInstanceReady(ctx, test.instanceName) @@ -568,7 +568,7 @@ func TestIsDBClusterReady(t *testing.T) { } provider := &AWSProvider{ - k8sClient: fakeClient, + Client: fakeClient, } ready, err := provider.isDBClusterReady(ctx, test.clusterName) From f1e6baffb78c64df5e133937878a652cba1c0b0d Mon Sep 17 00:00:00 2001 From: bfabricio Date: Sat, 15 Mar 2025 20:46:48 -0300 Subject: [PATCH 13/22] feat: add ensureResource to reduce code complexity --- pkg/providers/crossplane_aws.go | 150 +++++------- pkg/providers/crossplane_aws_test.go | 336 +++++++++++++++++++++++++++ pkg/providers/utils.go | 19 ++ 3 files changed, 407 insertions(+), 98 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index f6dc2cb..17e620a 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -146,41 +146,25 @@ func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSp } func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) error { - logger := log.FromContext(ctx) - - // Handle database parameter group - paramGroupName := getParameterGroupName(params) dbParamGroup := &crossplaneaws.DBParameterGroup{} - - err := p.Client.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup) - if errors.IsNotFound(err) { - logger.Info("Creating Postgres Instance parameter group", "paramGroupName", paramGroupName) - if createErr := p.Client.Create(ctx, p.postgresDBParameterGroup(params)); createErr != nil { - return fmt.Errorf("failed to create DB parameter group: %w", createErr) - } - } else if err != nil { - return fmt.Errorf("failed to get DB parameter group: %w", err) + paramGroupKey := client.ObjectKey{Name: getParameterGroupName(params)} + if err := ensureResource(ctx, p.Client, paramGroupKey, dbParamGroup, func() (*crossplaneaws.DBParameterGroup, error) { + return p.postgresDBParameterGroup(params), nil + }); err != nil { + return err } - // Handle database instance dbInstance := &crossplaneaws.DBInstance{} instanceKey := client.ObjectKey{Name: params.ResourceName} - - err = p.Client.Get(ctx, instanceKey, dbInstance) - if errors.IsNotFound(err) { + if err := ensureResource(ctx, p.Client, instanceKey, dbInstance, func() (*crossplaneaws.DBInstance, error) { dbInstance = p.postgresDBInstance(params) - - // Create master password secret before creating the DB instance - if passwordErr := ManageMasterPassword(ctx, dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef, p.Client); passwordErr != nil { - return fmt.Errorf("failed to manage master password for DB instance %s: %w", params.ResourceName, passwordErr) + secretRef := dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef + if err := ManageMasterPassword(ctx, secretRef, p.Client); err != nil { + return nil, fmt.Errorf("failed to create master password %v", err) } - - logger.Info("Creating Postgres db instance", "dbInstance", params.ResourceName) - if createErr := p.Client.Create(ctx, dbInstance); createErr != nil { - return fmt.Errorf("failed to create DB instance %s: %w", params.ResourceName, createErr) - } - } else if err != nil { - return fmt.Errorf("failed to get DB instance %s: %w", params.ResourceName, err) + return dbInstance, nil + }); err != nil { + return err } if !dbInstance.ObjectMeta.DeletionTimestamp.IsZero() { @@ -195,74 +179,50 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e } func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) error { - logger := log.FromContext(ctx) - // Handle database instance parameter group - paramGroupName := getParameterGroupName(params) + paramGroupKey := client.ObjectKey{Name: getParameterGroupName(params)} dbParamGroup := &crossplaneaws.DBParameterGroup{} - if err := p.Client.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbParamGroup); err != nil { - if errors.IsNotFound(err) { - logger.Info("Creating Aurora DB Instance parameter group", "paramGroupName", paramGroupName) - if err := p.Client.Create(ctx, p.auroraInstanceParamGroup(params)); err != nil { - return fmt.Errorf("failed to create DB parameter group: %w", err) - } - } else { - return fmt.Errorf("failed to get DB parameter group: %w", err) - } + err := ensureResource(ctx, p.Client, paramGroupKey, dbParamGroup, func() (*crossplaneaws.DBParameterGroup, error) { + return p.auroraInstanceParamGroup(params), nil + }) + if err != nil { + return err } - // Handle database cluster parameter group dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} - if err := p.Client.Get(ctx, client.ObjectKey{Name: paramGroupName}, dbClusterParamGroup); err != nil { - if errors.IsNotFound(err) { - logger.Info("Creating Aurora DB Cluster parameter group", "paramGroupName", paramGroupName) - if err := p.Client.Create(ctx, p.auroraClusterParamGroup(params)); err != nil { - return fmt.Errorf("failed to create DB parameter group: %w", err) - } - } else { - return fmt.Errorf("failed to get DB parameter group: %w", err) - } + err = ensureResource(ctx, p.Client, paramGroupKey, dbClusterParamGroup, func() (*crossplaneaws.DBClusterParameterGroup, error) { + return p.auroraClusterParamGroup(params), nil + }) + if err != nil { + return err } - // Handle aurora database cluster creation dbCluster := &crossplaneaws.DBCluster{} clusterKey := client.ObjectKey{Name: params.ResourceName} - if err := p.Client.Get(ctx, clusterKey, dbCluster); err != nil { - if errors.IsNotFound(err) { - dbCluster = p.auroraDBCluster(params) - // Create master password secret before creating the DB instance - passwordErr := ManageMasterPassword(ctx, dbCluster.Spec.ForProvider.CustomDBClusterParameters.MasterUserPasswordSecretRef, p.Client) - if passwordErr != nil { - return fmt.Errorf("failed to manage master password: %w", passwordErr) - } - logger.Info("Creating Aurora DB Cluster", "name", params.ResourceName) - if err := p.Client.Create(ctx, dbCluster); err != nil { - return fmt.Errorf("failed to create DB instance: %w", err) - } - } else { - return fmt.Errorf("failed to get DB instance: %w", err) + err = ensureResource(ctx, p.Client, clusterKey, dbCluster, func() (*crossplaneaws.DBCluster, error) { + dbCluster = p.auroraDBCluster(params) + newSecret := dbCluster.Spec.ForProvider.CustomDBClusterParameters.MasterUserPasswordSecretRef + if err := ManageMasterPassword(ctx, newSecret, p.Client); err != nil { + return nil, fmt.Errorf("failed to create master password %v", err) } - } + return dbCluster, nil + }) // Handle primary database instance primaryDbInstance := &crossplaneaws.DBInstance{} primaryInstanceKey := client.ObjectKey{Name: params.ResourceName} - if err := p.Client.Get(ctx, primaryInstanceKey, primaryDbInstance); err != nil { - if errors.IsNotFound(err) { - primaryDbInstance = p.auroraDBInstance(params, false) - logger.Info("Creating Aurora DB Instance", "name", params.ResourceName) - if err := p.Client.Create(ctx, primaryDbInstance); err != nil { - return fmt.Errorf("failed to create DB instance: %w", err) - } - } else { - return fmt.Errorf("failed to get DB instance: %w", err) - } + err = ensureResource(ctx, p.Client, primaryInstanceKey, primaryDbInstance, func() (*crossplaneaws.DBInstance, error) { + primaryDbInstance = p.postgresDBInstance(params) + return primaryDbInstance, nil + }) + if err != nil { + return err } if !primaryDbInstance.ObjectMeta.DeletionTimestamp.IsZero() || !dbCluster.ObjectMeta.DeletionTimestamp.IsZero() { return fmt.Errorf("can not create Cloud DB instance %s it is being deleted", params.ResourceName) } - err := p.updateAuroraDBCluster(ctx, params, dbCluster) + err = p.updateAuroraDBCluster(ctx, params, dbCluster) if err != nil { return fmt.Errorf("failed to update DB cluster: %v", err) } @@ -276,16 +236,12 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e if basefun.GetMultiAZEnabled(p.config) { secondaryDbInstance := &crossplaneaws.DBInstance{} secondaryInstanceKey := client.ObjectKey{Name: params.ResourceName + "-2"} - if err := p.Client.Get(ctx, secondaryInstanceKey, secondaryDbInstance); err != nil { - if errors.IsNotFound(err) { - secondaryDbInstance = p.auroraDBInstance(params, true) - logger.Info("Creating Aurora DB Secondary Instance", "name", params.ResourceName+"-2") - if err := p.Client.Create(ctx, secondaryDbInstance); err != nil { - return fmt.Errorf("failed to create DB instance: %w", err) - } - } else { - return fmt.Errorf("failed to get DB instance: %w", err) - } + err = ensureResource(ctx, p.Client, secondaryInstanceKey, secondaryDbInstance, func() (*crossplaneaws.DBInstance, error) { + secondaryDbInstance = p.postgresDBInstance(params) + return secondaryDbInstance, nil + }) + if err != nil { + return err } if !secondaryDbInstance.ObjectMeta.DeletionTimestamp.IsZero() { @@ -868,21 +824,19 @@ func changeToInactive(tags []*crossplaneaws.Tag) []*crossplaneaws.Tag { // addInactiveOperationalTag marks the instance with operational-status: inactive tag, // it returns ErrTagNotPropagated if the tag is not yet propagated to the cloud resource func (p *AWSProvider) addInactiveOperationalTag(ctx context.Context, spec DatabaseSpec) (bool, error) { - for _, tag := range spec.Tags { - if tag.Key == OperationalTAGInactive.Key || tag.Value == OperationalTAGInactive.Value { - return true, nil - } + instanceTagged, err := p.markInstanceAsInactive(ctx, spec.ResourceName) + if err != nil { + return false, err } - if tagged, err := p.markInstanceAsInactive(ctx, spec.ResourceName); err != nil || !tagged { - return false, err + if spec.DbType != AwsAuroraPostgres { + return instanceTagged, nil } - if spec.DbType == AwsAuroraPostgres { - if tagged, err := p.markClusterAsInactive(ctx, spec.ResourceName); err != nil || !tagged { - return false, err - } + clusterTagged, err := p.markClusterAsInactive(ctx, spec.ResourceName) + if err != nil { + return false, err } - return true, nil + return instanceTagged && clusterTagged, nil } diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index a738d8f..05c6b45 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -402,6 +402,342 @@ var _ = Describe("AWSProvider create Postgres database", func() { }) }) +var _ = Describe("AWSProvider create Aurora database", func() { + var ( + provider *AWSProvider + ctx context.Context + spec DatabaseSpec + ) + + BeforeEach(func() { + ctx = context.TODO() + spec = DatabaseSpec{ + ResourceName: "env-app-name-db-1d9fb876", + DatabaseName: "app-name-db", + MinStorageGB: 10, + MaxStorageGB: 20, + DBVersion: "15.7", + SkipFinalSnapshotBeforeDeletion: true, + MasterUsername: "root", + EnableIAMDatabaseAuthentication: true, + StorageType: "storage-type", + DeletionPolicy: xpv1.DeletionOrphan, + PubliclyAccessible: true, + InstanceClass: "db.t3.medium", + DbType: "aurora-postgresql", + EnablePerfInsight: true, + EnableCloudwatchLogsExport: []*string{ptr.To("postgresql"), ptr.To("upgrade")}, + BackupRetentionDays: 7, + CACertificateIdentifier: ptr.To("rds-ca-2019"), + Tags: []ProviderTag{ + {Key: "environment", Value: "test"}, + {Key: "managed-by", Value: "controller-test"}, + }, + Labels: map[string]string{ + "app": "test-app", + "environment": "test", + "team": "database", + }, + PreferredMaintenanceWindow: ptr.To("sun:02:00-sun:03:00"), + BackupPolicy: "daily", + SnapshotID: nil, + } + provider = &AWSProvider{ + Client: k8sClient, + config: controllerConfig, + serviceNS: "db-controller", + } + }) + + AfterEach(func() { + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBInstance{})).To(Succeed()) + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBParameterGroup{})).To(Succeed()) + Expect(k8sClient.DeleteAllOf(ctx, &crossplaneaws.DBCluster{})).To(Succeed()) + }) + + Describe("create aurora database", func() { + When("create function is called with correct parameters", func() { + It("should properly create crossplane resources", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + // Cluster Parameter group + clusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876-a-15"}, clusterParamGroup) + }).Should(Succeed()) + + // Instance parameter group + paramGroup := &crossplaneaws.DBParameterGroup{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876-a-15"}, paramGroup) + }).Should(Succeed()) + + // DB Cluster + dbCluster := &crossplaneaws.DBCluster{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbCluster) + }).Should(Succeed()) + + // Instance + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + }).Should(Succeed()) + + // Master secret + masterSecret := &corev1.Secret{} + Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: "env-app-name-db-1d9fb876-master", Namespace: "db-controller"}, masterSecret) + }).Should(Succeed()) + + }) + + It("should creates master password secret for the new database instance", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + // Define the expected secret key + secretKey := types.NamespacedName{ + Name: spec.ResourceName + MasterPasswordSuffix, + Namespace: provider.serviceNS, + } + + // Validate that the secret is created + masterSecret := &corev1.Secret{} + Eventually(func() error { + return k8sClient.Get(ctx, secretKey, masterSecret) + }).Should(Succeed()) + + // Validate the secret contains the expected key + Expect(masterSecret.Data).To(HaveKey(MasterPasswordSecretKey)) + Expect(masterSecret.Data[MasterPasswordSecretKey]).ToNot(BeEmpty()) + }) + + //It("should return true when database is already provisioned", func() { + // isReady, err := provider.CreateDatabase(ctx, spec) + // Expect(err).ToNot(HaveOccurred()) + // Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned + // + // dbInstance := &crossplaneaws.DBInstance{} + // Eventually(func() error { + // return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + // }).Should(Succeed()) + // + // updatedInstance := dbInstance.DeepCopy() + // updatedInstance.Status.Conditions = []xpv1.Condition{ + // { + // Type: xpv1.TypeReady, + // Status: corev1.ConditionTrue, + // LastTransitionTime: metav1.Now(), + // }, + // } + // + // // Manually trigger an Update (not Status().Update()) + // Expect(k8sClient.Update(ctx, updatedInstance)).To(Succeed()) + // + // // Call CreateDatabase again and check if it returns true + // isReady, err = provider.CreateDatabase(ctx, spec) + // Expect(err).ToNot(HaveOccurred()) + // Expect(isReady).To(BeTrue()) // Now, the database should be marked as ready + //}) + + It("should it propagates the spec provider tags to database instance, add operational active tag, and add backup tag", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + }).Should(Succeed()) + + expectedTags := spec.Tags + expectedTags = append(expectedTags, OperationalTAGActive) + expectedTags = append(expectedTags, ProviderTag{Key: BackupPolicyKey, Value: spec.BackupPolicy}) + // Validate Tags + Expect(dbInstance.Spec.ForProvider.Tags).To(ConsistOf(ConvertFromProviderTags(expectedTags, func(tag ProviderTag) *crossplaneaws.Tag { + return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} + }))) + }) + + It("should configures RestoreDBInstanceBackupConfiguration when snapshot id is passed", func() { + newSpec := spec + newSpec.SnapshotID = ptr.To("snapshot-id") + + _, err := provider.CreateDatabase(ctx, newSpec) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + }).Should(Succeed()) + + Expect(dbInstance.Spec.ForProvider.RestoreFrom).ToNot(BeNil()) + Expect(dbInstance.Spec.ForProvider.RestoreFrom.Snapshot.SnapshotIdentifier).To(Equal(newSpec.SnapshotID)) + Expect(dbInstance.Spec.ForProvider.RestoreFrom.Source).To(Equal(ptr.To(SnapshotSource))) + }) + + It("should propagate passed parameter provider labels to database instance", func() { + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + }).Should(Succeed()) + + Expect(dbInstance.Labels).To(Equal(spec.Labels)) + }) + }) + }) + + Describe("update aurora database", func() { + When("when crossplane cr preexists the creation call we need to update with provided parameters", func() { + It("should propagate the new spec fields to crossplane CR", func() { + isReady, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned + + dbInstance := &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + }).Should(Succeed()) + + updatedSpec := spec + updatedSpec.MaxStorageGB = 30 + updatedSpec.MinStorageGB = 20 + updatedSpec.EnableCloudwatchLogsExport = []*string{ptr.To("not"), ptr.To("upgrade")} + updatedSpec.EnablePerfInsight = false + updatedSpec.DeletionPolicy = xpv1.DeletionDelete + + isReady, err = provider.CreateDatabase(ctx, updatedSpec) + Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned + + dbInstance = &crossplaneaws.DBInstance{} + Eventually(func() error { + return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) + }).Should(Succeed()) + + Expect(dbInstance.Spec.ForProvider.EnablePerformanceInsights).To(Equal(ptr.To(updatedSpec.EnablePerfInsight))) + Expect(dbInstance.Spec.ResourceSpec.DeletionPolicy).To(Equal(updatedSpec.DeletionPolicy)) + Expect(dbInstance.Spec.ForProvider.Engine).To(Equal(ptr.To(updatedSpec.DbType))) + Expect(dbInstance.Spec.ForProvider.EngineVersion).To(Equal(GetEngineVersion(updatedSpec, provider.config))) + Expect(dbInstance.Spec.ForProvider.DBInstanceClass).To(Equal(ptr.To(updatedSpec.InstanceClass))) + Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-a-15")) + Expect(dbInstance.Spec.ForProvider.CACertificateIdentifier).To(Equal(updatedSpec.CACertificateIdentifier)) + Expect(dbInstance.Spec.ForProvider.MultiAZ).To(Equal(ptr.To(basefun.GetMultiAZEnabled(provider.config)))) + Expect(dbInstance.Spec.ForProvider.MasterUsername).To(Equal(ptr.To(updatedSpec.MasterUsername))) + Expect(dbInstance.Spec.ForProvider.PubliclyAccessible).To(Equal(ptr.To(updatedSpec.PubliclyAccessible))) + Expect(dbInstance.Spec.ForProvider.EnableIAMDatabaseAuthentication).To(Equal(ptr.To(updatedSpec.EnableIAMDatabaseAuthentication))) + Expect(dbInstance.Spec.ForProvider.BackupRetentionPeriod).To(Equal(ptr.To(updatedSpec.BackupRetentionDays))) + Expect(dbInstance.Spec.ForProvider.StorageEncrypted).To(Equal(ptr.To(true))) + Expect(dbInstance.Spec.ForProvider.StorageType).To(Equal(ptr.To(updatedSpec.StorageType))) + Expect(dbInstance.Spec.ForProvider.Port).To(Equal(ptr.To(updatedSpec.Port))) + Expect(dbInstance.Spec.ForProvider.PreferredMaintenanceWindow).To(Equal(updatedSpec.PreferredMaintenanceWindow)) + Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-a-15")) + + // master password + Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Name).To(Equal(updatedSpec.ResourceName + MasterPasswordSuffix)) + Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Namespace).To(Equal(provider.serviceNS)) + Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Name).To(Equal(updatedSpec.ResourceName)) + Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Namespace).To(Equal(provider.serviceNS)) + + // Validate VPC & Security Groups + Expect(dbInstance.Spec.ForProvider.VPCSecurityGroupIDRefs).To(ContainElement(xpv1.Reference{ + Name: basefun.GetVpcSecurityGroupIDRefs(provider.config), + })) + Expect(dbInstance.Spec.ForProvider.DBSubnetGroupNameRef.Name).To(Equal(basefun.GetDbSubnetGroupNameRef(provider.config))) + + // Validate Provider Config & Deletion Policy + Expect(dbInstance.Spec.ResourceSpec.ProviderConfigReference.Name).To(Equal(basefun.GetProviderConfig(provider.config))) + }) + }) + }) + + Describe("delete database interface usage", func() { + When("delete func is called with correct spec and no operational tagging", func() { + It("should delete postgres the database right away without adding operational tagging", func() { + By("creating a new database") + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + deleted, err := provider.DeleteDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + Expect(deleted).To(BeTrue()) + + paramGroup := &crossplaneaws.DBParameterGroup{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876-15"}, paramGroup) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + + dbInstance := &crossplaneaws.DBInstance{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + }) + }) + + When("delete func is called with correct spec and instructed to add inactive operational tagging", func() { + It("should delete postgres database with operational tagging only when tag is propagated to aws", func() { + By("creating a new database") + _, err := provider.CreateDatabase(ctx, spec) + Expect(err).ToNot(HaveOccurred()) + + deleteSpec := spec + deleteSpec.TagInactive = true + deleted, err := provider.DeleteDatabase(ctx, deleteSpec) + Expect(err).ToNot(HaveOccurred()) + Expect(deleted).To(BeFalse()) + + paramGroup := &crossplaneaws.DBParameterGroup{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876-a-15"}, paramGroup) + Expect(err).ToNot(HaveOccurred()) + + dbInstance := &crossplaneaws.DBInstance{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + Expect(err).ToNot(HaveOccurred()) + + Expect(dbInstance.Spec.ForProvider.Tags).To(ContainElement(&crossplaneaws.Tag{ + Key: ptr.To("operational-status"), + Value: ptr.To("inactive"), + })) + + By("simulating the operational tagging propagated to aws") + patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) + dbInstance.Status.AtProvider.TagList = append( + dbInstance.Status.AtProvider.TagList, + &crossplaneaws.Tag{Key: &OperationalTAGInactive.Key, Value: &OperationalTAGInactive.Value}) + + err = k8sClient.Patch(ctx, dbInstance, patchDBInstance) + Expect(err).ToNot(HaveOccurred()) + + // Cluster tags + cluster := &crossplaneaws.DBCluster{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, cluster) + Expect(err).ToNot(HaveOccurred()) + + Expect(cluster.Spec.ForProvider.Tags).To(ContainElement(&crossplaneaws.Tag{ + Key: ptr.To("operational-status"), + Value: ptr.To("inactive"), + })) + + By("simulating the operational tagging propagated to aws") + patchDBcluster := client.MergeFrom(cluster.DeepCopy()) + cluster.Status.AtProvider.TagList = append( + cluster.Status.AtProvider.TagList, + &crossplaneaws.Tag{Key: &OperationalTAGInactive.Key, Value: &OperationalTAGInactive.Value}) + + err = k8sClient.Patch(ctx, cluster, patchDBcluster) + Expect(err).ToNot(HaveOccurred()) + + deleted, err = provider.DeleteDatabase(ctx, deleteSpec) + Expect(err).ToNot(HaveOccurred()) + Expect(deleted).To(BeTrue()) + }) + }) + }) +}) + func createFakeClientWithObjects(objects ...k8sRuntime.Object) client.Client { sch := k8sRuntime.NewScheme() _ = scheme.AddToScheme(sch) diff --git a/pkg/providers/utils.go b/pkg/providers/utils.go index 07c5505..a426577 100644 --- a/pkg/providers/utils.go +++ b/pkg/providers/utils.go @@ -93,3 +93,22 @@ func isReady(cond []xpv1.Condition) (bool, error) { } return false, nil } + +// ensureResource ensures that a given Kubernetes resource exists in the cluster. +// If the resource does not exist, it creates a new instance using the provided createFunc. +func ensureResource[T client.Object](ctx context.Context, k8sClient client.Client, key client.ObjectKey, resource T, createFunc func() (T, error)) error { + if err := k8sClient.Get(ctx, key, resource); err != nil { + if errors.IsNotFound(err) { + newResource, createErr := createFunc() + if createErr != nil { + return createErr + } + if err := k8sClient.Create(ctx, newResource); err != nil { + return fmt.Errorf("failed to create resource: %w", err) + } + } else { + return fmt.Errorf("failed to get resource: %w", err) + } + } + return nil +} From 57dc3872650842e087be74de470b845d970961e3 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Tue, 18 Mar 2025 08:17:50 -0300 Subject: [PATCH 14/22] refactor: add feature toggle --- helm/db-controller/minikube.yaml | 1 + helm/db-controller/values.yaml | 1 + pkg/basefunctions/basefunctions.go | 4 ++ pkg/databaseclaim/databaseclaim.go | 93 +++++++++++++++++++++++++----- pkg/providers/crossplane_aws.go | 2 +- 5 files changed, 84 insertions(+), 17 deletions(-) diff --git a/helm/db-controller/minikube.yaml b/helm/db-controller/minikube.yaml index 9f31ab1..a5599a7 100644 --- a/helm/db-controller/minikube.yaml +++ b/helm/db-controller/minikube.yaml @@ -1,4 +1,5 @@ controllerConfig: + newProvider: true #AWS us-east-1 GCP us-east1 region: us-east-1 vpcSecurityGroupIDRefs: box-3 diff --git a/helm/db-controller/values.yaml b/helm/db-controller/values.yaml index 2f5e2f8..a71771a 100644 --- a/helm/db-controller/values.yaml +++ b/helm/db-controller/values.yaml @@ -134,6 +134,7 @@ zapLogger: timeEncoding: rfc3339nano controllerConfig: + newProvider: true athena-shared: masterUsername: root authSource: secret diff --git a/pkg/basefunctions/basefunctions.go b/pkg/basefunctions/basefunctions.go index 9e1e2a2..a1b3eb9 100644 --- a/pkg/basefunctions/basefunctions.go +++ b/pkg/basefunctions/basefunctions.go @@ -259,3 +259,7 @@ func GetDynamicHostWaitTime(viperConfig *viper.Viper) time.Duration { func GetDBIdentifierPrefix(viperConfig *viper.Viper) string { return viperConfig.GetString("env") } + +func IsProviderEnable(viperConfig *viper.Viper) bool { + return viperConfig.GetBool("newProvider") +} diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index fbd2fd8..5d6257e 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -3,17 +3,17 @@ package databaseclaim import ( "context" "fmt" - crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" "github.com/infobloxopen/db-controller/pkg/providers" - crossplanegcp "github.com/upbound/provider-gcp/apis/alloydb/v1beta2" "strings" "time" + crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/go-logr/logr" _ "github.com/lib/pq" gopassword "github.com/sethvargo/go-password/password" "github.com/spf13/viper" + crossplanegcp "github.com/upbound/provider-gcp/apis/alloydb/v1beta2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -190,10 +190,14 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques //ignore delete request, continue to process rds migration return r.executeDbClaimRequest(ctx, &reqInfo, &dbClaim) } - if basefun.GetCloud(r.Config.Viper) == "aws" { - // our finalizer is present, so lets handle any external dependency + if basefun.IsProviderEnable(r.Config.Viper) { spec := NewDatabaseSpecFromRequestInfo(&reqInfo, &dbClaim, r.getMode(ctx, &reqInfo, &dbClaim), r.Config.Viper) if _, err := r.provider.DeleteDatabase(ctx, spec); err != nil { + return ctrl.Result{}, err + } + } else if basefun.GetCloud(r.Config.Viper) == "aws" { + // our finalizer is present, so lets handle any external dependency + if err := r.deleteExternalResourcesAWS(ctx, &reqInfo, &dbClaim); err != nil { // if fail to delete the external dependency here, return with error // so that it can be retried return ctrl.Result{}, err @@ -254,23 +258,78 @@ func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, d logr := log.FromContext(ctx).WithValues("databaseclaim", dbClaim.Namespace+"/"+dbClaim.Name) logr.Info("post migration is in progress") - dbInstanceName := strings.Split(dbClaim.Status.OldDB.ConnectionInfo.Host, ".")[0] + if basefun.IsProviderEnable(r.Config.Viper) { + dbInstanceName := strings.Split(dbClaim.Status.OldDB.ConnectionInfo.Host, ".")[0] + deleted, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName}) + if err != nil { + return ctrl.Result{}, err + } - deleted, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName}) - if err != nil { + if time.Since(dbClaim.Status.OldDB.PostMigrationActionStartedAt.Time).Minutes() > 10 { + _, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName, TagInactive: false}) + if err != nil { + return ctrl.Result{}, err + } + dbClaim.Status.OldDB = v1.StatusForOldDB{} + } + + if !deleted { + return ctrl.Result{RequeueAfter: time.Minute}, nil + } + + if err := r.statusManager.ClearError(ctx, dbClaim); err != nil { + logr.Error(err, "Error updating DatabaseClaim status") + return ctrl.Result{}, err + } + + if !dbClaim.ObjectMeta.DeletionTimestamp.IsZero() { + return ctrl.Result{Requeue: true}, nil + } return ctrl.Result{}, err } - if time.Since(dbClaim.Status.OldDB.PostMigrationActionStartedAt.Time).Minutes() > 10 { - _, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName, TagInactive: false}) + // TODO: after provider implementation is validated, below code can be deprecated + // get name of DBInstance from connectionInfo + dbInstanceName := strings.Split(dbClaim.Status.OldDB.ConnectionInfo.Host, ".")[0] + + var dbParamGroupName string + // get name of DBParamGroup from connectionInfo + if dbClaim.Status.OldDB.Type == v1.AuroraPostgres { + dbParamGroupName = dbInstanceName + "-a-" + (strings.Split(dbClaim.Status.OldDB.DBVersion, "."))[0] + } else { + dbParamGroupName = dbInstanceName + "-" + (strings.Split(dbClaim.Status.OldDB.DBVersion, "."))[0] + } + + TagsVerified, err := r.manageOperationalTagging(ctx, logr, dbInstanceName, dbParamGroupName) + + // Even though we get error in updating tags, we log the error + // and go ahead with deleting resources + if err != nil || TagsVerified { + if err != nil { - return ctrl.Result{}, err + logr.Error(err, "Failed updating or verifying operational tags") } + + if err = r.deleteCloudDatabaseAWS(dbInstanceName, ctx); err != nil { + logr.Error(err, "Could not delete crossplane DBInstance/DBCLluster") + } + if err = r.deleteParameterGroupAWS(ctx, dbParamGroupName); err != nil { + logr.Error(err, "Could not delete crossplane DBParamGroup/DBClusterParamGroup") + } + dbClaim.Status.OldDB = v1.StatusForOldDB{} - } + } else if time.Since(dbClaim.Status.OldDB.PostMigrationActionStartedAt.Time).Minutes() > 10 { + // Lets keep the state of old as it is for defined time to wait and verify tags before actually deleting resources + logr.Info("defined wait time is over to verify operational tags on AWS resources. Moving ahead to delete associated crossplane resources anyway") - if !deleted { - return ctrl.Result{RequeueAfter: time.Minute}, nil + if err = r.deleteCloudDatabaseAWS(dbInstanceName, ctx); err != nil { + logr.Error(err, "Could not delete crossplane DBInstance/DBCLluster") + } + if err = r.deleteParameterGroupAWS(ctx, dbParamGroupName); err != nil { + logr.Error(err, "Could not delete crossplane DBParamGroup/DBClusterParamGroup") + } + + dbClaim.Status.OldDB = v1.StatusForOldDB{} } if err := r.statusManager.ClearError(ctx, dbClaim); err != nil { @@ -281,8 +340,7 @@ func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, d if !dbClaim.ObjectMeta.DeletionTimestamp.IsZero() { return ctrl.Result{Requeue: true}, nil } - - return ctrl.Result{}, err + return ctrl.Result{RequeueAfter: time.Minute}, nil } // Create, migrate or upgrade database @@ -500,9 +558,12 @@ func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, reqInfo *r isReady := false var err error - if cloud == "aws" { + // TODO: Once the providers implementation is ready, we could completely remove this if cloud condition + if basefun.IsProviderEnable(r.Config.Viper) { spec := NewDatabaseSpecFromRequestInfo(reqInfo, dbClaim, operationalMode, r.Config.Viper) isReady, err = r.provider.CreateDatabase(ctx, spec) + } else if cloud == "aws" { + isReady, err = r.manageCloudHostAWS(ctx, reqInfo, dbClaim, operationalMode) if err != nil { logr.Error(err, "manage_cloud_host_AWS") return ctrl.Result{}, err diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 17e620a..e6d1366 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -542,7 +542,7 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus }, EngineVersion: GetEngineVersion(params, p.config), }, - Engine: ¶ms.DBVersion, + Engine: ¶ms.DbType, Tags: ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} }), From a9b97c5993c4f2ab1abb8a7161932e93c540f9c7 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Tue, 18 Mar 2025 10:13:56 -0300 Subject: [PATCH 15/22] fix: call to aurora instance creation --- pkg/providers/crossplane_aws.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index e6d1366..5ffce10 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -211,7 +211,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e primaryDbInstance := &crossplaneaws.DBInstance{} primaryInstanceKey := client.ObjectKey{Name: params.ResourceName} err = ensureResource(ctx, p.Client, primaryInstanceKey, primaryDbInstance, func() (*crossplaneaws.DBInstance, error) { - primaryDbInstance = p.postgresDBInstance(params) + primaryDbInstance = p.auroraDBInstance(params, false) return primaryDbInstance, nil }) if err != nil { @@ -237,7 +237,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e secondaryDbInstance := &crossplaneaws.DBInstance{} secondaryInstanceKey := client.ObjectKey{Name: params.ResourceName + "-2"} err = ensureResource(ctx, p.Client, secondaryInstanceKey, secondaryDbInstance, func() (*crossplaneaws.DBInstance, error) { - secondaryDbInstance = p.postgresDBInstance(params) + secondaryDbInstance = p.auroraDBInstance(params, true) return secondaryDbInstance, nil }) if err != nil { From 4cb32cb1a38c91af28be3f2b5470885f99664f2c Mon Sep 17 00:00:00 2001 From: bfabricio Date: Tue, 18 Mar 2025 12:50:52 -0300 Subject: [PATCH 16/22] fix: aurora instance unit test --- pkg/providers/crossplane_aws_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index 05c6b45..f80aece 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -1292,7 +1292,8 @@ func TestAuroraDBCluster(t *testing.T) { name: "Creates Aurora DB Cluster with backup retention days", params: DatabaseSpec{ ResourceName: "test-db-cluster", - DBVersion: "aurora-mysql", + DBVersion: "15", + DbType: "aurora-postgresql", BackupRetentionDays: 7, SkipFinalSnapshotBeforeDeletion: true, MasterUsername: "admin", @@ -1333,7 +1334,7 @@ func TestAuroraDBCluster(t *testing.T) { }, EngineVersion: ptr.To("aurora-postgresql"), }, - Engine: ptr.To("aurora-mysql"), + Engine: ptr.To("aurora-postgresql"), Tags: []*crossplaneaws.Tag{ {Key: ptr.To("Environment"), Value: ptr.To("Test")}, }, @@ -1363,7 +1364,8 @@ func TestAuroraDBCluster(t *testing.T) { name: "Creates Aurora DB Cluster without backup retention days", params: DatabaseSpec{ ResourceName: "prod-db-cluster", - DBVersion: "aurora-postgresql", + DBVersion: "15", + DbType: "aurora-postgresql", BackupRetentionDays: 0, SkipFinalSnapshotBeforeDeletion: false, MasterUsername: "postgres", From 8b6995b1d89a2086c5c990ef306ddda5bc59b393 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Tue, 18 Mar 2025 13:22:02 -0300 Subject: [PATCH 17/22] fix: missing restore from on aurora cluster --- pkg/providers/crossplane_aws.go | 11 +++++ pkg/providers/crossplane_aws_test.go | 73 ++-------------------------- 2 files changed, 16 insertions(+), 68 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 5ffce10..61b5145 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -511,6 +511,16 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus auroraBackupRetentionPeriod = nil } + var restoreFrom *crossplaneaws.RestoreDBClusterBackupConfiguration + if params.SnapshotID != nil { + restoreFrom = &crossplaneaws.RestoreDBClusterBackupConfiguration{ + Snapshot: &crossplaneaws.SnapshotRestoreBackupConfiguration{ + SnapshotIdentifier: params.SnapshotID, + }, + Source: ptr.To(SnapshotSource), + } + } + p.configureDBTags(¶ms) return &crossplaneaws.DBCluster{ @@ -541,6 +551,7 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus Name: getParameterGroupName(params), }, EngineVersion: GetEngineVersion(params, p.config), + RestoreFrom: restoreFrom, }, Engine: ¶ms.DbType, Tags: ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index f80aece..49c2e8d 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -567,14 +567,14 @@ var _ = Describe("AWSProvider create Aurora database", func() { _, err := provider.CreateDatabase(ctx, newSpec) Expect(err).ToNot(HaveOccurred()) - dbInstance := &crossplaneaws.DBInstance{} + dbCluster := &crossplaneaws.DBCluster{} Eventually(func() error { - return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbInstance) + return k8sClient.Get(ctx, types.NamespacedName{Name: "env-app-name-db-1d9fb876"}, dbCluster) }).Should(Succeed()) - Expect(dbInstance.Spec.ForProvider.RestoreFrom).ToNot(BeNil()) - Expect(dbInstance.Spec.ForProvider.RestoreFrom.Snapshot.SnapshotIdentifier).To(Equal(newSpec.SnapshotID)) - Expect(dbInstance.Spec.ForProvider.RestoreFrom.Source).To(Equal(ptr.To(SnapshotSource))) + Expect(dbCluster.Spec.ForProvider.RestoreFrom).ToNot(BeNil()) + Expect(dbCluster.Spec.ForProvider.RestoreFrom.Snapshot.SnapshotIdentifier).To(Equal(newSpec.SnapshotID)) + Expect(dbCluster.Spec.ForProvider.RestoreFrom.Source).To(Equal(ptr.To(SnapshotSource))) }) It("should propagate passed parameter provider labels to database instance", func() { @@ -591,69 +591,6 @@ var _ = Describe("AWSProvider create Aurora database", func() { }) }) - Describe("update aurora database", func() { - When("when crossplane cr preexists the creation call we need to update with provided parameters", func() { - It("should propagate the new spec fields to crossplane CR", func() { - isReady, err := provider.CreateDatabase(ctx, spec) - Expect(err).ToNot(HaveOccurred()) - Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned - - dbInstance := &crossplaneaws.DBInstance{} - Eventually(func() error { - return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) - }).Should(Succeed()) - - updatedSpec := spec - updatedSpec.MaxStorageGB = 30 - updatedSpec.MinStorageGB = 20 - updatedSpec.EnableCloudwatchLogsExport = []*string{ptr.To("not"), ptr.To("upgrade")} - updatedSpec.EnablePerfInsight = false - updatedSpec.DeletionPolicy = xpv1.DeletionDelete - - isReady, err = provider.CreateDatabase(ctx, updatedSpec) - Expect(isReady).To(BeFalse()) // Initially, the database is not provisioned - - dbInstance = &crossplaneaws.DBInstance{} - Eventually(func() error { - return k8sClient.Get(ctx, types.NamespacedName{Name: spec.ResourceName}, dbInstance) - }).Should(Succeed()) - - Expect(dbInstance.Spec.ForProvider.EnablePerformanceInsights).To(Equal(ptr.To(updatedSpec.EnablePerfInsight))) - Expect(dbInstance.Spec.ResourceSpec.DeletionPolicy).To(Equal(updatedSpec.DeletionPolicy)) - Expect(dbInstance.Spec.ForProvider.Engine).To(Equal(ptr.To(updatedSpec.DbType))) - Expect(dbInstance.Spec.ForProvider.EngineVersion).To(Equal(GetEngineVersion(updatedSpec, provider.config))) - Expect(dbInstance.Spec.ForProvider.DBInstanceClass).To(Equal(ptr.To(updatedSpec.InstanceClass))) - Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-a-15")) - Expect(dbInstance.Spec.ForProvider.CACertificateIdentifier).To(Equal(updatedSpec.CACertificateIdentifier)) - Expect(dbInstance.Spec.ForProvider.MultiAZ).To(Equal(ptr.To(basefun.GetMultiAZEnabled(provider.config)))) - Expect(dbInstance.Spec.ForProvider.MasterUsername).To(Equal(ptr.To(updatedSpec.MasterUsername))) - Expect(dbInstance.Spec.ForProvider.PubliclyAccessible).To(Equal(ptr.To(updatedSpec.PubliclyAccessible))) - Expect(dbInstance.Spec.ForProvider.EnableIAMDatabaseAuthentication).To(Equal(ptr.To(updatedSpec.EnableIAMDatabaseAuthentication))) - Expect(dbInstance.Spec.ForProvider.BackupRetentionPeriod).To(Equal(ptr.To(updatedSpec.BackupRetentionDays))) - Expect(dbInstance.Spec.ForProvider.StorageEncrypted).To(Equal(ptr.To(true))) - Expect(dbInstance.Spec.ForProvider.StorageType).To(Equal(ptr.To(updatedSpec.StorageType))) - Expect(dbInstance.Spec.ForProvider.Port).To(Equal(ptr.To(updatedSpec.Port))) - Expect(dbInstance.Spec.ForProvider.PreferredMaintenanceWindow).To(Equal(updatedSpec.PreferredMaintenanceWindow)) - Expect(dbInstance.Spec.ForProvider.DBParameterGroupNameRef.Name).To(Equal("env-app-name-db-1d9fb876-a-15")) - - // master password - Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Name).To(Equal(updatedSpec.ResourceName + MasterPasswordSuffix)) - Expect(dbInstance.Spec.ForProvider.MasterUserPasswordSecretRef.SecretReference.Namespace).To(Equal(provider.serviceNS)) - Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Name).To(Equal(updatedSpec.ResourceName)) - Expect(dbInstance.Spec.ResourceSpec.WriteConnectionSecretToReference.Namespace).To(Equal(provider.serviceNS)) - - // Validate VPC & Security Groups - Expect(dbInstance.Spec.ForProvider.VPCSecurityGroupIDRefs).To(ContainElement(xpv1.Reference{ - Name: basefun.GetVpcSecurityGroupIDRefs(provider.config), - })) - Expect(dbInstance.Spec.ForProvider.DBSubnetGroupNameRef.Name).To(Equal(basefun.GetDbSubnetGroupNameRef(provider.config))) - - // Validate Provider Config & Deletion Policy - Expect(dbInstance.Spec.ResourceSpec.ProviderConfigReference.Name).To(Equal(basefun.GetProviderConfig(provider.config))) - }) - }) - }) - Describe("delete database interface usage", func() { When("delete func is called with correct spec and no operational tagging", func() { It("should delete postgres the database right away without adding operational tagging", func() { From a18808e1d0ba846ca3edf7661d437d43dd0ee6be Mon Sep 17 00:00:00 2001 From: bfabricio Date: Tue, 18 Mar 2025 13:25:10 -0300 Subject: [PATCH 18/22] test: add ensure resource test --- pkg/providers/utils_test.go | 120 ++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/pkg/providers/utils_test.go b/pkg/providers/utils_test.go index 0063cc3..8b4d572 100644 --- a/pkg/providers/utils_test.go +++ b/pkg/providers/utils_test.go @@ -1,7 +1,17 @@ package providers import ( + "context" + "fmt" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "testing" + + . "github.com/onsi/gomega" ) func TestGetParameterGroupName(t *testing.T) { @@ -44,3 +54,113 @@ func TestGetParameterGroupName(t *testing.T) { }) } } + +func TestEnsureResource(t *testing.T) { + RegisterTestingT(t) + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + + ctx := context.Background() + + t.Run("should create resource when it doesn't exist", func(t *testing.T) { + cl := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + key := client.ObjectKey{ + Name: "test-configmap", + Namespace: "default", + } + + createFunc := func() (*corev1.ConfigMap, error) { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: "default", + }, + Data: map[string]string{ + "test-key": "test-value", + }, + }, nil + } + + err := ensureResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) + Expect(err).To(BeNil()) + createdResource := &corev1.ConfigMap{} + err = cl.Get(ctx, key, createdResource) + Expect(err).To(BeNil()) + Expect(createdResource.Data["test-key"]).To(Equal("test-value")) + }) + + t.Run("should not create resource when it already exists", func(t *testing.T) { + existingResource := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-configmap", + Namespace: "default", + }, + Data: map[string]string{ + "existing-key": "existing-value", + }, + } + + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(existingResource). + Build() + + key := client.ObjectKey{ + Name: "existing-configmap", + Namespace: "default", + } + + createFunc := func() (*corev1.ConfigMap, error) { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-configmap", + Namespace: "default", + }, + Data: map[string]string{ + "should-not-be-used": "new-value", + }, + }, nil + } + err := ensureResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) + Expect(err).To(BeNil()) + existingAfter := &corev1.ConfigMap{} + err = cl.Get(ctx, key, existingAfter) + Expect(err).To(BeNil()) + Expect(existingAfter.Data).To(HaveKey("existing-key")) + Expect(existingAfter.Data).NotTo(HaveKey("should-not-be-used")) + }) + + t.Run("should return error when Create fails", func(t *testing.T) { + createInterceptor := func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + fmt.Println("Intercepting Create:", obj.GetName()) + return fmt.Errorf("test error") + } + + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Create: createInterceptor, + }).Build() + + key := client.ObjectKey{ + Name: "test-configmap", + Namespace: "default", + } + + createFunc := func() (*corev1.ConfigMap, error) { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: "default", + }, + }, nil + } + + err := ensureResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) + Expect(err).ToNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("failed to create resource")) + }) +} From 7173a3e6386e7aba8068232c57cf682dfab55a05 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Wed, 19 Mar 2025 07:56:54 -0300 Subject: [PATCH 19/22] fix: add update tag to postgres instance --- pkg/providers/crossplane_aws.go | 14 +++++++++----- pkg/providers/crossplane_aws_test.go | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 61b5145..09e0762 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -278,7 +278,7 @@ func (p *AWSProvider) postgresDBInstance(params DatabaseSpec) *crossplaneaws.DBI } } - p.configureDBTags(¶ms) + p.configureCrossplaneTags(¶ms) return &crossplaneaws.DBInstance{ ObjectMeta: metav1.ObjectMeta{ @@ -410,6 +410,10 @@ func (p *AWSProvider) postgresDBParameterGroup(params DatabaseSpec) *crossplanea func (p *AWSProvider) updateDBInstance(ctx context.Context, params DatabaseSpec, dbInstance *crossplaneaws.DBInstance) error { // Create a patch snapshot from current DBInstance patchDBInstance := client.MergeFrom(dbInstance.DeepCopy()) + p.configureCrossplaneTags(¶ms) + dbInstance.Spec.ForProvider.Tags = ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { + return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} + }) if params.DbType == AwsPostgres { multiAZ := basefun.GetMultiAZEnabled(p.config) @@ -462,7 +466,7 @@ func (p *AWSProvider) auroraDBInstance(params DatabaseSpec, isSecondInstance boo dbHostname = dbHostname + "-2" } - p.configureDBTags(¶ms) + p.configureCrossplaneTags(¶ms) return &crossplaneaws.DBInstance{ ObjectMeta: metav1.ObjectMeta{ @@ -521,7 +525,7 @@ func (p *AWSProvider) auroraDBCluster(params DatabaseSpec) *crossplaneaws.DBClus } } - p.configureDBTags(¶ms) + p.configureCrossplaneTags(¶ms) return &crossplaneaws.DBCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -696,7 +700,7 @@ func (p *AWSProvider) auroraInstanceParamGroup(params DatabaseSpec) *crossplanea func (p *AWSProvider) updateAuroraDBCluster(ctx context.Context, params DatabaseSpec, dbCluster *crossplaneaws.DBCluster) error { // Create a patch snapshot from current DBCluster patchDBCluster := client.MergeFrom(dbCluster.DeepCopy()) - p.configureDBTags(¶ms) + p.configureCrossplaneTags(¶ms) // Update DBCluster dbCluster.Spec.ForProvider.Tags = ConvertFromProviderTags(params.Tags, func(tag ProviderTag) *crossplaneaws.Tag { return &crossplaneaws.Tag{Key: &tag.Key, Value: &tag.Value} @@ -726,7 +730,7 @@ func (p *AWSProvider) updateAuroraDBCluster(ctx context.Context, params Database return nil } -func (p *AWSProvider) configureDBTags(params *DatabaseSpec) { +func (p *AWSProvider) configureCrossplaneTags(params *DatabaseSpec) { backupPolicy := params.BackupPolicy if backupPolicy == "" { backupPolicy = basefun.GetDefaultBackupPolicy(p.config) diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index 49c2e8d..a9654f0 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -917,7 +917,7 @@ func TestConfigureDBTags(t *testing.T) { mockConfig.Set("defaultBackupPolicyValue", test.backupPolicy) provider := &AWSProvider{config: mockConfig} - provider.configureDBTags(test.inputSpec) + provider.configureCrossplaneTags(test.inputSpec) if !CompareTags(test.inputSpec.Tags, test.expectedTags) { t.Errorf("[%s] expected tags: %+v, got: %+v", From 67c0d5c662efcde4af0053f7bcb5898706b5f726 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Wed, 19 Mar 2025 08:03:41 -0300 Subject: [PATCH 20/22] logs: add proper logging to ensure resource --- pkg/providers/utils.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/providers/utils.go b/pkg/providers/utils.go index a426577..8aea1e4 100644 --- a/pkg/providers/utils.go +++ b/pkg/providers/utils.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" "strings" ) @@ -97,18 +98,30 @@ func isReady(cond []xpv1.Condition) (bool, error) { // ensureResource ensures that a given Kubernetes resource exists in the cluster. // If the resource does not exist, it creates a new instance using the provided createFunc. func ensureResource[T client.Object](ctx context.Context, k8sClient client.Client, key client.ObjectKey, resource T, createFunc func() (T, error)) error { + logger := log.FromContext(ctx) + logger.Info("Ensuring resource existence", "resource", key) if err := k8sClient.Get(ctx, key, resource); err != nil { if errors.IsNotFound(err) { + logger.Info("Resource not found, creating a new one", "resource", key) newResource, createErr := createFunc() if createErr != nil { + logger.Error(createErr, "Failed to create resource instance", "resource", key) return createErr } + if err := k8sClient.Create(ctx, newResource); err != nil { + logger.Error(err, "Failed to create resource", "resource", key) return fmt.Errorf("failed to create resource: %w", err) } + + logger.Info("Resource successfully created", "resource", key) } else { + logger.Error(err, "Failed to get resource", "resource", key) return fmt.Errorf("failed to get resource: %w", err) } + } else { + logger.Info("Resource already exists", "resource", key) } + return nil } From a99ed247fa76874be7199f1a19b1f179b5d14578 Mon Sep 17 00:00:00 2001 From: bfabricio Date: Fri, 21 Mar 2025 14:15:28 -0300 Subject: [PATCH 21/22] refactor: improve overall namming convention --- helm/db-controller/minikube.yaml | 2 +- helm/db-controller/values.yaml | 2 +- pkg/basefunctions/basefunctions.go | 2 +- pkg/databaseclaim/databaseclaim.go | 18 +++++++++--------- pkg/databaseclaim/databaseclaim_test.go | 4 ++-- pkg/providers/crossplane_aws.go | 18 +++++++++--------- pkg/providers/crossplane_aws_test.go | 12 ++++++------ pkg/providers/crossplane_gcp.go | 2 +- pkg/providers/providers.go | 4 ++-- pkg/providers/utils.go | 4 ++-- pkg/providers/utils_test.go | 6 +++--- 11 files changed, 37 insertions(+), 37 deletions(-) diff --git a/helm/db-controller/minikube.yaml b/helm/db-controller/minikube.yaml index a5599a7..de04f84 100644 --- a/helm/db-controller/minikube.yaml +++ b/helm/db-controller/minikube.yaml @@ -1,5 +1,5 @@ controllerConfig: - newProvider: true + enableProvider: true #AWS us-east-1 GCP us-east1 region: us-east-1 vpcSecurityGroupIDRefs: box-3 diff --git a/helm/db-controller/values.yaml b/helm/db-controller/values.yaml index a71771a..1ffed3d 100644 --- a/helm/db-controller/values.yaml +++ b/helm/db-controller/values.yaml @@ -134,7 +134,7 @@ zapLogger: timeEncoding: rfc3339nano controllerConfig: - newProvider: true + enableProvider: true athena-shared: masterUsername: root authSource: secret diff --git a/pkg/basefunctions/basefunctions.go b/pkg/basefunctions/basefunctions.go index a1b3eb9..13fe211 100644 --- a/pkg/basefunctions/basefunctions.go +++ b/pkg/basefunctions/basefunctions.go @@ -261,5 +261,5 @@ func GetDBIdentifierPrefix(viperConfig *viper.Viper) string { } func IsProviderEnable(viperConfig *viper.Viper) bool { - return viperConfig.GetBool("newProvider") + return viperConfig.GetBool("enableProvider") } diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index 5d6257e..0966ad2 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -80,7 +80,7 @@ type DatabaseClaimReconciler struct { Config *DatabaseClaimConfig kctl *kctlutils.Client statusManager *StatusManager - provider providers.Provider + cloudProvider providers.Provider } // New returns a configured databaseclaim reconciler @@ -90,7 +90,7 @@ func New(cli client.Client, cfg *DatabaseClaimConfig) *DatabaseClaimReconciler { Config: cfg, kctl: kctlutils.New(cli, cfg.Viper.GetString("SERVICE_NAMESPACE")), statusManager: NewStatusManager(cli, cfg.Viper), - provider: providers.NewProvider(cfg.Viper, cli, cfg.Namespace), + cloudProvider: providers.NewProvider(cfg.Viper, cli, cfg.Namespace), } } @@ -192,7 +192,7 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques } if basefun.IsProviderEnable(r.Config.Viper) { spec := NewDatabaseSpecFromRequestInfo(&reqInfo, &dbClaim, r.getMode(ctx, &reqInfo, &dbClaim), r.Config.Viper) - if _, err := r.provider.DeleteDatabase(ctx, spec); err != nil { + if _, err := r.cloudProvider.DeleteDatabaseResources(ctx, spec); err != nil { return ctrl.Result{}, err } } else if basefun.GetCloud(r.Config.Viper) == "aws" { @@ -260,13 +260,13 @@ func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, d if basefun.IsProviderEnable(r.Config.Viper) { dbInstanceName := strings.Split(dbClaim.Status.OldDB.ConnectionInfo.Host, ".")[0] - deleted, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName}) + deleted, err := r.cloudProvider.DeleteDatabaseResources(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName}) if err != nil { return ctrl.Result{}, err } if time.Since(dbClaim.Status.OldDB.PostMigrationActionStartedAt.Time).Minutes() > 10 { - _, err := r.provider.DeleteDatabase(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName, TagInactive: false}) + _, err := r.cloudProvider.DeleteDatabaseResources(ctx, providers.DatabaseSpec{ResourceName: dbInstanceName, TagInactive: false}) if err != nil { return ctrl.Result{}, err } @@ -288,7 +288,7 @@ func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, d return ctrl.Result{}, err } - // TODO: after provider implementation is validated, below code can be deprecated + // TODO: after cloudProvider implementation is validated, below code can be deprecated // get name of DBInstance from connectionInfo dbInstanceName := strings.Split(dbClaim.Status.OldDB.ConnectionInfo.Host, ".")[0] @@ -561,7 +561,7 @@ func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, reqInfo *r // TODO: Once the providers implementation is ready, we could completely remove this if cloud condition if basefun.IsProviderEnable(r.Config.Viper) { spec := NewDatabaseSpecFromRequestInfo(reqInfo, dbClaim, operationalMode, r.Config.Viper) - isReady, err = r.provider.CreateDatabase(ctx, spec) + isReady, err = r.cloudProvider.CreateDatabase(ctx, spec) } else if cloud == "aws" { isReady, err = r.manageCloudHostAWS(ctx, reqInfo, dbClaim, operationalMode) if err != nil { @@ -674,7 +674,7 @@ func (r *DatabaseClaimReconciler) providerCRAlreadyExists(ctx context.Context, r case "gcp": instance, cluster = &crossplanegcp.Instance{}, &crossplanegcp.Cluster{} default: - return false, fmt.Errorf("unsupported cloud provider: %s", cloudProvider) + return false, fmt.Errorf("unsupported cloud cloudProvider: %s", cloudProvider) } exists := crExists(ctx, r.Client, dbHostIdentifier, cluster) && crExists(ctx, r.Client, dbHostIdentifier, instance) @@ -691,7 +691,7 @@ func (r *DatabaseClaimReconciler) reconcileMigrateToNewDB(ctx context.Context, r } if exists { - return r.statusManager.SetError(ctx, dbClaim, fmt.Errorf("migration attempt error: crossplane provider CR already exists")) + return r.statusManager.SetError(ctx, dbClaim, fmt.Errorf("migration attempt error: crossplane cloudProvider CR already exists")) } if dbClaim.Status.MigrationState == "" { diff --git a/pkg/databaseclaim/databaseclaim_test.go b/pkg/databaseclaim/databaseclaim_test.go index 3c410d0..a84a71f 100644 --- a/pkg/databaseclaim/databaseclaim_test.go +++ b/pkg/databaseclaim/databaseclaim_test.go @@ -132,8 +132,8 @@ func Test_providerCRAlreadyExists(t *testing.T) { _, err := r.providerCRAlreadyExists(ctx, reqInfo, dbClaim) if err == nil { t.Errorf("expected an error but got nil") - } else if err.Error() != "unsupported cloud provider: anything" { - t.Errorf("expected error 'unsupported cloud provider: anything', got '%s'", err.Error()) + } else if err.Error() != "unsupported cloud cloudProvider: anything" { + t.Errorf("expected error 'unsupported cloud cloudProvider: anything', got '%s'", err.Error()) } }) } diff --git a/pkg/providers/crossplane_aws.go b/pkg/providers/crossplane_aws.go index 09e0762..ad86f91 100644 --- a/pkg/providers/crossplane_aws.go +++ b/pkg/providers/crossplane_aws.go @@ -81,9 +81,9 @@ func (p *AWSProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bo return false, fmt.Errorf("%w: %s must be one of %s", v1.ErrInvalidDBType, spec.DbType, []v1.DatabaseType{v1.Postgres, v1.AuroraPostgres}) } -// DeleteDatabase attempt to delete all crossplane resources related to the provided spec +// DeleteDatabaseResources attempt to delete all crossplane resources related to the provided spec // optionally -func (p *AWSProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) { +func (p *AWSProvider) DeleteDatabaseResources(ctx context.Context, spec DatabaseSpec) (bool, error) { if spec.TagInactive { // it takes some time to propagate the tag to the underlying aws resource if tagged, err := p.addInactiveOperationalTag(ctx, spec); err != nil || !tagged { @@ -148,7 +148,7 @@ func (p *AWSProvider) GetDatabase(ctx context.Context, name string) (*DatabaseSp func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) error { dbParamGroup := &crossplaneaws.DBParameterGroup{} paramGroupKey := client.ObjectKey{Name: getParameterGroupName(params)} - if err := ensureResource(ctx, p.Client, paramGroupKey, dbParamGroup, func() (*crossplaneaws.DBParameterGroup, error) { + if err := verifyOrCreateResource(ctx, p.Client, paramGroupKey, dbParamGroup, func() (*crossplaneaws.DBParameterGroup, error) { return p.postgresDBParameterGroup(params), nil }); err != nil { return err @@ -156,7 +156,7 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e dbInstance := &crossplaneaws.DBInstance{} instanceKey := client.ObjectKey{Name: params.ResourceName} - if err := ensureResource(ctx, p.Client, instanceKey, dbInstance, func() (*crossplaneaws.DBInstance, error) { + if err := verifyOrCreateResource(ctx, p.Client, instanceKey, dbInstance, func() (*crossplaneaws.DBInstance, error) { dbInstance = p.postgresDBInstance(params) secretRef := dbInstance.Spec.ForProvider.CustomDBInstanceParameters.MasterUserPasswordSecretRef if err := ManageMasterPassword(ctx, secretRef, p.Client); err != nil { @@ -181,7 +181,7 @@ func (p *AWSProvider) createPostgres(ctx context.Context, params DatabaseSpec) e func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) error { paramGroupKey := client.ObjectKey{Name: getParameterGroupName(params)} dbParamGroup := &crossplaneaws.DBParameterGroup{} - err := ensureResource(ctx, p.Client, paramGroupKey, dbParamGroup, func() (*crossplaneaws.DBParameterGroup, error) { + err := verifyOrCreateResource(ctx, p.Client, paramGroupKey, dbParamGroup, func() (*crossplaneaws.DBParameterGroup, error) { return p.auroraInstanceParamGroup(params), nil }) if err != nil { @@ -189,7 +189,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e } dbClusterParamGroup := &crossplaneaws.DBClusterParameterGroup{} - err = ensureResource(ctx, p.Client, paramGroupKey, dbClusterParamGroup, func() (*crossplaneaws.DBClusterParameterGroup, error) { + err = verifyOrCreateResource(ctx, p.Client, paramGroupKey, dbClusterParamGroup, func() (*crossplaneaws.DBClusterParameterGroup, error) { return p.auroraClusterParamGroup(params), nil }) if err != nil { @@ -198,7 +198,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e dbCluster := &crossplaneaws.DBCluster{} clusterKey := client.ObjectKey{Name: params.ResourceName} - err = ensureResource(ctx, p.Client, clusterKey, dbCluster, func() (*crossplaneaws.DBCluster, error) { + err = verifyOrCreateResource(ctx, p.Client, clusterKey, dbCluster, func() (*crossplaneaws.DBCluster, error) { dbCluster = p.auroraDBCluster(params) newSecret := dbCluster.Spec.ForProvider.CustomDBClusterParameters.MasterUserPasswordSecretRef if err := ManageMasterPassword(ctx, newSecret, p.Client); err != nil { @@ -210,7 +210,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e // Handle primary database instance primaryDbInstance := &crossplaneaws.DBInstance{} primaryInstanceKey := client.ObjectKey{Name: params.ResourceName} - err = ensureResource(ctx, p.Client, primaryInstanceKey, primaryDbInstance, func() (*crossplaneaws.DBInstance, error) { + err = verifyOrCreateResource(ctx, p.Client, primaryInstanceKey, primaryDbInstance, func() (*crossplaneaws.DBInstance, error) { primaryDbInstance = p.auroraDBInstance(params, false) return primaryDbInstance, nil }) @@ -236,7 +236,7 @@ func (p *AWSProvider) createAuroraDB(ctx context.Context, params DatabaseSpec) e if basefun.GetMultiAZEnabled(p.config) { secondaryDbInstance := &crossplaneaws.DBInstance{} secondaryInstanceKey := client.ObjectKey{Name: params.ResourceName + "-2"} - err = ensureResource(ctx, p.Client, secondaryInstanceKey, secondaryDbInstance, func() (*crossplaneaws.DBInstance, error) { + err = verifyOrCreateResource(ctx, p.Client, secondaryInstanceKey, secondaryDbInstance, func() (*crossplaneaws.DBInstance, error) { secondaryDbInstance = p.auroraDBInstance(params, true) return secondaryDbInstance, nil }) diff --git a/pkg/providers/crossplane_aws_test.go b/pkg/providers/crossplane_aws_test.go index a9654f0..6169976 100644 --- a/pkg/providers/crossplane_aws_test.go +++ b/pkg/providers/crossplane_aws_test.go @@ -345,7 +345,7 @@ var _ = Describe("AWSProvider create Postgres database", func() { _, err := provider.CreateDatabase(ctx, spec) Expect(err).ToNot(HaveOccurred()) - deleted, err := provider.DeleteDatabase(ctx, spec) + deleted, err := provider.DeleteDatabaseResources(ctx, spec) Expect(err).ToNot(HaveOccurred()) Expect(deleted).To(BeTrue()) @@ -369,7 +369,7 @@ var _ = Describe("AWSProvider create Postgres database", func() { deleteSpec := spec deleteSpec.TagInactive = true - deleted, err := provider.DeleteDatabase(ctx, deleteSpec) + deleted, err := provider.DeleteDatabaseResources(ctx, deleteSpec) Expect(err).ToNot(HaveOccurred()) Expect(deleted).To(BeFalse()) @@ -394,7 +394,7 @@ var _ = Describe("AWSProvider create Postgres database", func() { err = k8sClient.Patch(ctx, dbInstance, patchDBInstance) Expect(err).ToNot(HaveOccurred()) - deleted, err = provider.DeleteDatabase(ctx, deleteSpec) + deleted, err = provider.DeleteDatabaseResources(ctx, deleteSpec) Expect(err).ToNot(HaveOccurred()) Expect(deleted).To(BeTrue()) }) @@ -598,7 +598,7 @@ var _ = Describe("AWSProvider create Aurora database", func() { _, err := provider.CreateDatabase(ctx, spec) Expect(err).ToNot(HaveOccurred()) - deleted, err := provider.DeleteDatabase(ctx, spec) + deleted, err := provider.DeleteDatabaseResources(ctx, spec) Expect(err).ToNot(HaveOccurred()) Expect(deleted).To(BeTrue()) @@ -622,7 +622,7 @@ var _ = Describe("AWSProvider create Aurora database", func() { deleteSpec := spec deleteSpec.TagInactive = true - deleted, err := provider.DeleteDatabase(ctx, deleteSpec) + deleted, err := provider.DeleteDatabaseResources(ctx, deleteSpec) Expect(err).ToNot(HaveOccurred()) Expect(deleted).To(BeFalse()) @@ -667,7 +667,7 @@ var _ = Describe("AWSProvider create Aurora database", func() { err = k8sClient.Patch(ctx, cluster, patchDBcluster) Expect(err).ToNot(HaveOccurred()) - deleted, err = provider.DeleteDatabase(ctx, deleteSpec) + deleted, err = provider.DeleteDatabaseResources(ctx, deleteSpec) Expect(err).ToNot(HaveOccurred()) Expect(deleted).To(BeTrue()) }) diff --git a/pkg/providers/crossplane_gcp.go b/pkg/providers/crossplane_gcp.go index 418198e..3b3f25d 100644 --- a/pkg/providers/crossplane_gcp.go +++ b/pkg/providers/crossplane_gcp.go @@ -17,7 +17,7 @@ func (p *GCPProvider) CreateDatabase(ctx context.Context, spec DatabaseSpec) (bo return false, nil } -func (p *GCPProvider) DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) { +func (p *GCPProvider) DeleteDatabaseResources(ctx context.Context, spec DatabaseSpec) (bool, error) { return false, nil } diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go index 5c64e19..7bea518 100644 --- a/pkg/providers/providers.go +++ b/pkg/providers/providers.go @@ -47,8 +47,8 @@ type Provider interface { // If the instance does not exist, it provisions a new one. // Returns true if the database is fully ready, along with any encountered error. CreateDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) - // DeleteDatabase deprovisions an existing database instance. - DeleteDatabase(ctx context.Context, spec DatabaseSpec) (bool, error) + // DeleteDatabaseResources deprovisions an existing database instance. + DeleteDatabaseResources(ctx context.Context, spec DatabaseSpec) (bool, error) // GetDatabase retrieves the current status of a database instance. GetDatabase(ctx context.Context, name string) (*DatabaseSpec, error) } diff --git a/pkg/providers/utils.go b/pkg/providers/utils.go index 8aea1e4..0d1974e 100644 --- a/pkg/providers/utils.go +++ b/pkg/providers/utils.go @@ -95,9 +95,9 @@ func isReady(cond []xpv1.Condition) (bool, error) { return false, nil } -// ensureResource ensures that a given Kubernetes resource exists in the cluster. +// verifyOrCreateResource ensures that a given Kubernetes resource exists in the cluster. // If the resource does not exist, it creates a new instance using the provided createFunc. -func ensureResource[T client.Object](ctx context.Context, k8sClient client.Client, key client.ObjectKey, resource T, createFunc func() (T, error)) error { +func verifyOrCreateResource[T client.Object](ctx context.Context, k8sClient client.Client, key client.ObjectKey, resource T, createFunc func() (T, error)) error { logger := log.FromContext(ctx) logger.Info("Ensuring resource existence", "resource", key) if err := k8sClient.Get(ctx, key, resource); err != nil { diff --git a/pkg/providers/utils_test.go b/pkg/providers/utils_test.go index 8b4d572..ef55de5 100644 --- a/pkg/providers/utils_test.go +++ b/pkg/providers/utils_test.go @@ -84,7 +84,7 @@ func TestEnsureResource(t *testing.T) { }, nil } - err := ensureResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) + err := verifyOrCreateResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) Expect(err).To(BeNil()) createdResource := &corev1.ConfigMap{} err = cl.Get(ctx, key, createdResource) @@ -124,7 +124,7 @@ func TestEnsureResource(t *testing.T) { }, }, nil } - err := ensureResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) + err := verifyOrCreateResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) Expect(err).To(BeNil()) existingAfter := &corev1.ConfigMap{} err = cl.Get(ctx, key, existingAfter) @@ -159,7 +159,7 @@ func TestEnsureResource(t *testing.T) { }, nil } - err := ensureResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) + err := verifyOrCreateResource(ctx, cl, key, &corev1.ConfigMap{}, createFunc) Expect(err).ToNot(BeNil()) Expect(err.Error()).To(ContainSubstring("failed to create resource")) }) From 94fda9eaabec8d2479f7be556a09586cb3dbc82f Mon Sep 17 00:00:00 2001 From: bfabricio Date: Tue, 15 Apr 2025 16:41:39 -0300 Subject: [PATCH 22/22] refactor: disable new provider for deployments --- helm/db-controller/values.yaml | 1 - pkg/databaseclaim/databaseclaim.go | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/helm/db-controller/values.yaml b/helm/db-controller/values.yaml index 1ffed3d..2f5e2f8 100644 --- a/helm/db-controller/values.yaml +++ b/helm/db-controller/values.yaml @@ -134,7 +134,6 @@ zapLogger: timeEncoding: rfc3339nano controllerConfig: - enableProvider: true athena-shared: masterUsername: root authSource: secret diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index 3cdb4b2..a67ab7f 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -257,8 +257,8 @@ func (r *DatabaseClaimReconciler) createMetricsDeployment(ctx context.Context, d } func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, dbClaim *v1.DatabaseClaim) (ctrl.Result, error) { - logr := log.FromContext(ctx).WithValues("databaseclaim", dbClaim.Namespace+"/"+dbClaim.Name) - logr.Info("post migration is in progress") + logger := log.FromContext(ctx).WithValues("databaseclaim", dbClaim.Namespace+"/"+dbClaim.Name) + logger.Info("post migration is in progress") if basefun.IsProviderEnable(r.Config.Viper) { dbInstanceName := strings.Split(dbClaim.Status.OldDB.ConnectionInfo.Host, ".")[0] @@ -280,7 +280,7 @@ func (r *DatabaseClaimReconciler) postMigrationInProgress(ctx context.Context, d } if err := r.statusManager.ClearError(ctx, dbClaim); err != nil { - logr.Error(err, "Error updating DatabaseClaim status") + logger.Error(err, "Error updating DatabaseClaim status") return ctrl.Result{}, err }