Add kunstruct impl of Get/SetDataMap, replace Resource.Merge

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.
+	// GetAnnotations returns the k8s annotations.
-	//  - resource.Resource.Merge
-	//  - resWrangler.appendReplaceOrMerge (AbsorbAll)
-	//  - api.internal.k8sdeps.transformer.patch.conflictdetector
 	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.

api/internal/accumulator/resaccumulator.go

@@ -41,13 +41,11 @@ func (ra *ResAccumulator) Vars() []types.Var {
 	return ra.varSet.AsSlice()
 }
 
-func (ra *ResAccumulator) AppendAll(
+func (ra *ResAccumulator) AppendAll(resources resmap.ResMap) error {
-	resources resmap.ResMap) error {
 	return ra.resMap.AppendAll(resources)
 }
 
-func (ra *ResAccumulator) AbsorbAll(
+func (ra *ResAccumulator) AbsorbAll(resources resmap.ResMap) error {
-	resources resmap.ResMap) error {
 	return ra.resMap.AbsorbAll(resources)
 }
 

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

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)

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()
+			}
+		})
+	}
+}

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", `

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,

api/resmap/reswrangler.go

@@ -456,8 +456,7 @@ func (m *resWrangler) AbsorbAll(other ResMap) error {
 	return nil
 }
 
-func (m *resWrangler) appendReplaceOrMerge(
+func (m *resWrangler) appendReplaceOrMerge(res *resource.Resource) error {
-	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)
+			return m.Append(res)
-			if err != nil {
-				return err
-			}
 		}
 	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

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.
+// CopyMergeMetaDataFields copies everything but the non-metadata in
-func (r *Resource) Replace(other *Resource) {
+// 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 {

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