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

Automatically Create Webhook #8

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

Automatically Create Webhook #8

wants to merge 3 commits into from

Conversation

kousu
Copy link
Member

@kousu kousu commented Feb 25, 2023

Simplify deployment by dropping the need to create a webhook ourselves.

@kousu kousu changed the base branch from main to output-subdirs February 25, 2023 06:19
@kousu
Copy link
Member Author

kousu commented Feb 25, 2023

Unfortunately this doesn't work because there seems to a recently introduced bug go-gitea/gitea#23139 :(

@kousu kousu force-pushed the auto-webhook branch 3 times, most recently from 13fae9d to 7b00314 Compare February 25, 2023 09:16
Subdirs are simply named by taking the prefix of the input UUID.

This is a common technique to prevent the chance of overflowing filesystem
limits on the number of files allowed in a single directory.

e.g.

Before
 - /srv/gitea/custom/public/90fa9a89-5e50-4bd4-834d-d42655e1ee8e.html
 - http://127.0.0.1:3000/assets/90fa9a89-5e50-4bd4-834d-d42655e1ee8e.html

After:
 - /srv/gitea/custom/public/bids-validator/84/b1/84b10e1c-b188-41e2-a92d-fb6ded9c6889.html
 - http://127.0.0.1:3000/assets/bids-validator/84/b1/84b10e1c-b188-41e2-a92d-fb6ded9c6889.html
Comment on lines +116 to +129
addr := server.Addr
if addr == "" {
addr = ":http"
}
sock, err := net.Listen("tcp", addr)
if err != nil {
log.Fatal(err)
}
log.Printf("main: listening on %q", bidsHookUrl)
log.Fatal(server.ListenAndServe())
err = installWebhook()
if err != nil {
log.Fatalf("error installing webhook: %v", err)
}
log.Fatal(server.Serve(sock))
Copy link
Member Author

@kousu kousu Feb 27, 2023

Choose a reason for hiding this comment

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

The reason for all this is so that we don't installWebhook until we know we own addr. Otherwise we might break an existing webhook.

log.Printf("main: listening on %q", bidsHookUrl)
log.Fatal(server.ListenAndServe())
err = installWebhook()
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
err = installWebhook()
// to avoid breaking a pre-existing bids-hook only tell Gitea about us once we know we own addr
err = installWebhook()

@kousu
Copy link
Member Author

kousu commented Feb 27, 2023

Unfortunately this doesn't work because there seems to a recently introduced bug go-gitea/gitea#23139 :(

Actually not: rather this didn't work at all until a month ago; the /admin/hooks API was started two years ago but only got merged recently. The bug is that in those two years things must have shifted around and broken the PR, and they didn't actually test it out.

Before then there would simply be no way to automate this deployment, as far as I can tell. We would have had to rewrite everything to do OAuth, or gone back to using Drone, or just settled for giving manual instructions >.<. And this would be true whether we set up the webhook with ansible or with golang.

@mguaypaq
Copy link
Member

Yeah, I had noticed that there was no API for system-level webhooks, and thought we would just give manual instructions for that step. But if/when your upstream PR gets merged and released, we can switch to automatic webhook creation/update.

Base automatically changed from output-subdirs to main February 28, 2023 18:56
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