Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: refactor code to decrease complexity #554

Merged
merged 8 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apis/core/v1beta1/featureflagsource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
48 changes: 20 additions & 28 deletions common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 0 additions & 17 deletions common/constant/configuration.go

This file was deleted.

6 changes: 0 additions & 6 deletions common/constant/errors.go

This file was deleted.

133 changes: 73 additions & 60 deletions common/flagdinjector/flagdinjector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}

Expand All @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -391,7 +404,7 @@ func (fi *FlagdContainerInjector) generateBasicFlagdContainer(flagSourceConfig *
},
},
SecurityContext: getSecurityContext(),
Resources: fi.FlagDResourceRequirements,
Resources: fi.FlagdResourceRequirements,
}
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -479,6 +492,6 @@ func buildProbe(path string, port int) *corev1.Probe {
ProbeHandler: corev1.ProbeHandler{
HTTPGet: httpGetAction,
},
InitialDelaySeconds: constant.ProbeInitialDelay,
InitialDelaySeconds: common.ProbeInitialDelay,
}
}
Loading
Loading