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. --- 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 ++++ 6 files changed, 155 insertions(+), 33 deletions(-) create mode 100644 internal/envfile/envfile.go create mode 100644 internal/health/health.go (limited to 'internal') 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