From c00ed0751b05fdee24a1d0ddd732f93b575e4697 Mon Sep 17 00:00:00 2001 From: Jim Ramsay Date: Fri, 25 Feb 2022 12:47:21 -0500 Subject: [PATCH] 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 Signed-off-by: Jim Ramsay --- api/filters/replacement/replacement.go | 7 +- api/filters/replacement/replacement_test.go | 214 +++++++++++++++++++- 2 files changed, 218 insertions(+), 3 deletions(-) diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go index 0cf269419f2..f4627ddbcb8 100644 --- 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 index e5077b93f69..ad277c14252 100644 --- 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,216 @@ 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 +`, + }, + // 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 +`, + }, + // 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 `, }, }