Dustin J. Mitchell [:dustin] (he/him) Assignee
	Description • 9 years ago

This is a proxy, similar to (read: "forked from") taskcluster-proxy that proxies communication from a task container to relengapi, adding an authorization token that's based on the task's scopes.

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 1 • 9 years ago

Don't click the attachment - it's just a link to https://github.com/djmitche/relengapi-proxy

This was a fork from taskcluster-proxy, but basically a complete rewrite from there on out.  I'm new to Go, so advice is very much appreciated on anything from good go style to why this won't work as a proxy within taskcluster.

Regarding tests, I tested stuff that was nicely isolated, but everything else seemed like it would require a lot of backflips to dependency-inject fakes, so I didn't do it.  I can revisit that decision, but for such a simple app it seems like the test code would overwhelm the production code.

	James Lal [:lightsofapollo]
	Comment 3 • 9 years ago

Comment on attachment 8622509 [details] [review]
https://github.com/djmitche/relengapi-proxy/pull/1

looks good. My complaints mostly revolve around returning errors instead of panicing on the spot where they occur.. Ideally I think errors should be handled by the higher level flows (CLI init, http handler).

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 4 • 9 years ago

Unfortunately, ReverseProxy doesn't provide a way to generate an HTTP error response from the director (or any other hook that I can see).  The options for handling an error are (1) panic, as currently implemented; and (2) proxy the request anyway without an Authorization header, sending a 403 back to the task.  I think (1) is the better choice, particularly when we have logs from the proxies displayed in the task status.

I've refactored a little bit to return errors from `getTmpToken`, and I think that's worth keeping, but I'll just send the resulting error to `log.Fatal`.

With the three additional commits added to the PR, do you think this is OK?  And with that in place, what's the next step to deploy this?

	James Lal [:lightsofapollo]
	Comment 5 • 9 years ago

LGTM- We do not (yet anyway) have a standard API for gluing this in (and I suspect we will need some credentials piped in here anyhow which involved some work in docker-worker). I am going to redirect a NI to Greg who has implemented the majority of our docker worker features.

	Greg Arndt [:garndt]
	Comment 6 • 9 years ago

Will this token be something encrypted in the task or should it be something the worker knows about (such as credentials baked into the worker)?  If it's something that's baked into the worker, that's fine, just a few things that need to be done in addition to creating the feature.

Either way, building out the feature should be very similar to our taskcluster proxy, just a matter of figuring out how that token gets there.

If we figure that out, and there is a token I can test with that would work with this proxy, something should be able to be integrated fairly quickly. I can create the feature and test it out if I could get a token, or I can point you in the right locations for integrating it.  Entirely up to you but I'm more than willing to get it integrated as a feature within the worker.

Let me know how I can help out.

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 7 • 9 years ago

The token will be baked into the worker.  You can create a test token at https://api.pub.build.mozilla.org/tokenauth/ and I can issue a permanent one once we're ready to roll.  It's probably easiest for you to just barrel ahead and deploy.  I have a rough idea of how it will work from looking at the TC proxy implementation, but I probably lack permissions to upload images, edit scopes, etc.

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 8 • 9 years ago

(the test token must have `base.tokens.tmp.issue` and should also have some other permission to grant to tasks, such as `tooltool.download.internal`)

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 9 • 9 years ago

I still need to figure out how to wire this up to try and the decision task, but I killed a try task and munged its task description to create

  https://tools.taskcluster.net/task-inspector/#rAXKRDNwQyCN5dTySEU1YQ/

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 10 • 9 years ago

Finding a lot of hard-coded 'java' paths.. some in the Android SDK:

16:05:40     INFO -  /home/worker/workspace/build/src/android-sdk-linux/build-tools/22.0.1/dx: line 89: exec: java: not found 

So I think we're going to have to add $JAVA_HOME/bin to $PATH.  Question is, where best to do that.

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 11 • 9 years ago

ignore that comment, sorry.

That try push didn't work -- it still tried to use the full RelengAPI URL, despite

diff --git a/configs/builds/build_pool_specifics.py b/configs/builds/build_pool_specifics.py
--- a/configs/builds/build_pool_specifics.py
+++ b/configs/builds/build_pool_specifics.py
@@ -34,10 +34,12 @@ config = {
         'stage_server': 'stage.mozilla.org',
         "sendchange_masters": ["buildbot-master81.build.mozilla.org:9301"],
         'taskcluster_index': 'index',
     },
     "taskcluster": {
         'graph_server': 'graphs.mozilla.org',
         'symbol_server_host': "symbolpush.mozilla.org",
         'stage_server': 'stage.mozilla.org',
+        # use the relengapi proxy to talk to tooltool
+        "tooltol_servers": ['http://relengapi/tooltool/'],
     },
 }

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 12 • 9 years ago

https://tools.taskcluster.net/task-inspector/#PXsX5CVASJCrotVTE_F9Pw/0

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 13 • 9 years ago

Bug 1170753: use the relengapi proxy for tooltool downloads; r=ted

	(not currently active) Ted Mielczarek
	Comment 14 • 9 years ago

Comment on attachment 8628964 [details]
MozReview Request: Bug 1170753: use the relengapi proxy for tooltool downloads; r=ted

https://reviewboard.mozilla.org/r/12485/#review10943

Ship It!

::: mozharness/mozilla/building/buildbase.py:1058
(Diff revision 1)
>      # TODO: replace with ToolToolMixin

LOL

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 15 • 9 years ago

Honestly every time I see "TODO" in production code I chuckle..

Morgan: once I land this, if env.RELENGAPI_TOKEN isn't set, no /builds/relengapi.tok will be created, so tooltool.py will be invoked without --authorization-file, which should work for all of your linux builds.  Even after we fix https://github.com/mozilla/build-relengapi/pull/288.  Is that reasoning sound?

	Morgan Phillips [:mrrrgn]
	Comment 16 • 9 years ago

(In reply to Dustin J. Mitchell [:dustin] from comment #15)
> Honestly every time I see "TODO" in production code I chuckle..
> 
> Morgan: once I land this, if env.RELENGAPI_TOKEN isn't set, no
> /builds/relengapi.tok will be created, so tooltool.py will be invoked
> without --authorization-file, which should work for all of your linux
> builds.  Even after we fix
> https://github.com/mozilla/build-relengapi/pull/288.  Is that reasoning
> sound?

LGTM: Sorry I didn't see the github mention.

	Dustin J. Mitchell [:dustin] (he/him) Assignee
	Comment 17 • 9 years ago

Comment on attachment 8628964 [details]
MozReview Request: Bug 1170753: use the relengapi proxy for tooltool downloads; r=ted

remote:   https://hg.mozilla.org/build/mozharness/rev/354aa7517304

	Jordan Lund (:jlund)
	Comment 18 • 9 years ago

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