Merge branch '15109-logs-table-admin-docs'

Closes #15109

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima@veritasgenetics.com>

diff --git a/build/run-tests.sh b/build/run-tests.sh
index a37a0f731e5a12c192c3a17094a9891ae920f9a6..dbf13940b2360997ab5885c8648b095a2b12ea33 100755 (executable)
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -394,7 +394,7 @@ start_services() {
         return 0
     fi
     . "$VENVDIR/bin/activate"
-    echo 'Starting API, keepproxy, keep-web, ws, arv-git-httpd, and nginx ssl proxy...'
+    echo 'Starting API, controller, keepproxy, keep-web, arv-git-httpd, ws, and nginx ssl proxy...'
     if [[ ! -d "$WORKSPACE/services/api/log" ]]; then
        mkdir -p "$WORKSPACE/services/api/log"
     fi
@@ -749,6 +749,7 @@ do_test_once() {
     title "test $1"
     timer_reset
 
+    result=
     if which deactivate >/dev/null; then deactivate; fi
     if ! . "$VENVDIR/bin/activate"
     then
@@ -821,6 +822,7 @@ do_install_once() {
     title "install $1"
     timer_reset
 
+    result=
     if which deactivate >/dev/null; then deactivate; fi
     if [[ "$1" != "env" ]] && ! . "$VENVDIR/bin/activate"; then
         result=1

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index fb900a993d464e809fd93ce0ecdacdae35995027..26fdadbd4e79fed55be147bb09c0e0789fd2643b 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -497,7 +497,7 @@ class Container < ArvadosModel
       return false
     end
 
-    if self.state == Running &&
+    if self.state_was == Running &&
        !current_api_client_authorization.nil? &&
        (current_api_client_authorization.uuid == self.auth_uuid ||
         current_api_client_authorization.token == self.runtime_token)
@@ -505,6 +505,8 @@ class Container < ArvadosModel
       # change priority or log.
       permitted.push *final_attrs
       permitted = permitted - [:log, :priority]
+    elsif !current_user.andand.is_admin
+      raise PermissionDeniedError
     elsif self.locked_by_uuid && self.locked_by_uuid != current_api_client_authorization.andand.uuid
       # When locked, progress fields cannot be updated by the wrong
       # dispatcher, even though it has admin privileges.

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 5ce3739a36dc7ca5d4002019d36144b90a584da0..320774189fc60ad1b63362c427e71c4ee2cdffb5 100644 (file)
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -184,7 +184,7 @@ class ContainerTest < ActiveSupport::TestCase
     assert_equal c1.runtime_status, {}
 
     assert_equal Container::Queued, c1.state
-    assert_raises ActiveRecord::RecordInvalid do
+    assert_raises ArvadosModel::PermissionDeniedError do
       c1.update_attributes! runtime_status: {'error' => 'Oops!'}
     end
 
@@ -777,6 +777,51 @@ class ContainerTest < ActiveSupport::TestCase
     end
   end
 
+  [
+    [Container::Queued, {state: Container::Locked}],
+    [Container::Queued, {state: Container::Running}],
+    [Container::Queued, {state: Container::Complete}],
+    [Container::Queued, {state: Container::Cancelled}],
+    [Container::Queued, {priority: 123456789}],
+    [Container::Queued, {runtime_status: {'error' => 'oops'}}],
+    [Container::Queued, {cwd: '/'}],
+    [Container::Locked, {state: Container::Running}],
+    [Container::Locked, {state: Container::Queued}],
+    [Container::Locked, {priority: 123456789}],
+    [Container::Locked, {runtime_status: {'error' => 'oops'}}],
+    [Container::Locked, {cwd: '/'}],
+    [Container::Running, {state: Container::Complete}],
+    [Container::Running, {state: Container::Cancelled}],
+    [Container::Running, {priority: 123456789}],
+    [Container::Running, {runtime_status: {'error' => 'oops'}}],
+    [Container::Running, {cwd: '/'}],
+    [Container::Complete, {state: Container::Cancelled}],
+    [Container::Complete, {priority: 123456789}],
+    [Container::Complete, {runtime_status: {'error' => 'oops'}}],
+    [Container::Complete, {cwd: '/'}],
+    [Container::Cancelled, {cwd: '/'}],
+  ].each do |start_state, updates|
+    test "Container update #{updates.inspect} when #{start_state} forbidden for non-admin" do
+      set_user_from_auth :active
+      c, _ = minimal_new
+      if start_state != Container::Queued
+        set_user_from_auth :dispatch1
+        c.lock
+        if start_state != Container::Locked
+          c.update_attributes! state: Container::Running
+          if start_state != Container::Running
+            c.update_attributes! state: start_state
+          end
+        end
+      end
+      assert_equal c.state, start_state
+      set_user_from_auth :active
+      assert_raises(ArvadosModel::PermissionDeniedError) do
+        c.update_attributes! updates
+      end
+    end
+  end
+
   test "Container only set exit code on complete" do
     set_user_from_auth :active
     c, _ = minimal_new
@@ -899,7 +944,9 @@ class ContainerTest < ActiveSupport::TestCase
     c.update_attributes! state: Container::Running
 
     set_user_from_auth :running_to_be_deleted_container_auth
-    refute c.update_attributes(output: collections(:foo_file).portable_data_hash)
+    assert_raises(ArvadosModel::PermissionDeniedError) do
+      c.update_attributes(output: collections(:foo_file).portable_data_hash)
+    end
   end
 
   test "can set trashed output on running container" do