Merge pull request #2983 from Shell32-Natsu/allow-null

ignore null value in fieldspec

api/filters/fieldspec/fieldspec.go

@@ -51,6 +51,9 @@ func (fltr Filter) filter(obj *yaml.RNode) error {
 		// found the field -- set its value
 		return fltr.SetValue(obj)
 	}
+	if obj.IsTaggedNull() {
+		return nil
+	}
 	switch obj.YNode().Kind {
 	case yaml.SequenceNode:
 		return fltr.seq(obj)
@@ -67,7 +70,7 @@ func (fltr Filter) field(obj *yaml.RNode) error {
 	// lookup the field matching the next path element
 	var lookupField yaml.Filter
 	var kind yaml.Kind
-	tag := "" // TODO: change to yaml.NodeTagEmpty
+	tag := yaml.NodeTagEmpty
 	switch {
 	case !fltr.FieldSpec.CreateIfNotPresent || fltr.CreateKind == 0 || isSeq:
 		// dont' create the field if we don't find it
@@ -95,9 +98,10 @@ func (fltr Filter) field(obj *yaml.RNode) error {
 		return errors.WrapPrefixf(err, "fieldName: %s", fieldName)
 	}
 
-	// if the value exists, but is null, then change it to the creation type
+	// if the value exists, but is null and kind is set,
+	// then change it to the creation type
 	// TODO: update yaml.LookupCreate to support this
-	if field.YNode().Tag == yaml.NodeTagNull {
+	if field.YNode().Tag == yaml.NodeTagNull && yaml.IsCreate(kind) {
 		field.YNode().Kind = kind
 		field.YNode().Tag = tag
 	}

api/filters/fieldspec/fieldspec_test.go

@@ -433,7 +433,6 @@ spec:
 			SetValue:   filtersutil.SetScalar("bar"),
 			CreateKind: yaml.ScalarNode,
 		},
-		error: "obj '' at path 'spec/containers/image': expected sequence or mapping node",
 	},
 	{
 		name: "filedname with slash '/'",

api/filters/nameref/nameref_test.go

@@ -168,6 +168,44 @@ metadata:
 map:
   name: newName
   namespace: oldNs
+`,
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "map"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+		},
+		"null value": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+map:
+  name: null
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: NotSecret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", ""},
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+map:
+  name: null
 `,
 			filter: Filter{
 				FieldSpec: types.FieldSpec{Path: "map"},
@@ -217,27 +255,6 @@ func TestNamerefFilterUnhappy(t *testing.T) {
 		filter        Filter
 		originalNames []string
 	}{
-		"invalid node type": {
-			input: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: dep
-ref:
-  name: null
-`,
-			candidates:    "",
-			originalNames: []string{},
-			expected:      "obj '' at path 'ref/name': node is expected to be either a string or a slice of string or a map of string",
-			filter: Filter{
-				FieldSpec: types.FieldSpec{Path: "ref/name"},
-				Target: resid.Gvk{
-					Group:   "apps",
-					Version: "v1",
-					Kind:    "Secret",
-				},
-			},
-		},
 		"multiple match": {
 			input: `
 apiVersion: apps/v1

api/filters/refvar/refvar_test.go

@@ -188,6 +188,26 @@ data:
 				FieldSpec: types.FieldSpec{Path: "data/slice2"},
 			},
 		},
+		"null value": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+data:
+  FOO: null`,
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+data:
+  FOO: null`,
+			filter: Filter{
+				MappingFunc: expansion2.MappingFuncFor(replacementCounts, map[string]interface{}{}),
+				FieldSpec:   types.FieldSpec{Path: "data/FOO"},
+			},
+		},
 	}
 
 	for tn, tc := range testCases {
@@ -260,20 +280,6 @@ data:
 				FieldSpec: types.FieldSpec{Path: "data"},
 			},
 		},
-		"null input": {
-			input: `
-apiVersion: apps/v1
-kind: Deployment
-metadata:
-  name: dep
-data:
-  FOO: null`,
-			expectedError: "obj '' at path 'data/FOO': invalid type encountered 0",
-			filter: Filter{
-				MappingFunc: expansion2.MappingFuncFor(replacementCounts, map[string]interface{}{}),
-				FieldSpec:   types.FieldSpec{Path: "data/FOO"},
-			},
-		},
 	}
 
 	for tn, tc := range testCases {

api/internal/accumulator/refvartransformer_test.go

@@ -132,7 +132,7 @@ func TestRefVarTransformer(t *testing.T) {
 ' at path 'data/slice': invalid value type expect a string`,
 		},
 		{
-			description: "var replacement panic in nil",
+			description: "var replacement in nil",
 			given: given{
 				varMap: map[string]interface{}{},
 				fs: []types.FieldSpec{
@@ -150,7 +150,19 @@ func TestRefVarTransformer(t *testing.T) {
 							"nil": nil, // noticeably *not* a []string
 						}}).ResMap(),
 			},
-			errMessage: `obj '' at path 'data/nil': invalid type encountered 0`,
+			expected: expected{
+				res: resmaptest_test.NewRmBuilder(
+					t, resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl())).
+					Add(map[string]interface{}{
+						"apiVersion": "v1",
+						"kind":       "ConfigMap",
+						"metadata": map[string]interface{}{
+							"name": "cm1",
+						},
+						"data": map[string]interface{}{
+							"nil": nil, // noticeably *not* a []string
+						}}).ResMap(),
+			},
 		},
 	}
 

api/krusty/nullvalues_test.go

@@ -4,7 +4,6 @@
 package krusty_test
 
 import (
-	"strings"
 	"testing"
 
 	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
@@ -73,7 +72,7 @@ metadata:
   name: test
 spec:
   template:
-    spec:      
+    spec:
       containers:
         - name: test
       volumes: null
@@ -82,12 +81,17 @@ spec:
 resources:
 - deploy.yaml
 `)
-	err := th.RunWithErr(".", th.MakeDefaultOptions())
-	if err == nil {
-		t.Fatalf("expected trouble")
-	}
-	if !strings.Contains(
-		err.Error(), "expected sequence or mapping node") {
-		t.Fatalf("Unexpected err: %v", err)
-	}
+	m := th.Run(".", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: test
+spec:
+  template:
+    spec:
+      containers:
+      - name: test
+      volumes: null
+`)
 }