-
Notifications
You must be signed in to change notification settings - Fork 629
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
fix: webhook expects REPOSITORY_ALLOW_LIST env var #3856
Conversation
Thanks for creating the PR. This problem was noticed already in the PR #3830. As mentioned in that PR it is great to fix this. But we are planning the depcreate this feture as well. The feature is from the early days of the module before runner groups were existing. Wondering what the use case is for this allow list. |
Oh, I see. Sorry about that, I somehow missed this in a search. To be fair, I only noticed it when working on an unrelated feature (#3855) and quickly put up the fix PR. I totally understand the plans to deprecate the feature altogether. One use case I did have was to enable a 2-step verification process of allowing new repositories to access the self-hosted runners. As in, it is not enough to allow webhook delivery to a new repository in the GitHub App, but you also have to explicitly modify the infrastructure (i.e. modify the allow list). Thank you for the explanation and the link 🙇 |
Thanks for the feedback. In case we have the option to check the event is part of the group for which it is scaling the runners. Would that be good enough. Implementing this is not straighforward due to a lack of GitHub api's. |
🤖 I have created a release *beep* *boop* --- ## [5.10.1](philips-labs/terraform-aws-github-runner@v5.10.0...v5.10.1) (2024-04-24) ### Bug Fixes * Add missing webhook_events_workflow_job_queue_policy to multi-runner queue ([#3848](https://github.com/philips-labs/terraform-aws-github-runner/issues/3848)) ([a8cba4e](philips-labs/terraform-aws-github-runner@a8cba4e)) * **lambda:** bump the aws group in /lambdas with 5 updates ([#3861](https://github.com/philips-labs/terraform-aws-github-runner/issues/3861)) ([6119354](philips-labs/terraform-aws-github-runner@6119354)) * **lambda:** bump typescript from 5.3.3 to 5.4.5 in /lambdas ([#3863](https://github.com/philips-labs/terraform-aws-github-runner/issues/3863)) ([e3f3d77](philips-labs/terraform-aws-github-runner@e3f3d77)) * webhook expects REPOSITORY_ALLOW_LIST env var ([#3856](https://github.com/philips-labs/terraform-aws-github-runner/issues/3856)) ([0006ab9](philips-labs/terraform-aws-github-runner@0006ab9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
Description
The
ConfigResolver
of the webhook lambda expects the env variable to be namedREPOSITORY_ALLOW_LIST
(seeterraform-aws-github-runner/lambdas/functions/webhook/src/ConfigResolver.ts
Line 18 in 1e40ecd
webhook
module wasREPOSITORY_WHITE_LIST
. I think it'd be great to follow up on #992, but it is beyond the scope of the fix proposed here now.