[ARVADOS] created: 1.2.0-32-gb7d199af0
Git user
git at public.curoverse.com
Mon Aug 20 21:21:00 EDT 2018
at b7d199af0a1844a4b6db38d315c26365617bfe41 (commit)
commit b7d199af0a1844a4b6db38d315c26365617bfe41
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Mon Aug 20 21:20:21 2018 -0400
14082: Datatype validation for command, environment, mounts
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index dd3ff767d..c434ee031 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -33,6 +33,7 @@ class ContainerRequest < ArvadosModel
validates :command, :container_image, :output_path, :cwd, :presence => true
validates :output_ttl, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 }
+ validate :validate_datatypes
validate :validate_scheduling_parameters
validate :validate_state_change
validate :check_update_whitelist
@@ -228,6 +229,43 @@ class ContainerRequest < ArvadosModel
end
end
+ def validate_datatypes
+ command.each do |c|
+ if !c.is_a? String
+ errors.add(:command, "must be an array of strings but has entry #{c.class}")
+ end
+ end
+ environment.each do |k,v|
+ if !k.is_a?(String) || !v.is_a?(String)
+ errors.add(:environment, "must be an map of String to String but has entry #{k.class} to #{v.class}")
+ end
+ end
+ [:mounts, :secret_mounts].each do |m|
+ self[m].each do |k, v|
+ if !k.is_a?(String) || !v.is_a?(Hash)
+ errors.add(m, "must be an map of String to Hash but is has entry #{k.class} to #{v.class}")
+ end
+ if v["kind"].nil?
+ errors.add(m, "each item must have a 'kind' field")
+ end
+ [[String, ["kind", "portable_data_hash", "uuid", "device_type",
+ "path", "commit", "repository_name", "git_url"]],
+ [Integer, ["capacity"]]].each do |t, fields|
+ fields.each do |f|
+ if !v[f].nil? && !v[f].is_a?(t)
+ errors.add(m, "#{k}: #{f} must be a #{t} but is #{v[f].class}")
+ end
+ end
+ end
+ ["writable", "exclude_from_output"].each do |f|
+ if !v[f].nil? && !v[f].is_a?(TrueClass) && !v[f].is_a?(FalseClass)
+ errors.add(m, "#{k}: #{f} must be a #{t} but is #{v[f].class}")
+ end
+ end
+ end
+ end
+ end
+
def validate_scheduling_parameters
if self.state == Committed
if scheduling_parameters.include? 'partitions' and
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index f266c096b..0a3b9b285 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -69,7 +69,7 @@ class ContainerRequestTest < ActiveSupport::TestCase
cr.container_image = "img3"
cr.cwd = "/tmp3"
cr.environment = {"BUP" => "BOP"}
- cr.mounts = {"BAR" => "BAZ"}
+ cr.mounts = {"BAR" => {"kind" => "BAZ"}}
cr.output_path = "/tmp4"
cr.priority = 2
cr.runtime_constraints = {"vcpus" => 4}
@@ -81,29 +81,33 @@ class ContainerRequestTest < ActiveSupport::TestCase
end
[
- {"vcpus" => 1},
- {"vcpus" => 1, "ram" => nil},
- {"vcpus" => 0, "ram" => 123},
- {"vcpus" => "1", "ram" => "123"}
- ].each do |invalid_constraints|
- test "Create with #{invalid_constraints}" do
+ {"runtime_constraints" => {"vcpus" => 1}},
+ {"runtime_constraints" => {"vcpus" => 1, "ram" => nil}},
+ {"runtime_constraints" => {"vcpus" => 0, "ram" => 123}},
+ {"runtime_constraints" => {"vcpus" => "1", "ram" => "123"}},
+ {"mounts" => {"FOO" => "BAR"}},
+ {"mounts" => {"FOO" => {}}},
+ {"mounts" => {"FOO" => {"kind" => "tmp", "capacity" => 42.222}}},
+ {"command" => ["echo", 55]},
+ {"environment" => {"FOO" => 55}}
+ ].each do |value|
+ test "Create with invalid #{value}" do
set_user_from_auth :active
assert_raises(ActiveRecord::RecordInvalid) do
- cr = create_minimal_req!(state: "Committed",
- priority: 1,
- runtime_constraints: invalid_constraints)
+ cr = create_minimal_req!({state: "Committed",
+ priority: 1}.merge(value))
cr.save!
end
end
- test "Update with #{invalid_constraints}" do
+ test "Update with invalid #{value}" do
set_user_from_auth :active
cr = create_minimal_req!(state: "Uncommitted", priority: 1)
cr.save!
assert_raises(ActiveRecord::RecordInvalid) do
cr = ContainerRequest.find_by_uuid cr.uuid
- cr.update_attributes!(state: "Committed",
- runtime_constraints: invalid_constraints)
+ cr.update_attributes!({state: "Committed",
+ priority: 1}.merge(value))
end
end
end
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 7ee5921e0..83ab59b60 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -84,7 +84,7 @@ class ContainerTest < ActiveSupport::TestCase
test "Container create" do
act_as_system_user do
c, _ = minimal_new(environment: {},
- mounts: {"BAR" => "FOO"},
+ mounts: {"BAR" => {"kind" => "FOO"}},
output_path: "/tmp",
priority: 1,
runtime_constraints: {"vcpus" => 1, "ram" => 1})
@@ -101,7 +101,7 @@ class ContainerTest < ActiveSupport::TestCase
test "Container valid priority" do
act_as_system_user do
c, _ = minimal_new(environment: {},
- mounts: {"BAR" => "FOO"},
+ mounts: {"BAR" => {"kind" => "FOO"}},
output_path: "/tmp",
priority: 1,
runtime_constraints: {"vcpus" => 1, "ram" => 1})
@@ -133,8 +133,8 @@ class ContainerTest < ActiveSupport::TestCase
test "Container serialized hash attributes sorted before save" do
- env = {"C" => 3, "B" => 2, "A" => 1}
- m = {"F" => {"kind" => 3}, "E" => {"kind" => 2}, "D" => {"kind" => 1}}
+ env = {"C" => "3", "B" => "2", "A" => "1"}
+ m = {"F" => {"kind" => "3"}, "E" => {"kind" => "2"}, "D" => {"kind" => "1"}}
rc = {"vcpus" => 1, "ram" => 1, "keep_cache_ram" => 1}
c, _ = minimal_new(environment: env, mounts: m, runtime_constraints: rc)
assert_equal c.environment.to_json, Container.deep_sort_hash(env).to_json
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list