Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove delegation to RNode in Resource #3888

Merged
merged 1 commit into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion api/builtins/NamespaceTransformer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions api/filters/fieldspec/fieldspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions api/filters/nameref/nameref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
7 changes: 1 addition & 6 deletions api/filters/replacement/replacement.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(),
}
}
1 change: 1 addition & 0 deletions api/internal/accumulator/resaccumulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suppresses a lint failure, because the next line is calling a deprecated function.
We need to remove that call, but its outside the scope of this pr

s, err := res.GetFieldValue(v.FieldRef.FieldPath)
if err != nil {
return "", fmt.Errorf(
Expand Down
22 changes: 11 additions & 11 deletions api/internal/plugins/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion api/internal/plugins/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
17 changes: 9 additions & 8 deletions api/internal/target/kusttarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,29 +303,30 @@ 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()
if _, found := labels[konfig.ValidatedByLabelKey]; !found {
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
Expand Down
23 changes: 13 additions & 10 deletions api/resmap/reswrangler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package resmap
import (
"bytes"
"fmt"
"reflect"

"github.com/pkg/errors"
"sigs.k8s.io/kustomize/api/resource"
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -606,7 +607,7 @@ func (m *resWrangler) ApplySmPatch(
return err
}
}
if !res.IsEmpty() {
if !res.IsNilOrEmpty() {
list = append(list, res)
}
}
Expand All @@ -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
}
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion api/resource/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading