Use temporary credentials issued from the queue for performing actions on behalf of a task. This includes extending graphs, creating artifacts. Note: before this is done, tasks currently scheduled need to be reviewed to ensure they have the correct scopes for retrieving/generating artifacts.
When this is complete, many of the scopes currently in the credentials from the provisioner can go away, too.
Worth noting that this *excludes* the credentials used to generate the bewit handed to bitbar (the taskdroid credentials in bug 1118394)
Greg, is this on the roadmap for next quarter?
Wander has added it to the q1 goals for 2016 for himself
Assignee: nobody → wcosta
Status: NEW → ASSIGNED
Created attachment 8706889 [details] [review] Use temp credentials for queue operations
Attachment #8706889 - Flags: review?(garndt)
Created attachment 8707440 [details] [diff] [review] Add necessary scopes to tasks. r=garndt. Up to now, queue operations on behalf we the task used to with worker scopes. We are going to change this behavior to use the task temporary credentials. Therefore, we need to add to the tasks the necessary scopes to run them.
Not all that useful now, but here is the try push to make sure there is not syntax bustage https://treeherder.mozilla.org/#/jobs?repo=try&revision=94ca694ad366
Comment on attachment 8707440 [details] [diff] [review] Add necessary scopes to tasks. r=garndt. Looking at the queue, it appears that claim-task, resolve-task, and create-artifact are granted for that task ID and run ID, so they shouldn't need to be added as task scopes. This should not be necessary now.
Attachment #8707440 - Attachment is obsolete: true
Comment on attachment 8706889 [details] [review] Use temp credentials for queue operations Right now there are reclaimer tests failing that should be looked at. Let me know if you need any help figuring out why those are failing now.
Attachment #8706889 - Flags: review?(garndt) → review-
Created attachment 8711078 [details] [review] Allow credentials update
Attachment #8706889 - Flags: review- → review?(garndt)
Comment on attachment 8711078 [details] [review] Allow credentials update Thanks Wander! I've added some feedback in the PR. It looks great, but a couple of bits still to do (doc updates, tests, and locking around updates). An awesome test would be having it run with temp credentials (see the other tests for how we do test setup for that), then create new temp creds, PUT them, and then make a new request, and validate that they used the new temp creds, not the old ones.
I thought about this some more Wander, and I wonder if we need this endpoint at all, or if we can't achieve the same by extending the lifetime of the original temporary credentials to be `maxRunTime` of the task. Adding this endpoint comes at quite a cost in terms of implementation, testing, documentation, maintenance, and head-space for people learning how taskcluster works. I think we could potentially drop all of it by just having one set of credentials that expire when the task run expires... I've pinged Jonas in https://github.com/taskcluster/taskcluster-proxy/pull/14/files#r50682350 to get his opinion too, as I could be overlooking something.
Of course we'd also have the benefit of losing the same (implementation, testing, documentation, ...) on the worker side that uses this endpoint.
Comment on attachment 8711078 [details] [review] Allow credentials update gave my feedback in the PR, pmoore can formally r +/- it. I think the big takeaway is to guard around concurrent access to updating/reading the credentials.
Attachment #8711078 - Flags: feedback?(garndt) → feedback+
Commits pushed to master at https://github.com/taskcluster/taskcluster-proxy https://github.com/taskcluster/taskcluster-proxy/commit/f2a582fae65612733f14186f7e55a9d85bf55145 Bug 1220738: Create endpoint for credentials update. r=pmoore docker-worker will use task temporary credentials for taskcluster requests on task behalf. During task reclaim, the temporary credentials might be updated, and if using taskcluster-proxy to forward requests, we should update the credentials in the proxy as well. We create a "/Credentials" endpoint which receive PUT requests with a json body containing the new credentials data for update. https://github.com/taskcluster/taskcluster-proxy/commit/1517a8504676ec8a5135be46c6f44ea195813c06 Merge pull request #14 from walac/master Bug 1220738: Create endpoint for credentials update. r=pmoore
Comment on attachment 8711078 [details] [review] Allow credentials update Great work Wander.
Attachment #8711078 - Flags: review?(pmoore) → review+
Wander, you'll need to release a new version of taskcluster-proxy (3.0.7). You can follow the instructions here: https://github.com/taskcluster/taskcluster-proxy/#making-a-release Be aware, docker worker will take `latest` docker tag for the taskcluster-proxy docker image, so keep your eyes open for problems when you roll out. Alternatively you could tag 3.0.7 first, test that in production e.g. on a single worker type, and if all fine, push the docker tag `latest`. However, that is quite a bit of work, for the service that is not heavily used - so maybe run a test like: https://tools.taskcluster.net/task-inspector/#B4vJQ3wCSu2CGNw8gMyyCA/ and check it is still working... Note, currently secrets service is down (bug 1235387) so you might need to trigger this task again when that is resolved. You should get back a response like https://bugzilla.mozilla.org/show_bug.cgi?id=1234929#c51 (note: just being green isn't enough, you should check you get the right headers and body). If you do decide to check on a single worker type first, I'd suggest using a workerType like ami-test, and adding "taskclusterProxyImage": "taskcluster/taskcluster-proxy:3.0.7" to the userData in the worker type definition https://tools.taskcluster.net/aws-provisioner/#ami-test/edit and this way you can test it before all other workertype pick it up. If it is good, just push the latest tag to the new version, as `latest` is what everything uses by default.
@wcosta, thanks for releasing taskcluster-proxy! :) Due to some issues with 3.0.7 (https://github.com/taskcluster/taskcluster-proxy/pull/17), this was released as taskcluster-proxy 3.0.8.
Comment on attachment 8706889 [details] [review] Use temp credentials for queue operations r+ once the merge conflicts are resolved. Things look good and we tested his with a push to try earlier last week.
Attachment #8706889 - Flags: review?(garndt) → review+
Commits pushed to master at https://github.com/taskcluster/docker-worker https://github.com/taskcluster/docker-worker/commit/b72c0832dad8f445aeb7a285d46a919e670b1369 Bug 1220738 part 1: Use temp credentials on task behalf. r=garndt For all task related queue operations, we use a queue object created with temporary credentials. https://github.com/taskcluster/docker-worker/commit/4d2c02e7523e9463aea4a06999e1fcd149f1f522 Bug 1220738 part 2: Make scheduler use temporary credentials. r=garndt Scheduler is currently only used to extend graphs. Instead of using the worker credentials, use the task temporary credentials for that. https://github.com/taskcluster/docker-worker/commit/c10ce872a4088a75fa43b6129ecf3f8e5a2d992f Bug 1220738 part 3: Update taskcluster-proxy temp credentials. r=garndt When we reclaim a task, the credentials might get updated. When this happens, we must send the new credentials to the taskcluster proxy, if this feature is set to "true". It uses the new taskcluster-proxy "Credentials" endpoint. The credentials update integration test will fail until https://github.com/taskcluster/taskcluster-proxy/pull/14 is merged. We also had to update the unit test reclaim_test because now the Task class extends EventEmitter. https://github.com/taskcluster/docker-worker/commit/d717293f519e703003c2a2622df8e48ec605e569 Bug 1220738 part 4: Use temp creds for docker images. r=garndt https://github.com/taskcluster/docker-worker/commit/4277d2743ccda8c353f5f73f7a927222d2e15c8f Merge pull request #203 from walac/master Bug 1220738: Use temp credentials on task behalf. r=garndt
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
AMIs are rolling out with these changes soon.
All worker types have been updated with this AMI as of 4/18.
You need to log in before you can comment on or make changes to this bug.