fix: Operator envVar positioning & tls.SecretRef.Name (feast-dev#4806)

* fix envVar positioning Signed-off-by: Tommy Hughes <[email protected]> * tls.SecretRef.Name Signed-off-by: Tommy Hughes <[email protected]> --------- Signed-off-by: Tommy Hughes <[email protected]>
lokeshrangineni · Dec 5, 2024 · 14d4c05 · 14d4c05
1 parent babc60d
commit 14d4c05
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 32 deletions.
diff --git a/infra/feast-operator/internal/controller/services/repo_config_test.go b/infra/feast-operator/internal/controller/services/repo_config_test.go
@@ -23,6 +23,7 @@ import (
 	. "github.com/onsi/gomega"
 	"gopkg.in/yaml.v3"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1"
 )
@@ -468,6 +469,7 @@ func emptyRegistryConfig() RegistryConfig {
 
 func minimalFeatureStore() *feastdevv1alpha1.FeatureStore {
 	return &feastdevv1alpha1.FeatureStore{
+		ObjectMeta: metav1.ObjectMeta{Name: "test"},
 		Spec: feastdevv1alpha1.FeatureStoreSpec{
 			FeastProject: projectName,
 		},

diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go
@@ -659,7 +659,7 @@ func (feast *FeastServices) initPVC(feastType FeastServiceType) *corev1.Persiste
 
 func applyOptionalContainerConfigs(container *corev1.Container, optionalConfigs feastdevv1alpha1.OptionalConfigs) {
 	if optionalConfigs.Env != nil {
-		container.Env = mergeEnvVarsArrays(container.Env, optionalConfigs.Env)
+		container.Env = envOverride(container.Env, *optionalConfigs.Env)
 	}
 	if optionalConfigs.ImagePullPolicy != nil {
 		container.ImagePullPolicy = *optionalConfigs.ImagePullPolicy
@@ -669,28 +669,6 @@ func applyOptionalContainerConfigs(container *corev1.Container, optionalConfigs
 	}
 }
 
-func mergeEnvVarsArrays(envVars1 []corev1.EnvVar, envVars2 *[]corev1.EnvVar) []corev1.EnvVar {
-	merged := make(map[string]corev1.EnvVar)
-
-	// Add all env vars from the first array
-	for _, envVar := range envVars1 {
-		merged[envVar.Name] = envVar
-	}
-
-	// Add all env vars from the second array, overriding duplicates
-	for _, envVar := range *envVars2 {
-		merged[envVar.Name] = envVar
-	}
-
-	// Convert the map back to an array
-	result := make([]corev1.EnvVar, 0, len(merged))
-	for _, envVar := range merged {
-		result = append(result, envVar)
-	}
-
-	return result
-}
-
 func mountPvcConfig(podSpec *corev1.PodSpec, pvcConfig *feastdevv1alpha1.PvcConfig, deployName string) {
 	if podSpec != nil && pvcConfig != nil {
 		container := &podSpec.Containers[0]

diff --git a/infra/feast-operator/internal/controller/services/tls.go b/infra/feast-operator/internal/controller/services/tls.go
@@ -42,22 +42,28 @@ func (feast *FeastServices) setTlsDefaults() error {
 
 func (feast *FeastServices) setOpenshiftTls() error {
 	appliedServices := feast.Handler.FeatureStore.Status.Applied.Services
-	tlsConfigs := &feastdevv1alpha1.TlsConfigs{
-		SecretRef: &corev1.LocalObjectReference{},
-	}
 	if feast.offlineOpenshiftTls() {
 		appliedServices.OfflineStore.TLS = &feastdevv1alpha1.OfflineTlsConfigs{
-			TlsConfigs: *tlsConfigs,
+			TlsConfigs: feastdevv1alpha1.TlsConfigs{
+				SecretRef: &corev1.LocalObjectReference{
+					Name: feast.initFeastSvc(OfflineFeastType).Name + tlsNameSuffix,
+				},
+			},
 		}
-		appliedServices.OfflineStore.TLS.TlsConfigs.SecretRef.Name = feast.initFeastSvc(OfflineFeastType).Name + tlsNameSuffix
 	}
 	if feast.onlineOpenshiftTls() {
-		appliedServices.OnlineStore.TLS = tlsConfigs
-		appliedServices.OnlineStore.TLS.SecretRef.Name = feast.initFeastSvc(OnlineFeastType).Name + tlsNameSuffix
+		appliedServices.OnlineStore.TLS = &feastdevv1alpha1.TlsConfigs{
+			SecretRef: &corev1.LocalObjectReference{
+				Name: feast.initFeastSvc(OnlineFeastType).Name + tlsNameSuffix,
+			},
+		}
 	}
 	if feast.localRegistryOpenshiftTls() {
-		appliedServices.Registry.Local.TLS = tlsConfigs
-		appliedServices.Registry.Local.TLS.SecretRef.Name = feast.initFeastSvc(RegistryFeastType).Name + tlsNameSuffix
+		appliedServices.Registry.Local.TLS = &feastdevv1alpha1.TlsConfigs{
+			SecretRef: &corev1.LocalObjectReference{
+				Name: feast.initFeastSvc(RegistryFeastType).Name + tlsNameSuffix,
+			},
+		}
 	} else if remote, err := feast.remoteRegistryOpenshiftTls(); remote {
 		// if the remote registry reference is using openshift's service serving certificates, we can use the injected service CA bundle configMap
 		if appliedServices.Registry.Remote.TLS == nil {

diff --git a/infra/feast-operator/internal/controller/services/tls_test.go b/infra/feast-operator/internal/controller/services/tls_test.go
@@ -113,11 +113,17 @@ var _ = Describe("TLS Config", func() {
 			tls = feast.getTlsConfigs(OfflineFeastType)
 			Expect(tls).NotTo(BeNil())
 			Expect(tls.IsTLS()).To(BeTrue())
+			Expect(tls.SecretRef).NotTo(BeNil())
+			Expect(tls.SecretRef.Name).To(Equal("feast-test-offline-tls"))
 			tls = feast.getTlsConfigs(OnlineFeastType)
 			Expect(tls).NotTo(BeNil())
 			Expect(tls.IsTLS()).To(BeTrue())
+			Expect(tls.SecretRef).NotTo(BeNil())
+			Expect(tls.SecretRef.Name).To(Equal("feast-test-online-tls"))
 			tls = feast.getTlsConfigs(RegistryFeastType)
 			Expect(tls).NotTo(BeNil())
+			Expect(tls.SecretRef).NotTo(BeNil())
+			Expect(tls.SecretRef.Name).To(Equal("feast-test-registry-tls"))
 			Expect(tls.SecretKeyNames).To(Equal(secretKeyNames))
 			Expect(tls.IsTLS()).To(BeTrue())
 

diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go
@@ -315,3 +315,26 @@ func SetIsOpenShift(cfg *rest.Config) {
 func missingOidcSecretProperty(property OidcPropertyType) error {
 	return fmt.Errorf(OidcMissingSecretError, property)
 }
+
+// getEnvVar returns the position of the EnvVar found by name
+func getEnvVar(envName string, env []corev1.EnvVar) int {
+	for pos, v := range env {
+		if v.Name == envName {
+			return pos
+		}
+	}
+	return -1
+}
+
+// envOverride replaces or appends the provided EnvVar to the collection
+func envOverride(dst, src []corev1.EnvVar) []corev1.EnvVar {
+	for _, cre := range src {
+		pos := getEnvVar(cre.Name, dst)
+		if pos != -1 {
+			dst[pos] = cre
+		} else {
+			dst = append(dst, cre)
+		}
+	}
+	return dst
+}