Skip to content

Conversation

@tgrunnagle
Copy link
Contributor

@tgrunnagle tgrunnagle commented Feb 11, 2026

Closes #3792

Summary

Adds Kubernetes CRD types for configuring Redis storage in the embedded authorization server. This defines the declarative schema that operators will use to configure Redis-backed storage for horizontal scaling, building on the core Redis storage backend (#3628) and integration tests (#3629). The controller and runner integration that consumes these types is tracked separately in #3630.

Changes Made

CRD Types (mcpexternalauthconfig_types.go)

  • Added AuthServerStorageConfig with type field supporting memory (default) and redis backends
  • Added RedisStorageConfig with Sentinel configuration, ACL user authentication, and configurable timeouts (dial, read, write)
  • Added RedisSentinelConfig with masterName, sentinelAddrs (explicit addresses), and sentinelService (Kubernetes Service discovery) — mutually exclusive
  • Added SentinelServiceRef for referencing a Kubernetes Service for Sentinel discovery (name, namespace, port)
  • Added RedisACLUserConfig with SecretKeyRef references for username and password
  • Added Storage field to EmbeddedAuthServerConfig

Validation Webhooks (mcpexternalauthconfig_webhook.go)

  • Added validateStorageConfig enforcing type-specific configuration presence
  • Added validateRedisStorageConfig requiring sentinelConfig and aclUserConfig, plus Go duration format validation for timeout fields
  • Added validateRedisSentinelConfig enforcing exactly one of sentinelAddrs or sentinelService
  • Added validateRedisACLUserConfig requiring both secret references
  • Integrated storage validation into existing validateEmbeddedAuthServer flow

Generated Files

  • Updated zz_generated.deepcopy.go with DeepCopy methods for all new types
  • Updated CRD manifests in deploy/charts/operator-crds/ (both files/ and templates/)

Implementation Details

  • Simplified the issue's proposed design by removing deploymentMode and authType enum fields — since only sentinel and aclUser are supported, these are implicit in the type structure rather than configurable enums
  • Follows existing patterns: SecretKeyRef for secret references, kubebuilder validation annotations for enums and defaults, optional fields with sensible defaults

Testing

  • Added TestMCPExternalAuthConfig_ValidateStorageConfig with 17 test cases covering:
    • Default/explicit memory storage, invalid memory+redis combinations
    • Valid Redis with sentinel addrs and sentinel service ref
    • Missing required fields (sentinel config, master name, ACL config, secret refs)
    • Mutual exclusivity of sentinelAddrs vs sentinelService (both set, neither set)
    • Empty sentinel service name
    • Valid and invalid Go duration timeout strings (dial, read, write)
  • All tests are parallel and use table-driven patterns consistent with existing tests

Additional Notes

Large PR Justification

  • Isolated CRD updates and corresponding webhooks/tests only

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Feb 11, 2026
@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 96.15385% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.76%. Comparing base (c8f2254) to head (b3c4bb3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ator/api/v1alpha1/mcpexternalauthconfig_webhook.go 96.15% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3793      +/-   ##
==========================================
- Coverage   66.78%   66.76%   -0.02%     
==========================================
  Files         437      437              
  Lines       43002    43057      +55     
==========================================
+ Hits        28718    28748      +30     
- Misses      12078    12105      +27     
+ Partials     2206     2204       -2     

☔ 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.

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Feb 11, 2026
@tgrunnagle tgrunnagle marked this pull request as ready for review February 11, 2026 21:55
// validateStorageConfig validates the auth server storage configuration
func validateStorageConfig(cfg *AuthServerStorageConfig) error {
switch cfg.Type {
case "memory", "":
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: constants would be nicer

UpstreamProviders []UpstreamProviderConfig `json:"upstreamProviders"`

// Storage configures the storage backend for the embedded auth server.
// If not specified, defaults to in-memory storage.
Copy link
Contributor

Choose a reason for hiding this comment

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

just a note, we should probably mention this in the docs, along with howto (just the thv part of it) on setting up the redis storage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auth Server: Add Operator CRD Types for Redis Storage

2 participants