bluefeet/GitLab-API-v4

cli tool greedily eats method params

Closed this issue · 8 comments

tyldum commented

Trying to manipulate the JIRA service which has an URL param, the cli tool will eat it and replace the gitlab url with the jira url.
gitlab-api-v4 edit_project_service project jira --url=https://jira.example.com/

tyldum commented

The minimum patch to resolve this is just to change the getopt parser. It will be a breaking change for the --url parameter and will break consistency with the config file.
I'm not sure how you prefer to do it.

Working patch:

diff --git a/lib/GitLab/API/v4/Config.pm b/lib/GitLab/API/v4/Config.pm
index 7929a1b..469509a 100644
--- a/lib/GitLab/API/v4/Config.pm
+++ b/lib/GitLab/API/v4/Config.pm
@@ -81,7 +81,7 @@ Returns a hashref of arguments derived from command line options.
 Supported options are:
 
     --config_file=...
-    --url=...
+    --api-url=...
     --private-token=...
     --access-token=...
     --retries=...
@@ -106,7 +106,7 @@ sub _build_opt_args {
 
     GetOptions(
         'config-file=s'   => \my $config_file,
-        'url=s'           => \my $url,
+        'api-url=s'       => \my $url,
         'private-token=s' => \my $private_token,
         'access-token=s'  => \my $access_token,
         'retries=i'       => \my $retries,
@@ -114,7 +114,7 @@ sub _build_opt_args {
 
     $opt_args = $self->_filter_args({
         config_file   => $config_file,
-        url           => $url,
+        api_url       => $url,
         private_token => $private_token,
         access_token  => $access_token,
         retries       => $retries,

I can do a PR depending on your preference.

bluefeet commented

I'm not digging the minimal approach as it still leaves the door open, could still be an issue with other api calls and their params conflicting with the script's own arguments like this.

Does this do the job?

gitlab-api-v4 edit_project_service project jira -- --url=https://jira.example.com/

Notice the -- in there, thats a standard unix-ism telling the script to not process any arguments after the --. This should cause the --url argument to pass through to the API call as a parameter.

If this works for you, I'll just add some documentation... sound OK?

tyldum commented

Sadly, that does not work:
edit_project_service must be called with 2 to 3 arguments at ...bin/gitlab-api-v4 line 89.

bluefeet commented

I think that 56c8a3d solves the issue correctly (making -- work as expected). I also added some extra debug output, so if you pass --verbose you get to see the final values for the method, args, and params.

Note that I tested this and, for me at least, just gets me to a different error but it looks like an error in my setup or how I'm using the api rather than the client itself.

Can you give the above patch a try with this one:

gitlab-api-v4 edit_project_service project jira -- --url=https://jira.example.com/
tyldum commented

Yes, it works.
For a general purpose tool that could get abused in unforeseeable ways I would say this would be great, while for a specialized tool like this I feel it should be combined with require_order to GetOpt to simply work better out of the box without stumbling into an issue and then realize you must use '--' before arguments.
There is also a risk that stuff that used to work without '--' at some point stops working because more arguments are added to gitlab-api-v4 that happen to overlap with something in the GitLab API.

bluefeet commented

I'm still not satisfied with this solution. It could break existing arguments that people rely on, yet at the same time does not fully resolve all the cases where someone could accidentally screw up.

Instead I've modified parameters to not use option formatting altogether. This also breaks any existing setups people have even more than using require_order, but I think it's a good change and removes all the ambiguity. Here's what it'll look like:

gitlab-api-v4 --url=xxx some-method url:yyy

Here's the commit that makes this work: 95a8498

bluefeet commented

v0.15 was released yesterday with this change.

@gregoa I wanted you to be aware that this change breaks gitlab-api-v4 backwards compatibility:

BREAKING CHANGE: gitlab-api-v4 now takes parameters in the form of param:value
rather than --param=value.  This is to avoid foot-gun ambiguities when options
have the same names as parameters (such as --url).

I'm not sure if over there in the Debian world if you are using the gitlab-api-v4 script, but if you are, heads up when you upgrade!

@tyldum thanks for the prodding.

gregoa commented