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

2.0.0: moving webhook endpoint behind /api prefix should probably be considered a breaking change #2824

Closed
5 tasks done
otaconix opened this issue Nov 15, 2023 · 3 comments · Fixed by #2826
Closed
5 tasks done
Labels
bug Something isn't working

Comments

@otaconix
Copy link

otaconix commented Nov 15, 2023

Component

server

Describe the bug

Hi, apologies if this has already been reported (I searched but couldn't find any issues), but I've recently updated my installation to 2.0.0-rc.0, and I was confused as to why webhooks didn't seem to work anymore.

I went over the changelog for the RC, and found a reference to #2212

This change breaks all webhooks created prior to the upgrade to 2.0.0-rc.0, as those still point at ${HOST}/hook. And by leaving no route behind for the old hooks endpoint, it appears as though incoming the hook succeeds, as Woodpecker responds with a statuscode 200 (the body being the landing page).

It would've been helpful if:

  1. The change had been marked as a breaking change
  2. There had been some route left behind that responds with some kind of error statuscode (perhaps 410 Gone?) so some kind of alerting would go off in whatever called the hook. Bonus points if this logs some warning in the woodpecker server logs!

I did eventually find https://github.com/woodpecker-ci/woodpecker/blob/main/docs/docs/91-migrations.md, which does describe what could be done. Still, it would have helped me personally if the changelog would have mentioned this more prominently.

System Info

{
    "source": "https://github.com/woodpecker-ci/woodpecker",
    "version": "2.0.0-rc.0"
}

Additional context

No response

Validations

  • Read the Contributing Guidelines.
  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • Checked that the bug isn't fixed in the next version already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]
  • Check that this is a concrete bug. For Q&A join our Discord Chat Server or the Matrix room.
@otaconix otaconix added the bug Something isn't working label Nov 15, 2023
@6543
Copy link
Member

6543 commented Nov 15, 2023

marked as breaking

@6543 6543 closed this as completed Nov 15, 2023
@6543
Copy link
Member

6543 commented Nov 15, 2023

There had been some route left behind that responds with some kind of error statuscode (perhaps 410 Gone?) so some kind of alerting would go off in whatever called the hook. Bonus points if this logs some warning in the woodpecker server logs!

nice idea!

@6543
Copy link
Member

6543 commented Nov 15, 2023

no breaking was done at #2639

@6543 6543 reopened this Nov 15, 2023
6543 added a commit that referenced this issue Nov 19, 2023
Signal to clients the hook and event routes where removed at #2639

close #2824
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants