chore: split controllers to separate packages + cover them with unit tests (#404)

Signed-off-by: odubajDT <[email protected]>

Author: odubajDT

Makefile

@@ -63,13 +63,7 @@ vet: ## Run go vet against code.
 
 .PHONY: component-test
 component-test: manifests generate fmt vet envtest ## Run tests.
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./controllers/... -coverprofile cover-controllers.out
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./webhooks/... -coverprofile cover-webhooks.out
-	sed -i '/mode: set/d' "cover-controllers.out"
-	sed -i '/mode: set/d' "cover-webhooks.out"
-	echo "mode: set" > cover.out
-	cat cover-controllers.out cover-webhooks.out >> cover.out
-	rm cover-controllers.out cover-webhooks.out
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./webhooks/... -coverprofile cover.out
 
 .PHONY: unit-test
 unit-test: manifests fmt vet generate envtest ## Run tests.

apis/core/v1alpha1/featureflagconfiguration_types.go

@@ -145,3 +145,7 @@ func FeatureFlagConfigurationId(namespace, name string) string {
 func FeatureFlagConfigurationConfigMapKey(namespace, name string) string {
 	return fmt.Sprintf("%s.flagd.json", FeatureFlagConfigurationId(namespace, name))
 }
+
+func (p *FeatureFlagServiceProvider) IsSet() bool {
+	return p != nil && p.Name != ""
+}

controllers/common/common.go

@@ -0,0 +1,42 @@
+package common
+
+import (
+	"fmt"
+	"time"
+
+	appsV1 "k8s.io/api/apps/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const (
+	CrdName                           = "FeatureFlagConfiguration"
+	ReconcileErrorInterval            = 10 * time.Second
+	ReconcileSuccessInterval          = 120 * time.Second
+	FinalizerName                     = "featureflagconfiguration.core.openfeature.dev/finalizer"
+	OpenFeatureAnnotationPath         = "spec.template.metadata.annotations.openfeature.dev/openfeature.dev"
+	FlagSourceConfigurationAnnotation = "flagsourceconfiguration"
+	OpenFeatureAnnotationRoot         = "openfeature.dev"
+)
+
+func FlagSourceConfigurationIndex(o client.Object) []string {
+	deployment, ok := o.(*appsV1.Deployment)
+	if !ok {
+		return []string{
+			"false",
+		}
+	}
+
+	if deployment.Spec.Template.ObjectMeta.Annotations == nil {
+		return []string{
+			"false",
+		}
+	}
+	if _, ok := deployment.Spec.Template.ObjectMeta.Annotations[fmt.Sprintf("openfeature.dev/%s", FlagSourceConfigurationAnnotation)]; ok {
+		return []string{
+			"true",
+		}
+	}
+	return []string{
+		"false",
+	}
+}

controllers/common/common_test.go

@@ -0,0 +1,69 @@
+package common
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	appsV1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func TestFlagSourceConfigurationIndex(t *testing.T) {
+	tests := []struct {
+		name string
+		obj  client.Object
+		out  []string
+	}{
+		{
+			name: "non-deployment object",
+			obj:  &appsV1.DaemonSet{},
+			out:  []string{"false"},
+		},
+		{
+			name: "no annotations",
+			obj:  &appsV1.Deployment{},
+			out:  []string{"false"},
+		},
+		{
+			name: "not existing right annotation",
+			obj: &appsV1.Deployment{
+				Spec: appsV1.DeploymentSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{
+								"silly": "some",
+							},
+						},
+					},
+				},
+			},
+			out: []string{"false"},
+		},
+		{
+			name: "existing annotation",
+			obj: &appsV1.Deployment{
+				Spec: appsV1.DeploymentSpec{
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Annotations: map[string]string{
+								fmt.Sprintf("openfeature.dev/%s", FlagSourceConfigurationAnnotation): "true",
+							},
+						},
+					},
+				},
+			},
+			out: []string{"true"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			out := FlagSourceConfigurationIndex(tt.obj)
+			require.Equal(t, tt.out, out)
+		})
+
+	}
+}

controllers/controllers_test.go

@@ -14,23 +14,23 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package controllers
+package featureflagconfiguration
 
 import (
 	"context"
-	"time"
 
 	"github.com/go-logr/logr"
+	"github.com/open-feature/open-feature-operator/controllers/common"
 	"github.com/open-feature/open-feature-operator/pkg/utils"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/client-go/tools/record"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	corev1alpha1 "github.com/open-feature/open-feature-operator/apis/core/v1alpha1"
 )
@@ -41,8 +41,6 @@ type FeatureFlagConfigurationReconciler struct {
 
 	// Scheme contains the scheme of this controller
 	Scheme *runtime.Scheme
-	// Recorder contains the Recorder of this controller
-	Recorder record.EventRecorder
 	// ReqLogger contains the Logger of this controller
 	Log logr.Logger
 }
@@ -61,42 +59,36 @@ type FeatureFlagConfigurationReconciler struct {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
 
-const (
-	crdName                  = "FeatureFlagConfiguration"
-	reconcileErrorInterval   = 10 * time.Second
-	reconcileSuccessInterval = 120 * time.Second
-	finalizerName            = "featureflagconfiguration.core.openfeature.dev/finalizer"
-)
+const CrdName = "FeatureFlagConfiguration"
 
 func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	r.Log = log.FromContext(ctx)
-	r.Log.Info("Reconciling" + crdName)
+	r.Log.Info("Reconciling" + CrdName)
 
 	ffconf := &corev1alpha1.FeatureFlagConfiguration{}
 	if err := r.Client.Get(ctx, req.NamespacedName, ffconf); err != nil {
 		if errors.IsNotFound(err) {
 			// taking down all associated K8s resources is handled by K8s
-			r.Log.Info(crdName + " resource not found. Ignoring since object must be deleted")
+			r.Log.Info(CrdName + " resource not found. Ignoring since object must be deleted")
 			return r.finishReconcile(nil, false)
 		}
-		r.Log.Error(err, "Failed to get the "+crdName)
+		r.Log.Error(err, "Failed to get the "+CrdName)
 		return r.finishReconcile(err, false)
 	}
 
 	if ffconf.ObjectMeta.DeletionTimestamp.IsZero() {
 		// The object is not being deleted, so if it does not have our finalizer,
 		// then lets add the finalizer and update the object. This is equivalent
 		// registering our finalizer.
-		if !utils.ContainsString(ffconf.GetFinalizers(), finalizerName) {
-			controllerutil.AddFinalizer(ffconf, finalizerName)
+		if !utils.ContainsString(ffconf.GetFinalizers(), common.FinalizerName) {
+			controllerutil.AddFinalizer(ffconf, common.FinalizerName)
 			if err := r.Update(ctx, ffconf); err != nil {
 				return r.finishReconcile(err, false)
 			}
 		}
 	} else {
 		// The object is being deleted
-		if utils.ContainsString(ffconf.GetFinalizers(), finalizerName) {
-			controllerutil.RemoveFinalizer(ffconf, finalizerName)
+		if utils.ContainsString(ffconf.GetFinalizers(), common.FinalizerName) {
+			controllerutil.RemoveFinalizer(ffconf, common.FinalizerName)
 			if err := r.Update(ctx, ffconf); err != nil {
 				return ctrl.Result{}, err
 			}
@@ -106,7 +98,7 @@ func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req
 	}
 
 	// Check the provider on the FeatureFlagConfiguration
-	if ffconf.Spec.ServiceProvider == nil {
+	if !ffconf.Spec.ServiceProvider.IsSet() {
 		r.Log.Info("No service provider specified for FeatureFlagConfiguration, using FlagD")
 		ffconf.Spec.ServiceProvider = &corev1alpha1.FeatureFlagServiceProvider{
 			Name: "flagd",
@@ -161,28 +153,20 @@ func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req
 	return r.finishReconcile(nil, false)
 }
 
-// SetupWithManager sets up the controller with the Manager.
-func (r *FeatureFlagConfigurationReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewControllerManagedBy(mgr).
-		For(&corev1alpha1.FeatureFlagConfiguration{}).
-		Owns(&corev1.ConfigMap{}).
-		Complete(r)
-}
-
 func (r *FeatureFlagConfigurationReconciler) finishReconcile(err error, requeueImmediate bool) (ctrl.Result, error) {
 	if err != nil {
-		interval := reconcileErrorInterval
+		interval := common.ReconcileErrorInterval
 		if requeueImmediate {
 			interval = 0
 		}
-		r.Log.Error(err, "Finished Reconciling "+crdName+" with error: %w")
+		r.Log.Error(err, "Finished Reconciling "+CrdName+" with error: %w")
 		return ctrl.Result{Requeue: true, RequeueAfter: interval}, err
 	}
-	interval := reconcileSuccessInterval
+	interval := common.ReconcileSuccessInterval
 	if requeueImmediate {
 		interval = 0
 	}
-	r.Log.Info("Finished Reconciling " + crdName)
+	r.Log.Info("Finished Reconciling " + CrdName)
 	return ctrl.Result{Requeue: true, RequeueAfter: interval}, nil
 }
 
@@ -194,3 +178,11 @@ func (r *FeatureFlagConfigurationReconciler) featureFlagResourceIsOwner(ff *core
 	}
 	return false
 }
+
+// SetupWithManager sets up the controller with the Manager.
+func (r *FeatureFlagConfigurationReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&corev1alpha1.FeatureFlagConfiguration{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
+		Owns(&corev1.ConfigMap{}).
+		Complete(r)
+}