diff --git a/pkg/target/kusttarget_test.go b/pkg/target/kusttarget_test.go index bce667681..68bc55d7f 100644 --- a/pkg/target/kusttarget_test.go +++ b/pkg/target/kusttarget_test.go @@ -6,6 +6,7 @@ package target_test import ( "encoding/base64" "fmt" + "reflect" "strings" "testing" @@ -333,7 +334,12 @@ vars: t.Fatalf("unexpected size %d", len(vars)) } for i := range vars[:2] { - if !vars[i].DeepEqual(someVars[i]) { + // We have to enforce the Defaulting call in someVars[i] + // to protect from a potential call of vars[i].ObjRef.GVK() + // during AccumulateTarget + vars[i].Defaulting() + someVars[i].Defaulting() + if !reflect.DeepEqual(vars[i], someVars[i]) { t.Fatalf("unexpected var[%d]:\n %v\n %v", i, vars[i], someVars[i]) } } @@ -386,7 +392,9 @@ resources: t.Fatalf("expected 4 vars, got %d", len(vars)) } for i := range vars { - if !vars[i].DeepEqual(someVars[i]) { + vars[i].Defaulting() + someVars[i].Defaulting() + if !reflect.DeepEqual(vars[i], someVars[i]) { t.Fatalf("unexpected var[%d]:\n %v\n %v", i, vars[i], someVars[i]) } } diff --git a/pkg/types/var.go b/pkg/types/var.go index 3a911f625..1306c1b02 100644 --- a/pkg/types/var.go +++ b/pkg/types/var.go @@ -66,22 +66,21 @@ 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() } -// VarEquals returns true if var a and b are Equals. +// 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.ObjRef.GVK() - if v.FieldRef.FieldPath == "" { - v.FieldRef.FieldPath = "metadata.name" - } - other.ObjRef.GVK() - if other.FieldRef.FieldPath == "" { - other.FieldRef.FieldPath = "metadata.name" - } + v.Defaulting() + other.Defaulting() return reflect.DeepEqual(v, other) } @@ -144,7 +143,7 @@ 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 } @@ -152,7 +151,9 @@ func (vs *VarSet) Merge(v Var) error { // AbsorbSet absorbs other vars with error on (name,value) collision. func (vs *VarSet) AbsorbSet(incoming VarSet) error { for _, v := range incoming.set { - vs.Absorb(v) + if err := vs.Absorb(v); err != nil { + return err + } } return nil } @@ -174,12 +175,12 @@ func (vs *VarSet) Absorb(v Var) error { conflicting := vs.Get(v.Name) if conflicting == nil { // no conflict. The var is valid. - v.defaulting() + v.Defaulting() vs.set[v.Name] = v return nil } - if !v.DeepEqual(*conflicting) { + if !reflect.DeepEqual(v, *conflicting) { // two vars with the same name are pointing at two // different resources. return fmt.Errorf( diff --git a/pkg/types/var_test.go b/pkg/types/var_test.go index 95f799e02..efd3db7a9 100644 --- a/pkg/types/var_test.go +++ b/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)