stackrox · vladbologa · Nov 3, 2022 · Oct 26, 2022 · Oct 3, 2022 · Oct 4, 2022
@@ -164,12 +164,9 @@ type CentralDBSpec struct {
 	//+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Administrator Password",order=1
 	PasswordSecret *LocalSecretReference `json:"passwordSecret,omitempty"`
 
-	// Disable database password generation. Do not use this for first-time installations in which the operator
-	// is managing Central DB as Central will have no way to connect to the database.
-	//+operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:hidden"}
-	PasswordGenerationDisabled *bool `json:"passwordGenerationDisabled,omitempty"`
-
 	// Specify a connection string that corresponds to an existing database. If set, the operator will not manage Central DB.
+	// When using this option, you must explicitly set a password secret; automatically generating a password will not
+	// be supported.
-	// When using this option, you must explicitly set a password secret; automatically generating a password will not
-	// be supported.
+	// When using this option, you must explicitly set a password secret; a password will not be automatically generated.
-	// When using this option, you must explicitly set a password secret; automatically generating a password will not
-	// be supported.
+	// When using this option, you must explicitly set a password secret; a password will not be automatically generated.
 	//+operator-sdk:csv:customresourcedefinitions:type=spec,order=2
 	ConnectionStringOverride *string `json:"connectionString,omitempty"`
 
@@ -182,6 +179,14 @@ type CentralDBSpec struct {
 	DeploymentSpec `json:",inline"`
 }
 
+// GetPasswordSecret provides a way to retrieve the admin password that is safe to use on a nil receiver object.
+func (c *CentralDBSpec) GetPasswordSecret() *LocalSecretReference {
+	if c == nil {
+		return nil
+	}
+	return c.PasswordSecret
+}
+
 // IsExternal specifies that the database should not be managed by the Operator
 func (c *CentralDBSpec) IsExternal() bool {
 	if c == nil {
@@ -298,6 +303,8 @@ type DBPersistentVolumeClaim struct {
 	// The name of the PVC to manage persistent data. If no PVC with the given name exists, it will be
 	// created. Defaults to "central-db" if not set.
 	//+operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Claim Name",order=1
+	//+kubebuilder:validation:Default=central-db
+	//+kubebuilder:default=central-db
 	ClaimName *string `json:"claimName,omitempty"`
 
 	// The size of the persistent volume when created through the claim. If a claim was automatically created,

@@ -66,20 +66,16 @@ spec:
                       connectionString:
                         description: Specify a connection string that corresponds
                           to an existing database. If set, the operator will not manage
-                          Central DB.
+                          Central DB. When using this option, you must explicitly
+                          set a password secret; automatically generating a password
+                          will not be supported.
                         type: string
                       nodeSelector:
                         additionalProperties:
                           type: string
                         description: If you want this component to only run on specific
                           nodes, you can configure a node selector here.
                         type: object
-                      passwordGenerationDisabled:
-                        description: Disable database password generation. Do not
-                          use this for first-time installations in which the operator
-                          is managing Central DB as Central will have no way to connect
-                          to the database.
-                        type: boolean
                       passwordSecret:
                         description: Specify a secret that contains the password in
                           the "password" data item. If omitted, the operator will
@@ -113,6 +109,7 @@ spec:
                               Recommended for most users.
                             properties:
                               claimName:
+                                default: central-db
                                 description: The name of the PVC to manage persistent
                                   data. If no PVC with the given name exists, it will
                                   be created. Defaults to "central-db" if not set.

@@ -115,7 +115,9 @@ spec:
         displayName: Scanner Component
         path: scanner.scannerComponent
       - description: Specify a connection string that corresponds to an existing database.
-          If set, the operator will not manage Central DB.
+          If set, the operator will not manage Central DB. When using this option,
+          you must explicitly set a password secret; automatically generating a password
+          will not be supported.
         displayName: Connection String Override
         path: central.db.connectionString
       - description: The size of the persistent volume when created through the claim.
@@ -362,13 +364,6 @@ spec:
         path: central.adminPasswordSecret.name
         x-descriptors:
         - urn:alm:descriptor:io.kubernetes:Secret
-      - description: Disable database password generation. Do not use this for first-time
-          installations in which the operator is managing Central DB as Central will
-          have no way to connect to the database.
-        displayName: Password Generation Disabled
-        path: central.db.passwordGenerationDisabled
-        x-descriptors:
-        - urn:alm:descriptor:com.tectonic.ui:hidden
       - description: The name of the referenced secret.
         displayName: Name
         path: central.db.passwordSecret.name

@@ -22,10 +22,11 @@ type secretVerifyFunc func(t *testing.T, data types.SecretDataMap)
 type statusVerifyFunc func(t *testing.T, status *platform.CentralStatus)
 
 type secretReconciliationTestCase struct {
-	Spec     platform.CentralSpec
-	Deleted  bool
-	Existing []*v1.Secret
-	Other    []ctrlClient.Object
+	Spec            platform.CentralSpec
+	Deleted         bool
+	Existing        []*v1.Secret
+	ExistingManaged []*v1.Secret
+	Other           []ctrlClient.Object
 
 	ExpectedCreatedSecrets     map[string]secretVerifyFunc
 	ExpectedError              string
@@ -74,6 +75,11 @@ func testSecretReconciliation(t *testing.T, runFn func(ctx context.Context, cent
 	for _, existingSecret := range c.Existing {
 		existingSecrets = append(existingSecrets, existingSecret.DeepCopy())
 	}
+	for _, existingManagedSecret := range c.ExistingManaged {
+		managedSecret := existingManagedSecret.DeepCopy()
+		managedSecret.SetOwnerReferences([]metav1.OwnerReference{*metav1.NewControllerRef(central, central.GroupVersionKind())})
+		existingSecrets = append(existingSecrets, managedSecret)
+	}
 	var otherExisting []runtime.Object
 	for _, existingObj := range c.Other {
 		otherExisting = append(otherExisting, existingObj.DeepCopyObject())
@@ -114,10 +120,10 @@ func testSecretReconciliation(t *testing.T, runFn func(ctx context.Context, cent
 
 	for _, existingSecret := range c.Existing {
 		found, ok := secretsByName[existingSecret.Name]
-		if !assert.Truef(t, ok, "pre-existing secret %s is gone", existingSecret.Name) {
+		if !assert.Truef(t, ok, "pre-existing unmanaged secret %s is gone", existingSecret.Name) {
 			continue
 		}
-		assert.Equalf(t, existingSecret.Data, found.Data, "data of pre-existing secret %s has changed", existingSecret.Name)
+		assert.Equalf(t, existingSecret.Data, found.Data, "data of pre-existing unmanaged secret %s has changed", existingSecret.Name)
 		delete(secretsByName, existingSecret.Name)
 	}
 

@@ -2,6 +2,7 @@ package extensions
 
 import (
 	"context"
+	"strings"
 
 	"github.com/go-logr/logr"
 	"github.com/operator-framework/helm-operator-plugins/pkg/extensions"
@@ -10,57 +11,116 @@ import (
 	commonExtensions "github.com/stackrox/rox/operator/pkg/common/extensions"
 	"github.com/stackrox/rox/operator/pkg/types"
 	"github.com/stackrox/rox/pkg/renderer"
+	coreV1 "k8s.io/api/core/v1"
 	ctrlClient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 const (
-	centralDBPasswordKey          = `password`
-	centralDBPasswordResourceName = "central-db-password"
+	centralDBPasswordKey = `password`
+
+	// canonicalCentralDBPasswordSecretName is the name of the secret that is mounted into Central (and Central DB).
+	// This is not configurable; if a user specifies a different password secret, the password from that needs to be
+	// mirrored into the canonical password secret.
+	canonicalCentralDBPasswordSecretName = `central-db-password`
 )
 
-// ReconcileCentralDBPasswordExtension returns an extension that takes care of creating the central-db-password
-// secret ahead of time.
+// ReconcileCentralDBPasswordExtension returns an extension that takes care of reconciling the central-db-password secret.
 func ReconcileCentralDBPasswordExtension(client ctrlClient.Client) extensions.ReconcileExtension {
 	return wrapExtension(reconcileCentralDBPassword, client)
 }
 
-func reconcileCentralDBPassword(ctx context.Context, c *platform.Central, client ctrlClient.Client, _ func(updateStatusFunc), _ logr.Logger) error {
-	if !c.Spec.Central.CentralDBEnabled() || c.Spec.Central.DB.IsExternal() {
-		return nil
-	}
+func reconcileCentralDBPassword(ctx context.Context, c *platform.Central, client ctrlClient.Client, statusUpdater func(updateStatusFunc), log logr.Logger) error {
 	run := &reconcileCentralDBPasswordExtensionRun{
 		SecretReconciliator: commonExtensions.NewSecretReconciliator(client, c),
-		obj:                 c,
+		centralObj:          c,
 	}
 	return run.Execute(ctx)
 }
 
 type reconcileCentralDBPasswordExtensionRun struct {
 	*commonExtensions.SecretReconciliator
-	obj *platform.Central
+	centralObj *platform.Central
+	password   string
+}
+
+func (r *reconcileCentralDBPasswordExtensionRun) readAndSetPasswordFromReferencedSecret(ctx context.Context) error {
+	if r.centralObj.Spec.Central.DB.GetPasswordSecret() == nil {
+		return errors.New("no password secret was specified in spec.central.db.passwordSecret")
+	}
+
+	passwordSecretName := r.centralObj.Spec.Central.DB.PasswordSecret.Name
-	passwordSecretName := r.centralObj.Spec.Central.DB.PasswordSecret.Name
+	passwordSecretName := r.centralObj.Spec.Central.DB.GetPasswordSecret().Name
-	passwordSecretName := r.centralObj.Spec.Central.DB.PasswordSecret.Name
+	passwordSecretName := r.centralObj.Spec.Central.DB.GetPasswordSecret().Name
+
+	passwordSecret := &coreV1.Secret{}
+	key := ctrlClient.ObjectKey{Namespace: r.centralObj.GetNamespace(), Name: passwordSecretName}
+	if err := r.Client().Get(ctx, key, passwordSecret); err != nil {
+		return errors.Wrapf(err, "failed to retrieve central db password secret %q", passwordSecretName)
+	}
+
+	password, err := passwordFromSecretData(passwordSecret.Data)
+	if err != nil {
+		return errors.Wrapf(err, "reading central db password from secret %s", passwordSecretName)
+	}
+
+	r.password = password
+	return nil
 }
 
 func (r *reconcileCentralDBPasswordExtensionRun) Execute(ctx context.Context) error {
-	// Delete any central-db password only if the CR is being deleted
-	shouldExist := r.obj.GetDeletionTimestamp() == nil
+	if r.centralObj.DeletionTimestamp != nil || !r.centralObj.Spec.Central.CentralDBEnabled() {
+		return r.ReconcileSecret(ctx, canonicalCentralDBPasswordSecretName, false, nil, nil, false)
+	}
 
-	if err := r.ReconcileSecret(ctx, centralDBPasswordResourceName, shouldExist, r.validateCentralDBPasswordData, r.generateCentralDBPasswordData, true); err != nil {
-		return errors.Wrapf(err, "reconciling %q secret", centralDBPasswordResourceName)
+	dbSpec := r.centralObj.Spec.Central.DB // non-nil thanks to CentralDBEnabled() check above
+	dbPasswordSecret := dbSpec.PasswordSecret
+	if dbSpec.IsExternal() && dbPasswordSecret == nil {
+		return errors.New("setting spec.central.db.passwordSecret is mandatory when using an external DB")
 	}
 
+	if dbPasswordSecret != nil {
+		if err := r.readAndSetPasswordFromReferencedSecret(ctx); err != nil {
+			return err
+		}
+		// If the user wants to use the central-db-password secret directly, that's fine, and we don't have anything more to do.
+		if dbPasswordSecret.Name == canonicalCentralDBPasswordSecretName {
+			return nil
+		}
+	}
+	// At this point, r.password was set via readAndSetPasswordFromReferencedSecret above (user-specified mode), or is unset,
+	// in which case the auto-generation logic will take effect.
+	if err := r.ReconcileSecret(ctx, canonicalCentralDBPasswordSecretName, true, r.validateSecretData, r.generateDBPassword, true); err != nil {
+		return errors.Wrapf(err, "reconciling %s secret", canonicalCentralDBPasswordSecretName)
+	}
 	return nil
 }
 
-func (r *reconcileCentralDBPasswordExtensionRun) validateCentralDBPasswordData(data types.SecretDataMap, _ bool) error {
-	if len(data[centralDBPasswordKey]) == 0 {
-		return errors.Errorf("%s secret must contain a non-empty %q entry", centralDBPasswordResourceName, centralDBPasswordKey)
+func (r *reconcileCentralDBPasswordExtensionRun) validateSecretData(data types.SecretDataMap, controllerOwned bool) error {
+	password, err := passwordFromSecretData(data)
+	if err != nil {
+		return errors.Wrap(err, "validating existing secret data")
 	}
+	if r.password != "" && r.password != password {
+		return errors.New("existing password does not match expected one")
+	}
+	// The following assignment shouldn't have any consequences, as a successful validation should prevent generation
+	// from being invoked, but better safe than sorry (about clobbering a user-set password).
+	r.password = password
 	return nil
 }
 
-func (r *reconcileCentralDBPasswordExtensionRun) generateCentralDBPasswordData() (types.SecretDataMap, error) {
-	data := types.SecretDataMap{
-		centralDBPasswordKey: []byte(renderer.CreatePassword()),
+func (r *reconcileCentralDBPasswordExtensionRun) generateDBPassword() (types.SecretDataMap, error) {
+	if r.password == "" {
+		r.password = renderer.CreatePassword()
+	}
+
+	return types.SecretDataMap{
+		centralDBPasswordKey: []byte(r.password),
+	}, nil
+}
+
+func passwordFromSecretData(data types.SecretDataMap) (string, error) {
+	password := strings.TrimSpace(string(data[centralDBPasswordKey]))
+	if password == "" || strings.ContainsAny(password, "\r\n") {
+		return "", errors.Errorf("secret must contain a non-empty, single-line %q entry", centralDBPasswordKey)
 	}
-	return data, nil
+	return password, nil
 }