fix: Remove verifyClient TLS offlineStore option from the Operator (#4847)

tchughesiv · web-flow · commit 79fa247026dd · 2024-12-16T14:46:52.000-05:00
remove verifyClient TLS option

Signed-off-by: Tommy Hughes &lt;tohughes@redhat.com&gt;
diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go
@@ -76,20 +76,13 @@ type FeatureStoreServices struct {
 type OfflineStore struct {
 	StoreServiceConfigs `json:",inline"`
 	Persistence         *OfflineStorePersistence `json:"persistence,omitempty"`
-	TLS                 *OfflineTlsConfigs       `json:"tls,omitempty"`
+	TLS                 *TlsConfigs              `json:"tls,omitempty"`
 	// 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"`
 }
 
-// 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"`
-	// verify the client TLS certificate.
-	VerifyClient *bool `json:"verifyClient,omitempty"`
-}
-
 // 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
@@ -432,9 +432,9 @@ spec:
                             type: object
                         type: object
                       tls:
-                        description: OfflineTlsConfigs configures server TLS for the
-                          offline feast service. in an openshift cluster, this is
-                          configured by default using service serving certificates.
+                        description: TlsConfigs configures server TLS for a feast
+                          service. in an openshift cluster, this is configured by
+                          default using service serving certificates.
                         properties:
                           disable:
                             description: will disable TLS for the feast service. useful
@@ -464,9 +464,6 @@ spec:
                                 type: string
                             type: object
                             x-kubernetes-map-type: atomic
-                          verifyClient:
-                            description: verify the client TLS certificate.
-                            type: boolean
                         type: object
                         x-kubernetes-validations:
                         - message: '`secretRef` required if `disable` is false.'
@@ -1690,10 +1687,9 @@ spec:
                                 type: object
                             type: object
                           tls:
-                            description: OfflineTlsConfigs configures server TLS for
-                              the offline feast service. in an openshift cluster,
-                              this is configured by default using service serving
-                              certificates.
+                            description: TlsConfigs configures server TLS for a feast
+                              service. in an openshift cluster, this is configured
+                              by default using service serving certificates.
                             properties:
                               disable:
                                 description: will disable TLS for the feast service.
@@ -1723,9 +1719,6 @@ spec:
                                     type: string
                                 type: object
                                 x-kubernetes-map-type: atomic
-                              verifyClient:
-                                description: verify the client TLS certificate.
-                                type: boolean
                             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
@@ -440,9 +440,9 @@ spec:
                             type: object
                         type: object
                       tls:
-                        description: OfflineTlsConfigs configures server TLS for the
-                          offline feast service. in an openshift cluster, this is
-                          configured by default using service serving certificates.
+                        description: TlsConfigs configures server TLS for a feast
+                          service. in an openshift cluster, this is configured by
+                          default using service serving certificates.
                         properties:
                           disable:
                             description: will disable TLS for the feast service. useful
@@ -472,9 +472,6 @@ spec:
                                 type: string
                             type: object
                             x-kubernetes-map-type: atomic
-                          verifyClient:
-                            description: verify the client TLS certificate.
-                            type: boolean
                         type: object
                         x-kubernetes-validations:
                         - message: '`secretRef` required if `disable` is false.'
@@ -1698,10 +1695,9 @@ spec:
                                 type: object
                             type: object
                           tls:
-                            description: OfflineTlsConfigs configures server TLS for
-                              the offline feast service. in an openshift cluster,
-                              this is configured by default using service serving
-                              certificates.
+                            description: TlsConfigs configures server TLS for a feast
+                              service. in an openshift cluster, this is configured
+                              by default using service serving certificates.
                             properties:
                               disable:
                                 description: will disable TLS for the feast service.
@@ -1731,9 +1727,6 @@ spec:
                                     type: string
                                 type: object
                                 x-kubernetes-map-type: atomic
-                              verifyClient:
-                                description: verify the client TLS certificate.
-                                type: boolean
                             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
@@ -56,7 +56,7 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() {
 		}
 		featurestore := &feastdevv1alpha1.FeatureStore{}
 		localRef := corev1.LocalObjectReference{Name: "test"}
-		tlsConfigs := feastdevv1alpha1.TlsConfigs{
+		tlsConfigs := &feastdevv1alpha1.TlsConfigs{
 			SecretRef: &localRef,
 		}
 		BeforeEach(func() {
@@ -72,16 +72,14 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() {
 						FeastProject: feastProject,
 						Services: &feastdevv1alpha1.FeatureStoreServices{
 							OnlineStore: &feastdevv1alpha1.OnlineStore{
-								TLS: &tlsConfigs,
+								TLS: tlsConfigs,
 							},
 							OfflineStore: &feastdevv1alpha1.OfflineStore{
-								TLS: &feastdevv1alpha1.OfflineTlsConfigs{
-									TlsConfigs: tlsConfigs,
-								},
+								TLS: tlsConfigs,
 							},
 							Registry: &feastdevv1alpha1.Registry{
 								Local: &feastdevv1alpha1.LocalRegistryConfig{
-									TLS: &tlsConfigs,
+									TLS: tlsConfigs,
 								},
 							},
 						},
@@ -396,9 +394,7 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() {
 						},
 					},
 					OfflineStore: &feastdevv1alpha1.OfflineStore{
-						TLS: &feastdevv1alpha1.OfflineTlsConfigs{
-							TlsConfigs: tlsConfigs,
-						},
+						TLS: tlsConfigs,
 					},
 					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
@@ -248,9 +248,8 @@ func getClientRepoConfig(
 			Host: strings.Split(status.ServiceHostnames.OfflineStore, ":")[0],
 			Port: HttpPort,
 		}
-		if appliedServices.OfflineStore != nil && appliedServices.OfflineStore.TLS != nil &&
-			(&appliedServices.OfflineStore.TLS.TlsConfigs).IsTLS() {
-			clientRepoConfig.OfflineStore.Cert = GetTlsPath(OfflineFeastType) + appliedServices.OfflineStore.TLS.TlsConfigs.SecretKeyNames.TlsCrt
+		if appliedServices.OfflineStore != nil && appliedServices.OfflineStore.TLS.IsTLS() {
+			clientRepoConfig.OfflineStore.Cert = GetTlsPath(OfflineFeastType) + appliedServices.OfflineStore.TLS.SecretKeyNames.TlsCrt
 			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
@@ -373,13 +373,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...)
@@ -549,11 +542,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,7 +29,7 @@ func (feast *FeastServices) setTlsDefaults() error {
 	}
 	appliedServices := feast.Handler.FeatureStore.Status.Applied.Services
 	if feast.isOfflinStore() && appliedServices.OfflineStore.TLS != nil {
-		tlsDefaults(&appliedServices.OfflineStore.TLS.TlsConfigs)
+		tlsDefaults(appliedServices.OfflineStore.TLS)
 	}
 	if feast.isOnlinStore() {
 		tlsDefaults(appliedServices.OnlineStore.TLS)
@@ -43,11 +43,9 @@ func (feast *FeastServices) setTlsDefaults() error {
 func (feast *FeastServices) setOpenshiftTls() error {
 	appliedServices := feast.Handler.FeatureStore.Status.Applied.Services
 	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,
 			},
 		}
 	}
@@ -103,8 +101,8 @@ func (feast *FeastServices) getTlsConfigs(feastType FeastServiceType) (tls *feas
 	appliedServices := feast.Handler.FeatureStore.Status.Applied.Services
 	switch feastType {
 	case OfflineFeastType:
-		if feast.isOfflinStore() && appliedServices.OfflineStore.TLS != nil {
-			tls = &appliedServices.OfflineStore.TLS.TlsConfigs
+		if feast.isOfflinStore() {
+			tls = appliedServices.OfflineStore.TLS
 		}
 	case OnlineFeastType:
 		if feast.isOnlinStore() {
@@ -154,12 +152,6 @@ func (feast *FeastServices) remoteRegistryOpenshiftTls() (bool, error) {
 	return false, nil
 }
 
-func (feast *FeastServices) offlineTls() bool {
-	return feast.isOfflinStore() &&
-		feast.Handler.FeatureStore.Status.Applied.Services.OfflineStore.TLS != nil &&
-		(&feast.Handler.FeatureStore.Status.Applied.Services.OfflineStore.TLS.TlsConfigs).IsTLS()
-}
-
 func (feast *FeastServices) localRegistryTls() bool {
 	return localRegistryTls(feast.Handler.FeatureStore)
 }
diff --git a/infra/feast-operator/internal/controller/services/tls_test.go b/infra/feast-operator/internal/controller/services/tls_test.go
@@ -58,7 +58,6 @@ var _ = Describe("TLS Config", func() {
 			Expect(tls.IsTLS()).To(BeFalse())
 			Expect(getPortStr(tls)).To(Equal("80"))
 
-			Expect(feast.offlineTls()).To(BeFalse())
 			Expect(feast.remoteRegistryTls()).To(BeFalse())
 			Expect(feast.localRegistryTls()).To(BeFalse())
 			Expect(feast.isOpenShiftTls(OfflineFeastType)).To(BeFalse())
@@ -87,7 +86,6 @@ var _ = Describe("TLS Config", func() {
 			Expect(getPortStr(tls)).To(Equal("443"))
 			Expect(GetTlsPath(RegistryFeastType)).To(Equal("/tls/registry/"))
 
-			Expect(feast.offlineTls()).To(BeFalse())
 			Expect(feast.remoteRegistryTls()).To(BeFalse())
 			Expect(feast.localRegistryTls()).To(BeTrue())
 			Expect(feast.isOpenShiftTls(OfflineFeastType)).To(BeFalse())
@@ -127,7 +125,6 @@ var _ = Describe("TLS Config", func() {
 			Expect(tls.SecretKeyNames).To(Equal(secretKeyNames))
 			Expect(tls.IsTLS()).To(BeTrue())
 
-			Expect(feast.offlineTls()).To(BeTrue())
 			Expect(feast.remoteRegistryTls()).To(BeFalse())
 			Expect(feast.localRegistryTls()).To(BeTrue())
 			Expect(feast.isOpenShiftTls(OfflineFeastType)).To(BeTrue())
@@ -189,7 +186,6 @@ var _ = Describe("TLS Config", func() {
 			Expect(getPortStr(tls)).To(Equal("443"))
 			Expect(GetTlsPath(RegistryFeastType)).To(Equal("/tls/registry/"))
 
-			Expect(feast.offlineTls()).To(BeFalse())
 			Expect(feast.remoteRegistryTls()).To(BeFalse())
 			Expect(feast.localRegistryTls()).To(BeTrue())
 			Expect(feast.isOpenShiftTls(OfflineFeastType)).To(BeFalse())
@@ -238,7 +234,6 @@ var _ = Describe("TLS Config", func() {
 			Expect(getPortStr(tls)).To(Equal("80"))
 			Expect(GetTlsPath(RegistryFeastType)).To(Equal("/tls/registry/"))
 
-			Expect(feast.offlineTls()).To(BeTrue())
 			Expect(feast.remoteRegistryTls()).To(BeFalse())
 			Expect(feast.localRegistryTls()).To(BeFalse())
 			Expect(feast.isOpenShiftTls(OfflineFeastType)).To(BeTrue())

Original file line number	Diff line number	Diff line change
`@@ -248,9 +248,8 @@ func getClientRepoConfig(`
`248`	`248`	`Host: strings.Split(status.ServiceHostnames.OfflineStore, ":")[0],`
`249`	`249`	`Port: HttpPort,`
`250`	`250`	`}`
`251`		`- if appliedServices.OfflineStore != nil && appliedServices.OfflineStore.TLS != nil &&`
`252`		`- (&appliedServices.OfflineStore.TLS.TlsConfigs).IsTLS() {`
`253`		`- clientRepoConfig.OfflineStore.Cert = GetTlsPath(OfflineFeastType) + appliedServices.OfflineStore.TLS.TlsConfigs.SecretKeyNames.TlsCrt`
	`251`	`+ if appliedServices.OfflineStore != nil && appliedServices.OfflineStore.TLS.IsTLS() {`
	`252`	`+ clientRepoConfig.OfflineStore.Cert = GetTlsPath(OfflineFeastType) + appliedServices.OfflineStore.TLS.SecretKeyNames.TlsCrt`
`254`	`253`	`clientRepoConfig.OfflineStore.Port = HttpsPort`
`255`	`254`	`clientRepoConfig.OfflineStore.Scheme = HttpsScheme`
`256`	`255`	`}`