Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions terraform/terraform.tfvars
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ node_max_count = 3
firewall_domain = "sfw.security.sentry.io."

replica_count = 2
helm_chart_version = "0.6.0"
firewall_image_tag = "2.0.4"
helm_chart_version = "0.9.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The Helm chart version bump from 0.6.0 to 0.9.0 may break critical value overrides in helm.tf, potentially causing a service outage due to a UID mismatch CrashLoop.
Severity: CRITICAL

Suggested Fix

Before merging, verify that all keys for the value overrides in helm.tf are still valid for the new Helm chart version (0.9.0). Check the chart's values.yaml and changelog for any changes to keys like initContainers.certGenerator.securityContext. If keys have changed, update the Terraform configuration accordingly.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: terraform/terraform.tfvars#L16

Potential issue: Bumping the Helm chart from version 0.6.0 to 0.9.0 introduces a risk of
service outage. The `helm.tf` configuration overrides chart values like
`initContainers.certGenerator.securityContext` to prevent a known UID mismatch issue
that causes pods to CrashLoop. If the structure or keys for these overridden values have
changed in the new chart version, Helm will silently ignore the overrides. This would
reintroduce the original bug, where the init container writes certificates as UID 1000,
which the main container (running as UID 1001) cannot read, leading to "Permission
denied" errors and a complete service outage.

Did we get this right? 👍 / 👎 to inform future reviews.

firewall_image_tag = "2.0.6"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The new firewall image may not be attested for Binary Authorization, which the auto-apply workflow doesn't check, risking a deployment failure and service outage.
Severity: CRITICAL

Suggested Fix

Manually attest the new image version (2.0.6) for the project's Binary Authorization policy before merging this pull request. For a long-term solution, add an automated step to the tf-apply.yaml workflow to check for or create the necessary attestation before running terraform apply.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: terraform/terraform.tfvars#L17

Potential issue: The new firewall image version `2.0.6` may not be attested by the
project's Binary Authorization policy, which is enforced on the GKE cluster. The CI/CD
pipeline automatically applies Terraform changes on merge to `main` with `auto_approve:
true` without an attestation step. If the image is not attested, GKE will reject all new
pods during the rolling update. Because old pods are terminated during the update, this
will lead to a complete firewall service outage until the issue is manually remediated.

Did we get this right? 👍 / 👎 to inform future reviews.

Loading