Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cross-repo race condition #4404

Merged
merged 6 commits into from
Feb 19, 2025
Merged

Conversation

lloeki
Copy link
Member

@lloeki lloeki commented Feb 19, 2025

What does this PR do?

  • Pass pipeline information
  • Select workflow by run name and properly identify it with absolute certainty
  • Fix a lot of corner cases
  • Enhance logging feedback for debugging
  • Prevent token leaks

Motivation:

Given that we run across repos and across CIs, there was a significant probability to pick incorrect workflows, that would be entirely unrelated to the dd-trace-rb item that triggered a workflow here.

On the vaccine side this is addressed by:

  • allowing an additional trigger-id input
  • reflecting that trigger-id in the workflow run name

As a bonus this allows direct identification of vaccine workflow runs straight from GitHub Actions UI.

The image is pulled by vaccine by tag containing only the commit SHA and so remains vulnerable to race conditions.

Change log entry

None.

Additional Notes:

How to test the change?

CI should pick the proper things throughout. Check Vaccine job logs in GitLab pipeline, e.g https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-rb/-/jobs/813418074 and https://github.com/DataDog/vaccine/actions/runs/13409446292

This should be reverted after the corresponding branch on
DataDog/vaccine is merged.
This narrows down the triggered workflow run to the exact one created by
the GitLab CI pipeline.

A number of corner cases were improperly handled, and there was an
opportunity for a secret leak, which led to a full script rewrite.
@lloeki lloeki requested a review from a team as a code owner February 19, 2025 09:31
@pr-commenter
Copy link

pr-commenter bot commented Feb 19, 2025

Benchmarks

Benchmark execution time: 2025-02-19 10:15:39

Comparing candidate commit ca2935b in PR branch lloeki/fix-cross-repo-race-condition with baseline commit 908154a in branch master.

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

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.70%. Comparing base (908154a) to head (ca2935b).
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4404   +/-   ##
=======================================
  Coverage   97.70%   97.70%           
=======================================
  Files        1366     1366           
  Lines       83372    83373    +1     
  Branches     4230     4230           
=======================================
+ Hits        81462    81463    +1     
  Misses       1910     1910           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Feb 19, 2025

Datadog Report

Branch report: lloeki/fix-cross-repo-race-condition
Commit report: ca2935b
Test service: dd-trace-rb

✅ 0 Failed, 20596 Passed, 1372 Skipped, 3m 18.5s Total Time

@TonyCTHsu TonyCTHsu added the dev/tooling Involves tools (e.g. Rubocop, CodeCov) label Feb 19, 2025
@TonyCTHsu TonyCTHsu enabled auto-merge February 19, 2025 09:54
@TonyCTHsu TonyCTHsu merged commit 18de083 into master Feb 19, 2025
487 checks passed
@TonyCTHsu TonyCTHsu deleted the lloeki/fix-cross-repo-race-condition branch February 19, 2025 10:18
@github-actions github-actions bot added this to the 2.11.0 milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/tooling Involves tools (e.g. Rubocop, CodeCov)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants