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

Skip riff-raff-artifact so fork PRs do not crash on roleArn missing #27310

Merged

Conversation

rtyley
Copy link
Member

@rtyley rtyley commented Jul 12, 2024

As mentioned in github/docs#6861:

It is common for projects to include a secret for deploying things and an action that uses that secret.

When one forks that repository, the secret won't be present and the workflow will break.

This experience is incredibly painful.

For guardian/frontend, we're using actions-riff-raff, which needs a roleArn which is provided as a repository secret. For PRs like #27233, from an external contributor, the CI appears to fail, even though all tests have passed.

CI never runs on forks unless we authorise the contributor

The GitHub docs say:

By default, all first-time contributors require approval to run workflows.

...so it could be argued that ideally, the build should still produce the riff-raff artifact for fork PRs.

As mentioned in github/docs#6861:

> It is common for projects to include a secret for deploying things and an action that uses that secret.
>
> When one forks that repository, the secret won't be present and the workflow will break.
>
> This experience is incredibly painful.

For guardian/frontend, we're using [actions-riff-raff](https://github.com/guardian/actions-riff-raff), which needs a `roleArn` which is provided as a repository secret. For PRs like #27233, from an external contributor, the CI appears to fail, even though all tests have passed.

### CI never runs on forks unless we authorise the contributor

The [GitHub docs](https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks#approving-workflow-runs-on-a-pull-request-from-a-public-fork) say:

> By default, all first-time contributors require approval to run workflows.

...so it could be argued that ideally, the build should still produce the riff-raff artifact for fork PRs.
@rtyley rtyley force-pushed the skip-riff-raff-artifact-so-forks-do-not-crash-on-roleArn-missing branch from 51979cf to d8e373a Compare July 25, 2024 15:27
@rtyley rtyley marked this pull request as ready for review July 25, 2024 15:28
@rtyley rtyley requested a review from a team as a code owner July 25, 2024 15:28
@rtyley
Copy link
Member Author

rtyley commented Jul 25, 2024

@ioannakok I think we left this in draft while we were waiting to check that riff-raff-artifact did get run for internal contributors - and having waited for the build to run, we can see that riff-raff-artifact does get run, so I think this is ready for merge!

@rtyley rtyley merged commit 0aa6316 into main Jul 25, 2024
6 checks passed
@rtyley rtyley deleted the skip-riff-raff-artifact-so-forks-do-not-crash-on-roleArn-missing branch July 25, 2024 15:50
@prout-bot
Copy link
Collaborator

Seen on FRONTS-PROD, ADMIN-PROD (merged by @rtyley 11 minutes and 6 seconds ago)

@ioannakok ioannakok mentioned this pull request Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants