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

Add org filter to webhook. #476

Merged
merged 2 commits into from
Sep 3, 2024
Merged

Add org filter to webhook. #476

merged 2 commits into from
Sep 3, 2024

Conversation

wlynch
Copy link
Collaborator

@wlynch wlynch commented Aug 30, 2024

We need to test out behavior in the webhook, but we're not 100% confident it's working as intended as is. This adds a configurable filter to enable selective responses to certain orgs.

For now, have the default deployment only look at octo-sts and chainguard-dev orgs.

@karlhaworth
Copy link
Contributor

Might be missing something here, but let's say the web hook doesn't stop the config from being merged. What in this PR is stopping the app from handing out the token anyways to a non-specified org?

@wlynch
Copy link
Collaborator Author

wlynch commented Aug 30, 2024

Great question! It doesn't 😅 This is not related to #469.

The webhook is currently disabled on webhook.octo-sts.dev because of #288.
We want to re-enable it, but we want to be careful not to start spamming everyone who has the app installed currently. This change effectively adds a preview flag to only have the webhook respond to certain orgs so we can make the fixes we need and validate it before we drop the flag and enable it for everyone.

We need to test out behavior in the webhook, but we're not 100%
confident it's working as intended as is. This adds a configurable
filter to enable selective responses to certain orgs.

For now, have the default deployment only look at octo-sts and
chainguard-dev orgs.
cpanato
cpanato previously approved these changes Sep 3, 2024
Copy link
Contributor

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

small nit comment, lgtm

thanks

pkg/webhook/webhook.go Show resolved Hide resolved
Copy link
Contributor

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

cool

@wlynch wlynch merged commit ccd3cf8 into octo-sts:main Sep 3, 2024
12 checks passed
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.

3 participants