fix: Remove verifyClient TLS offlineStore option from the Operator (f…

…east-dev#4847) remove verifyClient TLS option Signed-off-by: Tommy Hughes <tohughes@redhat.com>
nishantgaurav-dev · Jan 15, 2025 · 8a1cbfa · 8a1cbfa
1 parent 654c0af
commit 8a1cbfa
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 20 deletions.
diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go
@@ -96,14 +96,19 @@ type OfflineStore struct {
 =======
 	StoreServiceConfigs `json:",inline"`
 	Persistence         *OfflineStorePersistence `json:"persistence,omitempty"`
+<<<<<<< HEAD
 	TLS                 *OfflineTlsConfigs       `json:"tls,omitempty"`
 >>>>>>> 47204bcaf (feat: Add online/offline replica support (#4812))
+=======
+	TLS                 *TlsConfigs              `json:"tls,omitempty"`
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 	// LogLevel sets the logging level for the offline store service
 	// Allowed values: "debug", "info", "warning", "error", "critical".
 	// +kubebuilder:validation:Enum=debug;info;warning;error;critical
 	LogLevel string `json:"logLevel,omitempty"`
 }
 
+<<<<<<< HEAD
 // OfflineTlsConfigs configures server TLS for the offline feast service. in an openshift cluster, this is configured by default using service serving certificates.
 type OfflineTlsConfigs struct {
 	TlsConfigs `json:",inline"`
@@ -112,6 +117,8 @@ type OfflineTlsConfigs struct {
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
 }
 
+=======
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 // OfflineStorePersistence configures the persistence settings for the offline store service
 // +kubebuilder:validation:XValidation:rule="[has(self.file), has(self.store)].exists_one(c, c)",message="One selection required between file or store."
 type OfflineStorePersistence struct {

diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml
@@ -530,6 +530,7 @@ spec:
                             type: object
                         type: object
                       tls:
+<<<<<<< HEAD
 <<<<<<< HEAD
                         description: TlsConfigs configures server TLS for a feast
                           service. in an openshift cluster, this is configured by
@@ -539,6 +540,11 @@ spec:
                           offline feast service. in an openshift cluster, this is
                           configured by default using service serving certificates.
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+                        description: TlsConfigs configures server TLS for a feast
+                          service. in an openshift cluster, this is configured by
+                          default using service serving certificates.
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                         properties:
                           disable:
                             description: will disable TLS for the feast service. useful
@@ -569,11 +575,14 @@ spec:
                             type: object
                             x-kubernetes-map-type: atomic
 <<<<<<< HEAD
+<<<<<<< HEAD
 =======
                           verifyClient:
                             description: verify the client TLS certificate.
                             type: boolean
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                         type: object
                         x-kubernetes-validations:
                         - message: '`secretRef` required if `disable` is false.'
@@ -2123,6 +2132,7 @@ spec:
                                 type: object
                             type: object
                           tls:
+<<<<<<< HEAD
 <<<<<<< HEAD
                             description: TlsConfigs configures server TLS for a feast
                               service. in an openshift cluster, this is configured
@@ -2133,6 +2143,11 @@ spec:
                               this is configured by default using service serving
                               certificates.
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+                            description: TlsConfigs configures server TLS for a feast
+                              service. in an openshift cluster, this is configured
+                              by default using service serving certificates.
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                             properties:
                               disable:
                                 description: will disable TLS for the feast service.
@@ -2163,11 +2178,14 @@ spec:
                                 type: object
                                 x-kubernetes-map-type: atomic
 <<<<<<< HEAD
+<<<<<<< HEAD
 =======
                               verifyClient:
                                 description: verify the client TLS certificate.
                                 type: boolean
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                             type: object
                             x-kubernetes-validations:
                             - message: '`secretRef` required if `disable` is false.'

diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml
@@ -538,6 +538,7 @@ spec:
                             type: object
                         type: object
                       tls:
+<<<<<<< HEAD
 <<<<<<< HEAD
                         description: TlsConfigs configures server TLS for a feast
                           service. in an openshift cluster, this is configured by
@@ -547,6 +548,11 @@ spec:
                           offline feast service. in an openshift cluster, this is
                           configured by default using service serving certificates.
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+                        description: TlsConfigs configures server TLS for a feast
+                          service. in an openshift cluster, this is configured by
+                          default using service serving certificates.
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                         properties:
                           disable:
                             description: will disable TLS for the feast service. useful
@@ -577,11 +583,14 @@ spec:
                             type: object
                             x-kubernetes-map-type: atomic
 <<<<<<< HEAD
+<<<<<<< HEAD
 =======
                           verifyClient:
                             description: verify the client TLS certificate.
                             type: boolean
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                         type: object
                         x-kubernetes-validations:
                         - message: '`secretRef` required if `disable` is false.'
@@ -2131,6 +2140,7 @@ spec:
                                 type: object
                             type: object
                           tls:
+<<<<<<< HEAD
 <<<<<<< HEAD
                             description: TlsConfigs configures server TLS for a feast
                               service. in an openshift cluster, this is configured
@@ -2141,6 +2151,11 @@ spec:
                               this is configured by default using service serving
                               certificates.
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+                            description: TlsConfigs configures server TLS for a feast
+                              service. in an openshift cluster, this is configured
+                              by default using service serving certificates.
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                             properties:
                               disable:
                                 description: will disable TLS for the feast service.
@@ -2171,11 +2186,14 @@ spec:
                                 type: object
                                 x-kubernetes-map-type: atomic
 <<<<<<< HEAD
+<<<<<<< HEAD
 =======
                               verifyClient:
                                 description: verify the client TLS certificate.
                                 type: boolean
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
                             type: object
                             x-kubernetes-validations:
                             - message: '`secretRef` required if `disable` is false.'

diff --git a/infra/feast-operator/internal/controller/featurestore_controller_tls_test.go b/infra/feast-operator/internal/controller/featurestore_controller_tls_test.go
@@ -59,11 +59,15 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() {
 		}
 		featurestore := &feastdevv1alpha1.FeatureStore{}
 		localRef := corev1.LocalObjectReference{Name: "test"}
+<<<<<<< HEAD
 <<<<<<< HEAD
 		tlsConfigs := &feastdevv1alpha1.TlsConfigs{
 =======
 		tlsConfigs := feastdevv1alpha1.TlsConfigs{
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+		tlsConfigs := &feastdevv1alpha1.TlsConfigs{
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 			SecretRef: &localRef,
 		}
 		BeforeEach(func() {
@@ -79,6 +83,7 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() {
 						FeastProject: feastProject,
 						Services: &feastdevv1alpha1.FeatureStoreServices{
 							OnlineStore: &feastdevv1alpha1.OnlineStore{
+<<<<<<< HEAD
 <<<<<<< HEAD
 								TLS: tlsConfigs,
 							},
@@ -90,16 +95,21 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() {
 									TLS: tlsConfigs,
 =======
 								TLS: &tlsConfigs,
+=======
+								TLS: tlsConfigs,
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 							},
 							OfflineStore: &feastdevv1alpha1.OfflineStore{
-								TLS: &feastdevv1alpha1.OfflineTlsConfigs{
-									TlsConfigs: tlsConfigs,
-								},
+								TLS: tlsConfigs,
 							},
 							Registry: &feastdevv1alpha1.Registry{
 								Local: &feastdevv1alpha1.LocalRegistryConfig{
+<<<<<<< HEAD
 									TLS: &tlsConfigs,
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+									TLS: tlsConfigs,
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 								},
 							},
 						},
@@ -497,13 +507,17 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() {
 						},
 					},
 					OfflineStore: &feastdevv1alpha1.OfflineStore{
+<<<<<<< HEAD
 <<<<<<< HEAD
 						TLS: tlsConfigs,
 =======
 						TLS: &feastdevv1alpha1.OfflineTlsConfigs{
 							TlsConfigs: tlsConfigs,
 						},
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+						TLS: tlsConfigs,
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 					},
 					Registry: &feastdevv1alpha1.Registry{
 						Remote: &feastdevv1alpha1.RemoteRegistryConfig{

diff --git a/infra/feast-operator/internal/controller/services/repo_config.go b/infra/feast-operator/internal/controller/services/repo_config.go
@@ -469,6 +469,7 @@ func getClientRepoConfig(
 			Host: strings.Split(status.ServiceHostnames.OfflineStore, ":")[0],
 			Port: HttpPort,
 		}
+<<<<<<< HEAD
 <<<<<<< HEAD
 		if appliedServices.OfflineStore != nil && appliedServices.OfflineStore.TLS.IsTLS() {
 			clientRepoConfig.OfflineStore.Cert = GetTlsPath(OfflineFeastType) + appliedServices.OfflineStore.TLS.SecretKeyNames.TlsCrt
@@ -477,6 +478,10 @@ func getClientRepoConfig(
 			(&appliedServices.OfflineStore.TLS.TlsConfigs).IsTLS() {
 			clientRepoConfig.OfflineStore.Cert = GetTlsPath(OfflineFeastType) + appliedServices.OfflineStore.TLS.TlsConfigs.SecretKeyNames.TlsCrt
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+		if appliedServices.OfflineStore != nil && appliedServices.OfflineStore.TLS.IsTLS() {
+			clientRepoConfig.OfflineStore.Cert = GetTlsPath(OfflineFeastType) + appliedServices.OfflineStore.TLS.SecretKeyNames.TlsCrt
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 			clientRepoConfig.OfflineStore.Port = HttpsPort
 			clientRepoConfig.OfflineStore.Scheme = HttpsScheme
 		}

diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go
@@ -742,13 +742,6 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st
 	}
 	deploySettings.Args = append(deploySettings.Args, []string{"-p", strconv.Itoa(int(targetPort))}...)
 
-	if feastType == OfflineFeastType {
-		if tls.IsTLS() && feast.Handler.FeatureStore.Status.Applied.Services.OfflineStore.TLS.VerifyClient != nil {
-			deploySettings.Args = append(deploySettings.Args,
-				[]string{"--verify_client", strconv.FormatBool(*feast.Handler.FeatureStore.Status.Applied.Services.OfflineStore.TLS.VerifyClient)}...)
-		}
-	}
-
 	// Combine base command, options, and arguments
 	feastCommand := append([]string{baseCommand}, options...)
 	feastCommand = append(feastCommand, deploySettings.Args...)
@@ -1039,11 +1032,8 @@ func (feast *FeastServices) setServiceHostnames() error {
 	domain := svcDomain + ":"
 	if feast.isOfflinStore() {
 		objMeta := feast.GetObjectMeta(OfflineFeastType)
-		port := strconv.Itoa(HttpPort)
-		if feast.offlineTls() {
-			port = strconv.Itoa(HttpsPort)
-		}
-		feast.Handler.FeatureStore.Status.ServiceHostnames.OfflineStore = objMeta.Name + "." + objMeta.Namespace + domain + port
+		feast.Handler.FeatureStore.Status.ServiceHostnames.OfflineStore = objMeta.Name + "." + objMeta.Namespace + domain +
+			getPortStr(feast.Handler.FeatureStore.Status.Applied.Services.OfflineStore.TLS)
 	}
 	if feast.isOnlinStore() {
 		objMeta := feast.GetObjectMeta(OnlineFeastType)

diff --git a/infra/feast-operator/internal/controller/services/tls.go b/infra/feast-operator/internal/controller/services/tls.go
@@ -29,11 +29,15 @@ func (feast *FeastServices) setTlsDefaults() error {
 	}
 	appliedServices := feast.Handler.FeatureStore.Status.Applied.Services
 	if feast.isOfflinStore() && appliedServices.OfflineStore.TLS != nil {
+<<<<<<< HEAD
 <<<<<<< HEAD
 		tlsDefaults(appliedServices.OfflineStore.TLS)
 =======
 		tlsDefaults(&appliedServices.OfflineStore.TLS.TlsConfigs)
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+		tlsDefaults(appliedServices.OfflineStore.TLS)
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 	}
 	if feast.isOnlinStore() {
 		tlsDefaults(appliedServices.OnlineStore.TLS)
@@ -75,11 +79,9 @@ func (feast *FeastServices) setOpenshiftTls() error {
 =======
 >>>>>>> 33db9cabb (fix: Operator envVar positioning & tls.SecretRef.Name (#4806))
 	if feast.offlineOpenshiftTls() {
-		appliedServices.OfflineStore.TLS = &feastdevv1alpha1.OfflineTlsConfigs{
-			TlsConfigs: feastdevv1alpha1.TlsConfigs{
-				SecretRef: &corev1.LocalObjectReference{
-					Name: feast.initFeastSvc(OfflineFeastType).Name + tlsNameSuffix,
-				},
+		appliedServices.OfflineStore.TLS = &feastdevv1alpha1.TlsConfigs{
+			SecretRef: &corev1.LocalObjectReference{
+				Name: feast.initFeastSvc(OfflineFeastType).Name + tlsNameSuffix,
 			},
 		}
 	}
@@ -141,13 +143,18 @@ func (feast *FeastServices) getTlsConfigs(feastType FeastServiceType) (tls *feas
 	appliedServices := feast.Handler.FeatureStore.Status.Applied.Services
 	switch feastType {
 	case OfflineFeastType:
+<<<<<<< HEAD
 <<<<<<< HEAD
 		if feast.isOfflinStore() {
 			tls = appliedServices.OfflineStore.TLS
 =======
 		if feast.isOfflinStore() && appliedServices.OfflineStore.TLS != nil {
 			tls = &appliedServices.OfflineStore.TLS.TlsConfigs
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+		if feast.isOfflinStore() {
+			tls = appliedServices.OfflineStore.TLS
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 		}
 	case OnlineFeastType:
 		if feast.isOnlinStore() {
@@ -197,6 +204,7 @@ func (feast *FeastServices) remoteRegistryOpenshiftTls() (bool, error) {
 	return false, nil
 }
 
+<<<<<<< HEAD
 <<<<<<< HEAD
 =======
 func (feast *FeastServices) offlineTls() bool {
@@ -206,6 +214,8 @@ func (feast *FeastServices) offlineTls() bool {
 }
 
 >>>>>>> 668d47b8e (feat: Add TLS support to the Operator (#4796))
+=======
+>>>>>>> f36959cb2 (fix: Remove verifyClient TLS offlineStore option from the Operator (#4847))
 func (feast *FeastServices) localRegistryTls() bool {
 	return localRegistryTls(feast.Handler.FeatureStore)
 }