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

Don't run workflow steps that require secrets on forks #3089

Merged
merged 1 commit into from
Jun 28, 2020

Conversation

DasSkelett
Copy link
Member

Problem

Since #3085 we run CKAN's CI with GitHub Actions. An advantage of this is that they also run in forked repositories automatically, without the need to set up an external service.

However, this also leads the workflows trying to execute all the deployment steps and the Discord notification, which doesn't make sense in forked repos and will ultimately fail since the secrets needed to run these steps aren't available.

Changes

This PR adds ifs to all the steps that need secrets to work. These ifs check whether the needed secrets are set, and if not, skip the step.

Also fixed one network test failing since #3084.

@DasSkelett DasSkelett added Bug Something is not working as intended Pull request Build Issues affecting the build system labels Jun 28, 2020
@DasSkelett DasSkelett requested a review from HebaruSan June 28, 2020 00:37
@DasSkelett DasSkelett force-pushed the fix/no-workflow-on-forks branch from 66a961e to 90ef1b3 Compare June 28, 2020 00:49
@DasSkelett DasSkelett added the In progress We're still working on this label Jun 28, 2020
@DasSkelett
Copy link
Member Author

DasSkelett commented Jun 28, 2020

The workflow is not valid. .github/workflows/build.yml (Line: 95, Col: 13): Unrecognized named-value: 'secrets'. Located at position 1 within expression: secrets.DISCORD_WEBHOOK && failure()

Umpf, looks like you can't access the secrets context in the if. Maybe I can setup the env first and use the env vars for the check?

Edit: nice, that worked!

@DasSkelett DasSkelett force-pushed the fix/no-workflow-on-forks branch from b256542 to bd8e3b7 Compare June 28, 2020 01:15
Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@DasSkelett DasSkelett removed the In progress We're still working on this label Jun 28, 2020
@DasSkelett
Copy link
Member Author

Okay, confirmed skipping works: https://github.com/DasSkelett/CKAN/runs/815009204
Let's hope not-skipping works just as well in this repo.

@DasSkelett DasSkelett force-pushed the fix/no-workflow-on-forks branch from bd8e3b7 to e820859 Compare June 28, 2020 01:48
@HebaruSan HebaruSan merged commit 2a92697 into KSP-CKAN:master Jun 28, 2020
@DasSkelett DasSkelett deleted the fix/no-workflow-on-forks branch January 18, 2021 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Build Issues affecting the build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants