ate-setup: refuse to point an external-database install at the bundled one

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 {

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",

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) {