Skip to content

[TT-16977] fix: prevent dep-guard from skipping downstream jobs on push#971

Merged
buger merged 1 commit intomasterfrom
fix/dep-guard-skip-master
Apr 17, 2026
Merged

[TT-16977] fix: prevent dep-guard from skipping downstream jobs on push#971
buger merged 1 commit intomasterfrom
fix/dep-guard-skip-master

Conversation

@buger
Copy link
Copy Markdown
Member

@buger buger commented Apr 17, 2026

Summary

  • Add !cancelled() + result checks to all downstream jobs that depend on goreleaser (test-controller-api, api-tests, test-controller-distros, upgrade-deb, upgrade-rpm) to prevent GitHub Actions transitive skip propagation when dep-guard is skipped on push/tag events
  • Add dep-guard to aggregator-ci-test needs for complete status aggregation

Test plan

  • Push to branch triggers all downstream jobs
  • PR still runs dep-guard and blocks on failure

🤖 Generated with Claude Code

Add !cancelled() + result checks to all downstream jobs that depend
on goreleaser to prevent GitHub Actions transitive skip propagation
when dep-guard is skipped on push/tag events.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@buger buger requested a review from a team as a code owner April 17, 2026 11:57
@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Apr 17, 2026

This PR fixes a bug in the CI pipeline where jobs dependent on goreleaser were being skipped during push or tag events. This happened because the upstream dep-guard job is skipped on those events, and GitHub Actions propagates the skipped status transitively.

The fix introduces explicit if conditions to all downstream jobs (test-controller-api, api-tests, test-controller-distros, upgrade-deb, upgrade-rpm). These jobs will now run if their direct dependencies succeed and the workflow hasn't been cancelled, preventing them from being skipped unintentionally.

Additionally, the aggregator-ci-test job now includes dep-guard in its needs list to ensure a complete and accurate status aggregation on pull requests.

Files Changed Analysis

  • .github/workflows/release.yml (+19, -3): All changes are localized to the main release workflow file. The modifications involve adding if conditions to five jobs and updating the dependency list for the aggregation job.

Architecture & Impact Assessment

  • What this PR accomplishes: It ensures that essential testing and packaging jobs run reliably on every push, improving the integrity of the CI process by preventing tests from being silently skipped.
  • Key technical changes introduced: The core change is the addition of if: !cancelled() && needs.<job>.result == 'success' conditions. This alters the job execution trigger from simple dependency completion to explicit success of the dependency, decoupling it from the skipped status of unrelated upstream jobs.
  • Affected system components: The primary impact is on the GitHub Actions CI/CD pipeline defined in release.yml. It modifies job scheduling logic to enhance pipeline reliability.

Job Dependency Flow

graph TD
    subgraph ci_pipeline ["CI Pipeline"]
        dep-guard;
        goreleaser;
        test-controller-api;
        api-tests;
        test-controller-distros;
        upgrade-deb;
        upgrade-rpm;
        aggregator-ci-test;

        dep-guard --> goreleaser;
        goreleaser --> test-controller-api;
        goreleaser --> test-controller-distros;
        test-controller-api --> api-tests;
        test-controller-distros --> upgrade-deb;
        test-controller-distros --> upgrade-rpm;
        
        api-tests --> aggregator-ci-test;
        goreleaser --> aggregator-ci-test;
        dep-guard --|New Dependency|--> aggregator-ci-test;
    end
Loading

Scope Discovery & Context Expansion

  • The change is contained within the CI workflow configuration. Its impact is to harden the build and test process against unexpected skips. While the fix is local, it benefits the entire development lifecycle by ensuring tests are consistently executed.
  • A logical next step for a reviewer would be to inspect the dep-guard job's definition in release.yml to understand the conditions under which it is designed to be skipped. It would also be beneficial to check other workflows for similar transitive dependency patterns that could lead to similar issues.
Metadata
  • Review Effort: 1 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-04-17T12:00:55.222Z | Triggered by: pr_opened | Commit: 90c92b2

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Apr 17, 2026

✅ Security Check Passed

No security issues found – changes LGTM.

Architecture Issues (1)

Severity Location Issue
🟠 Error .github/workflows/release.yml:311-508
The logic in the `if` conditions added to jobs `test-controller-api`, `api-tests`, `test-controller-distros`, `upgrade-deb`, and `upgrade-rpm` does not align with the stated goal of the pull request. The goal is to prevent downstream jobs from being skipped when an upstream dependency is skipped (e.g., on a push event). However, the added condition `needs.<dependency>.result == 'success'` will explicitly cause the job to be skipped if the dependency's result is `'skipped'`. This enforces the default skip propagation behavior rather than preventing it.
💡 SuggestionTo allow these jobs to run when their dependencies have either succeeded or been skipped, the condition should be modified to check for both outcomes. For example, for the `test-controller-api` job, the condition could be `(needs.goreleaser.result == 'success' || needs.goreleaser.result == 'skipped')`. This ensures the jobs run on push events (when dependencies might be skipped) but still do not run if a dependency fails.

✅ Security Check Passed

No security issues found – changes LGTM.

\n\n

Architecture Issues (1)

Severity Location Issue
🟠 Error .github/workflows/release.yml:311-508
The logic in the `if` conditions added to jobs `test-controller-api`, `api-tests`, `test-controller-distros`, `upgrade-deb`, and `upgrade-rpm` does not align with the stated goal of the pull request. The goal is to prevent downstream jobs from being skipped when an upstream dependency is skipped (e.g., on a push event). However, the added condition `needs.<dependency>.result == 'success'` will explicitly cause the job to be skipped if the dependency's result is `'skipped'`. This enforces the default skip propagation behavior rather than preventing it.
💡 SuggestionTo allow these jobs to run when their dependencies have either succeeded or been skipped, the condition should be modified to check for both outcomes. For example, for the `test-controller-api` job, the condition could be `(needs.goreleaser.result == 'success' || needs.goreleaser.result == 'skipped')`. This ensures the jobs run on push events (when dependencies might be skipped) but still do not run if a dependency fails.
\n\n ### Performance Issues (1)
Severity Location Issue
🟠 Error .github/workflows/release.yml:315
The condition `github.event.pull_request.draft == false` will evaluate to `false` on `push` events because the `github.event.pull_request` context object is null. This will prevent this job and other downstream jobs (`api-tests`, `test-controller-distros`, `upgrade-deb`, `upgrade-rpm`) from running on `push` triggers. This appears to contradict the pull request's stated goal of ensuring these jobs run on push events.
💡 SuggestionTo allow the job to run on both `push` events and non-draft `pull_request` events, the condition should explicitly check the event type. Replace `github.event.pull_request.draft == false` with `github.event_name == 'push' || github.event.pull_request.draft == false` in all the newly added `if` blocks for the affected jobs.

Powered by Visor from Probelabs

Last updated: 2026-04-17T11:59:36.706Z | Triggered by: pr_opened | Commit: 90c92b2

💡 TIP: You can chat with Visor using /visor ask <your question>

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@buger buger merged commit 280595e into master Apr 17, 2026
39 of 44 checks passed
@buger buger deleted the fix/dep-guard-skip-master branch April 17, 2026 12:04
@probelabs probelabs Bot changed the title fix: prevent dep-guard from skipping downstream jobs on push [TT-16977] fix: prevent dep-guard from skipping downstream jobs on push Apr 17, 2026
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