Validate MCPOIDCConfig inline issuer and JWKS URLs

[lopster568]

Summary

MCPOIDCConfig resources with a malformed or plain-HTTP issuer or jwksUrl are accepted today. The reconciler's only spec gate, MCPOIDCConfig.Validate(), checks type/config consistency but never inspects the URLs, so a bad issuer surfaces later as an opaque OIDC verification failure. The equivalent checks ran for inline OIDC configs on MCPRemoteProxy until #4820 removed that field.

• Wire the existing validation.ValidateOIDCIssuerURL() and validation.ValidateJWKSURL() into the MCPOIDCConfig controller through a new validateOIDCConfigSpec() helper.
• The checks run in the controller rather than in Validate() because pkg/validation imports the v1beta1 package, so the types package cannot call it without an import cycle. This mirrors MCPRemoteProxy's validateSpec.
• spec.inline.insecureAllowHTTP maps to the issuer validator's allowInsecure, so HTTP issuers that opt in keep working.
• Add insecureAllowHTTP: true to deploy/keycloak/mcpserver-with-auth.yaml, whose plain-HTTP issuer would otherwise fail the new check.

Fixes #4823

Type of change

• Bug fix

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

TestValidateOIDCConfigSpec covers issuer and JWKS accept/reject cases including the insecureAllowHTTP opt-in and the empty-JWKS discovery case. TestMCPOIDCConfigReconciler_URLValidationFailureSetsCondition asserts a bad URL sets Valid=False. Existing MCPOIDCConfig controller tests pass unmodified.

Two pkg/api tests (TestNewServer_ReadTimeoutConfigured, TestSecurityHeaders) fail on my machine for want of a container runtime, and task lint-fix reports one gosec finding in cmd/thv/app/upgrade.go. Both reproduce unchanged on main and neither touches this PR's files.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

No CRD schema change; this is controller-level validation only.

Does this introduce a user-facing change?

Yes. Inline configs with a malformed issuer, an HTTP issuer without insecureAllowHTTP: true, or a non-HTTPS JWKS URL now get Valid=False at reconcile time instead of failing later during OIDC verification. Stored objects previously accepted with such values flip to invalid on their next reconcile.

Special notes for reviewers

Scope is limited to inline configs. kubernetesServiceAccount configs default their issuer to the in-cluster URL and have no insecureAllowHTTP escape hatch, so validating them here could break clusters using custom internal issuers. Happy to follow up separately.

deploy/keycloak/mcpserver-with-auth.yaml's jwksUrl is plain HTTP and ValidateJWKSURL requires HTTPS with no escape hatch, so that manifest still fails validation after this PR. Dropping the explicit jwksUrl would fall back to discovery, but its comment says the explicit URL exists to avoid discovery issues. Which would you prefer?

[ChrisJBurns]

Review summary

Thanks for the PR @lopster568 !

Small, focused operator-validation fix — wires the existing ValidateOIDCIssuerURL / ValidateJWKSURL into the MCPOIDCConfig reconciler via a new validateOIDCConfigSpec() helper. The approach is sound and idiomatic: it correctly routes bad URLs through the terminal-error path (Valid=False, no requeue), preserves idempotency (the helper is a pure read-only spec check that adds no writes), and its controller-package placement over v1beta1.Validate() is properly justified — pkg/validation imports the v1beta1 package, so calling it from the types package would create an import cycle. Test coverage of the accept/reject matrix is thorough.

The one item that should gate merge is that the companion manifest change is incomplete: the shipped MCPOIDCConfig example is now rejected by the very validation this PR adds (its plain-HTTP jwksUrl has no insecureAllowHTTP escape hatch). You already flagged this in the PR description — it's an open design decision to resolve, not a hidden defect. The remaining inline comment is a low-severity test-robustness nit.

Severity	Finding	File
HIGH	Shipped example manifest is rejected by the PR's own new JWKS validation	deploy/keycloak/mcpserver-with-auth.yaml
MEDIUM	JWKS is HTTPS-only with no insecureAllowHTTP opt-in while the issuer has one (root cause of the above)	cmd/thv-operator/controllers/mcpoidcconfig_controller.go
LOW	Reconcile test asserts on the brittle substring "HTTP scheme"	cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go

Recommendation: COMMENT — the reconciler change is correct and well-tested; resolve the manifest question (and document the JWKS asymmetry intent) before merge.

_{Reviewed with a multi-agent pass (operator-correctness, security/validation, tests/quality). Codex cross-review skipped (CLI not installed).}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.44%. Comparing base (ac16484) to head (c2ee8cf).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5936      +/-   ##
==========================================
+ Coverage   72.40%   72.44%   +0.03%     
==========================================
  Files         733      733              
  Lines       75845    75858      +13     
==========================================
+ Hits        54918    54954      +36     
+ Misses      17034    17016      -18     
+ Partials     3893     3888       -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ChrisJBurns]

CI failure: Operator CI / Operator Tests Integration — caused by the new URL validation

The Operator Tests Integration job is red (the plain Operator Tests job passes). All 5 failures are in the mcp-remote-proxy integration suite and trace back to the validation this PR adds:

[FAIL] MCPRemoteProxy OIDC CA bundle … validates the CABundleRef and mounts the ConfigMap into the Deployment
[FAIL] MCPRemoteProxy OIDC CA bundle … sets CABundleRefValidated to False with the NotFound reason
[FAIL] MCPRemoteProxy AuthServerRef … authServerRef pointing to embeddedAuthServer
[FAIL] MCPRemoteProxy AuthServerRef … authServerRef (embeddedAuthServer) + externalAuthConfigRef (awsSts)
[FAIL] MCPRemoteProxy AuthServerRef … externalAuthConfigRef only (no authServerRef)

FAIL! -- 44 Passed | 5 Failed | 0 Pending | 0 Skipped

Root cause

Each failure bottoms out in the exact error string this PR introduced:

MCPOIDCConfig legacy-oidc is not valid: OIDC issuer URL "http://localhost:9090"
uses HTTP scheme, which is insecure; use HTTPS or set insecureAllowHTTP: true for development only

The integration fixtures create inline MCPOIDCConfig objects with an HTTP issuer and no insecureAllowHTTP: true. Before this PR the reconciler only ran Validate() (type/config consistency, no URL check), so those fixtures reconciled cleanly. Now validateOIDCConfigSpec rejects the HTTP issuer → the config goes Valid=False → the referencing MCPRemoteProxy never reaches its expected condition/runconfig → the Its time out after 30s (k8s_helpers.go:61).

This is the "previously-accepted HTTP-issuer configs now flip to Valid=False" behavior change surfacing on the test fixtures. Note ValidateOIDCIssuerURL has no localhost carve-out, so http://localhost:9090 is rejected like any other HTTP host.

Fix — two fixture helpers build an inline MCPOIDCConfig with Issuer: "http://localhost:9090" and are missing the opt-in:

File	Helper
cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_authserverref_integration_test.go	newMCPOIDCConfig (issuer ~L291)
cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_cabundle_integration_test.go	newMCPOIDCConfigWithCABundle (issuer ~L28)

Adding InsecureAllowHTTP: true to those two InlineOIDCSharedConfig literals clears all 5 failures.

(The http://localhost:9090 at mcpremoteproxy_authserverref_integration_test.go:254 is a MCPExternalAuthConfig.EmbeddedAuthServer issuer — a different type/path not covered by this PR's validation — so it does not need changing.)

The deploy/keycloak manifest was updated for the new validation, but these operator integration fixtures exercising the same reconcile path were missed.

[lopster568]

Fixed in b1c6dfd: both fixture helpers now set InsecureAllowHTTP: true. Ran the mcp-remote-proxy suite locally under envtest and all specs pass. Thanks for tracing it to the exact helpers, that saved me the dig.

Also merged main in and resolved a conflict in mcpoidcconfig_controller_test.go from #5631, keeping the URL validation tests here and taking the referenceCount removal. Branch is mergeable again.