From 9919676836a4e478195c346fe607eeb7c1ec5a67 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 12 Sep 2026 16:22:27 +1000 Subject: [PATCH] catch four more class reference forms A params.pp is only ever reached by inherits and $class::var, so changing one resolved to no hosts at all. - index inherits, $class::var reads and create_resources() - match a defined type declared part-way along a line --- README.md | 3 ++- internal/impact/index.go | 17 ++++++++++------ internal/impact/resolve_test.go | 20 +++++++++++++++++++ .../repo/site/profiles/manifests/created.pp | 2 ++ .../repo/site/profiles/manifests/inherited.pp | 2 ++ .../site/profiles/manifests/inline/thing.pp | 2 ++ .../repo/site/profiles/manifests/leaf.pp | 8 ++++++++ .../repo/site/profiles/manifests/mid.pp | 2 +- .../repo/site/profiles/manifests/scopevar.pp | 3 +++ 9 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 internal/impact/testdata/repo/site/profiles/manifests/created.pp create mode 100644 internal/impact/testdata/repo/site/profiles/manifests/inherited.pp create mode 100644 internal/impact/testdata/repo/site/profiles/manifests/inline/thing.pp create mode 100644 internal/impact/testdata/repo/site/profiles/manifests/scopevar.pp diff --git a/README.md b/README.md index 32ea494..bfc9f98 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,8 @@ pluginsynced to every agent and is treated the same way. Finding what applies a class means searching two places, not one: 1. `.pp` files under `site/` and `modules/` for `include`, `contain`, - `class { '...': }`, capitalised resource references and defined-type usage. + `inherits`, `class { '...': }`, capitalised resource references, + `create_resources()`, `$class::var` reads and defined-type usage. 2. `hieradata/**/*.{yaml,eyaml}` for the class as a list item under `hiera_include:`. `profiles::base` declares `($hiera_include - $hiera_exclude).include` with a `unique` merge across the diff --git a/internal/impact/index.go b/internal/impact/index.go index c60dc75..bb39396 100644 --- a/internal/impact/index.go +++ b/internal/impact/index.go @@ -17,11 +17,16 @@ import ( var manifestRoots = []string{"site", "modules"} var ( - commentRe = regexp.MustCompile(`(?m)#.*$`) - includeRe = regexp.MustCompile(`\b(?:include|contain|require)\s*\(?\s*['"]?(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)['"]?`) - classParamRe = regexp.MustCompile(`\bclass\s*\{\s*['"](?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)['"]`) - classRefRe = regexp.MustCompile(`\bClass\s*\[\s*['"](?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)['"]`) - defineUseRe = regexp.MustCompile(`(?m)^\s*(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)+)\s*\{`) + commentRe = regexp.MustCompile(`(?m)#.*$`) + includeRe = regexp.MustCompile(`\b(?:include|contain|require|inherits)\s*\(?\s*['"]?(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)['"]?`) + classParamRe = regexp.MustCompile(`\bclass\s*\{\s*['"](?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)['"]`) + classRefRe = regexp.MustCompile(`\bClass\s*\[\s*['"](?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)['"]`) + defineUseRe = regexp.MustCompile(`(?m)(?:^|[^\w:$.])(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)+)\s*\{`) + // $mod::params::thing reads a class variable, which is a dependency on the + // class that declares it. + scopeVarRe = regexp.MustCompile(`\$(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)+)::[a-z_][a-z0-9_]*`) + // create_resources('mod::type', ...) names a defined type in a string. + createResRe = regexp.MustCompile(`create_resources\s*\(\s*['"](?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)['"]`) resourceRefRe = regexp.MustCompile(`\b([A-Z][a-zA-Z0-9_]*(?:::[A-Z][a-zA-Z0-9_]*)+)\s*\[`) assetRe = regexp.MustCompile(`(?:template|epp|file)\s*\(\s*['"]([a-z0-9_]+/[^'"]+)['"]|puppet:///modules/([a-z0-9_]+/[^'"\s]+)`) hieraBlockRe = regexp.MustCompile(`(?m)^(hiera_include|hiera_exclude):\s*$((?:\n\s+-\s*\S+)*)`) @@ -153,7 +158,7 @@ func references(body string) []string { seen[name] = true } } - for _, re := range []*regexp.Regexp{includeRe, classParamRe, classRefRe, defineUseRe} { + for _, re := range []*regexp.Regexp{includeRe, classParamRe, classRefRe, defineUseRe, scopeVarRe, createResRe} { for _, m := range re.FindAllStringSubmatch(body, -1) { add(m[1]) } diff --git a/internal/impact/resolve_test.go b/internal/impact/resolve_test.go index 37f61d5..b800af0 100644 --- a/internal/impact/resolve_test.go +++ b/internal/impact/resolve_test.go @@ -123,3 +123,23 @@ func TestParseHieraListsRegexFallback(t *testing.T) { t.Errorf("exclude = %v", exclude) } } + +func TestResolveInheritsScopeVarCreateResourcesAndInlineDefine(t *testing.T) { + // Four reference forms a plain include grep misses: class inheritance, + // a $class::var read, create_resources() naming a type in a string, and a + // defined type declared part-way along a line. + tests := map[string]string{ + "inherits": "profiles::inherited", + "scope variable": "profiles::scopevar", + "create_resources": "profiles::created", + "inline define": "profiles::inline::thing", + } + want := []string{"k8s1", "k8s2", "web1"} + for name, class := range tests { + t.Run(name, func(t *testing.T) { + if got := resolvedHosts(t, class); !reflect.DeepEqual(got, want) { + t.Errorf("%s = %v, want %v", class, got, want) + } + }) + } +} diff --git a/internal/impact/testdata/repo/site/profiles/manifests/created.pp b/internal/impact/testdata/repo/site/profiles/manifests/created.pp new file mode 100644 index 0000000..c89c36f --- /dev/null +++ b/internal/impact/testdata/repo/site/profiles/manifests/created.pp @@ -0,0 +1,2 @@ +define profiles::created () { +} diff --git a/internal/impact/testdata/repo/site/profiles/manifests/inherited.pp b/internal/impact/testdata/repo/site/profiles/manifests/inherited.pp new file mode 100644 index 0000000..e9f62d3 --- /dev/null +++ b/internal/impact/testdata/repo/site/profiles/manifests/inherited.pp @@ -0,0 +1,2 @@ +class profiles::inherited { +} diff --git a/internal/impact/testdata/repo/site/profiles/manifests/inline/thing.pp b/internal/impact/testdata/repo/site/profiles/manifests/inline/thing.pp new file mode 100644 index 0000000..01b88f5 --- /dev/null +++ b/internal/impact/testdata/repo/site/profiles/manifests/inline/thing.pp @@ -0,0 +1,2 @@ +define profiles::inline::thing () { +} diff --git a/internal/impact/testdata/repo/site/profiles/manifests/leaf.pp b/internal/impact/testdata/repo/site/profiles/manifests/leaf.pp index 16e54dc..56ad986 100644 --- a/internal/impact/testdata/repo/site/profiles/manifests/leaf.pp +++ b/internal/impact/testdata/repo/site/profiles/manifests/leaf.pp @@ -1,4 +1,12 @@ class profiles::leaf { + $copied = $profiles::scopevar::thing + create_resources('profiles::created', {}) + + case $copied { + 'x': { profiles::inline::thing { 'mid-line': } } + default: {} + } + file { '/tmp/leaf': content => template('profiles/leaf/x.erb'), source => 'puppet:///modules/profiles/leaf/y.txt', diff --git a/internal/impact/testdata/repo/site/profiles/manifests/mid.pp b/internal/impact/testdata/repo/site/profiles/manifests/mid.pp index 5e54151..3654ce8 100644 --- a/internal/impact/testdata/repo/site/profiles/manifests/mid.pp +++ b/internal/impact/testdata/repo/site/profiles/manifests/mid.pp @@ -1,3 +1,3 @@ -class profiles::mid { +class profiles::mid inherits profiles::inherited { include profiles::leaf } diff --git a/internal/impact/testdata/repo/site/profiles/manifests/scopevar.pp b/internal/impact/testdata/repo/site/profiles/manifests/scopevar.pp new file mode 100644 index 0000000..2d68546 --- /dev/null +++ b/internal/impact/testdata/repo/site/profiles/manifests/scopevar.pp @@ -0,0 +1,3 @@ +class profiles::scopevar { + $thing = 'x' +}