carlos / platform Public

appconfig: split syncHost into prepare and commit with a three-state outcome

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 8afa6e57199f7a75d926e4ee6783a08b5166b438 parent d5a8fa1
2 files changed, +94 −22
  • internal/appconfig/sync.go +60 −22
  • internal/appconfig/sync_test.go +34 −0
diff --git a/internal/appconfig/sync.go b/internal/appconfig/sync.go
index 608c9dc01..805146ac5 100644
--- a/internal/appconfig/sync.go
+++ b/internal/appconfig/sync.go
@@ -329,19 +329,60 @@ func UnitConfigApps(routes []registry.Route) [][2]string {
// defaultMissing says the merged view is a bound environment's bundle ALONE
// because the app has no config.json — see the guard below.
func syncHost(rt registry.Route, m *Manifest, defaultMissing bool, priv *ecdh.PrivateKey, envDir string, restart func(host string) error) {
+ path, rendered, cur, existed, ok := prepareHost(rt, m, defaultMissing, priv, envDir)
+ if !ok {
+ return
+ }
+ if existed && bytes.Equal(cur, rendered) {
+ return // up to date — stay quiet on the periodic tick
+ }
+ if !commitHost(rt.Host, path, rendered) {
+ return
+ }
+ // No rt.Hibernate special case on purpose: try-restart is a no-op on an
+ // inactive unit, so a hibernated route's file converges without waking it.
+ if err := restart(rt.Host); err != nil {
+ fmt.Printf(" %s: config updated but restart failed: %v\n", rt.Host, err)
+ return
+ }
+ fmt.Printf(" %s: config updated, restarted\n", rt.Host)
+}
+
+// hostOutcome is what one route's env file did this pass. Three states,
+// not two (spec §3.2, codex R2-2): a refusal, a render error or a write
+// error are all hostFailed, and a caller that treats "nothing changed" and
+// "could not write" alike would restart a unit against a stale or absent
+// file.
+type hostOutcome int
+
+const (
+ hostFailed hostOutcome = iota
+ hostUnchanged
+ hostChanged
+)
+
+// prepareHost is the read-and-render half of syncHost: everything up to
+// the decision to write, with every refusal syncHost makes. ok == false
+// means a refusal or error was printed and nothing may be written for this
+// route this pass. Split from the write so the stamped path (unitsync.go)
+// can decide what else must happen before the env file lands.
+func prepareHost(rt registry.Route, m *Manifest, defaultMissing bool, priv *ecdh.PrivateKey, envDir string) (path string, rendered, cur []byte, existed, ok bool) {
// The host is joined into a root-written path; a registry row carrying
// a separator must never place a file outside envDir. Same trust level
// as release's live/<host> symlinks, but this one is one line to pin.
if rt.Host == "" || rt.Host == "." || rt.Host == ".." || rt.Host != filepath.Base(rt.Host) {
fmt.Printf(" %q: not a bare hostname, refusing to derive an env file path from it\n", rt.Host)
- return
+ return "", nil, nil, false, false
}
- path := filepath.Join(envDir, rt.Host+".env")
+ path = filepath.Join(envDir, rt.Host+".env")
cur, err := os.ReadFile(path)
- existed := err == nil
+ existed = err == nil
if err != nil && !os.IsNotExist(err) {
fmt.Printf(" %s: read %s: %v\n", rt.Host, path, err)
- return
+ return "", nil, nil, false, false
+ }
+ if !existed {
+ cur = nil
}
// The same protection an UNBOUND route gets from Sync's "no manifest
// object at all ⇒ leave the file alone" skip, applied to the DEFAULT half
@@ -384,19 +425,19 @@ func syncHost(rt registry.Route, m *Manifest, defaultMissing bool, priv *ecdh.Pr
"refusing to replace %s, which this pass cannot parse as KEY=value and so cannot prove is safe to overwrite; "+
"import its config (`carlos env set --app %s KEY=value ...`) or remove the file\n",
rt.Host, rt.Environment, rt.App, path, rt.App)
- return
+ return "", nil, nil, false, false
case len(lost) > 0:
fmt.Printf(" %s: environment %q is bound but %s has no default config bundle, so the merged view is that environment ALONE — "+
"refusing to drop %d var(s) already in %s (%s); import them (`carlos env set --app %s KEY=value ...`) "+
"and this route converges on the next pass, or remove the file if it is dead\n",
rt.Host, rt.Environment, rt.App, len(lost), path, strings.Join(lost, ", "), rt.App)
- return
+ return "", nil, nil, false, false
}
}
- rendered, err := RenderEnv(m, priv)
+ rendered, err = RenderEnv(m, priv)
if err != nil {
fmt.Printf(" %s: render config: %v\n", rt.Host, err)
- return
+ return "", nil, nil, false, false
}
// Spec §7.3: deliver CARLOS_PURGE=header to unit tenants the same way
// the exec backend already does for exec-backed ones (withoutVars /
@@ -418,24 +459,21 @@ func syncHost(rt registry.Route, m *Manifest, defaultMissing bool, priv *ecdh.Pr
// var itself is the payload, so the one-time fleet-wide restart is the
// feature landing, not a side effect of it.
rendered = applyAgentVars(rendered)
- if existed && bytes.Equal(cur, rendered) {
- return // up to date — stay quiet on the periodic tick
- }
+ return path, rendered, cur, existed, true
+}
+
+// commitHost writes rendered to path atomically. False means the write
+// failed and was printed; the file on disk is whatever it was.
+func commitHost(host, path string, rendered []byte) bool {
tmp := path + ".tmp"
if err := os.WriteFile(tmp, rendered, 0o600); err != nil {
- fmt.Printf(" %s: write %s: %v\n", rt.Host, path, err)
- return
+ fmt.Printf(" %s: write %s: %v\n", host, path, err)
+ return false
}
if err := os.Rename(tmp, path); err != nil {
os.Remove(tmp) // don't leave rendered secrets behind
- fmt.Printf(" %s: rename %s: %v\n", rt.Host, path, err)
- return
+ fmt.Printf(" %s: rename %s: %v\n", host, path, err)
+ return false
}
- // No rt.Hibernate special case on purpose: try-restart is a no-op on an
- // inactive unit, so a hibernated route's file converges without waking it.
- if err := restart(rt.Host); err != nil {
- fmt.Printf(" %s: config updated but restart failed: %v\n", rt.Host, err)
- return
- }
- fmt.Printf(" %s: config updated, restarted\n", rt.Host)
+ return true
}
diff --git a/internal/appconfig/sync_test.go b/internal/appconfig/sync_test.go
index c331df0b5..d4e924fa3 100644
--- a/internal/appconfig/sync_test.go
+++ b/internal/appconfig/sync_test.go
@@ -1,6 +1,7 @@
package appconfig
import (
+ "bytes"
"context"
"os"
"path/filepath"
@@ -1132,3 +1133,36 @@ func TestSyncMaterializesEveryRouteOfAMultiRouteApp(t *testing.T) {
}
}
}
+
+func TestPrepareAndCommitHostOutcomes(t *testing.T) {
+ envDir := t.TempDir()
+ priv, _, err := GenerateKey()
+ if err != nil {
+ t.Fatalf("GenerateKey: %v", err)
+ }
+ privKey, err := ParsePrivateKey(priv, "test")
+ if err != nil {
+ t.Fatalf("ParsePrivateKey: %v", err)
+ }
+ rt := registry.Route{Host: "keymail.example.com", Kind: "instance", App: "keymail", Account: "bab", Status: "ready"}
+ m := &Manifest{Vars: map[string]Var{"LOG_LEVEL": {Kind: "env", Value: "debug"}}}
+
+ path, rendered, cur, existed, ok := prepareHost(rt, m, false, privKey, envDir)
+ if !ok || existed || cur != nil || path != filepath.Join(envDir, "keymail.example.com.env") {
+ t.Fatalf("first prepare: ok=%v existed=%v cur=%q path=%q", ok, existed, cur, path)
+ }
+ if !commitHost(rt.Host, path, rendered) {
+ t.Fatal("commitHost failed")
+ }
+ _, rendered2, cur2, existed2, ok2 := prepareHost(rt, m, false, privKey, envDir)
+ if !ok2 || !existed2 || !bytes.Equal(cur2, rendered2) {
+ t.Fatalf("second prepare: ok=%v existed=%v equal=%v", ok2, existed2, bytes.Equal(cur2, rendered2))
+ }
+
+ // A non-bare host is a refusal: ok == false, nothing written.
+ bad := rt
+ bad.Host = "../x"
+ if _, _, _, _, ok := prepareHost(bad, m, false, privKey, envDir); ok {
+ t.Fatal("prepareHost accepted a non-bare hostname")
+ }
+}