chore: refactor code to decrease complexity (#554)

Signed-off-by: odubajDT <[email protected]>
Signed-off-by: odubajDT <[email protected]>
Co-authored-by: Florian Bacher <[email protected]>

Author: odubajDT

apis/core/v1beta1/featureflagsource_types.go

@@ -194,7 +194,7 @@ func (fc *FeatureFlagSourceSpec) Merge(new *FeatureFlagSourceSpec) {
 	if len(new.EnvVars) != 0 {
 		fc.EnvVars = append(fc.EnvVars, new.EnvVars...)
 	}
-	if new.SyncProviderArgs != nil && len(new.SyncProviderArgs) > 0 {
+	if len(new.SyncProviderArgs) != 0 {
 		fc.SyncProviderArgs = append(fc.SyncProviderArgs, new.SyncProviderArgs...)
 	}
 	if new.EnvVarPrefix != "" {

common/common.go

@@ -2,46 +2,38 @@ package common
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
 	api "github.com/open-feature/open-feature-operator/apis/core/v1beta1"
 	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"
 )
 
 const (
-	ReconcileErrorInterval      = 10 * time.Second
-	ReconcileSuccessInterval    = 120 * time.Second
-	FinalizerName               = "featureflag.core.openfeature.dev/finalizer"
-	OpenFeatureAnnotationPath   = "spec.template.metadata.annotations.openfeature.dev/openfeature.dev"
-	FeatureFlagSourceAnnotation = "featureflagsource"
-	OpenFeatureAnnotationRoot   = "openfeature.dev"
+	ReconcileErrorInterval                          = 10 * time.Second
+	ReconcileSuccessInterval                        = 120 * time.Second
+	FinalizerName                                   = "featureflag.core.openfeature.dev/finalizer"
+	OpenFeatureAnnotationPath                       = "spec.template.metadata.annotations.openfeature.dev/openfeature.dev"
+	OpenFeatureAnnotationRoot                       = "openfeature.dev"
+	FlagdImagePullPolicy          corev1.PullPolicy = "Always"
+	ClusterRoleBindingName        string            = "open-feature-operator-flagd-kubernetes-sync"
+	AllowKubernetesSyncAnnotation                   = "allowkubernetessync"
+	OpenFeatureAnnotationPrefix                     = "openfeature.dev"
+	PodOpenFeatureAnnotationPath                    = "metadata.annotations.openfeature.dev"
+	SourceConfigParam                               = "--sources"
+	ProbeReadiness                                  = "/readyz"
+	ProbeLiveness                                   = "/healthz"
+	ProbeInitialDelay                               = 5
+	FeatureFlagSourceAnnotation                     = "featureflagsource"
+	EnabledAnnotation                               = "enabled"
 )
 
-type EnvConfig struct {
-	PodNamespace    string `envconfig:"POD_NAMESPACE" default:"open-feature-operator-system"`
-	FlagdProxyImage string `envconfig:"FLAGD_PROXY_IMAGE" default:"ghcr.io/open-feature/flagd-proxy"`
-	// renovate: datasource=github-tags depName=open-feature/flagd/flagd-proxy
-	FlagdProxyTag            string `envconfig:"FLAGD_PROXY_TAG" default:"v0.3.0"`
-	FlagdProxyPort           int    `envconfig:"FLAGD_PROXY_PORT" default:"8015"`
-	FlagdProxyManagementPort int    `envconfig:"FLAGD_PROXY_MANAGEMENT_PORT" default:"8016"`
-	FlagdProxyDebugLogging   bool   `envconfig:"FLAGD_PROXY_DEBUG_LOGGING" default:"false"`
-
-	SidecarEnvVarPrefix   string `envconfig:"SIDECAR_ENV_VAR_PREFIX" default:"FLAGD"`
-	SidecarManagementPort int    `envconfig:"SIDECAR_MANAGEMENT_PORT" default:"8014"`
-	SidecarPort           int    `envconfig:"SIDECAR_PORT" default:"8013"`
-	SidecarImage          string `envconfig:"SIDECAR_IMAGE" default:"ghcr.io/open-feature/flagd"`
-	// renovate: datasource=github-tags depName=open-feature/flagd/flagd-proxy
-	SidecarTag           string `envconfig:"SIDECAR_TAG" default:"v0.7.0"`
-	SidecarSocketPath    string `envconfig:"SIDECAR_SOCKET_PATH" default:""`
-	SidecarEvaluator     string `envconfig:"SIDECAR_EVALUATOR" default:"json"`
-	SidecarProviderArgs  string `envconfig:"SIDECAR_PROVIDER_ARGS" default:""`
-	SidecarSyncProvider  string `envconfig:"SIDECAR_SYNC_PROVIDER" default:"kubernetes"`
-	SidecarLogFormat     string `envconfig:"SIDECAR_LOG_FORMAT" default:"json"`
-	SidecarProbesEnabled bool   `envconfig:"SIDECAR_PROBES_ENABLED" default:"true"`
-}
+var ErrFlagdProxyNotReady = errors.New("flagd-proxy is not ready, deferring pod admission")
+var ErrUnrecognizedSyncProvider = errors.New("unrecognized sync provider")
 
 func FeatureFlagSourceIndex(o client.Object) []string {
 	deployment, ok := o.(*appsV1.Deployment)

common/constant/configuration.go

@@ -10,7 +10,6 @@ import (
 	api "github.com/open-feature/open-feature-operator/apis/core/v1beta1"
 	apicommon "github.com/open-feature/open-feature-operator/apis/core/v1beta1/common"
 	"github.com/open-feature/open-feature-operator/common"
-	"github.com/open-feature/open-feature-operator/common/constant"
 	"github.com/open-feature/open-feature-operator/common/flagdproxy"
 	"github.com/open-feature/open-feature-operator/common/types"
 	"github.com/open-feature/open-feature-operator/common/utils"
@@ -46,7 +45,7 @@ type FlagdContainerInjector struct {
 	Client                    client.Client
 	Logger                    logr.Logger
 	FlagdProxyConfig          *flagdproxy.FlagdProxyConfiguration
-	FlagDResourceRequirements corev1.ResourceRequirements
+	FlagdResourceRequirements corev1.ResourceRequirements
 	Image                     string
 	Tag                       string
 }
@@ -63,8 +62,8 @@ func (fi *FlagdContainerInjector) InjectFlagd(
 
 	// Enable probes
 	if flagSourceConfig.ProbesEnabled != nil && *flagSourceConfig.ProbesEnabled {
-		flagdContainer.LivenessProbe = buildProbe(constant.ProbeLiveness, int(flagSourceConfig.ManagementPort))
-		flagdContainer.ReadinessProbe = buildProbe(constant.ProbeReadiness, int(flagSourceConfig.ManagementPort))
+		flagdContainer.LivenessProbe = buildProbe(common.ProbeLiveness, int(flagSourceConfig.ManagementPort))
+		flagdContainer.ReadinessProbe = buildProbe(common.ProbeReadiness, int(flagSourceConfig.ManagementPort))
 	}
 
 	if err := fi.handleSidecarSources(ctx, objectMeta, podSpec, flagSourceConfig, &flagdContainer); err != nil {
@@ -128,48 +127,63 @@ func (fi *FlagdContainerInjector) InjectFlagd(
 // service account under the given namespace (required for kubernetes sync provider)
 func (fi *FlagdContainerInjector) EnableClusterRoleBinding(ctx context.Context, namespace, serviceAccountName string) error {
 	serviceAccount := client.ObjectKey{
-		Name:      serviceAccountName,
+		Name:      determineServiceAccountName(serviceAccountName),
 		Namespace: namespace,
 	}
-	if serviceAccountName == "" {
-		serviceAccount.Name = "default"
-	}
+
 	// Check if the service account exists
 	fi.Logger.V(1).Info(fmt.Sprintf("Fetching serviceAccount: %s/%s", serviceAccount.Namespace, serviceAccount.Name))
 	sa := corev1.ServiceAccount{}
 	if err := fi.Client.Get(ctx, serviceAccount, &sa); err != nil {
 		fi.Logger.V(1).Info(fmt.Sprintf("ServiceAccount not found: %s/%s", serviceAccount.Namespace, serviceAccount.Name))
 		return err
 	}
-	fi.Logger.V(1).Info(fmt.Sprintf("Fetching clusterrolebinding: %s", constant.ClusterRoleBindingName))
+
+	fi.Logger.V(1).Info(fmt.Sprintf("Fetching clusterrolebinding: %s", common.ClusterRoleBindingName))
 	// Fetch service account if it exists
 	crb := rbacv1.ClusterRoleBinding{}
-	if err := fi.Client.Get(ctx, client.ObjectKey{Name: constant.ClusterRoleBindingName}, &crb); errors.IsNotFound(err) {
-		fi.Logger.V(1).Info(fmt.Sprintf("ClusterRoleBinding not found: %s", constant.ClusterRoleBindingName))
+	if err := fi.Client.Get(ctx, client.ObjectKey{Name: common.ClusterRoleBindingName}, &crb); errors.IsNotFound(err) {
+		fi.Logger.V(1).Info(fmt.Sprintf("ClusterRoleBinding not found: %s", common.ClusterRoleBindingName))
 		return err
 	}
-	found := false
+
+	if !fi.isServiceAccountSet(&crb, serviceAccount) {
+		return fi.updateServiceAccount(ctx, &crb, serviceAccount)
+	}
+
+	return nil
+}
+
+func determineServiceAccountName(name string) string {
+	if name == "" {
+		return "default"
+	}
+	return name
+}
+
+func (fi *FlagdContainerInjector) isServiceAccountSet(crb *rbacv1.ClusterRoleBinding, serviceAccount client.ObjectKey) bool {
 	for _, subject := range crb.Subjects {
 		if subject.Kind == "ServiceAccount" && subject.Name == serviceAccount.Name && subject.Namespace == serviceAccount.Namespace {
 			fi.Logger.V(1).Info(fmt.Sprintf("ClusterRoleBinding already exists for service account: %s/%s", serviceAccount.Namespace, serviceAccount.Name))
-			found = true
+			return true
 		}
 	}
-	if !found {
-		fi.Logger.V(1).Info(fmt.Sprintf("Updating ClusterRoleBinding %s for service account: %s/%s", crb.Name,
-			serviceAccount.Namespace, serviceAccount.Name))
-		crb.Subjects = append(crb.Subjects, rbacv1.Subject{
-			Kind:      "ServiceAccount",
-			Name:      serviceAccount.Name,
-			Namespace: serviceAccount.Namespace,
-		})
-		if err := fi.Client.Update(ctx, &crb); err != nil {
-			fi.Logger.V(1).Info(fmt.Sprintf("Failed to update ClusterRoleBinding: %s", err.Error()))
-			return err
-		}
+	return false
+}
+
+func (fi *FlagdContainerInjector) updateServiceAccount(ctx context.Context, crb *rbacv1.ClusterRoleBinding, serviceAccount client.ObjectKey) error {
+	fi.Logger.V(1).Info(fmt.Sprintf("Updating ClusterRoleBinding %s for service account: %s/%s", crb.Name,
+		serviceAccount.Namespace, serviceAccount.Name))
+	crb.Subjects = append(crb.Subjects, rbacv1.Subject{
+		Kind:      "ServiceAccount",
+		Name:      serviceAccount.Name,
+		Namespace: serviceAccount.Namespace,
+	})
+	if err := fi.Client.Update(ctx, crb); err != nil {
+		fi.Logger.V(1).Info(fmt.Sprintf("Failed to update ClusterRoleBinding: %s", err.Error()))
+		return err
 	}
 	fi.Logger.V(1).Info(fmt.Sprintf("Updated ClusterRoleBinding: %s", crb.Name))
-
 	return nil
 }
 
@@ -186,7 +200,6 @@ func (fi *FlagdContainerInjector) handleSidecarSources(ctx context.Context, obje
 	return nil
 }
 
-//nolint:gocyclo
 func (fi *FlagdContainerInjector) buildSources(ctx context.Context, objectMeta *metav1.ObjectMeta, flagSourceConfig *api.FeatureFlagSourceSpec, podSpec *corev1.PodSpec, sidecar *corev1.Container) ([]types.SourceConfig, error) {
 	var sourceCfgCollection []types.SourceConfig
 
@@ -195,40 +208,40 @@ func (fi *FlagdContainerInjector) buildSources(ctx context.Context, objectMeta *
 			source.Provider = flagSourceConfig.DefaultSyncProvider
 		}
 
-		var sourceCfg types.SourceConfig
-		var err error
-
-		switch {
-		case source.Provider.IsKubernetes():
-			sourceCfg, err = fi.toKubernetesProviderConfig(ctx, objectMeta, podSpec, source)
-			if err != nil {
-				return []types.SourceConfig{}, err
-			}
-		case source.Provider.IsFilepath():
-			sourceCfg, err = fi.toFilepathProviderConfig(ctx, objectMeta, podSpec, sidecar, source)
-			if err != nil {
-				return []types.SourceConfig{}, err
-			}
-		case source.Provider.IsHttp():
-			sourceCfg = fi.toHttpProviderConfig(source)
-		case source.Provider.IsGrpc():
-			sourceCfg = fi.toGrpcProviderConfig(source)
-		case source.Provider.IsFlagdProxy():
-			sourceCfg, err = fi.toFlagdProxyConfig(ctx, objectMeta, source)
-			if err != nil {
-				return []types.SourceConfig{}, err
-			}
-		default:
-			return []types.SourceConfig{}, fmt.Errorf("could not add provider %s: %w", source.Provider, constant.ErrUnrecognizedSyncProvider)
+		sourceCfg, err := fi.newSourceConfig(ctx, source, objectMeta, podSpec, sidecar)
+		if err != nil {
+			return []types.SourceConfig{}, err
 		}
 
-		sourceCfgCollection = append(sourceCfgCollection, sourceCfg)
+		sourceCfgCollection = append(sourceCfgCollection, *sourceCfg)
 
 	}
 
 	return sourceCfgCollection, nil
 }
 
+func (fi *FlagdContainerInjector) newSourceConfig(ctx context.Context, source api.Source, objectMeta *metav1.ObjectMeta, podSpec *corev1.PodSpec, sidecar *corev1.Container) (*types.SourceConfig, error) {
+	sourceCfg := types.SourceConfig{}
+	var err error = nil
+
+	switch {
+	case source.Provider.IsKubernetes():
+		sourceCfg, err = fi.toKubernetesProviderConfig(ctx, objectMeta, podSpec, source)
+	case source.Provider.IsFilepath():
+		sourceCfg, err = fi.toFilepathProviderConfig(ctx, objectMeta, podSpec, sidecar, source)
+	case source.Provider.IsHttp():
+		sourceCfg = fi.toHttpProviderConfig(source)
+	case source.Provider.IsGrpc():
+		sourceCfg = fi.toGrpcProviderConfig(source)
+	case source.Provider.IsFlagdProxy():
+		sourceCfg, err = fi.toFlagdProxyConfig(ctx, objectMeta, source)
+	default:
+		err = fmt.Errorf("could not add provider %s: %w", source.Provider, common.ErrUnrecognizedSyncProvider)
+	}
+
+	return &sourceCfg, err
+}
+
 func (fi *FlagdContainerInjector) toFilepathProviderConfig(ctx context.Context, objectMeta *metav1.ObjectMeta, podSpec *corev1.PodSpec, sidecar *corev1.Container, source api.Source) (types.SourceConfig, error) {
 	// create config map
 	ns, n := utils.ParseAnnotation(source.Source, objectMeta.Namespace)
@@ -312,7 +325,7 @@ func (fi *FlagdContainerInjector) toFlagdProxyConfig(ctx context.Context, object
 		return types.SourceConfig{}, err
 	}
 	if !exists || (exists && !ready) {
-		return types.SourceConfig{}, constant.ErrFlagdProxyNotReady
+		return types.SourceConfig{}, common.ErrFlagdProxyNotReady
 	}
 	ns, n := utils.ParseAnnotation(source.Source, objectMeta.Namespace)
 	return types.SourceConfig{
@@ -339,7 +352,7 @@ func (fi *FlagdContainerInjector) isFlagdProxyReady(ctx context.Context) (bool,
 			return true, false, fmt.Errorf(
 				"flagd-proxy not ready after 3 minutes, was created at %s: %w",
 				d.CreationTimestamp.Time.String(),
-				constant.ErrFlagdProxyNotReady,
+				common.ErrFlagdProxyNotReady,
 			)
 		}
 		return true, false, nil
@@ -365,7 +378,7 @@ func (fi *FlagdContainerInjector) toKubernetesProviderConfig(ctx context.Context
 	if objectMeta.Annotations == nil {
 		objectMeta.Annotations = map[string]string{}
 	}
-	objectMeta.Annotations[fmt.Sprintf("%s/%s", constant.OpenFeatureAnnotationPrefix, constant.AllowKubernetesSyncAnnotation)] = "true"
+	objectMeta.Annotations[fmt.Sprintf("%s/%s", common.OpenFeatureAnnotationPrefix, common.AllowKubernetesSyncAnnotation)] = "true"
 
 	// build K8s config
 	return types.SourceConfig{
@@ -381,7 +394,7 @@ func (fi *FlagdContainerInjector) generateBasicFlagdContainer(flagSourceConfig *
 		Args: []string{
 			"start",
 		},
-		ImagePullPolicy: constant.FlagDImagePullPolicy,
+		ImagePullPolicy: common.FlagdImagePullPolicy,
 		VolumeMounts:    []corev1.VolumeMount{},
 		Env:             []corev1.EnvVar{},
 		Ports: []corev1.ContainerPort{
@@ -391,7 +404,7 @@ func (fi *FlagdContainerInjector) generateBasicFlagdContainer(flagSourceConfig *
 			},
 		},
 		SecurityContext: getSecurityContext(),
-		Resources:       fi.FlagDResourceRequirements,
+		Resources:       fi.FlagdResourceRequirements,
 	}
 }
 
@@ -437,7 +450,7 @@ func appendSources(sources []types.SourceConfig, sidecar *corev1.Container) erro
 		return err
 	}
 
-	sidecar.Args = append(sidecar.Args, constant.SourceConfigParam, string(bytes))
+	sidecar.Args = append(sidecar.Args, common.SourceConfigParam, string(bytes))
 	return nil
 }
 
@@ -479,6 +492,6 @@ func buildProbe(path string, port int) *corev1.Probe {
 		ProbeHandler: corev1.ProbeHandler{
 			HTTPGet: httpGetAction,
 		},
-		InitialDelaySeconds: constant.ProbeInitialDelay,
+		InitialDelaySeconds: common.ProbeInitialDelay,
 	}
 }