Skip to content

Both halves of #1801 are still live on GCP and AWS: branch-keyed concurrency plus unconditional lock deletion #1806

Closed

Description

@cristim

#1801 was fixed for Azure in #1803. Both halves of that bug are still live on GCP and AWS. Filed separately because the scope of #1803 was Azure and the fix is not a copy-paste.

Half 1: branch-keyed concurrency against environment-keyed state

  • deploy-aws-lambda.yml:33-35 -> group: deploy-lambda-${{ github.ref }}
  • deploy-aws-fargate.yml:28-30 -> group: deploy-fargate-${{ github.ref }}
  • deploy-gcp.yml:27-29 -> group: deploy-gcp-${{ github.ref }}

Each state key is built from the environment, so two runs on different refs that resolve to the same environment land in different concurrency groups and apply against one state file. Identical to the Azure defect.

Half 2: unconditional destruction of a live lock

Both steps run on if: failure() || cancelled() with no age check and no check that the lock is anyone else's:

deploy-gcp.yml:156-167

LOCK_FILE="gs://${BUCKET}/github-${ENVIRONMENT}/default.tflock"
gsutil rm "${LOCK_FILE}" 2>/dev/null || echo "No lock file found (already clean)"

deploy-aws-lambda.yml:269-280

LOCK_KEY="github-${ENVIRONMENT}/terraform.tfstate.tflock"
aws s3 rm "s3://${BUCKET}/${LOCK_KEY}" 2>/dev/null || echo "No lock file found (already clean)"

Deleting another run's live lock object is exactly as destructive as breaking its lease. The lock mechanism differs from Azure's blob lease; the failure mode does not. Combined with half 1, this reproduces #1801's silent state loss: the second writer removes the first's lock, both proceed, and the later writer's stale read drops the earlier one's entries.

An earlier draft of #1803's description claimed the silent-loss half did not transfer to AWS because S3 uses a lock file rather than a lease. That was wrong and is corrected there.

Why this is not a copy-paste of #1803

Suggested approach

Follow #1803: move each group to the job that writes state, key it on the same value that builds the state key, apply the identical group to every workflow writing that state, and delete the lock-removal steps so a real collision fails loudly.

Activity

  1. cristim commented on Aug 12, 2026

    @cristim
    MemberAuthor

    Fixed in #1812. Three corrections to this issue's scope, found by enumerating all 16 .yml files in .github/workflows/ rather than working from the ones named here:

    1. destroy-fargate-dev.yml is a fourth state writer this issue does not mention, and it carried both halves: no concurrency group at all, and an unconditional aws s3 rm of the .tflock inside Terraform Init (:111-114 on 17a568f42). It writes github-fargate-dev/terraform.tfstate, the same object deploy-aws-fargate.yml and cleanup-staging.yml write.

    2. deploy-gcp.yml had two lock deletions, not one. Besides the failure() || cancelled() step this issue cites, Terraform Init at :145-150 ran gsutil rm on the lock object unconditionally, on every run, before init, with no condition at all. Strictly worse than the one named here.

    3. deploy-aws-fargate.yml no longer has half 2. This issue correctly lists it under half 1 only, but for the record: its lock-removal step is already gated on inputs.clear_stale_lock == true (a workflow_dispatch-only boolean defaulting false) and its failure-path release step was already deleted. That step is retained in fix(ci): serialize AWS and GCP Terraform state writers on environment, not branch #1812 as the documented operator-confirmed recovery path.

    Net: 10 state-mutating jobs across AWS and GCP were ungrouped, not the 3 workflows listed. All 10 now carry a group keyed on the value that builds their own state key. Four lock-deletion steps removed, one retained with justification.

    Also filed #1811: rollback.yml's rollback-aws-fargate builds its backend key from the Lambda namespace (github-<env>/terraform.tfstate), not github-fargate-<env>/. #1812 therefore gives that job the aws-tfstate-* group, because the group has to name the object the job actually locks; keying it on the Fargate group would have reproduced this exact bug in a new place. Changing the key is a separate and riskier change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions