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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user