diff --git a/cmd/ate-setup/internal/steps/apienv.go b/cmd/ate-setup/internal/steps/apienv.go index 43e2e26dea..da988eb528 100644 --- a/cmd/ate-setup/internal/steps/apienv.go +++ b/cmd/ate-setup/internal/steps/apienv.go @@ -81,12 +81,18 @@ func (e *Env) CreateAPIServerEnvVars(ctx context.Context) error { return err } } else { + if err := e.refuseBundledOverExternal(ctx); err != nil { + return err + } if readWriteDSN, ownerDSN, err = e.postgresReadWriteConnectionStrings(); err != nil { return err } } } if ownerDSN == "" { + if err := e.refuseDroppingOwnerLogin(ctx); err != nil { + return err + } ownerDSN = readWriteDSN } schema := e.Cfg.PostgresSchemaName() @@ -156,8 +162,40 @@ func buildAPIServerEnvVars(readWriteDSN, ownerDSN, schema string) map[string]str } } -// recordedConnectionStrings reads the credentials paired with an adopted Cloud -// SQL instance. +// refuseBundledOverExternal stops a run that names no database from moving an +// install off its external PostgreSQL, since records hold no connection strings. +func (e *Env) refuseBundledOverExternal(ctx context.Context) error { + // An explicitly empty Cloud SQL instance asks for the bundled database. + if e.Cfg.CloudSQL.InstanceSet { + return nil + } + recorded, _, err := e.recordedConnectionStrings(ctx) + if err != nil { + return err + } + if recorded == "" || usesBundledPostgres(recorded) { + return nil + } + return fmt.Errorf("ate-api-server uses an external PostgreSQL, but ATE_API_POSTGRES_READ_WRITE_CONNECTION_STRING is not set") +} + +// refuseDroppingOwnerLogin stops a run that names no owner connection string +// from replacing a separate owner login with the read-write one. The bundled +// database's owner login is ate-setup's own, so it does not count. +func (e *Env) refuseDroppingOwnerLogin(ctx context.Context) error { + recordedReadWrite, recordedOwner, err := e.recordedConnectionStrings(ctx) + if err != nil { + return err + } + if recordedOwner == recordedReadWrite || usesBundledPostgres(recordedOwner) { + return nil + } + return fmt.Errorf("ate-api-server uses a separate PostgreSQL owner login, but ATE_API_POSTGRES_OWNER_CONNECTION_STRING is not set; " + + "set it to the read-write connection string to use one login") +} + +// recordedConnectionStrings reads the connection strings in the cluster's +// Secret. func (e *Env) recordedConnectionStrings(ctx context.Context) (string, string, error) { secret, err := e.Kube.GetSecret(ctx, e.Namespace(), SecretAPIEnvVars) if err != nil || secret == nil { diff --git a/cmd/ate-setup/internal/steps/apienv_test.go b/cmd/ate-setup/internal/steps/apienv_test.go index 16b28d78c3..903aeeef17 100644 --- a/cmd/ate-setup/internal/steps/apienv_test.go +++ b/cmd/ate-setup/internal/steps/apienv_test.go @@ -151,6 +151,114 @@ func TestCreateAPIServerEnvVarsRejectsCustomBundledIdentity(t *testing.T) { } } +// A run without a connection string must not point an install that uses an +// external PostgreSQL at the bundled one: records hold no connection strings, +// so a forgotten export is the common way to get here. +func TestCreateAPIServerEnvVarsKeepsExternalPostgres(t *testing.T) { + for _, tc := range []struct { + name string + cloudSQL config.CloudSQLConfig + recorded string + wantErr bool + }{ + {name: "fresh install", recorded: ""}, + {name: "bundled install", recorded: bundledPostgresDSN(postgressetup.ReadWriteUser)}, + {name: "external install", recorded: "postgres://runtime@database/atepg", wantErr: true}, + { + // An explicitly empty instance is how an operator leaves Cloud SQL. + name: "leaving Cloud SQL", + cloudSQL: config.CloudSQLConfig{InstanceSet: true}, + recorded: "user=svc@p.iam host=127.0.0.1 port=5432 dbname=atepg sslmode=disable", + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := config.Config{ + PostgresReadWriteRole: config.DefaultPostgresReadWriteRole, PostgresOwnerRole: config.DefaultPostgresOwnerRole, + CloudSQL: tc.cloudSQL, + } + e := &Env{Cfg: &cfg, Kube: fakeKube(t, + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: NamespaceAteSystem}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: ConfigMapAPIEnvVars, Namespace: NamespaceAteSystem}}, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: SecretAPIEnvVars, Namespace: NamespaceAteSystem}, + Data: map[string][]byte{"ATE_API_POSTGRES_READ_WRITE_CONNECTION_STRING": []byte(tc.recorded)}, + }, + )} + err := e.CreateAPIServerEnvVars(t.Context()) + if !tc.wantErr { + if err != nil { + t.Fatal(err) + } + return + } + if err == nil || !strings.Contains(err.Error(), "ATE_API_POSTGRES_READ_WRITE_CONNECTION_STRING is not set") { + t.Fatalf("CreateAPIServerEnvVars() error = %v, want the missing connection string named", err) + } + secret, err := e.Kube.GetSecret(t.Context(), NamespaceAteSystem, SecretAPIEnvVars) + if err != nil { + t.Fatal(err) + } + if got := string(secret.Data["ATE_API_POSTGRES_READ_WRITE_CONNECTION_STRING"]); got != tc.recorded || len(secret.StringData) != 0 { + t.Fatalf("Secret rewritten: Data %q, StringData %v", got, secret.StringData) + } + }) + } +} + +// The owner login is no more recorded than the read-write one, so a run that +// gives only the read-write connection string must not replace a separate owner. +func TestCreateAPIServerEnvVarsKeepsOwnerLogin(t *testing.T) { + const readWrite = "postgres://runtime@database/atepg" + for _, tc := range []struct { + name string + owner string + recordedRW string + recordedOwner string + wantErr bool + }{ + {name: "separate owner not given", recordedRW: readWrite, recordedOwner: "postgres://owner@database/atepg", wantErr: true}, + {name: "separate owner given", owner: "postgres://owner@database/atepg", recordedRW: readWrite, recordedOwner: "postgres://owner@database/atepg"}, + {name: "one login", recordedRW: readWrite, recordedOwner: readWrite}, + { + name: "moving off the bundled database", + recordedRW: bundledPostgresDSN(postgressetup.ReadWriteUser), + recordedOwner: bundledPostgresDSN(postgressetup.OwnerUser), + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := config.Config{PostgresReadWriteConnectionString: readWrite, PostgresOwnerConnectionString: tc.owner} + e := &Env{Cfg: &cfg, Kube: fakeKube(t, + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: NamespaceAteSystem}}, + &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: ConfigMapAPIEnvVars, Namespace: NamespaceAteSystem}}, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: SecretAPIEnvVars, Namespace: NamespaceAteSystem}, + Data: map[string][]byte{ + "ATE_API_POSTGRES_READ_WRITE_CONNECTION_STRING": []byte(tc.recordedRW), + "ATE_API_POSTGRES_OWNER_CONNECTION_STRING": []byte(tc.recordedOwner), + }, + }, + )} + err := e.CreateAPIServerEnvVars(t.Context()) + if !tc.wantErr { + if err != nil { + t.Fatal(err) + } + return + } + if err == nil || !strings.Contains(err.Error(), "ATE_API_POSTGRES_OWNER_CONNECTION_STRING is not set") { + t.Fatalf("CreateAPIServerEnvVars() error = %v, want the missing owner connection string named", err) + } + secret, err := e.Kube.GetSecret(t.Context(), NamespaceAteSystem, SecretAPIEnvVars) + if err != nil { + t.Fatal(err) + } + if got := string(secret.Data["ATE_API_POSTGRES_OWNER_CONNECTION_STRING"]); got != tc.recordedOwner || len(secret.StringData) != 0 { + t.Fatalf("Secret rewritten: owner %q, StringData %v", got, secret.StringData) + } + }) + } +} + func TestCreateAPIServerEnvVarsPoolSize(t *testing.T) { cfg := config.Config{ PostgresReadWriteConnectionString: "postgres://runtime@postgres/atepg", diff --git a/cmd/ate-setup/internal/steps/postgres.go b/cmd/ate-setup/internal/steps/postgres.go index dea4883edf..5067d6a4c8 100644 --- a/cmd/ate-setup/internal/steps/postgres.go +++ b/cmd/ate-setup/internal/steps/postgres.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "fmt" + "net/url" "os" "strings" @@ -31,10 +32,20 @@ import ( "github.com/agent-substrate/substrate/pkg/postgressetup" ) +// bundledPostgresHost is where ate-api-server reaches the bundled PostgreSQL. +const bundledPostgresHost = "postgres.ate-system.svc:5432" + // Each bundled connection presents a certificate for its own login user. func bundledPostgresDSN(user string) string { bundle := "/run/postgres.podcert.ate.dev/" + user + ".pem" - return fmt.Sprintf("postgresql://%s@postgres.ate-system.svc:5432/atepg?sslmode=verify-full&sslrootcert=/run/servicedns.podcert.ate.dev/trust-bundle.pem&sslcert=%s&sslkey=%s", user, bundle, bundle) + return fmt.Sprintf("postgresql://%s@%s/atepg?sslmode=verify-full&sslrootcert=/run/servicedns.podcert.ate.dev/trust-bundle.pem&sslcert=%s&sslkey=%s", user, bundledPostgresHost, bundle, bundle) +} + +// usesBundledPostgres reports whether a connection string names the bundled +// PostgreSQL. +func usesBundledPostgres(dsn string) bool { + u, err := url.Parse(dsn) + return err == nil && u.Host == bundledPostgresHost } func (e *Env) postgresReadWriteConnectionStrings() (string, string, error) {