Skip to content

Commit bb0e6c9

Browse files
JAORMXclaudeChrisJBurns
authored
Add SecretKeyRef support to InlineOIDCConfig for enhanced secret management (#2324)
* Add SecretKeyRef support to InlineOIDCConfig for enhanced secret management Add Kubernetes-native secret reference support to InlineOIDCConfig, following the pattern established by MCPExternalAuthConfig. This enables secure OIDC client secret management without exposing secrets in YAML manifests or ConfigMaps. Changes: - Add ClientSecretRef field to InlineOIDCConfig CRD type - Deprecate plaintext ClientSecret field (backward compatible) - Update OIDC resolver to skip embedding secrets when using SecretKeyRef - Create GenerateOIDCClientSecretEnvVar function for secret validation - Integrate secret injection in MCPServer and MCPRemoteProxy controllers - Update token validator to load secrets from TOOLHIVE_OIDC_CLIENT_SECRET - Bump CRD chart version from 0.0.43 to 0.0.44 - Update architecture documentation and add example manifests Security benefits: - Secrets managed via Kubernetes RBAC - Integration with external secret operators (Vault, AWS Secrets Manager) - Secrets not exposed in YAML manifests or Git history - Consistent pattern across all ToolHive secret management Resolves: #2321 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Bump operator chart version to 0.3.2 for CRD compatibility The operator chart version needs to be bumped when CRDs are updated to ensure compatibility during Helm chart testing. This fixes the Helm chart test failure where the operator pod was crashing due to CRD version mismatch. * Add comprehensive tests for OIDC ClientSecretRef functionality Add unit tests to verify: - GenerateOIDCClientSecretEnvVar function with various scenarios - OIDC resolver behavior with ClientSecretRef - Precedence when both ClientSecret and ClientSecretRef are provided - Backward compatibility with existing ClientSecret field All tests pass successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * adds an order to chart installation Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com> * covnerges lint and install to same step Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com> * separtes lint Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com> * cannot have `all` and `charts` together Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com> * removes operator bump because its not needed yet Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com> --------- Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com>
1 parent 3fffdcc commit bb0e6c9

18 files changed

+483
-20
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,15 @@ type InlineOIDCConfig struct {
465465
ClientID string `json:"clientId,omitempty"`
466466

467467
// ClientSecret is the client secret for introspection (optional)
468+
// Deprecated: Use ClientSecretRef instead for better security
468469
// +optional
469470
ClientSecret string `json:"clientSecret,omitempty"`
470471

472+
// ClientSecretRef is a reference to a Kubernetes Secret containing the client secret
473+
// If both ClientSecret and ClientSecretRef are provided, ClientSecretRef takes precedence
474+
// +optional
475+
ClientSecretRef *SecretKeyRef `json:"clientSecretRef,omitempty"`
476+
471477
// ThvCABundlePath is the path to CA certificate bundle file for HTTPS requests
472478
// The file must be mounted into the pod (e.g., via ConfigMap or Secret volume)
473479
// +optional

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

Lines changed: 6 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cmd/thv-operator/controllers/mcpremoteproxy_deployment.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,19 @@ func (r *MCPRemoteProxyReconciler) buildEnvVarsForProxy(
151151
}
152152
}
153153

154+
// Add OIDC client secret environment variable if using inline config with secretRef
155+
if proxy.Spec.OIDCConfig.Type == "inline" && proxy.Spec.OIDCConfig.Inline != nil {
156+
oidcClientSecretEnvVar, err := ctrlutil.GenerateOIDCClientSecretEnvVar(
157+
ctx, r.Client, proxy.Namespace, proxy.Spec.OIDCConfig.Inline.ClientSecretRef,
158+
)
159+
if err != nil {
160+
ctxLogger := log.FromContext(ctx)
161+
ctxLogger.Error(err, "Failed to generate OIDC client secret environment variable")
162+
} else if oidcClientSecretEnvVar != nil {
163+
env = append(env, *oidcClientSecretEnvVar)
164+
}
165+
}
166+
154167
// Add user-specified environment variables
155168
if proxy.Spec.ResourceOverrides != nil && proxy.Spec.ResourceOverrides.ProxyDeployment != nil {
156169
for _, envVar := range proxy.Spec.ResourceOverrides.ProxyDeployment.Env {

‎cmd/thv-operator/controllers/mcpserver_controller.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,19 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
986986
}
987987
}
988988

989+
// Add OIDC client secret environment variable if using inline config with secretRef
990+
if m.Spec.OIDCConfig != nil && m.Spec.OIDCConfig.Inline != nil {
991+
oidcClientSecretEnvVar, err := ctrlutil.GenerateOIDCClientSecretEnvVar(
992+
ctx, r.Client, m.Namespace, m.Spec.OIDCConfig.Inline.ClientSecretRef,
993+
)
994+
if err != nil {
995+
ctxLogger := log.FromContext(ctx)
996+
ctxLogger.Error(err, "Failed to generate OIDC client secret environment variable")
997+
} else if oidcClientSecretEnvVar != nil {
998+
env = append(env, *oidcClientSecretEnvVar)
999+
}
1000+
}
1001+
9891002
// Add user-specified proxy environment variables from ResourceOverrides
9901003
if m.Spec.ResourceOverrides != nil && m.Spec.ResourceOverrides.ProxyDeployment != nil {
9911004
for _, envVar := range m.Spec.ResourceOverrides.ProxyDeployment.Env {
@@ -1442,6 +1455,20 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
14421455
expectedProxyEnv = append(expectedProxyEnv, tokenExchangeEnvVars...)
14431456
}
14441457

1458+
// Add OIDC client secret environment variable if using inline config with secretRef
1459+
if mcpServer.Spec.OIDCConfig != nil && mcpServer.Spec.OIDCConfig.Inline != nil {
1460+
oidcClientSecretEnvVar, err := ctrlutil.GenerateOIDCClientSecretEnvVar(
1461+
ctx, r.Client, mcpServer.Namespace, mcpServer.Spec.OIDCConfig.Inline.ClientSecretRef,
1462+
)
1463+
if err != nil {
1464+
// If we can't generate env var, consider the deployment needs update
1465+
return true
1466+
}
1467+
if oidcClientSecretEnvVar != nil {
1468+
expectedProxyEnv = append(expectedProxyEnv, *oidcClientSecretEnvVar)
1469+
}
1470+
}
1471+
14451472
// Add user-specified environment variables
14461473
if mcpServer.Spec.ResourceOverrides != nil && mcpServer.Spec.ResourceOverrides.ProxyDeployment != nil {
14471474
for _, envVar := range mcpServer.Spec.ResourceOverrides.ProxyDeployment.Env {

‎cmd/thv-operator/pkg/controllerutil/oidc.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@ import (
44
"context"
55
"fmt"
66

7+
corev1 "k8s.io/api/core/v1"
8+
"k8s.io/apimachinery/pkg/types"
79
"sigs.k8s.io/controller-runtime/pkg/client"
810

11+
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
912
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc"
1013
"github.com/stacklok/toolhive/pkg/runner"
1114
)
@@ -45,3 +48,46 @@ func AddOIDCConfigOptions(
4548

4649
return nil
4750
}
51+
52+
// GenerateOIDCClientSecretEnvVar generates environment variable for OIDC client secret
53+
// when using a SecretKeyRef.
54+
// Returns nil if clientSecretRef is nil.
55+
func GenerateOIDCClientSecretEnvVar(
56+
ctx context.Context,
57+
c client.Client,
58+
namespace string,
59+
clientSecretRef *mcpv1alpha1.SecretKeyRef,
60+
) (*corev1.EnvVar, error) {
61+
if clientSecretRef == nil {
62+
return nil, nil
63+
}
64+
65+
// Validate that the referenced secret exists
66+
var secret corev1.Secret
67+
if err := c.Get(ctx, types.NamespacedName{
68+
Namespace: namespace,
69+
Name: clientSecretRef.Name,
70+
}, &secret); err != nil {
71+
return nil, fmt.Errorf("failed to get OIDC client secret %s/%s: %w",
72+
namespace, clientSecretRef.Name, err)
73+
}
74+
75+
// Validate that the key exists in the secret
76+
if _, ok := secret.Data[clientSecretRef.Key]; !ok {
77+
return nil, fmt.Errorf("OIDC client secret %s/%s is missing key %q",
78+
namespace, clientSecretRef.Name, clientSecretRef.Key)
79+
}
80+
81+
// Return environment variable with secret reference
82+
return &corev1.EnvVar{
83+
Name: "TOOLHIVE_OIDC_CLIENT_SECRET",
84+
ValueFrom: &corev1.EnvVarSource{
85+
SecretKeyRef: &corev1.SecretKeySelector{
86+
LocalObjectReference: corev1.LocalObjectReference{
87+
Name: clientSecretRef.Name,
88+
},
89+
Key: clientSecretRef.Key,
90+
},
91+
},
92+
}, nil
93+
}

‎cmd/thv-operator/pkg/controllerutil/oidc_test.go‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
package controllerutil
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
corev1 "k8s.io/api/core/v1"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
"k8s.io/apimachinery/pkg/runtime"
12+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
13+
14+
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
15+
)
16+
17+
func TestGenerateOIDCClientSecretEnvVar(t *testing.T) {
18+
t.Parallel()
19+
20+
tests := []struct {
21+
name string
22+
clientSecretRef *mcpv1alpha1.SecretKeyRef
23+
secret *corev1.Secret
24+
expectError bool
25+
errContains string
26+
validate func(*testing.T, *corev1.EnvVar)
27+
}{
28+
{
29+
name: "nil client secret ref returns nil",
30+
clientSecretRef: nil,
31+
expectError: false,
32+
validate: func(t *testing.T, envVar *corev1.EnvVar) {
33+
t.Helper()
34+
assert.Nil(t, envVar)
35+
},
36+
},
37+
{
38+
name: "valid secret ref generates env var",
39+
clientSecretRef: &mcpv1alpha1.SecretKeyRef{
40+
Name: "oidc-secret",
41+
Key: "client-secret",
42+
},
43+
secret: &corev1.Secret{
44+
ObjectMeta: metav1.ObjectMeta{
45+
Name: "oidc-secret",
46+
Namespace: "default",
47+
},
48+
Data: map[string][]byte{
49+
"client-secret": []byte("secret-value"),
50+
},
51+
},
52+
expectError: false,
53+
validate: func(t *testing.T, envVar *corev1.EnvVar) {
54+
t.Helper()
55+
require.NotNil(t, envVar)
56+
assert.Equal(t, "TOOLHIVE_OIDC_CLIENT_SECRET", envVar.Name)
57+
require.NotNil(t, envVar.ValueFrom)
58+
require.NotNil(t, envVar.ValueFrom.SecretKeyRef)
59+
assert.Equal(t, "oidc-secret", envVar.ValueFrom.SecretKeyRef.Name)
60+
assert.Equal(t, "client-secret", envVar.ValueFrom.SecretKeyRef.Key)
61+
},
62+
},
63+
{
64+
name: "missing secret returns error",
65+
clientSecretRef: &mcpv1alpha1.SecretKeyRef{
66+
Name: "missing-secret",
67+
Key: "client-secret",
68+
},
69+
expectError: true,
70+
errContains: "failed to get OIDC client secret",
71+
},
72+
{
73+
name: "missing key in secret returns error",
74+
clientSecretRef: &mcpv1alpha1.SecretKeyRef{
75+
Name: "oidc-secret",
76+
Key: "wrong-key",
77+
},
78+
secret: &corev1.Secret{
79+
ObjectMeta: metav1.ObjectMeta{
80+
Name: "oidc-secret",
81+
Namespace: "default",
82+
},
83+
Data: map[string][]byte{
84+
"client-secret": []byte("secret-value"),
85+
},
86+
},
87+
expectError: true,
88+
errContains: "is missing key",
89+
},
90+
}
91+
92+
for _, tt := range tests {
93+
t.Run(tt.name, func(t *testing.T) {
94+
t.Parallel()
95+
96+
scheme := runtime.NewScheme()
97+
err := corev1.AddToScheme(scheme)
98+
require.NoError(t, err)
99+
err = mcpv1alpha1.AddToScheme(scheme)
100+
require.NoError(t, err)
101+
102+
var fakeClient *fake.ClientBuilder
103+
if tt.secret != nil {
104+
fakeClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.secret)
105+
} else {
106+
fakeClient = fake.NewClientBuilder().WithScheme(scheme)
107+
}
108+
109+
ctx := context.TODO()
110+
envVar, err := GenerateOIDCClientSecretEnvVar(
111+
ctx,
112+
fakeClient.Build(),
113+
"default",
114+
tt.clientSecretRef,
115+
)
116+
117+
if tt.expectError {
118+
assert.Error(t, err)
119+
if tt.errContains != "" {
120+
assert.Contains(t, err.Error(), tt.errContains)
121+
}
122+
} else {
123+
assert.NoError(t, err)
124+
if tt.validate != nil {
125+
tt.validate(t, envVar)
126+
}
127+
}
128+
})
129+
}
130+
}

‎cmd/thv-operator/pkg/oidc/resolver.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,20 @@ func (*resolver) resolveInlineConfig(
204204
return nil, nil
205205
}
206206

207+
// Don't embed ClientSecret in the config if ClientSecretRef is set
208+
// The secret will be injected via environment variable instead
209+
clientSecret := config.ClientSecret
210+
if config.ClientSecretRef != nil {
211+
clientSecret = ""
212+
}
213+
207214
return &OIDCConfig{
208215
Issuer: config.Issuer,
209216
Audience: config.Audience,
210217
JWKSURL: config.JWKSURL,
211218
IntrospectionURL: config.IntrospectionURL,
212219
ClientID: config.ClientID,
213-
ClientSecret: config.ClientSecret,
220+
ClientSecret: clientSecret,
214221
ThvCABundlePath: config.ThvCABundlePath,
215222
JWKSAuthTokenPath: config.JWKSAuthTokenPath,
216223
ResourceURL: resourceURL,

0 commit comments

Comments
 (0)