check tag values for double quoting

kubernetes-sigs · Feb 2, 2022 · c9c982e · c9c982e
1 parent 6759176
commit c9c982e
Show file tree

Hide file tree

Showing 17 changed files with 100 additions and 45 deletions.
diff --git a/api/filters/fieldspec/example_test.go b/api/filters/fieldspec/example_test.go
@@ -31,7 +31,7 @@ metadata:
 	}
 	fltr := Filter{
 		CreateKind: yaml.ScalarNode,
-		SetValue:   filtersutil.SetScalar("green"),
+		SetValue:   filtersutil.SetScalar("green", yaml.NodeTagString),
 		FieldSpec:  types.FieldSpec{Path: "a/b", CreateIfNotPresent: true},
 	}
 

diff --git a/api/filters/fieldspec/fieldspec_test.go b/api/filters/fieldspec/fieldspec_test.go
@@ -40,7 +40,7 @@ kind: Bar
 xxx:
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 		"empty path": {
@@ -61,7 +61,7 @@ xxx:
 `,
 			error: `considering field '' of object Bar.v1.foo/[noName].[noNs]: cannot set or create an empty field name`,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -84,7 +84,7 @@ a:
   b: e
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -107,7 +107,7 @@ a:
   b: c
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -130,7 +130,7 @@ a:
   b: c
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -154,7 +154,7 @@ a:
   b: c
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -178,7 +178,7 @@ a:
   b: c
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -198,7 +198,7 @@ a:
   b: c
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -214,7 +214,7 @@ a:
 `,
 			error: `considering field 'a/b/c' of object Bar.[noVer].[noGrp]/[noName].[noNs]: expected sequence or mapping node`,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -236,7 +236,7 @@ kind: Bar
 a: {b: {c: {d: e}}}
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("e"),
+				SetValue:   filtersutil.SetScalar("e", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -264,7 +264,7 @@ a:
       d: e
 `,
 			filter: fieldspec.Filter{
-				SetValue: filtersutil.SetScalar("e"),
+				SetValue: filtersutil.SetScalar("e", yaml.NodeTagString),
 			},
 		},
 
@@ -287,7 +287,7 @@ kind: Bar
 a: {}
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("e"),
+				SetValue:   filtersutil.SetScalar("e", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -315,7 +315,7 @@ a:
   - c: {d: e}
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("e"),
+				SetValue:   filtersutil.SetScalar("e", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -336,7 +336,7 @@ apiVersion: v1
 kind: Bar
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("e"),
+				SetValue:   filtersutil.SetScalar("e", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -359,7 +359,7 @@ a:
   b: e
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("e"),
+				SetValue:   filtersutil.SetScalar("e", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -385,7 +385,7 @@ spec:
   - image: bar
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("bar"),
+				SetValue:   filtersutil.SetScalar("bar", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -411,7 +411,7 @@ spec:
   - image: bar
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("bar"),
+				SetValue:   filtersutil.SetScalar("bar", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -434,7 +434,7 @@ spec:
   containers: []
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("bar"),
+				SetValue:   filtersutil.SetScalar("bar", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -458,7 +458,7 @@ spec:
   containers:
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("bar"),
+				SetValue:   filtersutil.SetScalar("bar", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -483,7 +483,7 @@ a:
     d: bar
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("bar"),
+				SetValue:   filtersutil.SetScalar("bar", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -510,7 +510,7 @@ a:
       f: bar
 `,
 			filter: fieldspec.Filter{
-				SetValue:   filtersutil.SetScalar("bar"),
+				SetValue:   filtersutil.SetScalar("bar", yaml.NodeTagString),
 				CreateKind: yaml.ScalarNode,
 			},
 		},
@@ -616,7 +616,7 @@ kind: Pod
 			fieldPaths = append(fieldPaths, strings.Join(node.FieldPath(), "."))
 		})
 		filter := fieldspec.Filter{
-			SetValue: trackableSetter.SetScalar("foo"),
+			SetValue: trackableSetter.SetScalar("foo", yaml.NodeTagString),
 		}
 
 		t.Run(name, func(t *testing.T) {

diff --git a/api/filters/filtersutil/setters.go b/api/filters/filtersutil/setters.go
@@ -8,9 +8,16 @@ import (
 type SetFn func(*yaml.RNode) error
 
 // SetScalar returns a SetFn to set a scalar value
-func SetScalar(value string) SetFn {
+func SetScalar(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{StringValue: value})
+		return node.PipeE(yaml.FieldSetter{
+			Value: yaml.NewRNode(n),
+		})
 	}
 }
 
@@ -21,9 +28,6 @@ func SetEntry(key, value, tag string) SetFn {
 		Value: value,
 		Tag:   tag,
 	}
-	if tag == yaml.NodeTagString && yaml.IsYaml1_1NonString(n) {
-		n.Style = yaml.DoubleQuotedStyle
-	}
 	return func(node *yaml.RNode) error {
 		return node.PipeE(yaml.FieldSetter{
 			Name:  key,
@@ -45,11 +49,11 @@ func (s *TrackableSetter) WithMutationTracker(callback func(key, value, tag stri
 // 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)
+func (s TrackableSetter) SetScalar(value, tag string) SetFn {
+	origSetScalar := SetScalar(value, tag)
 	return func(node *yaml.RNode) error {
 		if s.setValueCallback != nil {
-			s.setValueCallback("", value, "", node)
+			s.setValueCallback("", value, tag, node)
 		}
 		return origSetScalar(node)
 	}

diff --git a/api/filters/fsslice/example_test.go b/api/filters/fsslice/example_test.go
@@ -31,7 +31,7 @@ metadata:
 	}
 	fltr := fsslice.Filter{
 		CreateKind: yaml.ScalarNode,
-		SetValue:   filtersutil.SetScalar("green"),
+		SetValue:   filtersutil.SetScalar("green", yaml.NodeTagString),
 		FsSlice: []types.FieldSpec{
 			{Path: "a/b", CreateIfNotPresent: true},
 		},

diff --git a/api/filters/fsslice/fsslice_test.go b/api/filters/fsslice/fsslice_test.go
@@ -36,7 +36,7 @@ apiVersion: foo/v1
 kind: Bar
 `,
 		filter: Filter{
-			SetValue:   filtersutil.SetScalar("e"),
+			SetValue:   filtersutil.SetScalar("e", yaml.NodeTagString),
 			CreateKind: yaml.ScalarNode,
 		},
 	},
@@ -70,7 +70,7 @@ a:
   b: e
 `,
 		filter: Filter{
-			SetValue:   filtersutil.SetScalar("e"),
+			SetValue:   filtersutil.SetScalar("e", yaml.NodeTagString),
 			CreateKind: yaml.ScalarNode,
 		},
 	},

diff --git a/api/filters/imagetag/imagetag_test.go b/api/filters/imagetag/imagetag_test.go
@@ -734,18 +734,22 @@ spec:
 			expectedSetValueArgs: []filtertest.SetValueArg{
 				{
 					Value:    "busybox:v3",
+					Tag:      "!!str",
 					NodePath: []string{"spec", "template", "spec", "containers", "image"},
 				},
 				{
 					Value:    "busybox:v3",
+					Tag:      "!!str",
 					NodePath: []string{"spec", "template", "spec", "containers", "image"},
 				},
 				{
 					Value:    "busybox:v3",
+					Tag:      "!!str",
 					NodePath: []string{"spec", "template", "spec", "initContainers", "image"},
 				},
 				{
 					Value:    "busybox:v3",
+					Tag:      "!!str",
 					NodePath: []string{"spec", "template", "spec", "initContainers", "image"},
 				},
 			},

diff --git a/api/filters/imagetag/updater.go b/api/filters/imagetag/updater.go
@@ -41,7 +41,7 @@ func (u imageTagUpdater) SetImageValue(rn *yaml.RNode) error {
 		tag = "@" + u.ImageTag.Digest
 	}
 
-	return u.trackableSetter.SetScalar(name + tag)(rn)
+	return u.trackableSetter.SetScalar(name+tag, yaml.NodeTagString)(rn)
 }
 
 func (u imageTagUpdater) Filter(rn *yaml.RNode) (*yaml.RNode, error) {

diff --git a/api/filters/namespace/namespace.go b/api/filters/namespace/namespace.go
@@ -52,9 +52,8 @@ func (ns Filter) run(node *yaml.RNode) (*yaml.RNode, error) {
 	// transformations based on data -- :)
 	err := node.PipeE(fsslice.Filter{
 		FsSlice:    ns.FsSlice,
-		SetValue:   ns.trackableSetter.SetScalar(ns.Namespace),
+		SetValue:   ns.trackableSetter.SetScalar(ns.Namespace, yaml.NodeTagString),
 		CreateKind: yaml.ScalarNode, // Namespace is a ScalarNode
-		CreateTag:  yaml.NodeTagString,
 	})
 	return node, err
 }
@@ -85,7 +84,8 @@ func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error {
 		FsSlice: []types.FieldSpec{
 			{Path: types.MetadataNamespacePath, CreateIfNotPresent: true},
 		},
-		SetValue:   ns.trackableSetter.SetScalar(ns.Namespace),
+		SetValue: ns.trackableSetter.SetScalar(ns.Namespace, yaml.NodeTagString),
+		// SetValue:   filtersutil.SetString(ns.Namespace),
 		CreateKind: yaml.ScalarNode, // Namespace is a ScalarNode
 	}
 	_, err := f.Filter(obj)
@@ -138,7 +138,7 @@ func (ns Filter) roleBindingHack(obj *yaml.RNode, gvk resid.Gvk) error {
 			return err
 		}
 
-		return ns.trackableSetter.SetScalar(ns.Namespace)(node)
+		return ns.trackableSetter.SetScalar(ns.Namespace, yaml.NodeTagString)(node)
 
 	})