Skip to content

fix(lambda): repair repository allow list #3830

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

Closed
wants to merge 2 commits into from
Closed

fix(lambda): repair repository allow list #3830

wants to merge 2 commits into from

Conversation

y3ti
Copy link

@y3ti y3ti commented Mar 28, 2024

I noticed that the environment variable was previously renamed from REPOSITORY_WHITE_LIST to REPOSITORY_ALLOW_LIST. However, this change was only applied in the webhook lambda, while Terraform continues to use the old name.

This pull request corrects the repository allow list functionality without introducing any breaking changes. It's unclear how deprecation and breaking changes are typically handled, so this approach avoids any potential disruptions.

I noticed that the environment variable was previously renamed from
`REPOSITORY_WHITE_LIST` to `REPOSITORY_ALLOW_LIST`. However, this change was
only applied in the `webhook` lambda, while Terraform continues to use the old
name.

This pull request corrects the repository allow list functionality without
introducing any breaking changes. It's unclear how deprecation and breaking
changes are typically handled, so this approach avoids any potential disruptions.
@y3ti
Copy link
Author

y3ti commented Apr 11, 2024

Any chance to review my pull request?
cc: @npalm

@npalm
Copy link
Member

npalm commented Apr 16, 2024

I think the repo allow list was introduced in early days, before having runner groups. Do we still need the allow list. Could you explain the use case? Happy to got it fixed right now. But would prefer if we can remove it at all.

@y3ti
Copy link
Author

y3ti commented Apr 16, 2024

@npalm I don't have any real usecase. Just tested this feature and found the bug :) In my case I set the webhook only for a single github repo.

@npalm
Copy link
Member

npalm commented Apr 18, 2024

@npalm I don't have any real usecase. Just tested this feature and found the bug :) In my case I set the webhook only for a single github repo.

clear, but this means most likely the feature is not used at all anymore. Thanks for you contribution.

@npalm npalm requested a review from GuptaNavdeep1983 April 18, 2024 10:09
@y3ti y3ti closed this Apr 18, 2024
@y3ti y3ti deleted the fix-repository-allow-list branch April 18, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants