allow vars of the same name that refer to the same concrete value

pkg/accumulator/resaccumulator.go

@@ -94,7 +94,21 @@ func (ra *ResAccumulator) MergeAccumulator(other *ResAccumulator) (err error) {
 	if err != nil {
 		return err
 	}
-	return ra.varSet.MergeSet(other.varSet)
+	err = ra.varSet.MergeSet(other.varSet)
+	if err != nil {
+		// when there are two variables of the same name, do not cause a conflict
+		// if the values are the same.
+		if mergeErr, ok := err.(*types.VarMergeError); ok {
+			incomingValue, ierr := ra.findVarValueFromResources(mergeErr.Incoming)
+			conflictValue, cerr := other.findVarValueFromResources(mergeErr.Conflict)
+			if ierr == nil && cerr == nil && incomingValue == conflictValue {
+				log.Printf("var %s detected more than once (contains same value, allowing)", mergeErr.Incoming.Name)
+				return nil
+			}
+		}
+		return err
+	}
+	return nil
 }
 
 func (ra *ResAccumulator) findVarValueFromResources(v types.Var) (interface{}, error) {

pkg/accumulator/resaccumulator_test.go

@@ -179,6 +179,141 @@ func TestResolveVarsVarNeedsDisambiguation(t *testing.T) {
 	}
 }
 
+func TestResolveVarsAllowableConflict(t *testing.T) {
+	rf := resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl())
+
+	ra0 := MakeEmptyAccumulator()
+	rm0 := resmap.New()
+
+	err := rm0.Append(
+		rf.FromMap(
+			map[string]interface{}{
+				"apiVersion": "v1",
+				"kind":       "ConfigMap",
+				"metadata": map[string]interface{}{
+					"name":      "environment",
+					"namespace": "foo",
+				},
+				"data": map[string]interface{}{
+					"name": "dev",
+				},
+			},
+		),
+	)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra0.AppendAll(rm0)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra0.MergeVars([]types.Var{
+		{
+			Name: "ENVIRONMENT_NAME",
+			ObjRef: types.Target{
+				Gvk:       gvk.Gvk{Version: "v1", Kind: "ConfigMap"},
+				Name:      "environment",
+				Namespace: "foo",
+			},
+			FieldRef: types.FieldSelector{FieldPath: "data.name"},
+		},
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	ra1 := MakeEmptyAccumulator()
+	rm1 := resmap.New()
+	err = rm1.Append(
+		rf.FromMap(
+			map[string]interface{}{
+				"apiVersion": "v1",
+				"kind":       "ConfigMap",
+				"metadata": map[string]interface{}{
+					"name":      "environment",
+					"namespace": "bar",
+				},
+				"data": map[string]interface{}{
+					"name": "dev",
+				},
+			},
+		),
+	)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra1.AppendAll(rm1)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra1.MergeVars([]types.Var{
+		{
+			Name: "ENVIRONMENT_NAME",
+			ObjRef: types.Target{
+				Gvk:       gvk.Gvk{Version: "v1", Kind: "ConfigMap"},
+				Name:      "environment",
+				Namespace: "bar",
+			},
+			FieldRef: types.FieldSelector{FieldPath: "data.name"},
+		},
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	// validate that two vars of the same name which reference the same concrete
+	// value do not produce a conflict.
+	err = ra0.MergeAccumulator(ra1)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	ra2 := MakeEmptyAccumulator()
+	rm2 := resmap.New()
+	err = rm2.Append(
+		rf.FromMap(
+			map[string]interface{}{
+				"apiVersion": "v1",
+				"kind":       "ConfigMap",
+				"metadata": map[string]interface{}{
+					"name":      "environment",
+					"namespace": "bar",
+				},
+				"data": map[string]interface{}{
+					"name": "DIFFERENT",
+				},
+			},
+		),
+	)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra2.AppendAll(rm1)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	err = ra2.MergeVars([]types.Var{
+		{
+			Name: "ENVIRONMENT_NAME",
+			ObjRef: types.Target{
+				Gvk:       gvk.Gvk{Version: "v1", Kind: "ConfigMap"},
+				Name:      "environment",
+				Namespace: "bar",
+			},
+			FieldRef: types.FieldSelector{FieldPath: "data.name"},
+		},
+	})
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+
+	err = ra1.MergeAccumulator(ra2)
+	if err == nil {
+		t.Fatalf("two variables with the same name referencing different concrete values should cause a failure")
+	}
+
+}
+
 func TestResolveVarsGoodResIdBadField(t *testing.T) {
 	ra, _ := makeResAccumulator(t)
 	err := ra.MergeVars([]types.Var{

pkg/types/var.go

@@ -65,6 +65,19 @@ type FieldSelector struct {
 	FieldPath string `json:"fieldPath,omitempty" yaml:"fieldPath,omitempty"`
 }
 
+// VarMergeError annotates a variable merge conflict with a reference to the
+// variable that encountered the conflict. This is used at higher levels of the
+// API to determine if the conflict can be ignored (e.g. two sets of resources
+// using the same variable name are not in conflict if the concrete values they
+// reference are the same).
+type VarMergeError struct {
+	msg      string
+	Incoming Var
+	Conflict Var
+}
+
+func (e *VarMergeError) Error() string { return e.msg }
+
 // defaulting sets reference to field used by default.
 func (v *Var) Defaulting() {
 	if v.FieldRef.FieldPath == "" {
@@ -140,8 +153,11 @@ func (vs *VarSet) MergeSlice(incoming []Var) error {
 // Empty fields in incoming Var is defaulted.
 func (vs *VarSet) Merge(v Var) error {
 	if vs.Contains(v) {
-		return fmt.Errorf(
-			"var '%s' already encountered", v.Name)
+		return &VarMergeError{
+			msg:      fmt.Sprintf("var '%s' already encountered", v.Name),
+			Incoming: v,
+			Conflict: *vs.Get(v.Name),
+		}
 	}
 	v.Defaulting()
 	vs.set[v.Name] = v