Merge branch '18596-preemptible-price-factor'
authorTom Clegg <tom@curii.com>
Thu, 24 Mar 2022 15:26:44 +0000 (11:26 -0400)
committerTom Clegg <tom@curii.com>
Thu, 24 Mar 2022 15:26:44 +0000 (11:26 -0400)
closes #18596

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

29 files changed:
doc/_includes/_container_scheduling_parameters.liquid
doc/api/methods/collections.html.textile.liquid
doc/api/methods/groups.html.textile.liquid
doc/install/install-postgresql.html.textile.liquid
lib/config/config.default.yml
lib/config/export.go
lib/controller/handler_test.go
lib/controller/router/response.go
lib/crunchrun/crunchrun.go
lib/crunchrun/crunchrun_test.go
sdk/cwl/test_with_arvbox.sh
sdk/go/arvados/config.go
sdk/go/arvados/group.go
services/api/app/models/arvados_model.rb
services/api/app/models/frozen_group.rb [new file with mode: 0644]
services/api/app/models/group.rb
services/api/app/models/user.rb
services/api/db/migrate/20220224203102_add_frozen_by_uuid_to_groups.rb [new file with mode: 0644]
services/api/db/migrate/20220301155729_frozen_groups.rb [new file with mode: 0644]
services/api/db/migrate/20220303204419_add_frozen_by_uuid_to_group_search_index.rb [new file with mode: 0644]
services/api/db/structure.sql
services/api/lib/20200501150153_permission_table_constants.rb
services/api/test/functional/arvados/v1/groups_controller_test.rb
services/api/test/unit/group_test.rb
services/api/test/unit/permission_test.rb
tools/arvbox/bin/arvbox
tools/salt-install/config_examples/multi_host/aws/states/shell_sudo_passwordless.sls [new file with mode: 0644]
tools/salt-install/config_examples/single_host/single_hostname/states/shell_sudo_passwordless.sls [new file with mode: 0644]
tools/salt-install/provision.sh

index be046173ad028b92fd9b26a4a2994f5b4c9fd295..636b6df59c455d5badac1bda9ded03434403207d 100644 (file)
@@ -11,5 +11,5 @@ Parameters to be passed to the container scheduler (e.g., Slurm) when running a
 table(table table-bordered table-condensed).
 |_. Key|_. Type|_. Description|_. Notes|
 |partitions|array of strings|The names of one or more compute partitions that may run this container. If not provided, the system will choose where to run the container.|Optional.|
-|preemptible|boolean|If true, the dispatcher will ask for a preemptible cloud node instance (eg: AWS Spot Instance) to run this container.|Optional. Default is false.|
+|preemptible|boolean|If true, the dispatcher should use a preemptible cloud node instance (eg: AWS Spot Instance) to run this container.  Whether a preemptible instance is actually used "depends on cluster configuration.":{{site.baseurl}}/admin/spot-instances.html|Optional. Default is false.|
 |max_run_time|integer|Maximum running time (in seconds) that this container will be allowed to run before being cancelled.|Optional. Default is 0 (no limit).|
index 5ff8d529f8376ceb8b5372f8a94873e921c0961e..a2a6a77e19f6350c1be3c770a1560f42c9cc6402 100644 (file)
@@ -109,6 +109,36 @@ table(table table-bordered table-condensed).
 
 Note: Because adding access tokens to manifests can be computationally expensive, the @manifest_text@ field is not included in results by default.  If you need it, pass a @select@ parameter that includes @manifest_text@.
 
+h4. Searching Collections for names of file or directories
+
+You can search collections for specific file or directory names (whole or part) using the following filter in a @list@ query.
+
+<pre>
+filters: [["file_names", "ilike", "%sample1234.fastq%"]]
+</pre>
+
+Note: @file_names@ is a hidden field used for indexing.  It is not returned by any API call.  On the client, you can programmatically enumerate all the files in a collection using @arv-ls@, the Python SDK @Collection@ class, Go SDK @FileSystem@ struct, the WebDAV API, or the S3-compatible API.
+
+As of this writing (Arvados 2.4), you can also search for directory paths, but _not_ complete file paths.
+
+In other words, this will work (when @dir3@ is a directory):
+
+<pre>
+filters: [["file_names", "ilike", "%dir1/dir2/dir3%"]]
+</pre>
+
+However, this will _not_ return the desired results (where @sample1234.fastq@ is a file):
+
+<pre>
+filters: [["file_names", "ilike", "%dir1/dir2/dir3/sample1234.fastq%"]]
+</pre>
+
+As a workaround, you can search for both the directory path and file name separately, and then filter on the client side.
+
+<pre>
+filters: [["file_names", "ilike", "%dir1/dir2/dir3%"], ["file_names", "ilike", "%sample1234.fastq%"]]
+</pre>
+
 h3. update
 
 Update attributes of an existing Collection.
index e4b8594dd195b7e7c4240d9628cc83065572f0f5..2a762d92480955c0a55ed3e2e4fe11d7139b6664 100644 (file)
@@ -34,6 +34,17 @@ table(table table-bordered table-condensed).
 |trash_at|datetime|If @trash_at@ is non-null and in the past, this group and all objects directly or indirectly owned by the group will be hidden from API calls.  May be untrashed.||
 |delete_at|datetime|If @delete_at@ is non-null and in the past, the group and all objects directly or indirectly owned by the group may be permanently deleted.||
 |is_trashed|datetime|True if @trash_at@ is in the past, false if not.||
+|frozen_by_uuid|string|For a frozen project, indicates the user who froze the project; null in all other cases. When a project is frozen, no further changes can be made to the project or its contents, even by admins. Attempting to add new items or modify, rename, move, trash, or delete the project or its contents, including any subprojects, will return an error.||
+
+h3. Frozen projects
+
+A user with @manage@ permission can set the @frozen_by_uuid@ attribute of a @project@ group to their own user UUID. Once this is done, no further changes can be made to the project or its contents, including subprojects.
+
+The @frozen_by_uuid@ attribute can be cleared by an admin user. It can also be cleared by a user with @manage@ permission, unless the @API.UnfreezeProjectRequiresAdmin@ configuration setting is active.
+
+The optional @API.FreezeProjectRequiresDescription@ and @API.FreezeProjectRequiresProperties@ configuration settings can be used to prevent users from freezing projects that have empty @description@ and/or specified @properties@ entries.
+
+h3. Filter groups
 
 @filter@ groups are virtual groups; they can not own other objects. Filter groups have a special @properties@ field named @filters@, which must be an array of filter conditions. See "list method filters":{{site.baseurl}}/api/methods.html#filters for details on the syntax of valid filters, but keep in mind that the attributes must include the object type (@collections@, @container_requests@, @groups@, @workflows@), separated with a dot from the field to be filtered on.
 
index 1413890cde7dbd5f6be60f64f7ffe9e32a552525..a9614b9be58eba7afb2f9fae5bab6f79458e487d 100644 (file)
@@ -9,7 +9,7 @@ Copyright (C) The Arvados Authors. All rights reserved.
 SPDX-License-Identifier: CC-BY-SA-3.0
 {% endcomment %}
 
-Arvados requires at least version *9.4* of PostgreSQL.
+Arvados requires at least version *9.4* of PostgreSQL. We recommend using version 10 or newer.
 
 * "AWS":#aws
 * "CentOS 7":#centos7
index 318e9ab1dcd5d84ee509862e02e897cbbd4eebca..6512389815dd4e3d1d786b09cf1cd5b3eedd929d 100644 (file)
@@ -240,6 +240,18 @@ Clusters:
       # https://doc.arvados.org/admin/metadata-vocabulary.html
       VocabularyPath: ""
 
+      # If true, a project must have a non-empty description field in
+      # order to be frozen.
+      FreezeProjectRequiresDescription: false
+
+      # Project properties that must have non-empty values in order to
+      # freeze a project. Example: {"property_name": true}
+      FreezeProjectRequiresProperties: {}
+
+      # If true, only an admin user can un-freeze a project. If false,
+      # any user with "manage" permission can un-freeze.
+      UnfreezeProjectRequiresAdmin: false
+
     Users:
       # Config parameters to automatically setup new users.  If enabled,
       # this users will be able to self-activate.  Enable this if you want
index 67b7c3fa0ecbd1ce49df2962cfda66ae056ec693..dae749c87470ecb5f4ab7c496ddf001c11abcdf3 100644 (file)
@@ -62,6 +62,8 @@ var whitelist = map[string]bool{
        "API":                                      true,
        "API.AsyncPermissionsUpdateInterval":       false,
        "API.DisabledAPIs":                         false,
+       "API.FreezeProjectRequiresDescription":     true,
+       "API.FreezeProjectRequiresProperties":      true,
        "API.KeepServiceRequestTimeout":            false,
        "API.MaxConcurrentRequests":                false,
        "API.MaxIndexDatabaseRead":                 false,
@@ -72,6 +74,7 @@ var whitelist = map[string]bool{
        "API.MaxTokenLifetime":                     false,
        "API.RequestTimeout":                       true,
        "API.SendTimeout":                          true,
+       "API.UnfreezeProjectRequiresAdmin":         true,
        "API.VocabularyPath":                       false,
        "API.WebsocketClientEventQueue":            false,
        "API.WebsocketServerEventQueue":            false,
index 723e1011f9de62ac871566ec21c4488d1f55b360..817cff79609dab91b7743f2869950974c8796082 100644 (file)
@@ -367,16 +367,14 @@ func (s *HandlerSuite) CheckObjectType(c *check.C, url string, token string, ski
        for k := range direct {
                if _, ok := skippedFields[k]; ok {
                        continue
-               } else if val, ok := proxied[k]; ok {
-                       if direct["kind"] == "arvados#collection" && k == "manifest_text" {
-                               // Tokens differ from request to request
-                               c.Check(strings.Split(val.(string), "+A")[0], check.Equals, strings.Split(direct[k].(string), "+A")[0])
-                       } else {
-                               c.Check(val, check.DeepEquals, direct[k],
-                                       check.Commentf("RailsAPI %s key %q's value %q differs from controller's %q.", direct["kind"], k, direct[k], val))
-                       }
-               } else {
+               } else if val, ok := proxied[k]; !ok {
                        c.Errorf("%s's key %q missing on controller's response.", direct["kind"], k)
+               } else if direct["kind"] == "arvados#collection" && k == "manifest_text" {
+                       // Tokens differ from request to request
+                       c.Check(strings.Split(val.(string), "+A")[0], check.Equals, strings.Split(direct[k].(string), "+A")[0])
+               } else {
+                       c.Check(val, check.DeepEquals, direct[k],
+                               check.Commentf("RailsAPI %s key %q's value %q differs from controller's %q.", direct["kind"], k, direct[k], val))
                }
        }
 }
index c0c599be8bcd4f1acb6fbbc5ed90e6541cf08dfe..42b34355935db015e72d7de3e8b7a6398ffc0680 100644 (file)
@@ -208,7 +208,7 @@ func (rtr *router) mungeItemFields(tmp map[string]interface{}) {
                // they appear in responses as null, rather than a
                // zero value.
                switch k {
-               case "output_uuid", "output_name", "log_uuid", "description", "requesting_container_uuid", "container_uuid":
+               case "output_uuid", "output_name", "log_uuid", "description", "requesting_container_uuid", "container_uuid", "modified_by_client_uuid", "frozen_by_uuid":
                        if v == "" {
                                tmp[k] = nil
                        }
index 4fa3f26ab54dc4fb5a0fba4cdc1a12fe4a3a94d0..65f43e96440aa57508cb7e2a80af99e455420120 100644 (file)
@@ -19,6 +19,7 @@ import (
        "os"
        "os/exec"
        "os/signal"
+       "os/user"
        "path"
        "path/filepath"
        "regexp"
@@ -1475,6 +1476,7 @@ func (runner *ContainerRunner) NewArvLogWriter(name string) (io.WriteCloser, err
 // Run the full container lifecycle.
 func (runner *ContainerRunner) Run() (err error) {
        runner.CrunchLog.Printf("crunch-run %s started", cmd.Version.String())
+       runner.CrunchLog.Printf("%s", currentUserAndGroups())
        runner.CrunchLog.Printf("Executing container '%s' using %s runtime", runner.Container.UUID, runner.executor.Runtime())
 
        hostname, hosterr := os.Hostname()
@@ -2045,3 +2047,30 @@ func startLocalKeepstore(configData ConfigData, logbuf io.Writer) (*exec.Cmd, er
        os.Setenv("ARVADOS_KEEP_SERVICES", url)
        return cmd, nil
 }
+
+// return current uid, gid, groups in a format suitable for logging:
+// "crunch-run process has uid=1234(arvados) gid=1234(arvados)
+// groups=1234(arvados),114(fuse)"
+func currentUserAndGroups() string {
+       u, err := user.Current()
+       if err != nil {
+               return fmt.Sprintf("error getting current user ID: %s", err)
+       }
+       s := fmt.Sprintf("crunch-run process has uid=%s(%s) gid=%s", u.Uid, u.Username, u.Gid)
+       if g, err := user.LookupGroupId(u.Gid); err == nil {
+               s += fmt.Sprintf("(%s)", g.Name)
+       }
+       s += " groups="
+       if gids, err := u.GroupIds(); err == nil {
+               for i, gid := range gids {
+                       if i > 0 {
+                               s += ","
+                       }
+                       s += gid
+                       if g, err := user.LookupGroupId(gid); err == nil {
+                               s += fmt.Sprintf("(%s)", g.Name)
+                       }
+               }
+       }
+       return s
+}
index 26f78d2bf7d537c7a44f0b8dd57eda4355211b5b..62df0032b40800e9b2c2eb59ed4c42e639f9e9a1 100644 (file)
@@ -885,6 +885,7 @@ func (s *TestSuite) TestLogVersionAndRuntime(c *C) {
 
        c.Assert(s.api.Logs["crunch-run"], NotNil)
        c.Check(s.api.Logs["crunch-run"].String(), Matches, `(?ms).*crunch-run \S+ \(go\S+\) start.*`)
+       c.Check(s.api.Logs["crunch-run"].String(), Matches, `(?ms).*crunch-run process has uid=\d+\(.+\) gid=\d+\(.+\) groups=\d+\(.+\)(,\d+\(.+\))*\n.*`)
        c.Check(s.api.Logs["crunch-run"].String(), Matches, `(?ms).*Executing container 'zzzzz-zzzzz-zzzzzzzzzzzzzzz' using stub runtime.*`)
 }
 
index 0021bc8d906c5531b70c79a87d9be169658b5c57..d38414fc811d433acb5751613c9974f1bbc0b0c5 100755 (executable)
@@ -118,14 +118,15 @@ elif [[ "$suite" =~ conformance-(.*) ]] ; then
      git clone https://github.com/common-workflow-language/cwl-\${version}.git
    fi
    cd cwl-\${version}
+   git checkout \${version}.0
 elif [[ "$suite" != "integration" ]] ; then
    echo "ERROR: unknown suite '$suite'"
    exit 1
 fi
 
-if [[ "$suite" != "integration" ]] ; then
-  git pull
-fi
+#if [[ "$suite" != "integration" ]] ; then
+#  git pull
+#fi
 
 export ARVADOS_API_HOST=localhost:8000
 export ARVADOS_API_HOST_INSECURE=1
@@ -154,18 +155,6 @@ else
   arv-keepdocker arvados/jobs latest
 fi
 
-cat >/tmp/cwltest/arv-cwl-jobs <<EOF2
-#!/bin/sh
-exec arvados-cwl-runner --api=jobs \\\$@
-EOF2
-chmod +x /tmp/cwltest/arv-cwl-jobs
-
-cat >/tmp/cwltest/arv-cwl-containers <<EOF2
-#!/bin/sh
-exec arvados-cwl-runner --api=containers \\\$@
-EOF2
-chmod +x /tmp/cwltest/arv-cwl-containers
-
 EXTRA=--compute-checksum
 
 if [[ $devcwl -eq 1 ]] ; then
@@ -176,8 +165,10 @@ env
 if [[ "$suite" = "integration" ]] ; then
    cd /usr/src/arvados/sdk/cwl/tests
    exec ./arvados-tests.sh $@
+elif [[ "$suite" = "conformance-v1.2" ]] ; then
+   exec cwltest --tool arvados-cwl-runner --test conformance_tests.yaml -N307 $@ -- \$EXTRA
 else
-   exec ./run_test.sh RUNNER=/tmp/cwltest/arv-cwl-${runapi} EXTRA="\$EXTRA" $@
+   exec ./run_test.sh RUNNER=arvados-cwl-runner EXTRA="\$EXTRA" $@
 fi
 EOF
 
index 6242f6cd56ccee653c54bf7d6e1df160652cc5fe..6c9324e478a273b68e83dcd7fb4e46150b8e0da1 100644 (file)
@@ -94,21 +94,24 @@ type Cluster struct {
        PostgreSQL      PostgreSQL
 
        API struct {
-               AsyncPermissionsUpdateInterval Duration
-               DisabledAPIs                   StringSet
-               MaxIndexDatabaseRead           int
-               MaxItemsPerResponse            int
-               MaxConcurrentRequests          int
-               MaxKeepBlobBuffers             int
-               MaxRequestAmplification        int
-               MaxRequestSize                 int
-               MaxTokenLifetime               Duration
-               RequestTimeout                 Duration
-               SendTimeout                    Duration
-               WebsocketClientEventQueue      int
-               WebsocketServerEventQueue      int
-               KeepServiceRequestTimeout      Duration
-               VocabularyPath                 string
+               AsyncPermissionsUpdateInterval   Duration
+               DisabledAPIs                     StringSet
+               MaxIndexDatabaseRead             int
+               MaxItemsPerResponse              int
+               MaxConcurrentRequests            int
+               MaxKeepBlobBuffers               int
+               MaxRequestAmplification          int
+               MaxRequestSize                   int
+               MaxTokenLifetime                 Duration
+               RequestTimeout                   Duration
+               SendTimeout                      Duration
+               WebsocketClientEventQueue        int
+               WebsocketServerEventQueue        int
+               KeepServiceRequestTimeout        Duration
+               VocabularyPath                   string
+               FreezeProjectRequiresDescription bool
+               FreezeProjectRequiresProperties  StringSet
+               UnfreezeProjectRequiresAdmin     bool
        }
        AuditLogs struct {
                MaxAge             Duration
index d5243dc170708afac1e51556adbf145c900c4f43..ad7ac1ee2b74a0d972b2adb4570a6ba417d455b6 100644 (file)
@@ -26,6 +26,7 @@ type Group struct {
        Properties           map[string]interface{} `json:"properties"`
        WritableBy           []string               `json:"writable_by,omitempty"`
        Description          string                 `json:"description"`
+       FrozenByUUID         string                 `json:"frozen_by_uuid"`
 }
 
 // GroupList is an arvados#groupList resource.
index 1c842c48d6bf1d283ae08608dc242967b0cc2b56..327bf63b5fa057779d6d03d99331b179077611db 100644 (file)
@@ -246,6 +246,15 @@ class ArvadosModel < ApplicationRecord
   # If current user cannot write this object, just return
   # [self.owner_uuid].
   def writable_by
+    # Return [] if this is a frozen project and the current user can't
+    # unfreeze
+    return [] if respond_to?(:frozen_by_uuid) && frozen_by_uuid &&
+                 (Rails.configuration.API.UnfreezeProjectRequiresAdmin ?
+                    !current_user.andand.is_admin :
+                    !current_user.can?(manage: uuid))
+    # Return [] if nobody can write because this object is inside a
+    # frozen project
+    return [] if FrozenGroup.where(uuid: owner_uuid).any?
     return [owner_uuid] if not current_user
     unless (owner_uuid == current_user.uuid or
             current_user.is_admin or
@@ -288,26 +297,41 @@ class ArvadosModel < ApplicationRecord
     user_uuids = users_list.map { |u| u.uuid }
     all_user_uuids = []
 
+    admin = users_list.select { |u| u.is_admin }.any?
+
     # For details on how the trashed_groups table is constructed, see
     # see db/migrate/20200501150153_permission_table.rb
 
-    exclude_trashed_records = ""
-    if !include_trash and (sql_table == "groups" or sql_table == "collections") then
-      # Only include records that are not trashed
-      exclude_trashed_records = "AND (#{sql_table}.trash_at is NULL or #{sql_table}.trash_at > statement_timestamp())"
-    end
+    # excluded_trash is a SQL expression that determines whether a row
+    # should be excluded from the results due to being trashed.
+    # Trashed items inside frozen projects are invisible to regular
+    # (non-admin) users even when using include_trash, so we have:
+    #
+    # (item_trashed || item_inside_trashed_project)
+    # &&
+    # (!caller_requests_include_trash ||
+    #  (item_inside_frozen_project && caller_is_not_admin))
+    if (admin && include_trash) || sql_table == "api_client_authorizations"
+      excluded_trash = "false"
+    else
+      excluded_trash = "(#{sql_table}.owner_uuid IN (SELECT group_uuid FROM #{TRASHED_GROUPS} " +
+                       "WHERE trash_at <= statement_timestamp()))"
+      if sql_table == "groups" || sql_table == "collections"
+        excluded_trash = "(#{excluded_trash} OR #{sql_table}.trash_at <= statement_timestamp() IS TRUE)"
+      end
 
-    trashed_check = ""
-    if !include_trash && sql_table != "api_client_authorizations"
-      trashed_check = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM #{TRASHED_GROUPS} " +
-                      "where trash_at <= statement_timestamp()) #{exclude_trashed_records}"
+      if include_trash
+        # Exclude trash inside frozen projects
+        excluded_trash = "(#{excluded_trash} AND #{sql_table}.owner_uuid IN (SELECT uuid FROM #{FROZEN_GROUPS}))"
+      end
     end
 
-    if users_list.select { |u| u.is_admin }.any?
-      # Admin skips most permission checks, but still want to filter on trashed items.
+    if admin
+      # Admin skips most permission checks, but still want to filter
+      # on trashed items.
       if !include_trash && sql_table != "api_client_authorizations"
         # Only include records where the owner is not trashed
-        sql_conds = trashed_check
+        sql_conds = "NOT (#{excluded_trash})"
       end
     else
       # The core of the permission check is a join against the
@@ -407,7 +431,7 @@ class ArvadosModel < ApplicationRecord
                      "    WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 3))) "
       end
 
-      sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) #{trashed_check.empty? ? "" : "AND"} #{trashed_check}"
+      sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) AND NOT (#{excluded_trash})"
 
     end
 
@@ -614,8 +638,12 @@ class ArvadosModel < ApplicationRecord
         if check_uuid.nil?
           # old_owner_uuid is nil? New record, no need to check.
         elsif !current_user.can?(write: check_uuid)
-          logger.warn "User #{current_user.uuid} tried to set ownership of #{self.class.to_s} #{self.uuid} but does not have permission to write #{which} owner_uuid #{check_uuid}"
-          errors.add :owner_uuid, "cannot be set or changed without write permission on #{which} owner"
+          if FrozenGroup.where(uuid: check_uuid).any?
+            errors.add :owner_uuid, "cannot be set or changed because #{which} owner is frozen"
+          else
+            logger.warn "User #{current_user.uuid} tried to set ownership of #{self.class.to_s} #{self.uuid} but does not have permission to write #{which} owner_uuid #{check_uuid}"
+            errors.add :owner_uuid, "cannot be set or changed without write permission on #{which} owner"
+          end
           raise PermissionDeniedError
         elsif rsc_class == Group && Group.find_by_uuid(owner_uuid).group_class != "project"
           errors.add :owner_uuid, "must be a project"
@@ -625,8 +653,12 @@ class ArvadosModel < ApplicationRecord
     else
       # If the object already existed and we're not changing
       # owner_uuid, we only need write permission on the object
-      # itself.
-      if !current_user.can?(write: self.uuid)
+      # itself. (If we're in the act of unfreezing, we only need
+      # :unfreeze permission, which means "what write permission would
+      # be if target weren't frozen")
+      unless ((respond_to?(:frozen_by_uuid) && frozen_by_uuid_was && !frozen_by_uuid) ?
+                current_user.can?(unfreeze: uuid) :
+                current_user.can?(write: uuid))
         logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{self.uuid} without write permission"
         errors.add :uuid, " #{uuid} is not writable by #{current_user.uuid}"
         raise PermissionDeniedError
@@ -643,7 +675,7 @@ class ArvadosModel < ApplicationRecord
   end
 
   def permission_to_create
-    current_user.andand.is_active
+    return current_user.andand.is_active
   end
 
   def permission_to_update
diff --git a/services/api/app/models/frozen_group.rb b/services/api/app/models/frozen_group.rb
new file mode 100644 (file)
index 0000000..bf4ee5d
--- /dev/null
@@ -0,0 +1,6 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class FrozenGroup < ApplicationRecord
+end
index 8565b2a417efc0a28d611c2ff1ed873acfc39a8c..b1b2e942c60c4bc79c13dbe731ad9bc1112684d2 100644 (file)
@@ -19,9 +19,11 @@ class Group < ArvadosModel
   validate :ensure_filesystem_compatible_name
   validate :check_group_class
   validate :check_filter_group_filters
+  validate :check_frozen_state_change_allowed
   before_create :assign_name
   after_create :after_ownership_change
   after_create :update_trash
+  after_create :update_frozen
 
   before_update :before_ownership_change
   after_update :after_ownership_change
@@ -29,7 +31,8 @@ class Group < ArvadosModel
   after_create :add_role_manage_link
 
   after_update :update_trash
-  before_destroy :clear_permissions_and_trash
+  after_update :update_frozen
+  before_destroy :clear_permissions_trash_frozen
 
   api_accessible :user, extend: :common do |t|
     t.add :name
@@ -40,6 +43,7 @@ class Group < ArvadosModel
     t.add :trash_at
     t.add :is_trashed
     t.add :properties
+    t.add :frozen_by_uuid
   end
 
   def ensure_filesystem_compatible_name
@@ -92,37 +96,104 @@ class Group < ArvadosModel
     end
   end
 
+  def check_frozen_state_change_allowed
+    if frozen_by_uuid == ""
+      self.frozen_by_uuid = nil
+    end
+    if frozen_by_uuid_changed? || (new_record? && frozen_by_uuid)
+      if group_class != "project"
+        errors.add(:frozen_by_uuid, "cannot be modified on a non-project group")
+        return
+      end
+      if frozen_by_uuid_was && Rails.configuration.API.UnfreezeProjectRequiresAdmin && !current_user.is_admin
+        errors.add(:frozen_by_uuid, "can only be changed by an admin user, once set")
+        return
+      end
+      if frozen_by_uuid && frozen_by_uuid != current_user.uuid
+        errors.add(:frozen_by_uuid, "can only be set to the current user's UUID")
+        return
+      end
+      if !new_record? && !current_user.can?(manage: uuid)
+        raise PermissionDeniedError
+      end
+      if trash_at || delete_at || (!new_record? && TrashedGroup.where(group_uuid: uuid).any?)
+        errors.add(:frozen_by_uuid, "cannot be set on a trashed project")
+      end
+      if frozen_by_uuid_was.nil?
+        if Rails.configuration.API.FreezeProjectRequiresDescription && !attribute_present?(:description)
+          errors.add(:frozen_by_uuid, "can only be set if description is non-empty")
+        end
+        Rails.configuration.API.FreezeProjectRequiresProperties.andand.each do |key, _|
+          key = key.to_s
+          if !properties[key] || properties[key] == ""
+            errors.add(:frozen_by_uuid, "can only be set if properties[#{key}] value is non-empty")
+          end
+        end
+      end
+    end
+  end
+
   def update_trash
-    if saved_change_to_trash_at? or saved_change_to_owner_uuid?
-      # The group was added or removed from the trash.
-      #
-      # Strategy:
-      #   Compute project subtree, propagating trash_at to subprojects
-      #   Remove groups that don't belong from trash
-      #   Add/update groups that do belong in the trash
-
-      temptable = "group_subtree_#{rand(2**64).to_s(10)}"
-      ActiveRecord::Base.connection.exec_query %{
-create temporary table #{temptable} on commit drop
-as select * from project_subtree_with_trash_at($1, LEAST($2, $3)::timestamp)
-},
-                                               'Group.update_trash.select',
-                                               [[nil, self.uuid],
-                                                [nil, TrashedGroup.find_by_group_uuid(self.owner_uuid).andand.trash_at],
-                                                [nil, self.trash_at]]
-
-      ActiveRecord::Base.connection.exec_delete %{
-delete from trashed_groups where group_uuid in (select target_uuid from #{temptable} where trash_at is NULL);
-},
-                                            "Group.update_trash.delete"
-
-      ActiveRecord::Base.connection.exec_query %{
-insert into trashed_groups (group_uuid, trash_at)
-  select target_uuid as group_uuid, trash_at from #{temptable} where trash_at is not NULL
-on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
-},
-                                            "Group.update_trash.insert"
+    return unless saved_change_to_trash_at? || saved_change_to_owner_uuid?
+
+    # The group was added or removed from the trash.
+    #
+    # Strategy:
+    #   Compute project subtree, propagating trash_at to subprojects
+    #   Ensure none of the newly trashed descendants were frozen (if so, bail out)
+    #   Remove groups that don't belong from trash
+    #   Add/update groups that do belong in the trash
+
+    temptable = "group_subtree_#{rand(2**64).to_s(10)}"
+    ActiveRecord::Base.connection.exec_query(
+      "create temporary table #{temptable} on commit drop " +
+      "as select * from project_subtree_with_trash_at($1, LEAST($2, $3)::timestamp)",
+      "Group.update_trash.select",
+      [[nil, self.uuid],
+       [nil, TrashedGroup.find_by_group_uuid(self.owner_uuid).andand.trash_at],
+       [nil, self.trash_at]])
+    frozen_descendants = ActiveRecord::Base.connection.exec_query(
+      "select uuid from frozen_groups, #{temptable} where uuid = target_uuid",
+      "Group.update_trash.check_frozen")
+    if frozen_descendants.any?
+      raise ArgumentError.new("cannot trash project containing frozen project #{frozen_descendants[0]["uuid"]}")
+    end
+    ActiveRecord::Base.connection.exec_delete(
+      "delete from trashed_groups where group_uuid in (select target_uuid from #{temptable} where trash_at is NULL)",
+      "Group.update_trash.delete")
+    ActiveRecord::Base.connection.exec_query(
+      "insert into trashed_groups (group_uuid, trash_at) "+
+      "select target_uuid as group_uuid, trash_at from #{temptable} where trash_at is not NULL " +
+      "on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at",
+      "Group.update_trash.insert")
+  end
+
+  def update_frozen
+    return unless saved_change_to_frozen_by_uuid? || saved_change_to_owner_uuid?
+    temptable = "group_subtree_#{rand(2**64).to_s(10)}"
+    ActiveRecord::Base.connection.exec_query(
+      "create temporary table #{temptable} on commit drop as select * from project_subtree_with_is_frozen($1,$2)",
+      "Group.update_frozen.select",
+      [[nil, self.uuid],
+       [nil, !self.frozen_by_uuid.nil?]])
+    if frozen_by_uuid
+      rows = ActiveRecord::Base.connection.exec_query(
+        "select cr.uuid, cr.state from container_requests cr, #{temptable} frozen " +
+        "where cr.owner_uuid = frozen.uuid and frozen.is_frozen " +
+        "and cr.state not in ($1, $2) limit 1",
+        "Group.update_frozen.check_container_requests",
+        [[nil, ContainerRequest::Uncommitted],
+         [nil, ContainerRequest::Final]])
+      if rows.any?
+        raise ArgumentError.new("cannot freeze project containing container request #{rows.first['uuid']} with state = #{rows.first['state']}")
+      end
     end
+    ActiveRecord::Base.connection.exec_delete(
+      "delete from frozen_groups where uuid in (select uuid from #{temptable} where not is_frozen)",
+      "Group.update_frozen.delete")
+    ActiveRecord::Base.connection.exec_query(
+      "insert into frozen_groups (uuid) select uuid from #{temptable} where is_frozen on conflict do nothing",
+      "Group.update_frozen.insert")
   end
 
   def before_ownership_change
@@ -138,12 +209,16 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
     end
   end
 
-  def clear_permissions_and_trash
+  def clear_permissions_trash_frozen
     MaterializedPermission.where(target_uuid: uuid).delete_all
-    ActiveRecord::Base.connection.exec_delete %{
-delete from trashed_groups where group_uuid=$1
-}, "Group.clear_permissions_and_trash", [[nil, self.uuid]]
-
+    ActiveRecord::Base.connection.exec_delete(
+      "delete from trashed_groups where group_uuid=$1",
+      "Group.clear_permissions_trash_frozen",
+      [[nil, self.uuid]])
+    ActiveRecord::Base.connection.exec_delete(
+      "delete from frozen_groups where uuid=$1",
+      "Group.clear_permissions_trash_frozen",
+      [[nil, self.uuid]])
   end
 
   def assign_name
@@ -181,4 +256,19 @@ delete from trashed_groups where group_uuid=$1
       end
     end
   end
+
+  def permission_to_update
+    if !super
+      return false
+    elsif frozen_by_uuid && frozen_by_uuid_was
+      errors.add :uuid, "#{uuid} is frozen and cannot be modified"
+      return false
+    else
+      return true
+    end
+  end
+
+  def self.full_text_searchable_columns
+    super - ["frozen_by_uuid"]
+  end
 end
index 811cd89758d9ef8642be5382c9bac54b4f649134..bbb2378f5c56becac22646212beb343549da5170 100644 (file)
@@ -87,6 +87,7 @@ class User < ArvadosModel
   VAL_FOR_PERM =
     {:read => 1,
      :write => 2,
+     :unfreeze => 3,
      :manage => 3}
 
 
@@ -141,6 +142,23 @@ SELECT 1 FROM #{PERMISSION_VIEW}
                   ).any?
         return false
       end
+
+      if action == :write
+        if FrozenGroup.where(uuid: [target_uuid, target_owner_uuid]).any?
+          # self or parent is frozen
+          return false
+        end
+      elsif action == :unfreeze
+        # "unfreeze" permission means "can write, but only if
+        # explicitly un-freezing at the same time" (see
+        # ArvadosModel#ensure_owner_uuid_is_permitted). If the
+        # permission query above passed the permission level of
+        # :unfreeze (which is the same as :manage), and the parent
+        # isn't also frozen, then un-freeze is allowed.
+        if FrozenGroup.where(uuid: target_owner_uuid).any?
+          return false
+        end
+      end
     end
     true
   end
diff --git a/services/api/db/migrate/20220224203102_add_frozen_by_uuid_to_groups.rb b/services/api/db/migrate/20220224203102_add_frozen_by_uuid_to_groups.rb
new file mode 100644 (file)
index 0000000..d730b25
--- /dev/null
@@ -0,0 +1,9 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class AddFrozenByUuidToGroups < ActiveRecord::Migration[5.2]
+  def change
+    add_column :groups, :frozen_by_uuid, :string
+  end
+end
diff --git a/services/api/db/migrate/20220301155729_frozen_groups.rb b/services/api/db/migrate/20220301155729_frozen_groups.rb
new file mode 100644 (file)
index 0000000..730d051
--- /dev/null
@@ -0,0 +1,39 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require '20200501150153_permission_table_constants'
+
+class FrozenGroups < ActiveRecord::Migration[5.0]
+  def up
+    create_table :frozen_groups, :id => false do |t|
+      t.string :uuid
+    end
+    add_index :frozen_groups, :uuid, :unique => true
+
+    ActiveRecord::Base.connection.execute %{
+create or replace function project_subtree_with_is_frozen (starting_uuid varchar(27), starting_is_frozen boolean)
+returns table (uuid varchar(27), is_frozen boolean)
+STABLE
+language SQL
+as $$
+WITH RECURSIVE
+  project_subtree(uuid, is_frozen) as (
+    values (starting_uuid, starting_is_frozen)
+    union
+    select groups.uuid, project_subtree.is_frozen or groups.frozen_by_uuid is not null
+      from groups join project_subtree on project_subtree.uuid = groups.owner_uuid
+  )
+  select uuid, is_frozen from project_subtree;
+$$;
+}
+
+    # Initialize the table. After this, it is updated incrementally.
+    # See app/models/group.rb#update_frozen_groups
+    refresh_frozen
+  end
+
+  def down
+    drop_table :frozen_groups
+  end
+end
diff --git a/services/api/db/migrate/20220303204419_add_frozen_by_uuid_to_group_search_index.rb b/services/api/db/migrate/20220303204419_add_frozen_by_uuid_to_group_search_index.rb
new file mode 100644 (file)
index 0000000..52fb16b
--- /dev/null
@@ -0,0 +1,17 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class AddFrozenByUuidToGroupSearchIndex < ActiveRecord::Migration[5.0]
+  disable_ddl_transaction!
+
+  def up
+    remove_index :groups, :name => 'groups_search_index'
+    add_index :groups, ["uuid", "owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "name", "group_class", "frozen_by_uuid"], name: 'groups_search_index', algorithm: :concurrently
+  end
+
+  def down
+    remove_index :groups, :name => 'groups_search_index'
+    add_index :groups, ["uuid", "owner_uuid", "modified_by_client_uuid", "modified_by_user_uuid", "name", "group_class"], name: 'groups_search_index', algorithm: :concurrently
+  end
+end
index da9959593c0a28853831d3cc6bd464be28bdbe07..cfe21f7c9ae29307b42fd25236a1f4c195254da0 100644 (file)
@@ -190,6 +190,24 @@ case (edges.edge_id = perm_edge_id)
 $$;
 
 
+--
+-- Name: project_subtree_with_is_frozen(character varying, boolean); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.project_subtree_with_is_frozen(starting_uuid character varying, starting_is_frozen boolean) RETURNS TABLE(uuid character varying, is_frozen boolean)
+    LANGUAGE sql STABLE
+    AS $$
+WITH RECURSIVE
+  project_subtree(uuid, is_frozen) as (
+    values (starting_uuid, starting_is_frozen)
+    union
+    select groups.uuid, project_subtree.is_frozen or groups.frozen_by_uuid is not null
+      from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
+  )
+  select uuid, is_frozen from project_subtree;
+$$;
+
+
 --
 -- Name: project_subtree_with_trash_at(character varying, timestamp without time zone); Type: FUNCTION; Schema: public; Owner: -
 --
@@ -548,6 +566,15 @@ CREATE SEQUENCE public.containers_id_seq
 ALTER SEQUENCE public.containers_id_seq OWNED BY public.containers.id;
 
 
+--
+-- Name: frozen_groups; Type: TABLE; Schema: public; Owner: -
+--
+
+CREATE TABLE public.frozen_groups (
+    uuid character varying
+);
+
+
 --
 -- Name: groups; Type: TABLE; Schema: public; Owner: -
 --
@@ -567,7 +594,8 @@ CREATE TABLE public.groups (
     trash_at timestamp without time zone,
     is_trashed boolean DEFAULT false NOT NULL,
     delete_at timestamp without time zone,
-    properties jsonb DEFAULT '{}'::jsonb
+    properties jsonb DEFAULT '{}'::jsonb,
+    frozen_by_uuid character varying
 );
 
 
@@ -1775,7 +1803,7 @@ CREATE INDEX group_index_on_properties ON public.groups USING gin (properties);
 -- Name: groups_search_index; Type: INDEX; Schema: public; Owner: -
 --
 
-CREATE INDEX groups_search_index ON public.groups USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, name, group_class);
+CREATE INDEX groups_search_index ON public.groups USING btree (uuid, owner_uuid, modified_by_client_uuid, modified_by_user_uuid, name, group_class, frozen_by_uuid);
 
 
 --
@@ -2058,6 +2086,13 @@ CREATE INDEX index_containers_on_secret_mounts_md5 ON public.containers USING bt
 CREATE UNIQUE INDEX index_containers_on_uuid ON public.containers USING btree (uuid);
 
 
+--
+-- Name: index_frozen_groups_on_uuid; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE UNIQUE INDEX index_frozen_groups_on_uuid ON public.frozen_groups USING btree (uuid);
+
+
 --
 -- Name: index_groups_on_created_at; Type: INDEX; Schema: public; Owner: -
 --
@@ -3147,6 +3182,9 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20210126183521'),
 ('20210621204455'),
 ('20210816191509'),
-('20211027154300');
+('20211027154300'),
+('20220224203102'),
+('20220301155729'),
+('20220303204419');
 
 
index 74c15bc2e9381a13ae57021386d9393b35d86543..7ee5039368a282e3fabf8ec2d62f6b7f0d28c943 100644 (file)
@@ -15,8 +15,8 @@
 # update_permissions reference that file instead.
 
 PERMISSION_VIEW = "materialized_permissions"
-
 TRASHED_GROUPS = "trashed_groups"
+FROZEN_GROUPS = "frozen_groups"
 
 # We need to use this parameterized query in a few different places,
 # including as a subquery in a larger query.
@@ -83,3 +83,21 @@ INSERT INTO materialized_permissions
 }, "refresh_permission_view.do"
   end
 end
+
+def refresh_frozen
+  ActiveRecord::Base.transaction do
+    ActiveRecord::Base.connection.execute("LOCK TABLE #{FROZEN_GROUPS}")
+    ActiveRecord::Base.connection.execute("DELETE FROM #{FROZEN_GROUPS}")
+
+    # Compute entire frozen_groups table, starting with top-level
+    # projects (i.e., project groups owned by a user).
+    ActiveRecord::Base.connection.execute(%{
+INSERT INTO #{FROZEN_GROUPS}
+select ps.uuid from groups,
+  lateral project_subtree_with_is_frozen(groups.uuid, groups.frozen_by_uuid is not null) ps
+  where groups.owner_uuid like '_____-tpzed-_______________'
+    and group_class = 'project'
+    and ps.is_frozen
+})
+  end
+end
index 0819c2306717f08cfac634494d1e55a1e6a677d4..fcdce0e600d6572fc69b682cd8b5ef68036d633a 100644 (file)
@@ -920,4 +920,24 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
 
     assert_response 422
   end
+
+  test "include_trash does not return trash inside frozen project" do
+    authorize_with :active
+    trashtime = Time.now - 1.second
+    outerproj = Group.create!(group_class: 'project')
+    innerproj = Group.create!(group_class: 'project', owner_uuid: outerproj.uuid)
+    innercoll = Collection.create!(name: 'inner-not-trashed', owner_uuid: innerproj.uuid)
+    innertrash = Collection.create!(name: 'inner-trashed', owner_uuid: innerproj.uuid, trash_at: trashtime)
+    innertrashproj = Group.create!(group_class: 'project', name: 'inner-trashed-proj', owner_uuid: innerproj.uuid, trash_at: trashtime)
+    outertrash = Collection.create!(name: 'outer-trashed', owner_uuid: outerproj.uuid, trash_at: trashtime)
+    innerproj.update_attributes!(frozen_by_uuid: users(:active).uuid)
+    get :contents, params: {id: outerproj.uuid, include_trash: true, recursive: true}
+    assert_response :success
+    uuids = json_response['items'].collect { |item| item['uuid'] }
+    assert_includes uuids, outertrash.uuid
+    assert_includes uuids, innerproj.uuid
+    assert_includes uuids, innercoll.uuid
+    refute_includes uuids, innertrash.uuid
+    refute_includes uuids, innertrashproj.uuid
+  end
 end
index 10932e116d7adbed60880f4fc84d0a55242039db..7a16962402c7a4609bfea6dddd5703526a4a2cbc 100644 (file)
@@ -313,4 +313,219 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
     assert Link.where(tail_uuid: g3, head_uuid: g4, link_class: "permission", name: "can_manage").any?
     assert !Link.where(link_class: 'permission', name: 'can_manage', tail_uuid: g5, head_uuid: g4).any?
   end
+
+  test "freeze project" do
+    act_as_user users(:active) do
+      Rails.configuration.API.UnfreezeProjectRequiresAdmin = false
+
+      test_cr_attrs = {
+        command: ["echo", "foo"],
+        container_image: links(:docker_image_collection_tag).name,
+        cwd: "/tmp",
+        environment: {},
+        mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}},
+        output_path: "/out",
+        runtime_constraints: {"vcpus" => 1, "ram" => 2},
+        name: "foo",
+        description: "bar",
+      }
+      parent = Group.create!(group_class: 'project', name: 'freeze-test-parent', owner_uuid: users(:active).uuid)
+      proj = Group.create!(group_class: 'project', name: 'freeze-test', owner_uuid: parent.uuid)
+      proj_inner = Group.create!(group_class: 'project', name: 'freeze-test-inner', owner_uuid: proj.uuid)
+      coll = Collection.create!(name: 'freeze-test-collection', manifest_text: '', owner_uuid: proj_inner.uuid)
+
+      # Cannot set frozen_by_uuid to a different user
+      assert_raises do
+        proj.update_attributes!(frozen_by_uuid: users(:spectator).uuid)
+      end
+      proj.reload
+
+      # Cannot set frozen_by_uuid without can_manage permission
+      act_as_system_user do
+        Link.create!(link_class: 'permission', name: 'can_write', tail_uuid: users(:spectator).uuid, head_uuid: proj.uuid)
+      end
+      act_as_user users(:spectator) do
+        # First confirm we have write permission
+        assert Collection.create(name: 'bar', owner_uuid: proj.uuid)
+        assert_raises(ArvadosModel::PermissionDeniedError) do
+          proj.update_attributes!(frozen_by_uuid: users(:spectator).uuid)
+        end
+      end
+      proj.reload
+
+      # Cannot set frozen_by_uuid without description (if so configured)
+      Rails.configuration.API.FreezeProjectRequiresDescription = true
+      err = assert_raises do
+        proj.update_attributes!(frozen_by_uuid: users(:active).uuid)
+      end
+      assert_match /can only be set if description is non-empty/, err.inspect
+      proj.reload
+      err = assert_raises do
+        proj.update_attributes!(frozen_by_uuid: users(:active).uuid, description: '')
+      end
+      assert_match /can only be set if description is non-empty/, err.inspect
+      proj.reload
+
+      # Cannot set frozen_by_uuid without properties (if so configured)
+      Rails.configuration.API.FreezeProjectRequiresProperties['frobity'] = true
+      err = assert_raises do
+        proj.update_attributes!(
+          frozen_by_uuid: users(:active).uuid,
+          description: 'ready to freeze')
+      end
+      assert_match /can only be set if properties\[frobity\] value is non-empty/, err.inspect
+      proj.reload
+
+      # Cannot set frozen_by_uuid while project or its parent is
+      # trashed
+      [parent, proj].each do |trashed|
+        trashed.update_attributes!(trash_at: db_current_time)
+        err = assert_raises do
+          proj.update_attributes!(
+            frozen_by_uuid: users(:active).uuid,
+            description: 'ready to freeze',
+            properties: {'frobity' => 'bar baz'})
+        end
+        assert_match /cannot be set on a trashed project/, err.inspect
+        proj.reload
+        trashed.update_attributes!(trash_at: nil)
+      end
+
+      # Can set frozen_by_uuid if all conditions are met
+      ok = proj.update_attributes(
+        frozen_by_uuid: users(:active).uuid,
+        description: 'ready to freeze',
+        properties: {'frobity' => 'bar baz'})
+      assert ok, proj.errors.messages.inspect
+
+      # Once project is frozen, cannot create new items inside it or
+      # its descendants
+      [proj, proj_inner].each do |frozen|
+        assert_raises do
+          collections(:collection_owned_by_active).update_attributes!(owner_uuid: frozen.uuid)
+        end
+        assert_raises do
+          Collection.create!(owner_uuid: frozen.uuid, name: 'inside-frozen-project')
+        end
+        assert_raises do
+          Group.create!(owner_uuid: frozen.uuid, group_class: 'project', name: 'inside-frozen-project')
+        end
+        cr = ContainerRequest.new(test_cr_attrs.merge(owner_uuid: frozen.uuid))
+        assert_raises ArvadosModel::PermissionDeniedError do
+          cr.save
+        end
+        assert_match /frozen/, cr.errors.inspect
+        # Check the frozen-parent condition is the only reason save failed.
+        cr.owner_uuid = users(:active).uuid
+        assert cr.save
+        cr.destroy
+      end
+
+      # Once project is frozen, cannot change name/contents, move,
+      # trash, or delete the project or anything beneath it
+      [proj, proj_inner, coll].each do |frozen|
+        assert_raises(StandardError, "should reject rename of #{frozen.uuid} (#{frozen.name}) with parent #{frozen.owner_uuid}") do
+          frozen.update_attributes!(name: 'foo2')
+        end
+        frozen.reload
+
+        if frozen.is_a?(Collection)
+          assert_raises(StandardError, "should reject manifest change of #{frozen.uuid}") do
+            frozen.update_attributes!(manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n")
+          end
+        else
+          assert_raises(StandardError, "should reject moving a project into #{frozen.uuid}") do
+            groups(:private).update_attributes!(owner_uuid: frozen.uuid)
+          end
+        end
+        frozen.reload
+
+        assert_raises(StandardError, "should reject moving #{frozen.uuid} to a different parent project") do
+          frozen.update_attributes!(owner_uuid: groups(:private).uuid)
+        end
+        frozen.reload
+        assert_raises(StandardError, "should reject setting trash_at of #{frozen.uuid}") do
+          frozen.update_attributes!(trash_at: db_current_time)
+        end
+        frozen.reload
+        assert_raises(StandardError, "should reject setting delete_at of #{frozen.uuid}") do
+          frozen.update_attributes!(delete_at: db_current_time)
+        end
+        frozen.reload
+        assert_raises(StandardError, "should reject delete of #{frozen.uuid}") do
+          frozen.destroy
+        end
+        frozen.reload
+        if frozen != proj
+          assert_equal [], frozen.writable_by
+        end
+      end
+
+      # User with write permission (but not manage) cannot unfreeze
+      act_as_user users(:spectator) do
+        # First confirm we have write permission on the parent project
+        assert Collection.create(name: 'bar', owner_uuid: parent.uuid)
+        assert_raises(ArvadosModel::PermissionDeniedError) do
+          proj.update_attributes!(frozen_by_uuid: nil)
+        end
+      end
+      proj.reload
+
+      # User with manage permission can unfreeze, then create items
+      # inside it and its children
+      assert proj.update_attributes(frozen_by_uuid: nil)
+      assert Collection.create!(owner_uuid: proj.uuid, name: 'inside-unfrozen-project')
+      assert Collection.create!(owner_uuid: proj_inner.uuid, name: 'inside-inner-unfrozen-project')
+
+      # Re-freeze, and reconfigure so only admins can unfreeze.
+      assert proj.update_attributes(frozen_by_uuid: users(:active).uuid)
+      Rails.configuration.API.UnfreezeProjectRequiresAdmin = true
+
+      # Owner cannot unfreeze, because not admin.
+      err = assert_raises do
+        proj.update_attributes!(frozen_by_uuid: nil)
+      end
+      assert_match /can only be changed by an admin user, once set/, err.inspect
+      proj.reload
+
+      # Cannot trash or delete a frozen project's ancestor
+      assert_raises(StandardError, "should not be able to set trash_at on parent of frozen project") do
+        parent.update_attributes!(trash_at: db_current_time)
+      end
+      parent.reload
+      assert_raises(StandardError, "should not be able to set delete_at on parent of frozen project") do
+        parent.update_attributes!(delete_at: db_current_time)
+      end
+      parent.reload
+      assert_nil parent.frozen_by_uuid
+
+      act_as_user users(:admin) do
+        # Even admin cannot change frozen_by_uuid to someone else's UUID.
+        err = assert_raises do
+          proj.update_attributes!(frozen_by_uuid: users(:project_viewer).uuid)
+        end
+        assert_match /can only be set to the current user's UUID/, err.inspect
+        proj.reload
+
+        # Admin can unfreeze.
+        assert proj.update_attributes(frozen_by_uuid: nil), proj.errors.messages
+      end
+
+      # Cannot freeze a project if it contains container requests in
+      # Committed state (this would cause operations on the relevant
+      # Containers to fail when syncing container request state)
+      creq_uncommitted = ContainerRequest.create!(test_cr_attrs.merge(owner_uuid: proj_inner.uuid))
+      creq_committed = ContainerRequest.create!(test_cr_attrs.merge(owner_uuid: proj_inner.uuid, state: 'Committed'))
+      err = assert_raises do
+        proj.update_attributes!(frozen_by_uuid: users(:active).uuid)
+      end
+      assert_match /container request zzzzz-xvhdp-.* with state = Committed/, err.inspect
+      proj.reload
+
+      # Can freeze once all container requests are in Uncommitted or
+      # Final state
+      creq_committed.update_attributes!(state: ContainerRequest::Final)
+      assert proj.update_attributes(frozen_by_uuid: users(:active).uuid)
+    end
+  end
 end
index 647139d9ec9d38d4d5570a19e658b866bd609544..efc43dfde54f6f9b3274c765465a067226a3de27 100644 (file)
@@ -602,4 +602,17 @@ class PermissionTest < ActiveSupport::TestCase
       end
     end
   end
+
+  # Show query plan for readable_by query. The plan for a test db
+  # might not resemble the plan for a production db, but it doesn't
+  # hurt to show the test db plan in test logs, and the .
+  [false, true].each do |include_trash|
+    test "query plan, include_trash=#{include_trash}" do
+      sql = Collection.readable_by(users(:active), include_trash: include_trash).to_sql
+      sql = "explain analyze #{sql}"
+      STDERR.puts sql
+      q = ActiveRecord::Base.connection.exec_query(sql)
+      q.rows.each do |row| STDERR.puts(row) end
+    end
+  end
 end
index e021b442f1d7a0db09848c4364c0dcca45c7d0f0..e7416947d65d2abd5023f77f1b4de997b71c910d 100755 (executable)
@@ -237,7 +237,7 @@ run() {
         fi
 
         if ! (docker ps -a | grep -E "$ARVBOX_CONTAINER-data$" -q) ; then
-            docker create -v /var/lib/postgresql -v $ARVADOS_CONTAINER_PATH --name $ARVBOX_CONTAINER-data arvados/arvbox-demo /bin/true
+            docker create -v /var/lib/postgresql -v $ARVADOS_CONTAINER_PATH --name $ARVBOX_CONTAINER-data arvados/arvbox-demo$TAG /bin/true
         fi
 
         docker run \
diff --git a/tools/salt-install/config_examples/multi_host/aws/states/shell_sudo_passwordless.sls b/tools/salt-install/config_examples/multi_host/aws/states/shell_sudo_passwordless.sls
new file mode 100644 (file)
index 0000000..dbcc9c9
--- /dev/null
@@ -0,0 +1,27 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+{%- set curr_tpldir = tpldir %}
+{%- set tpldir = 'arvados' %}
+{%- from "arvados/map.jinja" import arvados with context %}
+{%- set tpldir = curr_tpldir %}
+
+extra_shell_sudo_passwordless_sudo_pkg_installed:
+  pkg.installed:
+    - name: sudo
+
+extra_shell_sudo_passwordless_config_file_managed:
+  file.managed:
+    - name: /etc/sudoers.d/arvados_passwordless
+    - makedirs: true
+    - user: root
+    - group: root
+    - mode: '0440'
+    - replace: false
+    - contents: |
+        # This file managed by Salt, do not edit by hand!!
+        # Allow members of group sudo to execute any command without password
+        %sudo ALL=(ALL:ALL) NOPASSWD:ALL
+    - require:
+      - pkg: extra_shell_sudo_passwordless_sudo_pkg_installed
diff --git a/tools/salt-install/config_examples/single_host/single_hostname/states/shell_sudo_passwordless.sls b/tools/salt-install/config_examples/single_host/single_hostname/states/shell_sudo_passwordless.sls
new file mode 100644 (file)
index 0000000..dbcc9c9
--- /dev/null
@@ -0,0 +1,27 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: Apache-2.0
+
+{%- set curr_tpldir = tpldir %}
+{%- set tpldir = 'arvados' %}
+{%- from "arvados/map.jinja" import arvados with context %}
+{%- set tpldir = curr_tpldir %}
+
+extra_shell_sudo_passwordless_sudo_pkg_installed:
+  pkg.installed:
+    - name: sudo
+
+extra_shell_sudo_passwordless_config_file_managed:
+  file.managed:
+    - name: /etc/sudoers.d/arvados_passwordless
+    - makedirs: true
+    - user: root
+    - group: root
+    - mode: '0440'
+    - replace: false
+    - contents: |
+        # This file managed by Salt, do not edit by hand!!
+        # Allow members of group sudo to execute any command without password
+        %sudo ALL=(ALL:ALL) NOPASSWD:ALL
+    - require:
+      - pkg: extra_shell_sudo_passwordless_sudo_pkg_installed
index 0f3c9a14117b964907259d628afbb6de32f1e58d..c4ccfd12693997a3e82dca546fa3d15e8bf0ee54 100755 (executable)
@@ -514,7 +514,7 @@ if [ -d "${F_DIR}"/extra/extra ]; then
     # Same when using self-signed certificates.
     SKIP_SNAKE_OIL="dont_add_snakeoil_certs"
   fi
-  for f in $(ls "${F_DIR}"/extra/extra/*.sls | grep -v ${SKIP_SNAKE_OIL}); do
+  for f in $(ls "${F_DIR}"/extra/extra/*.sls | egrep -v "${SKIP_SNAKE_OIL}|shell_sudo_passwordless"); do
   echo "    - extra.$(basename ${f} | sed 's/.sls$//g')" >> ${S_DIR}/top.sls
   done
   # Use byo or self-signed certificates
@@ -544,6 +544,7 @@ if [ -z "${ROLES}" ]; then
     grep -q "custom_certs"    ${S_DIR}/top.sls || echo "    - extra.custom_certs" >> ${S_DIR}/top.sls
   fi
 
+  echo "    - extra.shell_sudo_passwordless" >> ${S_DIR}/top.sls
   echo "    - postgres" >> ${S_DIR}/top.sls
   echo "    - docker.software" >> ${S_DIR}/top.sls
   echo "    - arvados" >> ${S_DIR}/top.sls
@@ -753,6 +754,7 @@ else
       ;;
       "shell")
         # States
+        echo "    - extra.shell_sudo_passwordless" >> ${S_DIR}/top.sls
         grep -q "docker" ${S_DIR}/top.sls       || echo "    - docker.software" >> ${S_DIR}/top.sls
         grep -q "arvados.${R}" ${S_DIR}/top.sls || echo "    - arvados.${R}" >> ${S_DIR}/top.sls
         # Pillars