From b7e6814f50b59503de74710ec1a8d755f890e5ca Mon Sep 17 00:00:00 2001 From: Jiaming Hu Date: Sun, 10 Jan 2021 23:19:26 -0500 Subject: [PATCH 1/5] Bug: Enable SubscriptionConfig EnvFrom field Problem: EnvFrom is claimed supported in the document, but it isn't enabled from the code Solution: Wire the EnvFrom specified in the SubscriptionConfig into the deployments deployed when installing the operator. Signed-off-by: Jiaming Hu --- .../operators/olm/overrides/config.go | 3 +- .../operators/olm/overrides/initializer.go | 6 +- .../operators/olm/overrides/inject/inject.go | 44 ++++++ .../olm/overrides/inject/inject_test.go | 134 ++++++++++++++++++ 4 files changed, 185 insertions(+), 2 deletions(-) diff --git a/pkg/controller/operators/olm/overrides/config.go b/pkg/controller/operators/olm/overrides/config.go index 1db378302f..6727c8b837 100644 --- a/pkg/controller/operators/olm/overrides/config.go +++ b/pkg/controller/operators/olm/overrides/config.go @@ -16,7 +16,7 @@ type operatorConfig struct { logger *logrus.Logger } -func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOverrides []corev1.EnvVar, volumeOverrides []corev1.Volume, volumeMountOverrides []corev1.VolumeMount, tolerationOverrides []corev1.Toleration, resourcesOverride *corev1.ResourceRequirements, nodeSelectorOverride map[string]string, affinity *corev1.Affinity, annotations map[string]string, err error) { +func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOverrides []corev1.EnvVar, envFromOverrides []corev1.EnvFromSource, volumeOverrides []corev1.Volume, volumeMountOverrides []corev1.VolumeMount, tolerationOverrides []corev1.Toleration, resourcesOverride *corev1.ResourceRequirements, nodeSelectorOverride map[string]string, affinity *corev1.Affinity, annotations map[string]string, err error) { list, listErr := o.lister.OperatorsV1alpha1().SubscriptionLister().Subscriptions(ownerCSV.GetNamespace()).List(labels.Everything()) if listErr != nil { err = fmt.Errorf("failed to list subscription namespace=%s - %v", ownerCSV.GetNamespace(), listErr) @@ -35,6 +35,7 @@ func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOve } envVarOverrides = owner.Spec.Config.Env + envFromOverrides = owner.Spec.Config.EnvFrom volumeOverrides = owner.Spec.Config.Volumes volumeMountOverrides = owner.Spec.Config.VolumeMounts tolerationOverrides = owner.Spec.Config.Tolerations diff --git a/pkg/controller/operators/olm/overrides/initializer.go b/pkg/controller/operators/olm/overrides/initializer.go index 983c01709c..33f93f5164 100644 --- a/pkg/controller/operators/olm/overrides/initializer.go +++ b/pkg/controller/operators/olm/overrides/initializer.go @@ -45,7 +45,7 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment var envVarOverrides, proxyEnvVar, merged []corev1.EnvVar var err error - envVarOverrides, volumeOverrides, volumeMountOverrides, tolerationOverrides, resourcesOverride, nodeSelectorOverride, affinity, annotations, err := d.config.GetConfigOverrides(ownerCSV) + envVarOverrides, envFromOverrides, volumeOverrides, volumeMountOverrides, tolerationOverrides, resourcesOverride, nodeSelectorOverride, affinity, annotations, err := d.config.GetConfigOverrides(ownerCSV) if err != nil { err = fmt.Errorf("failed to get subscription pod configuration - %v", err) return err @@ -72,6 +72,10 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment return fmt.Errorf("failed to inject proxy env variable(s) into deployment spec name=%s - %v", deployment.Name, err) } + if err := inject.InjectEnvFromIntoDeployment(podSpec, envFromOverrides); err != nil { + return fmt.Errorf("failed to inject envFrom variable(s) into deployment spec name=%s - %v", deployment.Name, err) + } + if err = inject.InjectVolumesIntoDeployment(podSpec, volumeOverrides); err != nil { return fmt.Errorf("failed to inject volume(s) into deployment spec name=%s - %v", deployment.Name, err) } diff --git a/pkg/controller/operators/olm/overrides/inject/inject.go b/pkg/controller/operators/olm/overrides/inject/inject.go index 8f09eb353f..282e7a90c2 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject.go +++ b/pkg/controller/operators/olm/overrides/inject/inject.go @@ -62,6 +62,50 @@ func mergeEnvVars(containerEnvVars []corev1.EnvVar, newEnvVars []corev1.EnvVar) return merged } +// InjectEnvFromIntoDeployment injects the envFrom variables +// into the container(s) of the given PodSpec. +// +// If any Container in PodSpec already defines an envFrom variable +// as any of the provided envFrom then it will be overwritten. +func InjectEnvFromIntoDeployment(podSpec *corev1.PodSpec, envFromVars []corev1.EnvFromSource) error { + if podSpec == nil { + return errors.New("no pod spec provided") + } + + for i := range podSpec.Containers { + container := &podSpec.Containers[i] + container.EnvFrom = mergeEnvFromVars(container.EnvFrom, envFromVars) + } + + return nil +} + +func mergeEnvFromVars(containerEnvFromVars []corev1.EnvFromSource, newEnvFromVars []corev1.EnvFromSource) (merged []corev1.EnvFromSource) { + merged = containerEnvFromVars + + for _, newEnvFromVar := range newEnvFromVars { + found := findEnvFromVar(containerEnvFromVars, newEnvFromVar) + if found { + continue + } + + merged = append(merged, newEnvFromVar) + } + + return +} + +func findEnvFromVar(EnvFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) (found bool) { + for i := range EnvFromVar { + if newEnvFromVar == EnvFromVar[i] { + found = true + break + } + } + + return +} + // InjectVolumesIntoDeployment injects the provided Volumes // into the container(s) of the given PodSpec. // diff --git a/pkg/controller/operators/olm/overrides/inject/inject_test.go b/pkg/controller/operators/olm/overrides/inject/inject_test.go index fe86f3d458..435632db58 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject_test.go +++ b/pkg/controller/operators/olm/overrides/inject/inject_test.go @@ -30,6 +30,26 @@ var ( }, } + defaultEnvFromVars = []corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "test", + }, + corev1.EnvFromSource{ + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "configmapForTest", + }, + }, + }, + corev1.EnvFromSource{ + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secretForTest", + }, + }, + }, + } + defaultVolumeMounts = []corev1.VolumeMount{ { Name: "foo", @@ -526,6 +546,120 @@ func TestInjectEnvIntoDeployment(t *testing.T) { } } +func TestInjectEnvFromIntoDeployment(t *testing.T) { + tests := []struct { + name string + podSpec *corev1.PodSpec + envFromVar []corev1.EnvFromSource + expected *corev1.PodSpec + }{ + { + // PodSpec has one container and `EnvFrom` is empty. + // Expected: All env variable(s) specified are injected. + name: "WithContainerHasNoEnvFromVar", + podSpec: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{}, + }, + }, + envFromVar: defaultEnvFromVars, + expected: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + EnvFrom: defaultEnvFromVars, + }, + }, + }, + }, + + { + // PodSpec has one container and it has non overlapping envFrom var(s). + // Expected: existing non overlapping env vars are intact. + name: "WithContainerHasNonOverlappingEnvFromVar", + podSpec: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + EnvFrom: []corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "foo", + }, + }, + }, + }, + }, + envFromVar: defaultEnvFromVars, + expected: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + EnvFrom: append([]corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "foo", + }, + }, defaultEnvFromVars...), + }, + }, + }, + }, + { + // PodSpec has more than one container(s) + // Expected: All container(s) should be updated as expected. + name: "WithMultipleContainers", + podSpec: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{}, + corev1.Container{ + EnvFrom: []corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "foo", + }, + }, + }, + corev1.Container{ + EnvFrom: []corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "bar", + }, + }, + }, + }, + }, + envFromVar: defaultEnvFromVars, + expected: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + EnvFrom: defaultEnvFromVars, + }, + corev1.Container{ + EnvFrom: append([]corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "foo", + }, + }, defaultEnvFromVars...), + }, + corev1.Container{ + EnvFrom: append([]corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "bar", + }, + }, defaultEnvFromVars...), + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + inject.InjectEnvFromIntoDeployment(tt.podSpec, tt.envFromVar) + + podSpecWant := tt.expected + podSpecGot := tt.podSpec + + assert.Equal(t, podSpecWant, podSpecGot) + }) + } +} + func TestInjectTolerationsIntoDeployment(t *testing.T) { tests := []struct { name string From 8e74d7a54f1199340e484f7f4b411635db086319 Mon Sep 17 00:00:00 2001 From: Jiaming Hu Date: Sun, 21 Feb 2021 21:43:54 -0500 Subject: [PATCH 2/5] add config envFrom overlaps test case Signed-off-by: Jiaming Hu --- .../operators/olm/overrides/inject/inject.go | 35 +++++++++++- .../olm/overrides/inject/inject_test.go | 57 ++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/pkg/controller/operators/olm/overrides/inject/inject.go b/pkg/controller/operators/olm/overrides/inject/inject.go index 282e7a90c2..56594d4fdf 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject.go +++ b/pkg/controller/operators/olm/overrides/inject/inject.go @@ -97,7 +97,7 @@ func mergeEnvFromVars(containerEnvFromVars []corev1.EnvFromSource, newEnvFromVar func findEnvFromVar(EnvFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) (found bool) { for i := range EnvFromVar { - if newEnvFromVar == EnvFromVar[i] { + if compareEnvFromVar(EnvFromVar[i], newEnvFromVar) { found = true break } @@ -106,6 +106,39 @@ func findEnvFromVar(EnvFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvF return } +func compareEnvFromVar(envFromVar corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) (found bool) { + + compareprefix := newEnvFromVar.Prefix == envFromVar.Prefix + var compareConfigMap, compareSecret bool + + // Compare ConfigMapRef + if newEnvFromVar.ConfigMapRef == nil && envFromVar.ConfigMapRef == nil { + compareConfigMap = true + } else if newEnvFromVar.ConfigMapRef != nil && envFromVar.ConfigMapRef != nil { + if newEnvFromVar.ConfigMapRef.Optional == nil && envFromVar.ConfigMapRef.Optional == nil { + compareConfigMap = newEnvFromVar.ConfigMapRef.LocalObjectReference == envFromVar.ConfigMapRef.LocalObjectReference + } else if newEnvFromVar.ConfigMapRef.Optional != nil && envFromVar.ConfigMapRef.Optional != nil { + compareConfigMap = newEnvFromVar.ConfigMapRef.LocalObjectReference == envFromVar.ConfigMapRef.LocalObjectReference && *newEnvFromVar.ConfigMapRef.Optional == *envFromVar.ConfigMapRef.Optional + } else { + compareConfigMap = false + } + + } + // Compare SecretRef + if newEnvFromVar.SecretRef == nil && envFromVar.SecretRef == nil { + compareSecret = true + } else if newEnvFromVar.SecretRef != nil && envFromVar.SecretRef != nil { + if newEnvFromVar.SecretRef.Optional == nil && envFromVar.SecretRef.Optional == nil { + compareSecret = newEnvFromVar.SecretRef.LocalObjectReference == envFromVar.SecretRef.LocalObjectReference + } else if newEnvFromVar.SecretRef.Optional != nil && envFromVar.SecretRef.Optional != nil { + compareSecret = newEnvFromVar.SecretRef.LocalObjectReference == envFromVar.SecretRef.LocalObjectReference && *newEnvFromVar.SecretRef.Optional == *envFromVar.ConfigMapRef.Optional + } else { + compareSecret = false + } + } + return compareprefix && compareConfigMap && compareSecret +} + // InjectVolumesIntoDeployment injects the provided Volumes // into the container(s) of the given PodSpec. // diff --git a/pkg/controller/operators/olm/overrides/inject/inject_test.go b/pkg/controller/operators/olm/overrides/inject/inject_test.go index 435632db58..c2d6b695f9 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject_test.go +++ b/pkg/controller/operators/olm/overrides/inject/inject_test.go @@ -571,7 +571,62 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { }, }, }, - + { + // PodSpec has one container and it has overlapping envFrom var(s). + // Expected: existing duplicate env vars won't be appended in the envFrom. + name: "WithContainerHasOverlappingEnvFromVar", + podSpec: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + EnvFrom: []corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "test", + }, + corev1.EnvFromSource{ + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "configmapForTest", + }, + }, + }, + corev1.EnvFromSource{ + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secretForTest", + }, + }, + }, + }, + }, + }, + }, + envFromVar: defaultEnvFromVars, + expected: &corev1.PodSpec{ + Containers: []corev1.Container{ + corev1.Container{ + EnvFrom: append([]corev1.EnvFromSource{ + corev1.EnvFromSource{ + Prefix: "test", + }, + corev1.EnvFromSource{ + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "configmapForTest", + }, + }, + }, + corev1.EnvFromSource{ + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "secretForTest", + }, + }, + }, + }), + }, + }, + }, + }, { // PodSpec has one container and it has non overlapping envFrom var(s). // Expected: existing non overlapping env vars are intact. From 094b95fa982520908b86ba863a0da245285467c0 Mon Sep 17 00:00:00 2001 From: kevinrizza Date: Tue, 26 Mar 2024 15:00:26 -0400 Subject: [PATCH 3/5] Fix test changes for envfrom Signed-off-by: kevinrizza --- .../operators/olm/overrides/inject/inject.go | 2 - .../olm/overrides/inject/inject_test.go | 58 +++++++++---------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/pkg/controller/operators/olm/overrides/inject/inject.go b/pkg/controller/operators/olm/overrides/inject/inject.go index 56594d4fdf..9972fcf7fe 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject.go +++ b/pkg/controller/operators/olm/overrides/inject/inject.go @@ -107,7 +107,6 @@ func findEnvFromVar(EnvFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvF } func compareEnvFromVar(envFromVar corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) (found bool) { - compareprefix := newEnvFromVar.Prefix == envFromVar.Prefix var compareConfigMap, compareSecret bool @@ -122,7 +121,6 @@ func compareEnvFromVar(envFromVar corev1.EnvFromSource, newEnvFromVar corev1.Env } else { compareConfigMap = false } - } // Compare SecretRef if newEnvFromVar.SecretRef == nil && envFromVar.SecretRef == nil { diff --git a/pkg/controller/operators/olm/overrides/inject/inject_test.go b/pkg/controller/operators/olm/overrides/inject/inject_test.go index c2d6b695f9..52bf5088fb 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject_test.go +++ b/pkg/controller/operators/olm/overrides/inject/inject_test.go @@ -31,17 +31,17 @@ var ( } defaultEnvFromVars = []corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "test", }, - corev1.EnvFromSource{ + { ConfigMapRef: &corev1.ConfigMapEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "configmapForTest", }, }, }, - corev1.EnvFromSource{ + { SecretRef: &corev1.SecretEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "secretForTest", @@ -559,13 +559,13 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { name: "WithContainerHasNoEnvFromVar", podSpec: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{}, + {}, }, }, envFromVar: defaultEnvFromVars, expected: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { EnvFrom: defaultEnvFromVars, }, }, @@ -577,19 +577,19 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { name: "WithContainerHasOverlappingEnvFromVar", podSpec: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { EnvFrom: []corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "test", }, - corev1.EnvFromSource{ + { ConfigMapRef: &corev1.ConfigMapEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "configmapForTest", }, }, }, - corev1.EnvFromSource{ + { SecretRef: &corev1.SecretEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "secretForTest", @@ -603,26 +603,26 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { envFromVar: defaultEnvFromVars, expected: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ - EnvFrom: append([]corev1.EnvFromSource{ - corev1.EnvFromSource{ + { + EnvFrom: []corev1.EnvFromSource{ + { Prefix: "test", }, - corev1.EnvFromSource{ + { ConfigMapRef: &corev1.ConfigMapEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "configmapForTest", }, }, }, - corev1.EnvFromSource{ + { SecretRef: &corev1.SecretEnvSource{ LocalObjectReference: corev1.LocalObjectReference{ Name: "secretForTest", }, }, }, - }), + }, }, }, }, @@ -633,9 +633,9 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { name: "WithContainerHasNonOverlappingEnvFromVar", podSpec: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { EnvFrom: []corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "foo", }, }, @@ -645,9 +645,9 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { envFromVar: defaultEnvFromVars, expected: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { EnvFrom: append([]corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "foo", }, }, defaultEnvFromVars...), @@ -661,17 +661,17 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { name: "WithMultipleContainers", podSpec: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{}, - corev1.Container{ + {}, + { EnvFrom: []corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "foo", }, }, }, - corev1.Container{ + { EnvFrom: []corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "bar", }, }, @@ -681,19 +681,19 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { envFromVar: defaultEnvFromVars, expected: &corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { EnvFrom: defaultEnvFromVars, }, - corev1.Container{ + { EnvFrom: append([]corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "foo", }, }, defaultEnvFromVars...), }, - corev1.Container{ + { EnvFrom: append([]corev1.EnvFromSource{ - corev1.EnvFromSource{ + { Prefix: "bar", }, }, defaultEnvFromVars...), From b84732fbf274a92ad5857f2560033f0467c4f2a8 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Wed, 27 Mar 2024 10:38:55 -0400 Subject: [PATCH 4/5] Cleanup fromEnv code Signed-off-by: Todd Short --- .../operators/olm/overrides/inject/inject.go | 97 +++++++++++-------- .../olm/overrides/inject/inject_test.go | 6 +- 2 files changed, 57 insertions(+), 46 deletions(-) diff --git a/pkg/controller/operators/olm/overrides/inject/inject.go b/pkg/controller/operators/olm/overrides/inject/inject.go index 9972fcf7fe..8cae5743e8 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject.go +++ b/pkg/controller/operators/olm/overrides/inject/inject.go @@ -80,61 +80,76 @@ func InjectEnvFromIntoDeployment(podSpec *corev1.PodSpec, envFromVars []corev1.E return nil } -func mergeEnvFromVars(containerEnvFromVars []corev1.EnvFromSource, newEnvFromVars []corev1.EnvFromSource) (merged []corev1.EnvFromSource) { - merged = containerEnvFromVars +func mergeEnvFromVars(containerEnvFromVars []corev1.EnvFromSource, newEnvFromVars []corev1.EnvFromSource) []corev1.EnvFromSource { + merged := containerEnvFromVars for _, newEnvFromVar := range newEnvFromVars { - found := findEnvFromVar(containerEnvFromVars, newEnvFromVar) - if found { - continue + if !findEnvFromVar(containerEnvFromVars, newEnvFromVar) { + merged = append(merged, newEnvFromVar) } - - merged = append(merged, newEnvFromVar) } - return + return merged } -func findEnvFromVar(EnvFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) (found bool) { - for i := range EnvFromVar { - if compareEnvFromVar(EnvFromVar[i], newEnvFromVar) { - found = true - break +func findEnvFromVar(envFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) bool { + for i := range envFromVar { + if compareEnvFromVar(envFromVar[i], newEnvFromVar) { + return true } } + return false +} - return +func compareConfigMap(a, b *corev1.ConfigMapEnvSource) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + if a.LocalObjectReference != b.LocalObjectReference { + return false + } + if a.Optional == nil && b.Optional == nil { + return true + } + if a.Optional == nil || b.Optional == nil { + return false + } + return *a.Optional == *b.Optional } -func compareEnvFromVar(envFromVar corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) (found bool) { - compareprefix := newEnvFromVar.Prefix == envFromVar.Prefix - var compareConfigMap, compareSecret bool - - // Compare ConfigMapRef - if newEnvFromVar.ConfigMapRef == nil && envFromVar.ConfigMapRef == nil { - compareConfigMap = true - } else if newEnvFromVar.ConfigMapRef != nil && envFromVar.ConfigMapRef != nil { - if newEnvFromVar.ConfigMapRef.Optional == nil && envFromVar.ConfigMapRef.Optional == nil { - compareConfigMap = newEnvFromVar.ConfigMapRef.LocalObjectReference == envFromVar.ConfigMapRef.LocalObjectReference - } else if newEnvFromVar.ConfigMapRef.Optional != nil && envFromVar.ConfigMapRef.Optional != nil { - compareConfigMap = newEnvFromVar.ConfigMapRef.LocalObjectReference == envFromVar.ConfigMapRef.LocalObjectReference && *newEnvFromVar.ConfigMapRef.Optional == *envFromVar.ConfigMapRef.Optional - } else { - compareConfigMap = false - } +func compareSecretRef(a, b *corev1.SecretEnvSource) bool { + if a == nil && b == nil { + return true } - // Compare SecretRef - if newEnvFromVar.SecretRef == nil && envFromVar.SecretRef == nil { - compareSecret = true - } else if newEnvFromVar.SecretRef != nil && envFromVar.SecretRef != nil { - if newEnvFromVar.SecretRef.Optional == nil && envFromVar.SecretRef.Optional == nil { - compareSecret = newEnvFromVar.SecretRef.LocalObjectReference == envFromVar.SecretRef.LocalObjectReference - } else if newEnvFromVar.SecretRef.Optional != nil && envFromVar.SecretRef.Optional != nil { - compareSecret = newEnvFromVar.SecretRef.LocalObjectReference == envFromVar.SecretRef.LocalObjectReference && *newEnvFromVar.SecretRef.Optional == *envFromVar.ConfigMapRef.Optional - } else { - compareSecret = false - } + if a == nil || b == nil { + return false + } + if a.LocalObjectReference != b.LocalObjectReference { + return false + } + if a.Optional == nil && b.Optional == nil { + return true + } + if a.Optional == nil || b.Optional == nil { + return false + } + return *a.Optional == *b.Optional +} + +func compareEnvFromVar(envFromVar corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) bool { + if newEnvFromVar.Prefix != envFromVar.Prefix { + return false + } + if !compareConfigMap(envFromVar.ConfigMapRef, newEnvFromVar.ConfigMapRef) { + return false + } + if !compareSecretRef(envFromVar.SecretRef, newEnvFromVar.SecretRef) { + return false } - return compareprefix && compareConfigMap && compareSecret + return true } // InjectVolumesIntoDeployment injects the provided Volumes diff --git a/pkg/controller/operators/olm/overrides/inject/inject_test.go b/pkg/controller/operators/olm/overrides/inject/inject_test.go index 52bf5088fb..416214cba2 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject_test.go +++ b/pkg/controller/operators/olm/overrides/inject/inject_test.go @@ -706,11 +706,7 @@ func TestInjectEnvFromIntoDeployment(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { inject.InjectEnvFromIntoDeployment(tt.podSpec, tt.envFromVar) - - podSpecWant := tt.expected - podSpecGot := tt.podSpec - - assert.Equal(t, podSpecWant, podSpecGot) + assert.Equal(t, tt.expected, tt.podSpec) }) } } From a551799613a488b1407fe005d1059acb9cc2a980 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Wed, 27 Mar 2024 10:40:53 -0400 Subject: [PATCH 5/5] Use reflect to compare fromEnv Signed-off-by: Todd Short --- .../operators/olm/overrides/inject/inject.go | 53 +------------------ 1 file changed, 1 insertion(+), 52 deletions(-) diff --git a/pkg/controller/operators/olm/overrides/inject/inject.go b/pkg/controller/operators/olm/overrides/inject/inject.go index 8cae5743e8..e500f5623e 100644 --- a/pkg/controller/operators/olm/overrides/inject/inject.go +++ b/pkg/controller/operators/olm/overrides/inject/inject.go @@ -94,64 +94,13 @@ func mergeEnvFromVars(containerEnvFromVars []corev1.EnvFromSource, newEnvFromVar func findEnvFromVar(envFromVar []corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) bool { for i := range envFromVar { - if compareEnvFromVar(envFromVar[i], newEnvFromVar) { + if reflect.DeepEqual(envFromVar[i], newEnvFromVar) { return true } } return false } -func compareConfigMap(a, b *corev1.ConfigMapEnvSource) bool { - if a == nil && b == nil { - return true - } - if a == nil || b == nil { - return false - } - if a.LocalObjectReference != b.LocalObjectReference { - return false - } - if a.Optional == nil && b.Optional == nil { - return true - } - if a.Optional == nil || b.Optional == nil { - return false - } - return *a.Optional == *b.Optional -} - -func compareSecretRef(a, b *corev1.SecretEnvSource) bool { - if a == nil && b == nil { - return true - } - if a == nil || b == nil { - return false - } - if a.LocalObjectReference != b.LocalObjectReference { - return false - } - if a.Optional == nil && b.Optional == nil { - return true - } - if a.Optional == nil || b.Optional == nil { - return false - } - return *a.Optional == *b.Optional -} - -func compareEnvFromVar(envFromVar corev1.EnvFromSource, newEnvFromVar corev1.EnvFromSource) bool { - if newEnvFromVar.Prefix != envFromVar.Prefix { - return false - } - if !compareConfigMap(envFromVar.ConfigMapRef, newEnvFromVar.ConfigMapRef) { - return false - } - if !compareSecretRef(envFromVar.SecretRef, newEnvFromVar.SecretRef) { - return false - } - return true -} - // InjectVolumesIntoDeployment injects the provided Volumes // into the container(s) of the given PodSpec. //