Code review feedback

api/filters/filtersutil/setters.go

@@ -94,8 +94,7 @@ func hasExistingValue(node *yaml.RNode, key string) bool {
 	if node.IsNilOrEmpty() {
 		return false
 	}
-	if key == "" {
-		// scalar node
+	if err := yaml.ErrorIfInvalid(node, yaml.ScalarNode); err == nil {
 		return yaml.GetValue(node) != ""
 	}
 	entry := node.Field(key)

api/filters/nameref/nameref.go

@@ -337,7 +337,7 @@ func (f Filter) selectReferral(
 		return candidates[0], nil
 	}
 	ids := getIds(candidates)
-	return nil, fmt.Errorf(" found multiple possible referrals: %s\n%s", ids, f.failureDetails(candidates))
+	return nil, fmt.Errorf("found multiple possible referrals: %s\n%s", ids, f.failureDetails(candidates))
 }
 
 func (f Filter) failureDetails(resources []*resource.Resource) string {

api/filters/namespace/namespace.go

@@ -20,8 +20,8 @@ type Filter struct {
 	// FsSlice contains the FieldSpecs to locate the namespace field
 	FsSlice types.FsSlice `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
 
-	// SkipExisting means only blank namespace fields will be set
-	SkipExisting bool `json:"skipExisting" yaml:"skipExisting"`
+	// UnsetOnly means only blank namespace fields will be set
+	UnsetOnly bool `json:"UnsetOnly" yaml:"UnsetOnly"`
 
 	// SetRoleBindingSubjects determines which subject fields in RoleBinding and ClusterRoleBinding
 	// objects will have their namespace fields set. Overrides field specs provided for these types, if any.
@@ -160,12 +160,7 @@ func (ns Filter) roleBindingHack(obj *yaml.RNode, gvk resid.Gvk) error {
 }
 
 func isRoleBinding(kind string) bool {
-	switch kind {
-	case roleBindingKind, clusterRoleBindingKind:
-		return true
-	default:
-		return false
-	}
+	return kind == roleBindingKind || kind == clusterRoleBindingKind
 }
 
 // removeRoleBindingFieldSpecs removes from the list fieldspecs that
@@ -193,7 +188,7 @@ func (ns Filter) removeMetaNamespaceFieldSpecs(fs types.FsSlice) types.FsSlice {
 }
 
 func (ns *Filter) fieldSetter() filtersutil.SetFn {
-	if ns.SkipExisting {
+	if ns.UnsetOnly {
 		return ns.trackableSetter.SetEntryIfEmpty("", ns.Namespace, yaml.NodeTagString)
 	}
 	return ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString)

plugin/builtin/namespacetransformer/NamespaceTransformer.go

@@ -18,7 +18,7 @@ import (
 type plugin struct {
 	types.ObjectMeta       `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
 	FieldSpecs             []types.FieldSpec                `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"`
-	SkipExisting           bool                             `json:"skipExisting" yaml:"skipExisting"`
+	UnsetOnly              bool                             `json:"unsetOnly" yaml:"unsetOnly"`
 	SetRoleBindingSubjects namespace.RoleBindingSubjectMode `json:"setRoleBindingSubjects" yaml:"setRoleBindingSubjects"`
 }
 
@@ -58,7 +58,7 @@ func (p *plugin) Transform(m resmap.ResMap) error {
 			Namespace:              p.Namespace,
 			FsSlice:                p.FieldSpecs,
 			SetRoleBindingSubjects: p.SetRoleBindingSubjects,
-			SkipExisting:           p.SkipExisting,
+			UnsetOnly:              p.UnsetOnly,
 		}); err != nil {
 			return err
 		}

plugin/builtin/namespacetransformer/NamespaceTransformer_test.go

@@ -272,7 +272,7 @@ metadata:
 		})
 }
 
-func TestNamespaceTransformer_SkipExistingTrue(t *testing.T) {
+func TestNamespaceTransformer_UnsetOnlyTrue(t *testing.T) {
 	th := kusttest_test.MakeEnhancedHarness(t).
 		PrepBuiltin("NamespaceTransformer")
 	defer th.Reset()
@@ -282,7 +282,7 @@ kind: NamespaceTransformer
 metadata:
   name: notImportantHere
   namespace: test
-skipExisting: true
+unsetOnly: true
 fieldSpecs:
 - path: metadata/namespace
   create: true