Report unused variables.

pkg/expansion/expand.go

@@ -36,11 +36,14 @@ func syntaxWrap(input string) string {
 // implements the expansion semantics defined in the expansion spec; it
 // returns the input string wrapped in the expansion syntax if no mapping
 // for the input is found.
-func MappingFuncFor(context ...map[string]string) func(string) string {
+func MappingFuncFor(
+	counts map[string]int,
+	context ...map[string]string) func(string) string {
 	return func(input string) string {
 		for _, vars := range context {
 			val, ok := vars[input]
 			if ok {
+				counts[input]++
 				return val
 			}
 		}

pkg/expansion/expand_test.go

@@ -14,12 +14,19 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package expansion
+package expansion_test
 
 import (
 	"testing"
+
+	. "sigs.k8s.io/kustomize/pkg/expansion"
 )
 
+type expected struct {
+	count  int
+	edited string
+}
+
 func TestMapReference(t *testing.T) {
 	type env struct {
 		Name  string
@@ -46,21 +53,23 @@ func TestMapReference(t *testing.T) {
 		"BLU": "$(ZOO)-2",
 	}
 
-	mapping := MappingFuncFor(declaredEnv)
+	counts := make(map[string]int)
+	mapping := MappingFuncFor(counts, declaredEnv)
 
 	for _, env := range envs {
 		declaredEnv[env.Name] = Expand(env.Value, mapping)
 	}
 
-	expectedEnv := map[string]string{
+	expectedEnv := map[string]expected{
-		"FOO": "bar",
+		"FOO": {count: 1, edited: "bar"},
-		"ZOO": "bar-1",
+		"ZOO": {count: 1, edited: "bar-1"},
-		"BLU": "bar-1-2",
+		"BLU": {count: 0, edited: "bar-1-2"},
 	}
 
 	for k, v := range expectedEnv {
-		if e, a := v, declaredEnv[k]; e != a {
+		if e, a := v, declaredEnv[k]; e.edited != a || e.count != counts[k] {
-			t.Errorf("Expected %v, got %v", e, a)
+			t.Errorf("Expected %v count=%d, got %v count=%d",
+				e.edited, e.count, a, counts[k])
 		} else {
 			delete(declaredEnv, k)
 		}
@@ -79,9 +88,7 @@ func TestMapping(t *testing.T) {
 		"VAR_REF":   "$(VAR_A)",
 		"VAR_EMPTY": "",
 	}
-	mapping := MappingFuncFor(context)
+	doExpansionTest(t, context)
-
-	doExpansionTest(t, mapping)
 }
 
 func TestMappingDual(t *testing.T) {
@@ -94,51 +101,64 @@ func TestMappingDual(t *testing.T) {
 		"VAR_C":   "C",
 		"VAR_REF": "$(VAR_A)",
 	}
-	mapping := MappingFuncFor(context, context2)
 
-	doExpansionTest(t, mapping)
+	doExpansionTest(t, context, context2)
 }
 
-func doExpansionTest(t *testing.T, mapping func(string) string) {
+func doExpansionTest(t *testing.T, context ...map[string]string) {
 	cases := []struct {
 		name     string
 		input    string
 		expected string
+		counts   map[string]int
 	}{
 		{
 			name:     "whole string",
 			input:    "$(VAR_A)",
 			expected: "A",
+			counts:   map[string]int{"VAR_A": 1},
 		},
 		{
 			name:     "repeat",
 			input:    "$(VAR_A)-$(VAR_A)",
 			expected: "A-A",
+			counts:   map[string]int{"VAR_A": 2},
+		},
+		{
+			name:     "multiple repeats",
+			input:    "$(VAR_A)-$(VAR_B)-$(VAR_B)-$(VAR_B)-$(VAR_A)",
+			expected: "A-B-B-B-A",
+			counts:   map[string]int{"VAR_A": 2, "VAR_B": 3},
 		},
 		{
 			name:     "beginning",
 			input:    "$(VAR_A)-1",
 			expected: "A-1",
+			counts:   map[string]int{"VAR_A": 1},
 		},
 		{
 			name:     "middle",
 			input:    "___$(VAR_B)___",
 			expected: "___B___",
+			counts:   map[string]int{"VAR_B": 1},
 		},
 		{
 			name:     "end",
 			input:    "___$(VAR_C)",
 			expected: "___C",
+			counts:   map[string]int{"VAR_C": 1},
 		},
 		{
 			name:     "compound",
 			input:    "$(VAR_A)_$(VAR_B)_$(VAR_C)",
 			expected: "A_B_C",
+			counts:   map[string]int{"VAR_A": 1, "VAR_B": 1, "VAR_C": 1},
 		},
 		{
 			name:     "escape & expand",
 			input:    "$$(VAR_B)_$(VAR_A)",
 			expected: "$(VAR_B)_A",
+			counts:   map[string]int{"VAR_A": 1},
 		},
 		{
 			name:     "compound escape",
@@ -154,16 +174,19 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
 			name:     "backslash escape ignored",
 			input:    "foo\\$(VAR_C)bar",
 			expected: "foo\\Cbar",
+			counts:   map[string]int{"VAR_C": 1},
 		},
 		{
 			name:     "backslash escape ignored",
 			input:    "foo\\\\$(VAR_C)bar",
 			expected: "foo\\\\Cbar",
+			counts:   map[string]int{"VAR_C": 1},
 		},
 		{
 			name:     "lots of backslashes",
 			input:    "foo\\\\\\\\$(VAR_A)bar",
 			expected: "foo\\\\\\\\Abar",
+			counts:   map[string]int{"VAR_A": 1},
 		},
 		{
 			name:     "nested var references",
@@ -179,16 +202,19 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
 			name:     "value is a reference",
 			input:    "$(VAR_REF)",
 			expected: "$(VAR_A)",
+			counts:   map[string]int{"VAR_REF": 1},
 		},
 		{
 			name:     "value is a reference x 2",
 			input:    "%%$(VAR_REF)--$(VAR_REF)%%",
 			expected: "%%$(VAR_A)--$(VAR_A)%%",
+			counts:   map[string]int{"VAR_REF": 2},
 		},
 		{
 			name:     "empty var",
 			input:    "foo$(VAR_EMPTY)bar",
 			expected: "foobar",
+			counts:   map[string]int{"VAR_EMPTY": 1},
 		},
 		{
 			name:     "unterminated expression",
@@ -234,6 +260,7 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
 			name:     "multiple (odd) operators, var defined",
 			input:    "$$$$$$$(VAR_A)",
 			expected: "$$$A",
+			counts:   map[string]int{"VAR_A": 1},
 		},
 		{
 			name:     "missing open expression",
@@ -249,16 +276,19 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
 			name:     "trailing incomplete expression not consumed",
 			input:    "$(VAR_B)_______$(A",
 			expected: "B_______$(A",
+			counts:   map[string]int{"VAR_B": 1},
 		},
 		{
 			name:     "trailing incomplete expression, no content, is not consumed",
 			input:    "$(VAR_C)_______$(",
 			expected: "C_______$(",
+			counts:   map[string]int{"VAR_C": 1},
 		},
 		{
 			name:     "operator at end of input string is preserved",
 			input:    "$(VAR_A)foobarzab$",
 			expected: "Afoobarzab$",
+			counts:   map[string]int{"VAR_A": 1},
 		},
 		{
 			name:     "shell escaped incomplete expr",
@@ -293,9 +323,31 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
 	}
 
 	for _, tc := range cases {
+		counts := make(map[string]int)
+		mapping := MappingFuncFor(counts, context...)
 		expanded := Expand(tc.input, mapping)
 		if e, a := tc.expected, expanded; e != a {
 			t.Errorf("%v: expected %q, got %q", tc.name, e, a)
 		}
+		if len(counts) != len(tc.counts) {
+			t.Errorf("%v: len(counts)=%d != len(tc.counts)=%d",
+				tc.name, len(counts), len(tc.counts))
+		}
+		if len(tc.counts) > 0 {
+			for k, expectedCount := range tc.counts {
+				c, ok := counts[k]
+				if ok {
+					if c != expectedCount {
+						t.Errorf(
+							"%v: k=%s, expected count %d, got %d",
+							tc.name, k, expectedCount, c)
+					}
+				} else {
+					t.Errorf(
+						"%v: k=%s, expected count %d, got zero",
+						tc.name, k, expectedCount)
+				}
+			}
+		}
 	}
 }

pkg/target/resaccumulator.go

@@ -18,6 +18,9 @@ package target
 
 import (
 	"fmt"
+	"log"
+	"strings"
+
 	"sigs.k8s.io/kustomize/pkg/resid"
 	"sigs.k8s.io/kustomize/pkg/resmap"
 	"sigs.k8s.io/kustomize/pkg/transformers"
@@ -135,8 +138,18 @@ func (ra *ResAccumulator) ResolveVars() error {
 	if err != nil {
 		return err
 	}
-	return ra.Transform(transformers.NewRefVarTransformer(
+	if len(replacementMap) == 0 {
-		replacementMap, ra.tConfig.VarReference))
+		return nil
+	}
+	t := transformers.NewRefVarTransformer(
+		replacementMap, ra.tConfig.VarReference)
+	err = ra.Transform(t)
+	if len(t.UnusedVars()) > 0 {
+		log.Printf(
+			"well-defined vars that were never replaced: %s\n",
+			strings.Join(t.UnusedVars(), ","))
+	}
+	return err
 }
 
 func (ra *ResAccumulator) FixBackReferences() (err error) {

pkg/target/resaccumulator_test.go

@@ -17,6 +17,9 @@ limitations under the License.
 package target_test
 
 import (
+	"bytes"
+	"log"
+	"os"
 	"strings"
 	"testing"
 
@@ -120,6 +123,50 @@ func TestResolveVarsHappy(t *testing.T) {
 	}
 }
 
+func TestResolveVarsOneUnused(t *testing.T) {
+	ra, _, err := makeResAccumulator()
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra.MergeVars([]types.Var{
+		{
+			Name: "SERVICE_ONE",
+			ObjRef: types.Target{
+				Gvk:  gvk.Gvk{Version: "v1", Kind: "Service"},
+				Name: "backendOne"},
+		},
+		{
+			Name: "SERVICE_UNUSED",
+			ObjRef: types.Target{
+				Gvk:  gvk.Gvk{Version: "v1", Kind: "Service"},
+				Name: "backendTwo"},
+		},
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	var buf bytes.Buffer
+	log.SetOutput(&buf)
+	defer func() {
+		log.SetOutput(os.Stderr)
+	}()
+	err = ra.ResolveVars()
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	expectLog(t, buf, "well-defined vars that were never replaced: SERVICE_UNUSED")
+	c := getCommand(find("deploy1", ra.ResMap()))
+	if c != "myserver --somebackendService backendOne --yetAnother $(SERVICE_TWO)" {
+		t.Fatalf("unexpected command: %s", c)
+	}
+}
+
+func expectLog(t *testing.T, log bytes.Buffer, expect string) {
+	if !strings.Contains(log.String(), expect) {
+		t.Fatalf("expected log containing '%s', got '%s'", expect, log.String())
+	}
+}
+
 func TestResolveVarsVarNeedsDisambiguation(t *testing.T) {
 	ra, rf, err := makeResAccumulator()
 	if err != nil {

pkg/transformers/refvars.go

@@ -2,28 +2,26 @@ package transformers
 
 import (
 	"fmt"
-
 	"sigs.k8s.io/kustomize/pkg/expansion"
 	"sigs.k8s.io/kustomize/pkg/resmap"
 	"sigs.k8s.io/kustomize/pkg/transformers/config"
 )
 
-type refvarTransformer struct {
+type RefVarTransformer struct {
-	fieldSpecs  []config.FieldSpec
+	varMap            map[string]string
-	mappingFunc func(string) string
+	replacementCounts map[string]int
+	fieldSpecs        []config.FieldSpec
+	mappingFunc       func(string) string
 }
 
-// NewRefVarTransformer returns a Transformer that replaces $(VAR) style
+// NewRefVarTransformer returns a new RefVarTransformer
-// variables with values.
+// that replaces $(VAR) style variables with values.
 // The fieldSpecs are the places to look for occurrences of $(VAR).
 func NewRefVarTransformer(
-	varMap map[string]string, fs []config.FieldSpec) Transformer {
+	varMap map[string]string, fs []config.FieldSpec) *RefVarTransformer {
-	if len(varMap) == 0 {
+	return &RefVarTransformer{
-		return NewNoOpTransformer()
+		varMap:     varMap,
-	}
+		fieldSpecs: fs,
-	return &refvarTransformer{
-		fieldSpecs:  fs,
-		mappingFunc: expansion.MappingFuncFor(varMap),
 	}
 }
 
@@ -31,7 +29,7 @@ func NewRefVarTransformer(
 // embedded instances of $VAR style variables, e.g. a container command string.
 // The function returns the string with the variables expanded to their final
 // values.
-func (rv *refvarTransformer) replaceVars(in interface{}) (interface{}, error) {
+func (rv *RefVarTransformer) replaceVars(in interface{}) (interface{}, error) {
 	switch vt := in.(type) {
 	case []interface{}:
 		var xs []string
@@ -63,8 +61,24 @@ func (rv *refvarTransformer) replaceVars(in interface{}) (interface{}, error) {
 	}
 }
 
+// UnusedVars returns slice of Var names that were unused
+// after a Transform run.
+func (rv *RefVarTransformer) UnusedVars() []string {
+	var unused []string
+	for k := range rv.varMap {
+		_, ok := rv.replacementCounts[k]
+		if !ok {
+			unused = append(unused, k)
+		}
+	}
+	return unused
+}
+
 // Transform replaces $(VAR) style variables with values.
-func (rv *refvarTransformer) Transform(m resmap.ResMap) error {
+func (rv *RefVarTransformer) Transform(m resmap.ResMap) error {
+	rv.replacementCounts = make(map[string]int)
+	rv.mappingFunc = expansion.MappingFuncFor(
+		rv.replacementCounts, rv.varMap)
 	for id, res := range m {
 		for _, fieldSpec := range rv.fieldSpecs {
 			if id.Gvk().IsSelected(&fieldSpec.Gvk) {

pkg/transformers/refvars_test.go

@@ -5,7 +5,6 @@ import (
 	"testing"
 
 	"sigs.k8s.io/kustomize/pkg/resid"
-
 	"sigs.k8s.io/kustomize/pkg/resmap"
 	"sigs.k8s.io/kustomize/pkg/transformers/config"
 )
@@ -17,7 +16,8 @@ func TestVarRef(t *testing.T) {
 		res    resmap.ResMap
 	}
 	type expected struct {
-		res resmap.ResMap
+		res    resmap.ResMap
+		unused []string
 	}
 	testCases := []struct {
 		description string
@@ -28,7 +28,8 @@ func TestVarRef(t *testing.T) {
 			description: "var replacement in map[string]",
 			given: given{
 				varMap: map[string]string{
-					"FOO": "BAR",
+					"FOO": "replacementForFoo",
+					"BAR": "replacementForBar",
 				},
 				fs: []config.FieldSpec{
 					{Gvk: cmap, Path: "data"},
@@ -58,11 +59,12 @@ func TestVarRef(t *testing.T) {
 								"name": "cm1",
 							},
 							"data": map[string]interface{}{
-								"item1": "BAR",
+								"item1": "replacementForFoo",
 								"item2": "bla",
 							},
 						}),
 				},
+				unused: []string{"BAR"},
 			},
 		},
 	}