Merge pull request #1149 from qiujian16/var-reference

Keep var refernce in resources

pkg/accumulator/resaccumulator.go

@@ -63,6 +63,19 @@ func (ra *ResAccumulator) GetTransformerConfig() *config.TransformerConfig {
 }
 
 func (ra *ResAccumulator) MergeVars(incoming []types.Var) error {
+	for _, v := range incoming {
+		matched := ra.resMap.GetMatchingIds(
+			resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name).GvknEquals)
+		if len(matched) > 1 {
+			return fmt.Errorf(
+				"found %d resId matches for var %s "+
+					"(unable to disambiguate)",
+				len(matched), v)
+		}
+		if len(matched) == 1 {
+			ra.resMap.GetById(matched[0]).AppendRefVarName(v)
+		}
+	}
 	return ra.varSet.MergeSlice(incoming)
 }
 
@@ -78,34 +91,41 @@ func (ra *ResAccumulator) MergeAccumulator(other *ResAccumulator) (err error) {
 	return ra.varSet.MergeSet(other.varSet)
 }
 
+func (ra *ResAccumulator) findValueFromResources(v types.Var) (string, error) {
+	for _, res := range ra.resMap.Resources() {
+		for _, varName := range res.GetRefVarNames() {
+			if varName == v.Name {
+				s, err := res.GetFieldValue(v.FieldRef.FieldPath)
+				if err != nil {
+					return "", fmt.Errorf(
+						"field specified in var '%v' "+
+							"not found in corresponding resource", v)
+				}
+
+				return s, nil
+			}
+		}
+	}
+
+	return "", fmt.Errorf(
+		"var '%v' cannot be mapped to a field "+
+			"in the set of known resources", v)
+}
+
 // makeVarReplacementMap returns a map of Var names to
 // their final values. The values are strings intended
 // for substitution wherever the $(var.Name) occurs.
 func (ra *ResAccumulator) makeVarReplacementMap() (map[string]string, error) {
 	result := map[string]string{}
 	for _, v := range ra.Vars() {
-		matched := ra.resMap.GetMatchingIds(
+		s, err := ra.findValueFromResources(v)
-			resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name).GvknEquals)
+		if err != nil {
-		if len(matched) > 1 {
+			return nil, err
-			return nil, fmt.Errorf(
-				"found %d resId matches for var %s "+
-					"(unable to disambiguate)",
-				len(matched), v)
-		}
-		if len(matched) == 1 {
-			s, err := ra.resMap.GetById(matched[0]).GetFieldValue(v.FieldRef.FieldPath)
-			if err != nil {
-				return nil, fmt.Errorf(
-					"field specified in var '%v' "+
-						"not found in corresponding resource", v)
-			}
-			result[v.Name] = s
-		} else {
-			return nil, fmt.Errorf(
-				"var '%v' cannot be mapped to a field "+
-					"in the set of known resources", v)
 		}
+
+		result[v.Name] = s
 	}
+
 	return result, nil
 }
 

pkg/accumulator/resaccumulator_test.go

@@ -202,10 +202,6 @@ func TestResolveVarsVarNeedsDisambiguation(t *testing.T) {
 			},
 		},
 	})
-	if err != nil {
-		t.Fatalf("unexpected err: %v", err)
-	}
-	err = ra.ResolveVars()
 	if err == nil {
 		t.Fatalf("expected error")
 	}
@@ -270,6 +266,87 @@ func TestResolveVarsUnmappableVar(t *testing.T) {
 	}
 }
 
+func TestResolveVarsWithNoambiguiation(t *testing.T) {
+	ra, rf, 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",
+			},
+		},
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// Create another accumulator having a resource with different prefix
+	ra1 := MakeEmptyAccumulator()
+	rm1 := resmap.FromMap(map[resid.ResId]*resource.Resource{
+		resid.NewResId(gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"},
+			"deploy2"): rf.FromMap(
+			map[string]interface{}{
+				"apiVersion": "apps/v1",
+				"kind":       "Deployment",
+				"metadata": map[string]interface{}{
+					"name": "deploy2",
+				},
+				"spec": map[string]interface{}{
+					"template": map[string]interface{}{
+						"spec": map[string]interface{}{
+							"containers": []interface{}{
+								map[string]interface{}{
+									"command": []interface{}{
+										"myserver",
+										"--somebackendService $(SUB_SERVICE_ONE)",
+									},
+								},
+							},
+						},
+					},
+				},
+			}),
+		resid.NewResIdWithPrefixNamespace(
+			gvk.Gvk{Version: "v1", Kind: "Service"},
+			"backendOne", "sub-", ""): rf.FromMap(
+			map[string]interface{}{
+				"apiVersion": "v1",
+				"kind":       "Service",
+				"metadata": map[string]interface{}{
+					"name": "backendOne",
+				},
+			}),
+	})
+	ra1.AppendAll(rm1)
+
+	err = ra1.MergeVars([]types.Var{
+		{
+			Name: "SUB_SERVICE_ONE",
+			ObjRef: types.Target{
+				Gvk:  gvk.Gvk{Version: "v1", Kind: "Service"},
+				Name: "backendOne",
+			},
+		},
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra.MergeAccumulator(ra1)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	err = ra.ResolveVars()
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+}
+
 func find(name string, resMap resmap.ResMap) *resource.Resource {
 	for k, v := range resMap.AsMap() {
 		if k.Name() == name {

pkg/resource/resource.go

@@ -31,8 +31,9 @@ import (
 // paired with a GenerationBehavior.
 type Resource struct {
 	ifc.Kunstructured
-	options *types.GenArgs
+	options     *types.GenArgs
-	refBy   []resid.ResId
+	refBy       []resid.ResId
+	refVarNames []string
 }
 
 func (r *Resource) KunstructEqual(o *Resource) bool {
@@ -59,6 +60,11 @@ func (r *Resource) DeepCopy() *Resource {
 		copy(refby, r.refBy)
 		rc.refBy = refby
 	}
+	if len(r.refVarNames) > 0 {
+		refVarNames := make([]string, len(r.refVarNames))
+		copy(refVarNames, r.refVarNames)
+		rc.refVarNames = refVarNames
+	}
 	return rc
 }
 
@@ -98,6 +104,16 @@ func (r *Resource) AppendRefBy(id resid.ResId) {
 	r.refBy = append(r.refBy, id)
 }
 
+// GetRefVarNames returns vars that refer to current resource
+func (r *Resource) GetRefVarNames() []string {
+	return r.refVarNames
+}
+
+// AppendRefVarName appends a name of a var into the refVar list
+func (r *Resource) AppendRefVarName(variable types.Var) {
+	r.refVarNames = append(r.refVarNames, variable.Name)
+}
+
 // Merge performs merge with other resource.
 func (r *Resource) Merge(other *Resource) {
 	r.Replace(other)

pkg/resource/resource_test.go

@@ -24,6 +24,7 @@ import (
 	"sigs.k8s.io/kustomize/pkg/gvk"
 	"sigs.k8s.io/kustomize/pkg/resid"
 	. "sigs.k8s.io/kustomize/pkg/resource"
+	"sigs.k8s.io/kustomize/pkg/types"
 )
 
 var factory = NewFactory(
@@ -122,6 +123,14 @@ func TestDeepCopy(t *testing.T) {
 		})
 	r.AppendRefBy(resid.NewResId(gvk.Gvk{Group: "somegroup", Kind: "MyKind"}, "random"))
 
+	var1 := types.Var{
+		Name: "SERVICE_ONE",
+		ObjRef: types.Target{
+			Gvk:  gvk.Gvk{Version: "v1", Kind: "Service"},
+			Name: "backendOne"},
+	}
+	r.AppendRefVarName(var1)
+
 	cr := r.DeepCopy()
 	if !reflect.DeepEqual(r, cr) {
 		t.Errorf("expected %v\nbut got%v", r, cr)

pkg/target/kusttarget.go

@@ -228,11 +228,6 @@ func (kt *KustTarget) AccumulateTarget() (
 		return nil, errors.Wrapf(
 			err, "merging config %v", tConfig)
 	}
-	err = ra.MergeVars(kt.kustomization.Vars)
-	if err != nil {
-		return nil, errors.Wrapf(
-			err, "merging vars %v", kt.kustomization.Vars)
-	}
 	crdTc, err := config.LoadConfigFromCRDs(kt.ldr, kt.kustomization.Crds)
 	if err != nil {
 		return nil, errors.Wrapf(
@@ -248,7 +243,15 @@ func (kt *KustTarget) AccumulateTarget() (
 		return nil, err
 	}
 	err = kt.runTransformers(ra)
-	return ra, err
+	if err != nil {
+		return nil, err
+	}
+	err = ra.MergeVars(kt.kustomization.Vars)
+	if err != nil {
+		return nil, errors.Wrapf(
+			err, "merging vars %v", kt.kustomization.Vars)
+	}
+	return ra, nil
 }
 
 func (kt *KustTarget) runGenerators(

pkg/target/variableref_test.go

@@ -739,7 +739,7 @@ vars:
   apiVersion: apps/v1
   kind: Deployment
   metadata:
-    name: my-deployment 
+    name: my-deployment
   spec:
     template:
       spec:
@@ -820,7 +820,7 @@ vars:
       spec:
         containers:
         - name: app
-          image: busybox  
+          image: busybox
 `)
 	th.WriteF("/app/base/namespace.yaml", `
   apiVersion: v1
@@ -853,3 +853,147 @@ spec:
         name: app
 `)
 }
+
+func TestVaribaleRefDifferentPrefix(t *testing.T) {
+	th := kusttest_test.NewKustTestHarness(t, "/app/base")
+	th.WriteK("/app/base", `
+namePrefix: base-
+bases:
+- dev
+- test
+`)
+
+	th.WriteK("/app/base/dev", `
+namePrefix: dev-
+resources:
+- elasticsearch-dev-service.yml
+vars:
+- name: elasticsearch-dev-service-name
+  objref:
+    kind: Service
+    name: elasticsearch
+    apiVersion: v1
+  fieldref:
+    fieldpath: metadata.name
+
+`)
+	th.WriteF("/app/base/dev/elasticsearch-dev-service.yml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: elasticsearch
+spec:
+  template:
+    spec:
+      containers:
+        - name: elasticsearch
+          env:
+            - name: DISCOVERY_SERVICE
+              value: "$(elasticsearch-dev-service-name).monitoring.svc.cluster.local"
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: elasticsearch
+spec:
+  ports:
+    - name: transport
+      port: 9300
+      protocol: TCP
+  clusterIP: None
+`)
+
+	th.WriteK("/app/base/test", `
+namePrefix: test-
+resources:
+- elasticsearch-test-service.yml
+vars:
+- name: elasticsearch-test-service-name
+  objref:
+    kind: Service
+    name: elasticsearch
+    apiVersion: v1
+  fieldref:
+    fieldpath: metadata.name
+`)
+	th.WriteF("/app/base/test/elasticsearch-test-service.yml", `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: elasticsearch
+spec:
+  template:
+    spec:
+      containers:
+        - name: elasticsearch
+          env:
+            - name: DISCOVERY_SERVICE
+              value: "$(elasticsearch-test-service-name).monitoring.svc.cluster.local"
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: elasticsearch
+spec:
+  ports:
+    - name: transport
+      port: 9300
+      protocol: TCP
+  clusterIP: None
+`)
+
+	m, err := th.MakeKustTarget().MakeCustomizedResMap()
+	if err != nil {
+		t.Fatalf("Err: %v", err)
+	}
+
+	th.AssertActualEqualsExpected(m, `
+apiVersion: v1
+kind: Service
+metadata:
+  name: base-dev-elasticsearch
+spec:
+  clusterIP: None
+  ports:
+  - name: transport
+    port: 9300
+    protocol: TCP
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: base-test-elasticsearch
+spec:
+  clusterIP: None
+  ports:
+  - name: transport
+    port: 9300
+    protocol: TCP
+---
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: base-dev-elasticsearch
+spec:
+  template:
+    spec:
+      containers:
+      - env:
+        - name: DISCOVERY_SERVICE
+          value: base-dev-elasticsearch.monitoring.svc.cluster.local
+        name: elasticsearch
+---
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: base-test-elasticsearch
+spec:
+  template:
+    spec:
+      containers:
+      - env:
+        - name: DISCOVERY_SERVICE
+          value: base-test-elasticsearch.monitoring.svc.cluster.local
+        name: elasticsearch
+`)
+}