8470: Resolve mounts to PDH.

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 221da0d9bb3fe105fb8af1e9978ff1520531adc4..15418f237fd25f9c68b06647836252dbdca3dbf8 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -82,15 +82,17 @@ class ContainerRequest < ArvadosModel
   # Create a new container (or find an existing one) to satisfy this
   # request.
   def resolve
-    # TODO: resolve mounts and container_image to content addresses.
+    # TODO: resolve container_image to a content address.
+    c_mounts = mounts_for_container
+    c_runtime_constraints = runtime_constraints_for_container
     c = act_as_system_user do
       Container.create!(command: self.command,
                         container_image: self.container_image,
                         cwd: self.cwd,
                         environment: self.environment,
-                        mounts: self.mounts,
+                        mounts: c_mounts,
                         output_path: self.output_path,
-                        runtime_constraints: runtime_constraints_for_container)
+                        runtime_constraints: c_runtime_constraints)
     end
     self.container_uuid = c.uuid
   end
@@ -116,6 +118,37 @@ class ContainerRequest < ArvadosModel
     rc
   end
 
+  # Return a mounts hash suitable for a Container, i.e., with every
+  # readonly collection UUID resolved to a PDH.
+  def mounts_for_container
+    c_mounts = {}
+    mounts.each do |k, mount|
+      mount = mount.dup
+      c_mounts[k] = mount
+      if mount['kind'] != 'collection'
+        next
+      end
+      if (uuid = mount.delete 'uuid')
+        c = Collection.
+          readable_by(current_user).
+          where(uuid: uuid).
+          select(:portable_data_hash).
+          first
+        if !c
+          raise ActiveRecord::RecordNotFound.new "cannot mount collection #{uuid.inspect}: not found"
+        end
+        if mount['portable_data_hash'].nil?
+          # PDH not supplied by client
+          mount['portable_data_hash'] = c.portable_data_hash
+        elsif mount['portable_data_hash'] != c.portable_data_hash
+          # UUID and PDH supplied by client, but they don't agree
+          raise ArgumentError.new "cannot mount collection #{uuid.inspect}: current portable_data_hash #{c.portable_data_hash.inspect} does not match #{c['portable_data_hash'].inspect} in request"
+        end
+      end
+    end
+    return c_mounts
+  end
+
   def set_container
     if (container_uuid_changed? and
         not current_user.andand.is_admin and

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index f510f259b2763fa7b81a4b95863c562c7894a98a..46f564e0600d5704999570b3c21151caae3b5e5f 100644 (file)
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -126,20 +126,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
   end
 
   test "Container request priority must be non-nil" do
-    set_user_from_auth :active_trustedclient
-    cr = ContainerRequest.new
-    cr.command = ["echo", "foo"]
-    cr.container_image = "img"
-    cr.cwd = "/tmp"
-    cr.environment = {}
-    cr.mounts = {"BAR" => "FOO"}
-    cr.output_path = "/tmpout"
-    cr.runtime_constraints = {}
-    cr.name = "foo"
-    cr.description = "bar"
-    cr.save!
-
-    cr.reload
+    set_user_from_auth :active
+    cr = create_minimal_req!(priority: nil)
     cr.state = "Committed"
     assert_raises(ActiveRecord::RecordInvalid) do
       cr.save!
@@ -406,4 +394,68 @@ class ContainerRequestTest < ActiveSupport::TestCase
              "container runtime_constraints was #{resolved.inspect}")
     end
   end
+
+  [[{"/out" => {
+        "kind" => "collection",
+        "uuid" => "zzzzz-4zz18-znfnqtbbv4spc3w",
+        "path" => "/foo"}},
+    lambda do |resolved|
+      resolved["/out"] == {
+        "portable_data_hash" => "1f4b0bc7583c2a7f9102c395f4ffc5e3+45",
+        "kind" => "collection",
+        "path" => "/foo",
+      }
+    end],
+   [{"/out" => {
+        "kind" => "collection",
+        "uuid" => "zzzzz-4zz18-znfnqtbbv4spc3w",
+        "portable_data_hash" => "1f4b0bc7583c2a7f9102c395f4ffc5e3+45",
+        "path" => "/foo"}},
+    lambda do |resolved|
+      resolved["/out"] == {
+        "portable_data_hash" => "1f4b0bc7583c2a7f9102c395f4ffc5e3+45",
+        "kind" => "collection",
+        "path" => "/foo",
+      }
+    end],
+  ].each do |mounts, okfunc|
+    test "resolve mounts #{mounts.inspect} to values" do
+      set_user_from_auth :active
+      cr = ContainerRequest.new(mounts: mounts)
+      resolved = cr.send :mounts_for_container
+      assert(okfunc.call(resolved),
+             "mounts_for_container returned #{resolved.inspect}")
+    end
+  end
+
+  test 'mount unreadable collection' do
+    set_user_from_auth :spectator
+    m = {
+      "/foo" => {
+        "kind" => "collection",
+        "uuid" => "zzzzz-4zz18-znfnqtbbv4spc3w",
+        "path" => "/foo",
+      },
+    }
+    cr = ContainerRequest.new(mounts: m)
+    assert_raises(ActiveRecord::RecordNotFound) do
+      cr.send :mounts_for_container
+    end
+  end
+
+  test 'mount collection with mismatched UUID and PDH' do
+    set_user_from_auth :active
+    m = {
+      "/foo" => {
+        "kind" => "collection",
+        "uuid" => "zzzzz-4zz18-znfnqtbbv4spc3w",
+        "portable_data_hash" => "fa7aeb5140e2848d39b416daeef4ffc5+45",
+        "path" => "/foo",
+      },
+    }
+    cr = ContainerRequest.new(mounts: m)
+    assert_raises(ArgumentError) do
+      cr.send :mounts_for_container
+    end
+  end
 end