Repository: GoogleContainerTools/kpt
Pull request #3229 (2022-05-23 21:00:39)

tag configMap data nodes as 'string'
Fixes https://github.com/GoogleContainerTools/kpt/issues/3193

First commit moves some code around and adds a (broken) test demonstrating current behavior. Second commit provides the fix for https://github.com/GoogleContainerTools/kpt/issues/3193.

When underlying go-yaml sees non-string values (e.g. integers or booleans), they must be explicitly tagged as strings in order to be quoted and processed correctly by other yaml-processing code.

With the following input:

```
data := map[string]string{
    "normal string": "abc",
    "integer":       "8081",
    "float":         "1.23",
    "bool":          "true",
}
m, err := NewConfigMap(data)
fmt.Println(m.MustString())
```

Before this fix:

```
apiVersion: v1
kind: ConfigMap
metadata:
  name: function-input
data: {normal string: abc, integer: 8081, float: 1.23, bool: true} # this is invalid
```

After this fix:
```
apiVersion: v1
kind: ConfigMap
metadata:
  name: function-input
data: {normal string: abc, integer: "8081", float: "1.23", bool: "true"}
```





natasha41575: @droot PTAL
natasha41575: @droot PTAL

Patch:
diff --git a/internal/fnruntime/runner.go b/internal/fnruntime/runner.go
index 1e62b9a6..98c404b2 100644
--- a/internal/fnruntime/runner.go
+++ b/internal/fnruntime/runner.go
@@ -508,19 +508,7 @@ func newFnConfig(fsys filesys.FileSystem, f *kptfilev1.Function, pkgPath types.U
 		// directly use the config from file
 		return node, nil
 	case len(f.ConfigMap) != 0:
-		node = yaml.NewMapRNode(&f.ConfigMap)
-		if node == nil {
-			return nil, nil
-		}
-		// create a ConfigMap only for configMap config
-		configNode := yaml.MustParse(`
-apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: function-input
-data: {}
-`)
-		err := configNode.PipeE(yaml.SetField("data", node))
+		configNode, err := NewConfigMap(f.ConfigMap)
 		if err != nil {
 			return nil, errors.E(op, fn, err)
 		}
diff --git a/internal/fnruntime/utils.go b/internal/fnruntime/utils.go
index acfe6d65..b41a3624 100644
--- a/internal/fnruntime/utils.go
+++ b/internal/fnruntime/utils.go
@@ -221,3 +221,30 @@ func annoMatch(node *yaml.RNode, selector kptfilev1.Selector) bool {
 	}
 	return true
 }
+
+func NewConfigMap(data map[string]string) (*yaml.RNode, error) {
+	node := yaml.NewMapRNode(&data)
+	if node == nil {
+		return nil, nil
+	}
+	// create a ConfigMap only for configMap config
+	configMap := yaml.MustParse(`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: function-input
+data: {}
+`)
+	if err := node.VisitFields(func(node *yaml.MapNode) error {
+		v := node.Value.YNode()
+		v.Tag = yaml.NodeTagString
+		node.Value.SetYNode(v)
+		return nil
+	}); err != nil {
+		return nil, err
+	}
+	if err := configMap.PipeE(yaml.SetField("data", node)); err != nil {
+		return nil, err
+	}
+	return configMap, nil
+}
diff --git a/porch/pkg/engine/eval.go b/porch/pkg/engine/eval.go
index 5ef7fe84..74274794 100644
--- a/porch/pkg/engine/eval.go
+++ b/porch/pkg/engine/eval.go
@@ -18,10 +18,10 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/GoogleContainerTools/kpt/internal/fnruntime"
 	v1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1"
 	"github.com/GoogleContainerTools/kpt/pkg/fn"
 	api "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
-	"github.com/GoogleContainerTools/kpt/porch/pkg/kpt"
 	"github.com/GoogleContainerTools/kpt/porch/pkg/repository"
 	"go.opentelemetry.io/otel/trace"
 	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
@@ -51,7 +51,7 @@ func (m *evalFunctionMutation) Apply(ctx context.Context, resources repository.P
 
 	var functionConfig *yaml.RNode
 	if m.task.Eval.ConfigMap != nil {
-		if cm, err := kpt.NewConfigMap(m.task.Eval.ConfigMap); err != nil {
+		if cm, err := fnruntime.NewConfigMap(m.task.Eval.ConfigMap); err != nil {
 			return repository.PackageResources{}, nil, fmt.Errorf("failed to create function config: %w", err)
 		} else {
 			functionConfig = cm
diff --git a/porch/pkg/kpt/eval.go b/porch/pkg/kpt/eval.go
index 26d2cfc8..549007ae 100644
--- a/porch/pkg/kpt/eval.go
+++ b/porch/pkg/kpt/eval.go
@@ -23,7 +23,6 @@ import (
 	"github.com/GoogleContainerTools/kpt/porch/pkg/kpt/internal"
 	"sigs.k8s.io/kustomize/kyaml/fn/framework"
 	"sigs.k8s.io/kustomize/kyaml/kio"
-	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
 func NewSimpleFunctionRuntime() FunctionRuntime {
@@ -67,22 +66,3 @@ func (fr *runner) Run(r io.Reader, w io.Writer) error {
 
 	return framework.Execute(fr.processor, rw)
 }
-
-func NewConfigMap(data map[string]string) (*yaml.RNode, error) {
-	node := yaml.NewMapRNode(&data)
-	if node == nil {
-		return nil, nil
-	}
-	// create a ConfigMap only for configMap config
-	configMap := yaml.MustParse(`
-apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: function-input
-data: {}
-`)
-	if err := configMap.PipeE(yaml.SetField("data", node)); err != nil {
-		return nil, err
-	}
-	return configMap, nil
-}


Test patch:
diff --git a/internal/fnruntime/utils_test.go b/internal/fnruntime/utils_test.go
index dafb230d..b2f57776 100644
--- a/internal/fnruntime/utils_test.go
+++ b/internal/fnruntime/utils_test.go
@@ -140,3 +140,19 @@ spec:
 		})
 	}
 }
+
+func TestNewConfigMap(t *testing.T) {
+	data := map[string]string{
+		"normal string": "abc",
+		"integer":       "8081",
+		"float":         "1.23",
+		"bool":          "true",
+	}
+	m, err := NewConfigMap(data)
+	assert.NoError(t, err)
+	mapAsString := m.MustString()
+	assert.Contains(t, mapAsString, `bool: "true"`)
+	assert.Contains(t, mapAsString, `normal string: abc`)
+	assert.Contains(t, mapAsString, `integer: "8081"`)
+	assert.Contains(t, mapAsString, `float: "1.23"`)
+}
diff --git a/porch/pkg/kpt/eval_test.go b/porch/pkg/kpt/eval_test.go
index 13ef5863..bafe1789 100644
--- a/porch/pkg/kpt/eval_test.go
+++ b/porch/pkg/kpt/eval_test.go
@@ -20,6 +20,7 @@ import (
 	"path"
 	"testing"
 
+	"github.com/GoogleContainerTools/kpt/internal/fnruntime"
 	v1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
@@ -56,7 +57,7 @@ func TestSetLabels(t *testing.T) {
 		t.Errorf("GetRunner failed: %v", err)
 	}
 
-	config, err := NewConfigMap(map[string]string{
+	config, err := fnruntime.NewConfigMap(map[string]string{
 		"label-key": "label-value",
 	})
 	if err != nil {