RedHatQuickCourses / rhods-intro

Introduction to Red Hat OpenShift AI (RHOAI)
https://redhatquickcourses.github.io/rhods-intro/
13 stars 14 forks source link

Adjust CI to work with forks #25

Closed jramcast closed 9 months ago

jramcast commented 11 months ago

The GitHub actions pipeline fails when a PR is created from a fork. We should fix that.

rsriniva commented 9 months ago

@jramcast - is this even possible? or can we close this?

jramcast commented 9 months ago

I assume it's possible and would allow us to accept PRs from forks. Right now all the PRs open from forks make the pipeline fail.

stencell commented 9 months ago

We need to make sure this is possible, there should be no reason why it isn't. Developing in a fork and creating PRs is way more normal than being able to do this in the original repo. Plus, it removes the need for us to manage any access to these repositories now or in the future.

rsriniva commented 9 months ago

@jramcast - any ideas on how to fix this? is it even possible to implement this in GitHub actions?

tagging @psolarvi for ideas

jramcast commented 9 months ago

@rsriniva In the pipeline definition we use the rossjrw/pr-preview-action@v1 GitHub action. As the author explains in their repo:

This Action does not currently support deploying previews for PRs from forks, but will do so in https://github.com/rossjrw/pr-preview-action/pull/6.

So we might want to take a deeper look of a workaround or maybe just switch to a different GH action that supports forks.

rsriniva commented 9 months ago

@stencell - this does not prevent folks from submitting PRs in our repo. It;s just the HTML preview that fails. Looking at the linked PR from upstream for a patch - the GH actions author is concerned about severe security risks (I agree with his assessment) and he will not merge this into the main plugin.

@jramcast - can we do this instead - is there a way for GH action to check if PR is created in our repo and only then run the HTML preview? If we get a PR from a fork, then we simply disable HTML preview builds and just send notifications. Once we merge, then the HTML re-gen runs anyway correct?

jramcast commented 9 months ago

@rsriniva Definitely. That sounds like an easy fix https://github.com/orgs/community/discussions/25217

rsriniva commented 9 months ago

fixed in main. thanks for the tip @jramcast