From 5c4e363f119de2b692637890412bd0938f7d50b4 Mon Sep 17 00:00:00 2001 From: monopole Date: Fri, 7 May 2021 15:44:52 -0700 Subject: [PATCH] Remove delegation to RNode in Resource. --- api/builtins/NamespaceTransformer.go | 2 +- api/filters/fieldspec/fieldspec.go | 3 +- api/filters/nameref/nameref.go | 4 +- api/filters/replacement/replacement.go | 7 +- api/internal/accumulator/resaccumulator.go | 1 + api/internal/plugins/utils/utils.go | 22 +- api/internal/plugins/utils/utils_test.go | 4 +- api/internal/target/kusttarget.go | 17 +- api/resmap/reswrangler.go | 23 +- api/resource/factory.go | 2 +- api/resource/resource.go | 209 ++++-------------- kyaml/yaml/rnode.go | 110 ++++++--- kyaml/yaml/rnode_test.go | 68 ++---- .../NamespaceTransformer.go | 2 +- 14 files changed, 187 insertions(+), 287 deletions(-) diff --git a/api/builtins/NamespaceTransformer.go b/api/builtins/NamespaceTransformer.go index 6794f26e81..39a514e8e5 100644 --- a/api/builtins/NamespaceTransformer.go +++ b/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 } diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go index 09bd2e2dd9..cbc8776fe7 100644 --- a/api/filters/fieldspec/fieldspec.go +++ b/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) diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index 247c94dd8e..850059fb22 100644 --- a/api/filters/nameref/nameref.go +++ b/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 } diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 16d0c979c5..75fbb94671 100644 --- a/api/filters/replacement/replacement.go +++ b/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(), } } diff --git a/api/internal/accumulator/resaccumulator.go b/api/internal/accumulator/resaccumulator.go index 1c9b11575a..2723feafcf 100644 --- a/api/internal/accumulator/resaccumulator.go +++ b/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( diff --git a/api/internal/plugins/utils/utils.go b/api/internal/plugins/utils/utils.go index 1046e82bed..2d42507890 100644 --- a/api/internal/plugins/utils/utils.go +++ b/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, diff --git a/api/internal/plugins/utils/utils_test.go b/api/internal/plugins/utils/utils_test.go index 3e9b1264f4..bd75ebada8 100644 --- a/api/internal/plugins/utils/utils_test.go +++ b/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 } diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index efbb71ea61..43d3637dd8 100644 --- a/api/internal/target/kusttarget.go +++ b/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 diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index eb78376ab2..0305a55326 100644 --- a/api/resmap/reswrangler.go +++ b/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) } diff --git a/api/resource/factory.go b/api/resource/factory.go index 791180e1dd..2126e8eb77 100644 --- a/api/resource/factory.go +++ b/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 diff --git a/api/resource/resource.go b/api/resource/resource.go index 54ca906f9a..afbe2d4552 100644 --- a/api/resource/resource.go +++ b/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 } diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index c5f258ad65..75fc700f39 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -345,6 +345,11 @@ func (rn *RNode) GetKind() string { return "" } +// SetKind sets the kind. +func (rn *RNode) SetKind(k string) { + rn.SetMapField(NewScalarRNode(k), KindField) +} + // GetApiVersion returns the apiversion, if it exists, else empty string. func (rn *RNode) GetApiVersion() string { if node := rn.getMapFieldValue(APIVersionField); node != nil { @@ -353,6 +358,11 @@ func (rn *RNode) GetApiVersion() string { return "" } +// SetApiVersion sets the apiVersion. +func (rn *RNode) SetApiVersion(av string) { + rn.SetMapField(NewScalarRNode(av), APIVersionField) +} + // getMapFieldValue returns the value (*yaml.Node) of a mapping field. // The value might be nil. Also, the function returns nil, not an error, // if this node is not a mapping node, or if this node does not have the @@ -367,17 +377,43 @@ func (rn *RNode) getMapFieldValue(field string) *yaml.Node { return nil } -// GetName returns the name. +// GetName returns the name, or empty string if +// field not found. The setter is more restrictive. func (rn *RNode) GetName() string { - f := rn.Field(MetadataField) - if f.IsNilOrEmpty() { + return rn.getMetaStringField(NameField) +} + +// getMetaStringField returns the value of a string field in metadata. +func (rn *RNode) getMetaStringField(fName string) string { + md := rn.getMetaData() + if md == nil { return "" } - f = f.Value.Field(NameField) + f := md.Field(fName) if f.IsNilOrEmpty() { return "" } - return f.Value.YNode().Value + return GetValue(f.Value) +} + +// getMetaData returns the RNode holding the value of the metadata field. +// Return nil if field not found (no error). +func (rn *RNode) getMetaData() *RNode { + if IsMissingOrNull(rn) { + return nil + } + var n *RNode + if rn.YNode().Kind == DocumentNode { + // get the content if this is the document node + n = NewRNode(rn.Content()[0]) + } else { + n = rn + } + mf := n.Field(MetadataField) + if mf.IsNilOrEmpty() { + return nil + } + return mf.Value } // SetName sets the metadata name field. @@ -385,16 +421,14 @@ func (rn *RNode) SetName(name string) error { return rn.SetMapField(NewScalarRNode(name), MetadataField, NameField) } -// GetNamespace gets the metadata namespace field. -func (rn *RNode) GetNamespace() (string, error) { - meta, err := rn.GetMeta() - if err != nil { - return "", err - } - return meta.Namespace, nil +// GetNamespace gets the metadata namespace field, or empty string if +// field not found. The setter is more restrictive. +func (rn *RNode) GetNamespace() string { + return rn.getMetaStringField(NamespaceField) } -// SetNamespace tries to set the metadata namespace field. +// SetNamespace tries to set the metadata namespace field. If the argument +// is empty, the field is dropped. func (rn *RNode) SetNamespace(ns string) error { meta, err := rn.Pipe(Lookup(MetadataField)) if err != nil { @@ -411,12 +445,14 @@ func (rn *RNode) SetNamespace(ns string) error { } // GetAnnotations gets the metadata annotations field. -func (rn *RNode) GetAnnotations() (map[string]string, error) { - meta, err := rn.GetMeta() - if err != nil { - return nil, err +// If the field is missing, returns an empty map. +// Use another method to check for missing metadata. +func (rn *RNode) GetAnnotations() map[string]string { + meta := rn.getMetaData() + if meta == nil { + return make(map[string]string) } - return meta.Annotations, nil + return rn.getMapFromMeta(meta, AnnotationsField) } // SetAnnotations tries to set the metadata annotations field. @@ -425,12 +461,26 @@ func (rn *RNode) SetAnnotations(m map[string]string) error { } // GetLabels gets the metadata labels field. -func (rn *RNode) GetLabels() (map[string]string, error) { - meta, err := rn.GetMeta() - if err != nil { - return nil, err +// If the field is missing, returns an empty map. +// Use another method to check for missing metadata. +func (rn *RNode) GetLabels() map[string]string { + meta := rn.getMetaData() + if meta == nil { + return make(map[string]string) } - return meta.Labels, nil + return rn.getMapFromMeta(meta, LabelsField) +} + +// getMapFromMeta returns map, sometimes empty, from metadata. +func (rn *RNode) getMapFromMeta(meta *RNode, fName string) map[string]string { + result := make(map[string]string) + if f := meta.Field(fName); !f.IsNilOrEmpty() { + _ = f.Value.VisitFields(func(node *MapNode) error { + result[GetValue(node.Key)] = GetValue(node.Value) + return nil + }) + } + return result } // SetLabels sets the metadata labels field. @@ -440,7 +490,7 @@ func (rn *RNode) SetLabels(m map[string]string) error { // This established proper quoting on string values, and sorts by key. func (rn *RNode) setMapInMetadata(m map[string]string, field string) error { - meta, err := rn.Pipe(Lookup(MetadataField)) + meta, err := rn.Pipe(LookupCreate(MappingNode, MetadataField)) if err != nil { return err } @@ -807,11 +857,7 @@ func (rn *RNode) MatchesAnnotationSelector(selector string) (bool, error) { if err != nil { return false, err } - slice, err := rn.GetAnnotations() - if err != nil { - return false, err - } - return s.Matches(labels.Set(slice)), nil + return s.Matches(labels.Set(rn.GetAnnotations())), nil } // MatchesLabelSelector returns true on a selector match to labels. @@ -820,11 +866,7 @@ func (rn *RNode) MatchesLabelSelector(selector string) (bool, error) { if err != nil { return false, err } - slice, err := rn.GetLabels() - if err != nil { - return false, err - } - return s.Matches(labels.Set(slice)), nil + return s.Matches(labels.Set(rn.GetLabels())), nil } // HasNilEntryInList returns true if the RNode contains a list which has diff --git a/kyaml/yaml/rnode_test.go b/kyaml/yaml/rnode_test.go index 1562eafc92..5477750d47 100644 --- a/kyaml/yaml/rnode_test.go +++ b/kyaml/yaml/rnode_test.go @@ -954,41 +954,29 @@ const deploymentJSON = ` } ` -func getNamespaceOrDie(t *testing.T, n *RNode) string { - m, err := n.GetNamespace() - assert.NoError(t, err) - return m -} - func TestRNodeSetNamespace(t *testing.T) { n := NewRNode(nil) - assert.Equal(t, "", getNamespaceOrDie(t, n)) + assert.Equal(t, "", n.GetNamespace()) assert.NoError(t, n.SetNamespace("")) - assert.Equal(t, "", getNamespaceOrDie(t, n)) + assert.Equal(t, "", n.GetNamespace()) if err := n.UnmarshalJSON([]byte(deploymentJSON)); err != nil { t.Fatalf("unexpected unmarshaljson err: %v", err) } assert.NoError(t, n.SetNamespace("flanders")) - assert.Equal(t, "flanders", getNamespaceOrDie(t, n)) -} - -func getLabelsOrDie(t *testing.T, n *RNode) map[string]string { - m, err := n.GetLabels() - assert.NoError(t, err) - return m + assert.Equal(t, "flanders", n.GetNamespace()) } func TestRNodeSetLabels(t *testing.T) { n := NewRNode(nil) - assert.Equal(t, 0, len(getLabelsOrDie(t, n))) + assert.Equal(t, 0, len(n.GetLabels())) assert.NoError(t, n.SetLabels(map[string]string{})) - assert.Equal(t, 0, len(getLabelsOrDie(t, n))) + assert.Equal(t, 0, len(n.GetLabels())) n = NewRNode(nil) if err := n.UnmarshalJSON([]byte(deploymentJSON)); err != nil { t.Fatalf("unexpected unmarshaljson err: %v", err) } - labels := getLabelsOrDie(t, n) + labels := n.GetLabels() assert.Equal(t, 2, len(labels)) assert.Equal(t, "apple", labels["fruit"]) assert.Equal(t, "carrot", labels["veggie"]) @@ -996,31 +984,25 @@ func TestRNodeSetLabels(t *testing.T) { "label1": "foo", "label2": "bar", })) - labels = getLabelsOrDie(t, n) + labels = n.GetLabels() assert.Equal(t, 2, len(labels)) assert.Equal(t, "foo", labels["label1"]) assert.Equal(t, "bar", labels["label2"]) assert.NoError(t, n.SetLabels(map[string]string{})) - assert.Equal(t, 0, len(getLabelsOrDie(t, n))) -} - -func getAnnotationsOrDie(t *testing.T, n *RNode) map[string]string { - m, err := n.GetAnnotations() - assert.NoError(t, err) - return m + assert.Equal(t, 0, len(n.GetLabels())) } func TestRNodeGetAnnotations(t *testing.T) { n := NewRNode(nil) - assert.Equal(t, 0, len(getAnnotationsOrDie(t, n))) + assert.Equal(t, 0, len(n.GetAnnotations())) assert.NoError(t, n.SetAnnotations(map[string]string{})) - assert.Equal(t, 0, len(getAnnotationsOrDie(t, n))) + assert.Equal(t, 0, len(n.GetAnnotations())) n = NewRNode(nil) if err := n.UnmarshalJSON([]byte(deploymentJSON)); err != nil { t.Fatalf("unexpected unmarshaljson err: %v", err) } - annotations := getAnnotationsOrDie(t, n) + annotations := n.GetAnnotations() assert.Equal(t, 2, len(annotations)) assert.Equal(t, "51", annotations["area"]) assert.Equal(t, "Take me to your leader.", annotations["greeting"]) @@ -1028,12 +1010,12 @@ func TestRNodeGetAnnotations(t *testing.T) { "annotation1": "foo", "annotation2": "bar", })) - annotations = getAnnotationsOrDie(t, n) + annotations = n.GetAnnotations() assert.Equal(t, 2, len(annotations)) assert.Equal(t, "foo", annotations["annotation1"]) assert.Equal(t, "bar", annotations["annotation2"]) assert.NoError(t, n.SetAnnotations(map[string]string{})) - assert.Equal(t, 0, len(getAnnotationsOrDie(t, n))) + assert.Equal(t, 0, len(n.GetAnnotations())) } func TestRNodeMatchesAnnotationSelector(t *testing.T) { @@ -1644,18 +1626,12 @@ func TestGettingFields(t *testing.T) { if expected != actual { t.Fatalf("expected '%s', got '%s'", expected, actual) } - actualMap, err := rn.GetLabels() - if err != nil { - t.Fatalf("unexpected err '%v'", err) - } + actualMap := rn.GetLabels() v, ok := actualMap["fruit"] if !ok || v != "apple" { t.Fatalf("unexpected labels '%v'", actualMap) } - actualMap, err = rn.GetAnnotations() - if err != nil { - t.Fatalf("unexpected err '%v'", err) - } + actualMap = rn.GetAnnotations() v, ok = actualMap["greeting"] if !ok || v != "Take me to your leader." { t.Fatalf("unexpected annotations '%v'", actualMap) @@ -1717,12 +1693,11 @@ func TestSetLabels(t *testing.T) { if err := rn.UnmarshalJSON([]byte(deploymentBiggerJson)); err != nil { t.Fatalf("unexpected unmarshaljson err: %v", err) } - rn.SetLabels(map[string]string{ + assert.NoError(t, rn.SetLabels(map[string]string{ "label1": "foo", "label2": "bar", - }) - labels, err := rn.GetLabels() - assert.NoError(t, err) + })) + labels := rn.GetLabels() if expected, actual := 2, len(labels); expected != actual { t.Fatalf("expected '%d', got '%d'", expected, actual) } @@ -1739,12 +1714,11 @@ func TestGetAnnotations(t *testing.T) { if err := rn.UnmarshalJSON([]byte(deploymentBiggerJson)); err != nil { t.Fatalf("unexpected unmarshaljson err: %v", err) } - rn.SetAnnotations(map[string]string{ + assert.NoError(t, rn.SetAnnotations(map[string]string{ "annotation1": "foo", "annotation2": "bar", - }) - annotations, err := rn.GetAnnotations() - assert.NoError(t, err) + })) + annotations := rn.GetAnnotations() if expected, actual := 2, len(annotations); expected != actual { t.Fatalf("expected '%d', got '%d'", expected, actual) } diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index 8fda2d6409..807091bab7 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer.go @@ -34,7 +34,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { return nil } for _, r := range m.Resources() { - if r.IsEmpty() { + if r.IsNilOrEmpty() { // Don't mutate empty objects? continue }