From: Peter Amstutz Date: Tue, 23 Jul 2019 18:30:06 +0000 (-0400) Subject: 15467: Migrate lists to hashes X-Git-Tag: 2.0.0~241^2~5 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/f771c6733372f5fe91be11f22fc74cfb6274c8ee 15467: Migrate lists to hashes Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb index 649e0f5d09..292cf34ccb 100644 --- a/services/api/app/models/collection.rb +++ b/services/api/app/models/collection.rb @@ -489,17 +489,13 @@ class Collection < ArvadosModel [c.portable_data_hash, c] }] - puts "mg #{migrated_collections} #{collections}" - collections.map { |c| # Check if the listed image is compatible first, if not, then try the # migration link. manifest = Keep::Manifest.new(c.manifest_text) - puts "m1 #{manifest.exact_file_count?(1)} #{manifest.files[0][1]} #{pattern}" if manifest.exact_file_count?(1) and manifest.files[0][1] =~ pattern c elsif m = migrated_collections[migrations[c.portable_data_hash]] - puts "m2 #{manifest.exact_file_count?(1)} #{manifest.files[0][1]} #{pattern}" manifest = Keep::Manifest.new(m.manifest_text) if manifest.exact_file_count?(1) and manifest.files[0][1] =~ pattern m diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb index 575c5f717f..39d50cbbb3 100644 --- a/services/api/config/arvados_config.rb +++ b/services/api/config/arvados_config.rb @@ -75,8 +75,8 @@ $arvados_config = $arvados_config_global.deep_dup def arrayToHash cfg, k, v val = {} - v.each do |k| - val[k.to_s] = {} + v.each do |entry| + val[entry.to_s] = {} end ConfigLoader.set_cfg cfg, k, val end @@ -86,7 +86,7 @@ arvcfg = ConfigLoader.new arvcfg.declare_config "ClusterID", NonemptyString, :uuid_prefix arvcfg.declare_config "ManagementToken", String, :ManagementToken arvcfg.declare_config "Git.Repositories", String, :git_repositories_dir -arvcfg.declare_config "API.DisabledAPIs", Hash, :disable_api_methods, method(:arrayToHash) +arvcfg.declare_config "API.DisabledAPIs", Hash, :disable_api_methods, ->(cfg, k, v) { arrayToHash cfg, "API.DisabledAPIs", v } arvcfg.declare_config "API.MaxRequestSize", Integer, :max_request_size arvcfg.declare_config "API.MaxIndexDatabaseRead", Integer, :max_index_database_read arvcfg.declare_config "API.MaxItemsPerResponse", Integer, :max_items_per_response @@ -95,7 +95,7 @@ arvcfg.declare_config "API.RailsSessionSecretToken", NonemptyString, :secret_tok arvcfg.declare_config "Users.AutoSetupNewUsers", Boolean, :auto_setup_new_users arvcfg.declare_config "Users.AutoSetupNewUsersWithVmUUID", String, :auto_setup_new_users_with_vm_uuid arvcfg.declare_config "Users.AutoSetupNewUsersWithRepository", Boolean, :auto_setup_new_users_with_repository -arvcfg.declare_config "Users.AutoSetupUsernameBlacklist", Hash, :auto_setup_name_blacklist, method(:arrayToHash) +arvcfg.declare_config "Users.AutoSetupUsernameBlacklist", Hash, :auto_setup_name_blacklist, ->(cfg, k, v) { arrayToHash cfg, "Users.AutoSetupUsernameBlacklist", v } arvcfg.declare_config "Users.NewUsersAreActive", Boolean, :new_users_are_active arvcfg.declare_config "Users.AutoAdminUserWithEmail", String, :auto_admin_user arvcfg.declare_config "Users.AutoAdminFirstUser", Boolean, :auto_admin_first_user @@ -103,7 +103,7 @@ arvcfg.declare_config "Users.UserProfileNotificationAddress", String, :user_prof arvcfg.declare_config "Users.AdminNotifierEmailFrom", String, :admin_notifier_email_from arvcfg.declare_config "Users.EmailSubjectPrefix", String, :email_subject_prefix arvcfg.declare_config "Users.UserNotifierEmailFrom", String, :user_notifier_email_from -arvcfg.declare_config "Users.NewUserNotificationRecipients", Hash, :new_user_notification_recipients, method(:arrayToHash) +arvcfg.declare_config "Users.NewUserNotificationRecipients", Hash, :new_user_notification_recipients, ->(cfg, k, v) { arrayToHash cfg, "Users.NewUserNotificationRecipients", v } arvcfg.declare_config "Users.NewInactiveUserNotificationRecipients", Hash, :new_inactive_user_notification_recipients, method(:arrayToHash) arvcfg.declare_config "Login.ProviderAppSecret", NonemptyString, :sso_app_secret arvcfg.declare_config "Login.ProviderAppID", NonemptyString, :sso_app_id @@ -111,7 +111,7 @@ arvcfg.declare_config "TLS.Insecure", Boolean, :sso_insecure arvcfg.declare_config "Services.SSO.ExternalURL", NonemptyString, :sso_provider_url arvcfg.declare_config "AuditLogs.MaxAge", ActiveSupport::Duration, :max_audit_log_age arvcfg.declare_config "AuditLogs.MaxDeleteBatch", Integer, :max_audit_log_delete_batch -arvcfg.declare_config "AuditLogs.UnloggedAttributes", Hash, :unlogged_attributes, method(:arrayToHash) +arvcfg.declare_config "AuditLogs.UnloggedAttributes", Hash, :unlogged_attributes, ->(cfg, k, v) { arrayToHash cfg, "AuditLogs.UnloggedAttributes", v } arvcfg.declare_config "SystemLogs.MaxRequestLogParamsSize", Integer, :max_request_log_params_size arvcfg.declare_config "Collections.DefaultReplication", Integer, :default_collection_replication arvcfg.declare_config "Collections.DefaultTrashLifetime", ActiveSupport::Duration, :default_trash_lifetime @@ -121,7 +121,7 @@ arvcfg.declare_config "Collections.TrashSweepInterval", ActiveSupport::Duration, arvcfg.declare_config "Collections.BlobSigningKey", NonemptyString, :blob_signing_key arvcfg.declare_config "Collections.BlobSigningTTL", ActiveSupport::Duration, :blob_signature_ttl arvcfg.declare_config "Collections.BlobSigning", Boolean, :permit_create_collection_with_unsigned_manifest, ->(cfg, k, v) { ConfigLoader.set_cfg cfg, "Collections.BlobSigning", !v } -arvcfg.declare_config "Containers.SupportedDockerImageFormats", Hash, :docker_image_formats, method(:arrayToHash) +arvcfg.declare_config "Containers.SupportedDockerImageFormats", Hash, :docker_image_formats, ->(cfg, k, v) { arrayToHash cfg, "Containers.SupportedDockerImageFormats", v } arvcfg.declare_config "Containers.LogReuseDecisions", Boolean, :log_reuse_decisions arvcfg.declare_config "Containers.DefaultKeepCacheRAM", Integer, :container_default_keep_cache_ram arvcfg.declare_config "Containers.MaxDispatchAttempts", Integer, :max_container_dispatch_attempts @@ -143,7 +143,7 @@ arvcfg.declare_config "Containers.SLURM.Managed.DNSServerConfTemplate", Pathname arvcfg.declare_config "Containers.SLURM.Managed.DNSServerReloadCommand", String, :dns_server_reload_command arvcfg.declare_config "Containers.SLURM.Managed.DNSServerUpdateCommand", String, :dns_server_update_command arvcfg.declare_config "Containers.SLURM.Managed.ComputeNodeDomain", String, :compute_node_domain -arvcfg.declare_config "Containers.SLURM.Managed.ComputeNodeNameservers", Hash, :compute_node_nameservers, method(:arrayToHash) +arvcfg.declare_config "Containers.SLURM.Managed.ComputeNodeNameservers", Hash, :compute_node_nameservers, ->(cfg, k, v) { arrayToHash cfg, "Containers.SLURM.Managed.ComputeNodeNameservers", v } arvcfg.declare_config "Containers.SLURM.Managed.AssignNodeHostname", String, :assign_node_hostname arvcfg.declare_config "Containers.JobsAPI.Enable", String, :enable_legacy_jobs_api, ->(cfg, k, v) { ConfigLoader.set_cfg cfg, "Containers.JobsAPI.Enable", v.to_s } arvcfg.declare_config "Containers.JobsAPI.CrunchJobWrapper", String, :crunch_job_wrapper diff --git a/services/api/lib/enable_jobs_api.rb b/services/api/lib/enable_jobs_api.rb index c909ae9227..a4fdc5a1e7 100644 --- a/services/api/lib/enable_jobs_api.rb +++ b/services/api/lib/enable_jobs_api.rb @@ -2,38 +2,37 @@ # # SPDX-License-Identifier: AGPL-3.0 -Disable_jobs_api_method_list = ["jobs.create", - "pipeline_instances.create", - "pipeline_templates.create", - "jobs.get", - "pipeline_instances.get", - "pipeline_templates.get", - "jobs.list", - "pipeline_instances.list", - "pipeline_templates.list", - "jobs.index", - "pipeline_instances.index", - "pipeline_templates.index", - "jobs.update", - "pipeline_instances.update", - "pipeline_templates.update", - "jobs.queue", - "jobs.queue_size", - "job_tasks.create", - "job_tasks.get", - "job_tasks.list", - "job_tasks.index", - "job_tasks.update", - "jobs.show", - "pipeline_instances.show", - "pipeline_templates.show", - "jobs.show", - "job_tasks.show"] +Disable_jobs_api_method_list = {"jobs.create"=>{}, + "pipeline_instances.create"=>{}, + "pipeline_templates.create"=>{}, + "jobs.get"=>{}, + "pipeline_instances.get"=>{}, + "pipeline_templates.get"=>{}, + "jobs.list"=>{}, + "pipeline_instances.list"=>{}, + "pipeline_templates.list"=>{}, + "jobs.index"=>{}, + "pipeline_instances.index"=>{}, + "pipeline_templates.index"=>{}, + "jobs.update"=>{}, + "pipeline_instances.update"=>{}, + "pipeline_templates.update"=>{}, + "jobs.queue"=>{}, + "jobs.queue_size"=>{}, + "job_tasks.create"=>{}, + "job_tasks.get"=>{}, + "job_tasks.list"=>{}, + "job_tasks.index"=>{}, + "job_tasks.update"=>{}, + "jobs.show"=>{}, + "pipeline_instances.show"=>{}, + "pipeline_templates.show"=>{}, + "job_tasks.show"=>{}} def check_enable_legacy_jobs_api if Rails.configuration.Containers.JobsAPI.Enable == "false" || (Rails.configuration.Containers.JobsAPI.Enable == "auto" && Job.count == 0) - Rails.configuration.API.DisabledAPIs += Disable_jobs_api_method_list + Rails.configuration.API.DisabledAPIs.merge! Disable_jobs_api_method_list end end diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb index 4618305b32..30ab89c7e2 100644 --- a/services/api/test/functional/arvados/v1/groups_controller_test.rb +++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb @@ -431,7 +431,7 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase end test 'get contents with jobs and pipeline instances disabled' do - Rails.configuration.API.DisabledAPIs = ['jobs.index', 'pipeline_instances.index'] + Rails.configuration.API.DisabledAPIs = {'jobs.index'=>{}, 'pipeline_instances.index'=>{}} authorize_with :active get :contents, params: { diff --git a/services/api/test/functional/arvados/v1/jobs_controller_test.rb b/services/api/test/functional/arvados/v1/jobs_controller_test.rb index b3e10bf4a4..3803a0dc45 100644 --- a/services/api/test/functional/arvados/v1/jobs_controller_test.rb +++ b/services/api/test/functional/arvados/v1/jobs_controller_test.rb @@ -480,8 +480,8 @@ class Arvados::V1::JobsControllerTest < ActionController::TestCase end test 'jobs.create disabled in config' do - Rails.configuration.API.DisabledAPIs = ["jobs.create", - "pipeline_instances.create"] + Rails.configuration.API.DisabledAPIs = {"jobs.create"=>{}, + "pipeline_instances.create"=>{}} authorize_with :active post :create, params: { job: { diff --git a/services/api/test/functional/arvados/v1/schema_controller_test.rb b/services/api/test/functional/arvados/v1/schema_controller_test.rb index 53421a4cbc..3dd343b13c 100644 --- a/services/api/test/functional/arvados/v1/schema_controller_test.rb +++ b/services/api/test/functional/arvados/v1/schema_controller_test.rb @@ -66,7 +66,7 @@ class Arvados::V1::SchemaControllerTest < ActionController::TestCase test "non-empty disable_api_methods" do Rails.configuration.API.DisabledAPIs = - ['jobs.create', 'pipeline_instances.create', 'pipeline_templates.create'] + {'jobs.create'=>{}, 'pipeline_instances.create'=>{}, 'pipeline_templates.create'=>{}} get :index assert_response :success discovery_doc = JSON.parse(@response.body) diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb index 33985ac9ec..764aac3e47 100644 --- a/services/api/test/unit/job_test.rb +++ b/services/api/test/unit/job_test.rb @@ -650,7 +650,7 @@ class JobTest < ActiveSupport::TestCase test 'enable legacy api configuration option = true' do Rails.configuration.Containers.JobsAPI.Enable = "true" check_enable_legacy_jobs_api - assert_equal [], Rails.configuration.API.DisabledAPIs + assert_equal({}, Rails.configuration.API.DisabledAPIs) end test 'enable legacy api configuration option = false' do @@ -663,7 +663,7 @@ class JobTest < ActiveSupport::TestCase Rails.configuration.Containers.JobsAPI.Enable = "auto" assert Job.count > 0 check_enable_legacy_jobs_api - assert_equal [], Rails.configuration.API.DisabledAPIs + assert_equal({}, Rails.configuration.API.DisabledAPIs) end test 'enable legacy api configuration option = auto, no jobs' do @@ -672,7 +672,7 @@ class JobTest < ActiveSupport::TestCase Job.destroy_all end assert_equal 0, Job.count - assert_equal [], Rails.configuration.API.DisabledAPIs + assert_equal({}, Rails.configuration.API.DisabledAPIs) check_enable_legacy_jobs_api assert_equal Disable_jobs_api_method_list, Rails.configuration.API.DisabledAPIs end diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb index 8a878ada91..a1c8ff8a92 100644 --- a/services/api/test/unit/log_test.rb +++ b/services/api/test/unit/log_test.rb @@ -282,7 +282,7 @@ class LogTest < ActiveSupport::TestCase end test "non-empty configuration.unlogged_attributes" do - Rails.configuration.AuditLogs.UnloggedAttributes = ["manifest_text"] + Rails.configuration.AuditLogs.UnloggedAttributes = {"manifest_text"=>{}} txt = ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo\n" act_as_system_user do @@ -297,7 +297,7 @@ class LogTest < ActiveSupport::TestCase end test "empty configuration.unlogged_attributes" do - Rails.configuration.AuditLogs.UnloggedAttributes = [] + Rails.configuration.AuditLogs.UnloggedAttributes = {} txt = ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo\n" act_as_system_user do diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb index 185653e873..6d2157b144 100644 --- a/services/api/test/unit/user_test.rb +++ b/services/api/test/unit/user_test.rb @@ -110,7 +110,7 @@ class UserTest < ActiveSupport::TestCase end test "new username set avoiding blacklist" do - Rails.configuration.Users.AutoSetupUsernameBlacklist = ["root"] + Rails.configuration.Users.AutoSetupUsernameBlacklist = {"root"=>{}} check_new_username_setting("root", "root2") end @@ -341,45 +341,45 @@ class UserTest < ActiveSupport::TestCase test "create new user with notifications" do set_user_from_auth :admin - create_user_and_verify_setup_and_notifications true, 'active-notify-address@example.com', 'inactive-notify-address@example.com', nil, nil - create_user_and_verify_setup_and_notifications true, 'active-notify-address@example.com', [], nil, nil - create_user_and_verify_setup_and_notifications true, [], [], nil, nil - create_user_and_verify_setup_and_notifications false, 'active-notify-address@example.com', 'inactive-notify-address@example.com', nil, nil - create_user_and_verify_setup_and_notifications false, [], 'inactive-notify-address@example.com', nil, nil - create_user_and_verify_setup_and_notifications false, [], [], nil, nil + create_user_and_verify_setup_and_notifications true, {'active-notify-address@example.com'=>{}}, {'inactive-notify-address@example.com'=>{}}, nil, nil + create_user_and_verify_setup_and_notifications true, {'active-notify-address@example.com'=>{}}, {}, nil, nil + create_user_and_verify_setup_and_notifications true, {}, [], nil, nil + create_user_and_verify_setup_and_notifications false, {'active-notify-address@example.com'=>{}}, {'inactive-notify-address@example.com'=>{}}, nil, nil + create_user_and_verify_setup_and_notifications false, {}, {'inactive-notify-address@example.com'=>{}}, nil, nil + create_user_and_verify_setup_and_notifications false, {}, {}, nil, nil end [ # Easy inactive user tests. - [false, [], [], "inactive-none@example.com", false, false, "inactivenone"], - [false, [], [], "inactive-vm@example.com", true, false, "inactivevm"], - [false, [], [], "inactive-repo@example.com", false, true, "inactiverepo"], - [false, [], [], "inactive-both@example.com", true, true, "inactiveboth"], + [false, {}, {}, "inactive-none@example.com", false, false, "inactivenone"], + [false, {}, {}, "inactive-vm@example.com", true, false, "inactivevm"], + [false, {}, {}, "inactive-repo@example.com", false, true, "inactiverepo"], + [false, {}, {}, "inactive-both@example.com", true, true, "inactiveboth"], # Easy active user tests. - [true, "active-notify@example.com", "inactive-notify@example.com", "active-none@example.com", false, false, "activenone"], - [true, "active-notify@example.com", "inactive-notify@example.com", "active-vm@example.com", true, false, "activevm"], - [true, "active-notify@example.com", "inactive-notify@example.com", "active-repo@example.com", false, true, "activerepo"], - [true, "active-notify@example.com", "inactive-notify@example.com", "active-both@example.com", true, true, "activeboth"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "active-none@example.com", false, false, "activenone"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "active-vm@example.com", true, false, "activevm"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "active-repo@example.com", false, true, "activerepo"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "active-both@example.com", true, true, "activeboth"], # Test users with malformed e-mail addresses. - [false, [], [], nil, true, true, nil], - [false, [], [], "arvados", true, true, nil], - [false, [], [], "@example.com", true, true, nil], - [true, "active-notify@example.com", "inactive-notify@example.com", "*!*@example.com", true, false, nil], - [true, "active-notify@example.com", "inactive-notify@example.com", "*!*@example.com", false, false, nil], + [false, {}, {}, nil, true, true, nil], + [false, {}, {}, "arvados", true, true, nil], + [false, {}, {}, "@example.com", true, true, nil], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "*!*@example.com", true, false, nil], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "*!*@example.com", false, false, nil], # Test users with various username transformations. - [false, [], [], "arvados@example.com", false, false, "arvados2"], - [true, "active-notify@example.com", "inactive-notify@example.com", "arvados@example.com", false, false, "arvados2"], - [true, "active-notify@example.com", "inactive-notify@example.com", "root@example.com", true, false, "root2"], - [false, "active-notify@example.com", "inactive-notify@example.com", "root@example.com", true, false, "root2"], - [true, "active-notify@example.com", "inactive-notify@example.com", "roo_t@example.com", false, true, "root2"], - [false, [], [], "^^incorrect_format@example.com", true, true, "incorrectformat"], - [true, "active-notify@example.com", "inactive-notify@example.com", "&4a_d9.@example.com", true, true, "ad9"], - [true, "active-notify@example.com", "inactive-notify@example.com", "&4a_d9.@example.com", false, false, "ad9"], - [false, "active-notify@example.com", "inactive-notify@example.com", "&4a_d9.@example.com", true, true, "ad9"], - [false, "active-notify@example.com", "inactive-notify@example.com", "&4a_d9.@example.com", false, false, "ad9"], + [false, {}, {}, "arvados@example.com", false, false, "arvados2"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "arvados@example.com", false, false, "arvados2"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "root@example.com", true, false, "root2"], + [false, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "root@example.com", true, false, "root2"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "roo_t@example.com", false, true, "root2"], + [false, {}, {}, "^^incorrect_format@example.com", true, true, "incorrectformat"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "&4a_d9.@example.com", true, true, "ad9"], + [true, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "&4a_d9.@example.com", false, false, "ad9"], + [false, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "&4a_d9.@example.com", true, true, "ad9"], + [false, {"active-notify@example.com"=>{}}, {"inactive-notify@example.com"=>{}}, "&4a_d9.@example.com", false, false, "ad9"], ].each do |active, new_user_recipients, inactive_recipients, email, auto_setup_vm, auto_setup_repo, expect_username| test "create new user with auto setup #{active} #{email} #{auto_setup_vm} #{auto_setup_repo}" do set_user_from_auth :admin @@ -686,7 +686,7 @@ class UserTest < ActiveSupport::TestCase if not new_user_recipients.empty? then assert_not_nil new_user_email, 'Expected new user email after setup' assert_equal Rails.configuration.Users.UserNotifierEmailFrom, new_user_email.from[0] - assert_equal new_user_recipients, new_user_email.to[0] + assert_equal new_user_recipients.keys.first, new_user_email.to[0] assert_equal new_user_email_subject, new_user_email.subject else assert_nil new_user_email, 'Did not expect new user email after setup' @@ -696,7 +696,7 @@ class UserTest < ActiveSupport::TestCase if not inactive_recipients.empty? then assert_not_nil new_inactive_user_email, 'Expected new inactive user email after setup' assert_equal Rails.configuration.Users.UserNotifierEmailFrom, new_inactive_user_email.from[0] - assert_equal inactive_recipients, new_inactive_user_email.to[0] + assert_equal inactive_recipients.keys.first, new_inactive_user_email.to[0] assert_equal "#{Rails.configuration.Users.EmailSubjectPrefix}New inactive user notification", new_inactive_user_email.subject else assert_nil new_inactive_user_email, 'Did not expect new inactive user email after setup'