Skip to content

MCPRemoteProxy: Add remaining configuration validations#4037

Merged
ChrisJBurns merged 10 commits intomainfrom
cburns/2289-remaining-validations
Mar 16, 2026
Merged

MCPRemoteProxy: Add remaining configuration validations#4037
ChrisJBurns merged 10 commits intomainfrom
cburns/2289-remaining-validations

Conversation

@ChrisJBurns
Copy link
Collaborator

@ChrisJBurns ChrisJBurns commented Mar 6, 2026

Summary

Extends #4024 with the four remaining MCPRemoteProxy configuration validations, each surfacing errors via Kubernetes Events and Status Conditions:

  • Remote URL format validation — rejects malformed URLs or unsupported schemes (ftp://, empty host)
  • JWKS URL scheme validation — JWKS endpoints must use HTTPS (key material transport)
  • Cedar authorization policy syntax — validates inline Cedar policies parse correctly before deployment
  • ConfigMap/Secret reference existence — verifies referenced authz ConfigMaps and header Secrets exist in the namespace

All validations follow the foundation pattern from #4024: fail-fast in validateSpec(), set ConfigurationValid=False condition with a specific reason, emit a Warning event, and move the proxy to Failed phase.

No network calls are made — URL validations are format/scheme checks only; ConfigMap/Secret checks are in-cluster reads.

Closes #2289

Test plan

  • Unit tests for ValidateCedarPolicies (6 cases)
  • Unit tests for ValidateRemoteURL (6 cases) and ValidateJWKSURL (5 cases)
  • Unit tests for reconciler condition-setting (5 new cases in TestValidateSpecConfigurationConditions)
  • Integration tests for status conditions (remote URL, JWKS URL, Cedar syntax, missing ConfigMap, missing Secret)
  • Integration tests for event emission (Cedar syntax, missing ConfigMap, missing Secret)
  • go build ./cmd/thv-operator/... passes
  • golangci-lint passes on changed packages

🤖 Generated with Claude Code

Add four new validations to the MCPRemoteProxy controller, each
surfacing errors via Kubernetes Events and Status Conditions:

- Remote URL format validation (scheme and host)
- JWKS URL scheme validation (must use HTTPS)
- Cedar authorization policy syntax validation
- ConfigMap/Secret reference existence checks

Includes unit tests for all validation functions, reconciler condition
tests, and integration tests for both conditions and event emission.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the size/L Large PR: 600-999 lines changed label Mar 6, 2026
@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 69.17293% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.78%. Comparing base (ba38a12) to head (30a088a).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
...-operator/controllers/mcpremoteproxy_controller.go 64.07% 23 Missing and 14 partials ⚠️
cmd/thv-operator/pkg/validation/url_validation.go 81.81% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4037      +/-   ##
==========================================
- Coverage   68.88%   68.78%   -0.11%     
==========================================
  Files         461      463       +2     
  Lines       46562    46701     +139     
==========================================
+ Hits        32075    32124      +49     
- Misses      11987    12002      +15     
- Partials     2500     2575      +75     

☔ View full report in Codecov by Sentry.
📢 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 ChrisJBurns changed the title MCPRemoteProxy: Add remaining configuration validations Draft: MCPRemoteProxy: Add remaining configuration validations Mar 6, 2026
@ChrisJBurns ChrisJBurns changed the title Draft: MCPRemoteProxy: Add remaining configuration validations MCPRemoteProxy: Add remaining configuration validations Mar 8, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 8, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 9, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 9, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 9, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 10, 2026
Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits from going through the validation additions. Nothing blocking — the overall structure is solid.

ChrisJBurns and others added 2 commits March 13, 2026 15:29
- Add failValidation helper to consolidate the repeated
  recordValidationEvent → setConfigurationInvalidCondition → return
  pattern across validateSpec
- Remove redundant early empty-URL check; ValidateRemoteURL already
  handles empty strings and now goes through failValidation
- Fix external auth config block to emit events and set the
  ConfigurationValid condition on failure (was silently missing both)
- Replace fmt.Errorf("%s", msg) with errors.New(msg) where no wrapping
  is needed
- Sanitize non-NotFound K8s API errors so internal details (API server
  URLs, RBAC info) are logged but not exposed in status conditions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 13, 2026
@github-actions github-actions bot added the size/L Large PR: 600-999 lines changed label Mar 13, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 13, 2026
The RemoteURL field has a kubebuilder Pattern validation (^https?://)
that rejects non-HTTP schemes at the API server level. Update the
integration test to verify the CRD rejects the resource at creation
time rather than waiting for the controller to set a condition.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 13, 2026
@ChrisJBurns
Copy link
Collaborator Author

@jhrozek Give it another read on the latest commits, should have addressed your feedback

@ChrisJBurns ChrisJBurns merged commit f96a383 into main Mar 16, 2026
44 of 45 checks passed
@ChrisJBurns ChrisJBurns deleted the cburns/2289-remaining-validations branch March 16, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCPRemoteProxy: Improve error reporting via Kubernetes Events instead of crash loops

2 participants