From a4149183508b310d6cae501ee6cc79aaf125936d Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 26 Sep 2026 18:59:07 +1000 Subject: [PATCH] Fetch templates over HTTP instead of shelling out to git The runtime image is distroless and has no git binary, so every sync failed and bootapi silently served the stale embedded templates. - fetch the branch tarball (/archive/.tar.gz) and extract it into an in-memory FS; no checkout, no writable volume - digest the extracted tree, not the archive bytes, so a recompressed identical archive is not a change - skip entries that would escape the tree - log the source commit from Gitea's immutable Link header --- README.md | 2 +- cmd/bootapi/main.go | 12 +- docs/deployment.md | 7 +- docs/template-authoring.md | 3 +- internal/config/config.go | 5 +- internal/gitsync/gitsync.go | 258 +++++++++++++++++++++---------- internal/gitsync/gitsync_test.go | 230 +++++++++++++++++++-------- 7 files changed, 351 insertions(+), 166 deletions(-) diff --git a/README.md b/README.md index b31fb16..d03fefd 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ internal/model/ Host/Interface data model (incl. pxe_enabled gate) internal/netbox/ NetBox client (reads + pxe_enabled write) + TTL cache, behind an interface internal/catalog/ distro catalog: NetBox host -> boot images/kickstart internal/render/ text/template engine (swappable Set), selection, loader -internal/gitsync/ periodic git pull + atomic template reload (last-good) +internal/gitsync/ periodic templates-tarball fetch + atomic reload (last-good) internal/server/ chi HTTP handlers + Prometheus metrics templates/ embedded defaults: kickstart, iPXE, catalog/*.yaml docs/ see above diff --git a/cmd/bootapi/main.go b/cmd/bootapi/main.go index 868f340..021ce0e 100644 --- a/cmd/bootapi/main.go +++ b/cmd/bootapi/main.go @@ -9,7 +9,6 @@ import ( "log/slog" "os" "os/signal" - "path/filepath" "syscall" "git.unkin.net/unkin/bootapi/internal/config" @@ -133,10 +132,10 @@ func runValidate(dir string) int { return 0 } -// buildEngine constructs the render Engine and, when a templates git repo is -// configured, a Syncer that reloads it periodically. Precedence: git repo → -// local override dir → embedded defaults only. Git/dir failures degrade to the -// embedded defaults rather than failing startup. +// buildEngine constructs the render Engine and, when a templates repo is +// configured, a Syncer that reloads it periodically. Precedence: templates repo +// → local override dir → embedded defaults only. Fetch/dir failures degrade to +// the embedded defaults rather than failing startup. func buildEngine(ctx context.Context, cfg *config.Config, rcfg render.RenderConfig) (*render.Engine, *gitsync.Syncer, error) { switch { case cfg.TemplateGitURL != "": @@ -145,11 +144,10 @@ func buildEngine(ctx context.Context, cfg *config.Config, rcfg render.RenderConf Branch: cfg.TemplateGitBranch, Token: cfg.TemplateGitToken, Interval: cfg.TemplateGitInterval, - WorkDir: filepath.Join(os.TempDir(), "bootapi-templates"), }, templates.FS) set, gerr := syncer.Bootstrap(ctx) if gerr != nil { - slog.Warn("template git bootstrap degraded to embedded defaults", "err", gerr) + slog.Warn("template bootstrap degraded to embedded defaults", "err", gerr) } engine := render.NewEngine(rcfg, set) syncer.SetEngine(engine) diff --git a/docs/deployment.md b/docs/deployment.md index e62f951..7629c51 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -44,10 +44,9 @@ Create `apps/base/bootapi/` following the argocd-apps `AGENTS.md` pattern: as `BOOTAPI_NETBOX_TOKEN_FILE` / `BOOTAPI_PROVISION_TOKEN_FILE` / `BOOTAPI_ROOT_PASSWORD_HASH_FILE` (mount the Secret). Least-privilege securityContext (`runAsNonRoot`, `drop: [all]`). Baseline resources: requests - `512Mi`/`1`, limits `2Gi`/`2` cpu. The pod needs `git` on PATH for template - sync (the distroless image includes only the static binary — either add a git - layer, use an initContainer that seeds the checkout, or fall back to a - ConfigMap; simplest is a small alpine+git base for this service). + `512Mi`/`1`, limits `2Gi`/`2` cpu. No `git` binary and no writable volume + are needed: template sync is an HTTP fetch of the repo's branch tarball, held + in memory. 6. **Service + exposure**: see the Gateway section below. 7. Register in `argocd/applicationsets/platform.yaml` (`apps/overlays/*/bootapi`) and the platform AppProject destinations. diff --git a/docs/template-authoring.md b/docs/template-authoring.md index 04cc305..eeeb1b7 100644 --- a/docs/template-authoring.md +++ b/docs/template-authoring.md @@ -8,7 +8,8 @@ bootapi ships an embedded default set and lets you override or extend it. `templates/ipxe/*.ipxe.tmpl` and `templates/catalog/*.yaml`, compiled into the binary (`templates/embed.go`). These are the always-available startup fallback. - **Template git repo** (preferred in prod): `BOOTAPI_TEMPLATE_GIT_URL`. bootapi - clones it at startup and re-pulls every `BOOTAPI_TEMPLATE_GIT_INTERVAL` + fetches the branch tarball (`/archive/.tar.gz`) over HTTP at + startup and re-fetches every `BOOTAPI_TEMPLATE_GIT_INTERVAL` (default 3m, like argocd), atomically swapping the loaded set on change. A parse failure keeps the **last-good** set and is only logged + counted (`bootapi_template_sync_failures_total`), so a bad push can't take bootapi diff --git a/internal/config/config.go b/internal/config/config.go index d7a7e32..8989e8c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,14 +46,15 @@ type Config struct { DefaultTemplate string // --- template git-sync (preferred over TemplateDir) --- - // TemplateGitURL, when set, makes bootapi clone a templates repo and re-pull + // TemplateGitURL, when set, makes bootapi fetch a templates repo's branch + // tarball over HTTP (/archive/.tar.gz) and re-fetch // it every TemplateGitInterval, atomically swapping the loaded set on change // and keeping the last-good set on a parse failure. TemplateGitURL string TemplateGitBranch string TemplateGitInterval time.Duration // TemplateGitToken is an optional token for a private templates repo, - // injected into the HTTPS clone URL. Empty for a public repo. + // sent as a Gitea token header. Empty for a public repo. TemplateGitToken string // BaseURL is the http:// base PXE clients use to reach bootapi. It is baked diff --git a/internal/gitsync/gitsync.go b/internal/gitsync/gitsync.go index f411cd0..e706fef 100644 --- a/internal/gitsync/gitsync.go +++ b/internal/gitsync/gitsync.go @@ -1,42 +1,63 @@ -// Package gitsync keeps bootapi's template Set in step with a git repo. It -// clones the templates repo at startup and re-pulls it every interval (default -// 3m, like argocd), atomically swapping the Engine's active Set when the repo -// changes. A parse failure keeps the last-good Set and is only logged/counted, -// so a bad template push can never take bootapi down. The embedded defaults -// remain the fallback when git is unreachable at startup. +// Package gitsync keeps bootapi's template Set in step with the templates repo. +// It fetches the repo's branch tarball over plain HTTP (Gitea's +// /archive/.tar.gz) every interval (default 3m, like argocd), holds the +// template files in memory, and atomically swaps the Engine's active Set when +// the content changes. A parse failure keeps the last-good Set and is only +// logged/counted, so a bad template push can never take bootapi down. The +// embedded defaults remain the fallback when the repo is unreachable at startup. +// +// The repo is only ever read as a file tree plus a change signal, so no git +// binary is involved: the runtime image stays distroless and the pod needs no +// writable volume. package gitsync import ( + "archive/tar" + "compress/gzip" "context" + "crypto/sha256" + "encoding/hex" "fmt" + "io" "io/fs" "log/slog" - "os" - "os/exec" + "net/http" + "path" + "sort" "strings" "sync/atomic" + "testing/fstest" "time" "git.unkin.net/unkin/bootapi/internal/render" ) +// maxArchiveBytes caps the downloaded tarball. The templates repo is a handful +// of text files (~12KB compressed); this only exists to bound a hostile or +// broken response. +const maxArchiveBytes = 32 << 20 + +// maxFileBytes caps a single extracted template. +const maxFileBytes = 1 << 20 + // Options configures the syncer. type Options struct { - URL string + URL string // repo URL, e.g. https://git.unkin.net/unkin/bootapi-templates.git Branch string - Token string // optional; injected into the HTTPS URL for a private repo + Token string // optional; sent as a Gitea token header for a private repo Interval time.Duration - WorkDir string // local checkout path + Client *http.Client // optional; defaults to a 30s-timeout client } -// Syncer pulls a templates repo and reloads an Engine on change. +// Syncer fetches a templates repo and reloads an Engine on change. type Syncer struct { opt Options embedded fs.FS engine *render.Engine + digest string // content digest of the last fetched tree syncs atomic.Int64 // successful reloads (Set swapped) - failures atomic.Int64 // pull or parse failures (last-good kept) + failures atomic.Int64 // fetch or parse failures (last-good kept) generation atomic.Int64 // increments on every successful swap } @@ -50,6 +71,9 @@ func New(opt Options, embedded fs.FS) *Syncer { if opt.Interval <= 0 { opt.Interval = 3 * time.Minute } + if opt.Client == nil { + opt.Client = &http.Client{Timeout: 30 * time.Second} + } return &Syncer{opt: opt, embedded: embedded} } @@ -61,35 +85,46 @@ func (s *Syncer) Syncs() int64 { return s.syncs.Load() } func (s *Syncer) Failures() int64 { return s.failures.Load() } func (s *Syncer) Generation() int64 { return s.generation.Load() } -// Bootstrap clones the repo and builds the initial Set from embedded + the -// checkout. On any git/parse failure it returns an embedded-only Set plus a -// non-nil error (which the caller logs but treats as non-fatal, so bootapi +// ArchiveURL is the branch tarball URL derived from the repo URL. +func (o Options) ArchiveURL() string { + base := strings.TrimSuffix(strings.TrimRight(o.URL, "/"), ".git") + return base + "/archive/" + o.Branch + ".tar.gz" +} + +// Bootstrap fetches the repo and builds the initial Set from embedded + the +// fetched tree. On any fetch/parse failure it returns an embedded-only Set plus +// a non-nil error (which the caller logs but treats as non-fatal, so bootapi // always starts with at least the embedded defaults). func (s *Syncer) Bootstrap(ctx context.Context) (*render.Set, error) { - if err := s.clone(ctx); err != nil { - set, berr := render.BuildSet(s.embedded, nil) - if berr != nil { - return nil, berr // embedded defaults broken: genuinely fatal - } - return set, fmt.Errorf("git clone failed, using embedded defaults: %w", err) - } - set, err := render.BuildSet(s.embedded, os.DirFS(s.opt.WorkDir)) + tree, digest, commit, err := s.fetch(ctx) if err != nil { - emb, berr := render.BuildSet(s.embedded, nil) - if berr != nil { - return nil, berr - } - return emb, fmt.Errorf("git templates failed to parse, using embedded defaults: %w", err) + return s.embeddedSet(fmt.Errorf("template fetch failed, using embedded defaults: %w", err)) + } + s.digest = digest + set, err := render.BuildSet(s.embedded, tree) + if err != nil { + return s.embeddedSet(fmt.Errorf("fetched templates failed to parse, using embedded defaults: %w", err)) } s.generation.Add(1) + slog.Info("templates loaded", "commit", commit, "digest", digest) return set, nil } +// embeddedSet returns the embedded-only Set alongside the degrade reason. A +// broken embedded set is genuinely fatal. +func (s *Syncer) embeddedSet(reason error) (*render.Set, error) { + set, err := render.BuildSet(s.embedded, nil) + if err != nil { + return nil, err + } + return set, reason +} + // Run polls the repo every interval until ctx is cancelled. func (s *Syncer) Run(ctx context.Context) { t := time.NewTicker(s.opt.Interval) defer t.Stop() - slog.Info("template git-sync started", "url", s.opt.URL, "branch", s.opt.Branch, "interval", s.opt.Interval) + slog.Info("template sync started", "url", s.opt.ArchiveURL(), "interval", s.opt.Interval) for { select { case <-ctx.Done(): @@ -101,78 +136,133 @@ func (s *Syncer) Run(ctx context.Context) { } func (s *Syncer) pollOnce(ctx context.Context) { - changed, head, err := s.pull(ctx) + tree, digest, commit, err := s.fetch(ctx) if err != nil { s.failures.Add(1) - slog.Error("template git pull failed; keeping last-good set", "err", err) + slog.Error("template fetch failed; keeping last-good set", "err", err) return } - if !changed { + if digest == s.digest { return } - set, err := render.BuildSet(s.embedded, os.DirFS(s.opt.WorkDir)) + // Record the new digest before parsing so an unchanged bad push is counted + // once, not on every poll. + s.digest = digest + set, err := render.BuildSet(s.embedded, tree) if err != nil { s.failures.Add(1) - slog.Error("template reload failed to parse; keeping last-good set", "commit", head, "err", err) + slog.Error("template reload failed to parse; keeping last-good set", "commit", commit, "err", err) return } s.engine.Swap(set) s.syncs.Add(1) s.generation.Add(1) - slog.Info("templates reloaded from git", "commit", head, "generation", s.generation.Load()) + slog.Info("templates reloaded", "commit", commit, "digest", digest, "generation", s.generation.Load()) } -// authURL injects a token into the HTTPS clone URL when configured. -func (s *Syncer) authURL() string { - if s.opt.Token == "" { - return s.opt.URL - } - if rest, ok := strings.CutPrefix(s.opt.URL, "https://"); ok { - return "https://" + s.opt.Token + "@" + rest - } - return s.opt.URL -} - -func (s *Syncer) clone(ctx context.Context) error { - if err := os.RemoveAll(s.opt.WorkDir); err != nil { - return err - } - return run(ctx, "", "git", "clone", "--depth", "1", "--branch", s.opt.Branch, s.authURL(), s.opt.WorkDir) -} - -// pull fetches origin/branch and hard-resets to it, reporting whether HEAD moved. -func (s *Syncer) pull(ctx context.Context) (changed bool, head string, err error) { - old, _ := s.head(ctx) - if err := run(ctx, s.opt.WorkDir, "git", "fetch", "--depth", "1", "origin", s.opt.Branch); err != nil { - return false, "", err - } - if err := run(ctx, s.opt.WorkDir, "git", "reset", "--hard", "origin/"+s.opt.Branch); err != nil { - return false, "", err - } - newHead, err := s.head(ctx) +// fetch downloads the branch tarball and extracts it into an in-memory FS. The +// digest is taken over the extracted tree (not the gzip bytes) so a +// re-compressed but identical archive is not treated as a change. commit is the +// source commit Gitea advertises in its immutable Link header, for logging only. +func (s *Syncer) fetch(ctx context.Context) (fs.FS, string, string, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, s.opt.ArchiveURL(), nil) if err != nil { - return false, "", err + return nil, "", "", err } - return old != newHead, newHead, nil -} - -func (s *Syncer) head(ctx context.Context) (string, error) { - out, err := output(ctx, s.opt.WorkDir, "git", "rev-parse", "HEAD") - return strings.TrimSpace(out), err -} - -func run(ctx context.Context, dir, name string, args ...string) error { - cmd := exec.CommandContext(ctx, name, args...) - cmd.Dir = dir - if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("%s %s: %w: %s", name, strings.Join(args, " "), err, strings.TrimSpace(string(out))) + if s.opt.Token != "" { + req.Header.Set("Authorization", "token "+s.opt.Token) } - return nil + resp, err := s.opt.Client.Do(req) + if err != nil { + return nil, "", "", err + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + return nil, "", "", fmt.Errorf("GET %s: %s", s.opt.ArchiveURL(), resp.Status) + } + + tree, err := extract(io.LimitReader(resp.Body, maxArchiveBytes)) + if err != nil { + return nil, "", "", err + } + if len(tree) == 0 { + return nil, "", "", fmt.Errorf("archive contained no files") + } + return tree, digest(tree), commitFromLink(resp.Header.Get("Link")), nil } -func output(ctx context.Context, dir, name string, args ...string) (string, error) { - cmd := exec.CommandContext(ctx, name, args...) - cmd.Dir = dir - out, err := cmd.Output() - return string(out), err +// extract reads a gzipped tar and returns its regular files keyed by path with +// the archive's single top-level directory stripped (Gitea prefixes every entry +// with "/"). Entries that would escape the tree are skipped rather than +// trusted: the archive is a network input. +func extract(r io.Reader) (fstest.MapFS, error) { + gz, err := gzip.NewReader(r) + if err != nil { + return nil, fmt.Errorf("gzip: %w", err) + } + defer func() { _ = gz.Close() }() + + out := fstest.MapFS{} + tr := tar.NewReader(gz) + for { + h, err := tr.Next() + if err == io.EOF { + return out, nil + } + if err != nil { + return nil, fmt.Errorf("tar: %w", err) + } + if h.Typeflag != tar.TypeReg { + continue + } + name := stripRoot(h.Name) + if name == "" || !fs.ValidPath(name) { + continue + } + b, err := io.ReadAll(io.LimitReader(tr, maxFileBytes)) + if err != nil { + return nil, fmt.Errorf("tar %s: %w", h.Name, err) + } + out[name] = &fstest.MapFile{Data: b, Mode: 0o444} + } +} + +// stripRoot removes the archive's leading directory component. +func stripRoot(name string) string { + clean := path.Clean(strings.TrimPrefix(name, "./")) + if strings.HasPrefix(clean, "/") || strings.HasPrefix(clean, "..") { + return "" + } + _, rest, ok := strings.Cut(clean, "/") + if !ok { + return "" + } + return rest +} + +// digest hashes the extracted tree: every path and its contents, in path order. +func digest(tree fstest.MapFS) string { + names := make([]string, 0, len(tree)) + for n := range tree { + names = append(names, n) + } + sort.Strings(names) + + h := sha256.New() + for _, n := range names { + _, _ = fmt.Fprintf(h, "%s\x00%d\x00", n, len(tree[n].Data)) + _, _ = h.Write(tree[n].Data) + } + return hex.EncodeToString(h.Sum(nil))[:16] +} + +// commitFromLink pulls the commit SHA out of Gitea's immutable-archive Link +// header: <.../archive/.tar.gz?rev=>; rel="immutable". +func commitFromLink(link string) string { + _, rev, ok := strings.Cut(link, "rev=") + if !ok { + return "" + } + sha, _, _ := strings.Cut(rev, ">") + return strings.TrimSpace(sha) } diff --git a/internal/gitsync/gitsync_test.go b/internal/gitsync/gitsync_test.go index 74c2d39..0febe02 100644 --- a/internal/gitsync/gitsync_test.go +++ b/internal/gitsync/gitsync_test.go @@ -1,11 +1,14 @@ package gitsync import ( + "archive/tar" + "bytes" + "compress/gzip" "context" - "os" - "os/exec" - "path/filepath" + "net/http" + "net/http/httptest" "strings" + "sync/atomic" "testing" "time" @@ -14,31 +17,81 @@ import ( "git.unkin.net/unkin/bootapi/templates" ) -// gitRepo creates a real git repo at dir with an initial almalinux9 override. -func gitRepo(t *testing.T, dir string) { +// archive builds a gzipped tar shaped like Gitea's: every entry under a single +// "/" root, plus the directory entries Gitea includes. +func archive(t *testing.T, files map[string]string) []byte { t.Helper() - gitCmd(t, "", "git", "init", "-b", "main", dir) - gitCmd(t, dir, "git", "config", "user.email", "t@example.net") - gitCmd(t, dir, "git", "config", "user.name", "test") - writeKS(t, dir, "GITSYNC-V1 {{ .Hostname }}\n") - gitCmd(t, dir, "git", "add", "-A") - gitCmd(t, dir, "git", "commit", "-m", "v1") -} - -func writeKS(t *testing.T, dir, body string) { - t.Helper() - if err := os.WriteFile(filepath.Join(dir, "almalinux9.ks.tmpl"), []byte(body), 0o600); err != nil { + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + if err := tw.WriteHeader(&tar.Header{Name: "bootapi-templates/", Typeflag: tar.TypeDir, Mode: 0o755}); err != nil { t.Fatal(err) } + for name, body := range files { + h := &tar.Header{Name: "bootapi-templates/" + name, Typeflag: tar.TypeReg, Mode: 0o644, Size: int64(len(body))} + if err := tw.WriteHeader(h); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(body)); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + if err := gz.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() } -func gitCmd(t *testing.T, dir, name string, args ...string) { +// repo serves a mutable archive at Gitea's /archive path and counts requests. +type repo struct { + t *testing.T + srv *httptest.Server + body atomic.Value // []byte + status atomic.Int64 + fetches atomic.Int64 +} + +func newRepo(t *testing.T, files map[string]string) *repo { t.Helper() - cmd := exec.Command(name, args...) - cmd.Dir = dir - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("%s %v: %v: %s", name, args, err, out) + r := &repo{t: t} + r.body.Store(archive(t, files)) + r.status.Store(http.StatusOK) + mux := http.NewServeMux() + mux.HandleFunc("/unkin/bootapi-templates/archive/main.tar.gz", func(w http.ResponseWriter, _ *http.Request) { + r.fetches.Add(1) + if code := int(r.status.Load()); code != http.StatusOK { + w.WriteHeader(code) + return + } + w.Header().Set("Link", `; rel="immutable"`) + _, _ = w.Write(r.body.Load().([]byte)) + }) + r.srv = httptest.NewServer(mux) + t.Cleanup(r.srv.Close) + return r +} + +func (r *repo) url() string { return r.srv.URL + "/unkin/bootapi-templates.git" } +func (r *repo) push(files map[string]string) { r.body.Store(archive(r.t, files)) } +func ks(body string) map[string]string { return map[string]string{"almalinux9.ks.tmpl": body} } + +func syncer(t *testing.T, r *repo) *Syncer { + t.Helper() + return New(Options{URL: r.url(), Branch: "main", Interval: time.Hour}, templates.FS) +} + +func engineFor(t *testing.T, s *Syncer) *render.Engine { + t.Helper() + set, err := s.Bootstrap(context.Background()) + if err != nil { + t.Fatalf("Bootstrap: %v", err) } + eng := render.NewEngine(render.RenderConfig{DefaultTemplate: "almalinux9", ArtifactBase: "https://af"}, set) + s.SetEngine(eng) + return eng } func renderKS(t *testing.T, e *render.Engine) string { @@ -51,33 +104,32 @@ func renderKS(t *testing.T, e *render.Engine) string { return string(out) } +func TestArchiveURL(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {"https://git.unkin.net/unkin/bootapi-templates.git", "https://git.unkin.net/unkin/bootapi-templates/archive/main.tar.gz"}, + {"https://git.unkin.net/unkin/bootapi-templates", "https://git.unkin.net/unkin/bootapi-templates/archive/main.tar.gz"}, + {"https://git.unkin.net/unkin/bootapi-templates/", "https://git.unkin.net/unkin/bootapi-templates/archive/main.tar.gz"}, + } { + if got := (Options{URL: tc.in, Branch: "main"}).ArchiveURL(); got != tc.want { + t.Errorf("ArchiveURL(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + func TestBootstrapAndReload(t *testing.T) { - if _, err := exec.LookPath("git"); err != nil { - t.Skip("git not available") - } - src := t.TempDir() - gitRepo(t, src) + r := newRepo(t, ks("SYNC-V1 {{ .Hostname }}\n")) + s := syncer(t, r) + eng := engineFor(t, s) - s := New(Options{URL: src, Branch: "main", Interval: time.Hour, WorkDir: filepath.Join(t.TempDir(), "co")}, templates.FS) - set, err := s.Bootstrap(context.Background()) - if err != nil { - t.Fatalf("Bootstrap: %v", err) - } - eng := render.NewEngine(render.RenderConfig{DefaultTemplate: "almalinux9", ArtifactBase: "https://af"}, set) - s.SetEngine(eng) - - if got := renderKS(t, eng); !contains(got, "GITSYNC-V1 web01") { + if got := renderKS(t, eng); !strings.Contains(got, "SYNC-V1 web01") { t.Fatalf("initial render missing v1 override:\n%s", got) } gen1 := s.Generation() - // Commit v2 upstream, then poll: the engine must swap to the new content. - writeKS(t, src, "GITSYNC-V2 {{ .Hostname }}\n") - gitCmd(t, src, "git", "add", "-A") - gitCmd(t, src, "git", "commit", "-m", "v2") - + r.push(ks("SYNC-V2 {{ .Hostname }}\n")) s.pollOnce(context.Background()) - if got := renderKS(t, eng); !contains(got, "GITSYNC-V2 web01") { + + if got := renderKS(t, eng); !strings.Contains(got, "SYNC-V2 web01") { t.Fatalf("after reload, render missing v2:\n%s", got) } if s.Generation() <= gen1 { @@ -88,30 +140,33 @@ func TestBootstrapAndReload(t *testing.T) { } } -func TestReloadKeepsLastGoodOnParseError(t *testing.T) { - if _, err := exec.LookPath("git"); err != nil { - t.Skip("git not available") - } - src := t.TempDir() - gitRepo(t, src) - - s := New(Options{URL: src, Branch: "main", Interval: time.Hour, WorkDir: filepath.Join(t.TempDir(), "co")}, templates.FS) - set, err := s.Bootstrap(context.Background()) - if err != nil { - t.Fatal(err) - } - eng := render.NewEngine(render.RenderConfig{DefaultTemplate: "almalinux9", ArtifactBase: "https://af"}, set) - s.SetEngine(eng) - - // Push a template that fails to parse. - writeKS(t, src, "BROKEN {{ .Hostname \n") - gitCmd(t, src, "git", "add", "-A") - gitCmd(t, src, "git", "commit", "-m", "broken") +func TestUnchangedContentDoesNotReload(t *testing.T) { + r := newRepo(t, ks("SYNC-V1 {{ .Hostname }}\n")) + s := syncer(t, r) + engineFor(t, s) + // Re-archiving the same files yields fresh gzip bytes; the digest is taken + // over the extracted tree, so this must NOT count as a change. + r.push(ks("SYNC-V1 {{ .Hostname }}\n")) s.pollOnce(context.Background()) - // The last-good v1 set must still be served, and a failure recorded. - if got := renderKS(t, eng); !contains(got, "GITSYNC-V1 web01") { + if s.Syncs() != 0 { + t.Errorf("syncs = %d, want 0 (identical content is not a change)", s.Syncs()) + } + if s.Failures() != 0 { + t.Errorf("failures = %d, want 0", s.Failures()) + } +} + +func TestReloadKeepsLastGoodOnParseError(t *testing.T) { + r := newRepo(t, ks("SYNC-V1 {{ .Hostname }}\n")) + s := syncer(t, r) + eng := engineFor(t, s) + + r.push(ks("BROKEN {{ .Hostname \n")) + s.pollOnce(context.Background()) + + if got := renderKS(t, eng); !strings.Contains(got, "SYNC-V1 web01") { t.Fatalf("last-good not kept after parse failure:\n%s", got) } if s.Failures() != 1 { @@ -120,11 +175,19 @@ func TestReloadKeepsLastGoodOnParseError(t *testing.T) { if s.Syncs() != 0 { t.Errorf("syncs = %d, want 0 (bad push must not count as a sync)", s.Syncs()) } + + // The same bad content on the next poll must not be counted again. + s.pollOnce(context.Background()) + if s.Failures() != 1 { + t.Errorf("failures = %d, want 1 (an unchanged bad push is counted once)", s.Failures()) + } } func TestBootstrapDegradesToEmbedded(t *testing.T) { - // A bogus URL must not fail startup: Bootstrap returns the embedded set. - s := New(Options{URL: "/nonexistent/repo", Branch: "main", Interval: time.Hour, WorkDir: filepath.Join(t.TempDir(), "co")}, templates.FS) + r := newRepo(t, ks("SYNC-V1 {{ .Hostname }}\n")) + r.status.Store(http.StatusNotFound) + s := syncer(t, r) + set, err := s.Bootstrap(context.Background()) if err == nil { t.Error("expected a non-nil (non-fatal) error describing the degrade") @@ -133,10 +196,43 @@ func TestBootstrapDegradesToEmbedded(t *testing.T) { t.Fatal("expected the embedded fallback Set, got nil") } eng := render.NewEngine(render.RenderConfig{DefaultTemplate: "almalinux9", ArtifactBase: "https://af"}, set) - // Embedded almalinux9 template still renders. - if got := renderKS(t, eng); !contains(got, "rootpw") { + if got := renderKS(t, eng); !strings.Contains(got, "rootpw") { t.Errorf("embedded fallback did not render a real kickstart:\n%s", got) } } -func contains(s, sub string) bool { return strings.Contains(s, sub) } +func TestExtractStripsRootAndSkipsEscapes(t *testing.T) { + tree, err := extract(bytes.NewReader(archive(t, map[string]string{ + "catalog/almalinux9.yaml": "name: almalinux9\n", + "../escape.ks.tmpl": "nope\n", + }))) + if err != nil { + t.Fatalf("extract: %v", err) + } + if _, ok := tree["catalog/almalinux9.yaml"]; !ok { + t.Errorf("root not stripped; got keys %v", keys(tree)) + } + for k := range tree { + if strings.Contains(k, "escape") { + t.Errorf("traversal entry was kept: %q", k) + } + } +} + +func TestCommitFromLink(t *testing.T) { + link := `; rel="immutable"` + if got := commitFromLink(link); got != "5df1894" { + t.Errorf("commitFromLink = %q, want 5df1894", got) + } + if got := commitFromLink(""); got != "" { + t.Errorf("commitFromLink(\"\") = %q, want empty", got) + } +} + +func keys[V any](m map[string]V) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +}