[ARVADOS] created: 1.1.3-139-g2fa20d6
Git user
git at public.curoverse.com
Fri Mar 9 13:35:01 EST 2018
at 2fa20d60cae47148250cb58b44f520b2cb866644 (commit)
commit 2fa20d60cae47148250cb58b44f520b2cb866644
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Thu Mar 1 09:21:12 2018 -0500
13143: Add secret_mounts field to containers and container requests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg 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 8c63ea7..6ec92b0 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -60,4 +60,14 @@ class Arvados::V1::ContainersController < ApplicationController
end
end
end
+
+ def secret_mounts
+ if @object &&
+ @object.auth_uuid &&
+ @object.auth_uuid == Thread.current[:api_client_authorization].uuid
+ send_json({"secret_mounts" => @object.secret_mounts})
+ else
+ send_error("Token is not associated with this container.", status: 403)
+ end
+ end
end
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index b013776..a92dad7 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -20,6 +20,7 @@ class Container < ArvadosModel
serialize :runtime_constraints, Hash
serialize :command, Array
serialize :scheduling_parameters, Hash
+ serialize :secret_mounts, Hash
before_validation :fill_field_defaults, :if => :new_record?
before_validation :set_timestamps
@@ -121,6 +122,7 @@ class Container < ArvadosModel
mounts: resolve_mounts(req.mounts),
runtime_constraints: resolve_runtime_constraints(req.runtime_constraints),
scheduling_parameters: req.scheduling_parameters,
+ secret_mounts: req.secret_mounts,
}
act_as_system_user do
if req.use_existing && (reusable = find_reusable(c_attrs))
@@ -378,7 +380,8 @@ class Container < ArvadosModel
if self.new_record?
permitted.push(:owner_uuid, :command, :container_image, :cwd,
:environment, :mounts, :output_path, :priority,
- :runtime_constraints, :scheduling_parameters)
+ :runtime_constraints, :scheduling_parameters,
+ :secret_mounts)
end
case self.state
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index bcca407..e1253c7 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -18,6 +18,7 @@ class ContainerRequest < ArvadosModel
serialize :runtime_constraints, Hash
serialize :command, Array
serialize :scheduling_parameters, Hash
+ serialize :secret_mounts, Hash
before_validation :fill_field_defaults, :if => :new_record?
before_validation :validate_runtime_constraints
@@ -79,7 +80,7 @@ class ContainerRequest < ArvadosModel
:container_image, :cwd, :environment, :filters, :mounts,
:output_path, :priority, :properties, :requesting_container_uuid,
:runtime_constraints, :state, :container_uuid, :use_existing,
- :scheduling_parameters, :output_name, :output_ttl]
+ :scheduling_parameters, :secret_mounts, :output_name, :output_ttl]
def self.limit_index_columns_read
["mounts"]
@@ -216,7 +217,7 @@ class ContainerRequest < ArvadosModel
if self.new_record? || self.state_was == Uncommitted
# Allow create-and-commit in a single operation.
- permitted.push *AttrsPermittedBeforeCommit
+ permitted.push(*AttrsPermittedBeforeCommit)
end
case self.state
diff --git a/services/api/config/initializers/lograge.rb b/services/api/config/initializers/lograge.rb
index 564f31a..db9b225 100644
--- a/services/api/config/initializers/lograge.rb
+++ b/services/api/config/initializers/lograge.rb
@@ -15,6 +15,18 @@ Server::Application.configure do
}
exceptions = %w(controller action format id)
params = event.payload[:params].except(*exceptions)
+
+ # Omit secret_mounts field if supplied in create/update request
+ # body.
+ [
+ ['container', 'secret_mounts'],
+ ['container_request', 'secret_mounts'],
+ ].each do |resource, field|
+ if params[resource].is_a? Hash
+ params[resource] = params[resource].except(field)
+ end
+ end
+
params_s = SafeJSON.dump(params)
if params_s.length > Rails.configuration.max_request_log_params_size
payload[:params_truncated] = params_s[0..Rails.configuration.max_request_log_params_size] + "[...]"
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index fcd5c34..ad2406a 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -39,6 +39,7 @@ Server::Application.routes.draw do
get 'auth', on: :member
post 'lock', on: :member
post 'unlock', on: :member
+ get 'secret_mounts', on: :member
get 'current', on: :collection
end
resources :container_requests
diff --git a/services/api/db/migrate/20180228220311_add_secret_mounts_to_containers.rb b/services/api/db/migrate/20180228220311_add_secret_mounts_to_containers.rb
new file mode 100644
index 0000000..84a77b8
--- /dev/null
+++ b/services/api/db/migrate/20180228220311_add_secret_mounts_to_containers.rb
@@ -0,0 +1,7 @@
+class AddSecretMountsToContainers < ActiveRecord::Migration
+ def change
+ add_column :container_requests, :secret_mounts, :jsonb, default: {}
+ add_column :containers, :secret_mounts, :jsonb, default: {}
+ add_column :containers, :secret_mounts_md5, :string, default: "99914b932bd37a50b983c5e7c90ae93b"
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 357e95c..f02ccbf 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -299,7 +299,8 @@ CREATE TABLE container_requests (
output_uuid character varying(255),
log_uuid character varying(255),
output_name character varying(255) DEFAULT NULL::character varying,
- output_ttl integer DEFAULT 0 NOT NULL
+ output_ttl integer DEFAULT 0 NOT NULL,
+ secret_mounts jsonb DEFAULT '{}'::jsonb
);
@@ -352,7 +353,9 @@ CREATE TABLE containers (
exit_code integer,
auth_uuid character varying(255),
locked_by_uuid character varying(255),
- scheduling_parameters text
+ scheduling_parameters text,
+ secret_mounts jsonb DEFAULT '{}'::jsonb,
+ secret_mounts_md5 character varying DEFAULT '99914b932bd37a50b983c5e7c90ae93b'::character varying
);
@@ -3054,3 +3057,5 @@ INSERT INTO schema_migrations (version) VALUES ('20171212153352');
INSERT INTO schema_migrations (version) VALUES ('20180216203422');
+INSERT INTO schema_migrations (version) VALUES ('20180228220311');
+
diff --git a/services/api/test/fixtures/containers.yml b/services/api/test/fixtures/containers.yml
index 3995791..47b5471 100644
--- a/services/api/test/fixtures/containers.yml
+++ b/services/api/test/fixtures/containers.yml
@@ -39,6 +39,10 @@ running:
runtime_constraints:
ram: 12000000000
vcpus: 4
+ secret_mounts:
+ /secret/6x9:
+ kind: text
+ content: "42\n"
auth_uuid: zzzzz-gj3su-077z32aux8dg2s2
running_older:
diff --git a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
index 4ebda97..5423e84 100644
--- a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
@@ -23,4 +23,19 @@ class Arvados::V1::ContainerRequestsControllerTest < ActionController::TestCase
assert_not_nil cr, 'Expected container request'
assert_equal sp, cr['scheduling_parameters']
end
+
+ test "secret_mounts not in container API responses" do
+ end
+
+ test "secret_mounts not in container request API responses" do
+ end
+
+ test "create with secret_mounts" do
+ end
+
+ test "update with secret_mounts" do
+ end
+
+ test "update without deleting secret_mounts" do
+ end
end
diff --git a/services/api/test/functional/arvados/v1/containers_controller_test.rb b/services/api/test/functional/arvados/v1/containers_controller_test.rb
index 5510e90..567e118 100644
--- a/services/api/test/functional/arvados/v1/containers_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/containers_controller_test.rb
@@ -106,7 +106,7 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
[:running, :lock, 422, 'Running'],
[:running, :unlock, 422, 'Running'],
].each do |fixture, action, response, state|
- test "state transitions from #{fixture } to #{action}" do
+ test "state transitions from #{fixture} to #{action}" do
authorize_with :dispatch1
uuid = containers(fixture).uuid
post action, {id: uuid}
@@ -133,4 +133,21 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
assert_response 401
end
+ [
+ [true, :running_container_auth],
+ [false, :dispatch2],
+ [false, :admin],
+ [false, :active],
+ ].each do |expect_success, auth|
+ test "get secret_mounts with #{auth} token" do
+ authorize_with auth
+ get :secret_mounts, {id: containers(:running).uuid}
+ if expect_success
+ assert_response :success
+ assert_equal "42\n", json_response["secret_mounts"]["/secret/6x9"]["content"]
+ else
+ assert_response 403
+ end
+ end
+ end
end
diff --git a/services/api/test/helpers/container_test_helper.rb b/services/api/test/helpers/container_test_helper.rb
new file mode 100644
index 0000000..15f0a08
--- /dev/null
+++ b/services/api/test/helpers/container_test_helper.rb
@@ -0,0 +1,11 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+module ContainerTestHelper
+ def assert_no_secrets_logged
+ Log.all.map(&:properties).each do |props|
+ refute_match /secret\/6x9/, SafeJSON.dump(props)
+ end
+ end
+end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 0edc0f4..93be0b5 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -3,11 +3,13 @@
# SPDX-License-Identifier: AGPL-3.0
require 'test_helper'
+require 'helpers/container_test_helper'
require 'helpers/docker_migration_helper'
class ContainerRequestTest < ActiveSupport::TestCase
include DockerMigrationHelper
include DbCurrentTime
+ include ContainerTestHelper
def create_minimal_req! attrs={}
defaults = {
@@ -836,4 +838,58 @@ class ContainerRequestTest < ActiveSupport::TestCase
end
end
+ test "reuse container with same secret_mounts" do
+ set_user_from_auth :active
+ cr1 = create_minimal_req!(state: "Committed", priority: 1)
+ cr1.save!
+ cr2 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: {})
+ cr2.save!
+ assert_not_nil cr1.container_uuid
+ assert_equal cr1.container_uuid, cr2.container_uuid
+ end
+
+ secrets = {"/foo" => {"kind" => "binary"}}
+ [
+ [true, nil, nil],
+ [true, nil, {}],
+ [true, {}, {}],
+ [true, secrets, secrets],
+ [false, nil, secrets],
+ [false, {}, secrets],
+ ].each do |expect_reuse, sm1, sm2|
+ test "container reuse secret_mounts #{sm1.inspect}, #{sm2.inspect}" do
+ set_user_from_auth :active
+ cr1 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: sm1)
+ cr1.save!
+ cr2 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: sm2)
+ cr2.save!
+ assert_not_nil cr1.container_uuid
+ assert_not_nil cr2.container_uuid
+ if expect_reuse
+ assert_equal cr1.container_uuid, cr2.container_uuid
+ else
+ assert_not_equal cr1.container_uuid, cr2.container_uuid
+ end
+ end
+ end
+
+ test "secret_mounts is null after container request is final" do
+ fail # todo: submit with secrets and finalize
+ assert_no_secrets_logged
+ end
+
+ test "reuse container with identical secret_mounts, even if scrubbed" do
+ fail # todo: run, ensure scrubbed, run identical
+ end
+
+ test "not reuse container with different secret_mounts" do
+ secrets = {"/foo" => {"kind" => "binary"}}
+ set_user_from_auth :active
+ cr1 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: secrets.dup)
+ cr1.save!
+ cr2 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: secrets.dup)
+ cr2.save!
+ assert_not_nil cr1.container_uuid
+ assert_equal cr1.container_uuid, cr2.container_uuid
+ end
end
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 0e13ee9..bd07a88 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -3,9 +3,11 @@
# SPDX-License-Identifier: AGPL-3.0
require 'test_helper'
+require 'helpers/container_test_helper'
class ContainerTest < ActiveSupport::TestCase
include DbCurrentTime
+ include ContainerTestHelper
DEFAULT_ATTRS = {
command: ['echo', 'foo'],
@@ -621,4 +623,12 @@ class ContainerTest < ActiveSupport::TestCase
end
end
+ [
+ Container::Complete,
+ Container::Cancelled,
+ ].each do |final_state|
+ test "secret_mounts is null after container is #{final_state}" do
+ assert_no_secrets_logged
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list