[gitlab/webhook] return code 200 on Unauthorized #14421
Conversation
|
started the job as gitpod-build-at-gitlab-401-fix.1 because the annotations in the pull request description changed |
AlexTugarev
changed the title
AlexTugarev
force-pushed
the
at/gitlab-401-fix
branch
from
f69fb96 to
aaffb89
Compare
| await this.webhookEvents.updateEvent(event.id, { status: "dismissed_unauthorized" }); | ||
| // TODO(at) explore ways to mark a project having issues with permissions. |
There was a problem hiding this comment.
this as a followup to #13985
| @@ -75,11 +75,11 @@ export class GitLabApp { | |||
| TraceContext.setError({ span }, error); | |||
| } | |||
| if (!user) { | |||
| // If the webhook installer is no longer found in Gitpod's DB | |||
| // we should send a UNAUTHORIZED signal. | |||
| // Gitpod is not supposed to return 4xx codes on issues with project permissions. | |||
There was a problem hiding this comment.
Given this behaviour (200 on Error) is gonna look pretty bizzare to future readers, could you explain WHY it's doing it, rather than WHAT it's doing?
What I mean is to elaborate on why we cannot return a 4xx, and what are the implications if we do.
There was a problem hiding this comment.
That's a good question. The complete answer and docs are already contained at the top of this handler.
From the webhook event sender's perspective this is is not a 4xx case. We are not interested in a permanent disablement of the webhook if project permissions are temporarily "broken", instead prebuilds won't be executed until the maintainer had a chance to fix the issue.
Also noteworthy, the response text is completely optional and helps us to understand the delivery logs on GitLab, but it's ignored otherwise. So this is the place where we may encode the App level error.
The sender expects a 2xx response whenever it was a successful and legit delivery. It's up to the receiver to deal with issues of the application. For more on this see the linked issue and https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#failing-webhooks
There was a problem hiding this comment.
The complete answer and docs are already contained at the top of this handler.
@AlexTugarev Looks like it only mentions 500. Can you add a line about 4xx as well? 🙏
There was a problem hiding this comment.
Thanks both of you for the hint. Just added some more details on the special behavior of webhooks, so that reading code makes more sense to future readers.
|
Start ŧesting now... |
|
Worked: |
AlexTugarev
force-pushed
the
at/gitlab-401-fix
branch
from
aaffb89 to
2929343
Compare
|
/hold cancel |
|
/hold Blocks the queue. |
|
/werft run 👍 started the job as gitpod-build-at-gitlab-401-fix.4 |
Otherwise we provoke permanent deactivation of the webhook. In future we need to make the Unauthorized situation fixable from Gitpod.
AlexTugarev
force-pushed
the
at/gitlab-401-fix
branch
from
2929343 to
daa41d2
Compare
|
/hold cancel |
roboquat
added
deployed: webapp
To avoid provoking permanent deactivation of the webhooks we need to stop responding with status 401. In future we need to make the Unauthorized situation fixable from Gitpod.
Related Issue(s)
Fixes #13985
How to test
Release Notes
Documentation
Werft options:
If enabled this will build
install/previewValid options are
all,workspace,webapp,ide