Skip to content

Commit 8cfe29b

Browse files
committed
fix ci
1 parent ae87961 commit 8cfe29b

File tree

7 files changed

+191
-63
lines changed

7 files changed

+191
-63
lines changed

cmd/thv-operator/controllers/virtualmcpserver_controller.go

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import (
88
"fmt"
99
"maps"
1010
"reflect"
11-
"regexp"
12-
"strings"
1311
"time"
1412

1513
appsv1 "k8s.io/api/apps/v1"
@@ -1271,37 +1269,17 @@ func (*VirtualMCPServerReconciler) convertExternalAuthConfigToStrategy(
12711269
if strategy.TokenExchange != nil &&
12721270
externalAuthConfig.Spec.TokenExchange != nil &&
12731271
externalAuthConfig.Spec.TokenExchange.ClientSecretRef != nil {
1274-
strategy.TokenExchange.ClientSecretEnv = generateUniqueTokenExchangeEnvVarName(externalAuthConfig.Name)
1272+
strategy.TokenExchange.ClientSecretEnv = ctrlutil.GenerateUniqueTokenExchangeEnvVarName(externalAuthConfig.Name)
12751273
}
12761274
if strategy.HeaderInjection != nil &&
12771275
externalAuthConfig.Spec.HeaderInjection != nil &&
12781276
externalAuthConfig.Spec.HeaderInjection.ValueSecretRef != nil {
1279-
strategy.HeaderInjection.HeaderValueEnv = generateUniqueHeaderInjectionEnvVarName(externalAuthConfig.Name)
1277+
strategy.HeaderInjection.HeaderValueEnv = ctrlutil.GenerateUniqueHeaderInjectionEnvVarName(externalAuthConfig.Name)
12801278
}
12811279

12821280
return strategy, nil
12831281
}
12841282

1285-
// generateUniqueTokenExchangeEnvVarName generates a unique environment variable name for token exchange
1286-
// client secrets, incorporating the ExternalAuthConfig name to ensure uniqueness.
1287-
func generateUniqueTokenExchangeEnvVarName(configName string) string {
1288-
// Sanitize config name for use in env var (uppercase, replace invalid chars with underscore)
1289-
sanitized := strings.ToUpper(strings.ReplaceAll(configName, "-", "_"))
1290-
// Remove any remaining invalid characters (keep only alphanumeric and underscore)
1291-
sanitized = regexp.MustCompile(`[^A-Z0-9_]`).ReplaceAllString(sanitized, "_")
1292-
return fmt.Sprintf("TOOLHIVE_TOKEN_EXCHANGE_CLIENT_SECRET_%s", sanitized)
1293-
}
1294-
1295-
// generateUniqueHeaderInjectionEnvVarName generates a unique environment variable name for header injection
1296-
// values, incorporating the ExternalAuthConfig name to ensure uniqueness.
1297-
func generateUniqueHeaderInjectionEnvVarName(configName string) string {
1298-
// Sanitize config name for use in env var (uppercase, replace invalid chars with underscore)
1299-
sanitized := strings.ToUpper(strings.ReplaceAll(configName, "-", "_"))
1300-
// Remove any remaining invalid characters (keep only alphanumeric and underscore)
1301-
sanitized = regexp.MustCompile(`[^A-Z0-9_]`).ReplaceAllString(sanitized, "_")
1302-
return fmt.Sprintf("TOOLHIVE_HEADER_INJECTION_VALUE_%s", sanitized)
1303-
}
1304-
13051283
// convertBackendAuthConfigToVMCP converts a BackendAuthConfig from CRD to vmcp config.
13061284
func (r *VirtualMCPServerReconciler) convertBackendAuthConfigToVMCP(
13071285
ctx context.Context,

cmd/thv-operator/controllers/virtualmcpserver_deployment.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ func (r *VirtualMCPServerReconciler) getExternalAuthConfigSecretEnvVar(
425425
if externalAuthConfig.Spec.TokenExchange.ClientSecretRef == nil {
426426
return nil, nil // No secret to mount
427427
}
428-
envVarName = generateUniqueTokenExchangeEnvVarName(externalAuthConfigName)
428+
envVarName = ctrlutil.GenerateUniqueTokenExchangeEnvVarName(externalAuthConfigName)
429429
secretRef = externalAuthConfig.Spec.TokenExchange.ClientSecretRef
430430

431431
case mcpv1alpha1.ExternalAuthTypeHeaderInjection:
@@ -435,7 +435,7 @@ func (r *VirtualMCPServerReconciler) getExternalAuthConfigSecretEnvVar(
435435
if externalAuthConfig.Spec.HeaderInjection.ValueSecretRef == nil {
436436
return nil, nil // No secret to mount
437437
}
438-
envVarName = generateUniqueHeaderInjectionEnvVarName(externalAuthConfigName)
438+
envVarName = ctrlutil.GenerateUniqueHeaderInjectionEnvVarName(externalAuthConfigName)
439439
secretRef = externalAuthConfig.Spec.HeaderInjection.ValueSecretRef
440440

441441
case mcpv1alpha1.ExternalAuthTypeUnauthenticated:

cmd/thv-operator/controllers/virtualmcpserver_externalauth_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,7 @@ func TestGenerateUniqueTokenExchangeEnvVarName(t *testing.T) {
950950
for _, tt := range tests {
951951
t.Run(tt.name, func(t *testing.T) {
952952
t.Parallel()
953-
result := generateUniqueTokenExchangeEnvVarName(tt.configName)
953+
result := ctrlutil.GenerateUniqueTokenExchangeEnvVarName(tt.configName)
954954
assert.Contains(t, result, expectedPrefix)
955955
assert.Contains(t, result, tt.expectedSuffix)
956956
// Verify format: PREFIX_SUFFIX
@@ -1007,7 +1007,7 @@ func TestGenerateUniqueHeaderInjectionEnvVarName(t *testing.T) {
10071007
for _, tt := range tests {
10081008
t.Run(tt.name, func(t *testing.T) {
10091009
t.Parallel()
1010-
result := generateUniqueHeaderInjectionEnvVarName(tt.configName)
1010+
result := ctrlutil.GenerateUniqueHeaderInjectionEnvVarName(tt.configName)
10111011
assert.True(t, strings.HasPrefix(result, expectedPrefix+"_"), "Result should start with prefix")
10121012
assert.True(t, strings.HasSuffix(result, tt.expectedSuffix), "Result should end with suffix")
10131013
// Verify format: PREFIX_SUFFIX

cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,8 @@ func TestConvertBackendAuthConfig(t *testing.T) {
196196
authConfig: &mcpv1alpha1.BackendAuthConfig{
197197
Type: mcpv1alpha1.BackendAuthTypeDiscovered,
198198
},
199-
expectedType: mcpv1alpha1.BackendAuthTypeDiscovered,
199+
// "discovered" type is converted to "unauthenticated" by the converter
200+
expectedType: "unauthenticated",
200201
},
201202
{
202203
name: "external auth config ref",
@@ -206,7 +207,8 @@ func TestConvertBackendAuthConfig(t *testing.T) {
206207
Name: "auth-config",
207208
},
208209
},
209-
expectedType: mcpv1alpha1.BackendAuthTypeExternalAuthConfigRef,
210+
// For external_auth_config_ref, the type comes from the referenced MCPExternalAuthConfig
211+
expectedType: "unauthenticated",
210212
},
211213
}
212214

@@ -216,6 +218,10 @@ func TestConvertBackendAuthConfig(t *testing.T) {
216218
t.Parallel()
217219

218220
vmcpServer := &mcpv1alpha1.VirtualMCPServer{
221+
ObjectMeta: metav1.ObjectMeta{
222+
Name: "test-vmcp",
223+
Namespace: "default",
224+
},
219225
Spec: mcpv1alpha1.VirtualMCPServerSpec{
220226
GroupRef: mcpv1alpha1.GroupRef{
221227
Name: "test-group",
@@ -226,7 +232,34 @@ func TestConvertBackendAuthConfig(t *testing.T) {
226232
},
227233
}
228234

229-
converter := newTestConverter(t, newNoOpMockResolver(t))
235+
// For external_auth_config_ref test, create the referenced MCPExternalAuthConfig
236+
var converter *vmcpconfig.Converter
237+
if tt.authConfig.Type == mcpv1alpha1.BackendAuthTypeExternalAuthConfigRef {
238+
// Create a fake MCPExternalAuthConfig
239+
externalAuthConfig := &mcpv1alpha1.MCPExternalAuthConfig{
240+
ObjectMeta: metav1.ObjectMeta{
241+
Name: "auth-config",
242+
Namespace: "default",
243+
},
244+
Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{
245+
Type: mcpv1alpha1.ExternalAuthTypeUnauthenticated,
246+
},
247+
}
248+
249+
// Create converter with fake client that has the external auth config
250+
scheme := runtime.NewScheme()
251+
_ = mcpv1alpha1.AddToScheme(scheme)
252+
fakeClient := fake.NewClientBuilder().
253+
WithScheme(scheme).
254+
WithObjects(externalAuthConfig).
255+
Build()
256+
var err error
257+
converter, err = vmcpconfig.NewConverter(newNoOpMockResolver(t), fakeClient)
258+
require.NoError(t, err)
259+
} else {
260+
converter = newTestConverter(t, newNoOpMockResolver(t))
261+
}
262+
230263
config, err := converter.Convert(context.Background(), vmcpServer)
231264
require.NoError(t, err)
232265

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Package controllerutil provides utility functions for Kubernetes controllers.
2+
package controllerutil
3+
4+
import (
5+
"fmt"
6+
"regexp"
7+
"strings"
8+
)
9+
10+
// GenerateUniqueTokenExchangeEnvVarName generates a unique environment variable name for token exchange
11+
// client secrets, incorporating the ExternalAuthConfig name to ensure uniqueness.
12+
// This function is used by both the converter and deployment controller to ensure consistent
13+
// environment variable naming across the system.
14+
//
15+
// Example: For an ExternalAuthConfig named "my-auth-config", this returns:
16+
// "TOOLHIVE_TOKEN_EXCHANGE_CLIENT_SECRET_MY_AUTH_CONFIG"
17+
func GenerateUniqueTokenExchangeEnvVarName(configName string) string {
18+
// Sanitize config name for use in env var (uppercase, replace invalid chars with underscore)
19+
sanitized := strings.ToUpper(strings.ReplaceAll(configName, "-", "_"))
20+
// Remove any remaining invalid characters (keep only alphanumeric and underscore)
21+
sanitized = regexp.MustCompile(`[^A-Z0-9_]`).ReplaceAllString(sanitized, "_")
22+
return fmt.Sprintf("TOOLHIVE_TOKEN_EXCHANGE_CLIENT_SECRET_%s", sanitized)
23+
}
24+
25+
// GenerateUniqueHeaderInjectionEnvVarName generates a unique environment variable name for header injection
26+
// values, incorporating the ExternalAuthConfig name to ensure uniqueness.
27+
// This function is used by both the converter and deployment controller to ensure consistent
28+
// environment variable naming across the system.
29+
//
30+
// Example: For an ExternalAuthConfig named "my-auth-config", this returns:
31+
// "TOOLHIVE_HEADER_INJECTION_VALUE_MY_AUTH_CONFIG"
32+
func GenerateUniqueHeaderInjectionEnvVarName(configName string) string {
33+
// Sanitize config name for use in env var (uppercase, replace invalid chars with underscore)
34+
sanitized := strings.ToUpper(strings.ReplaceAll(configName, "-", "_"))
35+
// Remove any remaining invalid characters (keep only alphanumeric and underscore)
36+
sanitized = regexp.MustCompile(`[^A-Z0-9_]`).ReplaceAllString(sanitized, "_")
37+
return fmt.Sprintf("TOOLHIVE_HEADER_INJECTION_VALUE_%s", sanitized)
38+
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package controllerutil
2+
3+
import (
4+
"regexp"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
// TestGenerateUniqueTokenExchangeEnvVarName tests the GenerateUniqueTokenExchangeEnvVarName function
11+
func TestGenerateUniqueTokenExchangeEnvVarName(t *testing.T) {
12+
t.Parallel()
13+
14+
expectedPrefix := "TOOLHIVE_TOKEN_EXCHANGE_CLIENT_SECRET"
15+
tests := []struct {
16+
name string
17+
configName string
18+
expectedSuffix string
19+
}{
20+
{
21+
name: "simple name",
22+
configName: "test-config",
23+
expectedSuffix: "TEST_CONFIG",
24+
},
25+
{
26+
name: "multiple hyphens",
27+
configName: "my-test-config",
28+
expectedSuffix: "MY_TEST_CONFIG",
29+
},
30+
{
31+
name: "with special characters",
32+
configName: "test.config@123",
33+
expectedSuffix: "TEST_CONFIG_123",
34+
},
35+
{
36+
name: "single character",
37+
configName: "a",
38+
expectedSuffix: "A",
39+
},
40+
}
41+
42+
for _, tt := range tests {
43+
t.Run(tt.name, func(t *testing.T) {
44+
t.Parallel()
45+
result := GenerateUniqueTokenExchangeEnvVarName(tt.configName)
46+
assert.Contains(t, result, expectedPrefix)
47+
assert.Contains(t, result, tt.expectedSuffix)
48+
// Verify format: PREFIX_SUFFIX
49+
assert.Contains(t, result, "_")
50+
// Verify all characters are valid for env vars (uppercase, alphanumeric, underscore)
51+
envVarPattern := regexp.MustCompile(`^[A-Z0-9_]+$`)
52+
assert.Regexp(t, envVarPattern, result, "Result should be a valid environment variable name")
53+
})
54+
}
55+
}
56+
57+
// TestGenerateUniqueHeaderInjectionEnvVarName tests the GenerateUniqueHeaderInjectionEnvVarName function
58+
func TestGenerateUniqueHeaderInjectionEnvVarName(t *testing.T) {
59+
t.Parallel()
60+
61+
expectedPrefix := "TOOLHIVE_HEADER_INJECTION_VALUE"
62+
tests := []struct {
63+
name string
64+
configName string
65+
expectedSuffix string
66+
}{
67+
{
68+
name: "simple name",
69+
configName: "test-config",
70+
expectedSuffix: "TEST_CONFIG",
71+
},
72+
{
73+
name: "multiple hyphens",
74+
configName: "my-test-config",
75+
expectedSuffix: "MY_TEST_CONFIG",
76+
},
77+
{
78+
name: "with special characters",
79+
configName: "test.config@123",
80+
expectedSuffix: "TEST_CONFIG_123",
81+
},
82+
{
83+
name: "single character",
84+
configName: "x",
85+
expectedSuffix: "X",
86+
},
87+
}
88+
89+
for _, tt := range tests {
90+
t.Run(tt.name, func(t *testing.T) {
91+
t.Parallel()
92+
result := GenerateUniqueHeaderInjectionEnvVarName(tt.configName)
93+
assert.True(t, regexp.MustCompile("^"+expectedPrefix+"_").MatchString(result), "Result should start with prefix")
94+
assert.True(t, regexp.MustCompile(tt.expectedSuffix+"$").MatchString(result), "Result should end with suffix")
95+
// Verify format: PREFIX_SUFFIX
96+
assert.Contains(t, result, "_")
97+
// Verify all characters are valid for env vars (uppercase, alphanumeric, underscore)
98+
envVarPattern := regexp.MustCompile(`^[A-Z0-9_]+$`)
99+
assert.Regexp(t, envVarPattern, result, "Result should be a valid environment variable name")
100+
})
101+
}
102+
}

cmd/thv-operator/pkg/vmcpconfig/converter.go

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import (
55
"context"
66
"encoding/json"
77
"fmt"
8-
"regexp"
9-
"strings"
108
"time"
119

1210
"github.com/go-logr/logr"
@@ -17,6 +15,7 @@ import (
1715
"sigs.k8s.io/controller-runtime/pkg/log"
1816

1917
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
18+
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
2019
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc"
2120
authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types"
2221
vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
@@ -267,14 +266,14 @@ func (c *Converter) convertBackendAuthConfig(
267266
crdConfig *mcpv1alpha1.BackendAuthConfig,
268267
) (*authtypes.BackendAuthStrategy, error) {
269268
// If type is "discovered", return unauthenticated strategy
270-
if crdConfig.Type == "discovered" {
269+
if crdConfig.Type == mcpv1alpha1.BackendAuthTypeDiscovered {
271270
return &authtypes.BackendAuthStrategy{
272-
Type: "unauthenticated",
271+
Type: authtypes.StrategyTypeUnauthenticated,
273272
}, nil
274273
}
275274

276275
// If type is "external_auth_config_ref", resolve the MCPExternalAuthConfig
277-
if crdConfig.Type == "external_auth_config_ref" {
276+
if crdConfig.Type == mcpv1alpha1.BackendAuthTypeExternalAuthConfigRef {
278277
if crdConfig.ExternalAuthConfigRef == nil {
279278
return nil, fmt.Errorf("backend %s: external_auth_config_ref type requires externalAuthConfigRef field", backendName)
280279
}
@@ -307,27 +306,27 @@ func (*Converter) convertExternalAuthConfigToStrategy(
307306

308307
switch externalAuthConfig.Spec.Type {
309308
case mcpv1alpha1.ExternalAuthTypeUnauthenticated:
310-
strategy.Type = "unauthenticated"
309+
strategy.Type = authtypes.StrategyTypeUnauthenticated
311310

312311
case mcpv1alpha1.ExternalAuthTypeHeaderInjection:
313312
if externalAuthConfig.Spec.HeaderInjection == nil {
314313
return nil, fmt.Errorf("headerInjection config is required when type is headerInjection")
315314
}
316315

317-
strategy.Type = "header_injection"
316+
strategy.Type = authtypes.StrategyTypeHeaderInjection
318317
strategy.HeaderInjection = &authtypes.HeaderInjectionConfig{
319318
HeaderName: externalAuthConfig.Spec.HeaderInjection.HeaderName,
320319
// The secret value will be mounted as an environment variable by the deployment controller
321320
// Use the same env var naming convention as the deployment controller
322-
HeaderValueEnv: generateUniqueHeaderInjectionEnvVarName(externalAuthConfig.Name),
321+
HeaderValueEnv: controllerutil.GenerateUniqueHeaderInjectionEnvVarName(externalAuthConfig.Name),
323322
}
324323

325324
case mcpv1alpha1.ExternalAuthTypeTokenExchange:
326325
if externalAuthConfig.Spec.TokenExchange == nil {
327326
return nil, fmt.Errorf("tokenExchange config is required when type is tokenExchange")
328327
}
329328

330-
strategy.Type = "token_exchange"
329+
strategy.Type = authtypes.StrategyTypeTokenExchange
331330
strategy.TokenExchange = &authtypes.TokenExchangeConfig{
332331
TokenURL: externalAuthConfig.Spec.TokenExchange.TokenURL,
333332
ClientID: externalAuthConfig.Spec.TokenExchange.ClientID,
@@ -340,7 +339,7 @@ func (*Converter) convertExternalAuthConfigToStrategy(
340339
if externalAuthConfig.Spec.TokenExchange.ClientSecretRef != nil {
341340
// The secret value will be mounted as an environment variable by the deployment controller
342341
// Use the same env var naming convention as the deployment controller
343-
strategy.TokenExchange.ClientSecretEnv = generateUniqueTokenExchangeEnvVarName(externalAuthConfig.Name)
342+
strategy.TokenExchange.ClientSecretEnv = controllerutil.GenerateUniqueTokenExchangeEnvVarName(externalAuthConfig.Name)
344343
}
345344

346345
default:
@@ -880,25 +879,3 @@ func (*Converter) convertOperational(
880879

881880
return operational
882881
}
883-
884-
// generateUniqueTokenExchangeEnvVarName generates a unique environment variable name for token exchange
885-
// client secrets, incorporating the ExternalAuthConfig name to ensure uniqueness.
886-
// This must match the naming convention used by the deployment controller.
887-
func generateUniqueTokenExchangeEnvVarName(configName string) string {
888-
// Sanitize config name for use in env var (uppercase, replace invalid chars with underscore)
889-
sanitized := strings.ToUpper(strings.ReplaceAll(configName, "-", "_"))
890-
// Remove any remaining invalid characters (keep only alphanumeric and underscore)
891-
sanitized = regexp.MustCompile(`[^A-Z0-9_]`).ReplaceAllString(sanitized, "_")
892-
return fmt.Sprintf("TOOLHIVE_TOKEN_EXCHANGE_CLIENT_SECRET_%s", sanitized)
893-
}
894-
895-
// generateUniqueHeaderInjectionEnvVarName generates a unique environment variable name for header injection
896-
// values, incorporating the ExternalAuthConfig name to ensure uniqueness.
897-
// This must match the naming convention used by the deployment controller.
898-
func generateUniqueHeaderInjectionEnvVarName(configName string) string {
899-
// Sanitize config name for use in env var (uppercase, replace invalid chars with underscore)
900-
sanitized := strings.ToUpper(strings.ReplaceAll(configName, "-", "_"))
901-
// Remove any remaining invalid characters (keep only alphanumeric and underscore)
902-
sanitized = regexp.MustCompile(`[^A-Z0-9_]`).ReplaceAllString(sanitized, "_")
903-
return fmt.Sprintf("TOOLHIVE_HEADER_INJECTION_VALUE_%s", sanitized)
904-
}

0 commit comments

Comments
 (0)