Improve name reference transformer testing.

api/internal/accumulator/loadconfigfromcrds.go

@@ -162,7 +162,7 @@ func loadCrdIntoConfig(
 				err = theConfig.AddNamereferenceFieldSpec(
 					builtinconfig.NameBackReferences{
 						Gvk: resid.Gvk{Kind: kind, Version: version},
-						FieldSpecs: []types.FieldSpec{
+						Referrers: []types.FieldSpec{
 							makeFs(theGvk, append(path, propName, nameKey))},
 					})
 				if err != nil {

api/internal/accumulator/loadconfigfromcrds_test.go

@@ -8,10 +8,9 @@ import (
 	"testing"
 
 	"sigs.k8s.io/kustomize/api/filesys"
-	"sigs.k8s.io/kustomize/api/loader"
-
 	. "sigs.k8s.io/kustomize/api/internal/accumulator"
 	"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
+	"sigs.k8s.io/kustomize/api/loader"
 	"sigs.k8s.io/kustomize/api/resid"
 	"sigs.k8s.io/kustomize/api/types"
 )
@@ -140,21 +139,19 @@ func TestLoadCRDs(t *testing.T) {
 	nbrs := []builtinconfig.NameBackReferences{
 		{
 			Gvk: resid.Gvk{Kind: "Secret", Version: "v1"},
-			FieldSpecs: []types.FieldSpec{
+			Referrers: []types.FieldSpec{
 				{
-					CreateIfNotPresent: false,
-					Gvk:                resid.Gvk{Kind: "MyKind"},
-					Path:               "spec/secretRef/name",
+					Gvk:  resid.Gvk{Kind: "MyKind"},
+					Path: "spec/secretRef/name",
 				},
 			},
 		},
 		{
 			Gvk: resid.Gvk{Kind: "Bee", Version: "v1beta1"},
-			FieldSpecs: []types.FieldSpec{
+			Referrers: []types.FieldSpec{
 				{
-					CreateIfNotPresent: false,
-					Gvk:                resid.Gvk{Kind: "MyKind"},
-					Path:               "spec/beeRef/name",
+					Gvk:  resid.Gvk{Kind: "MyKind"},
+					Path: "spec/beeRef/name",
 				},
 			},
 		},

api/internal/accumulator/namereferencetransformer.go

@@ -4,11 +4,13 @@
 package accumulator
 
 import (
+	"fmt"
 	"log"
 
 	"sigs.k8s.io/kustomize/api/filters/nameref"
 	"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
 	"sigs.k8s.io/kustomize/api/resmap"
+	"sigs.k8s.io/kustomize/api/resource"
 )
 
 type nameReferenceTransformer struct {
@@ -17,6 +19,8 @@ type nameReferenceTransformer struct {
 
 var _ resmap.Transformer = &nameReferenceTransformer{}
 
+type filterMap map[*resource.Resource][]nameref.Filter
+
 // newNameReferenceTransformer constructs a nameReferenceTransformer
 // with a given slice of NameBackReferences.
 func newNameReferenceTransformer(
@@ -29,16 +33,62 @@ func newNameReferenceTransformer(
 
 // Transform updates name references in resource A that
 // refer to resource B, given that B's name may have
-// changed.  A is the referrer, B is the referralTarget.
+// changed.
 //
 // For example, a HorizontalPodAutoscaler (HPA)
 // necessarily refers to a Deployment, the thing that
-// the HPA scales. The Deployment's name might change
-// (e.g. prefix added), and the reference in the HPA
-// has to be fixed.
+// an HPA scales. In this case:
 //
-// In the outer loop over the ResMap below, say we
-// encounter a specific HPA. Then, in scanning the set
+//   - the HPA instance is the Referrer,
+//   - the Deployment instance is the ReferralTarget.
+//
+// If the Deployment's name changes, e.g. a prefix is added,
+// then the HPA's reference to the Deployment must be fixed.
+//
+func (t *nameReferenceTransformer) Transform(m resmap.ResMap) error {
+	fMap := t.determineFilters(m.Resources())
+	// debug(fMap)
+	for r, fList := range fMap {
+		c := m.SubsetThatCouldBeReferencedByResource(r)
+		for _, f := range fList {
+			f.Referrer = r
+			f.ReferralCandidates = c
+			if err := f.Referrer.ApplyFilter(f); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+//nolint: deadcode, unused
+func debug(fMap filterMap) {
+	fmt.Printf("filterMap has %d entries:\n", len(fMap))
+	rCount := 0
+	for r, fList := range fMap {
+		yml, _ := r.AsYAML()
+		rCount++
+		fmt.Printf(`
+---- %3d. possible referrer -------------
+%s
+---------`, rCount, string(yml),
+		)
+		for i, f := range fList {
+			fmt.Printf(`
+%3d/%3d update: %s
+          from: %s
+`, rCount, i+1, f.NameFieldToUpdate.Path, f.ReferralTarget,
+			)
+		}
+	}
+}
+
+// Produce a map from referrer resources that might need to be fixed
+// to filters that might fix them.  The keys to this map are potential
+// referrers, so won't include resources like ConfigMap or Secret.
+//
+// In the inner loop over the resources below, say we
+// encounter an HPA instance. Then, in scanning the set
 // of all known backrefs, we encounter an entry like
 //
 //   - kind: Deployment
@@ -48,64 +98,50 @@ func newNameReferenceTransformer(
 //
 // This entry says that an HPA, via its
 // 'spec/scaleTargetRef/name' field, may refer to a
-// Deployment.  This match to HPA means we may need to
-// modify the value in its 'spec/scaleTargetRef/name'
-// field, by searching for the thing it refers to,
-// and getting its new name.
+// Deployment.
 //
-// As a filter, and search optimization, we compute a
-// subset of all resources that the HPA could refer to,
-// by excluding objects from other namespaces, and
-// excluding objects that don't have the same prefix-
-// suffix mods as the HPA.
-//
-// We look in this subset for all Deployment objects
-// with a resId that has a Name matching the field value
-// present in the HPA.  If no match do nothing; if more
-// than one match, it's an error.
-//
-// We overwrite the HPA name field with the value found
-// in the Deployment's name field (the name in the raw
-// object - the modified name - not the unmodified name
-// in the Deployment's resId).
-//
-// This process assumes that the name stored in a ResId
-// (the ResMap key) isn't modified by name transformers.
-// Name transformers should only modify the name in the
-// body of the resource object (the value in the ResMap).
-//
-func (t *nameReferenceTransformer) Transform(m resmap.ResMap) error {
-	// TODO: Too much looping, here and in transitive calls.
-	for _, referrer := range m.Resources() {
-		var candidates resmap.ResMap
-		for _, referralTarget := range t.backRefs {
-			for _, fSpec := range referralTarget.FieldSpecs {
-				if referrer.OrgId().IsSelected(&fSpec.Gvk) {
-					if candidates == nil {
-						// This excludes objects from other namespaces.
-						// In most realistic uses, it returns all elements of m,
-						// (since they're all in the same namespace).
-						candidates = m.SubsetThatCouldBeReferencedByResource(referrer)
-					}
-					// One way to get here is with, say, a referrer that's an
-					// HPA, and a target that's a Deployment (one of the
-					// Deployment's fieldSpecs selects an HPA).  Now we look
-					// through the candidates to see if one is a Deployment
-					// (the target), and if so, get the Deployment's name and
-					// write it into the referrer, at the field specfied in
-					// fSpec.
-					err := referrer.ApplyFilter(nameref.Filter{
-						Referrer:           referrer,
-						NameFieldToUpdate:  fSpec,
-						ReferralTarget:     referralTarget.Gvk,
-						ReferralCandidates: candidates,
-					})
-					if err != nil {
-						return err
+// This means that a filter will need to hunt for the right Deployment,
+// obtain it's new name, and write that name into the HPA's
+// 'spec/scaleTargetRef/name' field. Return a filter that can do that.
+func (t *nameReferenceTransformer) determineFilters(
+	resources []*resource.Resource) (fMap filterMap) {
+	fMap = make(filterMap)
+	for _, backReference := range t.backRefs {
+		for _, referrerSpec := range backReference.Referrers {
+			for _, res := range resources {
+				if res.OrgId().IsSelected(&referrerSpec.Gvk) {
+					// If this is true, the res might be a referrer, and if
+					// so, the name reference it holds might need an update.
+					if resHasField(res, referrerSpec.Path) {
+						// Optimization - the referrer has the field
+						// that might need updating.
+						fMap[res] = append(fMap[res], nameref.Filter{
+							// Name field to write in the Referrer.
+							NameFieldToUpdate: referrerSpec,
+							// Specification of object class to read from.
+							// Always read from metadata/name field.
+							ReferralTarget: backReference.Gvk,
+						})
 					}
 				}
 			}
 		}
 	}
-	return nil
+	return fMap
+}
+
+// TODO: check res for field existence here to avoid extra work.
+// res.GetFieldValue, which uses yaml.Lookup under the hood, doesn't know
+// how to parse fieldspec-style paths that make no distinction
+// between maps and sequences.  This means it cannot lookup commonly
+// used "indeterminate" paths like
+//    spec/containers/env/valueFrom/configMapKeyRef/name
+// ('containers' is a list, not a map).
+// However, the fieldspec filter does know how to handle this;
+// extract that code and call it here?
+func resHasField(res *resource.Resource, path string) bool {
+	return true
+	// fld := strings.Join(utils.PathSplitter(path), ".")
+	// _, e := res.GetFieldValue(fld)
+	// return e == nil
 }

api/internal/accumulator/namereferencetransformer_test.go

@@ -7,15 +7,16 @@ import (
 	"strings"
 	"testing"
 
-	"sigs.k8s.io/kustomize/api/konfig"
-
 	"sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
+	"sigs.k8s.io/kustomize/api/konfig"
 	"sigs.k8s.io/kustomize/api/provider"
 	"sigs.k8s.io/kustomize/api/resid"
 	"sigs.k8s.io/kustomize/api/resmap"
 	resmaptest_test "sigs.k8s.io/kustomize/api/testutils/resmaptest"
 )
 
+const notEqualErrFmt = "expected (self) doesn't match actual (other): %v"
+
 func TestNameReferenceHappyRun(t *testing.T) {
 	m := resmaptest_test.NewRmBuilderDefault(t).AddWithName(
 		"cm1",
@@ -472,7 +473,7 @@ func TestNameReferenceHappyRun(t *testing.T) {
 	}
 
 	if err = expected.ErrorIfNotEqualLists(m); err != nil {
-		t.Fatalf("actual doesn't match expected: %v", err)
+		t.Fatalf(notEqualErrFmt, err)
 	}
 }
 
@@ -609,7 +610,7 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) {
 	v2.AppendRefBy(c2.CurId())
 
 	if err := m1.ErrorIfNotEqualLists(m2); err != nil {
-		t.Fatalf("actual doesn't match expected: %v", err)
+		t.Fatalf(notEqualErrFmt, err)
 	}
 }
 
@@ -745,7 +746,7 @@ func TestNameReferenceNamespace(t *testing.T) {
 
 	m.RemoveBuildAnnotations()
 	if err = expected.ErrorIfNotEqualLists(m); err != nil {
-		t.Fatalf("actual doesn't match expected: %v", err)
+		t.Fatalf(notEqualErrFmt, err)
 	}
 }
 
@@ -907,7 +908,7 @@ func TestNameReferenceClusterWide(t *testing.T) {
 
 	m.RemoveBuildAnnotations()
 	if err = expected.ErrorIfNotEqualLists(m); err != nil {
-		t.Fatalf("actual doesn't match expected: %v", err)
+		t.Fatalf(notEqualErrFmt, err)
 	}
 }
 
@@ -1034,7 +1035,7 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) {
 
 	m.RemoveBuildAnnotations()
 	if err = expected.ErrorIfNotEqualLists(m); err != nil {
-		t.Fatalf("actual doesn't match expected: %v", err)
+		t.Fatalf(notEqualErrFmt, err)
 	}
 }
 
@@ -1071,6 +1072,6 @@ func TestNameReferenceCandidateSelection(t *testing.T) {
 
 	m.RemoveBuildAnnotations()
 	if err = expected.ErrorIfNotEqualLists(m); err != nil {
-		t.Fatalf("actual doesn't match expected: %v", err)
+		t.Fatalf(notEqualErrFmt, err)
 	}
 }

api/internal/plugins/builtinconfig/namebackreferences.go

@@ -10,8 +10,8 @@ import (
 	"sigs.k8s.io/kustomize/api/types"
 )
 
-// NameBackReferences is an association between a gvk.GVK and a list
-// of FieldSpec instances that could refer to it.
+// NameBackReferences is an association between a gvk.GVK (a ReferralTarget)
+// and a list of Referrers that could refer to it.
 //
 // It is used to handle name changes, and can be thought of as a
 // a contact list.  If you change your own contact info (name,
@@ -19,16 +19,20 @@ import (
 // know about the change.
 //
 // For example, ConfigMaps can be used by Pods and everything that
-// contains a Pod; Deployment, Job, StatefulSet, etc.  To change
-// the name of a ConfigMap instance from 'alice' to 'bob', one
-// must visit all objects that could refer to the ConfigMap, see if
-// they mention 'alice', and if so, change the reference to 'bob'.
+// contains a Pod; Deployment, Job, StatefulSet, etc.
+// The ConfigMap is the ReferralTarget, the others are Referrers.
+//
+// If the the name of a ConfigMap instance changed from 'alice' to 'bob',
+// one must
+//  - visit all objects that could refer to the ConfigMap (the Referrers)
+//  - see if they mention 'alice',
+//  - if so, change the Referrer's name reference to 'bob'.
 //
 // The NameBackReferences instance to aid in this could look like
 //   {
 //     kind: ConfigMap
 //     version: v1
-//     FieldSpecs:
+//     fieldSpecs:
 //     - kind: Pod
 //       version: v1
 //       path: spec/volumes/configMap/name
@@ -39,13 +43,15 @@ import (
 //       (etc.)
 //   }
 type NameBackReferences struct {
-	resid.Gvk  `json:",inline,omitempty" yaml:",inline,omitempty"`
-	FieldSpecs types.FsSlice `json:"FieldSpecs,omitempty" yaml:"FieldSpecs,omitempty"`
+	resid.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"`
+	// TODO: rename json 'fieldSpecs' to 'referrers' for clarity.
+	// This will, however, break anyone using a custom config.
+	Referrers types.FsSlice `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
 }
 
 func (n NameBackReferences) String() string {
 	var r []string
-	for _, f := range n.FieldSpecs {
+	for _, f := range n.Referrers {
 		r = append(r, f.String())
 	}
 	return n.Gvk.String() + ":  (\n" +
@@ -77,7 +83,7 @@ func (s nbrSlice) mergeOne(other NameBackReferences) (nbrSlice, error) {
 	found := false
 	for _, c := range s {
 		if c.Gvk.Equals(other.Gvk) {
-			c.FieldSpecs, err = c.FieldSpecs.MergeAll(other.FieldSpecs)
+			c.Referrers, err = c.Referrers.MergeAll(other.Referrers)
 			if err != nil {
 				return nil, err
 			}

api/internal/plugins/builtinconfig/namebackreferences_test.go

@@ -50,13 +50,13 @@ func TestMergeAll(t *testing.T) {
 			Gvk: resid.Gvk{
 				Kind: "ConfigMap",
 			},
-			FieldSpecs: fsSlice1,
+			Referrers: fsSlice1,
 		},
 		{
 			Gvk: resid.Gvk{
 				Kind: "Secret",
 			},
-			FieldSpecs: fsSlice2,
+			Referrers: fsSlice2,
 		},
 	}
 	nbrsSlice2 := nbrSlice{
@@ -64,13 +64,13 @@ func TestMergeAll(t *testing.T) {
 			Gvk: resid.Gvk{
 				Kind: "ConfigMap",
 			},
-			FieldSpecs: fsSlice1,
+			Referrers: fsSlice1,
 		},
 		{
 			Gvk: resid.Gvk{
 				Kind: "Secret",
 			},
-			FieldSpecs: fsSlice2,
+			Referrers: fsSlice2,
 		},
 	}
 	expected := nbrSlice{
@@ -78,13 +78,13 @@ func TestMergeAll(t *testing.T) {
 			Gvk: resid.Gvk{
 				Kind: "ConfigMap",
 			},
-			FieldSpecs: fsSlice1,
+			Referrers: fsSlice1,
 		},
 		{
 			Gvk: resid.Gvk{
 				Kind: "Secret",
 			},
-			FieldSpecs: fsSlice2,
+			Referrers: fsSlice2,
 		},
 	}
 	actual, err := nbrsSlice1.mergeAll(nbrsSlice2)

api/internal/plugins/builtinconfig/transformerconfig_test.go

@@ -24,7 +24,7 @@ func TestAddNamereferenceFieldSpec(t *testing.T) {
 		Gvk: resid.Gvk{
 			Kind: "KindA",
 		},
-		FieldSpecs: []types.FieldSpec{
+		Referrers: []types.FieldSpec{
 			{
 				Gvk: resid.Gvk{
 					Kind: "KindB",
@@ -89,7 +89,7 @@ func TestMerge(t *testing.T) {
 			Gvk: resid.Gvk{
 				Kind: "KindA",
 			},
-			FieldSpecs: []types.FieldSpec{
+			Referrers: []types.FieldSpec{
 				{
 					Gvk: resid.Gvk{
 						Kind: "KindB",
@@ -103,7 +103,7 @@ func TestMerge(t *testing.T) {
 			Gvk: resid.Gvk{
 				Kind: "KindA",
 			},
-			FieldSpecs: []types.FieldSpec{
+			Referrers: []types.FieldSpec{
 				{
 					Gvk: resid.Gvk{
 						Kind: "KindC",

api/internal/wrappy/wnode_test.go

@@ -33,6 +33,38 @@ const (
       "area": "51",
       "greeting": "Take me to your leader."
     }
+  },
+  "spec": {
+    "template": {
+      "spec": {
+        "containers": [
+          {
+            "env": [
+              {
+                "name": "CM_FOO",
+                "valueFrom": {
+                  "configMapKeyRef": {
+                    "key": "somekey",
+                    "name": "myCm"
+                  }
+                }
+              },
+              {
+                "name": "SECRET_FOO",
+                "valueFrom": {
+                  "secretKeyRef": {
+                    "key": "someKey",
+                    "name": "mySecret"
+                  }
+                }
+              }
+            ],
+            "image": "nginx:1.7.9",
+            "name": "nginx"
+          }
+        ]
+      }
+    }
   }
 }
 `
@@ -358,6 +390,51 @@ func TestGetFieldValueReturnsMap(t *testing.T) {
 	}
 }
 
+func TestGetFieldValueReturnsStuff(t *testing.T) {
+	wn := NewWNode()
+	if err := wn.UnmarshalJSON([]byte(deploymentBiggerJson)); err != nil {
+		t.Fatalf("unexpected unmarshaljson err: %v", err)
+	}
+	expected := []interface{}{
+		map[string]interface{}{
+			"env": []interface{}{
+				map[string]interface{}{
+					"name": "CM_FOO",
+					"valueFrom": map[string]interface{}{
+						"configMapKeyRef": map[string]interface{}{
+							"key":  "somekey",
+							"name": "myCm",
+						},
+					},
+				},
+				map[string]interface{}{
+					"name": "SECRET_FOO",
+					"valueFrom": map[string]interface{}{
+						"secretKeyRef": map[string]interface{}{
+							"key":  "someKey",
+							"name": "mySecret",
+						},
+					},
+				},
+			},
+			"image": string("nginx:1.7.9"),
+			"name":  string("nginx"),
+		},
+	}
+	actual, err := wn.GetFieldValue("spec.template.spec.containers")
+	if err != nil {
+		t.Fatalf("error getting field value: %v", err)
+	}
+	if diff := cmp.Diff(expected, actual); diff != "" {
+		t.Fatalf("actual map does not deep equal expected map:\n%v", diff)
+	}
+	// Cannot go deeper yet.
+	_, err = wn.GetFieldValue("spec.template.spec.containers.env")
+	if err == nil {
+		t.Fatalf("expected err %v", err)
+	}
+}
+
 func TestGetFieldValueReturnsSlice(t *testing.T) {
 	bytes, err := yaml.Marshal(makeBigMap())
 	if err != nil {