diff --git a/api/ifc/ifc.go b/api/ifc/ifc.go index d5e125baa..e4321b5a6 100644 --- a/api/ifc/ifc.go +++ b/api/ifc/ifc.go @@ -43,12 +43,12 @@ type Kunstructured interface { // Several uses. Copy() Kunstructured - // Used by Resource.Replace, which in turn is used in many places, e.g. - // - resource.Resource.Merge - // - resWrangler.appendReplaceOrMerge (AbsorbAll) - // - api.internal.k8sdeps.transformer.patch.conflictdetector + // GetAnnotations returns the k8s annotations. GetAnnotations() map[string]string + // GetData returns a top-level "data" field, as in a ConfigMap. + GetDataMap() map[string]string + // Used by ResAccumulator and ReplacementTransformer. GetFieldValue(string) (interface{}, error) @@ -58,7 +58,7 @@ type Kunstructured interface { // Used by resource.Factory.SliceFromBytes GetKind() string - // Used by Resource.Replace + // GetLabels returns the k8s labels. GetLabels() map[string]string // Used by Resource.CurId and resource factory. @@ -84,19 +84,22 @@ type Kunstructured interface { // Used by resWrangler.Select MatchesLabelSelector(selector string) (bool, error) - // Used by Resource.Replace. + // SetAnnotations replaces the k8s annotations. SetAnnotations(map[string]string) + // SetDataMap sets a top-level "data" field, as in a ConfigMap. + SetDataMap(map[string]string) + // Used by PatchStrategicMergeTransformer. SetGvk(resid.Gvk) - // Used by Resource.Replace and used to remove "validated by" labels. + // SetLabels replaces the k8s labels. SetLabels(map[string]string) - // Used by Resource.Replace. + // SetName changes the name. SetName(string) - // Used by Resource.Replace. + // SetNamespace changes the namespace. SetNamespace(string) // Needed, for now, by kyaml/filtersutil.ApplyToJSON. diff --git a/api/internal/accumulator/resaccumulator.go b/api/internal/accumulator/resaccumulator.go index f1e744d94..bdf983eb5 100644 --- a/api/internal/accumulator/resaccumulator.go +++ b/api/internal/accumulator/resaccumulator.go @@ -41,13 +41,11 @@ func (ra *ResAccumulator) Vars() []types.Var { return ra.varSet.AsSlice() } -func (ra *ResAccumulator) AppendAll( - resources resmap.ResMap) error { +func (ra *ResAccumulator) AppendAll(resources resmap.ResMap) error { return ra.resMap.AppendAll(resources) } -func (ra *ResAccumulator) AbsorbAll( - resources resmap.ResMap) error { +func (ra *ResAccumulator) AbsorbAll(resources resmap.ResMap) error { return ra.resMap.AbsorbAll(resources) } diff --git a/api/internal/wrappy/wnode.go b/api/internal/wrappy/wnode.go index 0bbc48020..b00613936 100644 --- a/api/internal/wrappy/wnode.go +++ b/api/internal/wrappy/wnode.go @@ -108,6 +108,16 @@ func (wn *WNode) GetGvk() resid.Gvk { return resid.Gvk{Group: g, Version: v, Kind: meta.Kind} } +// GetDataMap implements ifc.Kunstructured. +func (wn *WNode) GetDataMap() map[string]string { + return wn.node.GetDataMap() +} + +// SetDataMap implements ifc.Kunstructured. +func (wn *WNode) SetDataMap(m map[string]string) { + wn.node.SetDataMap(m) +} + // GetKind implements ifc.Kunstructured. func (wn *WNode) GetKind() string { return wn.demandMetaData("GetKind").Kind diff --git a/api/k8sdeps/kunstruct/unstructadapter.go b/api/k8sdeps/kunstruct/unstructadapter.go index 7e123a5b5..f1424d91b 100644 --- a/api/k8sdeps/kunstruct/unstructadapter.go +++ b/api/k8sdeps/kunstruct/unstructadapter.go @@ -7,6 +7,7 @@ package kunstruct import ( "encoding/json" "fmt" + "log" jsonpatch "github.com/evanphx/json-patch" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -249,6 +250,29 @@ func (fs *UnstructAdapter) GetStringMap(path string) (map[string]string, error) return nil, NoFieldError{Field: path} } +func (fs *UnstructAdapter) GetDataMap() map[string]string { + m, err := fs.GetStringMap("data") + if err != nil { + return map[string]string{} + } + return m +} + +func (fs *UnstructAdapter) SetDataMap(m map[string]string) { + if m == nil { + unstructured.RemoveNestedField(fs.Object, "data") + return + } + s := make(map[string]interface{}, len(m)) + for i, v := range m { + s[i] = v + } + err := unstructured.SetNestedMap(fs.Object, s, "data") + if err != nil { + log.Fatal(err) + } +} + // GetMap returns value at the given fieldpath. func (fs *UnstructAdapter) GetMap(path string) (map[string]interface{}, error) { content, fields, found, err := fs.selectSubtree(path) diff --git a/api/k8sdeps/kunstruct/unstructadapter_test.go b/api/k8sdeps/kunstruct/unstructadapter_test.go index f1a7590e3..ec7b985b4 100644 --- a/api/k8sdeps/kunstruct/unstructadapter_test.go +++ b/api/k8sdeps/kunstruct/unstructadapter_test.go @@ -6,6 +6,8 @@ package kunstruct import ( "reflect" "testing" + + "github.com/stretchr/testify/assert" ) var kunstructured = NewKunstructuredFactoryImpl().FromMap(map[string]interface{}{ @@ -557,3 +559,139 @@ func compareValues(t *testing.T, name string, pathToField string, expectedValue t.Logf("%T value at `%s`", typedV, pathToField) } } + +func TestKunstGetDataMap(t *testing.T) { + emptyMap := map[string]string{} + testCases := map[string]struct { + theMap map[string]interface{} + expected map[string]string + }{ + "actuallyNil": { + theMap: nil, + expected: emptyMap, + }, + "empty": { + theMap: map[string]interface{}{}, + expected: emptyMap, + }, + "mostlyEmpty": { + theMap: map[string]interface{}{ + "hey": "there", + }, + expected: emptyMap, + }, + "noNameConfigMap": { + theMap: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + }, + expected: emptyMap, + }, + "configMap": { + theMap: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "winnie", + }, + "data": map[string]interface{}{ + "wine": "cabernet", + "truck": "ford", + "rocket": "falcon9", + "planet": "mars", + "city": "brownsville", + }, + }, + // order irrelevant, because assert.Equals is smart about maps. + expected: map[string]string{ + "city": "brownsville", + "wine": "cabernet", + "planet": "mars", + "rocket": "falcon9", + "truck": "ford", + }, + }, + } + + for n := range testCases { + tc := testCases[n] + t.Run(n, func(t *testing.T) { + kunstr := NewKunstructuredFactoryImpl().FromMap(tc.theMap) + m := kunstr.GetDataMap() + if !assert.Equal(t, tc.expected, m) { + t.FailNow() + } + }) + } +} + +func TestKunstSetDataMap(t *testing.T) { + testCases := map[string]struct { + theMap map[string]interface{} + input map[string]string + expected map[string]string + }{ + "empty": { + theMap: map[string]interface{}{}, + input: map[string]string{ + "wine": "cabernet", + "truck": "ford", + }, + expected: map[string]string{ + "wine": "cabernet", + "truck": "ford", + }, + }, + "replace": { + theMap: map[string]interface{}{ + "foo": 3, + "data": map[string]string{ + "rocket": "falcon9", + "planet": "mars", + }, + }, + input: map[string]string{ + "wine": "cabernet", + "truck": "ford", + }, + expected: map[string]string{ + "wine": "cabernet", + "truck": "ford", + }, + }, + "clear1": { + theMap: map[string]interface{}{ + "foo": 3, + "data": map[string]string{ + "rocket": "falcon9", + "planet": "mars", + }, + }, + input: map[string]string{}, + expected: map[string]string{}, + }, + "clear2": { + theMap: map[string]interface{}{ + "foo": 3, + "data": map[string]string{ + "rocket": "falcon9", + "planet": "mars", + }, + }, + input: nil, + expected: map[string]string{}, + }, + } + + for n := range testCases { + tc := testCases[n] + t.Run(n, func(t *testing.T) { + kunstr := NewKunstructuredFactoryImpl().FromMap(tc.theMap) + kunstr.SetDataMap(tc.input) + m := kunstr.GetDataMap() + if !assert.Equal(t, tc.expected, m) { + t.FailNow() + } + }) + } +} diff --git a/api/krusty/configmaps_test.go b/api/krusty/configmaps_test.go index a39daa68f..4afa4c7a1 100644 --- a/api/krusty/configmaps_test.go +++ b/api/krusty/configmaps_test.go @@ -170,6 +170,39 @@ metadata: `) } +func TestGeneratorSimpleOverlay(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteK("base", ` +namePrefix: p- +configMapGenerator: +- name: cm + behavior: create + literals: + - fruit=apple +`) + th.WriteK("overlay", ` +resources: +- ../base +configMapGenerator: +- name: cm + behavior: merge + literals: + - veggie=broccoli +`) + m := th.Run("overlay", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +data: + fruit: apple + veggie: broccoli +kind: ConfigMap +metadata: + annotations: {} + labels: {} + name: p-cm-877mt5hc89 +`) +} + func TestGeneratorOverlays(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK("/app/base1", ` diff --git a/api/krusty/options.go b/api/krusty/options.go index 1811179ea..85e72dac2 100644 --- a/api/krusty/options.go +++ b/api/krusty/options.go @@ -43,6 +43,7 @@ type Options struct { } // MakeDefaultOptions returns a default instance of Options. +// TODO(#3343): UseKyaml: konfig.FlagEnableKyamlDefaultValue func MakeDefaultOptions() *Options { return &Options{ DoLegacyResourceSort: false, diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index c6c894141..2b2bde6b7 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -456,8 +456,7 @@ func (m *resWrangler) AbsorbAll(other ResMap) error { return nil } -func (m *resWrangler) appendReplaceOrMerge( - res *resource.Resource) error { +func (m *resWrangler) appendReplaceOrMerge(res *resource.Resource) error { id := res.CurId() matches := m.GetMatchingResourcesByOriginalId(id.Equals) if len(matches) == 0 { @@ -471,10 +470,7 @@ func (m *resWrangler) appendReplaceOrMerge( "id %#v does not exist; cannot merge or replace", id) default: // presumably types.BehaviorCreate - err := m.Append(res) - if err != nil { - return err - } + return m.Append(res) } case 1: old := matches[0] @@ -487,9 +483,10 @@ func (m *resWrangler) appendReplaceOrMerge( } switch res.Behavior() { case types.BehaviorReplace: - res.Replace(old) + res.CopyMergeMetaDataFieldsFrom(old) case types.BehaviorMerge: - res.Merge(old) + res.CopyMergeMetaDataFieldsFrom(old) + res.MergeDataMapFrom(old) default: return fmt.Errorf( "id %#v exists; behavior must be merge or replace", id) @@ -499,14 +496,14 @@ func (m *resWrangler) appendReplaceOrMerge( return err } if i != index { - return fmt.Errorf("unexpected index in replacement") + return fmt.Errorf("unexpected target index in replacement") } + return nil default: return fmt.Errorf( "found multiple objects %v that could accept merge of %v", matches, id) } - return nil } // Select returns a list of resources that diff --git a/api/resource/resource.go b/api/resource/resource.go index 8376124ad..60570b30a 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -46,6 +46,10 @@ func (r *Resource) GetFieldValue(f string) (interface{}, error) { return r.kunStr.GetFieldValue(f) } +func (r *Resource) GetDataMap() map[string]string { + return r.kunStr.GetDataMap() +} + func (r *Resource) GetGvk() resid.Gvk { return r.kunStr.GetGvk() } @@ -94,6 +98,10 @@ func (r *Resource) SetAnnotations(m map[string]string) { r.kunStr.SetAnnotations(m) } +func (r *Resource) SetDataMap(m map[string]string) { + r.kunStr.SetDataMap(m) +} + func (r *Resource) SetGvk(gvk resid.Gvk) { r.kunStr.SetGvk(gvk) } @@ -139,10 +147,12 @@ func (r *Resource) DeepCopy() *Resource { return rc } -// Replace performs replace with other resource. -func (r *Resource) Replace(other *Resource) { +// CopyMergeMetaDataFields copies everything but the non-metadata in +// the ifc.Kunstructured map, merging labels and annotations. +func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) { r.SetLabels(mergeStringMaps(other.GetLabels(), r.GetLabels())) - r.SetAnnotations(mergeStringMaps(other.GetAnnotations(), r.GetAnnotations())) + r.SetAnnotations( + mergeStringMaps(other.GetAnnotations(), r.GetAnnotations())) r.SetName(other.GetName()) r.SetNamespace(other.GetNamespace()) r.copyOtherFields(other) @@ -158,6 +168,10 @@ func (r *Resource) copyOtherFields(other *Resource) { r.nameSuffixes = copyStringSlice(other.nameSuffixes) } +func (r *Resource) MergeDataMapFrom(o *Resource) { + r.SetDataMap(mergeStringMaps(o.GetDataMap(), r.GetDataMap())) +} + func (r *Resource) ErrIfNotEquals(o *Resource) error { meYaml, err := r.AsYAML() if err != nil { @@ -198,12 +212,6 @@ func (r *Resource) KunstructEqual(o *Resource) bool { return reflect.DeepEqual(r.kunStr, o.kunStr) } -// Merge performs merge with other resource. -func (r *Resource) Merge(other *Resource) { - r.Replace(other) - mergeConfigmap(r.Map(), other.Map(), r.Map()) -} - func (r *Resource) copyRefBy() []resid.ResId { if r.refBy == nil { return nil @@ -414,22 +422,6 @@ func (r *Resource) ApplySmPatch(patch *Resource) error { return err } -// TODO: Add BinaryData once we sync to new k8s.io/api -func mergeConfigmap( - mergedTo map[string]interface{}, - maps ...map[string]interface{}) { - mergedMap := map[string]interface{}{} - for _, m := range maps { - datamap, ok := m["data"].(map[string]interface{}) - if ok { - for key, value := range datamap { - mergedMap[key] = value - } - } - } - mergedTo["data"] = mergedMap -} - func mergeStringMaps(maps ...map[string]string) map[string]string { result := map[string]string{} for _, m := range maps { diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index 47e495632..28c426211 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -281,6 +281,42 @@ spec: `, string(bytes)) } +func TestMergeDataMapFrom(t *testing.T) { + resource, err := factory.FromBytes([]byte(` +apiVersion: v1 +kind: BlahBlah +metadata: + name: clown +data: + fruit: pear +`)) + if !assert.NoError(t, err) { + t.FailNow() + } + patch, err := factory.FromBytes([]byte(` +apiVersion: v1 +kind: Whatever +metadata: + name: spaceship +data: + spaceship: enterprise +`)) + if !assert.NoError(t, err) { + t.FailNow() + } + resource.MergeDataMapFrom(patch) + bytes, err := resource.AsYAML() + assert.NoError(t, err) + assert.Equal(t, `apiVersion: v1 +data: + fruit: pear + spaceship: enterprise +kind: BlahBlah +metadata: + name: clown +`, string(bytes)) +} + func TestApplySmPatch_SwapOrder(t *testing.T) { s1 := ` apiVersion: example.com/v1