Preserve scalar types when using the replacement filter

Erasing the scalar type tag leads to unfortunate circumstances, in that the resulting yaml code is valid yaml, but will not meet the object spec. For example, using the replacement transformer to take a port number as a string from a ConfigMap and set a Pod port would previously end up with: - containerPort: "8080" when the spec requires that this is not a string: - containerPort: 8080 Added unit tests for conversion to and from integers and booleans, plus creation from string and creation from integer. The creation behavior needs some refinement in a future PR. Signed-off-by: Jim Ramsay <i.am@jimramsay.com>
kubernetes-sigs · Mar 9, 2022 · cb80659 · cb80659
1 parent 6950a0d
commit cb80659
Show file tree

Hide file tree

Showing 2 changed files with 312 additions and 3 deletions.
diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go
@@ -174,7 +174,12 @@ func setTargetValue(options *types.FieldOptions, t *yaml.RNode, value *yaml.RNod
 		value.YNode().Value = strings.Join(tv, options.Delimiter)
 	}
 
-	t.SetYNode(value.YNode())
+	if t.YNode().Kind == yaml.ScalarNode {
+		// For scalar, only copy the value (leave any type intact to auto-convert int->string or string->int)
+		t.YNode().Value = value.YNode().Value
+	} else {
+		t.SetYNode(value.YNode())
+	}
 
 	return nil
 }

diff --git a/api/filters/replacement/replacement_test.go b/api/filters/replacement/replacement_test.go
@@ -205,13 +205,13 @@ spec:
         command: ["printenv"]
         args:
         - example.com
-        - 8080
+        - "8080"
       - name: busybox
         image: busybox:latest
         args:
         - echo
         - example.com
-        - 8080
+        - "8080"
 ---
 apiVersion: v1
 kind: ConfigMap
@@ -2011,6 +2011,310 @@ spec:
         name: nginx-tagged
       - image: postgres:1.8.0
         name: postgresdb
+`,
+		},
+		"string source -> integer target": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  PORT: "8080"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - containerPort: 80
+`,
+			replacements: `replacements:
+- source:
+    kind: ConfigMap
+    name: config
+    fieldPath: data.PORT
+  targets:
+  - select:
+      kind: Pod
+    fieldPaths:
+    - spec.containers.0.ports.0.containerPort
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  PORT: "8080"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - containerPort: 8080
+`,
+		},
+		"string source -> boolean target": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  MOUNT_TOKEN: "true"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    automountServiceAccountToken: false
+`,
+			replacements: `replacements:
+- source:
+    kind: ConfigMap
+    name: config
+    fieldPath: data.MOUNT_TOKEN
+  targets:
+  - select:
+      kind: Pod
+    fieldPaths:
+    - spec.containers.0.automountServiceAccountToken
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  MOUNT_TOKEN: "true"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    automountServiceAccountToken: true
+`,
+		},
+		// TODO: This is inconsistent with expectations; creating a numerical string would be
+		// expected, unless we had knowledge of the intended type of the field to be
+		// created.
+		"numerical string source -> integer creation": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  PORT: "8080"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - protocol: TCP
+`,
+			replacements: `replacements:
+- source:
+    kind: ConfigMap
+    name: config
+    fieldPath: data.PORT
+  targets:
+  - select:
+      kind: Pod
+    fieldPaths:
+    - spec.containers.0.ports.0.containerPort
+    - spec.containers.0.ports.0.name
+    options:
+      create: true
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  PORT: "8080"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - protocol: TCP
+      containerPort: 8080
+      name: 8080
+`,
+		},
+		"integer source -> string target": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  PORT: "8080"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - containerPort: 80
+`,
+			replacements: `replacements:
+- source:
+    kind: Pod
+    name: pod
+    fieldPath: spec.containers.0.ports.0.containerPort
+  targets:
+  - select:
+      kind: ConfigMap
+    fieldPaths:
+    - data.PORT
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  PORT: "80"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - containerPort: 80
+`,
+		},
+		"boolean source -> string target": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  MOUNT_TOKEN: "true"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    automountServiceAccountToken: false
+`,
+			replacements: `replacements:
+- source:
+    kind: Pod
+    name: pod
+    fieldPath: spec.containers.0.automountServiceAccountToken
+  targets:
+  - select:
+      kind: ConfigMap
+    fieldPaths:
+    - data.MOUNT_TOKEN
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  MOUNT_TOKEN: "false"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    automountServiceAccountToken: false
+`,
+		},
+		// TODO: This result is expected, but we should create a string and not an
+		// integer if we could know that the target type should be one. As a result,
+		// the actual ConfigMap produces here cannot be applied.
+		"integer source -> integer creation": {
+			input: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  FOO: "Bar"
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - containerPort: 80
+`,
+			replacements: `replacements:
+- source:
+    kind: Pod
+    name: pod
+    fieldPath: spec.containers.0.ports.0.containerPort
+  targets:
+  - select:
+      kind: ConfigMap
+    fieldPaths:
+    - data.PORT
+    options:
+      create: true
+`,
+			expected: `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+data:
+  FOO: "Bar"
+  PORT: 80
+---
+apiVersion: apps/v1
+kind: Pod
+metadata:
+  name: pod
+spec:
+  containers:
+  - image: busybox
+    name: myapp-container
+    ports:
+    - containerPort: 80
 `,
 		},
 	}