Repository: argoproj-labs/applicationset
Pull request #67 (2020-12-01 19:21:57)

Log warning if applicationset contains unrecognized generators

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/argoproj-labs/applicationset?pullRequest=67) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/argoproj-labs/applicationset?pullRequest=67) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/argoproj-labs/applicationset?pullRequest=67) it.</sub>
jopit: @jgwest thanks for the review.  I've made changes based on your feedback.

Patch:
diff --git a/pkg/controllers/applicationset_controller.go b/pkg/controllers/applicationset_controller.go
index e5df7f9..29663a5 100644
--- a/pkg/controllers/applicationset_controller.go
+++ b/pkg/controllers/applicationset_controller.go
@@ -18,8 +18,11 @@ package controllers
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"reflect"
+	"sort"
+	"strings"
 	"time"
 
 	"github.com/argoproj-labs/applicationset/pkg/generators"
@@ -67,6 +70,9 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	// Log a warning if there are unrecognized generators
+	checkInvalidGenerators(&applicationSetInfo)
+
 	// desiredApplications is the main list of all expected Applications from all generators in this appset.
 	desiredApplications, err := r.generateApplications(applicationSetInfo)
 	if err != nil {
@@ -100,6 +106,90 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 	}, nil
 }
 
+// Log a warning if there are unrecognized generators
+func checkInvalidGenerators(applicationSetInfo *argoprojiov1alpha1.ApplicationSet) {
+	hasInvalidGenerators, invalidGenerators := invalidGenerators(applicationSetInfo)
+	if len(invalidGenerators) > 0 {
+		gnames := []string{}
+		for n := range invalidGenerators {
+			gnames = append(gnames, n)
+		}
+		sort.Strings(gnames)
+		aname := applicationSetInfo.ObjectMeta.Name
+		msg := "ApplicationSet %s contains unrecognized generators: %s"
+		log.Warnf(msg, aname, strings.Join(gnames, ", "))
+	} else if hasInvalidGenerators {
+		name := applicationSetInfo.ObjectMeta.Name
+		msg := "ApplicationSet %s contains unrecognized generators"
+		log.Warnf(msg, name)
+	}
+}
+
+// Return true if there are unknown generators specified in the application set.  If we can discover the names
+// of these generators, return the names as the keys in a map
+func invalidGenerators(applicationSetInfo *argoprojiov1alpha1.ApplicationSet) (bool, map[string]bool) {
+	names := make(map[string]bool)
+	hasInvalidGenerators := false
+	for index, generator := range applicationSetInfo.Spec.Generators {
+		v := reflect.Indirect(reflect.ValueOf(generator))
+		found := false
+		for i := 0; i < v.NumField(); i++ {
+			field := v.Field(i)
+			if !field.CanInterface() {
+				continue
+			}
+			if !reflect.ValueOf(field.Interface()).IsNil() {
+				found = true
+				break
+			}
+		}
+		if !found {
+			hasInvalidGenerators = true
+			addInvalidGeneratorNames(names, applicationSetInfo, index)
+		}
+	}
+	return hasInvalidGenerators, names
+}
+
+func addInvalidGeneratorNames(names map[string]bool, applicationSetInfo *argoprojiov1alpha1.ApplicationSet, index int) {
+	// The generator names are stored in the "kubectl.kubernetes.io/last-applied-configuration" annotation
+	config := applicationSetInfo.ObjectMeta.Annotations["kubectl.kubernetes.io/last-applied-configuration"]
+	var values map[string]interface{}
+	err := json.Unmarshal([]byte(config), &values)
+	if err != nil {
+		log.Warnf("couldn't unmarshal kubectl.kubernetes.io/last-applied-configuration: %+v", config)
+		return
+	}
+
+	spec, ok := values["spec"].(map[string]interface{})
+	if !ok {
+		log.Warn("coundn't get spec from kubectl.kubernetes.io/last-applied-configuration annotation")
+		return
+	}
+
+	generators, ok := spec["generators"].([]interface{})
+	if !ok {
+		log.Warn("coundn't get generators from kubectl.kubernetes.io/last-applied-configuration annotation")
+		return
+	}
+
+	if index >= len(generators) {
+		log.Warnf("index %d out of range %d for generator in kubectl.kubernetes.io/last-applied-configuration", index, len(generators))
+		return
+	}
+
+	generator, ok := generators[index].(map[string]interface{})
+	if !ok {
+		log.Warn("coundn't get generator from kubectl.kubernetes.io/last-applied-configuration annotation")
+		return
+	}
+
+	for key := range generator {
+		names[key] = true
+		break
+	}
+}
+
 func (r *ApplicationSetReconciler) GetRelevantGenerators(requestedGenerator *argoprojiov1alpha1.ApplicationSetGenerator) []generators.Generator {
 	var res []generators.Generator
 


Test patch:
diff --git a/pkg/controllers/applicationset_controller_test.go b/pkg/controllers/applicationset_controller_test.go
index 7146bca..4c07f89 100644
--- a/pkg/controllers/applicationset_controller_test.go
+++ b/pkg/controllers/applicationset_controller_test.go
@@ -4,8 +4,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"testing"
+	"time"
+
 	"github.com/argoproj-labs/applicationset/pkg/generators"
 	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
+	"github.com/sirupsen/logrus"
+	logtest "github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -14,8 +19,6 @@ import (
 	crtclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"testing"
-	"time"
 
 	argoprojiov1alpha1 "github.com/argoproj-labs/applicationset/api/v1alpha1"
 )
@@ -731,3 +734,378 @@ func TestGetMinRequeueAfter(t *testing.T) {
 
 	assert.Equal(t, time.Duration(1)*time.Second, got)
 }
+
+func TestInvalidGenerators(t *testing.T) {
+
+	scheme := runtime.NewScheme()
+	err := argoprojiov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+	err = argov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+
+	for _, c := range []struct {
+		testName        string
+		appSet          argoprojiov1alpha1.ApplicationSet
+		expectedInvalid bool
+		expectedNames   map[string]bool
+	}{
+		{
+			testName: "valid generators, with annotation",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{
+							"spec":{
+								"generators":[
+									{"list":{}},
+									{"cluster":{}},
+									{"git":{}}
+								]
+							}
+						}`,
+					},
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     &argoprojiov1alpha1.ListGenerator{},
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      &argoprojiov1alpha1.GitGenerator{},
+						},
+					},
+				},
+			},
+			expectedInvalid: false,
+			expectedNames:   map[string]bool{},
+		},
+		{
+			testName: "invalid generators, no annotation",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+					},
+				},
+			},
+			expectedInvalid: true,
+			expectedNames:   map[string]bool{},
+		},
+		{
+			testName: "valid and invalid generators, no annotation",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     nil,
+							Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      &argoprojiov1alpha1.GitGenerator{},
+						},
+					},
+				},
+			},
+			expectedInvalid: true,
+			expectedNames:   map[string]bool{},
+		},
+		{
+			testName: "valid and invalid generators, with annotation",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{
+							"spec":{
+								"generators":[
+									{"cluster":{}},
+									{"bbb":{}},
+									{"git":{}},
+									{"aaa":{}}
+								]
+							}
+						}`,
+					},
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     nil,
+							Clusters: &argoprojiov1alpha1.ClusterGenerator{},
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      &argoprojiov1alpha1.GitGenerator{},
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+					},
+				},
+			},
+			expectedInvalid: true,
+			expectedNames: map[string]bool{
+				"aaa": true,
+				"bbb": true,
+			},
+		},
+		{
+			testName: "invalid generator, annotation with missing spec",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{
+						}`,
+					},
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+					},
+				},
+			},
+			expectedInvalid: true,
+			expectedNames:   map[string]bool{},
+		},
+		{
+			testName: "invalid generator, annotation with missing generators array",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{
+							"spec":{
+							}
+						}`,
+					},
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+					},
+				},
+			},
+			expectedInvalid: true,
+			expectedNames:   map[string]bool{},
+		},
+		{
+			testName: "invalid generator, annotation with empty generators array",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{
+							"spec":{
+								"generators":[
+								]
+							}
+						}`,
+					},
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+					},
+				},
+			},
+			expectedInvalid: true,
+			expectedNames:   map[string]bool{},
+		},
+		{
+			testName: "invalid generator, annotation with empty generator",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{
+							"spec":{
+								"generators":[
+									{}
+								]
+							}
+						}`,
+					},
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+					},
+				},
+			},
+			expectedInvalid: true,
+			expectedNames:   map[string]bool{},
+		},
+	} {
+		hasInvalid, names := invalidGenerators(&c.appSet)
+		assert.Equal(t, c.expectedInvalid, hasInvalid, c.testName)
+		assert.Equal(t, c.expectedNames, names, c.testName)
+	}
+}
+
+func TestCheckInvalidGenerators(t *testing.T) {
+
+	scheme := runtime.NewScheme()
+	err := argoprojiov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+	err = argov1alpha1.AddToScheme(scheme)
+	assert.Nil(t, err)
+
+	for _, c := range []struct {
+		testName    string
+		appSet      argoprojiov1alpha1.ApplicationSet
+		expectedMsg string
+	}{
+		{
+			testName: "invalid generator, without annotation",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-app-set",
+					Namespace: "namespace",
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     &argoprojiov1alpha1.ListGenerator{},
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      &argoprojiov1alpha1.GitGenerator{},
+						},
+					},
+				},
+			},
+			expectedMsg: "ApplicationSet test-app-set contains unrecognized generators",
+		},
+		{
+			testName: "invalid generator, with annotation",
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-app-set",
+					Namespace: "namespace",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{
+							"spec":{
+								"generators":[
+									{"list":{}},
+									{"bbb":{}},
+									{"git":{}},
+									{"aaa":{}}
+								]
+							}
+						}`,
+					},
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{
+						{
+							List:     &argoprojiov1alpha1.ListGenerator{},
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      &argoprojiov1alpha1.GitGenerator{},
+						},
+						{
+							List:     nil,
+							Clusters: nil,
+							Git:      nil,
+						},
+					},
+				},
+			},
+			expectedMsg: "ApplicationSet test-app-set contains unrecognized generators: aaa, bbb",
+		},
+	} {
+		oldhooks := logrus.StandardLogger().ReplaceHooks(logrus.LevelHooks{})
+		defer logrus.StandardLogger().ReplaceHooks(oldhooks)
+		hook := logtest.NewGlobal()
+
+		checkInvalidGenerators(&c.appSet)
+		assert.True(t, len(hook.Entries) >= 1, c.testName)
+		assert.Equal(t, logrus.WarnLevel, hook.LastEntry().Level, c.testName)
+		assert.Equal(t, c.expectedMsg, hook.LastEntry().Message, c.testName)
+		hook.Reset()
+	}
+}