1161794 - Enable tooltool use that does not require authentication

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Steve Fink [:sfink] [:s:]]

Currently, if you have a mozharness script that uses tooltool, but all resources it uses are publicly available, it will still require users to have set up authentication (as in, they must have an authentication file).

This adds an additional cryptic and unnecessary step for developers who just want to run a script that they have adequate privileges for running.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Do not pass nonexistent --authentication-file

There are a number of potential ways to fix this. Tooltool could interpret a nonexistent file to mean "no special permissions" and continue (failing if permissions *are* required.) Or it could autocreate an empty file. Or it could autocreate a real file, prompting for an LDAP password, but that's not very friendly for my use case (where some users may not have LDAP).

This patch deals with it in the tooltool mixin, by suppressing the --authentication-file parameter if no such file is available. I am not claiming this to be the best solution, but it is adequate and I'll let my reviewer and friends decide.

Comment 2

[Armen [:armenzg]]

Attachment: Do not request credentials if they're not needed

Would this work for you?
Would we need your patch with this?

Comment 3

[Steve Fink [:sfink] [:s:]]

(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #2)
> Created attachment 8602130 [details] [diff] [review]
> Do not request credentials if they're not needed
> 
> Would this work for you?
> Would we need your patch with this?

Not as things stand right now, unless tooltool.py somehow magically calls mozharness/mozilla/testing/testbase.py. My action is

    def get_blobs(self):
        dirs = self.query_abs_dirs()
        self.tooltool_fetch(self.query_compiler_manifest(), "sh " + self.config['compiler_setup'],
                            dirs['abs_work_dir'])
        self.tooltool_fetch(self.query_sixgill_manifest(), "sh " + self.config['sixgill_setup'],
                            dirs['abs_work_dir'])

which calls tooltool_fetch from TooltoolMixin, which invokes the tooltool.py binary, which is from the build-tooltool repo and won't be helped by anything in mozharness. One option, as I said, is to make tooltool ignore nonexistent auth files instead of erroring out.

Comment 4

[Armen [:armenzg]]

Comment on attachment 8601773 [details] [diff] [review]
Do not pass nonexistent --authentication-file

Review of attachment 8601773 [details] [diff] [review]:
-----------------------------------------------------------------

I think my patch would still be useful.
Do you want to review it? or should I pass it around?

Comment 5

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8602130 [details] [diff] [review]
Do not request credentials if they're not needed

Review of attachment 8602130 [details] [diff] [review]:
-----------------------------------------------------------------

I'll claim that it's good enough. It's definitely useful.

::: mozharness/mozilla/testing/testbase.py
@@ +227,5 @@
>  
> +            # Let's check if we need credentials
> +            request = urllib2.Request(url)
> +            request.get_method = lambda : 'HEAD'
> +            response = urllib2.urlopen(request)

This is a little odd in that urllib2.urlopen (whose API _urlopen is mimicking) accepts either a string url or a Request object. I don't know what this will do if url is already a Request.

But given that this is here already, it seems like nobody is ever passing in a Request, so I guess it's fine.

@@ +230,5 @@
> +            request.get_method = lambda : 'HEAD'
> +            response = urllib2.urlopen(request)
> +            if response.code == 200:
> +                self.info("The url is reachable without LDAP credentials")
> +                return urllib2.urlopen(url, **kwargs)

This does convert every HTTP call into two calls. Could you cache the opener, and reuse the one with authentication if you've already created it once (rather than doing the HEAD probe)?

Obviously, you don't want to do the converse -- the first url you request might not require auth while the 2nd one would.

You could almost use urllib2.install_opener to set up this logic once, then not have an custom _urlopen at all but just use urllib2.urlopen. But that seems trickier.

@@ +249,4 @@
>  
>          # If we have the developer_run flag enabled then we will switch
>          # URLs to the right place and enable http authentication
>          if "developer_config.py" in self.config["config_files"]:

Is this really the way we're testing for developer mode? I would have expected

  if self.config.get('developer_mode'):

Comment 6

[Rail Aliiev [:rail]]

mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/1d6e3a929329