Fix function output when annotation field empty.

* When the `annotations` field is empty it gets removed before setting the path annotations to make sure `kyaml.LookupCreate` creates the mapping node.
* Correspondingly, when forcefully setting a namespace, the namespace field is removed now before setting the namespace.

Closes #13

cmd/khelm/fn.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"encoding/json"
+	"log"
 	"path"
 	"sort"
 	"strconv"
@@ -41,7 +42,7 @@ func kptFnCommand(h *helm.Helm) *cobra.Command {
 				return errors.Errorf("no outputPath specified for outputMapping[%d]", i)
 			}
 			if len(m.ResourceSelectors) == 0 {
-				return errors.Errorf("no resourceSelectors specified for outputMapping[%d] -> %q", i, m.OutputPath)
+				return errors.Errorf("no selectors specified for outputMapping[%d] -> %q", i, m.OutputPath)
 			}
 		}
 
@@ -53,7 +54,7 @@ func kptFnCommand(h *helm.Helm) *cobra.Command {
 		}
 
 		// Apply output path mappings and annotate resources
-		kustomizationDirs, err := mapOutputPaths(rendered, fnCfg.Data.OutputPathMapping, outputPath)
+		kustomizationDirs, err := mapOutputPaths(rendered, fnCfg.Data.OutputPathMapping, outputPath, h.Settings.Debug)
 		if err != nil {
 			return err
 		}
@@ -72,15 +73,17 @@ func kptFnCommand(h *helm.Helm) *cobra.Command {
 			if err != nil {
 				return errors.Wrapf(err, "generate %s", kustomizationPath)
 			}
-			setKptAnnotations(kustomization, kustomizationPath, 0)
+			err = setKptAnnotations(kustomization, kustomizationPath, 0, h.Settings.Debug)
+			if err != nil {
+				return errors.Wrap(err, "set kpt annotations on kustomization")
+			}
 			kustomizationResources[i] = kustomization
 		}
 		rendered = append(kustomizationResources, rendered...)
 
 		// Apply output
 		resourceList.Items = filterByOutputPath(resourceList.Items, outputPaths)
 		resourceList.Items = append(resourceList.Items, rendered...)
-
 		return nil
 	})
 	return cmd
@@ -136,7 +139,7 @@ func isGeneratedOutputPath(path string, outputPaths []string) bool {
 	return false
 }
 
-func mapOutputPaths(resources []*yaml.RNode, outputMappings []kptFnOutputMapping, defaultOutputPath string) (map[string][]*yaml.RNode, error) {
+func mapOutputPaths(resources []*yaml.RNode, outputMappings []kptFnOutputMapping, defaultOutputPath string, debug bool) (map[string][]*yaml.RNode, error) {
 	matchers := make([]matcher.ResourceMatchers, len(outputMappings))
 	for i, m := range outputMappings {
 		matchers[i] = matcher.FromResourceSelectors(m.ResourceSelectors)
@@ -162,7 +165,10 @@ func mapOutputPaths(resources []*yaml.RNode, outputMappings []kptFnOutputMapping
 			kustomizationDirs[outPath] = append(kustomizationDirs[outPath], o)
 			outPath = output.ResourcePath(meta, outPath)
 		}
-		setKptAnnotations(o, outPath, i)
+		err = setKptAnnotations(o, outPath, i, debug)
+		if err != nil {
+			return nil, errors.Wrapf(err, "set annotations on %s/%s", meta.Kind, meta.Name)
+		}
 	}
 
 	for _, m := range matchers {
@@ -175,10 +181,31 @@ func mapOutputPaths(resources []*yaml.RNode, outputMappings []kptFnOutputMapping
 	return kustomizationDirs, nil
 }
 
-func setKptAnnotations(o *yaml.RNode, path string, index int) {
+func setKptAnnotations(o *yaml.RNode, path string, index int, debug bool) error {
+	if debug {
+		m, err := o.GetMeta()
+		if err != nil {
+			return err
+		}
+		log.Printf("Mapping %s %s to path %s", m.Kind, m.Name, path)
+	}
+	// Remove annotations field if empty.
+	// This is required because LookupCreate() doesn't create the MappingNode if it exists but is empty (#13).
+	err := o.PipeE(yaml.LookupCreate(yaml.MappingNode, yaml.MetadataField), yaml.FieldClearer{Name: yaml.AnnotationsField, IfEmpty: true})
+	if err != nil {
+		return err
+	}
+	// Add path annotations
 	lookupAnnotations := yaml.LookupCreate(yaml.MappingNode, yaml.MetadataField, yaml.AnnotationsField)
-	_ = o.PipeE(lookupAnnotations, yaml.FieldSetter{Name: annotationIndex, StringValue: strconv.Itoa(index)})
-	_ = o.PipeE(lookupAnnotations, yaml.FieldSetter{Name: annotationPath, StringValue: path})
+	err = o.PipeE(lookupAnnotations, yaml.FieldSetter{Name: annotationIndex, StringValue: strconv.Itoa(index)})
+	if err != nil {
+		return err
+	}
+	err = o.PipeE(lookupAnnotations, yaml.FieldSetter{Name: annotationPath, StringValue: path})
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 func resourceNames(resources []*yaml.RNode, outputBasePath string) []string {

cmd/khelm/fn_test.go

@@ -41,7 +41,7 @@ func TestKptFnCommand(t *testing.T) {
 		name           string
 		input          kptFnConfig
 		mustContainObj int
-		mustContain    string
+		mustContain    []string
 	}{
 		{
 			"chart path only",
@@ -50,7 +50,7 @@ func TestKptFnCommand(t *testing.T) {
 					Chart: filepath.Join(exampleDir, "namespace"),
 				},
 			}},
-			3, "myconfiga",
+			3, []string{"myconfiga"},
 		},
 		{
 			"latest cluster scoped remote chart",
@@ -60,7 +60,7 @@ func TestKptFnCommand(t *testing.T) {
 					Chart:      "cert-manager",
 				},
 			}},
-			-1, "acme.cert-manager.io",
+			-1, []string{"acme.cert-manager.io"},
 		},
 		{
 			"remote chart with version",
@@ -71,7 +71,7 @@ func TestKptFnCommand(t *testing.T) {
 					Version:    "0.9.x",
 				},
 			}},
-			34, "chart: cainjector-v0.9.1",
+			34, []string{"chart: cainjector-v0.9.1"},
 		},
 		{
 			"release name",
@@ -83,7 +83,7 @@ func TestKptFnCommand(t *testing.T) {
 					Name: "myrelease",
 				},
 			}},
-			1, "myrelease-config",
+			1, []string{"myrelease-config"},
 		},
 		{
 			"valueFiles",
@@ -94,7 +94,7 @@ func TestKptFnCommand(t *testing.T) {
 				RendererConfig: config.RendererConfig{
 					ValueFiles: []string{filepath.Join(exampleDir, "values-inheritance", "values.yaml")},
 				}}},
-			1, " valueoverwrite: overwritten by file",
+			1, []string{" valueoverwrite: overwritten by file"},
 		},
 		{
 			"values",
@@ -107,7 +107,7 @@ func TestKptFnCommand(t *testing.T) {
 						"example": map[string]string{"overrideValue": "explicitly"},
 					},
 				}}},
-			1, " valueoverwrite: explicitly",
+			1, []string{" valueoverwrite: explicitly"},
 		},
 		{
 			"values override",
@@ -121,7 +121,7 @@ func TestKptFnCommand(t *testing.T) {
 						"example": map[string]string{"overrideValue": "explicitly"},
 					},
 				}}},
-			1, " valueoverwrite: explicitly",
+			1, []string{" valueoverwrite: explicitly"},
 		},
 		{
 			"apiversions",
@@ -132,7 +132,7 @@ func TestKptFnCommand(t *testing.T) {
 				RendererConfig: config.RendererConfig{
 					APIVersions: []string{"myfancyapi/v1", ""},
 				}}},
-			1, "fancycr",
+			1, []string{"fancycr"},
 		},
 		{
 			"kubeversion",
@@ -143,7 +143,7 @@ func TestKptFnCommand(t *testing.T) {
 				RendererConfig: config.RendererConfig{
 					KubeVersion: "1.12",
 				}}},
-			1, "k8sVersion: v1.12.0",
+			1, []string{"k8sVersion: v1.12.0"},
 		},
 		{
 			"expand-list",
@@ -152,7 +152,7 @@ func TestKptFnCommand(t *testing.T) {
 					Chart: filepath.Join(exampleDir, "expand-list"),
 				},
 			}},
-			3, "\n  name: myserviceaccount2\n",
+			3, []string{"\n  name: myserviceaccount2\n"},
 		},
 		{
 			"namespace",
@@ -164,7 +164,7 @@ func TestKptFnCommand(t *testing.T) {
 					Namespace: "mynamespace",
 				},
 			}},
-			3, " namespace: mynamespace\n",
+			3, []string{" namespace: mynamespace\n"},
 		},
 		{
 			"force namespace",
@@ -176,7 +176,7 @@ func TestKptFnCommand(t *testing.T) {
 					ForceNamespace: "forced-namespace",
 				},
 			}},
-			3, " namespace: forced-namespace\n",
+			3, []string{" namespace: forced-namespace\n"},
 		},
 		{
 			"exclude",
@@ -194,7 +194,7 @@ func TestKptFnCommand(t *testing.T) {
 					},
 				},
 			}},
-			2, "myconfigb",
+			2, []string{"myconfigb"},
 		},
 		{
 			"include",
@@ -218,10 +218,10 @@ func TestKptFnCommand(t *testing.T) {
 					},
 				},
 			}},
-			1, "myconfigb",
+			1, []string{"myconfigb"},
 		},
 		{
-			"output path",
+			"annotate output path",
 			kptFnConfig{
 				ChartConfig: &config.ChartConfig{
 					LoaderConfig: config.LoaderConfig{
@@ -230,7 +230,23 @@ func TestKptFnCommand(t *testing.T) {
 				},
 				OutputPath: "my/output/manifest.yaml",
 			},
-			3, "  config.kubernetes.io/path: my/output/manifest.yaml\n",
+			3, []string{" annotations:\n    config.kubernetes.io/index: 1\n    config.kubernetes.io/path: my/output/manifest.yaml\n"},
+		},
+		{
+			"annotate output path when annotations empty",
+			kptFnConfig{
+				ChartConfig: &config.ChartConfig{
+					LoaderConfig: config.LoaderConfig{
+						Chart: filepath.Join(exampleDir, "empty-annotations"),
+					},
+				},
+				OutputPath: "my/output/path/",
+			},
+			3, []string{
+				"\n    config.kubernetes.io/path: my/output/path/kustomization.yaml\n",
+				"\n    config.kubernetes.io/path: my/output/path/serviceaccount_sa1.yaml\n",
+				"\n    config.kubernetes.io/path: my/output/path/serviceaccount_sa2.yaml\n",
+			},
 		},
 		{
 			"output kustomization",
@@ -242,7 +258,7 @@ func TestKptFnCommand(t *testing.T) {
 				},
 				OutputPath: "my/output/path/",
 			},
-			4, "resources:\n  - configmap_myconfiga.yaml\n  - configmap_myconfigb.yaml\n",
+			4, []string{"resources:\n  - configmap_myconfiga.yaml\n  - configmap_myconfigb.yaml\n"},
 		},
 	} {
 		t.Run(c.name, func(t *testing.T) {
@@ -285,7 +301,9 @@ func TestKptFnCommand(t *testing.T) {
 					t.Log("\n" + out.String())
 				}
 			}
-			require.Contains(t, out.String(), c.mustContain, "output of %#v", c.input)
+			for _, mustContain := range c.mustContain {
+				require.Contains(t, out.String(), mustContain, "output of %#v", c.input)
+			}
 		})
 	}
 }

cmd/khelm/template_test.go

@@ -80,9 +80,9 @@ func TestTemplateCommand(t *testing.T) {
 			3, "namespace: mynamespace",
 		},
 		{
-			"namespace",
-			[]string{filepath.Join(exampleDir, "namespace"), "--force-namespace=forced-namespace"},
-			3, "namespace: forced-namespace",
+			"force-namespace",
+			[]string{filepath.Join(exampleDir, "force-namespace"), "--force-namespace=forced-namespace"},
+			5, "namespace: forced-namespace",
 		},
 	} {
 		t.Run(c.name, func(t *testing.T) {

example/empty-annotations/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+description: example chart with resource with empty annotations field. This is to verify that the kpt function maps paths reliably
+name: empty-annotations
+version: 0.1.0

example/empty-annotations/templates/template.yaml

@@ -0,0 +1,10 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: sa1
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: sa2
+  annotations:

example/force-namespace/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+description: example chart with resources with and without namespace
+name: namespace
+version: 0.1.0

example/force-namespace/generator.yaml

@@ -1,6 +1,7 @@
 apiVersion: khelm.mgoltzsche.github.com/v1
 kind: ChartRenderer
 metadata:
-  name: force-namespace
-chart: ../namespace
+  name: namespace-test
+  namespace: default-namespace
+chart: .
 forceNamespace: forced-namespace

example/force-namespace/templates/clusterrolebinding.yaml

@@ -0,0 +1,12 @@
+apiVersion: rbac.authorization.k8s.io/v1beta1
+kind: ClusterRoleBinding
+metadata:
+  name: jenkins-role-binding
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: cluster-admin
+subjects:
+- kind: ServiceAccount
+  name: jenkins
+  namespace: cluster-role-binding-ns

example/force-namespace/templates/configmap.yaml

@@ -0,0 +1,30 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: myconfiga
+  namespace: {{ .Release.Namespace }}
+data:
+  key: a
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: myconfigb
+data:
+  key: b
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: myconfigc-with-empty-namespace
+  namespace:
+data:
+  key: c
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: myconfigd-with-namespace-mappingnode
+  namespace: {}
+data:
+  key: d

example/namespace/generator.yaml

@@ -2,5 +2,5 @@ apiVersion: khelm.mgoltzsche.github.com/v1
 kind: ChartRenderer
 metadata:
   name: namespace-test
-  namespace: install-namespace
+  namespace: default-namespace
 chart: .