diff --git a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go index 11cdc20513..10472b91f1 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go @@ -208,6 +208,9 @@ const ( // ConditionTypeConfigurationValid indicates whether the proxy spec has passed all pre-deployment validation checks ConditionTypeConfigurationValid = "ConfigurationValid" + + // ConditionTypeMCPRemoteProxyCABundleRefValidated indicates whether the CABundleRef is valid + ConditionTypeMCPRemoteProxyCABundleRefValidated = "CABundleRefValidated" ) // Condition reasons for MCPRemoteProxy diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go index a098dd02d0..6eb5618b5b 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go @@ -116,6 +116,9 @@ func (r *MCPRemoteProxyReconciler) validateAndHandleConfigs(ctx context.Context, // Validate the GroupRef if specified r.validateGroupRef(ctx, proxy) + // Validate the CABundleRef if specified + r.validateCABundleRef(ctx, proxy) + // Handle MCPToolConfig if err := r.handleToolConfig(ctx, proxy); err != nil { ctxLogger.Error(err, "Failed to handle MCPToolConfig") @@ -720,6 +723,72 @@ func (r *MCPRemoteProxyReconciler) validateGroupRef(ctx context.Context, proxy * } } +// validateCABundleRef validates the CABundleRef ConfigMap reference if specified +func (r *MCPRemoteProxyReconciler) validateCABundleRef(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) { + caBundleRef := getCABundleRef(&proxy.Spec.OIDCConfig) + if caBundleRef == nil || caBundleRef.ConfigMapRef == nil { + return + } + + ctxLogger := log.FromContext(ctx) + + // Validate the CABundleRef configuration + if err := validation.ValidateCABundleSource(caBundleRef); err != nil { + ctxLogger.Error(err, "Invalid CABundleRef configuration") + setCABundleRefConditionForProxy(proxy, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonCABundleRefInvalid, err.Error()) + r.updateCABundleStatusForProxy(ctx, proxy) + return + } + + // Check if the referenced ConfigMap exists + cmName := caBundleRef.ConfigMapRef.Name + configMap := &corev1.ConfigMap{} + if err := r.Get(ctx, types.NamespacedName{Namespace: proxy.Namespace, Name: cmName}, configMap); err != nil { + ctxLogger.Error(err, "Failed to find CA bundle ConfigMap", "configMap", cmName) + setCABundleRefConditionForProxy(proxy, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonCABundleRefNotFound, + fmt.Sprintf("CA bundle ConfigMap '%s' not found in namespace '%s'", cmName, proxy.Namespace)) + r.updateCABundleStatusForProxy(ctx, proxy) + return + } + + // Verify the key exists in the ConfigMap + key := caBundleRef.ConfigMapRef.Key + if key == "" { + key = validation.OIDCCABundleDefaultKey + } + if _, exists := configMap.Data[key]; !exists { + ctxLogger.Error(nil, "CA bundle key not found in ConfigMap", "configMap", cmName, "key", key) + setCABundleRefConditionForProxy(proxy, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonCABundleRefInvalid, + fmt.Sprintf("Key '%s' not found in ConfigMap '%s'", key, cmName)) + r.updateCABundleStatusForProxy(ctx, proxy) + return + } + + // Validation passed + setCABundleRefConditionForProxy(proxy, metav1.ConditionTrue, mcpv1alpha1.ConditionReasonCABundleRefValid, + fmt.Sprintf("CA bundle ConfigMap '%s' is valid (key: %s)", cmName, key)) + r.updateCABundleStatusForProxy(ctx, proxy) +} + +// updateCABundleStatusForProxy updates the MCPRemoteProxy status after CA bundle validation +func (r *MCPRemoteProxyReconciler) updateCABundleStatusForProxy(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) { + ctxLogger := log.FromContext(ctx) + if err := r.Status().Update(ctx, proxy); err != nil { + ctxLogger.Error(err, "Failed to update MCPRemoteProxy status after CABundleRef validation") + } +} + +// setCABundleRefConditionForProxy sets the CA bundle ref validation condition on an MCPRemoteProxy +func setCABundleRefConditionForProxy(proxy *mcpv1alpha1.MCPRemoteProxy, status metav1.ConditionStatus, reason, message string) { + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + Status: status, + Reason: reason, + Message: message, + ObservedGeneration: proxy.Generation, + }) +} + // ensureRBACResources ensures that the RBAC resources are in place for the remote proxy. // Uses the RBAC client (pkg/kubernetes/rbac) which creates or updates RBAC resources // automatically during operator upgrades. diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go index 7691be4b79..46b0e94fa8 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go @@ -787,6 +787,181 @@ func TestHandleExternalAuthConfig(t *testing.T) { } } +// TestMCPRemoteProxyValidateCABundleRef tests the CA bundle ref validation logic +func TestMCPRemoteProxyValidateCABundleRef(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + proxy *mcpv1alpha1.MCPRemoteProxy + configMap *corev1.ConfigMap + expectCondition bool + expectedStatus metav1.ConditionStatus + expectedReason string + }{ + { + name: "no CABundleRef configured", + proxy: &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-ca-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + OIDCConfig: mcpv1alpha1.OIDCConfigRef{ + Type: mcpv1alpha1.OIDCConfigTypeInline, + Inline: &mcpv1alpha1.InlineOIDCConfig{ + Issuer: "https://auth.example.com", + Audience: "mcp-proxy", + }, + }, + }, + }, + expectCondition: false, + }, + { + name: "valid CABundleRef with existing ConfigMap and key", + proxy: &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-ca-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + OIDCConfig: mcpv1alpha1.OIDCConfigRef{ + Type: mcpv1alpha1.OIDCConfigTypeInline, + Inline: &mcpv1alpha1.InlineOIDCConfig{ + Issuer: "https://auth.example.com", + Audience: "mcp-proxy", + CABundleRef: &mcpv1alpha1.CABundleSource{ + ConfigMapRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "ca-bundle-cm"}, + Key: "ca.crt", + }, + }, + }, + }, + }, + }, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-bundle-cm", + Namespace: "default", + }, + Data: map[string]string{ + "ca.crt": "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----", + }, + }, + expectCondition: true, + expectedStatus: metav1.ConditionTrue, + expectedReason: mcpv1alpha1.ConditionReasonCABundleRefValid, + }, + { + name: "CABundleRef referencing non-existent ConfigMap", + proxy: &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "missing-cm-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + OIDCConfig: mcpv1alpha1.OIDCConfigRef{ + Type: mcpv1alpha1.OIDCConfigTypeInline, + Inline: &mcpv1alpha1.InlineOIDCConfig{ + Issuer: "https://auth.example.com", + Audience: "mcp-proxy", + CABundleRef: &mcpv1alpha1.CABundleSource{ + ConfigMapRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "non-existent-cm"}, + Key: "ca.crt", + }, + }, + }, + }, + }, + }, + expectCondition: true, + expectedStatus: metav1.ConditionFalse, + expectedReason: mcpv1alpha1.ConditionReasonCABundleRefNotFound, + }, + { + name: "CABundleRef with missing key in ConfigMap", + proxy: &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "missing-key-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + OIDCConfig: mcpv1alpha1.OIDCConfigRef{ + Type: mcpv1alpha1.OIDCConfigTypeInline, + Inline: &mcpv1alpha1.InlineOIDCConfig{ + Issuer: "https://auth.example.com", + Audience: "mcp-proxy", + CABundleRef: &mcpv1alpha1.CABundleSource{ + ConfigMapRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "ca-bundle-cm"}, + Key: "wrong-key", + }, + }, + }, + }, + }, + }, + configMap: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-bundle-cm", + Namespace: "default", + }, + Data: map[string]string{ + "ca.crt": "cert-data", + }, + }, + expectCondition: true, + expectedStatus: metav1.ConditionFalse, + expectedReason: mcpv1alpha1.ConditionReasonCABundleRefInvalid, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + scheme := createRunConfigTestScheme() + objects := []runtime.Object{tt.proxy} + if tt.configMap != nil { + objects = append(objects, tt.configMap) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(objects...). + WithStatusSubresource(&mcpv1alpha1.MCPRemoteProxy{}). + Build() + + reconciler := &MCPRemoteProxyReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + reconciler.validateCABundleRef(context.TODO(), tt.proxy) + + cond := meta.FindStatusCondition(tt.proxy.Status.Conditions, + mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated) + + if tt.expectCondition { + assert.NotNil(t, cond, "CABundleRefValidated condition should be set") + if cond != nil { + assert.Equal(t, tt.expectedStatus, cond.Status) + assert.Equal(t, tt.expectedReason, cond.Reason) + } + } else { + assert.Nil(t, cond, "CABundleRefValidated condition should not be set") + } + }) + } +} + // TestLabelsForMCPRemoteProxy tests label generation func TestLabelsForMCPRemoteProxy(t *testing.T) { t.Parallel() diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go b/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go index f5c5c5f3cb..f969fa37e7 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go @@ -139,6 +139,11 @@ func (*MCPRemoteProxyReconciler) buildVolumesForProxy( volumes = append(volumes, *authzVolume) } + // Add OIDC CA bundle volumes if configured + caBundleVolumes, caBundleMounts := ctrlutil.AddOIDCCABundleVolumes(&proxy.Spec.OIDCConfig) + volumes = append(volumes, caBundleVolumes...) + volumeMounts = append(volumeMounts, caBundleMounts...) + return volumeMounts, volumes } diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go b/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go index 48677e8bd6..01243c64d0 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_deployment_test.go @@ -194,6 +194,61 @@ func TestDeploymentForMCPRemoteProxy(t *testing.T) { assert.Equal(t, int32(9090), container.Ports[0].ContainerPort) }, }, + { + name: "with OIDC CA bundle volumes", + proxy: &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cabundle-proxy", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://mcp.example.com", + ProxyPort: 8080, + OIDCConfig: mcpv1alpha1.OIDCConfigRef{ + Type: mcpv1alpha1.OIDCConfigTypeInline, + Inline: &mcpv1alpha1.InlineOIDCConfig{ + Issuer: "https://auth.example.com", + Audience: "mcp-proxy", + CABundleRef: &mcpv1alpha1.CABundleSource{ + ConfigMapRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "my-ca-bundle"}, + Key: "ca.crt", + }, + }, + }, + }, + }, + }, + validate: func(t *testing.T, dep *appsv1.Deployment) { + t.Helper() + // Verify CA bundle volume exists + foundVolume := false + for _, vol := range dep.Spec.Template.Spec.Volumes { + if vol.Name == "oidc-ca-bundle-my-ca-bundle" { + foundVolume = true + require.NotNil(t, vol.ConfigMap) + assert.Equal(t, "my-ca-bundle", vol.ConfigMap.Name) + require.Len(t, vol.ConfigMap.Items, 1) + assert.Equal(t, "ca.crt", vol.ConfigMap.Items[0].Key) + break + } + } + assert.True(t, foundVolume, "CA bundle volume should be present") + + // Verify CA bundle volume mount exists + container := dep.Spec.Template.Spec.Containers[0] + foundMount := false + for _, mount := range container.VolumeMounts { + if mount.Name == "oidc-ca-bundle-my-ca-bundle" { + foundMount = true + assert.Equal(t, "/config/certs/my-ca-bundle", mount.MountPath) + assert.True(t, mount.ReadOnly) + break + } + } + assert.True(t, foundMount, "CA bundle volume mount should be present") + }, + }, { name: "proxyPort takes precedence over port", proxy: &mcpv1alpha1.MCPRemoteProxy{ diff --git a/cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_validation_integration_test.go b/cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_validation_integration_test.go index 21e87a81a9..64f7fa37c8 100644 --- a/cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_validation_integration_test.go +++ b/cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_validation_integration_test.go @@ -178,6 +178,118 @@ var _ = Describe("MCPRemoteProxy Configuration Validation", Label("k8s", "remote }) }) + Context("CA Bundle ConfigMap Reference Validation", func() { + It("should set CABundleRefValidated=True when CA bundle ConfigMap exists with correct key", func() { + By("creating a CA bundle ConfigMap") + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-bundle-valid", + Namespace: testNamespace, + }, + Data: map[string]string{ + "ca.crt": "-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----", + }, + } + Expect(k8sClient.Create(testCtx, configMap)).To(Succeed()) + + By("creating an MCPRemoteProxy with valid CA bundle reference") + proxy := proxyHelper.NewRemoteProxyBuilder("test-valid-ca"). + WithCABundleRef("ca-bundle-valid", "ca.crt"). + Create(proxyHelper) + + By("waiting for the CABundleRefValidated condition to be True") + statusHelper.WaitForCondition( + proxy.Name, + mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + metav1.ConditionTrue, + MediumTimeout, + ) + + condition, err := proxyHelper.GetRemoteProxyCondition( + proxy.Name, mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(condition.Reason).To(Equal(mcpv1alpha1.ConditionReasonCABundleRefValid)) + }) + + It("should set CABundleRefValidated=False when CA bundle ConfigMap does not exist", func() { + By("creating an MCPRemoteProxy with non-existent CA bundle ConfigMap") + proxy := proxyHelper.NewRemoteProxyBuilder("test-missing-ca-cm"). + WithCABundleRef("non-existent-ca-bundle", "ca.crt"). + Create(proxyHelper) + + By("waiting for the CABundleRefValidated condition to be False") + statusHelper.WaitForConditionReason( + proxy.Name, + mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + mcpv1alpha1.ConditionReasonCABundleRefNotFound, + MediumTimeout, + ) + + condition, err := proxyHelper.GetRemoteProxyCondition( + proxy.Name, mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Message).To(ContainSubstring("not found")) + }) + + It("should set CABundleRefValidated=False when CA bundle ConfigMap exists but key is missing", func() { + By("creating a CA bundle ConfigMap with a different key") + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-bundle-wrong-key", + Namespace: testNamespace, + }, + Data: map[string]string{ + "other-key": "some-data", + }, + } + Expect(k8sClient.Create(testCtx, configMap)).To(Succeed()) + + By("creating an MCPRemoteProxy referencing a non-existent key") + proxy := proxyHelper.NewRemoteProxyBuilder("test-wrong-key-ca"). + WithCABundleRef("ca-bundle-wrong-key", "ca.crt"). + Create(proxyHelper) + + By("waiting for the CABundleRefValidated condition to be False") + statusHelper.WaitForConditionReason( + proxy.Name, + mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + mcpv1alpha1.ConditionReasonCABundleRefInvalid, + MediumTimeout, + ) + + condition, err := proxyHelper.GetRemoteProxyCondition( + proxy.Name, mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + ) + Expect(err).NotTo(HaveOccurred()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Message).To(ContainSubstring("ca.crt")) + }) + + It("should not set CABundleRefValidated condition when no CA bundle is configured", func() { + By("creating an MCPRemoteProxy without CA bundle reference") + proxy := proxyHelper.NewRemoteProxyBuilder("test-no-ca"). + Create(proxyHelper) + + By("waiting for the ConfigurationValid condition to be True") + statusHelper.WaitForCondition( + proxy.Name, + mcpv1alpha1.ConditionTypeConfigurationValid, + metav1.ConditionTrue, + MediumTimeout, + ) + + By("verifying the CABundleRefValidated condition is not set") + _, err := proxyHelper.GetRemoteProxyCondition( + proxy.Name, mcpv1alpha1.ConditionTypeMCPRemoteProxyCABundleRefValidated, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not found")) + }) + }) + Context("Kubernetes Events", func() { It("should emit a Warning event when OIDC issuer uses HTTP", func() { By("creating an MCPRemoteProxy with HTTP OIDC issuer") diff --git a/cmd/thv-operator/test-integration/mcp-remote-proxy/remoteproxy_helpers.go b/cmd/thv-operator/test-integration/mcp-remote-proxy/remoteproxy_helpers.go index 2300693758..2df4b0401d 100644 --- a/cmd/thv-operator/test-integration/mcp-remote-proxy/remoteproxy_helpers.go +++ b/cmd/thv-operator/test-integration/mcp-remote-proxy/remoteproxy_helpers.go @@ -10,6 +10,7 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -142,6 +143,20 @@ func (rb *RemoteProxyBuilder) WithInlineOIDCConfig(issuer, audience string, inse return rb } +// WithCABundleRef sets a CA bundle ConfigMap reference on the inline OIDC config +func (rb *RemoteProxyBuilder) WithCABundleRef(configMapName, key string) *RemoteProxyBuilder { + if rb.proxy.Spec.OIDCConfig.Inline == nil { + rb.proxy.Spec.OIDCConfig.Inline = &mcpv1alpha1.InlineOIDCConfig{} + } + rb.proxy.Spec.OIDCConfig.Inline.CABundleRef = &mcpv1alpha1.CABundleSource{ + ConfigMapRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: configMapName}, + Key: key, + }, + } + return rb +} + // WithRemoteURL overrides the default remote URL func (rb *RemoteProxyBuilder) WithRemoteURL(url string) *RemoteProxyBuilder { rb.proxy.Spec.RemoteURL = url