diff --git a/api/internal/plugins/builtinconfig/namebackreferences.go b/api/internal/plugins/builtinconfig/namebackreferences.go index 36ef42c2725..eb0f307338f 100644 --- a/api/internal/plugins/builtinconfig/namebackreferences.go +++ b/api/internal/plugins/builtinconfig/namebackreferences.go @@ -47,6 +47,8 @@ type NameBackReferences struct { // 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"` + + // Note: If any new pointer based members are added, DeepCopy needs to be updated } func (n NameBackReferences) String() string { @@ -66,6 +68,17 @@ func (s nbrSlice) Less(i, j int) bool { return s[i].Gvk.IsLessThan(s[j].Gvk) } +// DeepCopy returns a new copy of nbrSlice +func (s nbrSlice) DeepCopy() nbrSlice { + ret := make(nbrSlice, len(s)) + copy(ret, s) + for i, slice := range ret { + ret[i].Referrers = slice.Referrers.DeepCopy() + } + + return ret +} + func (s nbrSlice) mergeAll(o nbrSlice) (result nbrSlice, err error) { result = s for _, r := range o { diff --git a/api/internal/plugins/builtinconfig/namebackreferences_test.go b/api/internal/plugins/builtinconfig/namebackreferences_test.go index b9b60369b98..0e51b80cff7 100644 --- a/api/internal/plugins/builtinconfig/namebackreferences_test.go +++ b/api/internal/plugins/builtinconfig/namebackreferences_test.go @@ -95,3 +95,29 @@ func TestMergeAll(t *testing.T) { t.Fatalf("expected\n %v\n but got\n %v\n", expected, actual) } } + +func TestNbrSlice_DeepCopy(t *testing.T) { + original := make(nbrSlice, 2, 4) + original[0] = NameBackReferences{Gvk: resid.FromKind("A"), Referrers: types.FsSlice{{Path: "a"}}} + original[1] = NameBackReferences{Gvk: resid.FromKind("B"), Referrers: types.FsSlice{{Path: "b"}}} + + copied := original.DeepCopy() + + original, _ = original.mergeOne(NameBackReferences{Gvk: resid.FromKind("C"), Referrers: types.FsSlice{{Path: "c"}}}) + + // perform mutations which should not affect original + copied.Swap(0, 1) + copied[0].Referrers[0].Path = "very b" // ensure Referrers are not shared + _, _ = copied.mergeOne(NameBackReferences{Gvk: resid.FromKind("D"), Referrers: types.FsSlice{{Path: "d"}}}) + + // if DeepCopy does not work, original would be {very b,a,d} instead of {a,b,c} + expected := nbrSlice{ + {Gvk: resid.FromKind("A"), Referrers: types.FsSlice{{Path: "a"}}}, + {Gvk: resid.FromKind("B"), Referrers: types.FsSlice{{Path: "b"}}}, + {Gvk: resid.FromKind("C"), Referrers: types.FsSlice{{Path: "c"}}}, + } + + if !reflect.DeepEqual(original, expected) { + t.Fatalf("original affected by mutations to copied object:\ngot\t%+v,\nexpected: %+v", original, expected) + } +} diff --git a/api/internal/plugins/builtinconfig/transformerconfig.go b/api/internal/plugins/builtinconfig/transformerconfig.go index 8b714d815f5..ab847eb6ecf 100644 --- a/api/internal/plugins/builtinconfig/transformerconfig.go +++ b/api/internal/plugins/builtinconfig/transformerconfig.go @@ -6,6 +6,7 @@ package builtinconfig import ( "log" "sort" + "sync" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/konfig/builtinpluginconsts" @@ -15,6 +16,7 @@ import ( // TransformerConfig holds the data needed to perform transformations. type TransformerConfig struct { + // if any fields are added, update the DeepCopy implementation NamePrefix types.FsSlice `json:"namePrefix,omitempty" yaml:"namePrefix,omitempty"` NameSuffix types.FsSlice `json:"nameSuffix,omitempty" yaml:"nameSuffix,omitempty"` NameSpace types.FsSlice `json:"namespace,omitempty" yaml:"namespace,omitempty"` @@ -32,14 +34,43 @@ func MakeEmptyConfig() *TransformerConfig { return &TransformerConfig{} } +// DeepCopy returns a new copy of TransformerConfig +func (t *TransformerConfig) DeepCopy() *TransformerConfig { + return &TransformerConfig{ + NamePrefix: t.NamePrefix.DeepCopy(), + NameSuffix: t.NameSuffix.DeepCopy(), + NameSpace: t.NameSpace.DeepCopy(), + CommonLabels: t.CommonLabels.DeepCopy(), + TemplateLabels: t.TemplateLabels.DeepCopy(), + CommonAnnotations: t.CommonAnnotations.DeepCopy(), + NameReference: t.NameReference.DeepCopy(), + VarReference: t.VarReference.DeepCopy(), + Images: t.Images.DeepCopy(), + Replicas: t.Replicas.DeepCopy(), + } +} + +// the default transformer config is initialized by MakeDefaultConfig, +// and must only be accessed via that function. +var ( + initDefaultConfig sync.Once //nolint:gochecknoglobals + defaultConfig *TransformerConfig //nolint:gochecknoglobals +) + // MakeDefaultConfig returns a default TransformerConfig. func MakeDefaultConfig() *TransformerConfig { - c, err := makeTransformerConfigFromBytes( - builtinpluginconsts.GetDefaultFieldSpecs()) - if err != nil { - log.Fatalf("Unable to make default transformconfig: %v", err) - } - return c + // parsing is expensive when having a large tree with many kustomization modules, so only do it once + initDefaultConfig.Do(func() { + var err error + defaultConfig, err = makeTransformerConfigFromBytes( + builtinpluginconsts.GetDefaultFieldSpecs()) + if err != nil { + log.Fatalf("Unable to make default transformconfig: %v", err) + } + }) + + // return a copy to avoid any mutations to protect the reference copy + return defaultConfig.DeepCopy() } // MakeTransformerConfig returns a merger of custom config, diff --git a/api/internal/plugins/builtinconfig/transformerconfig_test.go b/api/internal/plugins/builtinconfig/transformerconfig_test.go index 112a2d8cde9..e3b3bfe13a0 100644 --- a/api/internal/plugins/builtinconfig/transformerconfig_test.go +++ b/api/internal/plugins/builtinconfig/transformerconfig_test.go @@ -173,3 +173,22 @@ func TestMerge(t *testing.T) { t.Fatalf("expected: %v\n but got: %v\n", cfga, actual) } } + +func TestMakeDefaultConfig_mutation(t *testing.T) { + a := MakeDefaultConfig() + + // mutate + a.NameReference[0].Kind = "mutated" + a.NameReference = a.NameReference[:1] + + clean := MakeDefaultConfig() + if clean.NameReference[0].Kind == "mutated" { + t.Errorf("MakeDefaultConfig() did not return a clean copy: %+v", clean.NameReference) + } +} + +func BenchmarkMakeDefaultConfig(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = MakeDefaultConfig() + } +} diff --git a/api/types/fieldspec.go b/api/types/fieldspec.go index 8d357954479..8b78889ea01 100644 --- a/api/types/fieldspec.go +++ b/api/types/fieldspec.go @@ -30,6 +30,8 @@ type FieldSpec struct { resid.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"` Path string `json:"path,omitempty" yaml:"path,omitempty"` CreateIfNotPresent bool `json:"create,omitempty" yaml:"create,omitempty"` + + // Note: If any new pointer based members are added, FsSlice.DeepCopy needs to be updated } func (fs FieldSpec) String() string { @@ -50,6 +52,13 @@ func (s FsSlice) Less(i, j int) bool { return s[i].Gvk.IsLessThan(s[j].Gvk) } +// DeepCopy returns a new copy of FsSlice +func (s FsSlice) DeepCopy() FsSlice { + ret := make(FsSlice, len(s)) + copy(ret, s) + return ret +} + // MergeAll merges the argument into this, returning the result. // Items already present are ignored. // Items that conflict (primary key matches, but remain data differs) diff --git a/api/types/fieldspec_test.go b/api/types/fieldspec_test.go index c0518b94a9f..83edd73f169 100644 --- a/api/types/fieldspec_test.go +++ b/api/types/fieldspec_test.go @@ -142,3 +142,27 @@ func TestFsSlice_MergeAll(t *testing.T) { } } } + +func TestFsSlice_DeepCopy(t *testing.T) { + original := make(FsSlice, 2, 4) + original[0] = FieldSpec{Path: "a"} + original[1] = FieldSpec{Path: "b"} + + copied := original.DeepCopy() + + original, _ = original.MergeOne(FieldSpec{Path: "c"}) + + // perform mutations which should not affect original + copied.Swap(0, 1) + _, _ = copied.MergeOne(FieldSpec{Path: "d"}) + + // if DeepCopy does not work, original would be {b,a,d} instead of {a,b,c} + expected := FsSlice{ + {Path: "a"}, + {Path: "b"}, + {Path: "c"}, + } + if !reflect.DeepEqual(original, expected) { + t.Fatalf("original affected by mutations to copied object:\ngot\t%+v,\nexpected: %+v", original, expected) + } +}