simp / simp/puppetsync

Follow-ups from Copilot review of the #44 rollout: await release-asset API calls, paginate the wipe, guard Forge tarball glob, validate build_container_oses

Open
#87 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
1
Forks
5
Avg merge
8d 1h
Merged PRs (30d)
4

Description

Copilot's review of the #44 rollout PRs (e.g. simp/pupmod-simp-aide#177) raised four points; all are valid and template-level, so they belong here rather than in the downstream repos. A fifth related instance turned up while verifying the first.

1. Asset-wipe step: forEach(async ...) deletions are not awaited (pre-existing, moved verbatim into resolve-release by #44)

modules/profile/files/_github/workflows/release_rpms.yml, "Wipe all previous assets" step:

existingAssets.data.forEach(async function(asset){
  asset_id = asset.id
  ...
  await github.rest.repos.deleteReleaseAsset({ owner, repo, asset_id })
})
  • forEach returns immediately; the step can report success before deletions complete (or after some fail silently).
  • asset_id is assigned without const/let.

Fix: for (const asset of existingAssets.data) { await ... } (or await Promise.all(...)), and declare the variable.

2. Same unawaited-promise pattern in the upload step (pre-existing; found while verifying #1)

The "Upload RPM file(s)" step builds conditionalClobber.then(...) promise chains inside a .map() and never awaits them — upload failures cannot fail the step, and completion relies on the node event loop draining before the runner tears down. Same fix: await the chains (for...of + await, or await Promise.all).

3. Wipe step only sees the first page of listReleaseAssets (pre-existing)

clean == 'yes' wipes at most one page (default 30 assets). Unlikely at 3-OS × (RPMs + GPG key) scale, but cheap to fix with github.paginate(github.rest.repos.listReleaseAssets, ...) while fixing #1.

4. Forge upload: find can match multiple tarballs (new in #44)

modules/profile/files/pupmod/_github/workflows/tag_deploy.yml, "Deploy to Puppet Forge" step:

file="$(find "$PWD/pkg" -name '*.tar.gz')"

With more than one *.tar.gz in pkg/, --form "file=@${file}" breaks (multi-line value) or uploads the wrong archive. checkout runs with clean: true and the build is fresh, so it shouldn't happen — but the guard is one line: fail with a clear error unless exactly one file matches. The same expansion feeds the artifact-upload and gh release upload steps, so a guard early in the job covers all three.

5. Validate build_container_oses before the matrix consumes it (new in #44)

An invalid JSON value fails the workflow at matrix-evaluation time with a cryptic error. Since resolve-release completes before the matrix job is scheduled, its existing "Validate inputs" step can parse the input (e.g. jq -e 'type == "array" and length > 0 and all(.[]; type == "string")') and emit a clear ::error:: naming the expected format ('["el8","el9","el10"]').

Rollout

All five are template fixes in release_rpms.yml / tag_deploy.yml; rolling them out is another scoped merge session (same shape as the 20260812 config). None are urgent: 1–3 only affect the manual clean: 'yes' path or >30-asset releases, and 4–5 are guards against conditions that don't occur in the tag-triggered path.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the "Wipe all previous assets" and "Upload RPM file(s)" steps in modules/profile/files/_github/workflows/release_rpms.yml, then inspect the "Deploy to Puppet Forge" step and build_container_oses validation in modules/profile/files/pupmod/_github/workflows/tag_deploy.yml. Verify the workflow expressions and shell commands around those steps, then confirm all five cases are handled without changing the rollout scope.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, javascript, shell
Domain
ci-cd, release
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.