From 1359dc2d39d7f3aae4579dd254f6424660318ea4 Mon Sep 17 00:00:00 2001 From: JonJagger Date: Wed, 7 Oct 2026 10:52:48 +0100 Subject: [PATCH 1/2] Attest the state file a failed apply rewrote terraform apply writes the state file to S3 even when it fails part-way, but apply.yml only attests the state file in reset-drift-detection, which needs the apply job to succeed. A failed CI apply therefore leaves a state file Kosli has no provenance for, and drift detection reports it exactly as if terraform had been run from a laptop. It stays that way until the same component next applies successfully. This happened on 2026-10-06 to the cyber-dojo prod apply of custom-start-points: the apply timed out waiting for the ECS service to stabilise after writing the state file, and aws-prod-terraform-drift-detection stayed non-compliant until a forced redeployment the next day. https://github.com/cyber-dojo/aws-prod-co-promotion/actions/runs/37458113081/job/112253462088 base.yml now records the state file's ETag before Apply and, only if Apply fails, attests the state file when its ETag has changed. A state file the failed apply did not change is left unattested, since it may have been written outside CI. The bucket and key come from the backend config tf plan initialised, so they are the ones terraform used. The ETag step is continue-on-error so it can never block an apply. A cancelled apply, or one whose AWS credentials have expired, attests nothing, so it is still reported as drift. A failed apply also does not reset the drift plan, so infrastructure it changed is reported as drift by the next detect-drift run until an apply succeeds. The README says so. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/base.yml | 49 ++++++++++++++++++++++++++++++++++++++ README.md | 5 ++++ 2 files changed, 54 insertions(+) diff --git a/.github/workflows/base.yml b/.github/workflows/base.yml index a9592c3..7cd6b69 100644 --- a/.github/workflows/base.yml +++ b/.github/workflows/base.yml @@ -220,12 +220,61 @@ jobs: # shellcheck disable=SC2086 kosli attest generic --name terraform-plan --attachments /tmp/${{ inputs.environment }}.plan.txt $COMMIT_ARG + # terraform apply writes the state file even when it fails part-way, and + # reset-drift-detection in apply.yml only attests it after a successful + # apply. Recording the state file's ETag here lets the step after Apply + # tell whether a failed apply rewrote it. The bucket and key are read from + # the backend config that `tf plan` initialised, so they are exactly the + # ones terraform uses. continue-on-error keeps this step from ever + # blocking the apply; if it fails, a failed apply's state is not attested. + - name: Record state file ETag before apply + id: state-before + if: inputs.tf_apply && inputs.kosli_template_file != '' + continue-on-error: true + working-directory: ${{ inputs.working_directory }} + run: | + ACCOUNT_ID=$(aws sts get-caller-identity --query Account --output text) + BACKEND=".terraform.${ACCOUNT_ID}-${AWS_DEFAULT_REGION}/terraform.tfstate" + BUCKET=$(jq -r '.backend.config.bucket' "$BACKEND") + KEY=$(jq -r '.backend.config.key' "$BACKEND") + ETAG=$(aws s3api head-object --bucket "$BUCKET" --key "$KEY" --query ETag --output text) + { + echo "bucket=${BUCKET}" + echo "key=${KEY}" + echo "etag=${ETAG}" + } >> "$GITHUB_OUTPUT" + - name: Apply + id: apply shell: bash if: inputs.tf_apply working-directory: ${{ inputs.working_directory }} run: tf apply -no-color 2>&1 | tee /tmp/${{ inputs.environment }}.apply.txt + # Attests the state file only when this failed apply changed it. A state + # file the apply did not change may have been written outside CI, so it is + # left unattested for drift detection to report. The object is downloaded + # with --if-match on the ETag just read, so a write between the two reads + # cannot be attested in its place. A write in the seconds between the + # apply releasing its lock and the first read would be attested; that is + # the same race reset-drift-detection has after a successful apply. + - name: Kosli attest state file written by failed apply + if: failure() && steps.apply.outcome == 'failure' && steps.state-before.outcome == 'success' + env: + BUCKET: ${{ steps.state-before.outputs.bucket }} + KEY: ${{ steps.state-before.outputs.key }} + ETAG_BEFORE: ${{ steps.state-before.outputs.etag }} + run: | + ETAG_AFTER=$(aws s3api head-object --bucket "$BUCKET" --key "$KEY" --query ETag --output text) + if [ "$ETAG_AFTER" = "$ETAG_BEFORE" ]; then + echo "The failed apply did not change the state file, so it is not attested." + exit 0 + fi + STATE_FILE="/tmp/$(basename "$KEY")" + aws s3api get-object --bucket "$BUCKET" --key "$KEY" --if-match "$ETAG_AFTER" "$STATE_FILE" > /dev/null + # shellcheck disable=SC2086 + kosli attest artifact "$STATE_FILE" --artifact-type file --name terraform-state $COMMIT_ARG + - name: Apply summary if: inputs.tf_apply run: | diff --git a/README.md b/README.md index ad49383..4578a06 100644 --- a/README.md +++ b/README.md @@ -258,6 +258,11 @@ Both workflows can optionally attest each Terraform run to Kosli. This is opt-i * in `apply.yml`, additionally attest the **state file** and the **drift plan** as artifacts so that any later out-of-band change to either file is detected as drift by the downstream [drift-detection job][drift-doc]. +* if `tf apply` fails but has already rewritten the state file, attest that state file too, so + the failed CI apply is not reported as drift. A failed apply that left the state file + unchanged attests nothing. A failed apply does not reset the drift plan, which still names + the commit of the last successful apply, so if the failed apply changed infrastructure the + next [drift-detection job][drift-doc] run reports it as drift until an apply succeeds. The trail template needs to declare every attestation/artifact name the workflow emits: From 36b173290f80735af6f9f2897c0a7e719d3a9499 Mon Sep 17 00:00:00 2001 From: JonJagger Date: Wed, 7 Oct 2026 11:48:03 +0100 Subject: [PATCH 2/2] Keep a failed apply non-compliant, and document why Attesting the state file a failed apply rewrote would report a partly-applied commit as compliant. Terraform writes the state file as it goes, so an apply that fails part-way leaves infrastructure only partly at the merged commit. That is an inconsistency worth seeing, and drift detection reporting it is the right outcome, not a false positive. So this drops the base.yml change from the previous commit, and the README now says that a failed apply attests nothing on purpose, and that the environment stays non-compliant until an apply of the same component succeeds, which is the remedy once the failure's cause is fixed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/base.yml | 49 -------------------------------------- README.md | 12 ++++++---- 2 files changed, 7 insertions(+), 54 deletions(-) diff --git a/.github/workflows/base.yml b/.github/workflows/base.yml index 7cd6b69..a9592c3 100644 --- a/.github/workflows/base.yml +++ b/.github/workflows/base.yml @@ -220,61 +220,12 @@ jobs: # shellcheck disable=SC2086 kosli attest generic --name terraform-plan --attachments /tmp/${{ inputs.environment }}.plan.txt $COMMIT_ARG - # terraform apply writes the state file even when it fails part-way, and - # reset-drift-detection in apply.yml only attests it after a successful - # apply. Recording the state file's ETag here lets the step after Apply - # tell whether a failed apply rewrote it. The bucket and key are read from - # the backend config that `tf plan` initialised, so they are exactly the - # ones terraform uses. continue-on-error keeps this step from ever - # blocking the apply; if it fails, a failed apply's state is not attested. - - name: Record state file ETag before apply - id: state-before - if: inputs.tf_apply && inputs.kosli_template_file != '' - continue-on-error: true - working-directory: ${{ inputs.working_directory }} - run: | - ACCOUNT_ID=$(aws sts get-caller-identity --query Account --output text) - BACKEND=".terraform.${ACCOUNT_ID}-${AWS_DEFAULT_REGION}/terraform.tfstate" - BUCKET=$(jq -r '.backend.config.bucket' "$BACKEND") - KEY=$(jq -r '.backend.config.key' "$BACKEND") - ETAG=$(aws s3api head-object --bucket "$BUCKET" --key "$KEY" --query ETag --output text) - { - echo "bucket=${BUCKET}" - echo "key=${KEY}" - echo "etag=${ETAG}" - } >> "$GITHUB_OUTPUT" - - name: Apply - id: apply shell: bash if: inputs.tf_apply working-directory: ${{ inputs.working_directory }} run: tf apply -no-color 2>&1 | tee /tmp/${{ inputs.environment }}.apply.txt - # Attests the state file only when this failed apply changed it. A state - # file the apply did not change may have been written outside CI, so it is - # left unattested for drift detection to report. The object is downloaded - # with --if-match on the ETag just read, so a write between the two reads - # cannot be attested in its place. A write in the seconds between the - # apply releasing its lock and the first read would be attested; that is - # the same race reset-drift-detection has after a successful apply. - - name: Kosli attest state file written by failed apply - if: failure() && steps.apply.outcome == 'failure' && steps.state-before.outcome == 'success' - env: - BUCKET: ${{ steps.state-before.outputs.bucket }} - KEY: ${{ steps.state-before.outputs.key }} - ETAG_BEFORE: ${{ steps.state-before.outputs.etag }} - run: | - ETAG_AFTER=$(aws s3api head-object --bucket "$BUCKET" --key "$KEY" --query ETag --output text) - if [ "$ETAG_AFTER" = "$ETAG_BEFORE" ]; then - echo "The failed apply did not change the state file, so it is not attested." - exit 0 - fi - STATE_FILE="/tmp/$(basename "$KEY")" - aws s3api get-object --bucket "$BUCKET" --key "$KEY" --if-match "$ETAG_AFTER" "$STATE_FILE" > /dev/null - # shellcheck disable=SC2086 - kosli attest artifact "$STATE_FILE" --artifact-type file --name terraform-state $COMMIT_ARG - - name: Apply summary if: inputs.tf_apply run: | diff --git a/README.md b/README.md index 4578a06..67b61e1 100644 --- a/README.md +++ b/README.md @@ -258,11 +258,13 @@ Both workflows can optionally attest each Terraform run to Kosli. This is opt-i * in `apply.yml`, additionally attest the **state file** and the **drift plan** as artifacts so that any later out-of-band change to either file is detected as drift by the downstream [drift-detection job][drift-doc]. -* if `tf apply` fails but has already rewritten the state file, attest that state file too, so - the failed CI apply is not reported as drift. A failed apply that left the state file - unchanged attests nothing. A failed apply does not reset the drift plan, which still names - the commit of the last successful apply, so if the failed apply changed infrastructure the - next [drift-detection job][drift-doc] run reports it as drift until an apply succeeds. + +A failed `tf apply` attests nothing, deliberately. Terraform writes the state file as it goes, so +an apply that fails part-way can leave a rewritten state file that has no provenance in Kosli, and +the [drift-detection job][drift-doc] reports it as drift. That is the intended outcome: the +infrastructure is only partly at the merged commit, so it should not be reported as compliant. +The environment stays non-compliant until an apply of the same component succeeds, so once the +cause of the failure is fixed, re-run the apply. The trail template needs to declare every attestation/artifact name the workflow emits: