feat(o11y): add Grafana and stable VM endpoint - #5405
Conversation
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Co-authored-by: Rudransh Shrivastava <rudransh.shr@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
Signed-off-by: hassaansaleem28 <iamhassaans@gmail.com>
github-actions
Bot
added
docs
|
Contribution validation failed:
|
Summary by CodeRabbit
WalkthroughAdds pinned local Grafana and VictoriaMetrics images. Adds a conditional Terraform observability module that deploys VictoriaMetrics on ECS Fargate with encrypted EFS storage, service discovery, logging, IAM, and Terraform tests. ChangesObservability container packaging
Terraform observability stack
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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: 4
🤖 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 `@docker/victoriametrics/Dockerfile`:
- Line 1: Configure the local VictoriaMetrics container to run as UID 65532 by
adding a USER directive to the Dockerfile or the equivalent user setting in
Compose. Preserve the existing image pin and ensure the setting applies to the
o11y-metrics service, matching ECS and EFS ownership requirements.
In `@infrastructure/modules/observability/main.tf`:
- Around line 289-317: Add a Grafana ECS service alongside aws_ecs_service.vm,
including its task definition, networking, capacity provider, service discovery,
and required dependencies. Create or integrate an ALB listener/routing path for
Grafana and provision an AWS Secrets Manager credential resource, injecting its
value into the Grafana task without exposing it through module outputs; update
the module caller in main.tf to supply any required integration inputs.
- Around line 117-167: Update the CMK policy associated with var.kms_key_arn to
add EFS permissions including kms:CreateGrant, and constrain both the EFS and
CloudWatch Logs statements with regional kms:ViaService endpoints. Locate the
policy resource or document defining the key permissions, preserving existing
principals and actions while adding the required service-specific restrictions
for the EFS resources above and aws_cloudwatch_log_group.vm.
In `@infrastructure/modules/observability/tests/setup/main.tf`:
- Around line 16-18: Update the aws_vpc.main Terraform resource to explicitly
manage its default security group with no ingress or egress rules, using the
appropriate default security group resource associated with the VPC. Preserve
the existing VPC configuration and ensure the default group cannot retain
AWS-created self-referenced ingress or unrestricted egress.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 01b02892-5046-4c7f-9419-4938264aec7d
📒 Files selected for processing (20)
.trivyignore.yamldocker-compose/local/compose.o11y.yamldocker-compose/local/grafana/provisioning/dashboards/dashboards.yamldocker/grafana/Dockerfiledocker/victoriametrics/Dockerfileinfrastructure/live/README.mdinfrastructure/live/main.tfinfrastructure/live/variables.tfinfrastructure/modules/observability/.terraform.lock.hclinfrastructure/modules/observability/README.mdinfrastructure/modules/observability/main.tfinfrastructure/modules/observability/outputs.tfinfrastructure/modules/observability/tests/integration.tftest.hclinfrastructure/modules/observability/tests/setup/.gitignoreinfrastructure/modules/observability/tests/setup/main.tfinfrastructure/modules/observability/tests/setup/outputs.tfinfrastructure/modules/observability/tests/setup/variables.tfinfrastructure/modules/observability/tests/unit.tftest.hclinfrastructure/modules/observability/variables.tfinfrastructure/scripts/localstack.py
| @@ -0,0 +1 @@ | |||
| FROM victoriametrics/victoria-metrics:v1.145.0@sha256:c014fb5a711d38cb24fd0673197592cd1394bb903dbb16aea565620c9c8a3d70 | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 3 '^(FROM|USER)' docker/victoriametrics/Dockerfile || true
rg -n -C 4 'o11y-metrics|user:' docker-compose/local/compose.o11y.yaml || true
rg -n -C 4 '65532|user\s*=|efs|access_point' infrastructure/modules/observability --glob '*.tf' || true
rg -n -C 3 'DS-0002|victoriametrics' .trivyignore.yaml || trueRepository: OWASP/Nest
Length of output: 10614
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Dockerfile and local service ---'
cat -n docker/victoriametrics/Dockerfile
sed -n '30,55p' docker-compose/local/compose.o11y.yaml
printf '%s\n' '--- VictoriaMetrics image config ---'
repo='victoriametrics/victoria-metrics'
digest='sha256:c014fb5a711d38cb24fd0673197592cd1394bb903dbb16aea565620c9c8a3d70'
token="$(curl -fsSL "https://auth.docker.io/token?service=registry.docker.io&scope=repository:${repo}:pull" | jq -r .token)"
manifest="$(curl -fsSL \
-H "Authorization: Bearer ${token}" \
-H 'Accept: application/vnd.oci.image.index.v1+json, application/vnd.docker.distribution.manifest.list.v2+json, application/vnd.oci.image.manifest.v1+json, application/vnd.docker.distribution.manifest.v2+json' \
"https://registry-1.docker.io/v2/${repo}/manifests/${digest}")"
printf '%s\n' "$manifest" | jq '{mediaType, config, manifests: (.manifests // []) | map({platform, digest})}'
config_digest="$(printf '%s\n' "$manifest" | jq -r '.config.digest // empty')"
if [ -n "$config_digest" ]; then
curl -fsSL -H "Authorization: Bearer ${token}" \
"https://registry-1.docker.io/v2/${repo}/blobs/${config_digest}" |
jq '{config: {User, WorkingDir, Entrypoint, Cmd}, rootfs}'
else
printf '%s\n' 'The pinned digest is a multi-platform index; inspect each platform manifest config.'
printf '%s\n' "$manifest" | jq -r '.manifests[] | [.platform.os, .platform.architecture, .digest] | `@tsv`' |
while IFS=$'\t' read -r os arch platform_digest; do
platform_manifest="$(curl -fsSL \
-H "Authorization: Bearer ${token}" \
-H 'Accept: application/vnd.oci.image.manifest.v1+json, application/vnd.docker.distribution.manifest.v2+json' \
"https://registry-1.docker.io/v2/${repo}/manifests/${platform_digest}")"
platform_config="$(printf '%s\n' "$platform_manifest" | jq -r '.config.digest')"
printf '%s/%s %s\n' "$os" "$arch" "$platform_digest"
curl -fsSL -H "Authorization: Bearer ${token}" \
"https://registry-1.docker.io/v2/${repo}/blobs/${platform_config}" |
jq --arg platform "$os/$arch" '{platform: $platform, config: {User, WorkingDir, Entrypoint, Cmd}, rootfs}'
done
fiRepository: OWASP/Nest
Length of output: 7516
Run the local VictoriaMetrics service as non-root.
The pinned image has no default USER, so o11y-metrics runs as root because Compose does not override it. Set USER 65532 in the Dockerfile or set user: "65532" in Compose. ECS already uses 65532, and the EFS access point grants that UID/GID ownership of /victoriametrics.
🧰 Tools
🪛 Checkov (3.3.9)
[low] 1-1: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-1: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 1-1: This image might run with "root" as the default user. Make sure it is safe here.
🤖 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 `@docker/victoriametrics/Dockerfile` at line 1, Configure the local
VictoriaMetrics container to run as UID 65532 by adding a USER directive to the
Dockerfile or the equivalent user setting in Compose. Preserve the existing
image pin and ensure the setting applies to the o11y-metrics service, matching
ECS and EFS ownership requirements.
Source: Linters/SAST tools
| resource "aws_efs_file_system" "vm" { | ||
| encrypted = true | ||
| kms_key_id = var.kms_key_arn | ||
| tags = merge(var.common_tags, { | ||
| Name = "${local.name_prefix}-vm" | ||
| }) | ||
|
|
||
| lifecycle { | ||
| prevent_destroy = true | ||
| } | ||
| } | ||
|
|
||
| resource "aws_efs_mount_target" "vm" { | ||
| for_each = toset(var.subnet_ids) | ||
|
|
||
| file_system_id = aws_efs_file_system.vm.id | ||
| security_groups = [aws_security_group.efs.id] | ||
| subnet_id = each.value | ||
| } | ||
|
|
||
| resource "aws_efs_access_point" "vm" { | ||
| file_system_id = aws_efs_file_system.vm.id | ||
|
|
||
| posix_user { | ||
| gid = 65532 | ||
| uid = 65532 | ||
| } | ||
|
|
||
| root_directory { | ||
| path = "/victoriametrics" | ||
|
|
||
| creation_info { | ||
| owner_gid = 65532 | ||
| owner_uid = 65532 | ||
| permissions = "0755" | ||
| } | ||
| } | ||
|
|
||
| tags = merge(var.common_tags, { | ||
| Name = "${local.name_prefix}-vm" | ||
| }) | ||
| } | ||
|
|
||
| resource "aws_cloudwatch_log_group" "vm" { | ||
| kms_key_id = var.kms_key_arn | ||
| name = "/aws/ecs/${local.name_prefix}" | ||
| retention_in_days = var.log_retention_in_days | ||
| tags = merge(var.common_tags, { | ||
| Name = "${local.name_prefix}-logs" | ||
| }) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the CMK policy and service-specific conditions.
rg -n -C 10 \
'aws_kms_key|aws_kms_key_policy|kms:ViaService|elasticfilesystem|logs\.' \
infrastructure/modules/kms infrastructure/live/main.tf
# Confirm this module consumes the same key for both services.
rg -n -C 4 'kms_key_arn|aws_efs_file_system|aws_cloudwatch_log_group' \
infrastructure/modules/observabilityRepository: OWASP/Nest
Length of output: 32022
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- KMS policy ---'
cat -n infrastructure/modules/kms/main.tf | sed -n '1,75p'
printf '%s\n' '--- observability wiring ---'
rg -n -C 8 'module "kms"|module "observability"|kms_key_arn|aws_region' infrastructure/live infrastructure/modules/observability
printf '%s\n' '--- all KMS policy references ---'
rg -n -C 5 'AllowCloudWatchLogs|elasticfilesystem|kms:ViaService|EncryptionContext' infrastructureRepository: OWASP/Nest
Length of output: 43984
🌐 Web query:
AWS EFS customer managed KMS key policy required permissions kms:ViaService elasticfilesystem.amazonaws.com CloudWatch Logs kms:ViaService
💡 Result:
When using a customer-managed KMS key to encrypt an Amazon EFS file system, you must configure the key's resource policy to allow the Amazon EFS service to interact with it [1][2]. This configuration typically requires the use of the kms:ViaService condition key to restrict access so that only requests originating from the EFS service can use the key [1][3]. Required Permissions and Policy Structure: 1. Service Principal and ViaService: The key policy must allow the Amazon EFS service principal (elasticfilesystem.amazonaws.com) to perform necessary cryptographic actions (such as kms:Encrypt, kms:Decrypt, kms:GenerateDataKey*, and kms:CreateGrant) [1][4]. The kms:ViaService condition must be set to the regional EFS service endpoint (e.g., elasticfilesystem.us-east-2.amazonaws.com) to ensure that the key is only used when the request is routed through EFS [1][4]. 2. Forward Access Session (FAS): Amazon EFS utilizes a Forward Access Session (FAS) to perform operations on your behalf [1][2]. Consequently, your key policy must explicitly permit the kms:CreateGrant action [1][2]. This allows EFS to create the necessary grants to manage key access during its operations [1]. 3. Example Policy Snippet: When defining these permissions in your KMS key policy, you generally use a statement similar to the following, replacing the region with your specific AWS region [1][4]: "Condition": { "StringEquals": { "kms:ViaService": "elasticfilesystem.us-east-2.amazonaws.com" } } CloudWatch Logs Note: While CloudWatch Logs also utilizes KMS for encryption, it is a separate AWS service. Permissions for CloudWatch Logs (if you are also encrypting log groups) will require a different kms:ViaService value, typically logs.<region>.amazonaws.com. Ensure you do not conflate the kms:ViaService requirement for EFS with that of CloudWatch Logs in a single policy statement; each service requires its own specific service endpoint in the kms:ViaService condition [3].
Citations:
- 1: https://docs.aws.amazon.com/efs/latest/ug/encryption-at-rest.html
- 2: https://docs.amazonaws.cn/en_us/efs/latest/ug/encryption-at-rest.html
- 3: https://docs.aws.amazon.com/kms/latest/developerguide/conditions-kms.html
- 4: https://docs.aws.amazon.com/efs/latest/ug/EFSKMS.html
Add EFS permissions to the CMK policy.
The policy has no EFS service statement and no kms:ViaService restriction. Add the required EFS permissions, including kms:CreateGrant, and restrict EFS and CloudWatch Logs statements to their regional service endpoints.
🤖 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 `@infrastructure/modules/observability/main.tf` around lines 117 - 167, Update
the CMK policy associated with var.kms_key_arn to add EFS permissions including
kms:CreateGrant, and constrain both the EFS and CloudWatch Logs statements with
regional kms:ViaService endpoints. Locate the policy resource or document
defining the key permissions, preserving existing principals and actions while
adding the required service-specific restrictions for the EFS resources above
and aws_cloudwatch_log_group.vm.
Source: Learnings
| resource "aws_ecs_service" "vm" { | ||
| cluster = aws_ecs_cluster.vm.id | ||
| deployment_maximum_percent = 100 | ||
| deployment_minimum_healthy_percent = 0 | ||
| desired_count = var.vm_desired_count | ||
| name = "${local.name_prefix}-service" | ||
| tags = merge(var.common_tags, { | ||
| Name = "${local.name_prefix}-service" | ||
| }) | ||
| task_definition = aws_ecs_task_definition.vm.arn | ||
|
|
||
| capacity_provider_strategy { | ||
| base = 0 | ||
| capacity_provider = "FARGATE" | ||
| weight = 1 | ||
| } | ||
|
|
||
| network_configuration { | ||
| assign_public_ip = var.assign_public_ip | ||
| security_groups = [aws_security_group.vm.id] | ||
| subnets = var.subnet_ids | ||
| } | ||
|
|
||
| service_registries { | ||
| registry_arn = aws_service_discovery_service.vm.arn | ||
| } | ||
|
|
||
| depends_on = [aws_efs_mount_target.vm] | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Implement the Grafana deployment path.
This module deploys only the VictoriaMetrics ECS service. The supplied module and its caller at infrastructure/live/main.tf Lines 186-204 contain no Grafana ECS service, ALB listener path, or AWS Secrets Manager credential resource.
Add Grafana deployment resources and inject Grafana credentials from Secrets Manager into the Grafana task. Add an ALB listener or explicit integration inputs for an existing ALB. Do not expose the secret through module outputs.
🤖 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 `@infrastructure/modules/observability/main.tf` around lines 289 - 317, Add a
Grafana ECS service alongside aws_ecs_service.vm, including its task definition,
networking, capacity provider, service discovery, and required dependencies.
Create or integrate an ALB listener/routing path for Grafana and provision an
AWS Secrets Manager credential resource, injecting its value into the Grafana
task without exposing it through module outputs; update the module caller in
main.tf to supply any required integration inputs.
| resource "aws_vpc" "main" { | ||
| cidr_block = "10.0.0.0/16" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict the default security group.
aws_vpc.main creates a default security group with AWS default rules. Those rules allow self-referenced ingress and unrestricted egress. Manage the default security group with no rules so accidental attachments cannot bypass the fixture security groups.
Proposed fix
resource "aws_vpc" "main" {
cidr_block = "10.0.0.0/16"
}
+
+resource "aws_default_security_group" "main" {
+ vpc_id = aws_vpc.main.id
+}Based on learnings: all Terraform files under infrastructure/ must implement production-grade security hardening.
📝 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.
| resource "aws_vpc" "main" { | |
| cidr_block = "10.0.0.0/16" | |
| } | |
| resource "aws_vpc" "main" { | |
| cidr_block = "10.0.0.0/16" | |
| } | |
| resource "aws_default_security_group" "main" { | |
| vpc_id = aws_vpc.main.id | |
| } |
🧰 Tools
🪛 Checkov (3.3.9)
[low] 16-18: Ensure the default security group of every VPC restricts all traffic
(CKV2_AWS_12)
🤖 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 `@infrastructure/modules/observability/tests/setup/main.tf` around lines 16 -
18, Update the aws_vpc.main Terraform resource to explicitly manage its default
security group with no ingress or egress rules, using the appropriate default
security group resource associated with the VPC. Preserve the existing VPC
configuration and ensure the default group cannot retain AWS-created
self-referenced ingress or unrestricted egress.
Sources: Learnings, Linters/SAST tools
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 20 files
Confidence score: 3/5
- In
infrastructure/modules/observability/main.tf, creatingaws_efs_mount_targetdirectly fromsubnet_idscan fail when two subnets share an AZ, which can breakterraform applyfor valid-looking inputs — enforce/derive one subnet per AZ before mount target creation. - In
infrastructure/modules/observability/tests/setup/main.tf, the test VPC lacks DNS hostnames/support, so private DNS service discovery (aws_service_discovery_private_dns_namespace+ ECSservice_registries) may not resolve/register and integration tests can fail for environment reasons — enable VPC DNS settings in the test fixture. - In
docker-compose/local/compose.o11y.yaml, using repo-root as the Grafana build context can send unnecessary files because root.dockerignoreis broad, leading to slower builds and larger contexts — pointo11y-grafanato a narrower context (or tighten ignore rules) to de-risk local/dev image builds.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="infrastructure/modules/observability/tests/setup/main.tf">
<violation number="1" location="infrastructure/modules/observability/tests/setup/main.tf:16">
P2: The test VPC doesn't enable DNS hostnames/support, so the module's private-DNS service discovery (aws_service_discovery_private_dns_namespace + ECS service_registries) can't resolve/register, failing the integration apply. The repo's own networking module sets enable_dns_hostnames=true and enable_dns_support=true; mirror that here.</violation>
</file>
<file name="infrastructure/modules/observability/main.tf">
<violation number="1" location="infrastructure/modules/observability/main.tf:130">
P2: Applying the module fails when `subnet_ids` contains two subnets in the same Availability Zone because this creates duplicate EFS mount targets for that AZ. Validate or derive one subnet per AZ before creating `aws_efs_mount_target.vm`.</violation>
</file>
<file name="docker/grafana/Dockerfile">
<violation number="1" location="docker/grafana/Dockerfile:6">
P3: The `USER grafana` line is a redundant no-op: the grafana-oss base image already ends with `USER 472` (the numeric UID of the `grafana` user), so the process already drops to the non-root grafana user before entrypoint. The added directive adds no behavior. Removing it (or replacing the COPYs by chowning to `grafana` if write access is ever needed) keeps the image definition aligned with the base image's user model.</violation>
</file>
<file name="docker-compose/local/compose.o11y.yaml">
<violation number="1" location="docker-compose/local/compose.o11y.yaml:56">
P2: The o11y-grafana build context is the repository root, but the repo-root `.dockerignore` only excludes `e2e/` caches and the e2e node_modules/playwright output (`.dockerignore` in-tree). The Dockerfile only needs `docker-compose/local/grafana/...`, so every `docker compose build` from `make run-o11y` ships the entire repo — including `frontend/node_modules`, `backend/.venv`, and `.git` — to the Docker daemon. In a normal dev checkout this makes the grafana build noticeably slow on every stack start. Consider scoping the context down to `docker-compose/local` (and adjusting the COPY paths in the Dockerfile to `grafana/...`) or adding a dedicated `.dockerignore` for this build so the context only contains the provisioning/dashboards subtree.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| state = "available" | ||
| } | ||
|
|
||
| resource "aws_vpc" "main" { |
There was a problem hiding this comment.
P2: The test VPC doesn't enable DNS hostnames/support, so the module's private-DNS service discovery (aws_service_discovery_private_dns_namespace + ECS service_registries) can't resolve/register, failing the integration apply. The repo's own networking module sets enable_dns_hostnames=true and enable_dns_support=true; mirror that here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrastructure/modules/observability/tests/setup/main.tf, line 16:
<comment>The test VPC doesn't enable DNS hostnames/support, so the module's private-DNS service discovery (aws_service_discovery_private_dns_namespace + ECS service_registries) can't resolve/register, failing the integration apply. The repo's own networking module sets enable_dns_hostnames=true and enable_dns_support=true; mirror that here.</comment>
<file context>
@@ -0,0 +1,37 @@
+ state = "available"
+}
+
+resource "aws_vpc" "main" {
+ cidr_block = "10.0.0.0/16"
+}
</file context>
| } | ||
|
|
||
| resource "aws_efs_mount_target" "vm" { | ||
| for_each = toset(var.subnet_ids) |
There was a problem hiding this comment.
P2: Applying the module fails when subnet_ids contains two subnets in the same Availability Zone because this creates duplicate EFS mount targets for that AZ. Validate or derive one subnet per AZ before creating aws_efs_mount_target.vm.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infrastructure/modules/observability/main.tf, line 130:
<comment>Applying the module fails when `subnet_ids` contains two subnets in the same Availability Zone because this creates duplicate EFS mount targets for that AZ. Validate or derive one subnet per AZ before creating `aws_efs_mount_target.vm`.</comment>
<file context>
@@ -0,0 +1,317 @@
+}
+
+resource "aws_efs_mount_target" "vm" {
+ for_each = toset(var.subnet_ids)
+
+ file_system_id = aws_efs_file_system.vm.id
</file context>
| o11y-grafana: | ||
| container_name: nest-o11y-grafana | ||
| build: | ||
| context: ../../ |
There was a problem hiding this comment.
P2: The o11y-grafana build context is the repository root, but the repo-root .dockerignore only excludes e2e/ caches and the e2e node_modules/playwright output (.dockerignore in-tree). The Dockerfile only needs docker-compose/local/grafana/..., so every docker compose build from make run-o11y ships the entire repo — including frontend/node_modules, backend/.venv, and .git — to the Docker daemon. In a normal dev checkout this makes the grafana build noticeably slow on every stack start. Consider scoping the context down to docker-compose/local (and adjusting the COPY paths in the Dockerfile to grafana/...) or adding a dedicated .dockerignore for this build so the context only contains the provisioning/dashboards subtree.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docker-compose/local/compose.o11y.yaml, line 56:
<comment>The o11y-grafana build context is the repository root, but the repo-root `.dockerignore` only excludes `e2e/` caches and the e2e node_modules/playwright output (`.dockerignore` in-tree). The Dockerfile only needs `docker-compose/local/grafana/...`, so every `docker compose build` from `make run-o11y` ships the entire repo — including `frontend/node_modules`, `backend/.venv`, and `.git` — to the Docker daemon. In a normal dev checkout this makes the grafana build noticeably slow on every stack start. Consider scoping the context down to `docker-compose/local` (and adjusting the COPY paths in the Dockerfile to `grafana/...`) or adding a dedicated `.dockerignore` for this build so the context only contains the provisioning/dashboards subtree.</comment>
<file context>
@@ -50,6 +52,9 @@ services:
o11y-grafana:
container_name: nest-o11y-grafana
+ build:
+ context: ../../
+ dockerfile: docker/grafana/Dockerfile
depends_on:
</file context>
| COPY docker-compose/local/grafana/provisioning /etc/grafana/provisioning | ||
| COPY docker-compose/local/grafana/dashboards /etc/grafana/dashboards | ||
|
|
||
| USER grafana |
There was a problem hiding this comment.
P3: The USER grafana line is a redundant no-op: the grafana-oss base image already ends with USER 472 (the numeric UID of the grafana user), so the process already drops to the non-root grafana user before entrypoint. The added directive adds no behavior. Removing it (or replacing the COPYs by chowning to grafana if write access is ever needed) keeps the image definition aligned with the base image's user model.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docker/grafana/Dockerfile, line 6:
<comment>The `USER grafana` line is a redundant no-op: the grafana-oss base image already ends with `USER 472` (the numeric UID of the `grafana` user), so the process already drops to the non-root grafana user before entrypoint. The added directive adds no behavior. Removing it (or replacing the COPYs by chowning to `grafana` if write access is ever needed) keeps the image definition aligned with the base image's user model.</comment>
<file context>
@@ -0,0 +1,6 @@
+COPY docker-compose/local/grafana/provisioning /etc/grafana/provisioning
+COPY docker-compose/local/grafana/dashboards /etc/grafana/dashboards
+
+USER grafana
</file context>
1 participant
Resolves: #5356
Depends on #5341