feat(cloud-native): add support for gRPC bridge#2679
Merged
Conversation
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors gateway configuration from flat Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GatewayAPI as "Gateway API (Gateway/HTTPRoute)"
participant Auth as "auth-server (Service)"
participant Lock as "Lock Audit (gRPC)"
Client->>GatewayAPI: HTTP or gRPC request
GatewayAPI->>Auth: route to auth-server (HTTP/GRPC)
Auth->>Lock: gRPC audit call (h2c) [if lockAuditEnabled]
Lock-->>Auth: audit ack
Auth-->>GatewayAPI: response
GatewayAPI-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
mo-auto
added
area-documentation
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu-all-in-one/templates/service.yml`:
- Line 64: Update the inline comment in
charts/gluu-all-in-one/templates/service.yml to accurately state that we're
avoiding using the same service port (not pod port) so it's clear the conflict
is at the gateway/service level; edit the comment near the service port line
(currently showing port: 50051 and targetPort: {{ .Values.service.port }}) to
read something like "avoid using the same service port as some gateway
implementations don't like shared service ports for gRPC and non-gRPC" so it
references the service port concept that matches targetPort: {{
.Values.service.port }}.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
charts/gluu-all-in-one/README.mdcharts/gluu-all-in-one/templates/configmap.yamlcharts/gluu-all-in-one/templates/deployment.ymlcharts/gluu-all-in-one/templates/gateway-api.yamlcharts/gluu-all-in-one/templates/service.ymlcharts/gluu-all-in-one/values.yamlcharts/gluu/README.mdcharts/gluu/charts/auth-server/templates/service.ymlcharts/gluu/charts/config/templates/configmaps.yamlcharts/gluu/templates/gateway-api.yamlcharts/gluu/values.yamldocker-admin-ui/Dockerfiledocker-persistence-loader/Dockerfile
🔥 Files not summarized due to errors (6)
- charts/gluu/charts/auth-server/templates/service.yml: Error: Server error: no LLM provider could handle the message
- charts/gluu-all-in-one/templates/gateway-api.yaml: Error: Server error: no LLM provider could handle the message
- charts/gluu-all-in-one/templates/configmap.yaml: Error: Server error: no LLM provider could handle the message
- charts/gluu/README.md: Error: Server error: no LLM provider could handle the message
- docker-admin-ui/Dockerfile: Error: Server error: no LLM provider could handle the message
- charts/gluu-all-in-one/templates/service.yml: Error: Server error: no LLM provider could handle the message
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu-all-in-one/templates/gateway-api.yaml`:
- Around line 458-470: Add a note to the chart README stating that the chart
requires Gateway API v1.1.0 or newer because the templates use
gateway.networking.k8s.io/v1 resources (GRPCRoute/HTTPRoute) such as the
GRPCRoute generated in gateway-api.yaml and the conditional on
.Values.gatewayApi.gatewayClassName; explicitly call out "Gateway API v1.1.0+"
as the minimum version and include a short sentence about potential failures on
older Gateway API versions and where to check the installed Gateway API version.
In `@charts/gluu/templates/gateway-api.yaml`:
- Around line 295-300: The added URLRewrite filter (filters -> type: URLRewrite
-> urlRewrite -> path -> type: ReplacePrefixMatch -> replacePrefixMatch: /)
strips the /jans-auth prefix before forwarding; confirm the auth-server backend
(auth-server service / ingress) is configured to accept requests without the
/jans-auth prefix and, if not, either remove this URLRewrite filter or change
replacePrefixMatch to the explicit prefix '/jans-auth' so requests remain
unchanged; locate the URLRewrite block in gateway-api.yaml and either delete it
or update replacePrefixMatch and/or update the auth-server routing/config to
accept pathless requests, then re-run integration tests to validate behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
charts/gluu-all-in-one/templates/gateway-api.yamlcharts/gluu/templates/gateway-api.yaml
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu/charts/gateway-api/Chart.yaml`:
- Around line 1-2: The Chart.yaml in the child chart currently sets name:
nginx-ingress which conflicts with the parent chart dependency entry expecting
gateway-api; update the child Chart.yaml (the name field) to exactly match the
dependency name 'gateway-api' so Helm dependency resolution succeeds, and verify
the chart's apiVersion and other metadata remain unchanged; ensure any
references to the old name (packaging/CI configs) are also updated to
'gateway-api'.
In `@charts/gluu/charts/gateway-api/templates/_helpers.tpl`:
- Around line 14-25: The current helper gateway-api.fullname truncates the base
name before adding suffixes, causing labels in gateway.yaml and route.yaml
(where "-gateway" / "-routes" are appended) to exceed Kubernetes' 63-char label
limit; add a new helper (e.g., gateway-api.appLabel) that accepts the chart
context and a suffix, builds the full name by concatenating .Release.Name and
.Chart.Name (or .Values.nameOverride), appends the provided suffix, then
truncates the final string to 63 chars and trimSuffix "-" so the suffix is
preserved and the result is label-safe; then replace the label renderings in the
templates (gateway.yaml and route.yaml) to call {{ include
"gateway-api.appLabel" (dict "context" . "suffix" "gateway") }} or "routes" as
appropriate.
In `@charts/gluu/values.yaml`:
- Around line 999-1000: Update the values.yaml comment to state that
lockAuditEnabled also requires global.auth-server.lockEnabled to be true;
reference the template that gates rendering
(charts/gluu/charts/gateway-api/templates/route.yaml) and the two flags
(lockAuditEnabled and global.auth-server.lockEnabled) so users know the gRPC
/io.jans.lock.audit.AuditService route is only created when both flags are
enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd385ff9-4c82-4109-b04a-a6dbeea184be
📒 Files selected for processing (13)
charts/gluu-all-in-one/templates/service.ymlcharts/gluu/Chart.yamlcharts/gluu/README.mdcharts/gluu/charts/auth-server/templates/service.ymlcharts/gluu/charts/gateway-api/Chart.yamlcharts/gluu/charts/gateway-api/README.mdcharts/gluu/charts/gateway-api/templates/_helpers.tplcharts/gluu/charts/gateway-api/templates/gateway.yamlcharts/gluu/charts/gateway-api/templates/ip-pool.yamlcharts/gluu/charts/gateway-api/templates/route.yamlcharts/gluu/charts/gateway-api/values.yamlcharts/gluu/templates/NOTES.txtcharts/gluu/values.yaml
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu-all-in-one/values.yaml`:
- Around line 251-252: The nginx-ingress template still checks the old key name;
update the conditional in templates/nginx-ingress.yaml that reads {{ if and
(index .Values "auth-server" "lockEnabled") (index .Values "auth-server"
"ingress" "lockEnabled") -}} to use the renamed key "lockAuditEnabled" (i.e.,
reference .Values."auth-server"."lockAuditEnabled" and
.Values."auth-server"."ingress"."lockAuditEnabled") so it matches values.yaml
and the other templates (service.yml/gateway-api.yaml) and allows the lock audit
ingress to be created.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac50fe4e-05a7-40df-961c-6fb4e6a98c58
📒 Files selected for processing (4)
charts/gluu-all-in-one/README.mdcharts/gluu-all-in-one/templates/NOTES.txtcharts/gluu-all-in-one/templates/gateway-api.yamlcharts/gluu-all-in-one/values.yaml
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai resume |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu/charts/gateway-api/templates/gateway.yaml`:
- Around line 33-36: The template currently hard-codes a GatewayParameters CR
named "airlock-gw-params" and always renders it; change it so the CR is only
created when the chart is also auto-wiring the parametersRef (i.e., when the
chart is not given gateway.infrastructure.parametersRef externally) and use a
release-scoped name (for example include the release/name template or
.Release.Name) instead of the fixed "airlock-gw-params"; update both the
parametersRef block (where GatewayParameters is referenced) and the
GatewayParameters resource template to conditionally render (with the same
if-test) and to reference the release-scoped name so parallel installs in the
same namespace and user-supplied parametersRef work correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 355136b9-d2f4-4bf7-a580-6a4d32047a12
📒 Files selected for processing (11)
charts/gluu-all-in-one/README.mdcharts/gluu-all-in-one/templates/NOTES.txtcharts/gluu-all-in-one/templates/gateway-api.yamlcharts/gluu-all-in-one/values.yamlcharts/gluu/README.mdcharts/gluu/charts/gateway-api/README.mdcharts/gluu/charts/gateway-api/templates/gateway.yamlcharts/gluu/charts/gateway-api/templates/route.yamlcharts/gluu/openbanking-values.yamlcharts/gluu/templates/NOTES.txtcharts/gluu/values.yaml
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
charts/gluu/charts/gateway-api/templates/route.yaml (1)
214-223:⚠️ Potential issue | 🟠 MajorMove
/adminoff the public route.This rule is attached to
{{ $fullName }}-routes-public, so it binds to the HTTP listener as well as HTTPS. That leaves the admin UI reachable over cleartext instead of following the HTTPS-only/redirect path used for the other interactive apps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu/charts/gateway-api/templates/route.yaml` around lines 214 - 223, The admin UI route for path "/admin" is being added to the public HTTP/HTTPS listener ({{ $fullName }}-routes-public) which exposes it over cleartext; instead remove or stop rendering the matches block under the public route and add the same backendRef (service {{ $adminSvc }} port 8080) to the internal/secure route template (e.g. the non-public routes chart section) so the admin path is only attached to the HTTPS-only/redirect listener; use the existing condition that checks .Values.global "admin-ui" "ingress" "adminUiEnabled" to gate the addition but render it in the secure route template rather than the public one.charts/gluu-all-in-one/templates/gateway-api.yaml (1)
330-339:⚠️ Potential issue | 🟠 MajorKeep
/adminon the HTTPS/redirect path.This route sits under
{{ $fullName }}-routes-public, so the admin UI is served on plain HTTP as well as HTTPS. That is a poor default for an administrative login surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu-all-in-one/templates/gateway-api.yaml` around lines 330 - 339, The /admin ingress path is being added under the public route ({{ $fullName }}-routes-public) which exposes the Admin UI over plain HTTP; change the template so the path value "/admin" (the block that sets path type PathPrefix and backendRefs to {{ $svcName }}:{{ $svcPort }}) is only rendered into the HTTPS/redirect route instead of the public route. Locate the admin UI conditional (if index .Values "admin-ui" "ingress" "adminUiEnabled") in gateway-api.yaml and move or guard the /admin match so it is emitted only when generating the TLS/redirect (HTTPS) route for {{ $fullName }}-routes-public (or add a check for the route being the TLS/redirect route) to ensure /admin is not served on plain HTTP.
♻️ Duplicate comments (1)
charts/gluu/charts/gateway-api/templates/gateway.yaml (1)
32-36:⚠️ Potential issue | 🟠 MajorScope the autogenerated Airlock
GatewayParametersresource to the release and only render it when auto-wired.This is still using a fixed
airlock-gw-paramsname and still emits the CR even whengateway.infrastructure.parametersRefalready points to a user-managed object. That breaks parallel installs in the same namespace and leaves an unused extra resource behind.Suggested fix
{{- $fullName := include "gateway-api.fullname" . -}} +{{- $airlockGwParamsName := printf "%s-airlock-gw-params" $fullName -}} {{- $namespace := .Release.Namespace -}} @@ {{- else if and (eq .Values.gateway.className "airlock-microgateway") (.Values.additionalConfig.airlock.createLbService) }} parametersRef: group: microgateway.airlock.com kind: GatewayParameters - name: airlock-gw-params + name: {{ $airlockGwParamsName }} {{- end }} @@ -{{- if and (eq .Values.gateway.className "airlock-microgateway") (.Values.additionalConfig.airlock.createLbService) }} +{{- if and (eq .Values.gateway.className "airlock-microgateway") (.Values.additionalConfig.airlock.createLbService) (empty .Values.gateway.infrastructure.parametersRef) }} --- @@ metadata: - name: airlock-gw-params + name: {{ $airlockGwParamsName }} namespace: {{ $namespace }}Also applies to: 57-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu/charts/gateway-api/templates/gateway.yaml` around lines 32 - 36, The template unconditionally emits a Cluster-scoped-looking GatewayParameters resource named "airlock-gw-params" and doesn't respect a user-provided gateway.infrastructure.parametersRef, causing name collisions and stray CRs; update the conditional around the GatewayParameters block to only render when .Values.additionalConfig.airlock.createLbService is true AND the user has not set .Values.gateway.infrastructure.parametersRef (or when that ref's name is empty), and scope the generated resource name to the release (e.g. include .Release.Name or .Values.releaseName in the generated name) so the produced GatewayParameters (used when .Values.gateway.className == "airlock-microgateway") is unique per release and only created when auto-wired. Ensure the same change is applied for the other instance referenced at lines 57-69.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu-all-in-one/templates/gateway-api.yaml`:
- Around line 34-39: The template currently hard-codes the namespaced
GatewayParameters resource as "airlock-gw-params" in the parametersRef block
(parametersRef -> group: microgateway.airlock.com, kind: GatewayParameters,
name: airlock-gw-params), which causes conflicts across releases; change the
name to a release-scoped identifier (e.g. include the Helm release name or
release-unique suffix) so each release creates/refs its own GatewayParameters
object—update both occurrences (the block shown and the similar one around lines
65-66) to construct the name from the release (e.g. use .Release.Name or a chart
helper to derive "<release>-airlock-gw-params").
In `@charts/gluu/openbanking-values.yaml`:
- Around line 799-801: Replace occurrences of the legacy key
global.auth-server.ingress.lockEnabled in this values file with the new lock
audit key global.auth-server.ingress.lockAuditEnabled (preserve boolean values
and intended default), updating any example/comment text that refers to the old
flag; search the file for all occurrences (e.g., around the shown block and the
860-918 region) and change them so users enabling the audit gRPC bridge set
lockAuditEnabled instead of the no-op lockEnabled.
---
Outside diff comments:
In `@charts/gluu-all-in-one/templates/gateway-api.yaml`:
- Around line 330-339: The /admin ingress path is being added under the public
route ({{ $fullName }}-routes-public) which exposes the Admin UI over plain
HTTP; change the template so the path value "/admin" (the block that sets path
type PathPrefix and backendRefs to {{ $svcName }}:{{ $svcPort }}) is only
rendered into the HTTPS/redirect route instead of the public route. Locate the
admin UI conditional (if index .Values "admin-ui" "ingress" "adminUiEnabled") in
gateway-api.yaml and move or guard the /admin match so it is emitted only when
generating the TLS/redirect (HTTPS) route for {{ $fullName }}-routes-public (or
add a check for the route being the TLS/redirect route) to ensure /admin is not
served on plain HTTP.
In `@charts/gluu/charts/gateway-api/templates/route.yaml`:
- Around line 214-223: The admin UI route for path "/admin" is being added to
the public HTTP/HTTPS listener ({{ $fullName }}-routes-public) which exposes it
over cleartext; instead remove or stop rendering the matches block under the
public route and add the same backendRef (service {{ $adminSvc }} port 8080) to
the internal/secure route template (e.g. the non-public routes chart section) so
the admin path is only attached to the HTTPS-only/redirect listener; use the
existing condition that checks .Values.global "admin-ui" "ingress"
"adminUiEnabled" to gate the addition but render it in the secure route template
rather than the public one.
---
Duplicate comments:
In `@charts/gluu/charts/gateway-api/templates/gateway.yaml`:
- Around line 32-36: The template unconditionally emits a Cluster-scoped-looking
GatewayParameters resource named "airlock-gw-params" and doesn't respect a
user-provided gateway.infrastructure.parametersRef, causing name collisions and
stray CRs; update the conditional around the GatewayParameters block to only
render when .Values.additionalConfig.airlock.createLbService is true AND the
user has not set .Values.gateway.infrastructure.parametersRef (or when that
ref's name is empty), and scope the generated resource name to the release (e.g.
include .Release.Name or .Values.releaseName in the generated name) so the
produced GatewayParameters (used when .Values.gateway.className ==
"airlock-microgateway") is unique per release and only created when auto-wired.
Ensure the same change is applied for the other instance referenced at lines
57-69.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 13805b0b-c744-4a74-b989-adc8444cddaf
📒 Files selected for processing (11)
charts/gluu-all-in-one/README.mdcharts/gluu-all-in-one/templates/NOTES.txtcharts/gluu-all-in-one/templates/gateway-api.yamlcharts/gluu-all-in-one/values.yamlcharts/gluu/README.mdcharts/gluu/charts/gateway-api/README.mdcharts/gluu/charts/gateway-api/templates/gateway.yamlcharts/gluu/charts/gateway-api/templates/route.yamlcharts/gluu/openbanking-values.yamlcharts/gluu/templates/NOTES.txtcharts/gluu/values.yaml
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
charts/gluu/charts/gateway-api/templates/gateway.yaml (1)
57-69:⚠️ Potential issue | 🟠 MajorSkip the autogenerated Airlock
GatewayParameterswhen a custom ref is already set.Line 57 still emits the chart-managed CR even when Lines 29-31 already use a caller-supplied
gateway.infrastructure.parametersRef. That leaves an unused resource behind and can force an unnecessarymicrogateway.airlock.com/v1alpha1object into installs that provide their own reference.Suggested fix
-{{- if and (eq .Values.gateway.className "airlock-microgateway") (.Values.additionalConfig.airlock.createLbService) }} +{{- if and (eq .Values.gateway.className "airlock-microgateway") (.Values.additionalConfig.airlock.createLbService) (not .Values.gateway.infrastructure.parametersRef) }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu/charts/gateway-api/templates/gateway.yaml` around lines 57 - 69, The autogenerated GatewayParameters CR is emitted unconditionally for the airlock-microgateway when .Values.additionalConfig.airlock.createLbService is true, which duplicates installs that provide their own gateway.infrastructure.parametersRef; update the template condition for the GatewayParameters block (the section creating kind: GatewayParameters named {{ $fullName }}-airlock-gw-params) to only render when no custom parametersRef is supplied by adding a check for the absence of .Values.gateway.infrastructure.parametersRef (i.e., require that .Values.gateway.infrastructure.parametersRef is empty/not set in the same and(...) expression that uses .Values.gateway.className and .Values.additionalConfig.airlock.createLbService) so the CR is skipped when a caller-supplied reference exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu-all-in-one/templates/gateway-api.yaml`:
- Around line 59-71: The template always renders the autogenerated
GatewayParameters CR when createLbService is true, causing duplicate resources
if consumers supply gateway-api.gateway.infrastructure.parametersRef; update the
if-condition that controls creation (the block producing kind: GatewayParameters
named via $fullName and using createLbService) to also require that
gateway-api.gateway.infrastructure.parametersRef is not set (or is empty), i.e.
only render this CR when createLbService is true AND no custom parametersRef is
provided.
---
Duplicate comments:
In `@charts/gluu/charts/gateway-api/templates/gateway.yaml`:
- Around line 57-69: The autogenerated GatewayParameters CR is emitted
unconditionally for the airlock-microgateway when
.Values.additionalConfig.airlock.createLbService is true, which duplicates
installs that provide their own gateway.infrastructure.parametersRef; update the
template condition for the GatewayParameters block (the section creating kind:
GatewayParameters named {{ $fullName }}-airlock-gw-params) to only render when
no custom parametersRef is supplied by adding a check for the absence of
.Values.gateway.infrastructure.parametersRef (i.e., require that
.Values.gateway.infrastructure.parametersRef is empty/not set in the same
and(...) expression that uses .Values.gateway.className and
.Values.additionalConfig.airlock.createLbService) so the CR is skipped when a
caller-supplied reference exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7d389510-2bae-42a2-9968-32c5317d5675
📒 Files selected for processing (3)
charts/gluu-all-in-one/templates/gateway-api.yamlcharts/gluu/charts/gateway-api/templates/gateway.yamlcharts/gluu/openbanking-values.yaml
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: Isman Firmansyah <iromli@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu/charts/gateway-api/templates/gateway.yaml`:
- Around line 19-23: The infrastructure.labels and infrastructure.annotations
rendering can produce `{}` or emit invalid/undesired YAML when values are null
or empty; update the gateway template (the block rendering
.Values.gateway.infrastructure.labels and
.Values.gateway.infrastructure.annotations) to mirror the metadata
labels/annotations conditional pattern: check if
.Values.gateway.infrastructure.labels (and
.Values.gateway.infrastructure.annotations) are defined and not empty before
calling toYaml and nindent, and only render the keys when present, otherwise
omit the labels/annotations block or render an empty, valid structure as needed
to keep YAML valid.
In `@charts/gluu/charts/gateway-api/templates/secret.yaml`:
- Around line 1-9: The Secret template for the gateway CA certificate currently
omits an explicit type; update the Secret manifest in the gateway CA template to
include "type: Opaque" under the Secret metadata (the template that generates
the Secret with kind: Secret and name: {{ .Release.Name }}-gateway-ca-cert) so
the intent is explicit and linters/readers see the type defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 90c4be69-d255-4215-aac5-a9a09aa67a48
📒 Files selected for processing (12)
charts/gluu/README.mdcharts/gluu/charts/gateway-api/README.mdcharts/gluu/charts/gateway-api/templates/gateway.yamlcharts/gluu/charts/gateway-api/templates/impl-airlock.yamlcharts/gluu/charts/gateway-api/templates/impl-cilium.yamlcharts/gluu/charts/gateway-api/templates/impl-envoy.yamlcharts/gluu/charts/gateway-api/templates/impl-nginx.yamlcharts/gluu/charts/gateway-api/templates/route.yamlcharts/gluu/charts/gateway-api/templates/secret.yamlcharts/gluu/charts/gateway-api/values.yamlcharts/gluu/openbanking-values.yamlcharts/gluu/values.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu/openbanking-values.yaml`:
- Around line 141-142: Update the description for the global.lbIp value to
reference Gateway API usage: note that when attachLbIp: true the chart will
attach global.lbIp to the Gateway spec.addresses using IPAddress type (in
addition to any existing notes about nginx/Istio), so change the descriptive
text for global.lbIp to clearly mention Gateway API and the attachLbIp behavior;
look for the global.lbIp description and the attachLbIp key to edit the wording
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10a94838-f75b-408e-964d-e2fae6e88a9a
📒 Files selected for processing (1)
charts/gluu/openbanking-values.yaml
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu/charts/gateway-api/templates/route.yaml`:
- Around line 516-522: The template currently injects a SnippetsFilter when
.Values.gateway.className == "nginx" and
.Values.gateway.verifyClientCertProtection and
.Values.additionalConfig.nginx.enableClientCertSnippets are true but doesn't
validate that the NGINX Snippets CRD exists; add a Helm capability check using
.Capabilities.APIVersions.Has (e.g. check for "gateway.nginx.org/v1alpha1" or
the exact group/version that provides SnippetsFilter) and if
enableClientCertSnippets is true but the API is not present call fail with a
clear message referencing enableClientCertSnippets, the SnippetsFilter
extensionRef name ({{ .Release.Name }}-nginx-verify-client) and that snippet
support must be enabled during NGINX installation so users get a hard validation
rather than a silent missing resource.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fd6cf856-1c4f-481e-8e96-52fb58fec031
📒 Files selected for processing (3)
charts/gluu/charts/gateway-api/templates/gateway.yamlcharts/gluu/charts/gateway-api/templates/route.yamlcharts/gluu/openbanking-values.yaml
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
|
Signed-off-by: Mohammad Abudayyeh <47318409+moabu@users.noreply.github.com>
Member
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved. Auto-approval is disabled; enable |
moabu
approved these changes
Mar 10, 2026
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
3 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.
Support for gRPC bridge.
Closes #2678
Summary by CodeRabbit
New Features
New Config
Documentation