Merge pull request #1308 from keleustes/varequal

Demonstrate need for Var.DeepEqual method equivalent

pkg/target/kusttarget_test.go

@@ -6,7 +6,6 @@ package target_test
 import (
 	"encoding/base64"
 	"fmt"
-	"reflect"
 	"strings"
 	"testing"
 
@@ -334,7 +333,10 @@ vars:
 		t.Fatalf("unexpected size %d", len(vars))
 	}
 	for i := range vars[:2] {
-		if !reflect.DeepEqual(vars[i], someVars[i]) {
+		// By using Var.DeepEqual, we are protecting the code
+		// from a potential invocation of vars[i].ObjRef.GVK()
+		// during AccumulateTarget
+		if !vars[i].DeepEqual(someVars[i]) {
 			t.Fatalf("unexpected var[%d]:\n  %v\n  %v", i, vars[i], someVars[i])
 		}
 	}
@@ -387,7 +389,10 @@ resources:
 		t.Fatalf("expected 4 vars, got %d", len(vars))
 	}
 	for i := range vars {
-		if !reflect.DeepEqual(vars[i], someVars[i]) {
+		// By using Var.DeepEqual, we are protecting the code
+		// from a potential invocation of vars[i].ObjRef.GVK()
+		// during AccumulateTarget
+		if !vars[i].DeepEqual(someVars[i]) {
 			t.Fatalf("unexpected var[%d]:\n  %v\n  %v", i, vars[i], someVars[i])
 		}
 	}

pkg/types/var.go

@@ -18,6 +18,7 @@ package types
 
 import (
 	"fmt"
+	"reflect"
 	"sort"
 	"strings"
 
@@ -65,10 +66,22 @@ type FieldSelector struct {
 }
 
 // defaulting sets reference to field used by default.
-func (v *Var) defaulting() {
+func (v *Var) Defaulting() {
 	if v.FieldRef.FieldPath == "" {
 		v.FieldRef.FieldPath = defaultFieldPath
 	}
+	v.ObjRef.GVK()
+}
+
+// DeepEqual returns true if var a and b are Equals.
+// Note 1: The objects are unchanged by the VarEqual
+// Note 2: Should be normalize be FieldPath before doing
+// the DeepEqual. spec.a[b] is supposed to be the same
+// as spec.a.b
+func (v Var) DeepEqual(other Var) bool {
+	v.Defaulting()
+	other.Defaulting()
+	return reflect.DeepEqual(v, other)
 }
 
 // VarSet is a set of Vars where no var.Name is repeated.
@@ -130,11 +143,52 @@ func (vs *VarSet) Merge(v Var) error {
 		return fmt.Errorf(
 			"var '%s' already encountered", v.Name)
 	}
-	v.defaulting()
+	v.Defaulting()
 	vs.set[v.Name] = v
 	return nil
 }
 
+// AbsorbSet absorbs other vars with error on (name,value) collision.
+func (vs *VarSet) AbsorbSet(incoming VarSet) error {
+	for _, v := range incoming.set {
+		if err := vs.Absorb(v); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// AbsorbSlice absorbs a Var slice with error on (name,value) collision.
+// Empty fields in incoming vars are defaulted.
+func (vs *VarSet) AbsorbSlice(incoming []Var) error {
+	for _, v := range incoming {
+		if err := vs.Absorb(v); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// Absorb absorbs another Var with error on (name,value) collision.
+// Empty fields in incoming Var is defaulted.
+func (vs *VarSet) Absorb(v Var) error {
+	conflicting := vs.Get(v.Name)
+	if conflicting == nil {
+		// no conflict. The var is valid.
+		v.Defaulting()
+		vs.set[v.Name] = v
+		return nil
+	}
+
+	if !reflect.DeepEqual(v, *conflicting) {
+		// two vars with the same name are pointing at two
+		// different resources.
+		return fmt.Errorf(
+			"var '%s' already encountered", v.Name)
+	}
+	return nil
+}
+
 // Contains is true if the set has the other var.
 func (vs *VarSet) Contains(other Var) bool {
 	return vs.Get(other.Name) != nil

pkg/types/var_test.go

@@ -81,7 +81,7 @@ func TestDefaulting(t *testing.T) {
 			Name: "my-secret",
 		},
 	}
-	v.defaulting()
+	v.Defaulting()
 	if v.FieldRef.FieldPath != defaultFieldPath {
 		t.Fatalf("expected %s, got %v",
 			defaultFieldPath, v.FieldRef.FieldPath)