MCPRemoteProxy: Add remaining configuration validations#4037
Open
ChrisJBurns wants to merge 10 commits intomainfrom
Open
MCPRemoteProxy: Add remaining configuration validations#4037ChrisJBurns wants to merge 10 commits intomainfrom
ChrisJBurns wants to merge 10 commits intomainfrom
Conversation
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>
ChrisJBurns
requested review from
JAORMX,
jerm-dro,
jhrozek and
yrobla
as code owners
Codecov Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
ChrisJBurns
changed the title
ChrisJBurns
changed the title
github-actions
bot
added
size/L
github-actions
bot
added
size/L
github-actions
bot
added
size/L
github-actions
bot
added
size/L
github-actions
bot
added
size/L
jhrozek
reviewed
Mar 11, 2026
Contributor
jhrozek
left a comment
jhrozek
left a comment
There was a problem hiding this comment.
A few nits from going through the validation additions. Nothing blocking — the overall structure is solid.
- 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
bot
added
size/L
github-actions
bot
added
size/L
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
bot
added
size/L
Collaborator
Author
|
@jhrozek Give it another read on the latest commits, should have addressed your feedback |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends #4024 with the four remaining MCPRemoteProxy configuration validations, each surfacing errors via Kubernetes Events and Status Conditions:
ftp://, empty host)All validations follow the foundation pattern from #4024: fail-fast in
validateSpec(), setConfigurationValid=Falsecondition with a specific reason, emit a Warning event, and move the proxy toFailedphase.No network calls are made — URL validations are format/scheme checks only; ConfigMap/Secret checks are in-cluster reads.
Closes #2289
Test plan
ValidateCedarPolicies(6 cases)ValidateRemoteURL(6 cases) andValidateJWKSURL(5 cases)TestValidateSpecConfigurationConditions)go build ./cmd/thv-operator/...passesgolangci-lintpasses on changed packages🤖 Generated with Claude Code