Avoid an exception in WorkerOwnerMatch while handling API request

[slydiman]

Summary

Ignoring if workerinfo['admin'] is None.

[gkistanova]

Thanks, Dmitry!

How about using something like email.utils.parseaddr instead of a regex to parse email addresses?

It would be also nice to support multiple owners of a single worker. Not necessarily a subject of this patch, though.

[slydiman]

I have updated the patch to extract all e-mail addresses in the admin info.
Note the dot at the end of the e-mail address is alloved in case of the admin info like Name1 <[e-mail]>, Name2 [e-mail].
email.utils.parseaddr keeps the dot at the end too. It seems email.utils.parseaddr uses a similar regex.

[gkistanova]

If you insist on using regex, could you use something like ([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|\"([]!#-[^-~ \t]|(\\[\t -~]))+\")@([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|\[[\t -Z^-~]*]) or even more RFC 5322 compliant regex, please? That also would handle terminating dots and such for you.

And it might worth logging the fact that valid admin info is missing. What do you think?

[slydiman]

I have updated the patch with the suggested regex.
I have tested it here https://regex101.com/r/CZN3Mg/1
The e-mail "us\er2"@domain.com looks strange.

I think adding the logging that the admin info is incorrect here is bad idea.
Note this method will be called on every authentication and it checks all workers.
We will get a lot of spam if few workers have no admin info or a valid e-mail in the admin info.

[gkistanova]

LGTM.

Admin info could change only at a worker connect handshake. We shall consider caching emails rather than parsing every time there is a use. Not a subject of this patch, though.

The e-mail "us\er2"@domain.com looks strange.

According to RFC 5322 that email is valid, unless I'm missing something. Here is the relevant quote from RFC:

addr-spec       =   local-part "@" domain
local-part      =   dot-atom / quoted-string / obs-local-part
...
the quote and backslash characters may appear in a quoted-string so long as they appear as a quoted-pair.

zorg/buildbot/util/workerowner.py

class WorkerOwnerMatch(Match):
 
     def __init__(self, master, workers, **kwargs):
         super().__init__(master, **kwargs)
         self.workerOwners = []
         for worker in workers:
             if 'workerinfo' in worker:
                 workerinfo = worker['workerinfo']
                 if 'admin' in workerinfo:
-                    email = re.search('.*<(.*)>.*', workerinfo['admin'])
-                    if email:
-                        self.workerOwners.append(email.group(1))
+                    admin = workerinfo['admin']
+                    if admin:
+                        match = re.findall(
+                          r"([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|\"([]!#-[^-~ \t]|(\\[\t -~]))+\")@([-!#-'*+/-9=?A-Z^-~]+(\.[-!#-'*+/-9=?A-Z^-~]+)*|\[[\t -Z^-~]*])",
+                          admin)
+                        for parts in match:
+                            self.workerOwners.append(parts[0]+'@'+parts[4])
 
     def getWorkerOwners(self):
         return self.workerOwners
 
 
 class WorkerEndpointMatcher(EndpointMatcherBase):
 
     def __init__(self, **kwargs):