diff --git a/api/filters/fieldspec/fieldspec.go b/api/filters/fieldspec/fieldspec.go index 6731cc403..d79a02652 100644 --- a/api/filters/fieldspec/fieldspec.go +++ b/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 } diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go index 4c135ac82..30efb8522 100644 --- a/api/filters/fieldspec/fieldspec_test.go +++ b/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 '/'", diff --git a/api/filters/nameref/nameref_test.go b/api/filters/nameref/nameref_test.go index 290878995..bbe927f38 100644 --- a/api/filters/nameref/nameref_test.go +++ b/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 diff --git a/api/filters/refvar/refvar_test.go b/api/filters/refvar/refvar_test.go index 7603f2f86..4d92ef703 100644 --- a/api/filters/refvar/refvar_test.go +++ b/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 { diff --git a/api/internal/accumulator/refvartransformer_test.go b/api/internal/accumulator/refvartransformer_test.go index afd1eed1d..42e33a31a 100644 --- a/api/internal/accumulator/refvartransformer_test.go +++ b/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(), + }, }, } diff --git a/api/krusty/nullvalues_test.go b/api/krusty/nullvalues_test.go index 9692c2739..9e2a685e4 100644 --- a/api/krusty/nullvalues_test.go +++ b/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 +`) }