Skip to content

refactor(release-pipeline): Ignore permissions from pipeline & better modifications - #6246

Merged
Aradhya-Tripathi merged 6 commits into
developfrom
pipeline-refactors
Apr 24, 2026
Merged

refactor(release-pipeline): Ignore permissions from pipeline & better modifications#6246
Aradhya-Tripathi merged 6 commits into
developfrom
pipeline-refactors

Conversation

@Aradhya-Tripathi

@Aradhya-Tripathi Aradhya-Tripathi commented Apr 24, 2026

Copy link
Copy Markdown
Contributor
  • All records created via the pipeline (deploy candidate, deploy candidate build) must be without a permissions check.
  • Updating pipeline status is causing timestamp errors correctly handle them
  • Just get the latest build for the Deploy Candidate
    • Avoiding things like fetching a previously failed build instead of a current running retried build

@greptile-apps

greptile-apps Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR threads an ignore_permissions_check flag through the release pipeline's document-creation chain (BenchUpdate.deployReleaseGroup.create_deploy_candidateDeployCandidate.schedule_build_and_deploy) so that DeployCandidate and DeployCandidateBuild records inserted by pipeline tasks bypass Frappe permission checks. It also fixes a test class-setup bug where server was a local variable instead of a class attribute.

  • DeployCandidateBuild.insert() still runs without ignore_permissions on the immediate build path: schedule_build_and_deploy delegates to self.build_and_deploy() when run_now=True and the system is not suspended; that helper calls .insert() with no bypass.
  • initiate_pre_build_validations is unprotected: this pipeline task calls candidate.schedule_build_and_deploy() without ignore_permissions_check=True, leaving DeployCandidateBuild creation subject to permissions there too.

Confidence Score: 4/5

Two P1 gaps mean the permissions bypass is incomplete — the most common runtime path and a separate pipeline task both still insert DeployCandidateBuild with full permission checks.

The refactor correctly wires ignore_permissions_check through most of the chain, but the immediate build_and_deploy() path (the normal production path when not suspended) and the initiate_pre_build_validations task both miss the flag. These are present defects on the hot path, not theoretical risks.

press/press/doctype/deploy_candidate/deploy_candidate.py (build_and_deploy helper) and press/press/doctype/release_pipeline/release_pipeline.py (initiate_pre_build_validations task).

Important Files Changed

Filename Overview
press/press/doctype/deploy_candidate/deploy_candidate.py Adds ignore_permissions_check to schedule_build_and_deploy, but the flag is only applied in the scheduled/suspended branch; the immediate build_and_deploy() path still inserts without ignore_permissions.
press/press/doctype/release_pipeline/release_pipeline.py create_deploy_candidate correctly hardcodes ignore_permissions_check=True, but initiate_pre_build_validations calls schedule_build_and_deploy() without the flag, leaving that task unprotected.
press/press/doctype/release_group/release_group.py Adds ignore_permissions_check parameter and threads it through to new_dc.insert() — straightforward and correct.
press/press/doctype/bench_update/bench_update.py Correctly adds ignore_permissions_check to deploy() and propagates it to both create_deploy_candidate and schedule_build_and_deploy.
press/press/doctype/release_pipeline/test_release_pipeline.py Fixes server variable scoping bug (was a local var, now stored as cls.server) and adds release_group_name parameter to create_deploy_and_update for flexibility.

Sequence Diagram

sequenceDiagram
    participant RP as ReleasePipeline
    participant BU as BenchUpdate
    participant RG as ReleaseGroup
    participant DC as DeployCandidate
    participant DCB as DeployCandidateBuild

    RP->>BU: get_bench_update(..., ignore_permissions_check=True)
    BU->>BU: insert(ignore_permissions=True)
    RP->>BU: deploy(..., ignore_permissions_check=True)
    BU->>RG: create_deploy_candidate(..., ignore_permissions_check=True)
    RG->>DC: new_dc.insert(ignore_permissions=True) ✅
    BU->>DC: schedule_build_and_deploy(ignore_permissions_check=True)
    alt run_now=True AND not suspended (hot path)
        DC->>DCB: build_and_deploy() → insert() ❌ NO ignore_permissions
    else scheduled / suspended
        DC->>DCB: insert(ignore_permissions=True) ✅
    end
    RP->>DC: initiate_pre_build_validations()
    DC->>DCB: schedule_build_and_deploy() ❌ NO ignore_permissions_check
Loading

Comments Outside Diff (1)

  1. press/press/doctype/release_pipeline/release_pipeline.py, line 244-249 (link)

    P1 initiate_pre_build_validations missing ignore_permissions_check

    The PR description states that all records created via the pipeline must bypass permission checks. create_deploy_candidate (line 225) correctly passes ignore_permissions_check=True throughout, but initiate_pre_build_validations calls candidate.schedule_build_and_deploy() without the flag. The DeployCandidateBuild record created here will still be subject to the normal permission check, which may cause the same permission failures this PR is trying to prevent.

    deploy_candidate_build = candidate.schedule_build_and_deploy(ignore_permissions_check=True)

Reviews (1): Last reviewed commit: "refactor(pipeline): Ingore permissions c..." | Re-trigger Greptile

Comment thread press/press/doctype/deploy_candidate/deploy_candidate.py Outdated
@codecov

codecov Bot commented Apr 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.93%. Comparing base (f5b48ad) to head (4cf5697).
⚠️ Report is 128 commits behind head on develop.

Files with missing lines Patch % Lines
...press/doctype/deploy_candidate/deploy_candidate.py 75.00% 1 Missing ⚠️
...press/doctype/release_pipeline/release_pipeline.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #6246       +/-   ##
============================================
- Coverage    90.75%   55.93%   -34.82%     
============================================
  Files          109      910      +801     
  Lines        17369    75837    +58468     
  Branches       525      525               
============================================
+ Hits         15763    42422    +26659     
- Misses        1578    33387    +31809     
  Partials        28       28               
Flag Coverage Δ
dashboard 90.75% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@Aradhya-Tripathi
Aradhya-Tripathi merged commit b0f4ba9 into develop Apr 24, 2026
13 of 15 checks passed
@Aradhya-Tripathi
Aradhya-Tripathi deleted the pipeline-refactors branch April 24, 2026 12:26
Aradhya-Tripathi added a commit that referenced this pull request Apr 24, 2026
refactor(release-pipeline): Ignore permissions from pipeline & better modifications (backport #6246)
@frappe-pr-bot

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 0.20.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants