From d3ee159018c957679113a8d33b4d70ca7eaf9b67 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Fri, 11 Aug 2023 15:30:02 +0200 Subject: [PATCH 1/5] ROX-18427: read central DB ID override from optional ConfigMap --- .../pkg/central/reconciler/reconciler.go | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 18ac53787d..b170662002 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -991,7 +991,12 @@ func (r *CentralReconciler) ensureCentralDeleted(ctx context.Context, remoteCent // skip Snapshot for remoteCentral created by probe skipSnapshot := remoteCentral.Metadata.Internal - err = r.managedDBProvisioningClient.EnsureDBDeprovisioned(remoteCentral.Id, skipSnapshot) + databaseID, err := r.getDatabaseID(ctx, remoteCentral.Metadata.Namespace, remoteCentral.Id) + if err != nil { + return false, fmt.Errorf("getting DB ID: %w", err) + } + + err = r.managedDBProvisioningClient.EnsureDBDeprovisioned(databaseID, skipSnapshot) if err != nil { if errors.Is(err, cloudprovider.ErrDBBackupInProgress) { glog.Infof("Can not delete Central DB for: %s, backup in progress", remoteCentral.Metadata.Namespace) @@ -1022,6 +1027,27 @@ func (r *CentralReconciler) ensureCentralDeleted(ctx context.Context, remoteCent return globalDeleted, nil } +func (r *CentralReconciler) getDatabaseID(ctx context.Context, remoteCentralNamespace, centralID string) (string, error) { + // By default the database ID (which is used to name the cloud DB resources) is the same as the central ID, + // but this value can be overriden + configMap := &corev1.ConfigMap{} + err := r.client.Get(ctx, ctrlClient.ObjectKey{Namespace: remoteCentralNamespace, Name: "central-db-override"}, configMap) + if err != nil { + if apiErrors.IsNotFound(err) { + return centralID, nil + } + + return centralID, fmt.Errorf("getting central DB ID override ConfigMap: %w", err) + } + + overrideValue, exists := configMap.Data["databaseID"] + if exists { + return overrideValue, nil + } + + return centralID, nil +} + // centralChanged compares the given central to the last central reconciled using a hash func (r *CentralReconciler) centralChanged(central private.ManagedCentral) (bool, error) { currentHash, err := util.MD5SumFromJSONStruct(¢ral) @@ -1158,7 +1184,12 @@ func (r *CentralReconciler) ensureManagedCentralDBInitialized(ctx context.Contex return fmt.Errorf("getting DB password from secret: %w", err) } - err = r.managedDBProvisioningClient.EnsureDBProvisioned(ctx, remoteCentral.Id, remoteCentral.Id, dbMasterPassword, remoteCentral.Metadata.Internal) + databaseID, err := r.getDatabaseID(ctx, remoteCentralNamespace, remoteCentral.Id) + if err != nil { + return fmt.Errorf("getting DB ID: %w", err) + } + + err = r.managedDBProvisioningClient.EnsureDBProvisioned(ctx, databaseID, remoteCentral.Id, dbMasterPassword, remoteCentral.Metadata.Internal) if err != nil { return fmt.Errorf("provisioning RDS DB: %w", err) } From fa417c2cc7f88b11ca51944b76452e70af3d00e4 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 11 Sep 2023 21:21:59 +0200 Subject: [PATCH 2/5] Fixes --- fleetshard/pkg/central/reconciler/reconciler.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index b170662002..55fdd0ed99 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -1042,9 +1042,11 @@ func (r *CentralReconciler) getDatabaseID(ctx context.Context, remoteCentralName overrideValue, exists := configMap.Data["databaseID"] if exists { + glog.Infof("The database ID for Central %s is overridden with: %s", centralID, overrideValue) return overrideValue, nil } + glog.Infof("The central-db-override ConfigMap exists but contains no databaseID field, using default: %s", centralID) return centralID, nil } @@ -1136,7 +1138,12 @@ func (r *CentralReconciler) getCentralDBConnectionString(ctx context.Context, re } } - dbConnection, err := r.managedDBProvisioningClient.GetDBConnection(remoteCentral.Id) + databaseID, err := r.getDatabaseID(ctx, remoteCentral.Metadata.Namespace, remoteCentral.Id) + if err != nil { + return "", fmt.Errorf("getting DB ID: %w", err) + } + + dbConnection, err := r.managedDBProvisioningClient.GetDBConnection(databaseID) if err != nil { if !errors.Is(err, cloudprovider.ErrDBNotFound) { return "", fmt.Errorf("getting RDS DB connection data: %w", err) @@ -1149,6 +1156,7 @@ func (r *CentralReconciler) getCentralDBConnectionString(ctx context.Context, re return "", fmt.Errorf("trying to restore DB: %w", err) } } + return dbConnection.GetConnectionForUserAndDB(dbCentralUserName, postgres.CentralDBName).WithSSLRootCert(postgres.DatabaseCACertificatePathCentral).AsConnectionString(), nil } @@ -1194,7 +1202,7 @@ func (r *CentralReconciler) ensureManagedCentralDBInitialized(ctx context.Contex return fmt.Errorf("provisioning RDS DB: %w", err) } - dbConnection, err := r.managedDBProvisioningClient.GetDBConnection(remoteCentral.Id) + dbConnection, err := r.managedDBProvisioningClient.GetDBConnection(databaseID) if err != nil { return fmt.Errorf("getting RDS DB connection data: %w", err) } From 1fc20e7565c95470abf7c94a042803057573b5fc Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Tue, 12 Sep 2023 15:04:43 +0200 Subject: [PATCH 3/5] Fix comment --- fleetshard/pkg/central/reconciler/reconciler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 55fdd0ed99..e40361bde7 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -1027,9 +1027,9 @@ func (r *CentralReconciler) ensureCentralDeleted(ctx context.Context, remoteCent return globalDeleted, nil } +// getDatabaseID returns the cloud database ID for a central tenant. +// By default the database ID is equal to the centralID. It can be overridden by a ConfigMap. func (r *CentralReconciler) getDatabaseID(ctx context.Context, remoteCentralNamespace, centralID string) (string, error) { - // By default the database ID (which is used to name the cloud DB resources) is the same as the central ID, - // but this value can be overriden configMap := &corev1.ConfigMap{} err := r.client.Get(ctx, ctrlClient.ObjectKey{Namespace: remoteCentralNamespace, Name: "central-db-override"}, configMap) if err != nil { From 84ee9945e019356aa229fbb3b950a7f3bbf61565 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Tue, 12 Sep 2023 16:21:29 +0200 Subject: [PATCH 4/5] Add tests --- .../pkg/central/reconciler/reconciler.go | 9 +- .../pkg/central/reconciler/reconciler_test.go | 108 +++++++++++++++--- 2 files changed, 100 insertions(+), 17 deletions(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index e40361bde7..f8b6c9e12d 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -76,8 +76,9 @@ const ( dbUserTypeCentral = "central" dbCentralUserName = "rhacs_central" - centralDbSecretName = "central-db-password" // pragma: allowlist secret - centralDeletePollInterval = 5 * time.Second + centralDbSecretName = "central-db-password" // pragma: allowlist secret + centralDbOverrideConfigMap = "central-db-override" + centralDeletePollInterval = 5 * time.Second sensibleDeclarativeConfigSecretName = "cloud-service-sensible-declarative-configs" // pragma: allowlist secret manualDeclarativeConfigSecretName = "cloud-service-manual-declarative-configs" // pragma: allowlist secret @@ -1031,7 +1032,7 @@ func (r *CentralReconciler) ensureCentralDeleted(ctx context.Context, remoteCent // By default the database ID is equal to the centralID. It can be overridden by a ConfigMap. func (r *CentralReconciler) getDatabaseID(ctx context.Context, remoteCentralNamespace, centralID string) (string, error) { configMap := &corev1.ConfigMap{} - err := r.client.Get(ctx, ctrlClient.ObjectKey{Namespace: remoteCentralNamespace, Name: "central-db-override"}, configMap) + err := r.client.Get(ctx, ctrlClient.ObjectKey{Namespace: remoteCentralNamespace, Name: centralDbOverrideConfigMap}, configMap) if err != nil { if apiErrors.IsNotFound(err) { return centralID, nil @@ -1046,7 +1047,7 @@ func (r *CentralReconciler) getDatabaseID(ctx context.Context, remoteCentralName return overrideValue, nil } - glog.Infof("The central-db-override ConfigMap exists but contains no databaseID field, using default: %s", centralID) + glog.Infof("The %s ConfigMap exists but contains no databaseID field, using default: %s", centralDbOverrideConfigMap, centralID) return centralID, nil } diff --git a/fleetshard/pkg/central/reconciler/reconciler_test.go b/fleetshard/pkg/central/reconciler/reconciler_test.go index 12397b15fb..43840cfac2 100644 --- a/fleetshard/pkg/central/reconciler/reconciler_test.go +++ b/fleetshard/pkg/central/reconciler/reconciler_test.go @@ -11,17 +11,8 @@ import ( "testing" "time" - "github.com/stackrox/rox/pkg/declarativeconfig" - "github.com/stackrox/rox/pkg/utils" - "gopkg.in/yaml.v2" - "k8s.io/apimachinery/pkg/api/resource" - "k8s.io/apimachinery/pkg/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - "github.com/aws/aws-sdk-go/aws/credentials/stscreds" - "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/credentials/stscreds" openshiftRouteV1 "github.com/openshift/api/route/v1" "github.com/pkg/errors" "github.com/stackrox/acs-fleet-manager/fleetshard/config" @@ -38,16 +29,23 @@ import ( "github.com/stackrox/acs-fleet-manager/pkg/client/fleetmanager" "github.com/stackrox/acs-fleet-manager/pkg/features" "github.com/stackrox/rox/operator/apis/platform/v1alpha1" + "github.com/stackrox/rox/pkg/declarativeconfig" + "github.com/stackrox/rox/pkg/utils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" "helm.sh/helm/v3/pkg/chart/loader" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) const ( @@ -578,13 +576,17 @@ func TestDisablePauseAnnotation(t *testing.T) { func TestReconcileDeleteWithManagedDB(t *testing.T) { managedDBProvisioningClient := &cloudprovider.DBClientMock{} - managedDBProvisioningClient.EnsureDBProvisionedFunc = func(_ context.Context, _string, _ string, _ string, _ bool) error { + managedDBProvisioningClient.EnsureDBProvisionedFunc = func(_ context.Context, databaseID, acsInstanceID, _ string, _ bool) error { + require.Equal(t, databaseID, acsInstanceID) + require.Equal(t, databaseID, simpleManagedCentral.Id) return nil } - managedDBProvisioningClient.EnsureDBDeprovisionedFunc = func(_ string, _ bool) error { + managedDBProvisioningClient.EnsureDBDeprovisionedFunc = func(databaseID string, _ bool) error { + require.Equal(t, databaseID, simpleManagedCentral.Id) return nil } - managedDBProvisioningClient.GetDBConnectionFunc = func(_ string) (postgres.DBConnection, error) { + managedDBProvisioningClient.GetDBConnectionFunc = func(databaseID string) (postgres.DBConnection, error) { + require.Equal(t, databaseID, simpleManagedCentral.Id) connection, err := postgres.NewDBConnection("localhost", 5432, "rhacs", "postgres") if err != nil { return postgres.DBConnection{}, err @@ -644,6 +646,86 @@ func TestReconcileDeleteWithManagedDB(t *testing.T) { assert.True(t, k8sErrors.IsNotFound(err)) } +func TestReconcileDeleteWithManagedDBOverride(t *testing.T) { + dbOverrideId := "override-1234" + + managedDBProvisioningClient := &cloudprovider.DBClientMock{} + managedDBProvisioningClient.EnsureDBProvisionedFunc = func(_ context.Context, databaseID, acsInstanceID, _ string, _ bool) error { + require.Equal(t, databaseID, dbOverrideId) + require.Equal(t, acsInstanceID, simpleManagedCentral.Id) + return nil + } + managedDBProvisioningClient.EnsureDBDeprovisionedFunc = func(databaseID string, _ bool) error { + require.Equal(t, databaseID, dbOverrideId) + return nil + } + managedDBProvisioningClient.GetDBConnectionFunc = func(databaseID string) (postgres.DBConnection, error) { + require.Equal(t, databaseID, dbOverrideId) + connection, err := postgres.NewDBConnection("localhost", 5432, "rhacs", "postgres") + if err != nil { + return postgres.DBConnection{}, err + } + return connection, nil + } + + reconcilerOptions := CentralReconcilerOptions{ + UseRoutes: true, + ManagedDBEnabled: true, + } + fakeClient, _, r := getClientTrackerAndReconciler( + t, + defaultCentralConfig, + managedDBProvisioningClient, + reconcilerOptions, + ) + + namespace := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: simpleManagedCentral.Metadata.Namespace, + }, + } + err := r.client.Create(context.TODO(), namespace) + require.NoError(t, err) + + dbOverrideConfigMap := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: simpleManagedCentral.Metadata.Namespace, + Name: centralDbOverrideConfigMap, + }, + Data: map[string]string{"databaseID": dbOverrideId}, + } + err = r.client.Create(context.TODO(), dbOverrideConfigMap) + require.NoError(t, err) + + _, err = r.Reconcile(context.TODO(), simpleManagedCentral) + require.NoError(t, err) + assert.Len(t, managedDBProvisioningClient.EnsureDBProvisionedCalls(), 1) + + deletedCentral := simpleManagedCentral + deletedCentral.Metadata.DeletionTimestamp = "2006-01-02T15:04:05+00:00" + + // trigger deletion + managedDBProvisioningClient.EnsureDBProvisionedFunc = func(_ context.Context, _ string, _ string, _ string, _ bool) error { + return nil + } + statusTrigger, err := r.Reconcile(context.TODO(), deletedCentral) + require.Error(t, err, ErrDeletionInProgress) + require.Nil(t, statusTrigger) + assert.Len(t, managedDBProvisioningClient.EnsureDBProvisionedCalls(), 1) + + // deletion completed needs second reconcile to check as deletion is async in a kubernetes cluster + statusDeletion, err := r.Reconcile(context.TODO(), deletedCentral) + require.NoError(t, err) + require.NotNil(t, statusDeletion) + + readyCondition, ok := conditionForType(statusDeletion.Conditions, conditionTypeReady) + require.True(t, ok, "Ready condition not found in conditions", statusDeletion.Conditions) + assert.Equal(t, "False", readyCondition.Status) + assert.Equal(t, "Deleted", readyCondition.Reason) + + assert.Len(t, managedDBProvisioningClient.EnsureDBDeprovisionedCalls(), 2) +} + func TestCentralChanged(t *testing.T) { tests := []struct { name string From c1db405499c3f6a47bd6a7391b9db27ed427342e Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Tue, 12 Sep 2023 16:42:22 +0200 Subject: [PATCH 5/5] Fix test --- fleetshard/pkg/central/reconciler/reconciler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fleetshard/pkg/central/reconciler/reconciler_test.go b/fleetshard/pkg/central/reconciler/reconciler_test.go index 43840cfac2..97b88e3e84 100644 --- a/fleetshard/pkg/central/reconciler/reconciler_test.go +++ b/fleetshard/pkg/central/reconciler/reconciler_test.go @@ -672,7 +672,7 @@ func TestReconcileDeleteWithManagedDBOverride(t *testing.T) { UseRoutes: true, ManagedDBEnabled: true, } - fakeClient, _, r := getClientTrackerAndReconciler( + _, _, r := getClientTrackerAndReconciler( t, defaultCentralConfig, managedDBProvisioningClient,