-
Notifications
You must be signed in to change notification settings - Fork 17
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
GitHub Actions #112
base: main
Are you sure you want to change the base?
GitHub Actions #112
Conversation
Signed-off-by: Matej Focko <[email protected]>
Signed-off-by: Matej Focko <[email protected]>
In case of utilizing API via GitHub Actions there is a risk of modifying actions that could result | ||
in leaking of the secrets. GitHub mitigates this by censoring the output of GitHub Actions, but with | ||
our use-case it would be still possible to (indirectly) modify scripts that are run and leak the | ||
secrets. |
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.
Secrets shouldn't be available during pull-request workflows ... and push workflows are fully under the maintainer's control.
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.
This is handy but will probably cause problems with the Copr builds for PRs. How we can do this?
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.
Ad pull request workflow, they disable secrets by default (with an override): https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#potential-impact-of-a-compromised-runner
In ideal case I would like to split the action to multiple parts and pass the secrets to the environment only for calling packit that directly queries API.
Follow-up question:
- Do we always create SRPM for
packit copr-build
, even if there is an SRPM already present?
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 **always** create SRPM for `packit copr-build`, **even if** there is an SRPM already present?
I think so, but if we want to change this and reuse the existing SRPM, this is easy to do. Maybe as another CLI argument?
- If we were to trigger Copr builds with SRPM created in GitHub Action we introduce blocking action of | ||
acquiring the SRPM from GitHub. |
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.
we could potentially trigger p-s' API from the action and POST the SRPM
would be interesting to discuss alternative solutions for this step
- No cache is present, which also introduces: | ||
- slower build time | ||
- potential time out, since Actions are time limited (iirc 24 hrs per action) |
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.
this is a good point, since job scheduling would be completely out of our control
In case we do not provide specialized images on which Packit can be run, we can run into issues with | ||
different versions of Packit being used (RPM vs PyPI vs images). |
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'm afraid we'd need to provide packit in a container where the env is in full control from us
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.
Thanks for starting with this. Do you want to meet and discuss it to move this forward?
I am mostly thinking about the implementation part and the whole approach. (Do I see correctly that this will be a wrapper on top of Packit's API and will use the user's secrets?)
#### Testing farm via API from GitHub Action | ||
|
||
_TODO_ |
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.
Ideally, leave this to the Testing Farm team (=Miro) and allow some integration.
In case of utilizing API via GitHub Actions there is a risk of modifying actions that could result | ||
in leaking of the secrets. GitHub mitigates this by censoring the output of GitHub Actions, but with | ||
our use-case it would be still possible to (indirectly) modify scripts that are run and leak the | ||
secrets. |
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.
This is handy but will probably cause problems with the Copr builds for PRs. How we can do this?
Rough outline of work
|
I want to point out an important use-case abouy this. This is necessary when upstream uses release actions, e.g. uploading to pypi. In this case it creates a race condition with |
TODO: