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

Allow build-* workflows to be ran on demand on any git ref #11221

Closed
wants to merge 4 commits into from

Conversation

Tyrrrz
Copy link
Contributor

@Tyrrrz Tyrrrz commented Sep 24, 2024

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-12022

📔 Objective

  • Streamline the community PR handling process by getting rid of the "Clone PR" stage. This is achieved by doing the following:
    • Allow build-*.yml workflows to run on manual dispatch. Add an input parameter called checkout_ref that allows the actor to manually specify any commit hash, tag, or branch to run the workflow against. This can be used with a commit hash from a 3rd-party fork.
    • Create an umbrella build-all.yml workflow that can run on manual dispatch. This workflow simply calls all other build-*.yml workflows.

With this approach, instead of going through the lengthy and wasteful process of rehoming the community PR branch via a Clone PR, a Bitwarden developer can simply manually run the build workflows (all or just a few specific ones) on the community PR branch. Then, they can include the link to the corresponding artifacts in the QA testing notes.

Because the trigger of the workflow is not a pull_request event, but a workflow_dispatch event, the corresponding run will have full access to secrets. While this is risky, it's not inherently any less secure than the current Clone PR process, which is just a messy workaround for doing the same thing -- triggering build workflows with elevated privileges on a PR branch from an outside fork.

📸 Screenshots

  • Running all build workflows for a specific git ref (in this case, the latest commit on this community PR):
image

(once this is merged, you will be able to do this from GitHub Web as well)

image

(as you can see, all jobs are running despite the target being a branch from an outside fork)

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.16%. Comparing base (01e530d) to head (8dc2101).
Report is 209 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11221      +/-   ##
==========================================
+ Coverage   35.07%   35.16%   +0.09%     
==========================================
  Files        2710     2719       +9     
  Lines       84550    84926     +376     
  Branches    16070    16158      +88     
==========================================
+ Hits        29656    29868     +212     
- Misses      53923    54080     +157     
- Partials      971      978       +7     

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

Copy link
Contributor

github-actions bot commented Sep 24, 2024

Logo
Checkmarx One – Scan Summary & Detailsf171af9a-84f5-443f-bd91-8cdab320ff92

New Issues

Severity Issue Source File / Package Checkmarx Insight
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1333 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 283 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 205 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 447 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1379 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 929 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 313 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 309 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-cli.yml: 429 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 406 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...
MEDIUM Unpinned Actions Full Length Commit SHA /build-web.yml: 358 Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps...

Fixed Issues

Severity Issue Source File / Package
MEDIUM Angular_Improper_Type_Pipe_Usage /libs/components/src/navigation/nav-divider.component.html: 1
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 379
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1328
MEDIUM Unpinned Actions Full Length Commit SHA /build-browser.yml: 420
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 1282
MEDIUM Unpinned Actions Full Length Commit SHA /build-desktop.yml: 884
MEDIUM Unpinned Actions Full Length Commit SHA /build-cli.yml: 405

@Tyrrrz Tyrrrz marked this pull request as ready for review September 24, 2024 15:36
@Tyrrrz Tyrrrz requested a review from a team as a code owner September 24, 2024 15:36
@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Sep 24, 2024

Looks like some builds are failing because a few workflows run twice (for example, build-desktop.yml includes build-browser.yml and also build-browser.yml runs on its own) and there is naming collision when it comes to artifacts:

image

The easy solution is to enable overwrite: true, which is what I did. However, I think it would be nicer if we figure out how to avoid running duplicate builds to reduce wasting time and resources.

@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Sep 24, 2024

Semi-successful run: https://github.com/bitwarden/clients/actions/runs/11017250075/job/30595187922

Not sure what to make of that error. Any ideas? @withinfocus

@djsmith85
Copy link
Contributor

djsmith85 commented Oct 14, 2024

Closing this for now, but we'll continue to look into enabling easier workflow execution for community PRs. Likely by removing or delaying access to secrets for workflows.

Thanks again @Tyrrrz for looking into this!

@djsmith85 djsmith85 closed this Oct 14, 2024
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.

2 participants