Repository: argoproj-labs/applicationset (go)

Issue:
Write tests for clustereventhandler.go
There are currently no tests for `clustereventhandler.go`, we should add tests against `queueRelatedAppGenerators` (IMHO testing the other methods is optional).

Pull request:
Write tests for clustereventhandler.go
Fixes #99

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/controllers/clustereventhandler.go b/pkg/controllers/clustereventhandler.go
index c657444..08c39ea 100644
--- a/pkg/controllers/clustereventhandler.go
+++ b/pkg/controllers/clustereventhandler.go
@@ -39,7 +39,14 @@ func (h *clusterSecretEventHandler) Generic(e event.GenericEvent, q workqueue.Ra
 	h.queueRelatedAppGenerators(q, e.Object)
 }
 
-func (h *clusterSecretEventHandler) queueRelatedAppGenerators(q workqueue.RateLimitingInterface, object client.Object) {
+// addRateLimitingInterface defines the Add method of workqueue.RateLimitingInterface, allow us to easily mock
+// it for testing purposes.
+type addRateLimitingInterface interface {
+	Add(item interface{})
+}
+
+func (h *clusterSecretEventHandler) queueRelatedAppGenerators(q addRateLimitingInterface, object client.Object) {
+
 	// Check for label, lookup all ApplicationSets that might match the cluster, queue them all
 	if object.GetLabels()[generators.ArgoCDSecretTypeLabel] != generators.ArgoCDSecretTypeCluster {
 		return
@@ -56,8 +63,10 @@ func (h *clusterSecretEventHandler) queueRelatedAppGenerators(q workqueue.RateLi
 		h.Log.WithError(err).Error("unable to list ApplicationSets")
 		return
 	}
+
 	h.Log.WithField("count", len(appSetList.Items)).Info("listed ApplicationSets")
 	for _, appSet := range appSetList.Items {
+
 		foundClusterGenerator := false
 		for _, generator := range appSet.Spec.Generators {
 			if generator.Clusters != nil {
@@ -66,6 +75,7 @@ func (h *clusterSecretEventHandler) queueRelatedAppGenerators(q workqueue.RateLi
 			}
 		}
 		if foundClusterGenerator {
+
 			// TODO: only queue the AppGenerator if the labels match this cluster
 			req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: appSet.Namespace, Name: appSet.Name}}
 			q.Add(req)
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/controllers/clustereventhandler_test.go b/pkg/controllers/clustereventhandler_test.go
new file mode 100644
index 0000000..82f6ff8
--- /dev/null
+++ b/pkg/controllers/clustereventhandler_test.go
@@ -0,0 +1,233 @@
+package controllers
+
+import (
+	"testing"
+
+	argoprojiov1alpha1 "github.com/argoproj-labs/applicationset/api/v1alpha1"
+	"github.com/argoproj-labs/applicationset/pkg/generators"
+	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
+	log "github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+)
+
+func TestClusterEventHandler(t *testing.T) {
+
+	scheme := runtime.NewScheme()
+	err := argoprojiov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+
+	err = argov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+
+	tests := []struct {
+		name             string
+		items            []argoprojiov1alpha1.ApplicationSet
+		secret           corev1.Secret
+		expectedRequests []ctrl.Request
+	}{
+		{
+			name:  "no application sets should mean no requests",
+			items: []argoprojiov1alpha1.ApplicationSet{},
+			secret: corev1.Secret{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: "argocd",
+					Name:      "my-secret",
+					Labels: map[string]string{
+						generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster,
+					},
+				},
+			},
+			expectedRequests: []reconcile.Request{},
+		},
+		{
+			name: "a cluster generator should produce a request",
+			items: []argoprojiov1alpha1.ApplicationSet{
+				{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "my-app-set",
+						Namespace: "argocd",
+					},
+					Spec: argoprojiov1alpha1.ApplicationSetSpec{
+						Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+							{
+								Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							},
+						},
+					},
+				},
+			},
+			secret: corev1.Secret{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: "argocd",
+					Name:      "my-secret",
+					Labels: map[string]string{
+						generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster,
+					},
+				},
+			},
+			expectedRequests: []reconcile.Request{{
+				NamespacedName: types.NamespacedName{Namespace: "argocd", Name: "my-app-set"},
+			}},
+		},
+		{
+			name: "multiple cluster generators should produce multiple requests",
+			items: []argoprojiov1alpha1.ApplicationSet{
+				{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "my-app-set",
+						Namespace: "argocd",
+					},
+					Spec: argoprojiov1alpha1.ApplicationSetSpec{
+						Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+							{
+								Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							},
+						},
+					},
+				},
+				{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "my-app-set2",
+						Namespace: "argocd",
+					},
+					Spec: argoprojiov1alpha1.ApplicationSetSpec{
+						Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+							{
+								Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							},
+						},
+					},
+				},
+			},
+			secret: corev1.Secret{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: "argocd",
+					Name:      "my-secret",
+					Labels: map[string]string{
+						generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster,
+					},
+				},
+			},
+			expectedRequests: []reconcile.Request{
+				{NamespacedName: types.NamespacedName{Namespace: "argocd", Name: "my-app-set"}},
+				{NamespacedName: types.NamespacedName{Namespace: "argocd", Name: "my-app-set2"}},
+			},
+		},
+		{
+			name: "non-cluster generator should not match",
+			items: []argoprojiov1alpha1.ApplicationSet{
+				{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "my-app-set",
+						Namespace: "another-namespace",
+					},
+					Spec: argoprojiov1alpha1.ApplicationSetSpec{
+						Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+							{
+								Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							},
+						},
+					},
+				},
+				{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "app-set-non-cluster",
+						Namespace: "argocd",
+					},
+					Spec: argoprojiov1alpha1.ApplicationSetSpec{
+						Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+							{
+								List: &argoprojiov1alpha1.ListGenerator{},
+							},
+						},
+					},
+				},
+			},
+			secret: corev1.Secret{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: "argocd",
+					Name:      "my-secret",
+					Labels: map[string]string{
+						generators.ArgoCDSecretTypeLabel: generators.ArgoCDSecretTypeCluster,
+					},
+				},
+			},
+			expectedRequests: []reconcile.Request{
+				{NamespacedName: types.NamespacedName{Namespace: "another-namespace", Name: "my-app-set"}},
+			},
+		},
+
+		{
+			name: "non-argo cd secret should not match",
+			items: []argoprojiov1alpha1.ApplicationSet{
+				{
+					ObjectMeta: v1.ObjectMeta{
+						Name:      "my-app-set",
+						Namespace: "another-namespace",
+					},
+					Spec: argoprojiov1alpha1.ApplicationSetSpec{
+						Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+							{
+								Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							},
+						},
+					},
+				},
+			},
+			secret: corev1.Secret{
+				ObjectMeta: v1.ObjectMeta{
+					Namespace: "argocd",
+					Name:      "my-non-argocd-secret",
+				},
+			},
+			expectedRequests: []reconcile.Request{},
+		},
+	}
+
+	for _, test := range tests {
+
+		t.Run(test.name, func(t *testing.T) {
+
+			appSetList := argoprojiov1alpha1.ApplicationSetList{
+				Items: test.items,
+			}
+
+			fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithLists(&appSetList).Build()
+
+			handler := &clusterSecretEventHandler{
+				Client: fakeClient,
+				Log:    log.WithField("type", "createSecretEventHandler"),
+			}
+
+			mockAddRateLimitingInterface := mockAddRateLimitingInterface{}
+
+			handler.queueRelatedAppGenerators(&mockAddRateLimitingInterface, &test.secret)
+
+			assert.False(t, mockAddRateLimitingInterface.errorOccurred)
+			assert.ElementsMatch(t, mockAddRateLimitingInterface.addedItems, test.expectedRequests)
+
+		})
+	}
+
+}
+
+// Add checks the type, and adds it to the internal list of received additions
+func (obj *mockAddRateLimitingInterface) Add(item interface{}) {
+	if req, ok := item.(ctrl.Request); ok {
+		obj.addedItems = append(obj.addedItems, req)
+	} else {
+		obj.errorOccurred = true
+	}
+}
+
+type mockAddRateLimitingInterface struct {
+	errorOccurred bool
+	addedItems    []ctrl.Request
+}
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)
+
+			}
+		})
+	}
+
+}