Refactor - Making testing & writing generators more easy

main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"flag"
+	"github.com/argoproj-labs/applicationset/pkg/generators"
 	"github.com/argoproj-labs/applicationset/pkg/services"
 	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	"os"
@@ -66,10 +67,14 @@ func main() {
 	k8s := kubernetes.NewForConfigOrDie(mgr.GetConfig())
 
 	if err = (&controllers.ApplicationSetReconciler{
+		Generators: []generators.Generator{
+			generators.NewListGenerator(),
+			generators.NewClusterGenerator(mgr.GetClient()),
+			generators.NewGitGenerator(services.NewArgoCDService(context.Background(), k8s, namespace, argocdRepoServer)),
+		},
 		Client:      mgr.GetClient(),
 		Scheme:      mgr.GetScheme(),
 		Recorder:    mgr.GetEventRecorderFor("applicationset-controller"),
-		AppsService: services.NewArgoCDService(context.Background(), k8s, namespace, argocdRepoServer),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ApplicationSet")
 		os.Exit(1)

pkg/controllers/applicationset_controller.go

@@ -20,7 +20,7 @@ import (
 	"context"
 	"fmt"
 	"github.com/argoproj-labs/applicationset/pkg/generators"
-	"github.com/argoproj-labs/applicationset/pkg/services"
+	"github.com/argoproj-labs/applicationset/pkg/utils"
 	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -41,10 +41,10 @@ import (
 // ApplicationSetReconciler reconciles a ApplicationSet object
 type ApplicationSetReconciler struct {
 	client.Client
-	Scheme         	*runtime.Scheme
-	Recorder       	record.EventRecorder
-	RepoServerAddr 	string
-	AppsService		services.Apps
+	Scheme   *runtime.Scheme
+	Recorder record.EventRecorder
+	Generators []generators.Generator
+	utils.Renderer
 }
 
 // +kubebuilder:rbac:groups=argoproj.io,resources=applicationsets,verbs=get;list;watch;create;update;patch;delete
@@ -61,39 +61,55 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	listGenerator := generators.NewListGenerator()
-	clusterGenerator := generators.NewClusterGenerator(r.Client)
-	GitGenerator := generators.NewGitGenerator(r.AppsService)
-
 	// desiredApplications is the main list of all expected Applications from all generators in this appset.
-	var desiredApplications []argov1alpha1.Application
-
-	for _, tmpGenerator := range applicationSetInfo.Spec.Generators {
-		var apps []argov1alpha1.Application
-		var err error
-		if tmpGenerator.List != nil {
-			apps, err = listGenerator.GenerateApplications(&tmpGenerator, &applicationSetInfo)
-		} else if tmpGenerator.Clusters != nil {
-			apps, err = clusterGenerator.GenerateApplications(&tmpGenerator, &applicationSetInfo)
-		} else if tmpGenerator.Git != nil {
-			apps, err = GitGenerator.GenerateApplications(&tmpGenerator, &applicationSetInfo)
-		}
-		log.Infof("apps from generator: %+v", apps)
-		if err != nil {
-			log.WithError(err).Error("error generating applications")
-			continue
-		}
-
-		desiredApplications = append(desiredApplications, apps...)
-
-	}
+	desiredApplications := r.extractApplications(applicationSetInfo)
 
 	r.createOrUpdateInCluster(ctx, applicationSetInfo, desiredApplications)
 	r.deleteInCluster(ctx, applicationSetInfo, desiredApplications)
 
 	return ctrl.Result{}, nil
 }
 
+func getTempApplication(applicationSetTemplate argoprojiov1alpha1.ApplicationSetTemplate) *argov1alpha1.Application{
+	var tmplApplication argov1alpha1.Application
+	tmplApplication.Namespace = applicationSetTemplate.Namespace
+	tmplApplication.Name = applicationSetTemplate.Name
+	tmplApplication.Spec = applicationSetTemplate.Spec
+
+	return &tmplApplication
+}
+
+func (r *ApplicationSetReconciler) extractApplications(applicationSetInfo argoprojiov1alpha1.ApplicationSet) []argov1alpha1.Application {
+	res := []argov1alpha1.Application{}
+
+	tmplApplication := getTempApplication(applicationSetInfo.Spec.Template)
+	for _, requestedGenerator := range applicationSetInfo.Spec.Generators {
+		for _, g := range r.Generators {
+			params, err := g.GenerateParams(&requestedGenerator)
+			if err != nil {
+				log.WithError(err).WithField("generator", g).
+					Error("error generating params")
+				continue
+			}
+
+			for _, p := range params {
+				app, err := r.Renderer.RenderTemplateParams(tmplApplication, p)
+				if err != nil {
+					log.WithError(err).WithField("params", params).WithField("generator", g).
+						Error("error generating application from params")
+					continue
+				}
+				res = append(res, *app)
+			}
+
+			log.WithField("generator", g).Infof("generate %d applications", len(res))
+			log.WithField("generator", g).Debugf("apps from generator: %+v", res)
+
+		}
+	}
+	return res
+}
+
 func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	if err := mgr.GetFieldIndexer().IndexField(&argov1alpha1.Application{}, ".metadata.controller", func(rawObj runtime.Object) []string {
 		// grab the job object, extract the owner...

pkg/controllers/applicationset_controller_test.go

@@ -2,9 +2,12 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"github.com/argoproj-labs/applicationset/pkg/generators"
 	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
@@ -16,7 +19,149 @@ import (
 	argoprojiov1alpha1 "github.com/argoproj-labs/applicationset/api/v1alpha1"
 )
 
-func TestCreateOrUpdateApplications(t *testing.T) {
+type generatorMock struct {
+	mock.Mock
+}
+
+func (g *generatorMock) GenerateParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) ([]map[string]string, error) {
+	args := g.Called(appSetGenerator)
+
+	return args.Get(0).([]map[string]string), args.Error(1)
+}
+
+type rendererMock struct {
+	mock.Mock
+}
+
+func (r *rendererMock) RenderTemplateParams(tmpl *argov1alpha1.Application, params map[string]string) (*argov1alpha1.Application, error) {
+	args := r.Called(tmpl, params)
+
+	if args.Error(1) != nil {
+		return nil, args.Error(1)
+	}
+
+	return args.Get(0).(*argov1alpha1.Application), args.Error(1)
+
+}
+
+func TestExtractApplications(t *testing.T) {
+	scheme := runtime.NewScheme()
+	argoprojiov1alpha1.AddToScheme(scheme)
+	argov1alpha1.AddToScheme(scheme)
+
+	client := fake.NewFakeClientWithScheme(scheme)
+
+	for _, c := range []struct {
+		name				string
+		params				[]map[string]string
+		template			argoprojiov1alpha1.ApplicationSetTemplate
+		generateParamsError	error
+		rendererError		error
+	}{
+		{
+			name: 		"Generate two applications",
+			params: 	[]map[string]string{{"name": "app1"}, {"name": "app2"}},
+			template:	argoprojiov1alpha1.ApplicationSetTemplate{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:                       "name",
+					Namespace:                  "namespace",
+					Labels:                     map[string]string{ "label_name": "label_value"},
+				},
+				Spec:       argov1alpha1.ApplicationSpec{
+
+				},
+			},
+		},
+		{
+			name: 		"Handles error for the generator",
+			generateParamsError: errors.New("error"),
+		},
+		{
+			name: 		"Handles error from the render",
+			params: 	[]map[string]string{{"name": "app1"}, {"name": "app2"}},
+			template:	argoprojiov1alpha1.ApplicationSetTemplate{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:                       "name",
+					Namespace:                  "namespace",
+					Labels:                     map[string]string{ "label_name": "label_value"},
+				},
+				Spec:       argov1alpha1.ApplicationSpec{
+
+				},
+			},
+			rendererError: errors.New("error"),
+		},
+	}{
+		cc := c
+		app := argov1alpha1.Application{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:                       "test",
+			},
+		}
+
+		t.Run(cc.name, func(t *testing.T) {
+
+			generatorMock := generatorMock{}
+			generator := argoprojiov1alpha1.ApplicationSetGenerator{
+				List: &argoprojiov1alpha1.ListGenerator{},
+			}
+
+			generatorMock.On("GenerateParams", &generator).
+				Return(cc.params, cc.generateParamsError)
+
+			rendererMock := rendererMock{}
+
+			expectedApps := []argov1alpha1.Application{}
+
+			if cc.generateParamsError == nil {
+				for _, p := range cc.params {
+
+					if cc.rendererError != nil {
+						rendererMock.On("RenderTemplateParams", getTempApplication(cc.template), p).
+							Return(nil, cc.rendererError)
+					} else{
+						rendererMock.On("RenderTemplateParams", getTempApplication(cc.template), p).
+							Return(&app, nil)
+						expectedApps = append(expectedApps, app)
+					}
+				}
+			}
+
+			r := ApplicationSetReconciler{
+				Client:   client,
+				Scheme:   scheme,
+				Recorder: record.NewFakeRecorder(1),
+				Generators: []generators.Generator{
+					&generatorMock,
+				},
+				Renderer: &rendererMock,
+			}
+
+			got := r.extractApplications(argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Generators: []argoprojiov1alpha1.ApplicationSetGenerator{generator},
+					Template: cc.template,
+				},
+			},)
+
+			assert.Equal(t, expectedApps, got)
+			generatorMock.AssertNumberOfCalls(t, "GenerateParams", 1)
+
+			if cc.generateParamsError == nil {
+				rendererMock.AssertNumberOfCalls(t, "RenderTemplateParams", len(cc.params))
+			}
+
+		})
+	}
+
+
+}
+
+func TestCreateOrUpdateInCluster(t *testing.T) {
 
 	scheme := runtime.NewScheme()
 	argoprojiov1alpha1.AddToScheme(scheme)
@@ -203,7 +348,7 @@ func TestCreateOrUpdateApplications(t *testing.T) {
 
 }
 
-func TestDeleteApplications(t *testing.T) {
+func TestDeleteInCluster(t *testing.T) {
 
 	scheme := runtime.NewScheme()
 	argoprojiov1alpha1.AddToScheme(scheme)

pkg/generators/cluster.go

@@ -11,8 +11,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	argoprojiov1alpha1 "github.com/argoproj-labs/applicationset/api/v1alpha1"
-	"github.com/argoproj-labs/applicationset/pkg/utils"
-	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 )
 
 const (
@@ -27,22 +25,23 @@ type ClusterGenerator struct {
 	client.Client
 }
 
+
 func NewClusterGenerator(c client.Client) Generator {
 	g := &ClusterGenerator{
 		Client: c,
 	}
 	return g
 }
 
-func (g *ClusterGenerator) GenerateApplications(
-	appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator,
-	appSet *argoprojiov1alpha1.ApplicationSet) ([]argov1alpha1.Application, error) {
+func (g *ClusterGenerator) GenerateParams(
+	appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) ([]map[string]string, error) {
 
 	if appSetGenerator == nil {
-		return nil, fmt.Errorf("ApplicationSetGenerator is empty")
+		return nil, EmptyAppSetGeneratorError{}
 	}
-	if appSet == nil {
-		return nil, fmt.Errorf("ApplicationSet is empty")
+
+	if appSetGenerator.Clusters == nil {
+		return nil, nil
 	}
 
 	// List all Clusters:
@@ -59,28 +58,18 @@ func (g *ClusterGenerator) GenerateApplications(
 	}
 	log.Debug("clusters matching labels", "count", len(clusterSecretList.Items))
 
-	var tmplApplication argov1alpha1.Application
-	tmplApplication.Namespace = appSet.Spec.Template.Namespace
-	tmplApplication.Name = appSet.Spec.Template.Name
-	tmplApplication.Spec = appSet.Spec.Template.Spec
-
-	var resultingApplications []argov1alpha1.Application
-
-	for _, cluster := range clusterSecretList.Items {
-		params := make(map[string]string)
+	res := make([]map[string]string, len(clusterSecretList.Items))
+	for i, cluster := range clusterSecretList.Items {
+		params := make(map[string]string, len(cluster.ObjectMeta.Labels) + 2)
 		params["name"] = cluster.Name
 		params["server"] = string(cluster.Data["server"])
 		for key, value := range cluster.ObjectMeta.Labels {
 			params[fmt.Sprintf("metadata.labels.%s", key)] = value
 		}
 		log.WithField("cluster", cluster.Name).Info("matched cluster secret")
-		tmpApplication, err := utils.RenderTemplateParams(&tmplApplication, params)
-		if err != nil {
-			log.WithField("cluster", cluster.Name).Error("Error during rendering template params")
-			continue
-		}
-		resultingApplications = append(resultingApplications, *tmpApplication)
+
+		res[i] = params
 	}
 
-	return resultingApplications, nil
+	return res, nil
 }