-
Notifications
You must be signed in to change notification settings - Fork 3
Create composite action to create constructor-based installers #63
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
base: main
Are you sure you want to change the base?
Conversation
action.yaml
Outdated
description: | | ||
Path to the environment.yaml file used to create the build environment. | ||
Takes precedence over envrionemnt-yaml-string. | ||
The file must not have the `name` property. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if it does? Ignored or error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I mixed something up here - name
can be used in the file, so I removed the comment.
CONSTRUCTOR_WORKSPACE=$(cygpath "${CONSTRUCTOR_WORKSPACE}") | ||
fi | ||
mkdir -p "${CONSTRUCTOR_WORKSPACE}" | ||
chmod 777 "${CONSTRUCTOR_WORKSPACE}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need 777?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, yes. For builds inside a container, the UID and GIDs are not the same for the runner and the container image, and you need x
permissions to access that directory. If we want to mitigate the risks here somewhat, we can set those permissions only for those builds that use a container image.
Would be interesting to run https://github.com/woodruffw/zizmor on this repository to catch some potential security risks. |
Co-authored-by: jaimergp <[email protected]>
Thank you for your review! I think I addressed your specific comments. Also thanks for recommending zizmor - I ran the checks and made several recommended changes. The chief concern was template injection vulnerabilities, so I replaced most with environment variables. The reason why I employed template substitutions is that adding environment variables into the To mitigate template injection vulnerabilities, I added an input validation step and added tests that try and inject a To be specific, these are the templates in the build step
If you see risks despite the input validation, we will have to accept the more cumbersome maintenance model though. The input validation is a good addition either way. |
This PR creates a composite action to build
constructor
-based installers. The goal of this action is to unify the build workflows for both Anaconda and Miniforge installers and pool maintenance resources.The action takes or creates an
environment.yaml
to create a build environment. Inside that build environment, it runsconstructor
on a provided recipe directory to create the installer. The output directory contains allconstructor
-created build artifacts.The workflow is intended to be flexible and can:
conda
installations on the runner/image or useconda-standalone
/micromamba
to create the build environment.constructor
's interface for signing installers.The README file contains several use cases and I added tests for each of them.
Closes #3