17774: Ignore addition of zero/default values to mounts when frozen.

Avoids errors when a client reads the CR and includes the resulting
mounts field (to which controller will have added zero/default values)
in a subsequent update request.

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

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 837f2cdc7010eaacc1182f868e9fb01084367ddb..e712acc6e9c37f85e5d9e40e7f5ec1990e0b947e 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -394,6 +394,20 @@ class ContainerRequest < ArvadosModel
     if self.new_record? || self.state_was == Uncommitted
       # Allow create-and-commit in a single operation.
       permitted.push(*AttrsPermittedBeforeCommit)
+    elsif mounts_changed? && mounts_was.keys == mounts.keys
+      # Ignore the updated mounts if the only changes are default/zero
+      # values as added by controller, see 17774
+      only_defaults = true
+      mounts.each do |path, mount|
+        (mount.to_a - mounts_was[path].to_a).each do |k, v|
+          if !["", false, nil].index(v)
+            only_defaults = false
+          end
+        end
+      end
+      if only_defaults
+        clear_attribute_change("mounts")
+      end
     end
 
     case self.state

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index b2dde7995606d71680b062b5e1717a07114557bb..d119377ad129ad8d3ab099deb921058eeb02602d 100644 (file)
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -1071,6 +1071,18 @@ class ContainerRequestTest < ActiveSupport::TestCase
    ['Committed', false, {container_count: 2}],
    ['Committed', false, {container_count: 0}],
    ['Committed', false, {container_count: nil}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}}}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp"}}}],
+   # addition of default values for mounts, as happens in a round-trip
+   # through controller, should be ignored when mounts is not allowed
+   # to change
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "exclude_from_output": false}}}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "repository_name": ""}}}],
+   ['Committed', true, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "content": nil}}}],
+   ['Committed', false, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "content": {}}}}],
+   ['Committed', false, {priority: 0, mounts: {"/out" => {"capacity" => 1000000, "kind" => "tmp", "repository_name": "foo"}}}],
+   ['Committed', false, {priority: 0, mounts: {"/out" => {"kind" => "tmp", "capacity" => 1234567}}}],
+   ['Committed', false, {priority: 0, mounts: {}}],
    ['Final', false, {state: ContainerRequest::Committed, name: "foobar"}],
    ['Final', false, {name: "foobar", priority: 123}],
    ['Final', false, {name: "foobar", output_uuid: "zzzzz-4zz18-znfnqtbbv4spc3w"}],