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

Accepting Merge Requests on project without pipeline fails with 405 Method Not Allowed #240

Closed
veeg opened this issue Dec 3, 2019 · 6 comments

Comments

@veeg
Copy link

veeg commented Dec 3, 2019

Looks like an API change on gitlab.com on the the Accept MR, however I cannot find the related issue in release notes.

We have a new project currently with no pipeline configured. The settings General/Merge Requests/ - Pipeline must succeed is NOT checked for this project.

The following curl requests fails:

curl --header "PRIVATE-TOKEN: [REVOKED]" --header "Content-Type: application/json" --request PUT --data '{"merge_when_pipeline_succeeds": true}' "https://gitlab.com/api/v4/projects/15520050/merge_requests/2/merge" --verbose 
*   Trying 35.231.145.151:443...
* TCP_NODELAY set
* Connected to gitlab.com (35.231.145.151) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/pki/tls/certs/ca-bundle.crt
  CApath: none
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: OU=Domain Control Validated; OU=PositiveSSL Multi-Domain; CN=gitlab.com
*  start date: Jun 27 00:00:00 2019 GMT
*  expire date: May 11 23:59:59 2020 GMT
*  subjectAltName: host "gitlab.com" matched cert's "gitlab.com"
*  issuer: C=GB; ST=Greater Manchester; L=Salford; O=Sectigo Limited; CN=Sectigo RSA Domain Validation Secure Server CA
*  SSL certificate verify ok.
> PUT /api/v4/projects/15520050/merge_requests/2/merge HTTP/1.1
> Host: gitlab.com
> User-Agent: curl/7.66.0
> Accept: */*
> PRIVATE-TOKEN: [REVOKED]
> Content-Type: application/json
> Content-Length: 38
> 
* upload completely sent off: 38 out of 38 bytes
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Mark bundle as not supporting multiuse
< HTTP/1.1 405 Method Not Allowed
< Server: nginx
< Date: Tue, 03 Dec 2019 12:06:18 GMT
< Content-Type: application/json
< Content-Length: 36
< Cache-Control: no-cache
< Vary: Origin
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-Request-Id: 5s59AZkJv8
< X-Runtime: 0.120678
< RateLimit-Limit: 600
< RateLimit-Observed: 40
< RateLimit-Remaining: 560
< RateLimit-Reset: 1575374838
< RateLimit-ResetTime: Tue, 03 Dec 2019 12:07:18 GMT
< GitLab-LB: fe-21-lb-gprd
< GitLab-SV: localhost
< 
* Connection #0 to host gitlab.com left intact
{"message":"405 Method Not Allowed"}%                            

Changing the merge_when_pipeline_succeeds to false successfully accepts the request.
This property is currently hardcoded to true:

merge_when_pipeline_succeeds=True,

@veeg
Copy link
Author

veeg commented Dec 6, 2019

See also related issue to this property, which also renders Marge bot useless in our deployment scenario at the moment (due to this gitlab API bug)

https://gitlab.com/gitlab-org/gitlab/issues/38330

@Jellby
Copy link

Jellby commented Dec 6, 2019

I was going to post that it might related ;)

@veeg
Copy link
Author

veeg commented Dec 6, 2019

I was going to post that it might related ;)

I was just working up an issue to Gitlab myself on the exact same report, double checking before submitting and seeing your issue freshly created. Nice writeup of the problem, had to debug a few scenarios.

@adds68
Copy link

adds68 commented Dec 9, 2019

We are also seeing this issue with our CI at: https://gitlab.com/freedesktop-sdk/freedesktop-sdk

@RobertKirk
Copy link
Contributor

Sorry for the delay, I'm planning on working on this now. I'll change the accept method on the merge_request object to take a merge_when_pipeline_succeeds argument, which it'll pass through to the api call. I think the parameter should just be the value of the General/Merge Requests/Pipeline must succeed setting, in that if the setting is on, we want to merge when the pipeline succeeds, and if the setting is off, we don't care about the pipeline, and Gitlab will merge the MR regardless of the pipeline status (it's code to check whether the MR is mergeable with regards to CI will return True is this setting is off).

RobertKirk added a commit to RobertKirk/marge-bot that referenced this issue Mar 20, 2020
Passing this value as True makes gitlab assume that the MR must have
a pipeline, which means if it's doesn't, then the MR will be denied.
Instead we should use the value of project.only_merge_if_pipeline_succeeds,
As this will mean the gitlab logic will be correct:
   If the MR requires a pipeline to merge then we'll only merge if a pipeline
succeeds
   If the MR doesn't needs a pipeline to succeed (i.e. has this setting on the
project off) then we won't pass the parameter and hence the merge will be allowed

Fixes issue smarkets#240
RobertKirk added a commit to RobertKirk/marge-bot that referenced this issue Mar 23, 2020
Passing this value as True makes gitlab assume that the MR must have
a pipeline, which means if it's doesn't, then the MR will be denied.
Instead we should use the value of project.only_merge_if_pipeline_succeeds,
As this will mean the gitlab logic will be correct:
   If the MR requires a pipeline to merge then we'll only merge if a pipeline
succeeds
   If the MR doesn't needs a pipeline to succeed (i.e. has this setting on the
project off) then we won't pass the parameter and hence the merge will be allowed

Fixes issue smarkets#240
JaimeLennox pushed a commit that referenced this issue Mar 23, 2020
Passing this value as True makes gitlab assume that the MR must have
a pipeline, which means if it's doesn't, then the MR will be denied.
Instead we should use the value of project.only_merge_if_pipeline_succeeds,
As this will mean the gitlab logic will be correct:
   If the MR requires a pipeline to merge then we'll only merge if a pipeline
succeeds
   If the MR doesn't needs a pipeline to succeed (i.e. has this setting on the
project off) then we won't pass the parameter and hence the merge will be allowed

Fixes issue #240
@JaimeLennox
Copy link
Contributor

Fixed in #251.

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

No branches or pull requests

5 participants