[ARVADOS] updated: 1.3.0-1040-g502005c30

Git user git at public.curoverse.com
Fri Jun 7 14:41:43 UTC 2019


Summary of changes:
 doc/admin/upgrading.html.textile.liquid            |  43 ++++++--
 services/api/app/models/arvados_model.rb           |  53 ----------
 services/api/lib/tasks/symbols.rake                | 109 +++++++++++++++++++++
 services/api/test/fixtures/links.yml               |   9 ++
 ...eep_munge_test.rb => container_request_test.rb} |  28 ++----
 services/api/test/unit/arvados_model_test.rb       |  11 ---
 6 files changed, 161 insertions(+), 92 deletions(-)
 create mode 100644 services/api/lib/tasks/symbols.rake
 copy services/api/test/integration/{noop_deep_munge_test.rb => container_request_test.rb} (62%)

       via  502005c3002c9ee9f07b36ac4a5aa370aa056c50 (commit)
       via  b15cc4afed604255dfee435aa8c1663c46895860 (commit)
      from  b93dc244f39ea9b8a4552804cd24d251658531e3 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 502005c3002c9ee9f07b36ac4a5aa370aa056c50
Merge: b93dc244f b15cc4afe
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Fri Jun 7 10:41:23 2019 -0400

    Merge branch '15311-api-no-colons' refs #15311
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>


commit b15cc4afed604255dfee435aa8c1663c46895860
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Thu Jun 6 11:36:34 2019 -0400

    15311: Remove stringify-symbol behavior on database load
    
    * Add test case demonstrating bug
    
    * Remove convert_serialized_symbols_to_strings hook
    
    * Add "rake symbols:check" to warn you if there are any serialized
    columns that may have legacy symbols.
    
    * Add "rake symbols:stringify" to warn you if there are any serialized
    columns that may have legacy symbols.
    
    * Add release notes for checking and stringifying symbols.
    
    * Add table of contents to upgrading doc page.
    
    * Add note about the has_symbol_keys_in_database_somehow fixture.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/doc/admin/upgrading.html.textile.liquid b/doc/admin/upgrading.html.textile.liquid
index 79e112857..b25dc1091 100644
--- a/doc/admin/upgrading.html.textile.liquid
+++ b/doc/admin/upgrading.html.textile.liquid
@@ -30,7 +30,24 @@ Note to developers: Add new items at the top. Include the date, issue number, co
 TODO: extract this information based on git commit messages and generate changelogs / release notes automatically.
 {% endcomment %}
 
-h3. v1.4.0 (2019-06-05)
+table(table table-bordered table-condensed).
+|_. development|"master":#master|\3.|
+|_. latest stable|"v1.4.0":#v1_4_0|\3.|
+|_\5. past stable|
+|"v1.3.3":#v1_3_3|"v1.3.0":#v1_3_0|\3.|
+|"v1.2.1":#v1_2_1|"v1.2.0":#v1_2_0|\3.|
+|"v1.1.4":#v1_1_4|"v1.1.3":#v1_1_3|"v1.1.2":#v1_1_2|"v1.1.1":#v1_1_1|"v1.1.0":#v1_1_0|
+|\5. "older":#older|
+
+h3(#master). development master (as of 2019-06-07)
+
+h4. No longer stripping ':' from strings in serialized database columns
+
+ (bug #15311) Strings read from serialized columns in the database with a leading ':' would have the ':' stripped after loading the record.  This behavior existed due to legacy serialization behavior which stored Ruby symbols with a leading ':'.  Unfortunately this corrupted fields where the leading ":" was intentional.  This behavior has been removed.
+
+You can test if any records in your database are affected by going to the API server directory and running @bundle exec rake symbols:check at .  This will report which records contain fields with a leading ':' that would previously have been stripped.  If there are records to be updated, you can update the database using @bundle exec rake symbols:stringify at .
+
+h3(#v1_4_0). v1.4.0 (2019-06-05)
 
 h4. Populating the new file_count and file_size_total columns on the collections table
 
@@ -135,7 +152,13 @@ h4. New configuration
 
 Arvados is migrating to a centralized configuration file for all components.  During the migration, legacy configuration files will continue to be loaded.  See "Migrating Configuration":config-migration.html for details.
 
-h3. v1.3.0 (2018-12-05)
+h3(#v1_3_3). v1.3.3 (2019-05-14)
+
+This release corrects a potential data loss issue, if you are running Arvados 1.3.0 or 1.3.1 we strongly recommended disabling @keep-balance@ until you can upgrade to 1.3.3 or 1.4.0. With keep-balance disabled, there is no chance of data loss.
+
+We've put together a "wiki page":https://dev.arvados.org/projects/arvados/wiki/Recovering_lost_data which outlines how to recover blocks which have been put in the trash, but not yet deleted, as well as how to identify any collections which have missing blocks so that they can be regenerated. The keep-balance component has been enhanced to provide a list of missing blocks and affected collections and we've provided a "utility script":https://github.com/curoverse/arvados/blob/master/tools/keep-xref/keep-xref.py  which can be used to identify the workflows that generated those collections and who ran those workflows, so that they can be rerun.
+
+h3(#v1_3_0). v1.3.0 (2018-12-05)
 
 This release includes several database migrations, which will be executed automatically as part of the API server upgrade. On large Arvados installations, these migrations will take a while. We've seen the upgrade take 30 minutes or more on installations with a lot of collections.
 
@@ -143,11 +166,11 @@ The @arvados-controller@ component now requires the /etc/arvados/config.yml file
 
 Support for the deprecated "jobs" API is broken in this release.  Users who rely on it should not upgrade.  This will be fixed in an upcoming 1.3.1 patch release, however users are "encouraged to migrate":upgrade-crunch2.html as support for the "jobs" API will be dropped in an upcoming release.  Users who are already using the "containers" API are not affected.
 
-h3. v1.2.1 (2018-11-26)
+h3(#v1_2_1). v1.2.1 (2018-11-26)
 
 There are no special upgrade notes for this release.
 
-h3. v1.2.0 (2018-09-05)
+h3(#v1_2_0). v1.2.0 (2018-09-05)
 
 h4. Regenerate Postgres table statistics
 
@@ -177,7 +200,7 @@ To add the Arvados Controller to your system please refer to the "installation i
 
 Verify your setup by confirming that API calls appear in the controller's logs (_e.g._, @journalctl -fu arvados-controller@) while loading a workbench page.
 
-h3. v1.1.4 (2018-04-10)
+h3(#v1_1_4). v1.1.4 (2018-04-10)
 
 h4. arvados-cwl-runner regressions (2018-04-05)
 
@@ -304,11 +327,11 @@ baseCommand: echo
 
 This bug has been fixed in Arvados release v1.2.0.
 
-h3. v1.1.3 (2018-02-08)
+h3(#v1_1_3). v1.1.3 (2018-02-08)
 
 There are no special upgrade notes for this release.
 
-h3. v1.1.2 (2017-12-22)
+h3(#v1_1_2). v1.1.2 (2017-12-22)
 
 h4. The minimum version for Postgres is now 9.4 (2017-12-08)
 
@@ -322,11 +345,11 @@ As part of story "#11908":https://dev.arvados.org/issues/11908, commit "8f987a92
 *# Install the @rh-postgresql94@ backport package from either Software Collections: http://doc.arvados.org/install/install-postgresql.html or the Postgres developers: https://www.postgresql.org/download/linux/redhat/
 *# Restore from the backup using @psql@
 
-h3. v1.1.1 (2017-11-30)
+h3(#v1_1_1). v1.1.1 (2017-11-30)
 
 There are no special upgrade notes for this release.
 
-h3. v1.1.0 (2017-10-24)
+h3(#v1_1_0). v1.1.0 (2017-10-24)
 
 h4. The minimum version for Postgres is now 9.3 (2017-09-25)
 
@@ -340,7 +363,7 @@ As part of story "#12032":https://dev.arvados.org/issues/12032, commit "68bdf4cb
 *# Install the @rh-postgresql94@ backport package from either Software Collections: http://doc.arvados.org/install/install-postgresql.html or the Postgres developers: https://www.postgresql.org/download/linux/redhat/
 *# Restore from the backup using @psql@
 
-h3. Older versions
+h3(#older). Older versions
 
 h4. Upgrade slower than usual (2017-06-30)
 
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 339bc9e23..efef7b812 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -27,7 +27,6 @@ class ArvadosModel < ApplicationRecord
   after_create :log_create
   after_update :log_update
   after_destroy :log_destroy
-  after_find :convert_serialized_symbols_to_strings
   before_validation :normalize_collection_uuids
   before_validation :set_default_owner
   validate :ensure_valid_uuids
@@ -602,41 +601,6 @@ class ArvadosModel < ApplicationRecord
     false
   end
 
-  def self.has_symbols? x
-    if x.is_a? Hash
-      x.each do |k,v|
-        return true if has_symbols?(k) or has_symbols?(v)
-      end
-    elsif x.is_a? Array
-      x.each do |k|
-        return true if has_symbols?(k)
-      end
-    elsif x.is_a? Symbol
-      return true
-    elsif x.is_a? String
-      return true if x.start_with?(':') && !x.start_with?('::')
-    end
-    false
-  end
-
-  def self.recursive_stringify x
-    if x.is_a? Hash
-      Hash[x.collect do |k,v|
-             [recursive_stringify(k), recursive_stringify(v)]
-           end]
-    elsif x.is_a? Array
-      x.collect do |k|
-        recursive_stringify k
-      end
-    elsif x.is_a? Symbol
-      x.to_s
-    elsif x.is_a? String and x.start_with?(':') and !x.start_with?('::')
-      x[1..-1]
-    else
-      x
-    end
-  end
-
   def self.where_serialized(colname, value, md5: false)
     colsql = colname.to_s
     if md5
@@ -677,23 +641,6 @@ class ArvadosModel < ApplicationRecord
     self.class.serialized_attributes
   end
 
-  def convert_serialized_symbols_to_strings
-    # ensure_serialized_attribute_type should prevent symbols from
-    # getting into the database in the first place. If someone managed
-    # to get them into the database (perhaps using an older version)
-    # we'll convert symbols to strings when loading from the
-    # database. (Otherwise, loading and saving an object with existing
-    # symbols in a serialized field will crash.)
-    jsonb_cols = self.class.columns.select{|c| c.type == :jsonb}.collect{|j| j.name}
-    (jsonb_cols + self.class.serialized_attributes.keys).uniq.each do |colname|
-      if self.class.has_symbols? attributes[colname]
-        attributes[colname] = self.class.recursive_stringify attributes[colname]
-        send(colname + '=',
-             self.class.recursive_stringify(attributes[colname]))
-      end
-    end
-  end
-
   def foreign_key_attributes
     attributes.keys.select { |a| a.match(/_uuid$/) }
   end
diff --git a/services/api/lib/tasks/symbols.rake b/services/api/lib/tasks/symbols.rake
new file mode 100644
index 000000000..a2e6df8b5
--- /dev/null
+++ b/services/api/lib/tasks/symbols.rake
@@ -0,0 +1,109 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'current_api_client'
+
+include CurrentApiClient
+
+def has_symbols? x
+  if x.is_a? Hash
+    x.each do |k,v|
+      return true if has_symbols?(k) or has_symbols?(v)
+    end
+  elsif x.is_a? Array
+    x.each do |k|
+      return true if has_symbols?(k)
+    end
+  elsif x.is_a? Symbol
+    return true
+  elsif x.is_a? String
+    return true if x.start_with?(':') && !x.start_with?('::')
+  end
+  false
+end
+
+def check_for_serialized_symbols rec
+  jsonb_cols = rec.class.columns.select{|c| c.type == :jsonb}.collect{|j| j.name}
+  (jsonb_cols + rec.class.serialized_attributes.keys).uniq.each do |colname|
+    if has_symbols? rec.attributes[colname]
+      st = recursive_stringify rec.attributes[colname]
+      puts "Found value potentially containing Ruby symbols in #{colname} attribute of #{rec.uuid}, current value is\n#{rec.attributes[colname].to_s[0..1024]}\nrake symbols:stringify will update it to:\n#{st.to_s[0..1024]}\n\n"
+    end
+  end
+end
+
+def recursive_stringify x
+  if x.is_a? Hash
+    Hash[x.collect do |k,v|
+           [recursive_stringify(k), recursive_stringify(v)]
+         end]
+  elsif x.is_a? Array
+    x.collect do |k|
+      recursive_stringify k
+    end
+  elsif x.is_a? Symbol
+    x.to_s
+  elsif x.is_a? String and x.start_with?(':') and !x.start_with?('::')
+    x[1..-1]
+  else
+    x
+  end
+end
+
+def stringify_serialized_symbols rec
+  # ensure_serialized_attribute_type should prevent symbols from
+  # getting into the database in the first place. If someone managed
+  # to get them into the database (perhaps using an older version)
+  # we'll convert symbols to strings when loading from the
+  # database. (Otherwise, loading and saving an object with existing
+  # symbols in a serialized field will crash.)
+  jsonb_cols = rec.class.columns.select{|c| c.type == :jsonb}.collect{|j| j.name}
+  (jsonb_cols + rec.class.serialized_attributes.keys).uniq.each do |colname|
+    if has_symbols? rec.attributes[colname]
+      begin
+        st = recursive_stringify rec.attributes[colname]
+        puts "Updating #{colname} attribute of #{rec.uuid} from\n#{rec.attributes[colname].to_s[0..1024]}\nto\n#{st.to_s[0..1024]}\n\n"
+        rec.write_attribute(colname, st)
+        rec.save!
+      rescue => e
+        puts "Failed to update #{rec.uuid}: #{e}"
+      end
+    end
+  end
+end
+
+namespace :symbols do
+  desc 'Warn about serialized values starting with ":" that may be symbols'
+  task check: :environment do
+    [ApiClientAuthorization, ApiClient,
+     AuthorizedKey, Collection,
+     Container, ContainerRequest, Group,
+     Human, Job, JobTask, KeepDisk, KeepService, Link,
+     Node, PipelineInstance, PipelineTemplate,
+     Repository, Specimen, Trait, User, VirtualMachine,
+     Workflow].each do |klass|
+      act_as_system_user do
+        klass.all.each do |c|
+          check_for_serialized_symbols c
+        end
+      end
+    end
+  end
+
+  task stringify: :environment do
+    [ApiClientAuthorization, ApiClient,
+     AuthorizedKey, Collection,
+     Container, ContainerRequest, Group,
+     Human, Job, JobTask, KeepDisk, KeepService, Link,
+     Node, PipelineInstance, PipelineTemplate,
+     Repository, Specimen, Trait, User, VirtualMachine,
+     Workflow].each do |klass|
+      act_as_system_user do
+        klass.all.each do |c|
+          stringify_serialized_symbols c
+        end
+      end
+    end
+  end
+end
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index e66baceb2..2f6643337 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -507,6 +507,15 @@ multilevel_collection_1_readable_by_active:
   head_uuid: zzzzz-4zz18-pyw8yp9g3pr7irn
   properties: {}
 
+#
+# This fixture was used in the test "Stringify symbols coming from
+# serialized attribute in database" which tested the hook
+# "convert_serialized_symbols_to_strings".  That hook (and the
+# corresponding test) was removed in #15311.  This fixture remains to
+# facilitate manual testing of the "rake symbols:check" and "rake
+# symbols:stringify" tasks that we added to assist with database
+# fixup.
+#
 has_symbol_keys_in_database_somehow:
   uuid: zzzzz-o0j2j-enl1wg58310loc6
   owner_uuid: zzzzz-tpzed-000000000000000
diff --git a/services/api/test/integration/container_request_test.rb b/services/api/test/integration/container_request_test.rb
new file mode 100644
index 000000000..26cc081a6
--- /dev/null
+++ b/services/api/test/integration/container_request_test.rb
@@ -0,0 +1,39 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'test_helper'
+
+class ContainerRequestIntegrationTest < ActionDispatch::IntegrationTest
+
+  test "test colon in input" do
+    # Tests for bug #15311 where strings with leading colons get
+    # corrupted when the leading ":" is stripped.
+    val = {"itemSeparator" => ":"}
+    post "/arvados/v1/container_requests",
+      params: {
+        :container_request => {
+          :name => "workflow",
+          :state => "Committed",
+          :command => ["echo"],
+          :container_image => "fa3c1a9cb6783f85f2ecda037e07b8c3+167",
+          :output_path => "/",
+          :priority => 1,
+          :runtime_constraints => {"vcpus" => 1, "ram" => 1},
+          :mounts => {
+            :foo => {
+              :kind => "json",
+              :content => JSON.parse(SafeJSON.dump(val)),
+            }
+          }
+        }
+      }.to_json,
+      headers: {
+        'HTTP_AUTHORIZATION' => "OAuth2 #{api_client_authorizations(:active).api_token}",
+        'CONTENT_TYPE' => 'application/json'
+      }
+    assert_response :success
+    assert_equal "arvados#containerRequest", json_response['kind']
+    assert_equal val, json_response['mounts']['foo']['content']
+  end
+end
diff --git a/services/api/test/unit/arvados_model_test.rb b/services/api/test/unit/arvados_model_test.rb
index 0fcdad704..7d9da1e56 100644
--- a/services/api/test/unit/arvados_model_test.rb
+++ b/services/api/test/unit/arvados_model_test.rb
@@ -82,17 +82,6 @@ class ArvadosModelTest < ActiveSupport::TestCase
     end
   end
 
-  test "Stringify symbols coming from serialized attribute in database" do
-    set_user_from_auth :admin_trustedclient
-    fixed = Link.find_by_uuid(links(:has_symbol_keys_in_database_somehow).uuid)
-    assert_equal(["baz", "foo"], fixed.properties.keys.sort,
-                 "Hash symbol keys from DB did not get stringified.")
-    assert_equal(['waz', 'waz', 'waz', 1, nil, false, true],
-                 fixed.properties['baz'],
-                 "Array symbol values from DB did not get stringified.")
-    assert_equal true, fixed.save, "Failed to save fixed model back to db."
-  end
-
   test "No HashWithIndifferentAccess in database" do
     set_user_from_auth :admin_trustedclient
     link = Link.create!(link_class: 'test',

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list