Standardize secret references to use custom SecretKeyRef across all CRDs
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 300
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 184
Description
Summary
MCPRegistry uses corev1.SecretKeySelector for 5 secret reference fields while every other CRD (MCPServer, MCPOIDCConfig, MCPExternalAuthConfig, MCPRemoteProxy, MCPTelemetryConfig) uses a custom SecretKeyRef type. This should be standardized before the API stabilizes.
The custom SecretKeyRef is the right target because:
- Already used by 5 of 6 CRDs (13+ fields vs 5)
- Simpler — just
Name+Key, both required, no unusedOptionalfield corev1.SecretKeySelectorexposes anOptional *boolfield that no controller respects — thesecrets.GetValue()function always fails if the secret is missing regardless of that flag
Custom SecretKeyRef definition
Already defined in cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types.go (lines 589-598):
type SecretKeyRef struct {
// Name is the name of the secret
// +kubebuilder:validation:Required
Name string `json:"name"`
// Key is the key within the secret
// +kubebuilder:validation:Required
Key string `json:"key"`
}
What to change
1. CRD type definitions (cmd/thv-operator/api/v1alpha1/mcpregistry_types.go)
Convert these 5 fields from corev1.SecretKeySelector to the custom SecretKeyRef type (from mcpexternalauthconfig_types.go). Since all types are in the same package, no import is needed.
| Line | Struct | Field | Current type | New type |
|---|---|---|---|---|
| ~181 | GitAuthConfig |
PasswordSecretRef |
corev1.SecretKeySelector |
SecretKeyRef |
| ~347 | MCPRegistryDatabaseConfig |
DBAppUserPasswordSecretRef |
corev1.SecretKeySelector |
SecretKeyRef |
| ~354 | MCPRegistryDatabaseConfig |
DBMigrationUserPasswordSecretRef |
corev1.SecretKeySelector |
SecretKeyRef |
| ~444 | MCPRegistryOAuthProviderConfig |
ClientSecretRef |
*corev1.SecretKeySelector |
*SecretKeyRef |
| ~461 | MCPRegistryOAuthProviderConfig |
AuthTokenRef |
*corev1.SecretKeySelector |
*SecretKeyRef |
Note: preserve pointer vs value semantics — ClientSecretRef and AuthTokenRef are pointers (optional), the others are values (required).
Important: Do NOT remove the corev1 import from this file — it's still used for corev1.ConfigMapKeySelector (lines ~86, ~450) and corev1.LocalObjectReference (line ~625).
2. Secrets client (cmd/thv-operator/pkg/kubernetes/secrets/secrets.go)
Change the GetValue method signature at line 50 from:
func (c *Client) GetValue(ctx context.Context, namespace string, secretRef corev1.SecretKeySelector) (string, error) {
to accept name and key strings directly:
func (c *Client) GetValue(ctx context.Context, namespace, name, key string) (string, error) {
Update the body (lines 51, 56, 58) to use name and key instead of secretRef.Name and secretRef.Key. This decouples the shared utility from any specific ref type.
3. Callers of GetValue (cmd/thv-operator/pkg/registryapi/pgpass.go)
Only 2 call sites (lines ~36 and ~43). Update from:
m.kubeHelper.Secrets.GetValue(ctx, mcpRegistry.Namespace, dbConfig.DBAppUserPasswordSecretRef)
to:
m.kubeHelper.Secrets.GetValue(ctx, mcpRegistry.Namespace, dbConfig.DBAppUserPasswordSecretRef.Name, dbConfig.DBAppUserPasswordSecretRef.Key)
Same for DBMigrationUserPasswordSecretRef.
4. Config builder (cmd/thv-operator/pkg/registryapi/config/config.go)
Two helper functions accept *corev1.SecretKeySelector — change them to *SecretKeyRef:
buildSecretFilePath(line ~764): change parameter from*corev1.SecretKeySelectorto*mcpv1alpha1.SecretKeyRef(or just*SecretKeyRefif in same package). The body accesses.Nameand.Keywhich exist on both types — no logic changes needed.buildGitPasswordFilePath(line ~549): same change.
Also update field access at lines ~533, ~537 in buildGitAuthConfig — these access auth.PasswordSecretRef.Name and .Key which work identically on the custom type.
5. PodTemplateSpec builder (cmd/thv-operator/pkg/registryapi/podtemplatespec.go)
WithGitAuthMount function (line ~350): change parameter from corev1.SecretKeySelector to the custom type. The body accesses .Name and .Key — no logic changes needed.
6. Test files
Update all test fixtures that construct corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "..."}, Key: "..."} to the simpler SecretKeyRef{Name: "...", Key: "..."}:
cmd/thv-operator/pkg/registryapi/pgpass_test.go(~lines 259-265)cmd/thv-operator/pkg/registryapi/config/config_test.go(~lines 498, 546, 584, 622, 1676, 2179, 2231)cmd/thv-operator/pkg/kubernetes/secrets/secrets_test.go(~lines 143, 166, 201, 246, 280) — update to passname, keystrings instead ofSecretKeySelector
7. Regenerate
After all changes:
task gen # Regenerates deepcopy and CRD manifests
task crdref-gen # Regenerates CRD API docs
task lint-fix # Fix any lint issues
task lint # Verify clean
task test # Unit tests pass
The deepcopy code in zz_generated.deepcopy.go will regenerate automatically — do not edit it manually.
Notes
- The JSON wire format is identical for both types (
{"name": "...", "key": "..."}) so existing manifests continue to work without changes. - The
Optionalfield fromcorev1.SecretKeySelectorwas never checked by any controller — removing it has no behavioral impact. - The custom
SecretKeyReftype is defined inmcpexternalauthconfig_types.gobut is used across multiple CRDs. Consider whether it should be moved to a shared types file in the future, but that's out of scope for this issue.
Generated with Claude Code
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with SecretKeyRef in cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types.go and the five fields in mcpregistry_types.go. Update the GetValue callers and builders in pkg/kubernetes/secrets, pkg/registryapi, and its config and podtemplatespec packages, then update the listed tests. Run task gen, task crdref-gen, task lint-fix, task lint, and task test; done means generated artifacts and tests are clean.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- backend-api-design, infrastructure
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100