Repository: argoproj-labs/applicationset (go)

Issue:
Write tests for pkg/utils/util.go
`pkg/utils/util.go` currently has no tests, and would benefit from a test of, at least, `RenderTemplateParams`.

Pull request description:
Write tests for pkg/utils/util.go
Fixes #100 


Patch:
diff --git a/Dockerfile b/Dockerfile
index 0c37411..bcccd56 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -18,7 +18,9 @@ COPY pkg/ pkg/
 # Build
 RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags="-w -s" -a -o applicationset-controller main.go
 
-FROM debian:10-slim
+FROM ubuntu:20.10
+
+ENV DEBIAN_FRONTEND=noninteractive
 RUN apt-get update && apt-get upgrade -y && \
   apt-get install -y git-all && \
   rm -r /var/lib/apt/lists /var/cache/apt/archives
diff --git a/examples/template-override/template-overrides-example.yaml b/examples/template-override/template-overrides-example.yaml
index 4f4142e..3970136 100644
--- a/examples/template-override/template-overrides-example.yaml
+++ b/examples/template-override/template-overrides-example.yaml
@@ -17,7 +17,7 @@ spec:
         spec:
           project: "default"
           source:
-            revision: HEAD
+            targetRevision: HEAD
             repoURL: https://github.com/argoproj-labs/applicationset.git
             path: 'examples/template-override/{{cluster}}-override'
           destination: {}
@@ -33,4 +33,4 @@ spec:
         path: examples/template-override/default
       destination:
         server: '{{url}}'
-        namespace: guestbook
\ No newline at end of file
+        namespace: guestbook
diff --git a/pkg/utils/util.go b/pkg/utils/util.go
index 042b2bc..9f2a903 100644
--- a/pkg/utils/util.go
+++ b/pkg/utils/util.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"strconv"
+	"strings"
 
 	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	"github.com/pkg/errors"
@@ -53,14 +54,16 @@ func (r *Render) RenderTemplateParams(tmpl *argov1alpha1.Application, params map
 }
 
 // Replace executes basic string substitution of a template with replacement values.
-// allowUnresolved indicates whether or not it is acceptable to have unresolved variables
-// remaining in the substituted template. prefixFilter will apply the replacements only
-// to variables with the specified prefix
+// 'allowUnresolved' indicates whether or not it is acceptable to have unresolved variables
+// remaining in the substituted template.
 func (r *Render) replace(fstTmpl *fasttemplate.Template, replaceMap map[string]string, allowUnresolved bool) (string, error) {
 	var unresolvedErr error
 	replacedTmpl := fstTmpl.ExecuteFuncString(func(w io.Writer, tag string) (int, error) {
-		replacement, ok := replaceMap[tag]
-		if !ok {
+
+		trimmedTag := strings.TrimSpace(tag)
+
+		replacement, ok := replaceMap[trimmedTag]
+		if len(trimmedTag) == 0 || !ok {
 			if allowUnresolved {
 				// just write the same string back
 				return w.Write([]byte(fmt.Sprintf("{{%s}}", tag)))


Test patch:
diff --git a/pkg/utils/util_test.go b/pkg/utils/util_test.go
new file mode 100644
index 0000000..1d9d272
--- /dev/null
+++ b/pkg/utils/util_test.go
@@ -0,0 +1,163 @@
+package utils
+
+import (
+	"testing"
+
+	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRenderTemplateParams(t *testing.T) {
+
+	// Believe it or not, this is actually less complex than the equivalent solution using reflection
+	fieldMap := map[string]func(app *argov1alpha1.Application) *string{}
+	fieldMap["Path"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Source.Path }
+	fieldMap["RepoURL"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Source.RepoURL }
+	fieldMap["TargetRevision"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Source.TargetRevision }
+	fieldMap["Chart"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Source.Chart }
+
+	fieldMap["Server"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Destination.Server }
+	fieldMap["Namespace"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Destination.Namespace }
+	fieldMap["Name"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Destination.Name }
+
+	fieldMap["Project"] = func(app *argov1alpha1.Application) *string { return &app.Spec.Project }
+
+	emptyApplication := &argov1alpha1.Application{
+		Spec: argov1alpha1.ApplicationSpec{
+			Source: argov1alpha1.ApplicationSource{
+				Path:           "",
+				RepoURL:        "",
+				TargetRevision: "",
+				Chart:          "",
+			},
+			Destination: argov1alpha1.ApplicationDestination{
+				Server:    "",
+				Namespace: "",
+				Name:      "",
+			},
+			Project: "",
+		},
+	}
+
+	tests := []struct {
+		name        string
+		fieldVal    string
+		params      map[string]string
+		expectedVal string
+	}{
+		{
+			name:        "simple substitution",
+			fieldVal:    "{{one}}",
+			expectedVal: "two",
+			params: map[string]string{
+				"one": "two",
+			},
+		},
+		{
+			name:        "simple substitution with whitespace",
+			fieldVal:    "{{ one }}",
+			expectedVal: "two",
+			params: map[string]string{
+				"one": "two",
+			},
+		},
+
+		{
+			name:        "template characters but not in a template",
+			fieldVal:    "}} {{",
+			expectedVal: "}} {{",
+			params: map[string]string{
+				"one": "two",
+			},
+		},
+
+		{
+			name:        "nested template",
+			fieldVal:    "{{ }}",
+			expectedVal: "{{ }}",
+			params: map[string]string{
+				"one": "{{ }}",
+			},
+		},
+		{
+			name:        "field with whitespace",
+			fieldVal:    "{{ }}",
+			expectedVal: "{{ }}",
+			params: map[string]string{
+				" ": "two",
+				"":  "three",
+			},
+		},
+
+		{
+			name:        "template contains itself, containing itself",
+			fieldVal:    "{{one}}",
+			expectedVal: "{{one}}",
+			params: map[string]string{
+				"{{one}}": "{{one}}",
+			},
+		},
+
+		{
+			name:        "template contains itself, containing something else",
+			fieldVal:    "{{one}}",
+			expectedVal: "{{one}}",
+			params: map[string]string{
+				"{{one}}": "{{two}}",
+			},
+		},
+
+		{
+			name:        "templates are case sensitive",
+			fieldVal:    "{{ONE}}",
+			expectedVal: "{{ONE}}",
+			params: map[string]string{
+				"{{one}}": "two",
+			},
+		},
+		{
+			name:        "multiple on a line",
+			fieldVal:    "{{one}}{{one}}",
+			expectedVal: "twotwo",
+			params: map[string]string{
+				"one": "two",
+			},
+		},
+		{
+			name:        "multiple different on a line",
+			fieldVal:    "{{one}}{{three}}",
+			expectedVal: "twofour",
+			params: map[string]string{
+				"one":   "two",
+				"three": "four",
+			},
+		},
+	}
+
+	for _, test := range tests {
+
+		t.Run(test.name, func(t *testing.T) {
+
+			for fieldName, getPtrFunc := range fieldMap {
+
+				// Clone the template application
+				application := emptyApplication.DeepCopy()
+
+				// Set the value of the target field, to the test value
+				*getPtrFunc(application) = test.fieldVal
+
+				// Render the cloned application, into a new application
+				render := Render{}
+				newApplication, err := render.RenderTemplateParams(application, test.params)
+
+				// Retrieve the value of the target field from the newApplication, then verify that
+				// the target field has been templated into the expected value
+				actualValue := *getPtrFunc(newApplication)
+				assert.Equal(t, test.expectedVal, actualValue, "Field '%s' had an unexpected value. expected: '%s' value: '%s'", fieldName, test.expectedVal, actualValue)
+				assert.NoError(t, err)
+
+			}
+		})
+	}
+
+}