From f34ba20fc566ab861179a8ac2fd55eaabbc6a327 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 23:22:03 -0500 Subject: Add skiff config set/unset/list, backed by /app/env in the app's dataset Config vars are plain env vars, injected into both the running app (via rc.d) and the release phase, and versioned on the same ZFS snapshot timeline as deploys - a bad config change gets the same automatic snapshot/restart/health-check/rollback guarantee a bad deploy already gets. Extracts deploy.go's health check into internal/health so both paths share it. Verified end-to-end against a FreeBSD VM: config set/unset, automatic rollback on a health-check failure, and explicit `skiff rollback` all correctly carry the env file along with the rest of the dataset. --- USER_GUIDE.md | 40 +++++++++-- cmd/skiff/main.go | 164 +++++++++++++++++++++++++++++++++++++++++++- internal/deploy/deploy.go | 31 +-------- internal/envfile/envfile.go | 90 ++++++++++++++++++++++++ internal/health/health.go | 40 +++++++++++ internal/layout/layout.go | 4 ++ internal/release/release.go | 16 +++-- internal/tmpl/rc.d.tmpl | 7 ++ 8 files changed, 351 insertions(+), 41 deletions(-) create mode 100644 internal/envfile/envfile.go create mode 100644 internal/health/health.go diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 5ff93f1..04ec92e 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -104,13 +104,35 @@ Skiff has no opinion on your language - the entire build/run contract is a care how the target is defined, only whether bmake thinks it exists. - Persistent state: `/app/data.db` and `/app/env` (jail-relative paths, - siblings of `/app/current`) are meant to be your durable state, - untouched by any single release's checkout/build. In practice today - this is a *convention*, not something Skiff actively manages for you - - there's no automatic env-var injection or database provisioning. If you - want a SQLite file that survives every deploy, just open it at - `/app/data.db` yourself and it'll sit outside the `releases/` - directories that get replaced on every push. + siblings of `/app/current`) are your durable state, untouched by any + single release's checkout/build. If you want a SQLite file that + survives every deploy, just open it at `/app/data.db` yourself and + it'll sit outside the `releases/` directories that get replaced on + every push. + + `/app/env` is actively managed by Skiff via `skiff config`: + + ``` + # skiff config set --app myapp GREETING=Howdy SECRET=abc123 + # skiff config list --app myapp + # skiff config unset --app myapp SECRET + ``` + + `set` merges into the existing config (keys you don't mention are left + alone); `unset` removes the keys you name. Multiple `KEY=VALUE` pairs + in one `set` call are applied atomically. Every var is plumbed in as a + plain environment variable - `os.Getenv("GREETING")` in Go, no bespoke + file format for your app to parse - and reaches both the running app + and any release-phase Makefile target. + + A config change carries the exact same safety guarantee as a code + deploy: Skiff snapshots the app's dataset first, restarts the jail, + waits for the health check, and automatically rolls back to the + pre-change snapshot if the app doesn't come back healthy. There's no + weaker, second-tier safety story for config versus code - `skiff + rollback --list` shows both `pre-` (deploy) and `precfg-` + (config) snapshots interleaved, and `skiff rollback --to` reverts + either kind. ### Deploying @@ -403,4 +425,8 @@ skiff deploy --app --new [--ref refs/heads/main] [--old ] skiff rollback --app --list skiff rollback --app --to + +skiff config set --app KEY=VALUE [KEY=VALUE...] +skiff config unset --app KEY [KEY...] +skiff config list --app ``` diff --git a/cmd/skiff/main.go b/cmd/skiff/main.go index 33aaf5e..2fe5907 100644 --- a/cmd/skiff/main.go +++ b/cmd/skiff/main.go @@ -7,10 +7,14 @@ import ( "flag" "fmt" "os" + "sort" "strings" + "time" "skiff/internal/appmeta" "skiff/internal/deploy" + "skiff/internal/envfile" + "skiff/internal/health" "skiff/internal/jailmgr" "skiff/internal/layout" "skiff/internal/sh" @@ -20,7 +24,7 @@ import ( func main() { if len(os.Args) < 2 { - fmt.Fprintln(os.Stderr, "usage: skiff [flags]") + fmt.Fprintln(os.Stderr, "usage: skiff [flags]") os.Exit(1) } @@ -32,6 +36,8 @@ func main() { err = cmdNewApp(os.Args[2:]) case "rollback": err = cmdRollback(os.Args[2:]) + case "config": + err = cmdConfig(os.Args[2:]) default: fmt.Fprintf(os.Stderr, "unknown subcommand %q\n", os.Args[1]) os.Exit(1) @@ -92,6 +98,9 @@ func cmdNewApp(args []string) error { if err := os.MkdirAll(cfg.ReleasesDir(*name), 0755); err != nil { return fmt.Errorf("create app dir: %w", err) } + if err := envfile.Save(cfg.EnvPath(*name), map[string]string{}); err != nil { + return fmt.Errorf("initialize env file: %w", err) + } if err := jailmgr.WriteConf(cfg, *name, *ip); err != nil { return fmt.Errorf("write jail config: %w", err) @@ -158,3 +167,156 @@ func cmdRollback(args []string) error { } return jailmgr.Restart(ctx, os.Stdout, *app) } + +func cmdConfig(args []string) error { + if len(args) < 1 { + return fmt.Errorf("usage: skiff config --app [args...]") + } + switch args[0] { + case "set": + return cmdConfigSet(args[1:]) + case "unset": + return cmdConfigUnset(args[1:]) + case "list": + return cmdConfigList(args[1:]) + default: + return fmt.Errorf("unknown config subcommand %q", args[0]) + } +} + +func cmdConfigSet(args []string) error { + fs := flag.NewFlagSet("config set", flag.ExitOnError) + app := fs.String("app", "", "app name") + // --app must precede the positional KEY=VALUE args: the flag package + // stops parsing flags at the first non-flag argument. + fs.Parse(args) + pairs := fs.Args() + + if *app == "" { + return fmt.Errorf("--app is required") + } + if len(pairs) == 0 { + return fmt.Errorf("usage: skiff config set --app KEY=VALUE [KEY=VALUE...]") + } + + return applyConfigChange(*app, func(kv map[string]string) error { + for _, pair := range pairs { + parts := strings.SplitN(pair, "=", 2) + if len(parts) != 2 { + return fmt.Errorf("invalid KEY=VALUE pair %q", pair) + } + key := parts[0] + if !envfile.ValidKey(key) { + return fmt.Errorf("invalid env var key %q", key) + } + kv[key] = parts[1] + } + return nil + }) +} + +func cmdConfigUnset(args []string) error { + fs := flag.NewFlagSet("config unset", flag.ExitOnError) + app := fs.String("app", "", "app name") + // --app must precede the positional KEY args, as above. + fs.Parse(args) + keys := fs.Args() + + if *app == "" { + return fmt.Errorf("--app is required") + } + if len(keys) == 0 { + return fmt.Errorf("usage: skiff config unset --app KEY [KEY...]") + } + + return applyConfigChange(*app, func(kv map[string]string) error { + for _, key := range keys { + delete(kv, key) + } + return nil + }) +} + +func cmdConfigList(args []string) error { + fs := flag.NewFlagSet("config list", flag.ExitOnError) + app := fs.String("app", "", "app name") + fs.Parse(args) + + if *app == "" { + return fmt.Errorf("--app is required") + } + cfg := layout.Default() + if _, err := os.Stat(cfg.MetaPath(*app)); err != nil { + return fmt.Errorf("app %q not found: %w", *app, err) + } + + kv, err := envfile.Load(cfg.EnvPath(*app)) + if err != nil { + return fmt.Errorf("load config: %w", err) + } + keys := make([]string, 0, len(kv)) + for key := range kv { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + fmt.Printf("%s=%s\n", key, kv[key]) + } + return nil +} + +// applyConfigChange loads app's current env file, applies mutate to it, and +// carries the same snapshot -> restart -> health-check -> rollback-on- +// failure sequence deploy.Run uses for code deploys - a bad config change +// is exactly as much of an outage as a bad deploy, guarded the same way. +func applyConfigChange(app string, mutate func(map[string]string) error) error { + cfg := layout.Default() + if _, err := os.Stat(cfg.MetaPath(app)); err != nil { + return fmt.Errorf("app %q not found: %w", app, err) + } + + kv, err := envfile.Load(cfg.EnvPath(app)) + if err != nil { + return fmt.Errorf("load config: %w", err) + } + if err := mutate(kv); err != nil { + return err + } + + ctx := context.Background() + out := os.Stdout + dataset := cfg.AppDataset(app) + tag := fmt.Sprintf("precfg-%d", time.Now().Unix()) + + fmt.Fprintln(out, "==> snapshotting current state") + if err := zfsutil.Snapshot(ctx, out, dataset, tag); err != nil { + return fmt.Errorf("snapshot: %w", err) + } + + if err := envfile.Save(cfg.EnvPath(app), kv); err != nil { + return fmt.Errorf("save config: %w", err) + } + + if err := jailmgr.Restart(ctx, out, app); err != nil { + return fmt.Errorf("restart: %w", err) + } + + meta, err := appmeta.Read(cfg.MetaPath(app)) + if err != nil { + return fmt.Errorf("read app metadata: %w", err) + } + + if err := health.WaitHealthy(ctx, meta); err != nil { + fmt.Fprintf(out, "==> health check failed (%v), rolling back to %s\n", err, tag) + if rbErr := zfsutil.Rollback(ctx, out, dataset, tag); rbErr != nil { + return fmt.Errorf("health check failed AND rollback failed: %v / %w", err, rbErr) + } + if rbErr := jailmgr.Restart(ctx, out, app); rbErr != nil { + return fmt.Errorf("health check failed, rolled back, but restart failed: %v / %w", err, rbErr) + } + return fmt.Errorf("config change failed health check and was rolled back: %w", err) + } + + fmt.Fprintln(out, "==> config updated") + return nil +} diff --git a/internal/deploy/deploy.go b/internal/deploy/deploy.go index a25701a..5430b7f 100644 --- a/internal/deploy/deploy.go +++ b/internal/deploy/deploy.go @@ -8,12 +8,11 @@ import ( "context" "fmt" "io" - "net/http" "os" "path/filepath" - "time" "skiff/internal/appmeta" + "skiff/internal/health" "skiff/internal/jailmgr" "skiff/internal/layout" "skiff/internal/nginx" @@ -76,7 +75,7 @@ func Run(ctx context.Context, out io.Writer, cfg layout.Config, app, sha string) return fmt.Errorf("read app metadata: %w", err) } - if err := waitHealthy(ctx, meta); err != nil { + if err := health.WaitHealthy(ctx, meta); err != nil { fmt.Fprintf(out, "==> health check failed (%v), rolling back to pre-%s\n", err, sha) if rbErr := zfsutil.Rollback(ctx, out, dataset, snapshotTag); rbErr != nil { return fmt.Errorf("health check failed AND rollback failed: %v / %w", err, rbErr) @@ -107,29 +106,3 @@ func swapCurrent(currentPath, newTarget string) error { } return os.Rename(tmp, currentPath) } - -func waitHealthy(ctx context.Context, meta appmeta.Meta) error { - url := fmt.Sprintf("http://%s:%d%s", meta.IP, meta.Port, meta.HealthPath) - deadline := time.Now().Add(layout.HealthTimeoutSecs * time.Second) - client := &http.Client{Timeout: 2 * time.Second} - - var lastErr error - for time.Now().Before(deadline) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return err - } - resp, err := client.Do(req) - if err == nil { - resp.Body.Close() - if resp.StatusCode < 300 { - return nil - } - lastErr = fmt.Errorf("status %d", resp.StatusCode) - } else { - lastErr = err - } - time.Sleep(500 * time.Millisecond) - } - return fmt.Errorf("never became healthy at %s: %w", url, lastErr) -} diff --git a/internal/envfile/envfile.go b/internal/envfile/envfile.go new file mode 100644 index 0000000..e729ab2 --- /dev/null +++ b/internal/envfile/envfile.go @@ -0,0 +1,90 @@ +// Package envfile reads and writes the KEY='value' shell-sourceable env +// file skiff stores at each app's /app/env - the on-disk form of `skiff +// config`. It only ever parses the format Save itself writes, not general +// POSIX shell syntax. +package envfile + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "sort" + "strings" +) + +var keyPattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + +// ValidKey reports whether key is a legal env var name. Save is the +// authoritative guard and re-checks every key itself; callers can use +// ValidKey for an earlier, pre-snapshot failure on a typo'd key. +func ValidKey(key string) bool { + return keyPattern.MatchString(key) +} + +// Load reads path into a KEY->value map. A missing file is not an error - +// it's treated as an empty config, covering a fresh app and list/unset +// against one. +func Load(path string) (map[string]string, error) { + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return map[string]string{}, nil + } + if err != nil { + return nil, err + } + + kv := map[string]string{} + for _, line := range strings.Split(string(data), "\n") { + if line == "" { + continue + } + eq := strings.IndexByte(line, '=') + if eq < 0 { + continue + } + key := line[:eq] + quoted := line[eq+1:] + if !strings.HasPrefix(quoted, "'") || !strings.HasSuffix(quoted, "'") { + continue + } + value := strings.ReplaceAll(quoted[1:len(quoted)-1], `'\''`, `'`) + kv[key] = value + } + return kv, nil +} + +// Save validates every key and writes kv to path atomically (write to a +// temp file, then rename over path) at mode 0600 - a deliberate departure +// from the codebase's universal 0644, since this file holds secrets. +func Save(path string, kv map[string]string) error { + keys := make([]string, 0, len(kv)) + for key := range kv { + if !keyPattern.MatchString(key) { + return fmt.Errorf("invalid env var key %q", key) + } + keys = append(keys, key) + } + sort.Strings(keys) + + var b strings.Builder + for _, key := range keys { + b.WriteString(key) + b.WriteByte('=') + b.WriteString(escape(kv[key])) + b.WriteByte('\n') + } + + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + tmp := path + ".tmp" + if err := os.WriteFile(tmp, []byte(b.String()), 0600); err != nil { + return err + } + return os.Rename(tmp, path) +} + +func escape(value string) string { + return `'` + strings.ReplaceAll(value, `'`, `'\''`) + `'` +} diff --git a/internal/health/health.go b/internal/health/health.go new file mode 100644 index 0000000..d1c9df3 --- /dev/null +++ b/internal/health/health.go @@ -0,0 +1,40 @@ +// Package health checks whether a freshly (re)started app is up, by polling +// its declared health endpoint - shared by both internal/deploy and skiff +// config's own restart-and-verify sequence. +package health + +import ( + "context" + "fmt" + "net/http" + "time" + + "skiff/internal/appmeta" + "skiff/internal/layout" +) + +func WaitHealthy(ctx context.Context, meta appmeta.Meta) error { + url := fmt.Sprintf("http://%s:%d%s", meta.IP, meta.Port, meta.HealthPath) + deadline := time.Now().Add(layout.HealthTimeoutSecs * time.Second) + client := &http.Client{Timeout: 2 * time.Second} + + var lastErr error + for time.Now().Before(deadline) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return err + } + resp, err := client.Do(req) + if err == nil { + resp.Body.Close() + if resp.StatusCode < 300 { + return nil + } + lastErr = fmt.Errorf("status %d", resp.StatusCode) + } else { + lastErr = err + } + time.Sleep(500 * time.Millisecond) + } + return fmt.Errorf("never became healthy at %s: %w", url, lastErr) +} diff --git a/internal/layout/layout.go b/internal/layout/layout.go index 636b8ce..d5888dc 100644 --- a/internal/layout/layout.go +++ b/internal/layout/layout.go @@ -102,3 +102,7 @@ func (c Config) ReleaseDirInJail(sha string) string { func (c Config) MetaPath(app string) string { return filepath.Join(c.AppDir(app), "meta.json") } + +func (c Config) EnvPath(app string) string { + return filepath.Join(c.AppDir(app), "env") +} diff --git a/internal/release/release.go b/internal/release/release.go index fe48aa4..a40cfb1 100644 --- a/internal/release/release.go +++ b/internal/release/release.go @@ -37,16 +37,24 @@ func Declared(ctx context.Context, dir string) (bool, error) { return false, fmt.Errorf("make -n release: %w\n%s", err, out) } +// releaseEnvWrapper sources /app/env (if present) before running the +// release recipe, so config vars set via `skiff config` reach the release +// phase the same way they reach the running app. $1 is the release's own +// checkout, passed positionally rather than string-interpolated so there's +// no manual shell-quoting surface. +const releaseEnvWrapper = `if [ -r /app/env ]; then set -a; . /app/env; set +a; fi; exec make -B -C "$1" release` + // Run executes the `release` Makefile target as a one-off inside app's // already-running jail (the jail holds the process's filesystem and any // state the target needs to reach - e.g. the app's own data.db). // releaseDirInJail must be the new release's own checkout, jail-relative - // not `current`, which still points at the previous release until cutover -// happens after this returns successfully. -B forces the recipe to run -// even if a stray file/directory named `release` already exists in the -// checkout. A non-nil error means the deploy must abort before +// happens after this returns successfully. -B is bmake's compatibility/ +// execution-mode flag, not an unconditional-rebuild flag; what actually +// guarantees the recipe always runs is `.PHONY: release` in the app's own +// Makefile. A non-nil error means the deploy must abort before // restart/cutover. func Run(ctx context.Context, out io.Writer, app, releaseDirInJail string) error { fmt.Fprintf(out, "==> release: make release (in %s)\n", releaseDirInJail) - return sh.Run(ctx, out, "", "jexec", app, "make", "-B", "-C", releaseDirInJail, "release") + return sh.Run(ctx, out, "", "jexec", app, "sh", "-c", releaseEnvWrapper, "sh", releaseDirInJail) } diff --git a/internal/tmpl/rc.d.tmpl b/internal/tmpl/rc.d.tmpl index db940fd..134152e 100644 --- a/internal/tmpl/rc.d.tmpl +++ b/internal/tmpl/rc.d.tmpl @@ -14,4 +14,11 @@ command="/usr/sbin/daemon" command_args="-P ${pidfile} -r -t {{.Name}} /app/current/app" load_rc_config $name + +if [ -r /app/env ]; then + set -a + . /app/env + set +a +fi + run_rc_command "$1" -- cgit v1.3