Skip to content

feat: wake up Promotions waiting on PRs via pull_request webhooks#5852

Open
DavidS-ovm wants to merge 56 commits intoakuity:mainfrom
overmindtech:feat/webhook-pr-promotion-wakeup
Open

feat: wake up Promotions waiting on PRs via pull_request webhooks#5852
DavidS-ovm wants to merge 56 commits intoakuity:mainfrom
overmindtech:feat/webhook-pr-promotion-wakeup

Conversation

@DavidS-ovm
Copy link
Copy Markdown
Contributor

@DavidS-ovm DavidS-ovm commented Mar 4, 2026

Summary

When a GitHub pull_request event with a closed action arrives, Kargo can now
immediately re-reconcile any running Promotion whose current git-wait-for-pr
step is waiting on that PR — reducing detection latency from ~5 minutes (polling)
to near-instant. Polling remains as a reliability fallback.

Closes #5834. Related: #3303, #647, #4969, #5450

Design

This follows the approach discussed in
#5450 (comment)
extending the existing webhook infrastructure rather than introducing a new
mechanism.

Indexer — A new RunningPromotionsByPullRequestField indexer maps
{normalizedRepoURL}:{prNumber} to running Promotions. It follows the
RunningPromotionsByArgoCDApplications pattern: filtering to running
Promotions, iterating steps up to CurrentStep, evaluating template
expressions for repoURL and prNumber, and normalizing Git URLs. Registered
in the external webhook server.

Webhook handler — The GitHub receiver now accepts pull_request events.
On closed action (merged or not), it queries the index with both the clone
URL and SSH URL from the event payload, de-dupes matching Promotions by
ObjectKey, and calls api.RefreshObject on each. Non-closed actions return
200 OK with no side effects. The new refreshPromotions helper in refresh.go
mirrors refreshWarehouses and is provider-agnostic so future GitLab/Bitbucket
handlers can reuse it.

Why only closed? A merged PR fires closed with merged: true; the
git-wait-for-pr step detects the merge on next reconcile and succeeds. A PR
closed without merge fires closed with merged: false; the step detects
closure and fails with a terminal error. All other actions (opened,
synchronize, etc.) are irrelevant to the step's outcome.

User-facing changes

  • Users must add pull_request to the event types on their GitHub webhook
    configuration (one-time change). Users who do not make this change experience
    no behavior change.
  • No new CRD fields, no new CLI flags, no breaking changes.

Documentation

  • GitHub webhook receiver page: added pull_request to supported events, added
    Promotion "refreshing" explanation, updated event selection instructions for
    all three setup methods (single repo, org, GitHub App)
  • git-wait-for-pr step reference: added a tip about webhook acceleration
  • Webhook receivers index, Working with Warehouses guide, and Cluster
    Configuration guide: broadened webhook framing to mention Promotion refresh

Test plan

  • 15 table-driven unit tests for the indexer covering: expression evaluation,
    literal values, non-git-wait-for-pr steps ignored, non-running Promotions
    excluded, multiple PRs across steps, URL normalization (clone URL, SSH URL),
    template expressions with outputs and vars, steps beyond CurrentStep
  • 5 unit tests for the webhook handler covering: closed+merged refreshes
    Promotion, closed+not-merged refreshes Promotion, non-closed action returns
    200, no matching Promotions returns 200 with 0 refreshed, SSH URL form matches
  • All existing tests pass unchanged

Signed-off-by: David Schmitt david@overmind.tech

@DavidS-ovm DavidS-ovm requested review from a team as code owners March 4, 2026 21:13
@netlify
Copy link
Copy Markdown

netlify bot commented Mar 4, 2026

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit 3d6dca1
🔍 Latest deploy log https://app.netlify.com/projects/docs-kargo-io/deploys/69d4d48c605a2b0008c6ce18
😎 Deploy Preview https://deploy-preview-5852.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 86.78815% with 58 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.33%. Comparing base (8fdd7f4) to head (3d6dca1).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
pkg/webhook/external/refresh.go 55.55% 16 Missing and 4 partials ⚠️
pkg/webhook/external/bitbucket.go 77.77% 15 Missing and 3 partials ⚠️
cmd/controlplane/external_webhooks.go 0.00% 8 Missing ⚠️
pkg/webhook/external/gitea.go 90.90% 5 Missing and 1 partial ⚠️
pkg/webhook/external/gitlab.go 94.33% 2 Missing and 1 partial ⚠️
pkg/gitprovider/azure/azure.go 77.77% 1 Missing and 1 partial ⚠️
pkg/webhook/external/azure.go 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5852      +/-   ##
==========================================
+ Coverage   57.03%   57.33%   +0.29%     
==========================================
  Files         463      470       +7     
  Lines       39112    39961     +849     
==========================================
+ Hits        22309    22911     +602     
- Misses      15474    15681     +207     
- Partials     1329     1369      +40     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@krancour krancour added kind/enhancement An entirely new feature kind/proposal Indicates maintainers have not yet committed to a feature request area/external-webhooks Affects the server that responds to webhooks originating from external systems needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed labels Mar 19, 2026
Comment on lines +307 to +310
func RunningPromotionsByPullRequest(
ctx context.Context,
cl client.Client,
) client.IndexerFunc {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We might be doing a lot more than is necessary to build this index. I think all the expression evaluation stuff probably isn't necessary. The PR URL is in a git-open-pr step's output. That value will always be static; not an expression. Isn't that sufficient information for inferring a relationship between a PR and a Promotion?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was an interesting one. I was under the impression that running steps didn't have outputs yet (but then I only looked at our 1.8 install, and maybe just the wrong steps, or it doesn't show up in the app). Since then I've updated to 1.9 and looked at our git-wait-for-pr step, and indeed, the required values are right there! So I removed the whole index and replaced it with a small check of those outputs.

This might have a small window where github is fast enough to deliver an event but the step hasn't yet updated - maybe if people are doing weird stuff in the stage between opening the PR and waiting for the merge, but in that case either the task already sees the merged PR and proceeds, or - worst case - the polling loop will catch it later.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be good if we didn't do this only for GitHub, but for all git hosting providers that support a similar event.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree that that would be good.

I don't have infrastructure to test, nor budget to support anything other than what my team uses.

@krancour
Copy link
Copy Markdown
Member

I have this an initial look, but wondering if @fuskovic would be so kind as to take over as the sherpa here.

@krancour krancour removed their assignment Mar 19, 2026
@krancour krancour added priority/normal This is the priority for most work and removed kind/proposal Indicates maintainers have not yet committed to a feature request needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed labels Mar 19, 2026
Add an indexer that maps running Promotions to the pull requests they
are waiting on via git-wait-for-pr steps. Index keys use the form
"{normalizedRepoURL}:{prNumber}", enabling efficient lookup when a
webhook arrives for a specific PR.

The indexer follows the existing RunningPromotionsByArgoCDApplications
pattern: filtering to running Promotions, iterating steps up to
CurrentStep, evaluating template expressions for repoURL and prNumber,
and normalizing Git URLs. It is registered in both the promotion
controller and the external webhook server.

Signed-off-by: David Schmitt <david.schmitt@overmind.tech>
Extend the GitHub webhook receiver to accept pull_request events. On
"closed" action (merged or not), look up running Promotions waiting on
the PR via the RunningPromotionsByPullRequestField index and call
RefreshObject on each to trigger immediate re-reconciliation. Non-closed
actions return 200 OK with no side effects.

The new refreshPromotions helper in refresh.go mirrors refreshWarehouses:
it queries both clone URL and SSH URL forms, de-dupes by ObjectKey, and
is provider-agnostic so future GitLab/Bitbucket handlers can reuse it.

Signed-off-by: David Schmitt <david.schmitt@overmind.tech>
…tions

Update the GitHub webhook receiver page to describe the new
pull_request event handling and add pull_request to the event selection
instructions across all three setup methods (single repo, org, GitHub
App). Add a webhook acceleration tip to the git-wait-for-pr step
reference. Broaden the webhook receivers index page, Working with
Warehouses guide, and Cluster Level Configuration guide to mention
Promotion refresh alongside Warehouse discovery.

Signed-off-by: David Schmitt <david.schmitt@overmind.tech>
The function takes a PR number as a use-case-specific argument, unlike
refreshWarehouses whose parameters are intrinsic to any Warehouse.
Using a more specific name leaves room for future
refreshPromotionsBy*() functions triggered by other events (e.g.
deployment health checks, external approvals).

Signed-off-by: David Schmitt <david.schmitt@overmind.tech>
Replace the expression-evaluation-based RunningPromotionsByPullRequest
indexer with a simpler version that reads PR data directly from
git-wait-for-pr step outputs in promo.Status.State.

The previous implementation (~170 lines) fetched Stage objects, built
promotion contexts, created step evaluators, and template-evaluated
repoURL/prNumber config fields. This was over-engineered and had a
latent bug: freightMetadata() in config fields (not vars) could not
be evaluated because the indexer did not pass the required expr options.

The new implementation (~40 lines) reads the already-resolved PR data
(pr.id, pr.url) from each git-wait-for-pr step's output in State.
The step writes this output on every reconciliation cycle, even when
the PR is still open, making expression evaluation unnecessary.

This also removes the ctx/client.Client parameters from the function
signature, making it a plain client.IndexerFunc like other indexers.

This also removes the Stage informer registration that is not needed
anymore.

Signed-off-by: David Schmitt <david.schmitt@overmind.tech>
… lookup

Remove the RunningPromotionsByPullRequestField index registration from
the promotion controller — it is only queried by the external webhooks
server and adds unnecessary memory and CPU overhead to every Promotion
status update.

Remove the SSH URL from the pull_request webhook handler's index lookup.
The index keys are always built from the HTTPS repoURL literal in
git-wait-for-pr step config, so the SSH form never matches.

Signed-off-by: David Schmitt <david.schmitt@overmind.tech>
fuskovic added 26 commits April 3, 2026 17:06
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
… accordingly

Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
…rver

Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Signed-off-by: fuskovic <fhuskovic92@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/external-webhooks Affects the server that responds to webhooks originating from external systems kind/enhancement An entirely new feature priority/normal This is the priority for most work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wake up Promotions waiting on git-wait-for-pr via pull_request webhooks

3 participants