Skip to content

fix(ci): publish system-tests image via S3 + GH Actions dispatch - #4065

Open
MilanGarnier wants to merge 8 commits into
masterfrom
milan.garnier/publish-ci-builds
Open

fix(ci): publish system-tests image via S3 + GH Actions dispatch#4065
MilanGarnier wants to merge 8 commits into
masterfrom
milan.garnier/publish-ci-builds

Conversation

@MilanGarnier

Copy link
Copy Markdown
Contributor

Description

#4013 pushed the per-branch system-tests image to GHCR directly from GitLab
CI, authenticating with an octo-sts token scoped packages: write. That
token is minted from a GitHub App installation, and GHCR refuses package
writes from installation tokens (permission_denied: installation not allowed to Write organization package). The job has allow_failure: true,
so it's been failing silently since #4013 merged — no image has actually
been published.

This replaces that job with a two-step, two-runtime pipeline:

  1. GitLab CI (publish docker image for system tests (upload)) uploads the
    built tarballs + installer to s3://dd-trace-php-builds/ci/<ref-slug>/,
    then (... (dispatch)) mints an actions: write-scoped octo-sts token
    and dispatches a new GitHub Actions workflow.
  2. .github/workflows/publish-system-tests-image.yml downloads the
    artifacts from S3 and pushes to GHCR using the workflow's native
    GITHUB_TOKEN, which — unlike the octo-sts token — is actually allowed
    to write org packages.

.github/chainguard/gitlab-ci-publish-packages.sts.yaml is updated from
packages: write to actions: write to match (nothing mints a
packages-scoped token through this policy anymore).

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Jul 27, 2026

Copy link
Copy Markdown

Pipelines  Tests

Unblock PR with BitsAI

⚠️ Warnings

🚦 6 Pipeline jobs failed

DataDog/apm-reliability/dd-trace-php | ASAN test_c with multiple observers: [8.5]   View in Datadog   GitLab

DataDog/apm-reliability/dd-trace-php | test_extension_ci: [7.0]   View in Datadog   GitLab

DataDog/apm-reliability/dd-trace-php | test_web_symfony_44: [8.0, cgi-fcgi]   View in Datadog   GitLab

View all 6 failed jobs.

ℹ️ Info

No other issues found (see more)

🧪 All tests passed
❄️ No new flaky tests detected

🔄 Datadog auto-retried 1 job - 1 passed on retry View in Datadog

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 60.64% (-0.03%)

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 6e12d4a | Docs | Datadog PR Page | Give us feedback!

#4013 pushed the per-branch system-tests image to GHCR directly from
GitLab CI, authenticating with an octo-sts token scoped
`packages: write`. That token comes from a GitHub App installation,
and GHCR refuses package writes from installation tokens
(permission_denied: installation not allowed to Write organization
package). The job has allow_failure: true, so it has been failing
silently since #4013 merged and no image has actually been published.

Replace it with a two-step, two-runtime pipeline:

1. GitLab CI ("publish docker image for system tests (upload)")
   uploads the built tarballs and installer to
   s3://dd-trace-php-builds/ci/<ref-slug>/, then
   ("... (dispatch)") mints an actions: write-scoped octo-sts token
   and dispatches a new GitHub Actions workflow.
2. .github/workflows/publish-system-tests-image.yml downloads the
   artifacts from S3 and pushes to GHCR using the workflow's native
   GITHUB_TOKEN, which is actually allowed to write org packages.

Update gitlab-ci-publish-packages.sts.yaml from packages: write to
actions: write to match, since nothing mints a packages-scoped token
through this policy anymore.
@MilanGarnier
MilanGarnier force-pushed the milan.garnier/publish-ci-builds branch from f4dedbc to 97f2a40 Compare July 27, 2026 08:11
@MilanGarnier

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97f2a40fe0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .gitlab/generate-package.php
Comment thread .gitlab/generate-package.php Outdated
Comment thread .gitlab/generate-package.php Outdated
- Scope both the S3 upload and the GHCR fetch by CI_COMMIT_SHORT_SHA,
  not just the branch slug. Two concurrent pipelines for the same
  branch were sharing one S3 prefix, so a slower upload could
  overwrite a faster one's objects mid-read on the GitHub Actions
  side, mixing old and new artifacts into one image.

- Fail the GitLab dispatch job loudly if the PR-lookup request itself
  errors, instead of treating any non-2xx response the same as "zero
  open PRs" and silently skipping the publish.

- Move package cleanup in the GH Actions workflow from before the
  push to after it, and match on untagged versions instead of the
  tag being retargeted. The old ordering deleted-by-tag before
  pushing, which 404s on the very first publish ever (no package
  exists yet) and, since Actions runs steps with `bash -eo
  pipefail`, took the whole job down before it reached the push
  itself. Cleanup is also now best-effort (continue-on-error): it's
  storage hygiene, not a precondition for publishing.

- Add an org.opencontainers.image.source label to the built image so
  GHCR links it to this repo, which is what lets the repo's own
  GITHUB_TOKEN manage the package going forward.
The upload job is allow_failure: true so the pipeline doesn't go red
over this optional publish, but that also makes needs: run the
dispatch job even when upload failed or only partially uploaded.
Re-verify the three expected S3 objects exist for this commit before
dispatching, instead of trusting the upload job's (possibly
falsely-green) status.
A failed or partial publish should be visible and block merge like
any other job, rather than going green silently. This also makes the
needs: chain behave normally: dispatch simply won't run if upload
fails, no allow_failure/needs interaction to reason about.
needs: already refuses to run dispatch if upload failed now that
upload isn't allow_failure. The only case this check still caught
(retrying dispatch alone from the GitLab UI, which bypasses needs:)
isn't worth the extra script complexity.
Drop CI_COMMIT_SHORT_SHA/commit_slug from the S3 upload path and the
workflow_dispatch inputs, keeping CI_COMMIT_REF_SLUG as the sole
identifier.
Drop two comments that just restated the adjacent config (packages:
write, continue-on-error) instead of explaining non-obvious behavior.
@MilanGarnier
MilanGarnier marked this pull request as ready for review July 28, 2026 12:59
@MilanGarnier
MilanGarnier requested a review from a team as a code owner July 28, 2026 12:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e12d4abed

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +67 to +69
gh api "${PACKAGE_PATH}/versions" \
--jq '.[] | select(.metadata.container.tags == []) | .id' \
| xargs -r -I {} gh api "${PACKAGE_PATH}/versions/{}" -X DELETE

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid deleting child manifests after multi-arch push

When this workflow publishes the linux/amd64,linux/arm64 tag, Buildx creates a tagged image index plus untagged per-platform manifests; GHCR exposes those child manifests as package versions with empty metadata.container.tags (the same failure mode has been reported for GHCR untagged cleanup: actions/delete-package-versions#162). This selector deletes every untagged version immediately after the push, including the current amd64/arm64 manifests referenced by ${REF_SLUG}, so the branch tag can exist but fail to pull in system-tests; delete only known stale versions for this tag or skip the untagged cleanup.

Useful? React with 👍 / 👎.

Comment on lines +1585 to +1587
aws s3 cp packages/dd-library-php-*-x86_64-linux-gnu.tar.gz "${DEST}/dd-library-php-x86_64-linux-gnu.tar.gz"
aws s3 cp packages/dd-library-php-*-aarch64-linux-gnu.tar.gz "${DEST}/dd-library-php-aarch64-linux-gnu.tar.gz"
aws s3 cp packages/datadog-setup.php "${DEST}/datadog-setup.php"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Gate S3 uploads on the PR check

For any non-default branch that has no open GitHub PR, this upload job still copies the branch tarballs to the public S3 prefix that the Actions workflow later downloads anonymously. The PR gate only runs in the following dispatch job, so internal/throwaway branch pipelines now publish build artifacts even though the image publish is skipped; move the open-PR check before these aws s3 cp calls or make the upload job conditional on the same gate.

Useful? React with 👍 / 👎.

Comment on lines +1625 to +1629
PAYLOAD=$(jq -n --arg ref "${CI_DEFAULT_BRANCH}" --arg slug "${CI_COMMIT_REF_SLUG}" \
'{ref: $ref, inputs: {ref_slug: $slug}}')
HTTP=$(curl -s -o /tmp/dispatch.json -w "%{http_code}" -X POST \
-H "Authorization: Bearer ${GITHUB_TOKEN}" -H "Accept: application/vnd.github+json" \
"https://api.github.com/repos/DataDog/dd-trace-php/actions/workflows/publish-system-tests-image.yml/dispatches" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bootstrap the workflow before dispatching it

When this change is exercised from the PR branch that adds publish-system-tests-image.yml, the dispatch targets ${CI_DEFAULT_BRANCH} even though that branch does not contain the new workflow file yet; GitHub documents that a workflow_dispatch workflow must be on the default branch before it can be triggered (https://docs.github.com/en/actions/how-tos/manage-workflow-runs/manually-run-a-workflow#configuring-a-workflow-to-run-manually). Since this dispatch job is no longer allowed to fail, open-PR pipelines for this PR will fail with a 404 instead of validating the fix; land the workflow on the default branch first or keep this dispatch optional until it exists there.

Useful? React with 👍 / 👎.

@pr-commenter

pr-commenter Bot commented Jul 28, 2026

Copy link
Copy Markdown

Benchmarks [ tracer ]

Benchmark execution time: 2026-07-28 14:17:37

Comparing candidate commit 6e12d4a in PR branch milan.garnier/publish-ci-builds with baseline commit b2d669e in branch master.

Found 2 performance improvements and 0 performance regressions! Performance is the same for 192 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟩 execution_time [-6.925µs; -5.095µs] or [-6.227%; -4.581%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟩 execution_time [-6.469µs; -4.831µs] or [-5.607%; -4.187%]

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant