From 5ba5bc97faa8bf18a07a380d685c518f6e093145 Mon Sep 17 00:00:00 2001 From: odubajDT <93584209+odubajDT@users.noreply.github.com> Date: Thu, 30 Mar 2023 10:19:47 +0200 Subject: [PATCH] chore: introduce additional unit tests for api packages (#420) Signed-off-by: odubajDT Co-authored-by: Michael Beemer --- .../featureflagconfiguration_types.go | 18 +- .../featureflagconfiguration_types_test.go | 80 +++++ .../v1alpha1/flagsourceconfiguration_types.go | 4 +- .../flagsourceconfiguration_types_test.go | 288 ++++++++++++++++++ .../featureflagconfiguration_types.go | 11 - .../featureflagconfiguration/controller.go | 6 +- .../controller_test.go | 5 +- pkg/utils/utils.go | 12 + pkg/utils/utils_test.go | 31 ++ webhooks/pod_webhook.go | 8 +- 10 files changed, 426 insertions(+), 37 deletions(-) create mode 100644 apis/core/v1alpha1/featureflagconfiguration_types_test.go create mode 100644 apis/core/v1alpha1/flagsourceconfiguration_types_test.go create mode 100644 pkg/utils/utils_test.go diff --git a/apis/core/v1alpha1/featureflagconfiguration_types.go b/apis/core/v1alpha1/featureflagconfiguration_types.go index 8c92c8a9a..ead14c34b 100644 --- a/apis/core/v1alpha1/featureflagconfiguration_types.go +++ b/apis/core/v1alpha1/featureflagconfiguration_types.go @@ -17,8 +17,6 @@ limitations under the License. package v1alpha1 import ( - "fmt" - "github.com/open-feature/open-feature-operator/pkg/utils" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -110,7 +108,7 @@ func init() { SchemeBuilder.Register(&FeatureFlagConfiguration{}, &FeatureFlagConfigurationList{}) } -func GetFfReference(ff *FeatureFlagConfiguration) metav1.OwnerReference { +func (ff *FeatureFlagConfiguration) GetReference() metav1.OwnerReference { return metav1.OwnerReference{ APIVersion: ff.APIVersion, Kind: ff.Kind, @@ -120,7 +118,7 @@ func GetFfReference(ff *FeatureFlagConfiguration) metav1.OwnerReference { } } -func GenerateFfConfigMap(name string, namespace string, references []metav1.OwnerReference, spec FeatureFlagConfigurationSpec) corev1.ConfigMap { +func (ff *FeatureFlagConfiguration) GenerateConfigMap(name string, namespace string, references []metav1.OwnerReference) corev1.ConfigMap { return corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -131,21 +129,11 @@ func GenerateFfConfigMap(name string, namespace string, references []metav1.Owne OwnerReferences: references, }, Data: map[string]string{ - FeatureFlagConfigurationConfigMapKey(namespace, name): spec.FeatureFlagSpec, + utils.FeatureFlagConfigurationConfigMapKey(namespace, name): ff.Spec.FeatureFlagSpec, }, } } -// unique string used to create unique volume mount and file name -func FeatureFlagConfigurationId(namespace, name string) string { - return fmt.Sprintf("%s_%s", namespace, name) -} - -// unique key (and filename) for configMap data -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 != "" } diff --git a/apis/core/v1alpha1/featureflagconfiguration_types_test.go b/apis/core/v1alpha1/featureflagconfiguration_types_test.go new file mode 100644 index 000000000..dd886342c --- /dev/null +++ b/apis/core/v1alpha1/featureflagconfiguration_types_test.go @@ -0,0 +1,80 @@ +package v1alpha1 + +import ( + "testing" + + "github.com/open-feature/open-feature-operator/pkg/utils" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +func Test_FeatureFlagConfiguration(t *testing.T) { + ffConfig := FeatureFlagConfiguration{ + ObjectMeta: v1.ObjectMeta{ + Name: "ffconf1", + Namespace: "test", + OwnerReferences: []v1.OwnerReference{ + { + APIVersion: "ver", + Kind: "kind", + Name: "ffconf1", + UID: types.UID("5"), + Controller: utils.TrueVal(), + }, + }, + }, + Spec: FeatureFlagConfigurationSpec{ + FeatureFlagSpec: "spec", + }, + } + + name := "cmname" + namespace := "cmnamespace" + references := []v1.OwnerReference{ + { + APIVersion: "ver", + Kind: "kind", + Name: "ffconf1", + UID: types.UID("5"), + Controller: utils.TrueVal(), + }, + } + + require.Equal(t, v1.OwnerReference{ + APIVersion: ffConfig.APIVersion, + Kind: ffConfig.Kind, + Name: ffConfig.Name, + UID: ffConfig.UID, + Controller: utils.TrueVal(), + }, ffConfig.GetReference()) + + require.Equal(t, corev1.ConfigMap{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{ + "openfeature.dev/featureflagconfiguration": name, + }, + OwnerReferences: references, + }, + Data: map[string]string{ + "cmnamespace_cmname.flagd.json": "spec", + }, + }, ffConfig.GenerateConfigMap(name, namespace, references)) + + require.False(t, ffConfig.Spec.ServiceProvider.IsSet()) + + ffConfig.Spec.ServiceProvider = &FeatureFlagServiceProvider{ + Name: "", + } + + require.False(t, ffConfig.Spec.ServiceProvider.IsSet()) + + ffConfig.Spec.ServiceProvider = &FeatureFlagServiceProvider{ + Name: "prov", + } + + require.True(t, ffConfig.Spec.ServiceProvider.IsSet()) +} diff --git a/apis/core/v1alpha1/flagsourceconfiguration_types.go b/apis/core/v1alpha1/flagsourceconfiguration_types.go index f6a741302..a5bc213c6 100644 --- a/apis/core/v1alpha1/flagsourceconfiguration_types.go +++ b/apis/core/v1alpha1/flagsourceconfiguration_types.go @@ -223,7 +223,7 @@ func NewFlagSourceConfigurationSpec() (*FlagSourceConfigurationSpec, error) { fsc.DefaultSyncProvider = SyncProviderType(syncProvider) } - if logFormat := os.Getenv(fmt.Sprintf("%s_%s", InputConfigurationEnvVarPrefix, SidecarLogFormatEnvVar)); logFormat != "" { + if logFormat := os.Getenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarLogFormatEnvVar)); logFormat != "" { fsc.LogFormat = logFormat } @@ -231,7 +231,7 @@ func NewFlagSourceConfigurationSpec() (*FlagSourceConfigurationSpec, error) { fsc.EnvVarPrefix = envVarPrefix } - if probesEnabled := os.Getenv(fmt.Sprintf("%s_%s", InputConfigurationEnvVarPrefix, SidecarProbesEnabledVar)); probesEnabled != "" { + if probesEnabled := os.Getenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarProbesEnabledVar)); probesEnabled != "" { b, err := strconv.ParseBool(probesEnabled) if err != nil { return fsc, fmt.Errorf("unable to parse sidecar probes enabled %s to boolean: %w", probesEnabled, err) diff --git a/apis/core/v1alpha1/flagsourceconfiguration_types_test.go b/apis/core/v1alpha1/flagsourceconfiguration_types_test.go new file mode 100644 index 000000000..1299cc9a9 --- /dev/null +++ b/apis/core/v1alpha1/flagsourceconfiguration_types_test.go @@ -0,0 +1,288 @@ +package v1alpha1 + +import ( + "testing" + + "github.com/open-feature/open-feature-operator/pkg/utils" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" +) + +func Test_FLagSourceConfiguration_SyncProvider(t *testing.T) { + k := SyncProviderKubernetes + f := SyncProviderFilepath + h := SyncProviderHttp + + require.True(t, k.IsKubernetes()) + require.True(t, f.IsFilepath()) + require.True(t, h.IsHttp()) + + require.False(t, f.IsKubernetes()) + require.False(t, h.IsFilepath()) + require.False(t, k.IsHttp()) +} + +func Test_FLagSourceConfiguration_envVarKey(t *testing.T) { + require.Equal(t, "pre_suf", envVarKey("pre", "suf")) +} + +func Test_FLagSourceConfiguration_ToEnvVars(t *testing.T) { + ff := FlagSourceConfiguration{ + Spec: FlagSourceConfigurationSpec{ + EnvVars: []v1.EnvVar{ + { + Name: "env1", + Value: "val1", + }, + { + Name: "env2", + Value: "val2", + }, + }, + EnvVarPrefix: "PRE", + MetricsPort: 22, + Port: 33, + Evaluator: "evaluator", + SocketPath: "socket-path", + LogFormat: "log", + }, + } + expected := []v1.EnvVar{ + { + Name: "PRE_env1", + Value: "val1", + }, + { + Name: "PRE_env2", + Value: "val2", + }, + { + Name: "PRE_METRICS_PORT", + Value: "22", + }, + { + Name: "PRE_PORT", + Value: "33", + }, + { + Name: "PRE_EVALUATOR", + Value: "evaluator", + }, + { + Name: "PRE_SOCKET_PATH", + Value: "socket-path", + }, + { + Name: "PRE_LOG_FORMAT", + Value: "log", + }, + } + require.Equal(t, expected, ff.Spec.ToEnvVars()) +} + +func Test_FLagSourceConfiguration_Merge(t *testing.T) { + ff_old := &FlagSourceConfiguration{ + Spec: FlagSourceConfigurationSpec{ + EnvVars: []v1.EnvVar{ + { + Name: "env1", + Value: "val1", + }, + { + Name: "env2", + Value: "val2", + }, + }, + EnvVarPrefix: "PRE", + MetricsPort: 22, + Port: 33, + Evaluator: "evaluator", + SocketPath: "socket-path", + LogFormat: "log", + Image: "img", + Tag: "tag", + Sources: []Source{ + { + Source: "src1", + Provider: SyncProviderKubernetes, + HttpSyncBearerToken: "token1", + LogFormat: "log1", + }, + }, + SyncProviderArgs: []string{"arg1", "arg2"}, + DefaultSyncProvider: SyncProviderKubernetes, + RolloutOnChange: utils.TrueVal(), + ProbesEnabled: utils.TrueVal(), + }, + } + + ff_old.Spec.Merge(nil) + + require.Equal(t, &FlagSourceConfiguration{ + Spec: FlagSourceConfigurationSpec{ + EnvVars: []v1.EnvVar{ + { + Name: "env1", + Value: "val1", + }, + { + Name: "env2", + Value: "val2", + }, + }, + EnvVarPrefix: "PRE", + MetricsPort: 22, + Port: 33, + Evaluator: "evaluator", + SocketPath: "socket-path", + LogFormat: "log", + Image: "img", + Tag: "tag", + Sources: []Source{ + { + Source: "src1", + Provider: SyncProviderKubernetes, + HttpSyncBearerToken: "token1", + LogFormat: "log1", + }, + }, + SyncProviderArgs: []string{"arg1", "arg2"}, + DefaultSyncProvider: SyncProviderKubernetes, + RolloutOnChange: utils.TrueVal(), + ProbesEnabled: utils.TrueVal(), + }, + }, ff_old) + + ff_new := &FlagSourceConfiguration{ + Spec: FlagSourceConfigurationSpec{ + EnvVars: []v1.EnvVar{ + { + Name: "env3", + Value: "val3", + }, + { + Name: "env4", + Value: "val4", + }, + }, + EnvVarPrefix: "PREFIX", + MetricsPort: 221, + Port: 331, + Evaluator: "evaluator1", + SocketPath: "socket-path1", + LogFormat: "log1", + Image: "img1", + Tag: "tag1", + Sources: []Source{ + { + Source: "src2", + Provider: SyncProviderFilepath, + HttpSyncBearerToken: "token2", + LogFormat: "log2", + }, + }, + SyncProviderArgs: []string{"arg3", "arg4"}, + DefaultSyncProvider: SyncProviderFilepath, + RolloutOnChange: utils.FalseVal(), + ProbesEnabled: utils.FalseVal(), + }, + } + + ff_old.Spec.Merge(&ff_new.Spec) + + require.Equal(t, &FlagSourceConfiguration{ + Spec: FlagSourceConfigurationSpec{ + EnvVars: []v1.EnvVar{ + { + Name: "env1", + Value: "val1", + }, + { + Name: "env2", + Value: "val2", + }, + { + Name: "env3", + Value: "val3", + }, + { + Name: "env4", + Value: "val4", + }, + }, + EnvVarPrefix: "PREFIX", + MetricsPort: 221, + Port: 331, + Evaluator: "evaluator1", + SocketPath: "socket-path1", + LogFormat: "log1", + Image: "img1", + Tag: "tag1", + Sources: []Source{ + { + Source: "src1", + Provider: SyncProviderKubernetes, + HttpSyncBearerToken: "token1", + LogFormat: "log1", + }, + { + Source: "src2", + Provider: SyncProviderFilepath, + HttpSyncBearerToken: "token2", + LogFormat: "log2", + }, + }, + SyncProviderArgs: []string{"arg1", "arg2", "arg3", "arg4"}, + DefaultSyncProvider: SyncProviderFilepath, + RolloutOnChange: utils.FalseVal(), + ProbesEnabled: utils.FalseVal(), + }, + }, ff_old) +} + +func Test_FLagSourceConfiguration_NewFlagSourceConfigurationSpec(t *testing.T) { + //happy path + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarMetricPortEnvVar), "22") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarPortEnvVar), "33") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarSocketPathEnvVar), "val1") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarEvaluatorEnvVar), "val2") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarImageEnvVar), "val3") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarVersionEnvVar), "val4") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarProviderArgsEnvVar), "val11,val22") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarDefaultSyncProviderEnvVar), "kubernetes") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarLogFormatEnvVar), "val5") + t.Setenv(SidecarEnvVarPrefix, "val6") + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarProbesEnabledVar), "true") + + fs, err := NewFlagSourceConfigurationSpec() + + require.Nil(t, err) + require.Equal(t, &FlagSourceConfigurationSpec{ + MetricsPort: 22, + Port: 33, + SocketPath: "val1", + Evaluator: "val2", + Image: "val3", + Tag: "val4", + Sources: []Source{}, + EnvVars: []v1.EnvVar{}, + SyncProviderArgs: []string{"val11", "val22"}, + DefaultSyncProvider: SyncProviderKubernetes, + EnvVarPrefix: "val6", + LogFormat: "val5", + ProbesEnabled: utils.TrueVal(), + }, fs) + + //error paths + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarProbesEnabledVar), "blah") + _, err = NewFlagSourceConfigurationSpec() + require.NotNil(t, err) + + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarPortEnvVar), "blah") + _, err = NewFlagSourceConfigurationSpec() + require.NotNil(t, err) + + t.Setenv(envVarKey(InputConfigurationEnvVarPrefix, SidecarMetricPortEnvVar), "blah") + _, err = NewFlagSourceConfigurationSpec() + require.NotNil(t, err) +} diff --git a/apis/core/v1alpha2/featureflagconfiguration_types.go b/apis/core/v1alpha2/featureflagconfiguration_types.go index 6039714dc..b284a709c 100644 --- a/apis/core/v1alpha2/featureflagconfiguration_types.go +++ b/apis/core/v1alpha2/featureflagconfiguration_types.go @@ -19,7 +19,6 @@ package v1alpha2 import ( "encoding/json" - "github.com/open-feature/open-feature-operator/pkg/utils" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -136,13 +135,3 @@ type FeatureFlagConfigurationList struct { func init() { SchemeBuilder.Register(&FeatureFlagConfiguration{}, &FeatureFlagConfigurationList{}) } - -func GetFfReference(ff *FeatureFlagConfiguration) metav1.OwnerReference { - return metav1.OwnerReference{ - APIVersion: ff.APIVersion, - Kind: ff.Kind, - Name: ff.Name, - UID: ff.UID, - Controller: utils.TrueVal(), - } -} diff --git a/controllers/core/featureflagconfiguration/controller.go b/controllers/core/featureflagconfiguration/controller.go index 713c78bdd..42a23be94 100644 --- a/controllers/core/featureflagconfiguration/controller.go +++ b/controllers/core/featureflagconfiguration/controller.go @@ -128,7 +128,7 @@ func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req // Append OwnerReference if not set if !r.featureFlagResourceIsOwner(ffconf, cm) { r.Log.Info("Setting owner reference for " + cm.Name) - cm.OwnerReferences = append(cm.OwnerReferences, corev1alpha1.GetFfReference(ffconf)) + cm.OwnerReferences = append(cm.OwnerReferences, ffconf.GetReference()) err := r.Client.Update(ctx, &cm) if err != nil { return r.finishReconcile(err, true) @@ -142,7 +142,7 @@ func (r *FeatureFlagConfigurationReconciler) Reconcile(ctx context.Context, req // Update ConfigMap Spec r.Log.Info("Updating ConfigMap Spec " + cm.Name) cm.Data = map[string]string{ - corev1alpha1.FeatureFlagConfigurationConfigMapKey(cm.Namespace, cm.Name): ffconf.Spec.FeatureFlagSpec, + utils.FeatureFlagConfigurationConfigMapKey(cm.Namespace, cm.Name): ffconf.Spec.FeatureFlagSpec, } err := r.Client.Update(ctx, &cm) if err != nil { @@ -172,7 +172,7 @@ func (r *FeatureFlagConfigurationReconciler) finishReconcile(err error, requeueI func (r *FeatureFlagConfigurationReconciler) featureFlagResourceIsOwner(ff *corev1alpha1.FeatureFlagConfiguration, cm corev1.ConfigMap) bool { for _, cmOwner := range cm.OwnerReferences { - if cmOwner.UID == corev1alpha1.GetFfReference(ff).UID { + if cmOwner.UID == ff.GetReference().UID { return true } } diff --git a/controllers/core/featureflagconfiguration/controller_test.go b/controllers/core/featureflagconfiguration/controller_test.go index f32f53c5a..f26d7d0f0 100644 --- a/controllers/core/featureflagconfiguration/controller_test.go +++ b/controllers/core/featureflagconfiguration/controller_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/open-feature/open-feature-operator/apis/core/v1alpha1" + "github.com/open-feature/open-feature-operator/pkg/utils" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -58,7 +59,7 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { }, }, Data: map[string]string{ - v1alpha1.FeatureFlagConfigurationConfigMapKey(testNamespace, cmName): "spec", + utils.FeatureFlagConfigurationConfigMapKey(testNamespace, cmName): "spec", }, }, cmDeleted: false, @@ -121,7 +122,7 @@ func TestFlagSourceConfigurationReconciler_Reconcile(t *testing.T) { err = fakeClient.Get(ctx, types.NamespacedName{Name: cmName, Namespace: testNamespace}, cm) require.Nil(t, err) - cm.OwnerReferences = append(cm.OwnerReferences, v1alpha1.GetFfReference(ffConfig)) + cm.OwnerReferences = append(cm.OwnerReferences, ffConfig.GetReference()) err := r.Client.Update(ctx, cm) require.Nil(t, err) } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 7d5fdf61d..ae63d8220 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -1,5 +1,7 @@ package utils +import "fmt" + func TrueVal() *bool { b := true return &b @@ -18,3 +20,13 @@ func ContainsString(slice []string, s string) bool { } return false } + +// unique string used to create unique volume mount and file name +func FeatureFlagConfigurationId(namespace, name string) string { + return fmt.Sprintf("%s_%s", namespace, name) +} + +// unique key (and filename) for configMap data +func FeatureFlagConfigurationConfigMapKey(namespace, name string) string { + return fmt.Sprintf("%s.flagd.json", FeatureFlagConfigurationId(namespace, name)) +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go new file mode 100644 index 000000000..7e7b6d170 --- /dev/null +++ b/pkg/utils/utils_test.go @@ -0,0 +1,31 @@ +package utils + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_FeatureFlagConfigurationId(t *testing.T) { + require.Equal(t, "namespace_name", FeatureFlagConfigurationId("namespace", "name")) +} + +func Test_FeatureFlagConfigurationConfigMapKey(t *testing.T) { + require.Equal(t, "namespace_name.flagd.json", FeatureFlagConfigurationConfigMapKey("namespace", "name")) +} + +func Test_FalseVal(t *testing.T) { + f := false + require.Equal(t, &f, FalseVal()) +} + +func Test_TrueVal(t *testing.T) { + tt := true + require.Equal(t, &tt, TrueVal()) +} + +func Test_ContainsString(t *testing.T) { + slice := []string{"str1", "str2"} + require.True(t, ContainsString(slice, "str1")) + require.False(t, ContainsString(slice, "some")) +} diff --git a/webhooks/pod_webhook.go b/webhooks/pod_webhook.go index 044e1f32f..1a4942f68 100644 --- a/webhooks/pod_webhook.go +++ b/webhooks/pod_webhook.go @@ -319,7 +319,7 @@ func (m *PodMutator) handleFilepathProvider(ctx context.Context, pod *corev1.Pod }, }, }) - mountPath := fmt.Sprintf("%s/%s", rootFileSyncMountPath, v1alpha1.FeatureFlagConfigurationId(ns, n)) + mountPath := fmt.Sprintf("%s/%s", rootFileSyncMountPath, utils.FeatureFlagConfigurationId(ns, n)) sidecar.VolumeMounts = append(sidecar.VolumeMounts, corev1.VolumeMount{ Name: n, // create a directory mount per featureFlag spec @@ -331,7 +331,7 @@ func (m *PodMutator) handleFilepathProvider(ctx context.Context, pod *corev1.Pod "--uri", fmt.Sprintf("file:%s/%s", mountPath, - v1alpha1.FeatureFlagConfigurationConfigMapKey(ns, n), + utils.FeatureFlagConfigurationConfigMapKey(ns, n), ), ) return nil @@ -470,9 +470,9 @@ func (m *PodMutator) createConfigMap(ctx context.Context, namespace string, name if ff.Name == "" { return fmt.Errorf("feature flag configuration %s/%s not found", namespace, name) } - references = append(references, v1alpha1.GetFfReference(&ff)) + references = append(references, ff.GetReference()) - cm := v1alpha1.GenerateFfConfigMap(name, namespace, references, ff.Spec) + cm := ff.GenerateConfigMap(name, namespace, references) return m.Client.Create(ctx, &cm) }