kustomize: persist DB_URI for managed postgres (PROJQUAY-1635)

Change DB_URI to be persisted rather than generated on each reconcile. This prevents downtime on reconciles because the old and new Quay pods continue to use the same database password. Signed-off-by: Alec Merdler <[email protected]>
quay · Apr 28, 2021 · 84ea60b · 84ea60b
1 parent 6e17305
commit 84ea60b
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 33 deletions.
diff --git a/apis/quay/v1/quayregistry_types.go b/apis/quay/v1/quayregistry_types.go
@@ -64,6 +64,8 @@ var requiredComponents = []ComponentKind{
 	ComponentRoute,
 }
 
+const ManagedKeysName = "quay-registry-managed-secret-keys"
+
 // QuayRegistrySpec defines the desired state of QuayRegistry.
 type QuayRegistrySpec struct {
 	// ConfigBundleSecret is the name of the Kubernetes `Secret` in the same namespace which contains the base Quay config and extra certs.
@@ -376,8 +378,6 @@ func RemoveOwnerReference(quay *QuayRegistry, obj client.Object) (client.Object,
 	return obj, nil
 }
 
-const ManagedKeysName = "quay-registry-managed-secret-keys"
-
 // ManagedKeysSecretNameFor returns the name of the `Secret` in which generated secret keys are stored.
 func ManagedKeysSecretNameFor(quay *QuayRegistry) string {
 	return strings.Join([]string{quay.GetName(), ManagedKeysName}, "-")

diff --git a/controllers/quay/features.go b/controllers/quay/features.go
@@ -34,6 +34,7 @@ const (
 
 	databaseSecretKey = "DATABASE_SECRET_KEY"
 	secretKey         = "SECRET_KEY"
+	dbURI             = "DB_URI"
 
 	GrafanaDashboardConfigNamespace = "openshift-config-managed"
 )
@@ -55,6 +56,7 @@ func (r *QuayRegistryReconciler) checkManagedKeys(ctx *quaycontext.QuayRegistryC
 		if v1.IsManagedKeysSecretFor(quay, &secret) {
 			ctx.DatabaseSecretKey = string(secret.Data[databaseSecretKey])
 			ctx.SecretKey = string(secret.Data[secretKey])
+			ctx.DbUri = string(secret.Data[dbURI])
 			break
 		}
 	}

diff --git a/kustomize/components/postgres/postgres.deployment.yaml b/kustomize/components/postgres/postgres.deployment.yaml
@@ -6,8 +6,6 @@ metadata:
     quay-component: postgres
 spec:
   replicas: 1
-  strategy:
-    type: Recreate
   selector:
     matchLabels:
       quay-component: postgres

diff --git a/pkg/context/context.go b/pkg/context/context.go
@@ -22,6 +22,9 @@ type QuayRegistryContext struct {
 	// Secret Keys
 	DatabaseSecretKey string
 	SecretKey         string
+
+	// Database
+	DbUri string
 }
 
 // NewQuayRegistryContext returns a fresh context for reconciling a `QuayRegistry`.

diff --git a/pkg/kustomize/kustomize.go b/pkg/kustomize/kustomize.go
@@ -280,6 +280,7 @@ func KustomizationFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistr
 					LiteralSources: []string{
 						"DATABASE_SECRET_KEY=" + ctx.DatabaseSecretKey,
 						"SECRET_KEY=" + ctx.SecretKey,
+						"DB_URI=" + ctx.DbUri,
 					},
 				},
 			},
@@ -421,6 +422,7 @@ func Inflate(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, baseCo
 
 	// Generate or pull out the `SECRET_KEY` and `DATABASE_SECRET_KEY`. Since these must be stable across
 	// runs of the same config, we store them (and re-read them) from a specialized `Secret`.
+	// TODO(alecmerdler): Refector these three blocks...
 	if ctx.DatabaseSecretKey == "" {
 		if key, found := parsedUserConfig["DATABASE_SECRET_KEY"]; found {
 			log.Info("`DATABASE_SECRET_KEY` found in user-provided config")
@@ -437,6 +439,7 @@ func Inflate(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, baseCo
 			ctx.DatabaseSecretKey = databaseSecretKey
 		}
 	}
+	parsedUserConfig["DATABASE_SECRET_KEY"] = ctx.DatabaseSecretKey
 
 	if ctx.SecretKey == "" {
 		if key, found := parsedUserConfig["SECRET_KEY"]; found {
@@ -454,11 +457,28 @@ func Inflate(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, baseCo
 			ctx.SecretKey = secretKey
 		}
 	}
-
-	parsedUserConfig["SETUP_COMPLETE"] = true
-	parsedUserConfig["DATABASE_SECRET_KEY"] = ctx.DatabaseSecretKey
 	parsedUserConfig["SECRET_KEY"] = ctx.SecretKey
 
+	if ctx.DbUri == "" && v1.ComponentIsManaged(quay.Spec.Components, v1.ComponentPostgres) {
+		if _, found := parsedUserConfig["DB_URI"]; found {
+			return nil, fmt.Errorf("`postgres` component marked as managed, but `DB_URI` found in user-provided config")
+		} else {
+			log.Info("`DB_URI` not found in config, generating new one")
+
+			user := quay.GetName() + "-quay-database"
+			name := quay.GetName() + "-quay-database"
+			host := quay.GetName() + "-quay-database"
+			port := "5432"
+			password, err := generateRandomString(secretKeyLength)
+			if err != nil {
+				return nil, err
+			}
+
+			ctx.DbUri = fmt.Sprintf("postgresql://%s:%s@%s:%s/%s", user, password, host, port, name)
+		}
+	}
+	parsedUserConfig["DB_URI"] = ctx.DbUri
+
 	for field, value := range BaseConfig() {
 		if _, ok := parsedUserConfig[field]; !ok {
 			parsedUserConfig[field] = value

diff --git a/pkg/kustomize/kustomize_test.go b/pkg/kustomize/kustomize_test.go
@@ -389,6 +389,26 @@ var inflateTests = []struct {
 		withComponents([]string{"base", "clair", "postgres", "redis", "objectstorage", "mirror"}),
 		nil,
 	},
+	{
+		"PostgresManagedDbUriExists",
+		&v1.QuayRegistry{
+			Spec: v1.QuayRegistrySpec{
+				Components: []v1.Component{
+					{Kind: "postgres", Managed: true},
+				},
+			},
+		},
+		quaycontext.QuayRegistryContext{
+			DbUri: "postgresql://test-quay-database:postgres@test-quay-database:5432/test-quay-database",
+		},
+		&corev1.Secret{
+			Data: map[string][]byte{
+				"config.yaml": encode(map[string]interface{}{"SERVER_HOSTNAME": "quay.io"}),
+			},
+		},
+		withComponents([]string{"base", "postgres"}),
+		nil,
+	},
 }
 
 func TestInflate(t *testing.T) {
@@ -421,11 +441,32 @@ func TestInflate(t *testing.T) {
 			if strings.Contains(objectMeta.GetName(), v1.ManagedKeysSecretNameFor(test.quayRegistry)) {
 				managedKeys := obj.(*corev1.Secret)
 
-				assert.Equal(test.ctx.DatabaseSecretKey, string(managedKeys.Data["DATABASE_SECRET_KEY"]), test.name)
-				assert.Equal(test.ctx.SecretKey, string(managedKeys.Data["SECRET_KEY"]), test.name)
+				if test.ctx.DatabaseSecretKey == "" {
+					assert.Greater(len(string(managedKeys.Data["DATABASE_SECRET_KEY"])), 0, test.name)
+					assert.Greater(len(config["DATABASE_SECRET_KEY"].(string)), 0, test.name)
+				} else {
+					assert.Equal(test.ctx.DatabaseSecretKey, string(managedKeys.Data["DATABASE_SECRET_KEY"]), test.name)
+					assert.Equal(test.ctx.DatabaseSecretKey, config["DATABASE_SECRET_KEY"], test.name)
+				}
+				assert.Equal(string(managedKeys.Data["DATABASE_SECRET_KEY"]), config["DATABASE_SECRET_KEY"], test.name)
+
+				if test.ctx.SecretKey == "" {
+					assert.Greater(len(string(managedKeys.Data["SECRET_KEY"])), 0, test.name)
+					assert.Greater(len(config["SECRET_KEY"].(string)), 0, test.name)
+				} else {
+					assert.Equal(test.ctx.SecretKey, string(managedKeys.Data["SECRET_KEY"]), test.name)
+					assert.Equal(test.ctx.SecretKey, config["SECRET_KEY"], test.name)
+				}
+				assert.Equal(string(managedKeys.Data["SECRET_KEY"]), config["SECRET_KEY"], test.name)
 
-				assert.Equal(test.ctx.DatabaseSecretKey, config["DATABASE_SECRET_KEY"], test.name)
-				assert.Equal(test.ctx.SecretKey, config["SECRET_KEY"], test.name)
+				if test.ctx.DbUri == "" && v1.ComponentIsManaged(test.quayRegistry.Spec.Components, v1.ComponentPostgres) {
+					assert.Greater(len(string(managedKeys.Data["DB_URI"])), 0, test.name)
+					assert.Greater(len(config["DB_URI"].(string)), 0, test.name)
+				} else {
+					assert.Equal(test.ctx.DbUri, string(managedKeys.Data["DB_URI"]), test.name)
+					assert.Equal(test.ctx.DbUri, config["DB_URI"], test.name)
+				}
+				assert.Equal(string(managedKeys.Data["DB_URI"]), config["DB_URI"], test.name)
 			}
 		}
 	}

diff --git a/pkg/kustomize/secrets.go b/pkg/kustomize/secrets.go
@@ -30,8 +30,7 @@ import (
 var underTest = false
 
 const (
-	secretKeySecretName = "quay-registry-managed-secret-keys"
-	secretKeyLength     = 80
+	secretKeyLength = 80
 
 	clairService = "clair-app"
 	// FIXME: Ensure this includes the `QuayRegistry` name prefix when we add `builder` managed component.
@@ -81,16 +80,8 @@ func FieldGroupFor(ctx *quaycontext.QuayRegistryContext, component v1.ComponentK
 		if err != nil {
 			return nil, err
 		}
-		user := quay.GetName() + "-quay-database"
-		name := quay.GetName() + "-quay-database"
-		host := strings.Join([]string{quay.GetName(), "quay-database"}, "-")
-		port := "5432"
-		password, err := generateRandomString(32)
-		if err != nil {
-			return nil, err
-		}
 
-		fieldGroup.DbUri = fmt.Sprintf("postgresql://%s:%s@%s:%s/%s", user, password, host, port, name)
+		fieldGroup.DbUri = ctx.DbUri
 
 		return fieldGroup, nil
 	case v1.ComponentObjectStorage:
@@ -150,6 +141,7 @@ func BaseConfig() map[string]interface{} {
 	}
 
 	return map[string]interface{}{
+		"SETUP_COMPLETE":                     true,
 		"FEATURE_MAILING":                    false,
 		"REGISTRY_TITLE":                     registryTitle,
 		"REGISTRY_TITLE_SHORT":               registryTitle,

diff --git a/pkg/kustomize/secrets_test.go b/pkg/kustomize/secrets_test.go
@@ -2,7 +2,6 @@ package kustomize
 
 import (
 	"fmt"
-	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -80,7 +79,9 @@ var fieldGroupForTests = []struct {
 		"postgres",
 		"postgres",
 		quayRegistry("test"),
-		quaycontext.QuayRegistryContext{},
+		quaycontext.QuayRegistryContext{
+			DbUri: "postgresql://test-quay-database:postgres@test-quay-database:5432/test-quay-database",
+		},
 		&database.DatabaseFieldGroup{
 			DbUri: "postgresql://test-quay-database:postgres@test-quay-database:5432/test-quay-database",
 			DbConnectionArgs: &database.DbConnectionArgsStruct{
@@ -168,15 +169,6 @@ func TestFieldGroupFor(t *testing.T) {
 
 			assert.True(len(secscanFieldGroup.SecurityScannerV4PSK) > 0, test.name)
 			secscanFieldGroup.SecurityScannerV4PSK = "abc123"
-		} else if test.name == "postgres" {
-			databaseFieldGroup := fieldGroup.(*database.DatabaseFieldGroup)
-			dbURI, err := url.Parse(databaseFieldGroup.DbUri)
-			assert.Nil(err, test.name)
-
-			password, _ := dbURI.User.Password()
-			assert.True(len(password) > 0, test.name)
-			dbURI.User = url.UserPassword(dbURI.User.Username(), "postgres")
-			databaseFieldGroup.DbUri = dbURI.String()
 		}
 
 		expected, err := yaml.Marshal(test.expected)