Change GroupRef from bare string to typed MCPGroupRef struct (#4809)

* Change GroupRef from bare string to typed MCPGroupRef struct

GroupRef was a bare string while every other cross-CRD reference
(ExternalAuthConfigRef, ToolConfigRef, AuthServerRef, EmbeddingServerRef)
uses a typed struct. This inconsistency prevented extending GroupRef
with additional fields (like namespace) without a breaking change.

Define MCPGroupRef struct with Name field and nil-safe GetName() helper.
Replace GroupRef string with *MCPGroupRef on MCPServerSpec,
MCPRemoteProxySpec, MCPServerEntrySpec, and add a new top-level GroupRef
field on VirtualMCPServerSpec that takes precedence over the deprecated
config.groupRef string path.

Internal types (vmcpserver.Config, StatusResponse, BackendReconciler,
config.Config) remain as strings since they are not CRD API types.

This is a breaking wire-format change (groupRef: "name" becomes
groupRef: {name: "name"}) that must happen before v1beta1.

Closes #4634

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address review feedback on MCPGroupRef refactoring

- Add nil guard to MCPServerEntry validateGroupRef using GetName()
  instead of direct .Name access to prevent potential panic
- Add unit tests for MCPGroupRef.GetName() and
  VirtualMCPServer.ResolveGroupName() covering precedence logic
- Normalize nil-check pattern across all field index extractors to
  use GetName() == "" (handles both nil and empty name)
- Mark config.Config.Group as optional and deprecated since
  spec.groupRef is now the preferred path
- Improve Validate() error message to mention both spec.groupRef
  and config.groupRef paths

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update missed example and docs for MCPGroupRef wire format

- Fix vmcp_with_telemetry_ref.yaml (missed in initial pass)
- Update virtualmcpserver-api.md to document spec.groupRef as primary
  field and update all YAML examples to struct format
- Update virtualmcpcompositetooldefinition-guide.md and
  virtualmcpserver-observability.md examples
- Regenerate crd-api.md to show MCPGroupRef typed struct

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: ChrisJBurns

cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go

@@ -143,10 +143,10 @@ type MCPRemoteProxySpec struct {
 	// +optional
 	ResourceOverrides *ResourceOverrides `json:"resourceOverrides,omitempty"`
 
-	// GroupRef is the name of the MCPGroup this proxy belongs to
-	// Must reference an existing MCPGroup in the same namespace
+	// GroupRef references the MCPGroup this proxy belongs to.
+	// The referenced MCPGroup must be in the same namespace.
 	// +optional
-	GroupRef string `json:"groupRef,omitempty"`
+	GroupRef *MCPGroupRef `json:"groupRef,omitempty"`
 
 	// SessionAffinity controls whether the Service routes repeated client connections to the same pod.
 	// MCP protocols (SSE, streamable-http) are stateful, so ClientIP is the default.

cmd/thv-operator/api/v1alpha1/mcpserver_types.go

@@ -328,10 +328,10 @@ type MCPServerSpec struct {
 	// +optional
 	EndpointPrefix string `json:"endpointPrefix,omitempty"`
 
-	// GroupRef is the name of the MCPGroup this server belongs to
-	// Must reference an existing MCPGroup in the same namespace
+	// GroupRef references the MCPGroup this server belongs to.
+	// The referenced MCPGroup must be in the same namespace.
 	// +optional
-	GroupRef string `json:"groupRef,omitempty"`
+	GroupRef *MCPGroupRef `json:"groupRef,omitempty"`
 
 	// SessionAffinity controls whether the Service routes repeated client connections to the same pod.
 	// MCP protocols (SSE, streamable-http) are stateful, so ClientIP is the default.
@@ -911,6 +911,23 @@ type ToolConfigRef struct {
 	Name string `json:"name"`
 }
 
+// MCPGroupRef defines a reference to an MCPGroup resource.
+// The referenced MCPGroup must be in the same namespace.
+type MCPGroupRef struct {
+	// Name is the name of the MCPGroup resource in the same namespace
+	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:MinLength=1
+	Name string `json:"name"`
+}
+
+// GetName returns the name, or empty string if the receiver is nil.
+func (r *MCPGroupRef) GetName() string {
+	if r == nil {
+		return ""
+	}
+	return r.Name
+}
+
 // InlineAuthzConfig contains direct authorization configuration
 type InlineAuthzConfig struct {
 	// Policies is a list of Cedar policy strings

cmd/thv-operator/api/v1alpha1/mcpserverentry_types.go

@@ -24,11 +24,10 @@ type MCPServerEntrySpec struct {
 	// +kubebuilder:validation:Enum=sse;streamable-http
 	Transport string `json:"transport"`
 
-	// GroupRef is the name of the MCPGroup this entry belongs to.
+	// GroupRef references the MCPGroup this entry belongs to.
 	// Required — every MCPServerEntry must be part of a group for vMCP discovery.
 	// +kubebuilder:validation:Required
-	// +kubebuilder:validation:MinLength=1
-	GroupRef string `json:"groupRef"`
+	GroupRef *MCPGroupRef `json:"groupRef"`
 
 	// ExternalAuthConfigRef references a MCPExternalAuthConfig resource for token exchange
 	// when connecting to the remote MCP server. The referenced MCPExternalAuthConfig must
@@ -155,7 +154,7 @@ const (
 //+kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"
 //+kubebuilder:printcolumn:name="Transport",type="string",JSONPath=".spec.transport"
 //+kubebuilder:printcolumn:name="Remote URL",type="string",JSONPath=".spec.remoteUrl"
-//+kubebuilder:printcolumn:name="Group",type="string",JSONPath=".spec.groupRef"
+//+kubebuilder:printcolumn:name="Group",type="string",JSONPath=".spec.groupRef.name"
 //+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
 
 // MCPServerEntry is the Schema for the mcpserverentries API.

cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go

@@ -63,11 +63,15 @@ type VirtualMCPServerSpec struct {
 	// +kubebuilder:validation:Type=object
 	PodTemplateSpec *runtime.RawExtension `json:"podTemplateSpec,omitempty"`
 
-	// Config is the Virtual MCP server configuration
-	// The only field currently required within config is `config.groupRef`.
-	// GroupRef references an existing MCPGroup that defines backend workloads.
+	// GroupRef references the MCPGroup that defines backend workloads.
 	// The referenced MCPGroup must exist in the same namespace.
+	// This field takes precedence over config.groupRef and should be preferred.
+	// +optional
+	GroupRef *MCPGroupRef `json:"groupRef,omitempty"`
+
+	// Config is the Virtual MCP server configuration.
 	// The audit config from here is also supported, but not required.
+	// Note: config.groupRef is deprecated in favor of spec.groupRef.
 	// Note: config.telemetry is deprecated — use spec.telemetryConfigRef to reference
 	// a shared MCPTelemetryConfig resource instead.
 	// +optional
@@ -447,13 +451,22 @@ func (*VirtualMCPServer) GetProxyPort() int32 {
 	return 4483
 }
 
+// ResolveGroupName returns the effective group name, preferring spec.groupRef
+// over the deprecated config.groupRef string.
+func (r *VirtualMCPServer) ResolveGroupName() string {
+	if name := r.Spec.GroupRef.GetName(); name != "" {
+		return name
+	}
+	return r.Spec.Config.Group
+}
+
 // Validate performs validation for VirtualMCPServer
 // This method is called by the controller during reconciliation
 func (r *VirtualMCPServer) Validate() error {
-	// Validate Group is set (required field)
+	// Validate Group is set — prefer spec.groupRef, fall back to config.groupRef
 	// Note: CEL cannot validate embedded types from other packages
-	if r.Spec.Config.Group == "" {
-		return fmt.Errorf("spec.config.groupRef is required")
+	if r.Spec.GroupRef.GetName() == "" && r.Spec.Config.Group == "" {
+		return fmt.Errorf("either spec.groupRef.name or config.groupRef must be set")
 	}
 
 	// Note: IncomingAuth validation is handled by kubebuilder markers and CEL rules

cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go

@@ -535,3 +535,71 @@ func TestVirtualMCPServerSpecScalingFieldsJSONRoundtrip(t *testing.T) {
 		})
 	}
 }
+
+func TestMCPGroupRef_GetName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		ref  *MCPGroupRef
+		want string
+	}{
+		{name: "nil receiver", ref: nil, want: ""},
+		{name: "empty name", ref: &MCPGroupRef{Name: ""}, want: ""},
+		{name: "non-empty name", ref: &MCPGroupRef{Name: "my-group"}, want: "my-group"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tt.want, tt.ref.GetName())
+		})
+	}
+}
+
+func TestVirtualMCPServer_ResolveGroupName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		groupRef *MCPGroupRef
+		cfgGroup string
+		want     string
+	}{
+		{
+			name:     "spec.groupRef takes precedence over config.groupRef",
+			groupRef: &MCPGroupRef{Name: "from-spec"},
+			cfgGroup: "from-config",
+			want:     "from-spec",
+		},
+		{
+			name:     "falls back to config.groupRef when spec.groupRef is nil",
+			groupRef: nil,
+			cfgGroup: "from-config",
+			want:     "from-config",
+		},
+		{
+			name:     "only spec.groupRef set",
+			groupRef: &MCPGroupRef{Name: "from-spec"},
+			cfgGroup: "",
+			want:     "from-spec",
+		},
+		{
+			name:     "neither set returns empty",
+			groupRef: nil,
+			cfgGroup: "",
+			want:     "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			vmcp := &VirtualMCPServer{
+				Spec: VirtualMCPServerSpec{
+					GroupRef: tt.groupRef,
+					Config:   config.Config{Group: tt.cfgGroup},
+				},
+			}
+			assert.Equal(t, tt.want, vmcp.ResolveGroupName())
+		})
+	}
+}

cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

@@ -431,7 +431,8 @@ func (r *MCPGroupReconciler) findMCPGroupForMCPServer(ctx context.Context, obj c
 		ctxLogger.Error(nil, "Object is not an MCPServer", "object", obj.GetName())
 		return []ctrl.Request{}
 	}
-	if mcpServer.Spec.GroupRef == "" {
+	groupName := mcpServer.Spec.GroupRef.GetName()
+	if groupName == "" {
 		// No MCPGroup reference, nothing to do
 		return []ctrl.Request{}
 	}
@@ -444,10 +445,10 @@ func (r *MCPGroupReconciler) findMCPGroupForMCPServer(ctx context.Context, obj c
 		"mcpserver",
 		obj.GetName(),
 		"groupRef",
-		mcpServer.Spec.GroupRef)
+		groupName)
 	group := &mcpv1alpha1.MCPGroup{}
-	if err := r.Get(ctx, types.NamespacedName{Namespace: obj.GetNamespace(), Name: mcpServer.Spec.GroupRef}, group); err != nil {
-		ctxLogger.Error(err, "Failed to get MCPGroup for MCPServer", "namespace", obj.GetNamespace(), "name", mcpServer.Spec.GroupRef)
+	if err := r.Get(ctx, types.NamespacedName{Namespace: obj.GetNamespace(), Name: groupName}, group); err != nil {
+		ctxLogger.Error(err, "Failed to get MCPGroup for MCPServer", "namespace", obj.GetNamespace(), "name", groupName)
 		return []ctrl.Request{}
 	}
 	return []ctrl.Request{
@@ -469,7 +470,8 @@ func (r *MCPGroupReconciler) findMCPGroupForMCPRemoteProxy(ctx context.Context,
 		ctxLogger.Error(nil, "Object is not an MCPRemoteProxy", "object", obj.GetName())
 		return []ctrl.Request{}
 	}
-	if mcpRemoteProxy.Spec.GroupRef == "" {
+	groupName := mcpRemoteProxy.Spec.GroupRef.GetName()
+	if groupName == "" {
 		// No MCPGroup reference, nothing to do
 		return []ctrl.Request{}
 	}
@@ -482,12 +484,12 @@ func (r *MCPGroupReconciler) findMCPGroupForMCPRemoteProxy(ctx context.Context,
 		"mcpremoteproxy",
 		obj.GetName(),
 		"groupRef",
-		mcpRemoteProxy.Spec.GroupRef)
+		groupName)
 	group := &mcpv1alpha1.MCPGroup{}
-	groupKey := types.NamespacedName{Namespace: obj.GetNamespace(), Name: mcpRemoteProxy.Spec.GroupRef}
+	groupKey := types.NamespacedName{Namespace: obj.GetNamespace(), Name: groupName}
 	if err := r.Get(ctx, groupKey, group); err != nil {
 		ctxLogger.Error(err, "Failed to get MCPGroup for MCPRemoteProxy",
-			"namespace", obj.GetNamespace(), "name", mcpRemoteProxy.Spec.GroupRef)
+			"namespace", obj.GetNamespace(), "name", groupName)
 		return []ctrl.Request{}
 	}
 	return []ctrl.Request{
@@ -508,20 +510,21 @@ func (r *MCPGroupReconciler) findMCPGroupForMCPServerEntry(ctx context.Context,
 		ctxLogger.Error(nil, "Object is not an MCPServerEntry", "object", obj.GetName())
 		return []ctrl.Request{}
 	}
-	if mcpServerEntry.Spec.GroupRef == "" {
+	groupName := mcpServerEntry.Spec.GroupRef.GetName()
+	if groupName == "" {
 		return []ctrl.Request{}
 	}
 
 	ctxLogger.Info(
 		"Finding MCPGroup for MCPServerEntry",
 		"namespace", obj.GetNamespace(),
 		"mcpserverentry", obj.GetName(),
-		"groupRef", mcpServerEntry.Spec.GroupRef)
+		"groupRef", groupName)
 	group := &mcpv1alpha1.MCPGroup{}
-	groupKey := types.NamespacedName{Namespace: obj.GetNamespace(), Name: mcpServerEntry.Spec.GroupRef}
+	groupKey := types.NamespacedName{Namespace: obj.GetNamespace(), Name: groupName}
 	if err := r.Get(ctx, groupKey, group); err != nil {
 		ctxLogger.Error(err, "Failed to get MCPGroup for MCPServerEntry",
-			"namespace", obj.GetNamespace(), "name", mcpServerEntry.Spec.GroupRef)
+			"namespace", obj.GetNamespace(), "name", groupName)
 		return []ctrl.Request{}
 	}
 	return []ctrl.Request{