Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
run: |
os=$(go env GOOS)
arch=$(go env GOARCH)
curl -L https://go.kubebuilder.io/dl/2.3.1/${os}/${arch} | tar -xz -C /tmp/
curl -L https://github.com/kubernetes-sigs/kubebuilder/releases/download/v2.3.1/kubebuilder_2.3.1_${os}_${arch}.tar.gz | tar -xz -C /tmp/
mv /tmp/kubebuilder_2.3.1_${os}_${arch} /usr/local/kubebuilder
export PATH=$PATH:/usr/local/kubebuilder/bin
- name: Tests
Expand Down
4 changes: 2 additions & 2 deletions apis/quay/v1/quayregistry_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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}, "-")
Expand Down
2 changes: 2 additions & 0 deletions controllers/quay/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (

databaseSecretKey = "DATABASE_SECRET_KEY"
secretKey = "SECRET_KEY"
dbURI = "DB_URI"

GrafanaDashboardConfigNamespace = "openshift-config-managed"
)
Expand All @@ -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
}
}
Expand Down
2 changes: 0 additions & 2 deletions kustomize/components/postgres/postgres.deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ metadata:
quay-component: postgres
spec:
replicas: 1
strategy:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the Postgres pod will still be recreated, but changing the strategy to type: RollingUpdate and persisting the password across reconciles means the database will remain available.

type: Recreate
selector:
matchLabels:
quay-component: postgres
Expand Down
3 changes: 3 additions & 0 deletions pkg/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
26 changes: 23 additions & 3 deletions pkg/kustomize/kustomize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
Expand Down Expand Up @@ -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")
Expand All @@ -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 {
Expand All @@ -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
Expand Down
49 changes: 45 additions & 4 deletions pkg/kustomize/kustomize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}
}
Expand Down
14 changes: 3 additions & 11 deletions pkg/kustomize/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 3 additions & 11 deletions pkg/kustomize/secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package kustomize

import (
"fmt"
"net/url"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand Down