[ARVADOS] updated: 1.2.0-174-g84c6185b3
Git user
git at public.curoverse.com
Fri Oct 12 17:07:52 EDT 2018
Summary of changes:
.../arvados/v1/containers_controller.rb | 15 ++++++++++----
services/api/app/models/container.rb | 24 +++++++++++++++-------
services/api/app/models/container_request.rb | 15 +++++++++-----
services/api/test/unit/container_request_test.rb | 14 +++++++++++--
services/crunch-run/crunchrun.go | 2 +-
5 files changed, 51 insertions(+), 19 deletions(-)
via 84c6185b3aa1bb38ae9bb69ff93dbc22c4adc833 (commit)
via da87a814c360a8e1b5decf6ffa498e731748f247 (commit)
via 0afe226862094cd10d630cd53f04b33a76b11212 (commit)
from 3453440f95dc76db1bd39fe8c85d9a898d474c6b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 84c6185b3aa1bb38ae9bb69ff93dbc22c4adc833
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Fri Oct 12 17:07:33 2018 -0400
14260: crunch-run constructs a v2 token with added container uuid
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index be98a3ee1..44560b80a 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -1432,7 +1432,7 @@ func (runner *ContainerRunner) ContainerToken() (string, error) {
if err != nil {
return "", err
}
- runner.token = auth.APIToken
+ runner.token = fmt.Sprintf("v2/%s/%s/%s", auth.UUID, auth.APIToken, runner.Container.UUID)
return runner.token, nil
}
commit da87a814c360a8e1b5decf6ffa498e731748f247
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Fri Oct 12 17:01:23 2018 -0400
14260: Container auth tokens must have the container uuid in the 4th position
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb
index e9ec4123c..064a829db 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -58,11 +58,11 @@ class Arvados::V1::ContainersController < ApplicationController
if Thread.current[:api_client_authorization].nil?
send_error("Not logged in", status: 401)
else
- c = Container.where(auth_uuid: Thread.current[:api_client_authorization].uuid).first
- if c.nil?
+ c = Container.for_current_token
+ if c.nil? or c.first.nil?
send_error("Token is not associated with a container.", status: 404)
else
- @object = c
+ @object = c.first
show
end
end
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 21530888b..20158cad3 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -390,6 +390,18 @@ class Container < ArvadosModel
[Complete, Cancelled].include?(self.state)
end
+ def self.for_current_token
+ _, _, _, container_uuid = Thread.current[:token].split('/')
+ if container_uuid.nil?
+ Container.where(auth_uuid: current_api_client_authorization.uuid)
+ else
+ Container.where('auth_uuid=? or (uuid=? and runtime_token=?)',
+ current_api_client_authorization.uuid,
+ container_uuid,
+ current_api_client_authorization.token)
+ end
+ end
+
protected
def fill_field_defaults
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index cd6851709..61b65b1ed 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -106,8 +106,12 @@ class ContainerRequest < ArvadosModel
end
def skip_uuid_read_permission_check
- # XXX temporary until permissions are sorted out.
- %w(modified_by_client_uuid container_uuid requesting_container_uuid)
+ # The uuid_read_permission_check prevents users from making
+ # references to objects they can't view. However, in this case we
+ # don't want to do that check since there's a circular dependency
+ # where user can't view the container until the user has
+ # constructed the container request that references the container.
+ %w(container_uuid)
end
def finalize_if_needed
@@ -345,7 +349,7 @@ class ContainerRequest < ArvadosModel
end
def validate_runtime_token
- if !self.runtime_token.nil?
+ if !self.runtime_token.nil? && self.runtime_token_changed?
if !runtime_token[0..2] == "v2/"
errors.add :runtime_token, "not a v2 token"
return
@@ -389,8 +393,9 @@ class ContainerRequest < ArvadosModel
def get_requesting_container
return self.requesting_container_uuid if !self.requesting_container_uuid.nil?
return if !current_api_client_authorization
- if (c = Container.where('auth_uuid=?', current_api_client_authorization.uuid).select([:uuid, :priority]).first)
- return c
+ c = Container.for_current_token
+ if !c.nil?
+ c.select([:uuid, :priority]).first
end
end
end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index db81c0844..c7807826f 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -380,7 +380,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
[
['running_container_auth', 'zzzzz-dz642-runningcontainr', 501],
- ['active_no_prefs', nil, 0],
+ ['active_no_prefs', nil, 0]
].each do |token, expected, expected_priority|
test "create as #{token} and expect requesting_container_uuid to be #{expected}" do
set_user_from_auth token
@@ -391,6 +391,15 @@ class ContainerRequestTest < ActiveSupport::TestCase
end
end
+ test "create as container_runtime_token and expect requesting_container_uuid to be zzzzz-dz642-20isqbkl8xwnsao" do
+ set_user_from_auth :container_runtime_token
+ Thread.current[:token] = "#{Thread.current[:token]}/zzzzz-dz642-20isqbkl8xwnsao"
+ cr = ContainerRequest.create(container_image: "img", output_path: "/tmp", command: ["echo", "foo"])
+ assert_not_nil cr.uuid, 'uuid should be set for newly created container_request'
+ assert_equal 'zzzzz-dz642-20isqbkl8xwnsao', cr.requesting_container_uuid
+ assert_equal 1, cr.priority
+ end
+
[[{"vcpus" => [2, nil]},
lambda { |resolved| resolved["vcpus"] == 2 }],
[{"vcpus" => [3, 7]},
commit 0afe226862094cd10d630cd53f04b33a76b11212
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Fri Oct 12 14:46:02 2018 -0400
14260: Don't set auth_uuid when runtime_token is set.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb
index 65d8385ad..e9ec4123c 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -17,7 +17,14 @@ class Arvados::V1::ContainersController < ApplicationController
if @object.locked_by_uuid != Thread.current[:api_client_authorization].uuid
raise ArvadosModel::PermissionDeniedError.new("Not locked by your token")
end
- @object = @object.auth
+ if @object.runtime_token.nil?
+ @object = @object.auth
+ else
+ @object = ApiClientAuthorization.validate(token: @object.runtime_token)
+ if @object.nil?
+ raise ArvadosModel::PermissionDeniedError.new("Invalid runtime_token")
+ end
+ end
show
end
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 86201955a..21530888b 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -540,7 +540,7 @@ class Container < ArvadosModel
def assign_auth
if self.auth_uuid_changed?
- return errors.add :auth_uuid, 'is readonly'
+ return errors.add :auth_uuid, 'is readonly'
end
if not [Locked, Running].include? self.state
# don't need one
@@ -553,6 +553,10 @@ class Container < ArvadosModel
end
if self.runtime_token.nil?
if self.runtime_user_uuid.nil?
+ # legacy behavior, we don't have a runtime_user_uuid so get
+ # the user from the highest priority container request, needed
+ # when performing an upgrade and there are queued containers,
+ # and some tests.
cr = ContainerRequest.
where('container_uuid=? and priority>0', self.uuid).
order('priority desc').
@@ -569,12 +573,6 @@ class Container < ArvadosModel
create!(user_id: User.find_by_uuid(self.runtime_user_uuid).id,
api_client_id: 0,
scopes: self.runtime_auth_scopes)
- else
- # using runtime_token
- self.auth = ApiClientAuthorization.validate(token: self.runtime_token)
- if self.auth.nil?
- raise ArgumentError.new "Invalid runtime token"
- end
end
end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 14fa5796d..db81c0844 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -1082,7 +1082,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr.save!
c = Container.find_by_uuid cr.container_uuid
lock_and_run c
- assert_equal c.auth_uuid, spec.uuid
+ assert_nil c.auth_uuid
+ assert_equal c.runtime_token, spec.token
assert_not_nil ApiClientAuthorization.find_by_uuid(spec.uuid)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list