From 9ff9edefa88cd535d6946cff44df07a54465e548 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 12 Sep 2026 16:26:29 +1000 Subject: [PATCH] report changed files that reach no host A hieradata file for a role with no members produces an empty host list, which is indistinguishable from a mapping failure. - list files that mapped cleanly but selected nobody in --explain --- README.md | 3 +++ internal/impact/impact.go | 21 ++++++++++++++++++++- internal/impact/impact_test.go | 31 +++++++++++++++++++++++++++++++ internal/impact/render.go | 3 +++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index bfc9f98..8d5b79b 100644 --- a/README.md +++ b/README.md @@ -231,6 +231,9 @@ A missed host means an unvalidated catalog, so every ambiguity widens: Paths outside the Puppet modulepath — `.woodpecker/`, `doc/`, `spec/`, `vendor/`, `Gemfile`, `*.md` — cannot reach a catalog and are skipped. +`--explain` also lists changed files that mapped cleanly but reached no host, +which normally means the role they belong to has no members. + Two things the tool cannot see: - A class nothing declares and no `hiera_include` lists reaches no hosts. That diff --git a/internal/impact/impact.go b/internal/impact/impact.go index 3856e33..3f79272 100644 --- a/internal/impact/impact.go +++ b/internal/impact/impact.go @@ -1,6 +1,9 @@ package impact -import "sort" +import ( + "slices" + "sort" +) // Reason explains why one changed file put one host in the result. type Reason struct { @@ -37,6 +40,9 @@ type Result struct { // OrphanClasses are classes a changed file defines that nothing declares // and no hieradata file pulls in. OrphanClasses []string `json:"-"` + // EmptyFiles are changed files that mapped cleanly but reached no host, + // which usually means the role they belong to has no members. + EmptyFiles []string `json:"-"` // Truncated is set when --limit cut the host list. Truncated bool `json:"-"` // TotalHosts is the estate size the selectors were evaluated against. @@ -66,6 +72,8 @@ func (a *Analyzer) Analyze(base, branch string, files []string, limit int) *Resu reasons := map[string][]Reason{} orphans := map[string]bool{} + // hitFiles records which changed files put at least one host in the result. + hitFiles := map[string]bool{} result := &Result{ Base: base, Branch: branch, @@ -90,6 +98,7 @@ func (a *Analyzer) Analyze(base, branch string, files []string, limit int) *Resu for _, certname := range t.sel.Select(a.Nodes) { reasons[certname] = append(reasons[certname], Reason{File: file, Rule: t.rule, Detail: t.detail}) + hitFiles[file] = true } } @@ -103,11 +112,21 @@ func (a *Analyzer) Analyze(base, branch string, files []string, limit int) *Resu for _, h := range hits { reasons[certname] = append(reasons[certname], Reason{File: file, Rule: h.rule, Detail: ref.why + "; " + h.detail}) + hitFiles[file] = true } } } } + // A file that mapped cleanly but reached nobody usually belongs to a role + // with no members. That is a real answer, not a failure, but it is worth + // showing rather than leaving as silence. + for _, file := range result.ChangedFiles { + if !hitFiles[file] && !slices.Contains(result.IgnoredFiles, file) { + result.EmptyFiles = append(result.EmptyFiles, file) + } + } + for name := range orphans { result.OrphanClasses = append(result.OrphanClasses, name) } diff --git a/internal/impact/impact_test.go b/internal/impact/impact_test.go index 03e29ef..ae5bf30 100644 --- a/internal/impact/impact_test.go +++ b/internal/impact/impact_test.go @@ -254,3 +254,34 @@ func TestLoadNodesRejectsEmptyEstate(t *testing.T) { t.Fatal("expected an error for an empty estate") } } + +func TestAnalyzeReportsFilesThatReachNoHost(t *testing.T) { + // A hieradata file for a role nothing is assigned to maps cleanly and + // still selects nobody; so does an orphan class. + r := analyze(t, "hieradata/roles/infra/nosuch.yaml", "modules/orphanmod/manifests/init.pp", ".woodpecker/test.yaml") + want := []string{"hieradata/roles/infra/nosuch.yaml", "modules/orphanmod/manifests/init.pp"} + if !reflect.DeepEqual(r.EmptyFiles, want) { + t.Errorf("EmptyFiles = %v, want %v", r.EmptyFiles, want) + } + if len(r.Hosts) != 0 { + t.Errorf("hosts = %v", r.Certnames()) + } +} + +func TestAnalyzeDoesNotCallAHittingFileEmpty(t *testing.T) { + r := analyze(t, "hieradata/roles/apps/media/jellyfin.yaml", "hieradata/roles/infra/nosuch.yaml") + if got, want := r.EmptyFiles, []string{"hieradata/roles/infra/nosuch.yaml"}; !reflect.DeepEqual(got, want) { + t.Errorf("EmptyFiles = %v, want %v", got, want) + } +} + +func TestRenderExplainListsFilesThatReachNoHost(t *testing.T) { + var buf bytes.Buffer + if err := RenderExplain(&buf, analyze(t, "hieradata/roles/infra/nosuch.yaml")); err != nil { + t.Fatalf("RenderExplain: %v", err) + } + if !strings.Contains(buf.String(), "reach no host") || + !strings.Contains(buf.String(), "hieradata/roles/infra/nosuch.yaml") { + t.Errorf("explain output = %q", buf.String()) + } +} diff --git a/internal/impact/render.go b/internal/impact/render.go index 9558c4a..eac1a12 100644 --- a/internal/impact/render.go +++ b/internal/impact/render.go @@ -63,6 +63,9 @@ func RenderExplain(w io.Writer, r *Result) error { if err := section(w, "ignored files (outside the Puppet modulepath)", r.IgnoredFiles); err != nil { return err } + if err := section(w, "changed files that reach no host (the role they belong to has no members)", r.EmptyFiles); err != nil { + return err + } if err := section(w, "orphan classes (nothing declares them and no hiera_include lists them)", r.OrphanClasses); err != nil { return err }