Codecov Report

Merging #2149 (c0f35d4) into main (98bdb98) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2149   +/-   ##
=======================================
  Coverage   95.46%   95.46%           
=======================================
  Files          81       81           
  Lines        5353     5362    +9     
=======================================
+ Hits         5110     5119    +9     
  Misses        243      243           

Flags with carried forward coverage won't be shown. Click here to find out more.

I wonder if this would be better to be a config option? As probably the most likely case when this occurs is that something is wrong with their config.

I wonder if this would be better to be a config option? As probably the most likely case when this occurs is that something is wrong with their config.

do you mean by set the config as parameter in class Gitlab, eg: keep_base_url=True, once they have the same case then just set it as True ?

Thanks for the work here @iomarmochtar! Have a look at #1978 as well, which covers mostly the same topic.

I agree with John we should at least not completely blindly follow either the server or the client-provided URL when there is a mismatch, hence my proposal in the issue above: I think we should issue a warning instead, unless explicitly configured to follow the base URL, and only then reconstruct the URL. Could we add that here? 🙇 Thanks again!

For context: I actually would consider this approach a misconfiguration of the external_url, at least until GitLab supports multiple urls. E.g. the way your admins have configured it does not really scale - if your devs use other tech you'll have to implement this in the node gitbeaker library, in the terraform GitLab provider, and other libraries/apps, since most of these follow the link headers.

Instead I think they should use clone_url in the runner config (which was designed for this purpose), and if needed have additional proxying done in front of specific runner endpoints. Because this also affects other aspects including many other types of links that GitLab returns in its API responses.

@iomarmochtar would you still like to work on this as an optional parameter as discussed above?

thanks you for remind me @nejch ,almost forgot to continue it due office task. let me try in this weekend.

push as requested, please review it @nejch @JohnVillalovos

Thanks @iomarmochtar. I have some ideas to reuse this elsewhere that I think are outside the scope of this PR, so I'll merge this as is, and I will do a little refactor after.