Conversation
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughThis PR refactors the Kubernetes gateway-api HTTPRoute template by splitting monolithic auth-server routes into separate service-specific resources. Each service (Auth Server, CASA, Config API, FIDO2, SCIM) now has dedicated public/secure/redirect routes with explicit section references instead of consolidated bundles. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/janssen/charts/gateway-api/templates/route.yaml (1)
17-150:⚠️ Potential issue | 🟠 MajorAuth Server public route may render with empty
rules:array.This HTTPRoute is always created (no outer conditional), but every rule inside
rules:is wrapped in a separate conditional. If all ingress config flags (openidConfigEnabled,deviceCodeEnabled,firebaseMessagingEnabled,uma2ConfigEnabled,webfingerEnabled,webdiscoveryEnabled,u2fConfigEnabled,lockConfigEnabled,authzenConfigEnabled) are disabled, the resulting resource will have an emptyrules:section, which is invalid per the Gateway API spec.Consider wrapping the entire route in a conditional that checks if at least one public endpoint is enabled, similar to how the FIDO2 public route (line 377) uses an
orcondition.🔧 Suggested approach
Add an outer conditional guard before line 17:
+{{- $authPublicEnabled := or (index .Values.global "auth-server" "ingress" "openidConfigEnabled") (index .Values.global "auth-server" "ingress" "deviceCodeEnabled") (index .Values.global "auth-server" "ingress" "firebaseMessagingEnabled") (index .Values.global "auth-server" "ingress" "uma2ConfigEnabled") (index .Values.global "auth-server" "ingress" "webfingerEnabled") (index .Values.global "auth-server" "ingress" "webdiscoveryEnabled") (index .Values.global "auth-server" "ingress" "u2fConfigEnabled") (index .Values.global "auth-server" "ingress" "authzenConfigEnabled") (and (index .Values.global "auth-server" "lockEnabled") (index .Values.global "auth-server" "ingress" "lockConfigEnabled")) -}} +{{- if $authPublicEnabled }} {{- /* Auth Server: Public (HTTP/HTTPS No Redirect) */}} --- apiVersion: gateway.networking.k8s.io/v1 ... +{{- end }}Alternatively, define a helper variable at the top of the file for cleaner conditionals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/janssen/charts/gateway-api/templates/route.yaml` around lines 17 - 150, The HTTPRoute resource named {{ $fullName }}-auth-server-public can render with an empty spec.rules when none of the auth-server ingress flags are true; wrap the entire HTTPRoute block (the resource that defines kind: HTTPRoute and spec: rules) in an outer conditional that checks if at least one of the flags (openidConfigEnabled, deviceCodeEnabled, firebaseMessagingEnabled, uma2ConfigEnabled, webfingerEnabled, webdiscoveryEnabled, u2fConfigEnabled, lockConfigEnabled, authzenConfigEnabled) under .Values.global "auth-server" "ingress" (or compute a helper variable at the top) is true before emitting the resource so spec.rules is never empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@charts/janssen/charts/gateway-api/templates/route.yaml`:
- Around line 17-150: The HTTPRoute resource named {{ $fullName
}}-auth-server-public can render with an empty spec.rules when none of the
auth-server ingress flags are true; wrap the entire HTTPRoute block (the
resource that defines kind: HTTPRoute and spec: rules) in an outer conditional
that checks if at least one of the flags (openidConfigEnabled,
deviceCodeEnabled, firebaseMessagingEnabled, uma2ConfigEnabled,
webfingerEnabled, webdiscoveryEnabled, u2fConfigEnabled, lockConfigEnabled,
authzenConfigEnabled) under .Values.global "auth-server" "ingress" (or compute a
helper variable at the top) is true before emitting the resource so spec.rules
is never empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 557a1bb3-7ba9-4e54-8cb3-c76a9b62e40e
📒 Files selected for processing (1)
charts/janssen/charts/gateway-api/templates/route.yaml
closes #13582
Summary by CodeRabbit