Carry the environment on a config write

controlplane/attach_env_key_test.go

@@ -169,7 +169,7 @@ func TestAttachRefusesOccupiedConfigKey(t *testing.T) {
 	ctx := context.Background()
 	e, _, d, _ := newPostgresEngine(t)
 
-	if err := d.SetAppEnv(ctx, "web", "DB_URL", "postgres://elsewhere/db"); err != nil {
+	if err := d.SetAppEnv(ctx, "web", cp.DefaultEnvironment, "DB_URL", "postgres://elsewhere/db"); err != nil {
 		t.Fatalf("seed config: %v", err)
 	}
 	_, err := e.AttachAddon(ctx, cp.AddonPostgres, "web", "", cp.AttachAddonOptions{Confirm: true, EnvKey: "DB_URL"})

controlplane/audit_test.go

@@ -287,10 +287,10 @@ func TestAuditRedactsEnvValues(t *testing.T) {
 	ctx := context.Background()
 
 	const secretValue = "super-secret-token-value"
-	if err := d.SetAppEnv(ctx, "web", "API_KEY", secretValue); err != nil {
+	if err := d.SetAppEnv(ctx, "web", cp.DefaultEnvironment, "API_KEY", secretValue); err != nil {
 		t.Fatalf("SetAppEnv: %v", err)
 	}
-	if err := d.SetAppEnv(ctx, "web", "DB_HOST", "db.internal"); err != nil {
+	if err := d.SetAppEnv(ctx, "web", cp.DefaultEnvironment, "DB_HOST", "db.internal"); err != nil {
 		t.Fatalf("SetAppEnv: %v", err)
 	}
 

controlplane/config_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	cp "github.com/burrow-cloud/burrow/controlplane"
+	"github.com/burrow-cloud/burrow/controlplane/internal/fake"
 )
 
 func TestSetConfigPersistsAndLists(t *testing.T) {
@@ -19,7 +20,7 @@ func TestSetConfigPersistsAndLists(t *testing.T) {
 	if err := e.SetConfig(ctx, "web", "", "LOG_LEVEL", "debug", false, false); err != nil {
 		t.Fatalf("SetConfig (no release): %v", err)
 	}
-	cfg, err := e.ListConfig(ctx, "web", "")
+	cfg, err := e.ListConfig(ctx, "web", cp.DefaultEnvironment)
 	if err != nil {
 		t.Fatalf("ListConfig: %v", err)
 	}
@@ -168,3 +169,84 @@ func TestRollbackRendersCurrentStoreConfig(t *testing.T) {
 		t.Errorf("spec env = %+v, want A=2 (current store value), not the v1 snapshot", spec.Env)
 	}
 }
+
+// TestConfigWriteCarriesTheEnvironment pins what the config seam is told, as distinct from what it
+// stores. A config write happens IN an environment — the engine resolves that environment's
+// namespace and re-applies that environment's workload — and until the environment travelled with
+// the write, an implementation of controlplane.Database saw two indistinguishable calls for a
+// staging change and a production one.
+//
+// The other half of the assertion is that carrying it changed nothing: what an app READS is still
+// app-global (ADR-0028), so a value set while pointed at staging is in the config production
+// renders, and a removal made anywhere removes it everywhere. That is the behaviour
+// `docs/CAPABILITIES.md` promises, and the reason the environment is not on AppEnv.
+func TestConfigWriteCarriesTheEnvironment(t *testing.T) {
+	ctx := context.Background()
+	e, _, d, _ := newEngine(t, permissive())
+	if _, err := e.AddEnvironment(ctx, "staging", "burrow-apps-staging"); err != nil {
+		t.Fatalf("AddEnvironment: %v", err)
+	}
+
+	// --no-restart throughout: this is about the store call, and no app is deployed anywhere.
+	if err := e.SetConfig(ctx, "web", "staging", "LOG_LEVEL", "debug", true, false); err != nil {
+		t.Fatalf("SetConfig (staging): %v", err)
+	}
+	// The default environment is named rather than left empty: with a second environment
+	// registered, Burrow refuses to pick one for a mutating operation.
+	if err := e.SetConfig(ctx, "web", cp.DefaultEnvironment, "REGION", "eu", true, false); err != nil {
+		t.Fatalf("SetConfig (default environment): %v", err)
+	}
+	if err := e.UnsetConfig(ctx, "web", "staging", "REGION", true, false); err != nil {
+		t.Fatalf("UnsetConfig (staging): %v", err)
+	}
+
+	want := []fake.AppEnvWrite{
+		{App: "web", Env: "staging", Key: "LOG_LEVEL"},
+		{App: "web", Env: cp.DefaultEnvironment, Key: "REGION"},
+		{App: "web", Env: "staging", Key: "REGION", Unset: true},
+	}
+	got := d.AppEnvWrites()
+	if len(got) != len(want) {
+		t.Fatalf("writes = %+v, want %+v", got, want)
+	}
+	for i := range want {
+		if got[i] != want[i] {
+			t.Errorf("write %d = %+v, want %+v", i, got[i], want[i])
+		}
+	}
+
+	// The default environment's config holds the staging write and has lost the key staging
+	// removed: one store per app, not one per environment.
+	cfg, err := e.ListConfig(ctx, "web", "")
+	if err != nil {
+		t.Fatalf("ListConfig: %v", err)
+	}
+	if cfg["LOG_LEVEL"] != "debug" {
+		t.Errorf("config = %+v, want the staging write visible in the default environment", cfg)
+	}
+	if _, present := cfg["REGION"]; present {
+		t.Errorf("config = %+v, want REGION removed everywhere", cfg)
+	}
+}
+
+// TestConfigWriteCarriesTheCanonicalEnvironmentName pins the OTHER half of the seam's contract: the
+// name that arrives is canonical (ADR-0067 §2), so an implementation keying anything off it never
+// has to know that an empty selector and "prod" are the same environment. A caller may leave the
+// environment unnamed when there is only one, and that is the case this covers — with a second
+// environment registered Burrow refuses to choose for a mutating operation at all.
+func TestConfigWriteCarriesTheCanonicalEnvironmentName(t *testing.T) {
+	ctx := context.Background()
+	e, _, d, _ := newEngine(t, permissive())
+
+	if err := e.SetConfig(ctx, "web", "", "LOG_LEVEL", "debug", true, false); err != nil {
+		t.Fatalf("SetConfig: %v", err)
+	}
+	if err := e.UnsetConfig(ctx, "web", "", "LOG_LEVEL", true, false); err != nil {
+		t.Fatalf("UnsetConfig: %v", err)
+	}
+	for _, w := range d.AppEnvWrites() {
+		if w.Env != cp.DefaultEnvironment {
+			t.Errorf("write %+v carried %q, want the canonical %q", w, w.Env, cp.DefaultEnvironment)
+		}
+	}
+}

controlplane/dependencies_test.go

@@ -199,7 +199,7 @@ func TestDeployRunsTheDerivedCheckInTheAppsOwnImage(t *testing.T) {
 	ctx := context.Background()
 	installPostgresAddon(t, d, cp.DefaultEnvironment)
 	prov.SetAttachedApps(cp.DefaultEnvironment, "web")
-	if err := d.SetAppEnv(ctx, "web", "LOG_LEVEL", "debug"); err != nil {
+	if err := d.SetAppEnv(ctx, "web", cp.DefaultEnvironment, "LOG_LEVEL", "debug"); err != nil {
 		t.Fatalf("SetAppEnv: %v", err)
 	}
 	k.SetRunResult(cp.RunResult{Stdout: probeStdout(cp.DependencyResult{

controlplane/engine.go

@@ -689,7 +689,11 @@ func (e *Engine) SetConfig(ctx context.Context, app, env, key, value string, noR
 			configWhat("setting", key, app, noRestart))); err != nil {
 		return err
 	}
-	if err := e.db.SetAppEnv(ctx, app, key, value); err != nil {
+	// The environment goes to the store with the write (ADR-0028): what is stored is app-global, but
+	// which environment the change was made in is a fact the engine holds — it resolved that
+	// environment's namespace above and re-applies that environment's workload below — and an
+	// implementation of the seam has no way to reconstruct it.
+	if err := e.db.SetAppEnv(ctx, app, envName(env), key, value); err != nil {
 		e.recordExecution(ctx, auditOpConfigSet, app, args, err)
 		return fmt.Errorf("set config %s: persisting %s: %w", app, key, err)
 	}
@@ -731,7 +735,8 @@ func (e *Engine) UnsetConfig(ctx context.Context, app, env, key string, noRestar
 			configWhat("removing", key, app, noRestart))); err != nil {
 		return err
 	}
-	if err := e.db.UnsetAppEnv(ctx, app, key); err != nil {
+	// The environment travels with the removal for SetConfig's reason.
+	if err := e.db.UnsetAppEnv(ctx, app, envName(env), key); err != nil {
 		e.recordExecution(ctx, auditOpConfigUnset, app, args, err)
 		return fmt.Errorf("unset config %s: removing %s: %w", app, key, err)
 	}

controlplane/engine_test.go

@@ -134,7 +134,7 @@ func TestDeployHappyPath(t *testing.T) {
 	e, k, d, _ := newEngine(t, permissive())
 
 	// Env is sourced from the app's config store at deploy time, not from the request (ADR-0028).
-	if err := d.SetAppEnv(ctx, "web", "K", "V"); err != nil {
+	if err := d.SetAppEnv(ctx, "web", cp.DefaultEnvironment, "K", "V"); err != nil {
 		t.Fatalf("SetAppEnv: %v", err)
 	}
 

controlplane/internal/fake/database.go

@@ -19,21 +19,26 @@ var _ controlplane.Database = (*Database)(nil)
 // copied in and out, so callers never share Env/Command memory with the store — the
 // same isolation a real database gives. Errors can be injected per operation.
 type Database struct {
-	mu         sync.Mutex
-	byID       map[string]controlplane.Release
-	order      map[string][]string // app -> release IDs, save order, deduplicated
-	providers  map[string]controlplane.Provider
-	addons     map[string]controlplane.AddonInfo
-	appEnv     map[string]map[string]string                       // app -> key -> value
-	hooks      map[string]controlplane.Hook                       // (app, env, phase) -> configured lifecycle hook
-	autoDeploy map[string]map[string]controlplane.AutoDeployLevel // app -> env -> level
-	reason     map[string]map[string]string                       // app -> env -> disable reason
-	audit      []controlplane.AuditEntry                          // append-only, in append order
-	backups    map[string]controlplane.Backup
-	backupSeq  []string                            // backup IDs in record order, for deterministic newest-first listing
-	envs       map[string]controlplane.Environment // registered environments by name
-	errs       map[Op]error
-	policy     controlplane.Policy
+	mu        sync.Mutex
+	byID      map[string]controlplane.Release
+	order     map[string][]string // app -> release IDs, save order, deduplicated
+	providers map[string]controlplane.Provider
+	addons    map[string]controlplane.AddonInfo
+	appEnv    map[string]map[string]string // app -> key -> value
+	// Every config write in the order it was made, WITH the environment it was made in. Storage
+	// above stays app-global, exactly as Postgres is, so the fake is never the looser of the two;
+	// this is a separate log of what the seam was told, which is the only place the environment a
+	// write belongs to survives in an implementation that does not act on it.
+	appEnvWrites []AppEnvWrite
+	hooks        map[string]controlplane.Hook                       // (app, env, phase) -> configured lifecycle hook
+	autoDeploy   map[string]map[string]controlplane.AutoDeployLevel // app -> env -> level
+	reason       map[string]map[string]string                       // app -> env -> disable reason
+	audit        []controlplane.AuditEntry                          // append-only, in append order
+	backups      map[string]controlplane.Backup
+	backupSeq    []string                            // backup IDs in record order, for deterministic newest-first listing
+	envs         map[string]controlplane.Environment // registered environments by name
+	errs         map[Op]error
+	policy       controlplane.Policy
 	// The failure ledger and its coverage record (ADR-0074 §4). They are separate from the audit
 	// slice above for the same reason they are separate tables in the store: one is what Burrow was
 	// asked to do, the other is what happened afterwards, and only the second is pruned (§7).
@@ -391,8 +396,32 @@ func (d *Database) AppEnv(ctx context.Context, app string) (map[string]string, e
 	return out, nil
 }
 
-// SetAppEnv upserts one env key for app.
-func (d *Database) SetAppEnv(ctx context.Context, app, key, value string) error {
+// AppEnvWrite is one config write the fake was asked to perform, and the environment it was made
+// in. It exists so a test can assert the environment REACHED the seam: the value itself is
+// app-global once written, so nothing about the stored config could ever show which environment
+// put it there.
+type AppEnvWrite struct {
+	App string
+	Env string
+	Key string
+	// Unset distinguishes the two directions. A removal carries no value, so a zero Value on a
+	// write and a removal would otherwise read the same.
+	Unset bool
+}
+
+// AppEnvWrites returns a copy of every config write made through the seam, in order.
+func (d *Database) AppEnvWrites() []AppEnvWrite {
+	d.mu.Lock()
+	defer d.mu.Unlock()
+	out := make([]AppEnvWrite, len(d.appEnvWrites))
+	copy(out, d.appEnvWrites)
+	return out
+}
+
+// SetAppEnv upserts one env key for app. env is recorded in the write log and NOT in the stored
+// config: the value is app-global once written, which is the real store's behaviour and therefore
+// has to be the fake's.
+func (d *Database) SetAppEnv(ctx context.Context, app, env, key, value string) error {
 	d.mu.Lock()
 	defer d.mu.Unlock()
 	if err := d.errs[OpSetAppEnv]; err != nil {
@@ -402,17 +431,20 @@ func (d *Database) SetAppEnv(ctx context.Context, app, key, value string) error
 		d.appEnv[app] = make(map[string]string)
 	}
 	d.appEnv[app][key] = value
+	d.appEnvWrites = append(d.appEnvWrites, AppEnvWrite{App: app, Env: env, Key: key})
 	return nil
 }
 
-// UnsetAppEnv removes one env key for app. Removing a key that is not set is a no-op.
-func (d *Database) UnsetAppEnv(ctx context.Context, app, key string) error {
+// UnsetAppEnv removes one env key for app. Removing a key that is not set is a no-op. env is
+// recorded for SetAppEnv's reason, and the removal is app-global for the same one.
+func (d *Database) UnsetAppEnv(ctx context.Context, app, env, key string) error {
 	d.mu.Lock()
 	defer d.mu.Unlock()
 	if err := d.errs[OpUnsetAppEnv]; err != nil {
 		return err
 	}
 	delete(d.appEnv[app], key)
+	d.appEnvWrites = append(d.appEnvWrites, AppEnvWrite{App: app, Env: env, Key: key, Unset: true})
 	return nil
 }
 

controlplane/postgres/store.go

@@ -221,8 +221,14 @@ func (s *Store) AppEnv(ctx context.Context, app string) (map[string]string, erro
 	return env, nil
 }
 
-// SetAppEnv upserts one env key for app.
-func (s *Store) SetAppEnv(ctx context.Context, app, key, value string) error {
+// SetAppEnv upserts one env key for app. The environment the write was made in is accepted and
+// deliberately NOT stored, which is why it arrives here as a blank: this store is app-global by
+// design (ADR-0028), the `app_env` table is keyed by (app, key) with no environment column, and the
+// value it holds is rendered into the workload in every environment the app is deployed to. Writing
+// the environment into the row would be a schema change and a change of meaning, not a record of
+// one. The seam carries it for implementations that act per-environment on a config write; Postgres
+// records exactly what it has always recorded.
+func (s *Store) SetAppEnv(ctx context.Context, app, _, key, value string) error {
 	const q = `
 INSERT INTO app_env (app, key, value) VALUES ($1, $2, $3)
 ON CONFLICT (app, key) DO UPDATE SET value = EXCLUDED.value`
@@ -232,8 +238,10 @@ ON CONFLICT (app, key) DO UPDATE SET value = EXCLUDED.value`
 	return nil
 }
 
-// UnsetAppEnv removes one env key for app. Removing a key that is not set is a no-op.
-func (s *Store) UnsetAppEnv(ctx context.Context, app, key string) error {
+// UnsetAppEnv removes one env key for app. Removing a key that is not set is a no-op. The
+// environment is blank here for SetAppEnv's reason, and the removal is app-global for the same
+// one: there is a single row behind the key, so it goes everywhere at once.
+func (s *Store) UnsetAppEnv(ctx context.Context, app, _, key string) error {
 	const q = `DELETE FROM app_env WHERE app = $1 AND key = $2`
 	if _, err := s.db.ExecContext(ctx, q, app, key); err != nil {
 		return fmt.Errorf("postgres: unset app env %q for %q: %w", key, app, err)

controlplane/postgres/store_test.go

@@ -384,18 +384,18 @@ func TestStoreAppEnvRoundTrip(t *testing.T) {
 		t.Errorf("AppEnv (empty) = %v, want empty", got)
 	}
 
-	if err := s.SetAppEnv(ctx, app, "LOG_LEVEL", "debug"); err != nil {
+	if err := s.SetAppEnv(ctx, app, cp.DefaultEnvironment, "LOG_LEVEL", "debug"); err != nil {
 		t.Fatalf("SetAppEnv: %v", err)
 	}
-	if err := s.SetAppEnv(ctx, app, "FEATURE", "on"); err != nil {
+	if err := s.SetAppEnv(ctx, app, cp.DefaultEnvironment, "FEATURE", "on"); err != nil {
 		t.Fatalf("SetAppEnv: %v", err)
 	}
 	// Upsert overwrites in place.
-	if err := s.SetAppEnv(ctx, app, "LOG_LEVEL", "info"); err != nil {
+	if err := s.SetAppEnv(ctx, app, cp.DefaultEnvironment, "LOG_LEVEL", "info"); err != nil {
 		t.Fatalf("SetAppEnv (upsert): %v", err)
 	}
 	// A different app's env is isolated.
-	if err := s.SetAppEnv(ctx, other, "LOG_LEVEL", "trace"); err != nil {
+	if err := s.SetAppEnv(ctx, other, cp.DefaultEnvironment, "LOG_LEVEL", "trace"); err != nil {
 		t.Fatalf("SetAppEnv (other): %v", err)
 	}
 
@@ -407,11 +407,37 @@ func TestStoreAppEnvRoundTrip(t *testing.T) {
 		t.Errorf("AppEnv = %v, want {LOG_LEVEL:info FEATURE:on}", got)
 	}
 
+	// A write made in ANOTHER environment lands in the same app-global store and reads back
+	// alongside the rest (ADR-0028). This is the property the seam's environment must not have
+	// quietly turned into a filter: an app deployed to prod renders the value a staging write set,
+	// because there is one config store per app and not one per environment.
+	if err := s.SetAppEnv(ctx, app, "staging", "REGION", "eu"); err != nil {
+		t.Fatalf("SetAppEnv (other environment): %v", err)
+	}
+	got, err = s.AppEnv(ctx, app)
+	if err != nil {
+		t.Fatalf("AppEnv after a write in another environment: %v", err)
+	}
+	if got["REGION"] != "eu" {
+		t.Errorf("AppEnv = %v, want a staging write to be visible app-globally", got)
+	}
+	// And the removal is app-global too, whichever environment asks for it.
+	if err := s.UnsetAppEnv(ctx, app, cp.DefaultEnvironment, "REGION"); err != nil {
+		t.Fatalf("UnsetAppEnv (other environment): %v", err)
+	}
+	got, err = s.AppEnv(ctx, app)
+	if err != nil {
+		t.Fatalf("AppEnv after the cross-environment unset: %v", err)
+	}
+	if _, present := got["REGION"]; present {
+		t.Errorf("AppEnv = %v, want REGION removed everywhere", got)
+	}
+
 	// Unset removes a key; removing a missing key is a no-op.
-	if err := s.UnsetAppEnv(ctx, app, "FEATURE"); err != nil {
+	if err := s.UnsetAppEnv(ctx, app, cp.DefaultEnvironment, "FEATURE"); err != nil {
 		t.Fatalf("UnsetAppEnv: %v", err)
 	}
-	if err := s.UnsetAppEnv(ctx, app, "NOPE"); err != nil {
+	if err := s.UnsetAppEnv(ctx, app, cp.DefaultEnvironment, "NOPE"); err != nil {
 		t.Fatalf("UnsetAppEnv (absent): %v", err)
 	}
 	got, err = s.AppEnv(ctx, app)
@@ -423,8 +449,8 @@ func TestStoreAppEnvRoundTrip(t *testing.T) {
 	}
 
 	// Cleanup so the shared database stays tidy across re-runs.
-	_ = s.UnsetAppEnv(ctx, app, "LOG_LEVEL")
-	_ = s.UnsetAppEnv(ctx, other, "LOG_LEVEL")
+	_ = s.UnsetAppEnv(ctx, app, cp.DefaultEnvironment, "LOG_LEVEL")
+	_ = s.UnsetAppEnv(ctx, other, cp.DefaultEnvironment, "LOG_LEVEL")
 }
 
 // TestStoreReleaseRollout round-trips what the deploy observed of a release's rollout (ADR-0092 §4).