diff --git a/internal/impact/index.go b/internal/impact/index.go index bb39396..80ed921 100644 --- a/internal/impact/index.go +++ b/internal/impact/index.go @@ -17,10 +17,16 @@ import ( var manifestRoots = []string{"site", "modules"} var ( - 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_]+)*)['"]`) + commentRe = regexp.MustCompile(`(?m)#.*$`) + // include/contain/require are functions taking a list of names; the + // argument list is parsed rather than matched so every name is seen. + declFuncRe = regexp.MustCompile(`(?:^|[^\w:$.])(?:include|contain|require)`) + // inherits takes exactly one name, so a list parse does not apply. + inheritsRe = regexp.MustCompile(`\binherits\s+(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)*)`) + classParamRe = regexp.MustCompile(`\bclass\s*\{`) + classRefRe = regexp.MustCompile(`\bClass\s*\[`) + classNameRe = regexp.MustCompile(`^(?:::)?[a-z][a-z0-9_]*(?:::[a-z0-9_]+)*$`) + typeNameRe = regexp.MustCompile(`^(?:::)?[A-Z][a-zA-Z0-9_]*(?:::[A-Z][a-zA-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. @@ -158,9 +164,36 @@ func references(body string) []string { seen[name] = true } } - for _, re := range []*regexp.Regexp{includeRe, classParamRe, classRefRe, defineUseRe, scopeVarRe, createResRe} { + + // include/contain/require: every name in the argument list. + for _, loc := range declFuncRe.FindAllStringIndex(body, -1) { + args, ok := callArguments(body, loc[1]) + if !ok { + continue + } + for _, name := range classNames(args) { + add(name) + } + } + // class { ['a', 'b']: } declares one resource per title. + for _, loc := range classParamRe.FindAllStringIndex(body, -1) { + for _, name := range classNames(resourceTitles(body, loc[1])) { + add(name) + } + } + // Class['a', 'b'] is an ordering reference to each named class. + for _, loc := range classRefRe.FindAllStringIndex(body, -1) { + inner, _, ok := balanced(body, loc[1]-1, '[', ']') + if !ok { + continue + } + for _, name := range classNames(inner) { + add(name) + } + } + for _, re := range []*regexp.Regexp{inheritsRe, defineUseRe, scopeVarRe, createResRe} { for _, m := range re.FindAllStringSubmatch(body, -1) { - add(m[1]) + add(strings.TrimPrefix(m[1], "::")) } } // Resource references (Profiles::Foo::Bar['x']) name a defined type in @@ -177,6 +210,165 @@ func references(body string) []string { return out } +// callArguments returns the argument text of the function call whose name ends +// at i, in either the parenthesised or the bare statement form. It reports +// false when the keyword is not a call at all, such as a require => attribute. +func callArguments(s string, i int) (string, bool) { + if i >= len(s) { + return "", false + } + if s[i] != '(' && s[i] != ' ' && s[i] != '\t' { + return "", false + } + for i < len(s) && (s[i] == ' ' || s[i] == '\t') { + i++ + } + if i >= len(s) { + return "", false + } + if s[i] == '(' { + inner, _, ok := balanced(s, i, '(', ')') + return inner, ok + } + if s[i] == '=' || s[i] == ',' || s[i] == ';' || s[i] == '\n' { + return "", false + } + depth := 0 + for j := i; j < len(s); j++ { + switch c := s[j]; { + case c == '\'' || c == '"': + j = skipString(s, j) - 1 + case c == '(' || c == '[' || c == '{': + depth++ + case c == ')' || c == ']' || c == '}': + if depth == 0 { + return s[i:j], true + } + depth-- + case c == ';' && depth == 0: + return s[i:j], true + case c == '\n' && depth == 0 && !continued(s[i:j]): + return s[i:j], true + } + } + return s[i:], true +} + +// classNames pulls every class name out of a comma-separated argument list, +// descending into array literals and unwrapping quotes. +func classNames(args string) []string { + var out []string + for _, arg := range splitArgs(args) { + arg = strings.TrimSpace(arg) + if arg == "" { + continue + } + if arg[0] == '[' { + if inner, _, ok := balanced(arg, 0, '[', ']'); ok { + out = append(out, classNames(inner)...) + } + continue + } + if len(arg) > 1 && (arg[0] == '\'' || arg[0] == '"') && arg[len(arg)-1] == arg[0] { + arg = arg[1 : len(arg)-1] + } + switch { + case classNameRe.MatchString(arg): + out = append(out, strings.TrimPrefix(arg, "::")) + case typeNameRe.MatchString(arg): + // Class[Apache::Ssl] names the class in type form. + out = append(out, strings.ToLower(strings.TrimPrefix(arg, "::"))) + } + } + return out +} + +// splitArgs splits an argument list on its top-level commas. +func splitArgs(s string) []string { + var out []string + depth, start := 0, 0 + for j := 0; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case '(', '[', '{': + depth++ + case ')', ']', '}': + depth-- + case ',': + if depth == 0 { + out = append(out, s[start:j]) + start = j + 1 + } + } + } + return append(out, s[start:]) +} + +// resourceTitles returns the title expression of a resource body whose opening +// brace ends at i, i.e. everything up to the colon that closes the title. +func resourceTitles(s string, i int) string { + depth := 0 + for j := i; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case '(', '[', '{': + depth++ + case ')', ']', '}': + if depth == 0 { + return "" + } + depth-- + case ':': + if depth == 0 { + return s[i:j] + } + } + } + return "" +} + +// balanced returns the text between the delimiter at i and its match. +func balanced(s string, i int, opener, closer byte) (string, int, bool) { + depth := 0 + for j := i; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case opener: + depth++ + case closer: + depth-- + if depth == 0 { + return s[i+1 : j], j + 1, true + } + } + } + return "", len(s), false +} + +// skipString returns the index just past the quoted string starting at i. +func skipString(s string, i int) int { + quote := s[i] + for j := i + 1; j < len(s); j++ { + switch s[j] { + case '\\': + j++ + case quote: + return j + 1 + } + } + return len(s) +} + +// continued reports whether an argument list so far ends on a comma, meaning +// the list carries on over the newline. +func continued(s string) bool { + s = strings.TrimRight(s, " \t\r") + return strings.HasSuffix(s, ",") +} + func assetReferences(body string) []string { var out []string for _, m := range assetRe.FindAllStringSubmatch(body, -1) { diff --git a/internal/impact/index_test.go b/internal/impact/index_test.go new file mode 100644 index 0000000..bf680c6 --- /dev/null +++ b/internal/impact/index_test.go @@ -0,0 +1,131 @@ +package impact + +import ( + "reflect" + "testing" +) + +// refs runs a manifest body through the same comment stripping addManifest +// does, then extracts its references. +func refs(body string) []string { + return references(commentRe.ReplaceAllString(body, "")) +} + +func TestReferencesReadsWholeArgumentList(t *testing.T) { + tests := []struct { + name string + body string + want []string + }{ + { + name: "bare list", + body: "class p {\n include foo, bar, baz\n}\n", + want: []string{"bar", "baz", "foo"}, + }, + { + name: "top scope names", + body: "class p {\n include ::foo::bar, ::baz\n}\n", + want: []string{"baz", "foo::bar"}, + }, + { + name: "require function", + body: "class p {\n require foo, bar\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "contain", + body: "class p {\n contain foo, bar\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "parenthesised", + body: "class p {\n include(foo, bar)\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "quoted names", + body: "class p {\n include 'foo::bar', \"baz::qux\"\n}\n", + want: []string{"baz::qux", "foo::bar"}, + }, + { + name: "multi line with trailing comma", + body: "class p {\n include(\n foo::a,\n foo::b,\n )\n}\n", + want: []string{"foo::a", "foo::b"}, + }, + { + name: "multi line statement form", + body: "class p {\n include foo::a,\n foo::b\n}\n", + want: []string{"foo::a", "foo::b"}, + }, + { + name: "array argument", + body: "class p {\n include [foo, bar]\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "class resource title list", + body: "class p {\n class { ['foo', 'bar']: }\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "class reference title list", + body: "class p {\n Class['foo'] -> Class['bar', 'baz']\n}\n", + want: []string{"bar", "baz", "foo"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := refs(tc.body); !reflect.DeepEqual(got, tc.want) { + t.Errorf("references = %v, want %v", got, tc.want) + } + }) + } +} + +func TestReferencesStopsAtStatementEnd(t *testing.T) { + tests := []struct { + name string + body string + want []string + }{ + { + name: "next statement is not swallowed", + body: "class p {\n include foo\n notify { 'x': }\n include bar\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "trailing comment is not a name", + body: "class p {\n include foo, bar # baz, qux\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "block close ends the list", + body: "class p {\n if $x { include foo }\n include bar\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "require attribute is not a call", + body: "class p {\n service { 'x':\n require => Class['foo'],\n }\n include bar\n}\n", + want: []string{"bar", "foo"}, + }, + { + name: "semicolon ends the list", + body: "class p {\n include foo; include bar\n}\n", + want: []string{"bar", "foo"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := refs(tc.body); !reflect.DeepEqual(got, tc.want) { + t.Errorf("references = %v, want %v", got, tc.want) + } + }) + } +} + +func TestReferencesInheritsTakesOneName(t *testing.T) { + // inherits is not a function and never takes a list. + if got, want := refs("class p inherits foo::bar {\n}\n"), []string{"foo::bar"}; !reflect.DeepEqual(got, want) { + t.Errorf("references = %v, want %v", got, want) + } +} diff --git a/internal/impact/resolve_test.go b/internal/impact/resolve_test.go index b800af0..4411692 100644 --- a/internal/impact/resolve_test.go +++ b/internal/impact/resolve_test.go @@ -143,3 +143,14 @@ func TestResolveInheritsScopeVarCreateResourcesAndInlineDefine(t *testing.T) { }) } } + +func TestResolveEveryNameInAnIncludeList(t *testing.T) { + // profiles::base pulls both classes in with one comma-separated include; + // the second name must not be dropped. + want := []string{"k8s1", "k8s2", "web1"} + for _, class := range []string{"profiles::listed::a", "profiles::listed::b"} { + 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/base.pp b/internal/impact/testdata/repo/site/profiles/manifests/base.pp index bbe4146..56da530 100644 --- a/internal/impact/testdata/repo/site/profiles/manifests/base.pp +++ b/internal/impact/testdata/repo/site/profiles/manifests/base.pp @@ -4,5 +4,6 @@ class profiles::base { contain profiles::contained class { 'profiles::viaclass': } include profiles::cycle::a + include profiles::listed::a, profiles::listed::b profiles::defined::thing { 'x': } } diff --git a/internal/impact/testdata/repo/site/profiles/manifests/listed/a.pp b/internal/impact/testdata/repo/site/profiles/manifests/listed/a.pp new file mode 100644 index 0000000..9472e7f --- /dev/null +++ b/internal/impact/testdata/repo/site/profiles/manifests/listed/a.pp @@ -0,0 +1,2 @@ +class profiles::listed::a { +} diff --git a/internal/impact/testdata/repo/site/profiles/manifests/listed/b.pp b/internal/impact/testdata/repo/site/profiles/manifests/listed/b.pp new file mode 100644 index 0000000..6d70de6 --- /dev/null +++ b/internal/impact/testdata/repo/site/profiles/manifests/listed/b.pp @@ -0,0 +1,2 @@ +class profiles::listed::b { +}