Option to customize NamespaceTransformer overwrite behaviour

.golangci.yml

@@ -37,7 +37,7 @@ linters:
     - gocyclo
 #    - godot
 #    - godox
-    - goerr113
+#    - goerr113
     - gofmt
 #    - gofumpt
     - goheader

Makefile-tools.mk

@@ -28,7 +28,7 @@ uninstall-out-of-tree-tools:
 	rm -f $(MYGOBIN)/stringer
 
 $(MYGOBIN)/golangci-lint:
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.45.2
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.2
 
 $(MYGOBIN)/mdrip:
 	go install github.com/monopole/mdrip@v1.0.2

api/filters/filtersutil/setters.go

@@ -12,58 +12,95 @@ type SetFn func(*yaml.RNode) error
 
 // SetScalar returns a SetFn to set a scalar value
 func SetScalar(value string) SetFn {
-	return func(node *yaml.RNode) error {
-		return node.PipeE(yaml.FieldSetter{StringValue: value})
-	}
+	return SetEntry("", value, yaml.NodeTagEmpty)
 }
 
-// SetEntry returns a SetFn to set an entry in a map
-func SetEntry(key, value, tag string) SetFn {
+// SetEntry returns a SetFn to set a field or a map entry to a value.
+// It can be used with an empty name to set both a value and a tag on a scalar node.
+// When setting only a value on a scalar node, use SetScalar instead.
+func SetEntry(name, value, tag string) SetFn {
 	n := &yaml.Node{
 		Kind:  yaml.ScalarNode,
 		Value: value,
 		Tag:   tag,
 	}
 	return func(node *yaml.RNode) error {
 		return node.PipeE(yaml.FieldSetter{
-			Name:  key,
+			Name:  name,
 			Value: yaml.NewRNode(n),
 		})
 	}
 }
 
 type TrackableSetter struct {
 	// SetValueCallback will be invoked each time a field is set
-	setValueCallback func(key, value, tag string, node *yaml.RNode)
+	setValueCallback func(name, value, tag string, node *yaml.RNode)
 }
 
 // WithMutationTracker registers a callback which will be invoked each time a field is mutated
-func (s *TrackableSetter) WithMutationTracker(callback func(key, value, tag string, node *yaml.RNode)) {
+func (s *TrackableSetter) WithMutationTracker(callback func(key, value, tag string, node *yaml.RNode)) *TrackableSetter {
 	s.setValueCallback = callback
+	return s
 }
 
-// SetScalar returns a SetFn to set a scalar value
+// SetScalar returns a SetFn to set a scalar value.
 // if a mutation tracker has been registered, the tracker will be invoked each
 // time a scalar is set
 func (s TrackableSetter) SetScalar(value string) SetFn {
-	origSetScalar := SetScalar(value)
+	return s.SetEntry("", value, yaml.NodeTagEmpty)
+}
+
+// SetScalarIfEmpty returns a SetFn to set a scalar value only if it isn't already set.
+// If a mutation tracker has been registered, the tracker will be invoked each
+// time a scalar is actually set.
+func (s TrackableSetter) SetScalarIfEmpty(value string) SetFn {
+	return s.SetEntryIfEmpty("", value, yaml.NodeTagEmpty)
+}
+
+// SetEntry returns a SetFn to set a field or a map entry to a value.
+// It can be used with an empty name to set both a value and a tag on a scalar node.
+// When setting only a value on a scalar node, use SetScalar instead.
+// If a mutation tracker has been registered, the tracker will be invoked each
+// time an entry is set.
+func (s TrackableSetter) SetEntry(name, value, tag string) SetFn {
+	origSetEntry := SetEntry(name, value, tag)
 	return func(node *yaml.RNode) error {
 		if s.setValueCallback != nil {
-			s.setValueCallback("", value, "", node)
+			s.setValueCallback(name, value, tag, node)
 		}
-		return origSetScalar(node)
+		return origSetEntry(node)
 	}
 }
 
-// SetEntry returns a SetFn to set an entry in a map
-// if a mutation tracker has been registered, the tracker will be invoked each
-// time an entry is set
-func (s TrackableSetter) SetEntry(key, value, tag string) SetFn {
+// SetEntryIfEmpty returns a SetFn to set a field or a map entry to a value only if it isn't already set.
+// It can be used with an empty name to set both a value and a tag on a scalar node.
+// When setting only a value on a scalar node, use SetScalar instead.
+// If a mutation tracker has been registered, the tracker will be invoked each
+// time an entry is actually set.
+func (s TrackableSetter) SetEntryIfEmpty(key, value, tag string) SetFn {
 	origSetEntry := SetEntry(key, value, tag)
 	return func(node *yaml.RNode) error {
+		if hasExistingValue(node, key) {
+			return nil
+		}
 		if s.setValueCallback != nil {
 			s.setValueCallback(key, value, tag, node)
 		}
 		return origSetEntry(node)
 	}
 }
+
+func hasExistingValue(node *yaml.RNode, key string) bool {
+	if node.IsNilOrEmpty() {
+		return false
+	}
+	if key == "" {
+		// scalar node
+		return yaml.GetValue(node) != ""
+	}
+	entry := node.Field(key)
+	if entry.IsNilOrEmpty() {
+		return false
+	}
+	return yaml.GetValue(entry.Value) != ""
+}

api/filters/filtersutil/setters_test.go

@@ -0,0 +1,106 @@
+// Copyright 2022 The Kubernetes Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package filtersutil_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/kustomize/api/filters/filtersutil"
+	"sigs.k8s.io/kustomize/kyaml/yaml"
+)
+
+func TestTrackableSetter_SetScalarIfEmpty(t *testing.T) {
+	tests := []struct {
+		name  string
+		input *yaml.RNode
+		value string
+		want  string
+	}{
+		{
+			name:  "sets null values",
+			input: yaml.MakeNullNode(),
+			value: "foo",
+			want:  "foo",
+		},
+		{
+			name:  "sets empty values",
+			input: yaml.NewScalarRNode(""),
+			value: "foo",
+			want:  "foo",
+		},
+		{
+			name:  "does not overwrite values",
+			input: yaml.NewStringRNode("a"),
+			value: "foo",
+			want:  "a",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			wasSet := false
+			s := (&filtersutil.TrackableSetter{}).WithMutationTracker(func(_, _, _ string, _ *yaml.RNode) {
+				wasSet = true
+			})
+			wantSet := tt.value == tt.want
+			fn := s.SetScalarIfEmpty(tt.value)
+			require.NoError(t, fn(tt.input))
+			assert.Equal(t, tt.want, yaml.GetValue(tt.input))
+			assert.Equal(t, wantSet, wasSet, "tracker invoked even though value was not changed")
+		})
+	}
+}
+
+func TestTrackableSetter_SetEntryIfEmpty(t *testing.T) {
+	tests := []struct {
+		name  string
+		input *yaml.RNode
+		key   string
+		value string
+		want  string
+	}{
+		{
+			name:  "sets empty values",
+			input: yaml.NewMapRNode(&map[string]string{"setMe": ""}),
+			key:   "setMe",
+			value: "foo",
+			want:  "foo",
+		},
+		{
+			name:  "sets missing keys",
+			input: yaml.NewMapRNode(&map[string]string{}),
+			key:   "setMe",
+			value: "foo",
+			want:  "foo",
+		},
+		{
+			name:  "does not overwrite values",
+			input: yaml.NewMapRNode(&map[string]string{"existing": "original"}),
+			key:   "existing",
+			value: "foo",
+			want:  "original",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			wasSet := false
+			s := (&filtersutil.TrackableSetter{}).WithMutationTracker(func(_, _, _ string, _ *yaml.RNode) {
+				wasSet = true
+			})
+			wantSet := tt.value == tt.want
+			fn := s.SetEntryIfEmpty(tt.key, tt.value, "")
+			require.NoError(t, fn(tt.input))
+			assert.Equal(t, tt.want, yaml.GetValue(tt.input.Field(tt.key).Value))
+			assert.Equal(t, wantSet, wasSet, "tracker invoked even though value was not changed")
+		})
+	}
+}
+
+func TestTrackableSetter_SetEntryIfEmpty_BadInputNodeKind(t *testing.T) {
+	fn := filtersutil.TrackableSetter{}.SetEntryIfEmpty("foo", "false", yaml.NodeTagBool)
+	rn := yaml.NewListRNode("nope")
+	rn.AppendToFieldPath("dummy", "path")
+	assert.EqualError(t, fn(rn), "wrong Node Kind for dummy.path expected: MappingNode was SequenceNode: value: {- nope}")
+}

api/filters/nameref/nameref.go

@@ -337,19 +337,16 @@ func (f Filter) selectReferral(
 		return candidates[0], nil
 	}
 	ids := getIds(candidates)
-	f.failureDetails(candidates)
-	return nil, fmt.Errorf(" found multiple possible referrals: %s", ids)
+	return nil, fmt.Errorf(" found multiple possible referrals: %s\n%s", ids, f.failureDetails(candidates))
 }
 
-func (f Filter) failureDetails(resources []*resource.Resource) {
-	fmt.Printf(
-		"\n**** Too many possible referral targets to referrer:\n%s\n",
-		f.Referrer.MustYaml())
+func (f Filter) failureDetails(resources []*resource.Resource) string {
+	msg := strings.Builder{}
+	msg.WriteString(fmt.Sprintf("\n**** Too many possible referral targets to referrer:\n%s\n", f.Referrer.MustYaml()))
 	for i, r := range resources {
-		fmt.Printf(
-			"--- possible referral %d:\n%s", i, r.MustYaml())
-		fmt.Println("------")
+		msg.WriteString(fmt.Sprintf("--- possible referral %d:\n%s\n", i, r.MustYaml()))
 	}
+	return msg.String()
 }
 
 func allNamesAreTheSame(resources []*resource.Resource) bool {