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

Webhook management #683

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

blaketigges
Copy link
Contributor

Adds a feature to allow gcpe to add webhooks to every project added by name or wildcard then have the option to remove them on shutdown.

@coveralls
Copy link

Coverage Status

coverage: 74.052% (-1.6%) from 75.617% when pulling 23852b5 on blaketigges:webhook-manager into 2024af8 on mvisonneau:main.

Copy link
Owner

@mvisonneau mvisonneau left a comment

Choose a reason for hiding this comment

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

thanks a lot for this feature @blaketigges and sorry for not taking the time to look into it earler. There are a few nits regarding nomenclature which I feel would be easier for users to leverage the solution. Happy to look into it if you do not plan to work further onto this though.

@@ -50,6 +50,23 @@ server:
# environment variable)
secret_token: 063f51ec-09a4-11eb-adc1-0242ac120002

add_webhooks:
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
add_webhooks:
manage_projects:

Comment on lines +64 to +65
# GCPE Webhook endpoint URL
webhook_url: https://gcpe.example.net/webhook
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
# GCPE Webhook endpoint URL
webhook_url: https://gcpe.example.net/webhook
# GCPE Webhook endpoint URL accessible from GitLab
advertised_url: https://gcpe.example.net/webhook

Comment on lines +67 to +68
# Whether to remove webhooks when GCPE shutsdown
remove_webhooks: false
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
# Whether to remove webhooks when GCPE shutsdown
remove_webhooks: false
# Whether or not to remove webhooks when GCPE shuts down
remove_on_delete: false

return err
}

WURL := c.Config.Server.Webhook.URL
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
WURL := c.Config.Server.Webhook.URL
webhookURL := c.Config.Server.Webhook.URL

@blaketigges
Copy link
Contributor Author

I don't have the environment set up anymore for testing, can you look into it?

@mvisonneau
Copy link
Owner

no worries, will do thanks! :)

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.

None yet

3 participants