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

build - build with the correct security context #1764

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Conversation

Erwinvandervalk
Copy link
Contributor

What issue does this PR address?
This PR changes the build so that a build runs only once, and in the right security context, depending on their origins.

Previously, the build used pull_request_target. Effectively, this means that the entire context (including which code to build) was pointing to the target of the PR (likely main). This means it wasn't building anything in the PR, but actually building main (not very useful in a PR workflow)

How does it work?

  1. Builds / pr's from contributors are run trusted and can access security tokens needed for signing. If you create a branch in the DuendeSoftware repo, then you'll automatically get a CI build for this. You can (must) create a PR for this to get it merged. When you create a PR, the build script will detect that there is still a branch for it and will built it normally (skip the PR build).

  2. Builds from PR's created from external contributors (which have to originate from a different fork) will NOT run with security tokens, so artifacts will not be signed / pushed.

Publishing artifacts somewhere from external contributors.

/
If we ever want to to anything with the output of external pr's other than merge them into main (IE: deploy them somewhere), then we should:
a. let the build publish the artifacts to github.
b. setup a different workflow that will run only if the after the previous build is complete AND if pr is labelled (which only contributors can do) This workflow can then publish the changes.

Important: Any code or remarks in your Pull Request are under the following terms:

If You provide us with any comments, bug reports, feedback, enhancements, or modifications proposed or suggested by You for the Software, such Feedback is provided on a non-confidential basis (notwithstanding any notice to the contrary You may include in any accompanying communication), and Licensor shall have the right to use such Feedback at its discretion, including, but not limited to the incorporation of such suggested changes into the Software. You hereby grant Licensor a perpetual, irrevocable, transferable, sublicensable, nonexclusive license under all rights necessary to incorporate and use your Feedback for any purpose, including to make and sell any products and services.

(see our license, section 7)

@Erwinvandervalk Erwinvandervalk added area/all Related to all components impact/non-breaking The fix or change is not a breaking one labels Feb 5, 2025
@Erwinvandervalk Erwinvandervalk self-assigned this Feb 5, 2025
@Erwinvandervalk Erwinvandervalk merged commit 1dee810 into main Feb 5, 2025
15 checks passed
@Erwinvandervalk Erwinvandervalk deleted the ev/build branch February 5, 2025 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/all Related to all components impact/non-breaking The fix or change is not a breaking one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants