fix: Add ingress for grafana report#121
Conversation
Summary by CodeRabbit
WalkthroughA new Helm template file is added that conditionally renders three NGINX Ingress resources for Grafana integration. These ingresses validate query parameters, manipulate cookies, inject request headers, and route traffic to backend and frontend services with SSL enforcement and regex-based path matching. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@kubernetes/report-ui/templates/refelx-grafana-ingress.yaml`:
- Line 1: The file name contains a typo: rename the template file from
refelx-grafana-ingress.yaml to reflex-grafana-ingress.yaml to match the project
naming convention; update any references to this template elsewhere in the
chart/manifest (e.g., Chart templates, values, or include references) so they
point to reflex-grafana-ingress.yaml and ensure the template internals (the
$basehref variable) remain unchanged.
- Around line 133-134: The ingress path entry using the regex path "path: \"{{
$basehref }}/report/grafana/(.*)\"" currently sets "pathType: Prefix" which is
incorrect for regex paths; update that path's "pathType" to
"ImplementationSpecific" (matching the other backend ingress entry that uses
use-regex: \"true\") so the regex capture group is handled correctly and
consistent behavior is maintained with the existing backend ingress
configuration.
- Around line 61-74: The ingress rule using the regex path "{{ $basehref
}}/grafana-redirect-report-nmaa(/|$)(.*)" is missing the nginx annotation to
enable regex and uses the wrong pathType; update the
grafana-redirect-report-ingress resource by adding the annotation
nginx.ingress.kubernetes.io/use-regex: "true" alongside the existing annotations
and change pathType from Prefix to ImplementationSpecific for the path entry so
NGINX will evaluate the regex correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31754222-d796-4380-acb7-749c26b5a31f
📒 Files selected for processing (1)
kubernetes/report-ui/templates/refelx-grafana-ingress.yaml
| @@ -0,0 +1,136 @@ | |||
| {{- $basehref := .Values.backend.config.basehref | default "" -}} | |||
There was a problem hiding this comment.
Filename typo: refelx should be reflex.
The file is named refelx-grafana-ingress.yaml but based on the naming convention of other files in this directory (reflex-backend-svc.yaml, reflex-frontend-svc.yaml), it should be reflex-grafana-ingress.yaml.
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kubernetes/report-ui/templates/refelx-grafana-ingress.yaml` at line 1, The
file name contains a typo: rename the template file from
refelx-grafana-ingress.yaml to reflex-grafana-ingress.yaml to match the project
naming convention; update any references to this template elsewhere in the
chart/manifest (e.g., Chart templates, values, or include references) so they
point to reflex-grafana-ingress.yaml and ensure the template internals (the
$basehref variable) remain unchanged.
| nginx.ingress.kubernetes.io/force-ssl-redirect: "true" | ||
| name: grafana-redirect-report-ingress | ||
| spec: | ||
| ingressClassName: {{ .Release.Name }} | ||
| rules: | ||
| - http: | ||
| paths: | ||
| - backend: | ||
| service: | ||
| name: {{ .Chart.Name }}-frontend | ||
| port: | ||
| number: {{ .Values.frontend.service.port }} | ||
| path: "{{ $basehref }}/grafana-redirect-report-nmaa(/|$)(.*)" | ||
| pathType: Prefix |
There was a problem hiding this comment.
Missing use-regex annotation and incorrect pathType for regex path.
The path "{{ $basehref }}/grafana-redirect-report-nmaa(/|$)(.*)" contains regex patterns, but:
- The
nginx.ingress.kubernetes.io/use-regex: "true"annotation is missing (compare to lines 91 and 121) pathType: Prefixshould bepathType: ImplementationSpecificwhen using regex (compare to line 104)
Without these changes, NGINX won't interpret the regex and the path matching will fail.
🐛 Proposed fix
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
+ nginx.ingress.kubernetes.io/use-regex: "true"
name: grafana-redirect-report-ingress
spec:
ingressClassName: {{ .Release.Name }}
rules:
- http:
paths:
- backend:
service:
name: {{ .Chart.Name }}-frontend
port:
number: {{ .Values.frontend.service.port }}
path: "{{ $basehref }}/grafana-redirect-report-nmaa(/|$)(.*)"
- pathType: Prefix
+ pathType: ImplementationSpecific🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 64-64: too many spaces inside braces
(braces)
[error] 64-64: too many spaces inside braces
(braces)
[error] 70-70: too many spaces inside braces
(braces)
[error] 70-70: too many spaces inside braces
(braces)
[error] 72-72: too many spaces inside braces
(braces)
[error] 72-72: too many spaces inside braces
(braces)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kubernetes/report-ui/templates/refelx-grafana-ingress.yaml` around lines 61 -
74, The ingress rule using the regex path "{{ $basehref
}}/grafana-redirect-report-nmaa(/|$)(.*)" is missing the nginx annotation to
enable regex and uses the wrong pathType; update the
grafana-redirect-report-ingress resource by adding the annotation
nginx.ingress.kubernetes.io/use-regex: "true" alongside the existing annotations
and change pathType from Prefix to ImplementationSpecific for the path entry so
NGINX will evaluate the regex correctly.
| path: "{{ $basehref }}/report/grafana/(.*)" | ||
| pathType: Prefix |
There was a problem hiding this comment.
Incorrect pathType for regex path.
The path contains a regex capture group (.*) and the ingress has use-regex: "true", but pathType: Prefix should be pathType: ImplementationSpecific for consistency with the backend ingress at line 104.
🔧 Proposed fix
path: "{{ $basehref }}/report/grafana/(.*)"
- pathType: Prefix
+ pathType: ImplementationSpecific📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| path: "{{ $basehref }}/report/grafana/(.*)" | |
| pathType: Prefix | |
| path: "{{ $basehref }}/report/grafana/(.*)" | |
| pathType: ImplementationSpecific |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@kubernetes/report-ui/templates/refelx-grafana-ingress.yaml` around lines 133
- 134, The ingress path entry using the regex path "path: \"{{ $basehref
}}/report/grafana/(.*)\"" currently sets "pathType: Prefix" which is incorrect
for regex paths; update that path's "pathType" to "ImplementationSpecific"
(matching the other backend ingress entry that uses use-regex: \"true\") so the
regex capture group is handled correctly and consistent behavior is maintained
with the existing backend ingress configuration.
1 participant
No description provided.