Feature/grafana - #76
Conversation
📝 WalkthroughWalkthroughAdds Grafana as a monitoring component alongside existing Prometheus infrastructure across Docker Compose, the Helm chart (ConfigMaps, Deployment, Service, Secret, ingress, values), Terraform networking rules, Ansible deployment tasks/variables, CI summary output, and documentation. ChangesPrometheus + Grafana Monitoring Rollout
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Ingress as Ingress (grafana path)
participant GrafanaSvc as grafana-service
participant GrafanaPod as bytebite-grafana Deployment
participant PromSvc as prometheus-service
Client->>Ingress: GET /grafana
Ingress->>GrafanaSvc: route request
GrafanaSvc->>GrafanaPod: forward to pod
GrafanaPod->>PromSvc: query metrics (datasource provisioning)
PromSvc-->>GrafanaPod: metrics data
GrafanaPod-->>Client: rendered dashboard
Related PRs: None found. Suggested labels: infrastructure, monitoring, helm, terraform, ansible Suggested reviewers: None found. 🐰 A rabbit hops through charts and yaml streams, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
helm/bytebite/values.yaml (1)
120-122: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
monitoring.grafana.ingress.pathlacks a trailing slash used for sub-path serving.This path value is reused in
ingress.yaml(Ingress rule) and ingrafana-deployment.yamlto buildGF_SERVER_ROOT_URLunderGF_SERVER_SERVE_FROM_SUB_PATH=true. Grafana's documented sub-path examples always end the root URL in/(e.g..../grafana/); omitting it is a known cause of Grafana failing to load static assets behind a reverse-proxy sub-path. See related comment ongrafana-deployment.yamlfor a defensive template-side fix; alternatively set the default here to/grafana/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/bytebite/values.yaml` around lines 120 - 122, The Grafana ingress path default is missing the trailing slash needed for sub-path serving. Update the monitoring.grafana.ingress.path value in values.yaml to use the sub-path form with a trailing slash so it matches its reuse in ingress.yaml and grafana-deployment.yaml when building GF_SERVER_ROOT_URL. Keep the path consistent with the Grafana sub-path configuration used by the chart.
🧹 Nitpick comments (5)
helm/bytebite/dashboards/bytebite-overview.json (1)
1-490: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate dashboard JSON risks drift between local and Helm deployments.
This file is byte-for-byte identical to
monitoring/grafana/dashboards/bytebite-overview.json. Maintaining two independent copies of a 490-line dashboard definition means future panel/query edits must be manually kept in sync in both places, or the Helm-deployed dashboard silently diverges from the local one.Consider generating this file from a single source of truth (e.g., a build/sync script, symlink, or Helm
.Files.Getreference to a shared dashboards directory) so the two never drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/bytebite/dashboards/bytebite-overview.json` around lines 1 - 490, The ByteBite Overview dashboard JSON is duplicated in two places, which can cause the Helm-deployed version to drift from the local version. Consolidate the dashboard definition into a single source of truth and have the Helm dashboard consume it directly, for example via a shared dashboards asset or Helm file reference, so changes to the dashboard title/uid and panel targets only need to be made once.helm/bytebite/templates/prometheus-deployment.yaml (2)
33-38: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPrometheus storage uses
emptyDir— metrics lost on pod restart/reschedule.Any pod restart, node drain, or rescheduling wipes all historical metrics since
prometheus-datais backed byemptyDir. Consider aPersistentVolumeClaimif metric retention across restarts matters for this deployment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/bytebite/templates/prometheus-deployment.yaml` around lines 33 - 38, The Prometheus data volume is currently backed by emptyDir in the prometheus-deployment.yaml template, so metrics are lost on pod restarts or rescheduling. Update the prometheus-data volume configuration to use persistent storage instead, such as a PersistentVolumeClaim, and wire it into the Prometheus deployment spec so the data survives restarts. Use the prometheus-data volume name and the Prometheus deployment template as the main reference points when making the change.
16-38: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd container security context.
Trivy flags this Deployment for using the default (root-capable, writable-root-fs) security context. For a service exposed externally (per the Terraform NSG changes opening port 9090), hardening the container is worthwhile.
🔒 Proposed fix
containers: - name: prometheus image: "{{ .Values.monitoring.prometheus.image.repository }}:{{ .Values.monitoring.prometheus.image.tag }}" imagePullPolicy: {{ .Values.monitoring.prometheus.image.pullPolicy }} + securityContext: + runAsNonRoot: true + readOnlyRootFilesystem: true + allowPrivilegeEscalation: false args: - --config.file=/etc/prometheus/prometheus.yml - --storage.tsdb.path=/prometheus🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/bytebite/templates/prometheus-deployment.yaml` around lines 16 - 38, The prometheus Deployment is missing an explicit container security context, so the container runs with the default root-capable settings. Update the prometheus container in the prometheus-deployment template to add a hardened securityContext for the container, using the prometheus container spec as the anchor. Disable privilege escalation, set a non-root user, and drop unnecessary capabilities; if the image and mounted paths allow it, also mark the root filesystem read-only while keeping the existing /prometheus volume writable.Source: Linters/SAST tools
helm/bytebite/templates/grafana-deployment.yaml (2)
16-58: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a securityContext (non-root, read-only rootfs).
Static analysis flags the Deployment/container as using the default security context (root-capable, writable rootfs). Since
grafana-datais already anemptyDir, addingsecurityContextwithrunAsNonRoot, a fixedrunAsUser/fsGroup(Grafana's official image runs as uid 472), andreadOnlyRootFilesystem: truewould harden this without functional loss.🔒 Suggested hardening
spec: + securityContext: + runAsNonRoot: true + runAsUser: 472 + fsGroup: 472 containers: - name: grafana image: "{{ .Values.monitoring.grafana.image.repository }}:{{ .Values.monitoring.grafana.image.tag }}" imagePullPolicy: {{ .Values.monitoring.grafana.image.pullPolicy }} + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/bytebite/templates/grafana-deployment.yaml` around lines 16 - 58, The Grafana container in the deployment template is missing a hardened securityContext and is still using the default root-capable, writable root filesystem. Update the grafana container spec to add a securityContext with runAsNonRoot, a fixed runAsUser/fsGroup matching Grafana’s official uid 472, and readOnlyRootFilesystem set to true, while keeping the existing grafana-data emptyDir mount usable. Use the grafana container block in grafana-deployment.yaml to apply this without changing the provisioning or volumeMount setup.Source: Linters/SAST tools
56-57: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoffGrafana state is not persisted across restarts.
grafana-data(/var/lib/grafana, DB/session/org state) is anemptyDir, so any manual dashboard edits, users, or annotations created outside provisioning are lost on pod restart/reschedule. Since dashboards/datasources are provisioned via ConfigMap this may be acceptable, but worth calling out for production use.Also applies to: 74-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/bytebite/templates/grafana-deployment.yaml` around lines 56 - 57, Grafana state is currently backed by an ephemeral emptyDir, so manual dashboards/users/annotations are lost on restart; update the grafana-data volume in the grafana-deployment template to use persistent storage instead of emptyDir, or make persistence configurable for production use. Keep the mountPath at /var/lib/grafana and ensure the corresponding volume definition for grafana-data is changed consistently wherever it is referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy-azure.yml:
- Around line 121-124: The summary lines in the deploy-azure workflow are using
inline ${{ }} expressions inside the run script, which triggers
template-injection warnings. Move steps.tf.outputs.public_ip into env for that
step, then reference it via a shell variable in the summary writes instead of
interpolating it directly in the run block. Update the three echo statements
that build the App, Prometheus, and Grafana URLs to use the env-bound value
while keeping the logic in the same workflow step.
In `@helm/bytebite/README.md`:
- Line 35: The Grafana admin password default is documented as cleartext, so
update the chart docs and default configuration around
monitoring.grafana.adminPassword to avoid shipping a known credential. Remove
the committed password default from the README and values-related templates, and
either require an explicit override or generate a random secret via the chart
helpers (for example in _helpers.tpl) so users do not deploy with a predictable
admin password.
In `@helm/bytebite/values.yaml`:
- Around line 118-119: The Grafana admin credentials in values.yaml are
hardcoded defaults, which should not be committed for the real deployment.
Update the chart values around adminUser/adminPassword so only the username
stays as a default and the password must be supplied externally at install time,
then ensure the bytebite-monitoring-secret path still reads from an injected
secret or --set value in monitoring-secret.yaml and any related Grafana config.
In `@infra/ansible/group_vars/bytebite.yml`:
- Around line 15-17: The Grafana defaults in the bytebite group vars are
insecure because they ship a real, guessable admin credential. Update the
Grafana variables in the bytebite vars file to use empty placeholders instead of
a working username/password, and add a fail-fast check in the Ansible flow (for
example near the Grafana setup task) to assert both values are explicitly
provided. Also document the required override in the example deploy vars/README,
ideally pointing users to vault-encrypted secrets, so the playbook never deploys
with a public default login.
In `@infra/ansible/roles/deploy/templates/env.j2`:
- Around line 7-8: The Grafana credential values in the env template still fall
back to weak defaults, duplicating the same fallback in other deploy paths.
Update the env.j2 template to remove the default filters from GRAFANA_ADMIN_USER
and GRAFANA_ADMIN_PASSWORD so the deploy must supply explicit or vaulted values,
and keep the credential handling consistent with the related Grafana config in
the Ansible variables and compose setup.
In `@infra/terraform/main.tf`:
- Around line 6-15: The Grafana and Prometheus ingress rules in the Terraform
security group are too open because they inherit the wildcard source prefix,
which exposes Grafana on port 3000 with default admin credentials. Update the
inbound rule handling in the main Terraform security group so the `grafana` (and
preferably `prometheus`) entry uses a restricted `source_address_prefix` or a
safer allowlist/VPN/SSH-tunnel approach instead of public access. If you keep
Grafana exposed for dev-stage, also add a guard in the Ansible deploy path for
the Grafana defaults (`grafana_admin_user` / `grafana_admin_password`) so
deployment fails when the password is still the default.
---
Duplicate comments:
In `@helm/bytebite/values.yaml`:
- Around line 120-122: The Grafana ingress path default is missing the trailing
slash needed for sub-path serving. Update the monitoring.grafana.ingress.path
value in values.yaml to use the sub-path form with a trailing slash so it
matches its reuse in ingress.yaml and grafana-deployment.yaml when building
GF_SERVER_ROOT_URL. Keep the path consistent with the Grafana sub-path
configuration used by the chart.
---
Nitpick comments:
In `@helm/bytebite/dashboards/bytebite-overview.json`:
- Around line 1-490: The ByteBite Overview dashboard JSON is duplicated in two
places, which can cause the Helm-deployed version to drift from the local
version. Consolidate the dashboard definition into a single source of truth and
have the Helm dashboard consume it directly, for example via a shared dashboards
asset or Helm file reference, so changes to the dashboard title/uid and panel
targets only need to be made once.
In `@helm/bytebite/templates/grafana-deployment.yaml`:
- Around line 16-58: The Grafana container in the deployment template is missing
a hardened securityContext and is still using the default root-capable, writable
root filesystem. Update the grafana container spec to add a securityContext with
runAsNonRoot, a fixed runAsUser/fsGroup matching Grafana’s official uid 472, and
readOnlyRootFilesystem set to true, while keeping the existing grafana-data
emptyDir mount usable. Use the grafana container block in
grafana-deployment.yaml to apply this without changing the provisioning or
volumeMount setup.
- Around line 56-57: Grafana state is currently backed by an ephemeral emptyDir,
so manual dashboards/users/annotations are lost on restart; update the
grafana-data volume in the grafana-deployment template to use persistent storage
instead of emptyDir, or make persistence configurable for production use. Keep
the mountPath at /var/lib/grafana and ensure the corresponding volume definition
for grafana-data is changed consistently wherever it is referenced.
In `@helm/bytebite/templates/prometheus-deployment.yaml`:
- Around line 33-38: The Prometheus data volume is currently backed by emptyDir
in the prometheus-deployment.yaml template, so metrics are lost on pod restarts
or rescheduling. Update the prometheus-data volume configuration to use
persistent storage instead, such as a PersistentVolumeClaim, and wire it into
the Prometheus deployment spec so the data survives restarts. Use the
prometheus-data volume name and the Prometheus deployment template as the main
reference points when making the change.
- Around line 16-38: The prometheus Deployment is missing an explicit container
security context, so the container runs with the default root-capable settings.
Update the prometheus container in the prometheus-deployment template to add a
hardened securityContext for the container, using the prometheus container spec
as the anchor. Disable privilege escalation, set a non-root user, and drop
unnecessary capabilities; if the image and mounted paths allow it, also mark the
root filesystem read-only while keeping the existing /prometheus volume
writable.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aa68787d-fe47-4df1-9cac-5683035e37bf
📒 Files selected for processing (28)
.github/workflows/deploy-azure.ymlREADME.mdcompose.yamlhelm/bytebite/README.mdhelm/bytebite/dashboards/bytebite-overview.jsonhelm/bytebite/templates/grafana-configmap.yamlhelm/bytebite/templates/grafana-dashboard-configmap.yamlhelm/bytebite/templates/grafana-deployment.yamlhelm/bytebite/templates/grafana-service.yamlhelm/bytebite/templates/ingress.yamlhelm/bytebite/templates/monitoring-secret.yamlhelm/bytebite/templates/prometheus-configmap.yamlhelm/bytebite/templates/prometheus-deployment.yamlhelm/bytebite/templates/prometheus-service.yamlhelm/bytebite/values-local.yamlhelm/bytebite/values.yamlinfra/ansible/README.mdinfra/ansible/deploy-vars.example.ymlinfra/ansible/group_vars/bytebite.ymlinfra/ansible/roles/deploy/tasks/main.ymlinfra/ansible/roles/deploy/templates/env.j2infra/terraform/README.mdinfra/terraform/main.tfinfra/terraform/outputs.tfmonitoring/grafana/dashboards/bytebite-overview.jsonmonitoring/grafana/provisioning/dashboards/bytebite.ymlmonitoring/grafana/provisioning/datasources/prometheus.ymlmonitoring/prometheus.yml
| run: | | ||
| echo "App: http://${{ steps.tf.outputs.public_ip }}:8081" >> "$GITHUB_STEP_SUMMARY" | ||
| echo "Prometheus: http://${{ steps.tf.outputs.public_ip }}:9090" >> "$GITHUB_STEP_SUMMARY" | ||
| echo "Grafana: http://${{ steps.tf.outputs.public_ip }}:3000" >> "$GITHUB_STEP_SUMMARY" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Avoid inline ${{ }} expansion in run: — use env: instead.
zizmor flags all three lines as template-injection. Even though public_ip here comes from Terraform (Azure-assigned), the safe, idiomatic pattern is to bind it to an env var and reference it as a shell variable rather than interpolating the expression directly into the script.
🔒 Proposed fix
- name: Summary
if: success()
+ env:
+ PUBLIC_IP: ${{ steps.tf.outputs.public_ip }}
run: |
- echo "App: http://${{ steps.tf.outputs.public_ip }}:8081" >> "$GITHUB_STEP_SUMMARY"
- echo "Prometheus: http://${{ steps.tf.outputs.public_ip }}:9090" >> "$GITHUB_STEP_SUMMARY"
- echo "Grafana: http://${{ steps.tf.outputs.public_ip }}:3000" >> "$GITHUB_STEP_SUMMARY"
+ echo "App: http://${PUBLIC_IP}:8081" >> "$GITHUB_STEP_SUMMARY"
+ echo "Prometheus: http://${PUBLIC_IP}:9090" >> "$GITHUB_STEP_SUMMARY"
+ echo "Grafana: http://${PUBLIC_IP}:3000" >> "$GITHUB_STEP_SUMMARY"📝 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.
| run: | | |
| echo "App: http://${{ steps.tf.outputs.public_ip }}:8081" >> "$GITHUB_STEP_SUMMARY" | |
| echo "Prometheus: http://${{ steps.tf.outputs.public_ip }}:9090" >> "$GITHUB_STEP_SUMMARY" | |
| echo "Grafana: http://${{ steps.tf.outputs.public_ip }}:3000" >> "$GITHUB_STEP_SUMMARY" | |
| env: | |
| PUBLIC_IP: ${{ steps.tf.outputs.public_ip }} | |
| run: | | |
| echo "App: http://${PUBLIC_IP}:8081" >> "$GITHUB_STEP_SUMMARY" | |
| echo "Prometheus: http://${PUBLIC_IP}:9090" >> "$GITHUB_STEP_SUMMARY" | |
| echo "Grafana: http://${PUBLIC_IP}:3000" >> "$GITHUB_STEP_SUMMARY" |
🧰 Tools
🪛 zizmor (1.26.1)
[info] 122-122: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 123-123: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 124-124: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/deploy-azure.yml around lines 121 - 124, The summary lines
in the deploy-azure workflow are using inline ${{ }} expressions inside the run
script, which triggers template-injection warnings. Move
steps.tf.outputs.public_ip into env for that step, then reference it via a shell
variable in the summary writes instead of interpolating it directly in the run
block. Update the three echo statements that build the App, Prometheus, and
Grafana URLs to use the env-bound value while keeping the logic in the same
workflow step.
Source: Linters/SAST tools
| | `genai.openaiApiKey` | OpenAI API key for the selectable OpenAI provider | `""` | | ||
| | `monitoring.enabled` | Deploy Prometheus and Grafana | `true` | | ||
| | `monitoring.grafana.adminUser` | Grafana admin username | `admin` | | ||
| | `monitoring.grafana.adminPassword` | Grafana admin password | `bytebite` | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Default Grafana admin password documented in cleartext.
Publishing bytebite as the default monitoring.grafana.adminPassword in docs (and presumably values.yaml) means anyone reading the repo knows the default credential. Combined with monitoring.grafana.ingress.enabled: true and the values-local.yaml LoadBalancer service type in this same cohort, this could expose Grafana with a well-known default password if operators don't override it before deploying. Consider generating a random default (e.g., via Helm's randAlphaNum in _helpers.tpl) or requiring an explicit override with no committed default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/bytebite/README.md` at line 35, The Grafana admin password default is
documented as cleartext, so update the chart docs and default configuration
around monitoring.grafana.adminPassword to avoid shipping a known credential.
Remove the committed password default from the README and values-related
templates, and either require an explicit override or generate a random secret
via the chart helpers (for example in _helpers.tpl) so users do not deploy with
a predictable admin password.
| adminUser: admin | ||
| adminPassword: bytebite |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Default Grafana admin credentials committed to repo.
adminUser: admin / adminPassword: bytebite are plaintext, predictable defaults checked into values.yaml, which is the file used for the real cluster deployment (host team-bytebite.stud.k8s.aet.cit.tum.de, not values-local.yaml). These flow straight into bytebite-monitoring-secret (see monitoring-secret.yaml) and, if not overridden at install time, leave Grafana admin access exposed with a widely-known default password, especially once monitoring.grafana.ingress.enabled exposes it publicly.
Consider removing the default password from tracked values (require --set / a sealed secret / external secret manager at install time) and only keeping the username default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/bytebite/values.yaml` around lines 118 - 119, The Grafana admin
credentials in values.yaml are hardcoded defaults, which should not be committed
for the real deployment. Update the chart values around adminUser/adminPassword
so only the username stays as a default and the password must be supplied
externally at install time, then ensure the bytebite-monitoring-secret path
still reads from an injected secret or --set value in monitoring-secret.yaml and
any related Grafana config.
|
|
||
| grafana_admin_user: admin | ||
| grafana_admin_password: bytebite |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Weak default Grafana credentials committed in plaintext, exposed to the internet.
grafana_admin_user: admin / grafana_admin_password: bytebite are checked into version control as functional defaults (not placeholders like deploy-vars.example.yml's "change-me"). Since the paired Terraform layer opens an NSG rule for Grafana's port 3000 to the internet, any deployment that doesn't explicitly override these in the gitignored deploy-vars.yml will run a publicly reachable Grafana instance with a known, weak admin password.
Compare this to ghcr_username/ghcr_token, which default to empty strings, forcing explicit configuration rather than silently using a working-but-insecure default. Consider the same pattern here (empty defaults + a "fail fast" check, or a vaulted secret) instead of a real, guessable password baked into source control.
🔒 Suggested fix: force explicit override instead of shipping a working default
-grafana_admin_user: admin
-grafana_admin_password: bytebite
+grafana_admin_user: ""
+grafana_admin_password: ""Then add a pre-task assertion (or ansible.builtin.assert) that fails the play if these are empty, and document in deploy-vars.example.yml/README that they must be supplied (ideally via ansible-vault encrypt_string).
📝 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.
| grafana_admin_user: admin | |
| grafana_admin_password: bytebite | |
| grafana_admin_user: "" | |
| grafana_admin_password: "" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/ansible/group_vars/bytebite.yml` around lines 15 - 17, The Grafana
defaults in the bytebite group vars are insecure because they ship a real,
guessable admin credential. Update the Grafana variables in the bytebite vars
file to use empty placeholders instead of a working username/password, and add a
fail-fast check in the Ansible flow (for example near the Grafana setup task) to
assert both values are explicitly provided. Also document the required override
in the example deploy vars/README, ideally pointing users to vault-encrypted
secrets, so the playbook never deploys with a public default login.
| GRAFANA_ADMIN_USER={{ grafana_admin_user | default('admin') }} | ||
| GRAFANA_ADMIN_PASSWORD={{ grafana_admin_password | default('bytebite') }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== env.j2 ==\n'
cat -n infra/ansible/roles/deploy/templates/env.j2
printf '\n== bytebite group vars ==\n'
cat -n group_vars/bytebite.yml
printf '\n== search for grafana_admin_user/password references ==\n'
rg -n "grafana_admin_(user|password)|default\\('admin'\\)|default\\('bytebite'\\)" infra group_vars -SRepository: AET-DevOps26/team-bytebite
Length of output: 738
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate bytebite group vars and grafana vars ==\n'
fd -a 'bytebite.yml' .
fd -a 'group_vars' .
rg -n "grafana_admin_(user|password)|bytebite" infra group_vars . -S --hidden
printf '\n== find env.j2 consumers and related defaults ==\n'
rg -n "GRAFANA_ADMIN_USER|GRAFANA_ADMIN_PASSWORD|default\\('admin'\\)|default\\('bytebite'\\)" infra -S --hiddenRepository: AET-DevOps26/team-bytebite
Length of output: 40273
Remove the Grafana credential fallback here. This template still silently falls back to admin/bytebite, duplicating the same weak defaults in infra/ansible/group_vars/bytebite.yml and compose.yaml. Make the Grafana credentials explicit or vaulted so every deploy path has to supply them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/ansible/roles/deploy/templates/env.j2` around lines 7 - 8, The Grafana
credential values in the env template still fall back to weak defaults,
duplicating the same fallback in other deploy paths. Update the env.j2 template
to remove the default filters from GRAFANA_ADMIN_USER and GRAFANA_ADMIN_PASSWORD
so the deploy must supply explicit or vaulted values, and keep the credential
handling consistent with the related Grafana config in the Ansible variables and
compose setup.
| # gen-ai and the DBs are internal to the Compose network. | ||
| # NOTE: Prometheus (9090) has no authentication, and Grafana (3000) uses basic | ||
| # app credentials. These are exposed here for the dev-stage only; do not carry | ||
| # these rules into production without putting auth / an SSH tunnel in front of them. | ||
| inbound_ports = { | ||
| ssh = { priority = 100, port = "22" } | ||
| client = { priority = 110, port = "8081" } | ||
| api_gateway = { priority = 120, port = "8080" } | ||
| prometheus = { priority = 130, port = "9090" } | ||
| grafana = { priority = 140, port = "3000" } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Grafana opened to the internet with default weak credentials.
The new grafana entry inherits source_address_prefix = "*" from the existing dynamic security_rule block, so port 3000 is reachable from anywhere. Per the Ansible defaults (infra/ansible/group_vars/bytebite.yml / env.j2), grafana_admin_user/grafana_admin_password default to admin/bytebite unless explicitly overridden — a well-known default combo.
The comment frames this as "exposed here for the dev-stage only," but deploy-azure.yml applies this exact config on every green build of main; there's no separate hardened/production Terraform config. That leaves a public brute-forceable Grafana admin login on whatever is actually deployed from main.
Consider restricting source_address_prefix for the prometheus/grafana rules to known IPs (or fronting with a VPN/SSH tunnel as the comment itself suggests), and/or failing the Ansible deploy if the Grafana password is left at its default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/terraform/main.tf` around lines 6 - 15, The Grafana and Prometheus
ingress rules in the Terraform security group are too open because they inherit
the wildcard source prefix, which exposes Grafana on port 3000 with default
admin credentials. Update the inbound rule handling in the main Terraform
security group so the `grafana` (and preferably `prometheus`) entry uses a
restricted `source_address_prefix` or a safer allowlist/VPN/SSH-tunnel approach
instead of public access. If you keep Grafana exposed for dev-stage, also add a
guard in the Ansible deploy path for the Grafana defaults (`grafana_admin_user`
/ `grafana_admin_password`) so deployment fails when the password is still the
default.
timn21
left a comment
timn21
left a comment
There was a problem hiding this comment.
Looks good to me. We can research later how to handle the duplicate dashboard json, which is for now necessary due to the folder structure.
2 participants
Added grafana dashboard
Summary by CodeRabbit
New Features
Documentation
Bug Fixes