Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor mapping node traversal and optimize RNode.GetAnnotations and RNode.GetLabels #4944

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion api/internal/utils/makeResIds.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ func PrevIds(n *yaml.RNode) ([]resid.ResId, error) {
var ids []resid.ResId
// TODO: merge previous names and namespaces into one list of
// pairs on one annotation so there is no chance of error
annotations := n.GetAnnotations()
annotations := n.GetAnnotations(
BuildAnnotationPreviousNames,
BuildAnnotationPreviousNamespaces,
BuildAnnotationPreviousKinds)
if _, ok := annotations[BuildAnnotationPreviousNames]; !ok {
return nil, nil
}
Expand All @@ -51,6 +54,7 @@ func PrevIds(n *yaml.RNode) ([]resid.ResId, error) {
}
apiVersion := n.GetApiVersion()
group, version := resid.ParseGroupVersion(apiVersion)
ids = make([]resid.ResId, 0, len(names))
for i := range names {
gvk := resid.Gvk{
Group: group,
Expand Down
87 changes: 56 additions & 31 deletions kyaml/yaml/rnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,16 +435,18 @@ func (rn *RNode) getMetaStringField(fName string) string {
if md == nil {
return ""
}
f := md.Field(fName)
if f.IsNilOrEmpty() {
return ""
}
return GetValue(f.Value)
var result string
visitMappingNodeFields(md.Content, func(key, value *yaml.Node) {
if !IsYNodeNilOrEmpty(value) {
result = value.Value
}
}, fName)
return result
}

// getMetaData returns the RNode holding the value of the metadata field.
// getMetaData returns the *yaml.Node of the metadata field.
// Return nil if field not found (no error).
func (rn *RNode) getMetaData() *RNode {
func (rn *RNode) getMetaData() *yaml.Node {
if IsMissingOrNull(rn) {
return nil
}
Expand All @@ -455,11 +457,13 @@ func (rn *RNode) getMetaData() *RNode {
} else {
n = rn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think we can avoid creating a new RNode above as well

	content := rn.Content()
	if rn.YNode().Kind == DocumentNode {
		// get the content if this is the document node
		content = content[0].Content
	} 

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch. I'll plan to provide a small PR to tidy that up. Thanks!

}
mf := n.Field(MetadataField)
if mf.IsNilOrEmpty() {
return nil
}
return mf.Value
var mf *yaml.Node
visitMappingNodeFields(n.Content(), func(key, value *yaml.Node) {
if !IsYNodeNilOrEmpty(value) {
mf = value
}
}, MetadataField)
return mf
}

// SetName sets the metadata name field.
Expand Down Expand Up @@ -491,14 +495,14 @@ func (rn *RNode) SetNamespace(ns string) error {
}

// GetAnnotations gets the metadata annotations field.
// If the field is missing, returns an empty map.
// If the annotations 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 rn.getMapFromMeta(meta, AnnotationsField)
// If specific annotations are provided, then the map is
// restricted to only those entries with keys that match
// one of the specific annotations. If no annotations are
// provided, then the map will contain all entries.
func (rn *RNode) GetAnnotations(annotations ...string) map[string]string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this change, I like the option of providing specific annotations/labels to get.

return rn.getMapFromMeta(AnnotationsField, annotations...)
}

// SetAnnotations tries to set the metadata annotations field.
Expand All @@ -507,24 +511,45 @@ func (rn *RNode) SetAnnotations(m map[string]string) error {
}

// GetLabels gets the metadata labels field.
// If the field is missing, returns an empty map.
// If the labels field is missing, returns an empty map.
// Use another method to check for missing metadata.
func (rn *RNode) GetLabels() map[string]string {
// If specific labels are provided, then the map is
// restricted to only those entries with keys that match
// one of the specific labels. If no labels are
// provided, then the map will contain all entries.
func (rn *RNode) GetLabels(labels ...string) map[string]string {
return rn.getMapFromMeta(LabelsField, labels...)
}

// getMapFromMeta returns a map, sometimes empty, from the fName
// field in the node's metadata field.
// If specific fields are provided, then the map is
// restricted to only those entries with keys that match
// one of the specific fields. If no fields are
// provided, then the map will contain all entries.
func (rn *RNode) getMapFromMeta(fName string, fields ...string) map[string]string {
meta := rn.getMetaData()
if meta == nil {
return make(map[string]string)
}
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
})
var result map[string]string

visitMappingNodeFields(meta.Content, func(_, fNameValue *yaml.Node) {
// fName is found in metadata; create the map from its content
expectedSize := len(fields)
if expectedSize == 0 {
expectedSize = len(fNameValue.Content) / 2
}
result = make(map[string]string, expectedSize)

visitMappingNodeFields(fNameValue.Content, func(key, value *yaml.Node) {
result[key.Value] = value.Value
}, fields...)
}, fName)

if result == nil {
return make(map[string]string)
}
return result
}
Expand Down