feat: Refactor webhook configuration logic and initial support for Github pull request

[stanleyz]

This change includes the following:

1. Split the workspace settings page to three tabs: General, Webhook and Advance
2. Rewrite how webhook can be configured to allow configuration of multiple webhooks per workspace.
3. Database change to support the above and SQL clause for migration.

fix #703

[stanleyz]

@alfespa17 @BenjaminDecreusefond please review, sorry, this certainly took much longer time than I'd thought because of lack of self-discipline :)

[alfespa17]

Thank you @stanleyz I will check in the following days

[alfespa17]

Hello @stanleyz quick comments:

Can you please add mssql reference

        <sql dbms="postgresql, mysql">
            insert into webhook_event (id, branch, path, event, template_id, webhook_id, created_date, updated_date, created_by, updated_by)
            select id, branch, path, event, template_id, id, created_date, updated_date, created_by, updated_by from webhook;
        </sql>

[alfespa17]

Hello @stanleyz

You will also have to refactor the workspace create page, because those properties were deleted in a liquibase script

https://github.com/stanleyz/terrakube/blob/75ed2408ea3a88531477432c8c5c1239c2add32e/ui/src/domain/Workspaces/Create.jsx#L341

New workspace failed with the following

[stanleyz]

Hello @stanleyz

You will also have to refactor the workspace create page, because those properties were deleted in a liquibase script

https://github.com/stanleyz/terrakube/blob/75ed2408ea3a88531477432c8c5c1239c2add32e/ui/src/domain/Workspaces/Create.jsx#L341

that's right, forgot this, I'll remove the configuration section for webhook from the workspace creation process since configuring webhook is getting much complicated with this new feature added in.

[stanleyz]

Hello @stanleyz quick comments:

Can you please add mssql reference

        <sql dbms="postgresql, mysql">
            insert into webhook_event (id, branch, path, event, template_id, webhook_id, created_date, updated_date, created_by, updated_by)
            select id, branch, path, event, template_id, id, created_date, updated_date, created_by, updated_by from webhook;
        </sql>

Updated both, please check @alfespa17

[BenjaminDecreusefond]

Wow Stanleyz the goat !! thank you very much !

Just to clarify, with this PR terrakube now uses one github webhook per workspace ? Or one github webhook for all workspaces ?

[alfespa17]

Wow Stanleyz the goat !! thank you very much !

Just to clarify, with this PR terrakube now uses one github webhook per workspace ? Or one github webhook for all workspaces ?

That didn't change from what I saw, is still using one for each workspace

[alfespa17]

@stanleyz I am getting the following when trying to save a new webhook, any idea?

[stanleyz]

@stanleyz I am getting the following when trying to save a new webhook, any idea?

No from the above information, did you create the VCS provider successfully? base64, do you use Github App for VCS? If so do you want to check the private key for the VCS provider?

Additionally the GitHub App needs to have access to "pull request" if you have configured Pull_request based webhook.

[stanleyz]

created a discord server for collaboration if you guys are keen

https://discord.gg/5jAQAvMY

[alfespa17]

@stanleyz I am getting the following when trying to save a new webhook, any idea?

No from the above information, did you create the VCS provider successfully? base64, do you use Github App for VCS? If so do you want to check the private key for the VCS provider?

Additionally the GitHub App needs to have access to "pull request" if you have configured Pull_request based webhook.

I will double check tomorrow, maybe I added the wrong infornation inside the vcs connection

[alfespa17]

created a discord server for collaboration if you guys are keen

https://discord.gg/5jAQAvMY

Thank you but I don't use discord, there is a slack community that you can find inside the Readme.md that you can join, there you can find several terrakube users.

[alfespa17]

Looks like I was using the wrong private key, I was able to create the webhooks

But I am getting the following when creating the pull request:

2025-03-14T15:21:10.635Z  INFO 8789 --- [nio-8080-exec-9] o.t.api.plugin.vcs.WebhookServiceBase    : WorkspaceId: 6acfbfb5-f6b2-418a-8882-3f42d68311ea
2025-03-14T15:21:10.635Z  INFO 8789 --- [nio-8080-exec-9] o.t.api.plugin.vcs.WebhookServiceBase    : verify signature for Github webhook
2025-03-14T15:21:10.635Z  INFO 8789 --- [nio-8080-exec-9] o.t.api.plugin.vcs.WebhookServiceBase    : Parsing Github webhook payload
2025-03-14T15:21:10.842Z  INFO 8789 --- [nio-8080-exec-9] o.t.api.plugin.vcs.WebhookService        : webhook result WebhookResult(workspaceId=6acfbfb5-f6b2-418a-8882-3f42d68311ea, branch=feat/testing-pr, isValid=true, event=pull_request, createdBy=alfespa17, via=Github, fileChanges=[.gitmodules, main.tf], commit=ff0c5682b8e5f3c24b6ac41afb341ab7ee9c7aff, prNumber=28)
2025-03-14T15:21:10.844Z ERROR 8789 --- [nio-8080-exec-9] o.t.api.plugin.vcs.WebhookService        : Error creating the job

java.lang.IllegalArgumentException: No valid template found for the configured webhook event pull_request
        at org.terrakube.api.plugin.vcs.WebhookService.lambda$1(WebhookService.java:211) ~[classes/:na]
        at java.base/java.util.Optional.orElseThrow(Optional.java:403) ~[na:na]
        at org.terrakube.api.plugin.vcs.WebhookService.findTemplateId(WebhookService.java:211) ~[classes/:na]
        at org.terrakube.api.plugin.vcs.WebhookService.processWebhook(WebhookService.java:89) ~[classes/:na]

[alfespa17]

I found the issue I was using the wrong regex my bad :(

Pull request logic is working

Push logic to main branch

[stanleyz]

created a discord server for collaboration if you guys are keen
https://discord.gg/5jAQAvMY

Thank you but I don't use discord, there is a slack community that you can find inside the Readme.md that you can join, there you can find several terrakube users.

Ok, will give that a try. The thing I don't like Slack is that it doesn't provide multiple person huddle with free version.

[BenjaminDecreusefond]

Hi @stanleyz !

Thank you very much for your work it's amazing!

I was wondering something, at our company we are now having almost 200 workspaces. The issue is that if Terrakube creates one webhook per workspace we should have 200 webhooks but github is sadly limited to 20 only :(.

Would you know if it is possible to refactor a bit of logic to make this incredible feature usable at production level ? :)
I can help you with refactor if needed ! :)

regards !

[stanleyz]

@BenjaminDecreusefond I remember the 20 webhooks limit is on repository right, are you suggesting you have more than 20 workspaces per repository?

Would you know if it is possible to refactor a bit of logic to make this incredible feature usable at production level ? :)
I can help you with refactor if needed ! :)

Yes, it's achievable, but I wonder whether you can reduce the workspace number per repository to under 20, other than that, I'd be more keen to implement GitHub check run and check suite instead of adding this feature.

[adamlc]

I too am getting java.lang.IllegalArgumentException: No valid template found for the configured webhook event push

None of my existing push hooks seem to work any more. I've added the base directory too to the file regex but that doesn't seem to work either I'm afraid.

[alfespa17]

I too am getting java.lang.IllegalArgumentException: No valid template found for the configured webhook event push

None of my existing push hooks seem to work any more. I've added the base directory too to the file regex but that doesn't seem to work either I'm afraid.

Can you share one example? , I had the same issue but I was using a wrong regex

[adamlc]

Sure!

Webhook was previously just modules/.*.tf, which didn't work either.

I also want a Pull Request hook to trigger on the same working directory with every branch too (except main I guess)

[alfespa17]

I think you are missing one . it should be bookbuilder/development/.*.tf

[adamlc]

Good spot! That was it. Probably worth considering this as a breaking change as before it used to factor in the working directory.

I'll go through and update all my webhooks now. Thanks!

[alfespa17]

Good spot! That was it. Probably worth considering this as a breaking change as before it used to factor in the working directory.

I'll go through and update all my webhooks now. Thanks!

I will add it as comment inside the release notes once we create the final 2.26.0 version, by the way if you are using github apps for the connection dont forget to put the PULL REQUEST permission to the app

[stanleyz]

Right, my bloody memory, I thought that was changed by somebody else, no, it was done by me. Sorry.

Thinking about this again, it makes sense I believe. With the support of multiple events and webhook triggers, the settings of each trigger should be independent, so we should not take the workspace settings as the default values for corresponding fields, similar logic applies also template and branch settings.

[adamlc]

Right, my bloody memory, I thought that was changed by somebody else, no, it was done by me. Sorry.

Thinking about this again, it makes sense I believe. With the support of multiple events and webhook triggers, the settings of each trigger should be independent, so we should not take the workspace settings as the default values for corresponding fields, similar logic applies also template and branch settings.

Agreed. Maybe we can come up with a way of migrating all the existing push webhooks? I have about 70 I'll have to update to fix this. I don't mind doing it, just a lot of work!

[BenjaminDecreusefond]

@BenjaminDecreusefond I remember the 20 webhooks limit is on repository right, are you suggesting you have more than 20 workspaces per repository?

Would you know if it is possible to refactor a bit of logic to make this incredible feature usable at production level ? :)
I can help you with refactor if needed ! :)

Yes, it's achievable, but I wonder whether you can reduce the workspace number per repository to under 20, other than that, I'd be more keen to implement GitHub check run and check suite instead of adding this feature.

Hi @stanleyz !

Yep we do use a single repository to host all our workspaces ! Refactoring all our infra to split it across multiple repos is not really achievable and I tend to believe that 20 workspace per repos is very short if we count that for instance, for a single bucket, we might want 2 workspaces (one to deploy your bucket in prod account and one to deploy in staging account) so it adds up very fast.

Your work on webhooks is amazing and we would really like to use it in production but we are limited by the number
of webhooks per workspace.

[stanleyz]

Agreed. Maybe we can come up with a way of migrating all the existing push webhooks? I have about 70 I'll have to update to fix this. I don't mind doing it, just a lot of work!

Ah, sorry about that. Personally I think it's good to list this as a breaking feature instead of adding the workspace folder as a trigger for every webhook. The webhook logic changed anyway. I am sorry for the process you have to go through. However there should be some SQL you can use to do this in bulk. Something like the below, make sure to test in several individual workspaces first.

update webhook_event we1 set path=concat(path, ',', (select ws.folder from workspace ws join webhook w on w.workspace_id = ws.id join webhook_event we on we.webhook_id=w.id and we.id=we1.id));

[stanleyz]

@BenjaminDecreusefond I remember the 20 webhooks limit is on repository right, are you suggesting you have more than 20 workspaces per repository?

Would you know if it is possible to refactor a bit of logic to make this incredible feature usable at production level ? :)
I can help you with refactor if needed ! :)

Yes, it's achievable, but I wonder whether you can reduce the workspace number per repository to under 20, other than that, I'd be more keen to implement GitHub check run and check suite instead of adding this feature.
Yep we do use a single repository to host all our workspaces ! Refactoring all our infra to split it across multiple repos is not really achievable and I tend to believe that 20 workspace per repos is very short if we count that for instance, for a single bucket, we might want 2 workspaces (one to deploy your bucket in prod account and one to deploy in staging account) so it adds up very fast.

Yeah, that's a bit annoying. I'll be keep working on the check run and check suite any way, will evaluate how hard it is and whether it's good to move this forward to the approach you suggested.

[adamlc]

Agreed. Maybe we can come up with a way of migrating all the existing push webhooks? I have about 70 I'll have to update to fix this. I don't mind doing it, just a lot of work!

Ah, sorry about that. Personally I think it's good to list this as a breaking feature instead of adding the workspace folder as a trigger for every webhook. The webhook logic changed anyway. I am sorry for the process you have to go through. However there should be some SQL you can use to do this in bulk. Something like the below, make sure to test in several individual workspaces first.

update webhook_event we1 set path=concat(path, ',', (select ws.folder from workspace ws join webhook w on w.workspace_id = ws.id join webhook_event we on we.webhook_id=w.id and we.id=we1.id));

No problem, it's all good. Part of testing and open source :D

In the end I just went through them all manually as I enabled the PR hook too, which works perfectly by the way, thanks for your amazing contribution! Comes at a great time as we're going to need it for compliance reasons.

@BenjaminDecreusefond I remember the 20 webhooks limit is on repository right, are you suggesting you have more than 20 workspaces per repository?

Would you know if it is possible to refactor a bit of logic to make this incredible feature usable at production level ? :)
I can help you with refactor if needed ! :)

Yes, it's achievable, but I wonder whether you can reduce the workspace number per repository to under 20, other than that, I'd be more keen to implement GitHub check run and check suite instead of adding this feature.

Hi @stanleyz !

Yep we do use a single repository to host all our workspaces ! Refactoring all our infra to split it across multiple repos is not really achievable and I tend to believe that 20 workspace per repos is very short if we count that for instance, for a single bucket, we might want 2 workspaces (one to deploy your bucket in prod account and one to deploy in staging account) so it adds up very fast.

Your work on webhooks is amazing and we would really like to use it in production but we are limited by the number of webhooks per workspace.

We actually have something similar. Luckily not quite hit the limit yet, our biggest repo is 15, so approaching that. Probably not a use case thats common though.

[stanleyz]

In the end I just went through them all manually as I enabled the PR hook too,

Cool, you probably want to consider using the Terakube Terraform provider to automate this, it's going to be easier for such situations if automated. Speaking that, I do need to add the feature of this PR to the Terrakube provider.

https://registry.terraform.io/providers/AzBuilder/terrakube/latest/docs

[adamlc]

In the end I just went through them all manually as I enabled the PR hook too,

Cool, you probably want to consider using the Terakube Terraform provider to automate this, it's going to be easier for such situations if automated. Speaking that, I do need to add the feature of this PR to the Terrakube provider.

https://registry.terraform.io/providers/AzBuilder/terrakube/latest/docs

Heh yes, I do actually already use it for other things, didn't really consider it until afterwards 🙈