Merge pull request #3888 from monopole/popRNodeUp

Remove delegation to RNode in Resource

api/builtins/NamespaceTransformer.go

@@ -30,7 +30,7 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error {
 		return nil
 	}
 	for _, r := range m.Resources() {
-		if r.IsEmpty() {
+		if r.IsNilOrEmpty() {
 			// Don't mutate empty objects?
 			continue
 		}

api/filters/fieldspec/fieldspec.go

@@ -50,8 +50,7 @@ func (fltr Filter) Filter(obj *yaml.RNode) (*yaml.RNode, error) {
 		return obj, nil
 	}
 	fltr.path = utils.PathSplitter(fltr.FieldSpec.Path)
-	err := fltr.filter(obj)
-	if err != nil {
+	if err := fltr.filter(obj); err != nil {
 		s, _ := obj.String()
 		return nil, errors.WrapPrefixf(err,
 			"considering field '%s' of object\n%v", fltr.FieldSpec.Path, s)

api/filters/nameref/nameref.go

@@ -184,7 +184,7 @@ func (f Filter) recordTheReferral(referral *resource.Resource) {
 
 // getRoleRefGvk returns a Gvk in the roleRef field. Return error
 // if the roleRef, roleRef/apiGroup or roleRef/kind is missing.
-func getRoleRefGvk(n *yaml.RNode) (*resid.Gvk, error) {
+func getRoleRefGvk(n *resource.Resource) (*resid.Gvk, error) {
 	roleRef, err := n.Pipe(yaml.Lookup("roleRef"))
 	if err != nil {
 		return nil, err
@@ -269,7 +269,7 @@ func (f Filter) roleRefFilter() sieveFunc {
 	if !strings.HasSuffix(f.NameFieldToUpdate.Path, "roleRef/name") {
 		return acceptAll
 	}
-	roleRefGvk, err := getRoleRefGvk(f.Referrer.AsRNode())
+	roleRefGvk, err := getRoleRefGvk(f.Referrer)
 	if err != nil {
 		return acceptAll
 	}

api/filters/replacement/replacement.go

@@ -168,11 +168,6 @@ func selectSourceNode(nodes []*yaml.RNode, selector *types.SourceSelector) (*yam
 
 // makeResId makes a ResId from an RNode.
 func makeResId(n *yaml.RNode) *resid.ResId {
-	ns, err := n.GetNamespace()
-	if err != nil {
-		// Resource has no metadata (no apiVersion, kind, nor metadata field).
-		return nil
-	}
 	apiVersion := n.Field(yaml.APIVersionField)
 	var group, version string
 	if apiVersion != nil {
@@ -181,6 +176,6 @@ func makeResId(n *yaml.RNode) *resid.ResId {
 	return &resid.ResId{
 		Gvk:       resid.Gvk{Group: group, Version: version, Kind: n.GetKind()},
 		Name:      n.GetName(),
-		Namespace: ns,
+		Namespace: n.GetNamespace(),
 	}
 }

api/internal/accumulator/resaccumulator.go

@@ -107,6 +107,7 @@ func (ra *ResAccumulator) findVarValueFromResources(v types.Var) (interface{}, e
 	for _, res := range ra.resMap.Resources() {
 		for _, varName := range res.GetRefVarNames() {
 			if varName == v.Name {
+				//nolint: staticcheck
 				s, err := res.GetFieldValue(v.FieldRef.FieldPath)
 				if err != nil {
 					return "", fmt.Errorf(

api/internal/plugins/utils/utils.go

@@ -137,7 +137,9 @@ func GetResMapWithIDAnnotation(rm resmap.ResMap) (resmap.ResMap, error) {
 		}
 		annotations := r.GetAnnotations()
 		annotations[idAnnotation] = string(idString)
-		r.SetAnnotations(annotations)
+		if err = r.SetAnnotations(annotations); err != nil {
+			return nil, err
+		}
 	}
 	return inputRM, nil
 }
@@ -159,7 +161,9 @@ func UpdateResMapValues(pluginName string, h *resmap.PluginHelpers, output []byt
 
 	for _, r := range resources {
 		// stale--not manipulated by plugin transformers
-		removeIDAnnotation(r)
+		if err = removeIDAnnotation(r); err != nil {
+			return err
+		}
 
 		// Add to the new map, checking for duplicates
 		if err := newMap.Append(r); err != nil {
@@ -176,7 +180,7 @@ func UpdateResMapValues(pluginName string, h *resmap.PluginHelpers, output []byt
 			return err
 		}
 		if oldIdx != -1 {
-			rm.GetByIndex(oldIdx).ResetPrimaryData(r)
+			rm.GetByIndex(oldIdx).ResetRNode(r)
 		} else {
 			if err := rm.Append(r); err != nil {
 				return err
@@ -195,14 +199,11 @@ func UpdateResMapValues(pluginName string, h *resmap.PluginHelpers, output []byt
 	return nil
 }
 
-func removeIDAnnotation(r *resource.Resource) {
+func removeIDAnnotation(r *resource.Resource) error {
 	// remove the annotation set by Kustomize to track the resource
 	annotations := r.GetAnnotations()
 	delete(annotations, idAnnotation)
-	if len(annotations) == 0 {
-		annotations = nil
-	}
-	r.SetAnnotations(annotations)
+	return r.SetAnnotations(annotations)
 }
 
 // UpdateResourceOptions updates the generator options for each resource in the
@@ -225,10 +226,9 @@ func UpdateResourceOptions(rm resmap.ResMap) (resmap.ResMap, error) {
 		}
 		delete(annotations, HashAnnotation)
 		delete(annotations, BehaviorAnnotation)
-		if len(annotations) == 0 {
-			annotations = nil
+		if err := r.SetAnnotations(annotations); err != nil {
+			return nil, err
 		}
-		r.SetAnnotations(annotations)
 		r.SetOptions(types.NewGenArgs(
 			&types.GeneratorArgs{
 				Behavior: behavior,

api/internal/plugins/utils/utils_test.go

@@ -45,7 +45,9 @@ func makeConfigMap(rf *resource.Factory, name, behavior string, hashValue *strin
 		annotations[HashAnnotation] = *hashValue
 	}
 	if len(annotations) > 0 {
-		r.SetAnnotations(annotations)
+		if err := r.SetAnnotations(annotations); err != nil {
+			panic(err)
+		}
 	}
 	return r
 }

api/internal/target/kusttarget.go

@@ -303,16 +303,18 @@ func (kt *KustTarget) runValidators(ra *accumulator.ResAccumulator) error {
 		if err != nil {
 			return err
 		}
-		new := ra.ResMap().DeepCopy()
-		kt.removeValidatedByLabel(new)
-		if err = orignal.ErrorIfNotEqualSets(new); err != nil {
+		newMap := ra.ResMap().DeepCopy()
+		if err = kt.removeValidatedByLabel(newMap); err != nil {
+			return err
+		}
+		if err = orignal.ErrorIfNotEqualSets(newMap); err != nil {
 			return fmt.Errorf("validator shouldn't modify the resource map: %v", err)
 		}
 	}
 	return nil
 }
 
-func (kt *KustTarget) removeValidatedByLabel(rm resmap.ResMap) {
+func (kt *KustTarget) removeValidatedByLabel(rm resmap.ResMap) error {
 	resources := rm.Resources()
 	for _, r := range resources {
 		labels := r.GetLabels()
@@ -320,12 +322,11 @@ func (kt *KustTarget) removeValidatedByLabel(rm resmap.ResMap) {
 			continue
 		}
 		delete(labels, konfig.ValidatedByLabelKey)
-		if len(labels) == 0 {
-			r.SetLabels(nil)
-		} else {
-			r.SetLabels(labels)
+		if err := r.SetLabels(labels); err != nil {
+			return err
 		}
 	}
+	return nil
 }
 
 // accumulateResources fills the given resourceAccumulator

api/resmap/reswrangler.go

@@ -6,6 +6,7 @@ package resmap
 import (
 	"bytes"
 	"fmt"
+	"reflect"
 
 	"github.com/pkg/errors"
 	"sigs.k8s.io/kustomize/api/resource"
@@ -42,7 +43,7 @@ func (m *resWrangler) Clear() {
 func (m *resWrangler) DropEmpties() {
 	var rList []*resource.Resource
 	for _, r := range m.rList {
-		if !r.IsEmpty() {
+		if !r.IsNilOrEmpty() {
 			rList = append(rList, r)
 		}
 	}
@@ -324,7 +325,7 @@ func (m *resWrangler) ErrorIfNotEqualSets(other ResMap) error {
 				"id in self matches %d in other; id: %s", len(others), id)
 		}
 		r2 := others[0]
-		if !r1.NodeEqual(r2) {
+		if !reflect.DeepEqual(r1.RNode, r2.RNode) {
 			return fmt.Errorf(
 				"nodes unequal: \n -- %s,\n -- %s\n\n--\n%#v\n------\n%#v\n",
 				r1, r2, r1, r2)
@@ -587,7 +588,7 @@ func (m *resWrangler) Select(s types.Selector) ([]*resource.Resource, error) {
 func (m *resWrangler) ToRNodeSlice() []*kyaml.RNode {
 	result := make([]*kyaml.RNode, len(m.rList))
 	for i := range m.rList {
-		result[i] = m.rList[i].AsRNode()
+		result[i] = m.rList[i].Copy()
 	}
 	return result
 }
@@ -606,7 +607,7 @@ func (m *resWrangler) ApplySmPatch(
 				return err
 			}
 		}
-		if !res.IsEmpty() {
+		if !res.IsNilOrEmpty() {
 			list = append(list, res)
 		}
 	}
@@ -625,7 +626,7 @@ func (m *resWrangler) ApplyFilter(f kio.Filter) error {
 	reverseLookup := make(map[*kyaml.RNode]*resource.Resource, len(m.rList))
 	nodes := make([]*kyaml.RNode, len(m.rList))
 	for i, r := range m.rList {
-		ptr := r.Node()
+		ptr := &(r.RNode)
 		nodes[i] = ptr
 		reverseLookup[ptr] = r
 	}
@@ -646,11 +647,13 @@ func (m *resWrangler) ApplyFilter(f kio.Filter) error {
 		res, ok := reverseLookup[rn]
 		if !ok {
 			// A node was created; make a Resource to wrap it.
-			// Leave remaining Resource fields empty.
-			// At time of writing, seeking to eliminate those fields.
-			// Alternatively, could just return error on creation attempt
-			// until remaining fields eliminated.
-			res = resource.NewResource(rn)
+			res = &resource.Resource{
+				RNode: *rn,
+				// Leave remaining fields empty.
+				// At at time of writing, seeking to eliminate those fields.
+				// Alternatively, could just return error on creation attempt
+				// until remaining fields eliminated.
+			}
 		}
 		nRList = append(nRList, res)
 	}

api/resource/factory.go

@@ -69,7 +69,7 @@ func (rf *Factory) makeOne(rn *yaml.RNode, o *types.GenArgs) *Resource {
 	if o == nil {
 		o = types.NewGenArgs(nil)
 	}
-	return &Resource{node: rn, options: o}
+	return &Resource{RNode: *rn, options: o}
 }
 
 // SliceFromPatches returns a slice of resources given a patch path

api/resource/resource.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"log"
-	"reflect"
 	"strings"
 
 	"sigs.k8s.io/kustomize/api/filters/patchstrategicmerge"
@@ -23,8 +22,7 @@ import (
 // Resource is an RNode, representing a Kubernetes Resource Model object,
 // paired with metadata used by kustomize.
 type Resource struct {
-	// TODO: Inline RNode, dropping complexity. Resource is just a decorator.
-	node        *kyaml.RNode
+	kyaml.RNode
 	options     *types.GenArgs
 	refBy       []resid.ResId
 	refVarNames []string
@@ -54,146 +52,21 @@ var buildAnnotations = []string{
 	buildAnnotationAllowKindChange,
 }
 
-func NewResource(rn *kyaml.RNode) *Resource {
-	return &Resource{node: rn}
-}
-
-func (r *Resource) AsRNode() *kyaml.RNode {
-	return r.node.Copy()
-}
-
-func (r *Resource) Node() *kyaml.RNode {
-	return r.node
-}
-
-func (r *Resource) ResetPrimaryData(incoming *Resource) {
-	r.node = incoming.node.Copy()
-}
-
-func (r *Resource) GetAnnotations() map[string]string {
-	annotations, err := r.node.GetAnnotations()
-	if err != nil || annotations == nil {
-		return make(map[string]string)
-	}
-	return annotations
-}
-
-func (r *Resource) GetFieldValue(f string) (interface{}, error) {
-	//nolint:staticcheck
-	return r.node.GetFieldValue(f)
-}
-
-func (r *Resource) GetDataMap() map[string]string {
-	return r.node.GetDataMap()
-}
-
-func (r *Resource) GetBinaryDataMap() map[string]string {
-	return r.node.GetBinaryDataMap()
+func (r *Resource) ResetRNode(incoming *Resource) {
+	r.RNode = *incoming.Copy()
 }
 
 func (r *Resource) GetGvk() resid.Gvk {
-	return resid.GvkFromNode(r.node)
+	return resid.GvkFromNode(&r.RNode)
 }
 
 func (r *Resource) Hash(h ifc.KustHasher) (string, error) {
-	return h.Hash(r.node)
-}
-
-func (r *Resource) GetKind() string {
-	return r.node.GetKind()
-}
-
-func (r *Resource) GetLabels() map[string]string {
-	l, err := r.node.GetLabels()
-	if err != nil {
-		return map[string]string{}
-	}
-	return l
-}
-
-func (r *Resource) GetName() string {
-	return r.node.GetName()
-}
-
-func (r *Resource) GetSlice(p string) ([]interface{}, error) {
-	//nolint:staticcheck
-	return r.node.GetSlice(p)
-}
-
-func (r *Resource) GetString(p string) (string, error) {
-	//nolint:staticcheck
-	return r.node.GetString(p)
-}
-
-func (r *Resource) IsEmpty() bool {
-	return r.node.IsNilOrEmpty()
-}
-
-func (r *Resource) Map() (map[string]interface{}, error) {
-	return r.node.Map()
-}
-
-func (r *Resource) MarshalJSON() ([]byte, error) {
-	return r.node.MarshalJSON()
-}
-
-func (r *Resource) MatchesLabelSelector(selector string) (bool, error) {
-	return r.node.MatchesLabelSelector(selector)
-}
-
-func (r *Resource) MatchesAnnotationSelector(selector string) (bool, error) {
-	return r.node.MatchesAnnotationSelector(selector)
-}
-
-func (r *Resource) SetAnnotations(m map[string]string) {
-	if len(m) == 0 {
-		// Force field erasure.
-		r.node.SetAnnotations(nil)
-		return
-	}
-	r.node.SetAnnotations(m)
-}
-
-func (r *Resource) SetDataMap(m map[string]string) {
-	r.node.SetDataMap(m)
-}
-
-func (r *Resource) SetBinaryDataMap(m map[string]string) {
-	r.node.SetBinaryDataMap(m)
+	return h.Hash(&r.RNode)
 }
 
 func (r *Resource) SetGvk(gvk resid.Gvk) {
-	r.node.SetMapField(
-		kyaml.NewScalarRNode(gvk.Kind), kyaml.KindField)
-	r.node.SetMapField(
-		kyaml.NewScalarRNode(gvk.ApiVersion()), kyaml.APIVersionField)
-}
-
-func (r *Resource) SetLabels(m map[string]string) {
-	if len(m) == 0 {
-		// Force field erasure.
-		r.node.SetLabels(nil)
-		return
-	}
-	r.node.SetLabels(m)
-}
-
-func (r *Resource) SetName(n string) {
-	r.node.SetName(n)
-}
-
-func (r *Resource) SetNamespace(n string) {
-	r.node.SetNamespace(n)
-}
-
-func (r *Resource) SetKind(k string) {
-	gvk := r.GetGvk()
-	gvk.Kind = k
-	r.SetGvk(gvk)
-}
-
-func (r *Resource) UnmarshalJSON(s []byte) error {
-	return r.node.UnmarshalJSON(s)
+	r.SetKind(gvk.Kind)
+	r.SetApiVersion(gvk.ApiVersion())
 }
 
 // ResCtx is an interface describing the contextual added
@@ -213,7 +86,7 @@ type ResCtxMatcher func(ResCtx) bool
 // DeepCopy returns a new copy of resource
 func (r *Resource) DeepCopy() *Resource {
 	rc := &Resource{
-		node: r.node.Copy(),
+		RNode: *r.Copy(),
 	}
 	rc.copyKustomizeSpecificFields(r)
 	return rc
@@ -221,13 +94,25 @@ func (r *Resource) DeepCopy() *Resource {
 
 // CopyMergeMetaDataFieldsFrom copies everything but the non-metadata in
 // the resource.
-func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) {
-	r.SetLabels(mergeStringMaps(other.GetLabels(), r.GetLabels()))
-	r.SetAnnotations(
-		mergeStringMaps(other.GetAnnotations(), r.GetAnnotations()))
-	r.SetName(other.GetName())
-	r.SetNamespace(other.GetNamespace())
+// TODO: move to RNode, use GetMeta to improve performance.
+// Must remove the kustomize bit at the end.
+func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) error {
+	if err := r.SetLabels(
+		mergeStringMaps(other.GetLabels(), r.GetLabels())); err != nil {
+		return fmt.Errorf("copyMerge cannot set labels - %w", err)
+	}
+	if err := r.SetAnnotations(
+		mergeStringMaps(other.GetAnnotations(), r.GetAnnotations())); err != nil {
+		return fmt.Errorf("copyMerge cannot set annotations - %w", err)
+	}
+	if err := r.SetName(other.GetName()); err != nil {
+		return fmt.Errorf("copyMerge cannot set name - %w", err)
+	}
+	if err := r.SetNamespace(other.GetNamespace()); err != nil {
+		return fmt.Errorf("copyMerge cannot set namespace - %w", err)
+	}
 	r.copyKustomizeSpecificFields(other)
+	return nil
 }
 
 func (r *Resource) copyKustomizeSpecificFields(other *Resource) {
@@ -286,12 +171,6 @@ func (r *Resource) ReferencesEqual(other *Resource) bool {
 	return len(setSelf) == len(setOther)
 }
 
-// NodeEqual returns true if the resource's nodes are
-// equal, ignoring ancillary information like genargs, refby, etc.
-func (r *Resource) NodeEqual(o *Resource) bool {
-	return reflect.DeepEqual(r.node, o.node)
-}
-
 func (r *Resource) copyRefBy() []resid.ResId {
 	if r.refBy == nil {
 		return nil
@@ -330,7 +209,9 @@ func (r *Resource) appendCsvAnnotation(name, value string) {
 	} else {
 		annotations[name] = value
 	}
-	r.SetAnnotations(annotations)
+	if err := r.SetAnnotations(annotations); err != nil {
+		panic(err)
+	}
 }
 
 func SameEndingSubarray(shortest, longest []string) bool {
@@ -385,7 +266,9 @@ func (r *Resource) RemoveBuildAnnotations() {
 	for _, a := range buildAnnotations {
 		delete(annotations, a)
 	}
-	r.SetAnnotations(annotations)
+	if err := r.SetAnnotations(annotations); err != nil {
+		panic(err)
+	}
 }
 
 func (r *Resource) setPreviousId(ns string, n string, k string) *Resource {
@@ -395,10 +278,13 @@ func (r *Resource) setPreviousId(ns string, n string, k string) *Resource {
 	return r
 }
 
+// AllowNameChange allows name changes to the resource.
 func (r *Resource) AllowNameChange() {
 	annotations := r.GetAnnotations()
 	annotations[buildAnnotationAllowNameChange] = allowed
-	r.SetAnnotations(annotations)
+	if err := r.SetAnnotations(annotations); err != nil {
+		panic(err)
+	}
 }
 
 func (r *Resource) NameChangeAllowed() bool {
@@ -407,10 +293,13 @@ func (r *Resource) NameChangeAllowed() bool {
 	return ok && v == allowed
 }
 
+// AllowKindChange allows kind changes to the resource.
 func (r *Resource) AllowKindChange() {
 	annotations := r.GetAnnotations()
 	annotations[buildAnnotationAllowKindChange] = allowed
-	r.SetAnnotations(annotations)
+	if err := r.SetAnnotations(annotations); err != nil {
+		panic(err)
+	}
 }
 
 func (r *Resource) KindChangeAllowed() bool {
@@ -463,13 +352,6 @@ func (r *Resource) NeedHashSuffix() bool {
 	return r.options != nil && r.options.ShouldAddHashSuffixToName()
 }
 
-// GetNamespace returns the namespace the resource thinks it's in.
-func (r *Resource) GetNamespace() string {
-	namespace, _ := r.GetString("metadata.namespace")
-	// if err, namespace is empty, so no need to check.
-	return namespace
-}
-
 // OrgId returns the original, immutable ResId for the resource.
 // This doesn't have to be unique in a ResMap.
 func (r *Resource) OrgId() resid.ResId {
@@ -550,11 +432,11 @@ func (r *Resource) ApplySmPatch(patch *Resource) error {
 		r.StorePreviousId()
 	}
 	if err := r.ApplyFilter(patchstrategicmerge.Filter{
-		Patch: patch.node,
+		Patch: &patch.RNode,
 	}); err != nil {
 		return err
 	}
-	if r.IsEmpty() {
+	if r.IsNilOrEmpty() {
 		return nil
 	}
 	if !patch.KindChangeAllowed() {
@@ -568,10 +450,11 @@ func (r *Resource) ApplySmPatch(patch *Resource) error {
 }
 
 func (r *Resource) ApplyFilter(f kio.Filter) error {
-	l, err := f.Filter([]*kyaml.RNode{r.node})
+	l, err := f.Filter([]*kyaml.RNode{&r.RNode})
 	if len(l) == 0 {
-		// The node was deleted.  The following makes r.IsEmpty() true.
-		r.node = nil
+		// The node was deleted, which means the entire resource
+		// must be deleted.  Signal that via the following:
+		r.SetYNode(nil)
 	}
 	return err
 }