Skip to content

chore(DO-7930): sync with cloudposse upstream to inherit deployment-id support - #3

Merged
gokul-das-ext merged 5 commits into
mainfrom
sync-cloudposse-deployment-id
Sep 4, 2026
Merged

chore(DO-7930): sync with cloudposse upstream to inherit deployment-id support#3
gokul-das-ext merged 5 commits into
mainfrom
sync-cloudposse-deployment-id

Conversation

@gokul-das-ext

@gokul-das-ext gokul-das-ext commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

⚠️ Scope — this brings in two upstream changes, not just deployment-id

A git merge pulls in every commit cloudposse made on main since our fork last diverged
(90f3213, 2026-08-19) — not just the one PR we wanted. That range includes:

  • 9db127d / cloudposse#92 — the deployment-id input we actually need (see "why" below).
  • e005beb / cloudposse#90 — upgrades cloudposse's actions to the Node 24 runtime, and
    switches several uses: refs (including actions/checkout) from version tags (@v6) to
    pinned commit SHAs, as a supply-chain-hardening change. Unrelated to deployment-id, but came
    along as part of the same merge — this is why the diff includes actions/checkout and similar
    version-to-SHA changes that aren't otherwise explained by this PR's stated purpose.
  • Two chore: update README.md commits (auto-regenerated docs following the above).

This is a deliberate tradeoff: merging the whole branch (rather than cherry-picking just the
deployment-id commit) keeps our fork in sync with upstream going forward, at the cost of a wider
diff in this one PR. Both #90 and #92 are cloudposse's own tested, already-merged changes — not
new here.

Why deployment-id

ArgoCD's notification oncePer trigger dedupes on whichever field it is pointed at. Keying it on
the application commit has a gap: rollback. Replaying a previously-deployed commit reuses an
app_commit value ArgoCD already marked as notified, so the rollback's own success/failure is
silently suppressed — no GitHub status at all, not even a wrong one.

Keying the dedupe on something unique per deploy attempt rather than per code state closes
that gap. The recommended value is ${{ github.run_id }}-${{ github.run_attempt }}.

implementation notes (cloudposse's, inherited via this merge)

  • The value is stamped with yq + strenv rather than interpolated into the YAML literal, so
    the input never reaches the shell command string and doesn't risk YAML type-coercion issues.
  • Optional, empty-default. Left unset, the stamping step is skipped and deployment_id is
    omitted from config.yaml entirely — existing callers get byte-identical output.

Testing — validated end-to-end on real infrastructure

Exercised through DriveWealth/example-eks-project's real preview-deploy pipeline (not local
emulation), via a temporary feature-branch ref override (dw-github-workflows PR #308,
do-7930-deployment-id):

That's the core property this whole change exists for: the same underlying commit gets a fresh
dedupe key on every deploy attempt, which is what fixes the rollback-suppression bug.

Part of a larger change

  1. This PR — brings deployment-id support (and the Node 24/SHA-pinning changes above) into our fork
  2. DriveWealth/dw-github-workflows#308 — passes deployment-id through
  3. DriveWealth/dwt-infra#2702 — flips ArgoCD's oncePer to read it (held in draft pending this PR)

🤖 Generated with Claude Code

johncblandii and others added 5 commits August 21, 2026 09:53
* chore: upgrade actions to Node 24 runtime (SHA-pinned)

* chore: bump wait-commit-status to v0.2.2 (node24, SHA-pinned)
…dupe (#92)

ArgoCD's notification `oncePer` trigger dedupes on whichever field it is
pointed at. Keying it on the application commit means a rollback --
replaying a commit ArgoCD has already notified for -- is silently
suppressed, so the rollback reports no status at all.

Add an optional `deployment-id` input whose value is written to the
generated `config.yaml` as `deployment_id`, giving `oncePer` a field that
is unique per deploy attempt rather than per code state.

The value is stamped with `yq` and `strenv` rather than interpolated into
the YAML literal, so IDs that would otherwise be type-coerced or break the
document (`01234`, `true`, values containing `:`) round-trip correctly, and
the input never reaches the shell command string.

Left at its empty default the step is skipped entirely and the key is
omitted, keeping `config.yaml` byte-identical for existing callers. This
matters because the action only commits to the GitOps repo when the
rendered output actually changes -- always emitting the key would push a
config-only commit, and an ArgoCD sync, for every app on upgrade.

Covered by the helm raw tests in both directions: the value round-trips
through the committed `config.yaml` when passed, and the key is absent when
it is not.
@gokul-das-ext
gokul-das-ext merged commit f41823f into main Sep 4, 2026
18 of 33 checks passed
@gokul-das-ext
gokul-das-ext deleted the sync-cloudposse-deployment-id branch September 4, 2026 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants