fix: Changes as per comments

Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
feast-dev · tchughesiv · Jun 13, 2025 · May 16, 2025 · May 21, 2025 · May 21, 2025
commit 68480ab68b021d1c6a42f90cbd7399c347bf15ea
@@ -503,10 +503,11 @@ type ServerConfigs struct {
 }
 
 // RegistryServerConfigs creates a registry server for the feast service, with specified container configurations.
+// +kubebuilder:validation:XValidation:rule="self.restAPI == true || self.grpc == true || !has(self.grpc)", message="At least one of restAPI or grpc must be true"
 type RegistryServerConfigs struct {
 	ServerConfigs `json:",inline"`
 
-	// Enable REST API registry server. Defaults to false if unset.
+	// Enable REST API registry server.
 	RestAPI *bool `json:"restAPI,omitempty"`
 
 	// Enable gRPC registry server. Defaults to true if unset.

@@ -1992,8 +1992,7 @@ spec:
                                     type: object
                                 type: object
                               restAPI:
-                                description: Enable REST API registry server. Defaults
-                                  to false if unset.
+                                description: Enable REST API registry server.
                                 type: boolean
                               tls:
                                 description: TlsConfigs configures server TLS for
@@ -2079,6 +2078,9 @@ spec:
                                   type: object
                                 type: array
                             type: object
+                            x-kubernetes-validations:
+                            - message: At least one of restAPI or grpc must be true
+                              rule: self.restAPI == true || self.grpc == true || !has(self.grpc)
                         type: object
                       remote:
                         description: RemoteRegistryConfig points to a remote feast
@@ -5979,7 +5981,6 @@ spec:
                                     type: object
                                   restAPI:
                                     description: Enable REST API registry server.
-                                      Defaults to false if unset.
                                     type: boolean
                                   tls:
                                     description: TlsConfigs configures server TLS
@@ -6069,6 +6070,11 @@ spec:
                                       type: object
                                     type: array
                                 type: object
+                                x-kubernetes-validations:
+                                - message: At least one of restAPI or grpc must be
+                                    true
+                                  rule: self.restAPI == true || self.grpc == true
+                                    || !has(self.grpc)
                             type: object
                           remote:
                             description: RemoteRegistryConfig points to a remote feast

@@ -2000,8 +2000,7 @@ spec:
                                     type: object
                                 type: object
                               restAPI:
-                                description: Enable REST API registry server. Defaults
-                                  to false if unset.
+                                description: Enable REST API registry server.
                                 type: boolean
                               tls:
                                 description: TlsConfigs configures server TLS for
@@ -2087,6 +2086,9 @@ spec:
                                   type: object
                                 type: array
                             type: object
+                            x-kubernetes-validations:
+                            - message: At least one of restAPI or grpc must be true
+                              rule: self.restAPI == true || self.grpc == true || !has(self.grpc)
                         type: object
                       remote:
                         description: RemoteRegistryConfig points to a remote feast
@@ -5987,7 +5989,6 @@ spec:
                                     type: object
                                   restAPI:
                                     description: Enable REST API registry server.
-                                      Defaults to false if unset.
                                     type: boolean
                                   tls:
                                     description: TlsConfigs configures server TLS
@@ -6077,6 +6078,11 @@ spec:
                                       type: object
                                     type: array
                                 type: object
+                                x-kubernetes-validations:
+                                - message: At least one of restAPI or grpc must be
+                                    true
+                                  rule: self.restAPI == true || self.grpc == true
+                                    || !has(self.grpc)
                             type: object
                           remote:
                             description: RemoteRegistryConfig points to a remote feast

@@ -695,7 +695,7 @@ Allowed values: "debug", "info", "warning", "error", "critical". |
 This allows attaching persistent storage, config files, secrets, or other resources
 required by the Feast components. Ensure that each volume mount has a corresponding
 volume definition in the Volumes field. |
-| `restAPI` _boolean_ | Enable REST API registry server. Defaults to false if unset. |
+| `restAPI` _boolean_ | Enable REST API registry server. |
 | `grpc` _boolean_ | Enable gRPC registry server. Defaults to true if unset. |
 
 

@@ -1265,7 +1265,6 @@ var _ = Describe("FeatureStore Controller", func() {
 					Name:      name,
 					Namespace: "default",
 				}
-
 				resource := &feastdevv1alpha1.FeatureStore{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      name,
@@ -1320,7 +1319,40 @@ var _ = Describe("FeatureStore Controller", func() {
 						To(ContainElement(expectedArg),
 							"expected %s to be present in container command: %v", expectedArg, registryContainer.Command)
 				}
+				Expect(resource.Status.Conditions).NotTo(BeEmpty())
+				cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType)
+				Expect(cond).ToNot(BeNil())
+				Expect(cond.Status).To(Equal(metav1.ConditionTrue))
+				Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason))
+				Expect(cond.Type).To(Equal(feastdevv1alpha1.RegistryReadyType))
+				Expect(cond.Message).To(Equal(feastdevv1alpha1.RegistryReadyMessage))
+			}
+
+			By("Verifying that creation fails when both REST API and gRPC are disabled")
+			disabledResource := &feastdevv1alpha1.FeatureStore{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "disabled-both",
+					Namespace: "default",
+				},
+				Spec: feastdevv1alpha1.FeatureStoreSpec{
+					FeastProject: feastProject,
+					Services: &feastdevv1alpha1.FeatureStoreServices{
+						Registry: &feastdevv1alpha1.Registry{
+							Local: &feastdevv1alpha1.LocalRegistryConfig{
+								Server: &feastdevv1alpha1.RegistryServerConfigs{
+									RestAPI: ptr(false),
+									GRPC:    ptr(false),
+								},
+							},
+						},
+					},
+				},
 			}
+			disabledResource.SetGroupVersionKind(feastdevv1alpha1.GroupVersion.WithKind("FeatureStore"))
+
+			err := k8sClient.Create(ctx, disabledResource)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("At least one of restAPI or grpc must be true"))
 		})
 
 		It("should error on reconcile", func() {

@@ -445,25 +445,29 @@ func (feast *FeastServices) setContainer(containers *[]corev1.Container, feastTy
 		probeHandler := feast.getProbeHandler(feastType, tls)
 		container.Ports = []corev1.ContainerPort{}
 
-		isRegistry := feastType == RegistryFeastType && feast.isRegistryServer()
-
-		grpcEnabled := !feast.isRegistryServer() || feast.isRegistryGrpcEnabled()
-		if grpcEnabled {
+		if feastType == RegistryFeastType {
+			if feast.isRegistryGrpcEnabled() {
+				container.Ports = append(container.Ports, corev1.ContainerPort{
+					Name:          name,
+					ContainerPort: getTargetPort(feastType, tls),
+					Protocol:      corev1.ProtocolTCP,
+				})
+			}
+			if feast.isRegistryRestEnabled() {
+				container.Ports = append(container.Ports, corev1.ContainerPort{
+					Name:          name + "-rest",
+					ContainerPort: getTargetRestPort(feastType, tls),
+					Protocol:      corev1.ProtocolTCP,
+				})
+			}
+		} else {
 			container.Ports = append(container.Ports, corev1.ContainerPort{
 				Name:          name,
 				ContainerPort: getTargetPort(feastType, tls),
 				Protocol:      corev1.ProtocolTCP,
 			})
 		}
 
-		if isRegistry && feast.isRegistryRestEnabled() {
-			container.Ports = append(container.Ports, corev1.ContainerPort{
-				Name:          name + "-rest",
-				ContainerPort: getTargetRestPort(feastType, tls),
-				Protocol:      corev1.ProtocolTCP,
-			})
-		}
-
 		container.StartupProbe = &corev1.Probe{
 			ProbeHandler:     probeHandler,
 			PeriodSeconds:    3,
@@ -1120,9 +1124,18 @@ func getTargetRestPort(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConf
 }
 
 func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) corev1.ProbeHandler {
+	targetPort := getTargetPort(feastType, tls)
+
 	if feastType == RegistryFeastType {
+		if feast.isRegistryGrpcEnabled() {
+			return corev1.ProbeHandler{
+				TCPSocket: &corev1.TCPSocketAction{
+					Port: intstr.FromInt(int(targetPort)),
+				},
+			}
+		}
 		if feast.isRegistryRestEnabled() {
-			targetPort := getTargetRestPort(feastType, tls)
+			targetPort = getTargetRestPort(feastType, tls)
 			probeHandler := corev1.ProbeHandler{
 				HTTPGet: &corev1.HTTPGetAction{
 					Port: intstr.FromInt(int(targetPort)),
@@ -1133,16 +1146,8 @@ func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *fea
 			}
 			return probeHandler
 		}
-		if feast.isRegistryGrpcEnabled() {
-			targetPort := getTargetPort(feastType, tls)
-			return corev1.ProbeHandler{
-				TCPSocket: &corev1.TCPSocketAction{
-					Port: intstr.FromInt(int(targetPort)),
-				},
-			}
-		}
-	} else if feastType == OnlineFeastType {
-		targetPort := getTargetPort(feastType, tls)
+	}
+	if feastType == OnlineFeastType {
 		probeHandler := corev1.ProbeHandler{
 			HTTPGet: &corev1.HTTPGetAction{
 				Path: "/health",
@@ -1154,7 +1159,6 @@ func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *fea
 		}
 		return probeHandler
 	}
-	targetPort := getTargetPort(feastType, tls)
 	return corev1.ProbeHandler{
 		TCPSocket: &corev1.TCPSocketAction{
 			Port: intstr.FromInt(int(targetPort)),
@@ -1179,12 +1183,18 @@ func (feast *FeastServices) GetFeastRestServiceName(feastType FeastServiceType)
 
 // isRegistryGrpcEnabled checks if gRPC is enabled for registry service
 func (feast *FeastServices) isRegistryGrpcEnabled() bool {
-	registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry
-	return registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC
+	if feast.isRegistryServer() {
+		registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry
+		return registry.Local.Server.GRPC != nil && *registry.Local.Server.GRPC
+	}
+	return false
 }
 
 // isRegistryRestEnabled checks if REST API is enabled for registry service
 func (feast *FeastServices) isRegistryRestEnabled() bool {
-	registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry
-	return registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI
+	if feast.isRegistryServer() {
+		registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry
+		return registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI
+	}
+	return false
 }