From 67a60267e71d5a236ce1e1498b6774e4a207f5f3 Mon Sep 17 00:00:00 2001 From: Pirate Praveen Date: Wed, 22 Apr 2020 19:44:27 +0530 Subject: [PATCH] Refresh patches --- debian/patches/0050-relax-stable-libs.patch | 48 +- .../0100-remove-development-test.patch | 8 +- ...0-make-test-dependencies-conditional.patch | 2 +- .../patches/0430-remove-gitlab-markup.patch | 2 +- .../0460-embed-derailed-benchmarks.patch | 14 - .../0484-relax-asciidoctor-plantuml.patch | 2 +- .../0510-remove-dev-dependencies.patch | 11 +- .../patches/0610-source-init-functions.patch | 2 +- .../patches/0740-use-packaged-modules.patch | 10 +- debian/patches/0760-update-grape.patch | 855 +++++++++++++----- debian/patches/0790-protobuf-compat.patch | 2 +- debian/patches/series | 1 - 12 files changed, 684 insertions(+), 273 deletions(-) delete mode 100644 debian/patches/0460-embed-derailed-benchmarks.patch diff --git a/debian/patches/0050-relax-stable-libs.patch b/debian/patches/0050-relax-stable-libs.patch index 41b42dccee..6d36dc2099 100644 --- a/debian/patches/0050-relax-stable-libs.patch +++ b/debian/patches/0050-relax-stable-libs.patch @@ -111,8 +111,8 @@ gitlab Gemfile +gem 'rack-cors', '~> 1.0', '>= 1.0.6', require: 'rack/cors' # GraphQL API --gem 'graphql', '~> 1.9.19' -+gem 'graphql', '~> 1.9', '>= 1.9.19' +-gem 'graphql', '~> 1.10.5' ++gem 'graphql', '~> 1.10', '>= 1.10.5' # NOTE: graphiql-rails v1.5+ doesn't work: https://gitlab.com/gitlab-org/gitlab/issues/31771 # TODO: remove app/views/graphiql/rails/editors/show.html.erb when https://github.com/rmosolgo/graphiql-rails/pull/71 is released: # https://gitlab.com/gitlab-org/gitlab/issues/31747 @@ -151,12 +151,14 @@ gitlab Gemfile # Search gem 'elasticsearch-model', '~> 6.1' -@@ -138,21 +138,21 @@ +@@ -137,22 +137,22 @@ + # Markdown and HTML processing gem 'html-pipeline', '~> 2.12' - gem 'deckar01-task_list', '2.3.1' +-gem 'deckar01-task_list', '2.3.1' -gem 'gitlab-markup', '~> 1.7.0' -gem 'github-markup', '~> 1.7.0', require: 'github/markup' ++gem 'deckar01-task_list', '~> 2.3', '>= 2.3.1' +gem 'gitlab-markup', '~> 1.7' +gem 'github-markup', '~> 1.7', require: 'github/markup' gem 'commonmarker', '~> 0.20' @@ -171,8 +173,8 @@ gitlab Gemfile +gem 'asciidoctor', '~> 2.0', '>= 2.0.10' gem 'asciidoctor-include-ext', '~> 0.3.1', require: false gem 'asciidoctor-plantuml', '0.0.10' --gem 'rouge', '~> 3.17.0' -+gem 'rouge', '~> 3.17' +-gem 'rouge', '~> 3.18.0' ++gem 'rouge', '~> 3.18' gem 'truncato', '~> 0.7.11' -gem 'bootstrap_form', '~> 4.2.0' -gem 'nokogiri', '~> 1.10.5' @@ -302,7 +304,7 @@ gitlab Gemfile +gem 'premailer-rails', '~> 1.10', '>= 1.10.3' # LabKit: Tracing and Correlation - gem 'gitlab-labkit', '0.11.0' + gem 'gitlab-labkit', '0.12.0' @@ -306,11 +306,11 @@ # I18n gem 'ruby_parser', '~> 3.8', require: false @@ -329,8 +331,9 @@ gitlab Gemfile gem 'awesome_print', require: false - gem 'database_cleaner', '~> 1.7.0' +- gem 'factory_bot_rails', '~> 5.1.0' + gem 'database_cleaner', '~> 1.7' - gem 'factory_bot_rails', '~> 5.1.0' ++ gem 'factory_bot_rails', '~> 5.1' gem 'rspec-rails', '~> 4.0.0.beta4' # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826) @@ -345,8 +348,8 @@ gitlab Gemfile + gem 'spring', '~> 2.0' + gem 'spring-commands-rspec', '~> 1.0', '>= 1.0.4' -- gem 'gitlab-styles', '~> 3.1.0', require: false -+ gem 'gitlab-styles', '~> 3.1', require: false +- gem 'gitlab-styles', '~> 3.2.0', require: false ++ gem 'gitlab-styles', '~> 3.2', require: false # Pin these dependencies, otherwise a new rule could break the CI pipelines - gem 'rubocop', '~> 0.74.0' - gem 'rubocop-performance', '~> 1.4.1' @@ -370,7 +373,7 @@ gitlab Gemfile gem 'timecop', '~> 0.9.1' -@@ -396,20 +396,20 @@ +@@ -396,19 +396,19 @@ end group :test do @@ -378,21 +381,18 @@ gitlab Gemfile + gem 'fuubar', '~> 2.2' gem 'rspec-retry', '~> 0.6.1' gem 'rspec_profiling', '~> 0.0.5' - gem 'rspec-set', '~> 0.1.3' gem 'rspec-parameterized', require: false - gem 'capybara', '~> 3.22.0' - gem 'capybara-screenshot', '~> 1.0.22' -- gem 'selenium-webdriver', '~> 3.142' -- ++ gem 'capybara', '~> 3.22' ++ gem 'capybara-screenshot', '~> 1.0', '>= 1.0.22' + gem 'selenium-webdriver', '~> 3.142' + - gem 'shoulda-matchers', '~> 4.0.1', require: false - gem 'email_spec', '~> 2.2.0' - gem 'json-schema', '~> 2.8.0' - gem 'webmock', '~> 3.5.1' -+ gem 'capybara', '~> 3.22' -+ gem 'capybara-screenshot', '~> 1.0', '>= 1.0.22' -+ gem 'selenium-webdriver', '~> 3.1', '>= 3.142' -+ + gem 'shoulda-matchers', '~> 4.0', '>= 4.0.1', require: false + gem 'email_spec', '~> 2.2' + gem 'json-schema', '~> 2.8' @@ -400,16 +400,16 @@ gitlab Gemfile gem 'rails-controller-testing' gem 'concurrent-ruby', '~> 1.1' gem 'test-prof', '~> 0.10.0' -@@ -425,7 +425,7 @@ +@@ -424,7 +424,7 @@ gem 'email_reply_trimmer', '~> 0.1' gem 'html2text' --gem 'ruby-prof', '~> 1.0.0' -+gem 'ruby-prof', '~> 1.0' +-gem 'ruby-prof', '~> 1.3.0' ++gem 'ruby-prof', '~> 1.3' gem 'stackprof', '~> 0.2.15', require: false gem 'rbtrace', '~> 0.4', require: false gem 'memory_profiler', '~> 0.9', require: false -@@ -436,11 +436,11 @@ +@@ -435,11 +435,11 @@ gem 'oauth2', '~> 1.4' # Health check @@ -424,7 +424,7 @@ gitlab Gemfile # NTP client gem 'net-ntp' -@@ -458,11 +458,11 @@ +@@ -457,11 +457,11 @@ # Gitaly GRPC protocol definitions gem 'gitaly', '~> 12.9.0.pre.rc4' @@ -439,7 +439,7 @@ gitlab Gemfile # Feature toggles gem 'flipper', '~> 0.17.1' -@@ -480,14 +480,14 @@ +@@ -479,14 +479,14 @@ # Countries list gem 'countries', '~> 3.0' diff --git a/debian/patches/0100-remove-development-test.patch b/debian/patches/0100-remove-development-test.patch index 10b8249a5f..e21e8077e1 100644 --- a/debian/patches/0100-remove-development-test.patch +++ b/debian/patches/0100-remove-development-test.patch @@ -42,11 +42,11 @@ Bundler will fail when it can't find these locally gem 'pry-byebug', '~> 3.5','>= 3.5.1', platform: :mri gem 'pry-rails', '~> 0.3.9' -@@ -366,16 +348,7 @@ +@@ -366,16 +348,6 @@ gem 'spring', '~> 2.0' gem 'spring-commands-rspec', '~> 1.0', '>= 1.0.4' -- gem 'gitlab-styles', '~> 3.1', require: false +- gem 'gitlab-styles', '~> 3.2', require: false - # Pin these dependencies, otherwise a new rule could break the CI pipelines - gem 'rubocop', '~> 0.74' - gem 'rubocop-performance', '~> 1.4', '>= 1.4.1' @@ -54,12 +54,12 @@ Bundler will fail when it can't find these locally - - gem 'scss_lint', '~> 0.56.0', require: false - gem 'haml_lint', '~> 0.34.0', require: false - gem 'simplecov', '~> 0.18.5', require: false +- gem 'simplecov', '~> 0.18.5', require: false - gem 'bundler-audit', '~> 0.6.1', require: false gem 'benchmark-ips', '~> 2.3', require: false -@@ -390,11 +363,6 @@ +@@ -390,11 +362,6 @@ gem 'parallel', '~> 1.19', require: false end diff --git a/debian/patches/0110-make-test-dependencies-conditional.patch b/debian/patches/0110-make-test-dependencies-conditional.patch index fd43d960e8..9e486672a8 100644 --- a/debian/patches/0110-make-test-dependencies-conditional.patch +++ b/debian/patches/0110-make-test-dependencies-conditional.patch @@ -11,7 +11,7 @@ Make test dependencies conditional so we can enable them when running autopkgtes gem 'pry-byebug', '~> 3.5','>= 3.5.1', platform: :mri gem 'pry-rails', '~> 0.3.9' -@@ -361,9 +361,6 @@ +@@ -360,9 +360,6 @@ gem 'png_quantizator', '~> 0.2.1', require: false gem 'parallel', '~> 1.19', require: false diff --git a/debian/patches/0430-remove-gitlab-markup.patch b/debian/patches/0430-remove-gitlab-markup.patch index 760cfa7858..4c38fece54 100644 --- a/debian/patches/0430-remove-gitlab-markup.patch +++ b/debian/patches/0430-remove-gitlab-markup.patch @@ -7,7 +7,7 @@ maintaining two almost same packages. @@ -137,7 +137,6 @@ # Markdown and HTML processing gem 'html-pipeline', '~> 2.12' - gem 'deckar01-task_list', '2.3.1' + gem 'deckar01-task_list', '~> 2.3', '>= 2.3.1' -gem 'gitlab-markup', '~> 1.7' gem 'github-markup', '~> 1.7', require: 'github/markup' gem 'commonmarker', '~> 0.20' diff --git a/debian/patches/0460-embed-derailed-benchmarks.patch b/debian/patches/0460-embed-derailed-benchmarks.patch deleted file mode 100644 index 244e8152b0..0000000000 --- a/debian/patches/0460-embed-derailed-benchmarks.patch +++ /dev/null @@ -1,14 +0,0 @@ -Embed this gem until it is packaged - ---- a/Gemfile -+++ b/Gemfile -@@ -310,7 +310,8 @@ - gem 'snowplow-tracker', '~> 0.6.1' - - # Memory benchmarks --gem 'gitlab-derailed_benchmarks', require: false -+gem 'gitlab-derailed_benchmarks', require: false, path: 'vendor/gems/derailed-benchmarks' -+gem 'heapy', require: false - - # Metrics - group :metrics do diff --git a/debian/patches/0484-relax-asciidoctor-plantuml.patch b/debian/patches/0484-relax-asciidoctor-plantuml.patch index 9d43e0736e..6345c4ef03 100644 --- a/debian/patches/0484-relax-asciidoctor-plantuml.patch +++ b/debian/patches/0484-relax-asciidoctor-plantuml.patch @@ -8,6 +8,6 @@ Allow newer patch releases to match requirement gem 'asciidoctor-include-ext', '~> 0.3.1', require: false -gem 'asciidoctor-plantuml', '0.0.10' +gem 'asciidoctor-plantuml', '~> 0.0.10' - gem 'rouge', '~> 3.17' + gem 'rouge', '~> 3.18' gem 'truncato', '~> 0.7.11' gem 'bootstrap_form', '~> 4.2' diff --git a/debian/patches/0510-remove-dev-dependencies.patch b/debian/patches/0510-remove-dev-dependencies.patch index 5587256661..c4a09738eb 100644 --- a/debian/patches/0510-remove-dev-dependencies.patch +++ b/debian/patches/0510-remove-dev-dependencies.patch @@ -2,12 +2,12 @@ These are not required in production --- a/package.json +++ b/package.json -@@ -145,65 +145,7 @@ +@@ -144,65 +144,7 @@ "xterm": "^3.5.0" }, "devDependencies": { - "@babel/plugin-transform-modules-commonjs": "^7.8.3", -- "@gitlab/eslint-config": "^3.0.0", +- "@gitlab/eslint-plugin": "2.2.1", - "@vue/test-utils": "^1.0.0-beta.30", - "axios-mock-adapter": "^1.15.0", - "babel-jest": "^24.1.0", @@ -69,10 +69,3 @@ These are not required in production "blockedDependencies": { "bootstrap-vue": "https://docs.gitlab.com/ee/development/fe_guide/dependencies.md#bootstrapvue" }, -@@ -215,4 +157,4 @@ - "node": ">=10.13.0", - "yarn": "^1.10.0" - } --} -\ No newline at end of file -+} diff --git a/debian/patches/0610-source-init-functions.patch b/debian/patches/0610-source-init-functions.patch index bc69d98eca..8eef9efb7c 100644 --- a/debian/patches/0610-source-init-functions.patch +++ b/debian/patches/0610-source-init-functions.patch @@ -14,7 +14,7 @@ Bug: https://gitlab.com/gitlab-org/gitlab-ce/issues/12954 ### Environment variables RAILS_ENV="production" @@ -39,7 +42,7 @@ - sidekiq_pid_path="$pid_path/sidekiq.pid" + web_server_pid_path="$pid_path/unicorn.pid" mail_room_enabled=false mail_room_pid_path="$pid_path/mail_room.pid" -gitlab_workhorse_dir=$(cd $app_root/../gitlab-workhorse 2> /dev/null && pwd) diff --git a/debian/patches/0740-use-packaged-modules.patch b/debian/patches/0740-use-packaged-modules.patch index 5e126df38d..955cf213b2 100644 --- a/debian/patches/0740-use-packaged-modules.patch +++ b/debian/patches/0740-use-packaged-modules.patch @@ -75,7 +75,7 @@ Use debian packaged node modules when available }; --- a/package.json +++ b/package.json -@@ -49,63 +49,40 @@ +@@ -49,62 +49,39 @@ "apollo-link": "^1.2.11", "apollo-link-batch-http": "^1.2.11", "apollo-upload-client": "^10.0.0", @@ -96,8 +96,7 @@ Use debian packaged node modules when available - "core-js": "^3.6.4", "cropper": "^2.3.0", - "css-loader": "^1.0.0", -- "d3": "^4.13.0", -- "d3-scale": "^1.0.7", +- "d3-scale": "^2.2.2", - "d3-selection": "^1.2.0", - "dateformat": "^3.0.3", - "deckar01-task_list": "^2.3.1", @@ -112,7 +111,6 @@ Use debian packaged node modules when available "fuzzaldrin-plus": "^0.6.0", "glob": "^7.1.6", "graphql": "^14.0.2", -+ "graphql-tag": "^2.0.0", "immer": "^5.2.1", - "imports-loader": "^0.8.0", - "jed": "^1.1.1", @@ -140,7 +138,7 @@ Use debian packaged node modules when available "raphael": "^2.2.7", "raw-loader": "^4.0.0", "sanitize-html": "^1.22.0", -@@ -118,16 +95,11 @@ +@@ -117,16 +94,11 @@ "svg4everybody": "2.1.9", "swagger-ui-dist": "^3.24.3", "three": "^0.84.0", @@ -157,7 +155,7 @@ Use debian packaged node modules when available "visibilityjs": "^1.2.4", "vue": "^2.6.10", "vue-apollo": "^3.0.0-beta.28", -@@ -137,12 +109,7 @@ +@@ -136,12 +108,7 @@ "vue-virtual-scroll-list": "^1.4.4", "vuedraggable": "^2.23.0", "vuex": "^3.1.0", diff --git a/debian/patches/0760-update-grape.patch b/debian/patches/0760-update-grape.patch index fb94b88fa8..f814f5f6f6 100644 --- a/debian/patches/0760-update-grape.patch +++ b/debian/patches/0760-update-grape.patch @@ -1,30 +1,48 @@ -From e02626b961ce60fcc92757f977dff211e414e572 Mon Sep 17 00:00:00 2001 +From a9ffc8f1bc91ab7d837dfdfd50801a6a35f04717 Mon Sep 17 00:00:00 2001 From: Stan Hu -Date: Thu, 12 Mar 2020 15:24:13 -0700 -Subject: [PATCH 01/12] Upgrade Grape v1.1.0 to v1.3.1 +Date: Tue, 21 Apr 2020 23:31:35 -0700 +Subject: [PATCH 1/2] Upgrade Grape v1.1.0 to v1.3.2 This brings in Ruby 2.7 suport and a number of fixes: https://github.com/ruby-grape/grape/blob/master/CHANGELOG.md -Move all inherited Grape::API -> Grape::API::Instance +1. Move all inherited Grape::API -> Grape::API::Instance +2. Remove use of Virtus since this has been removed from Grape. +3. Extract Rack::Response from API error +4. Grape v1.2.3 pulled in a fix used in SafeFile: +https://github.com/ruby-grape/grape/pull/1844, so we no longer need +to maintain our custom type. +5. Adapt WorkhorseFile with the latest changes to make custom types work +with Grape and dry-types. +6. Ensure Array[String] is coerced properly. -Remove use of Virtus since this has been removed from Grape. +The change from Virtus to dry-types now requires all strings to be +coerced to arrays. Before this was done within Virtus. + +7. Coerce Array[Integer] types to arrays of integers + +The change from Virtus to dry-types now requires all strings to be +coerced to arrays of integers. Before this was done within Virtus. --- Gemfile | 4 +- Gemfile.lock | 55 +++++++++++-------- changelogs/unreleased/sh-update-grape-gem.yml | 5 ++ + doc/development/api_styleguide.md | 8 +++ doc/development/ee_features.md | 12 ++-- ee/lib/api/analytics/code_review_analytics.rb | 2 +- + .../api/analytics/group_activity_analytics.rb | 2 +- ee/lib/api/audit_events.rb | 2 +- + ee/lib/api/composer_packages.rb | 2 +- ee/lib/api/conan_packages.rb | 2 +- - ee/lib/api/container_registry_event.rb | 2 +- - ee/lib/api/dependencies.rb | 2 +- + ee/lib/api/dependencies.rb | 3 +- + ee/lib/api/dependency_proxy.rb | 2 +- .../api/elasticsearch_indexed_namespaces.rb | 2 +- ee/lib/api/epic_issues.rb | 2 +- ee/lib/api/epic_links.rb | 2 +- - ee/lib/api/epics.rb | 2 +- + ee/lib/api/epics.rb | 8 +-- ee/lib/api/feature_flag_scopes.rb | 2 +- ee/lib/api/feature_flags.rb | 2 +- + ee/lib/api/feature_flags_user_lists.rb | 2 +- ee/lib/api/geo.rb | 2 +- ee/lib/api/geo_nodes.rb | 2 +- ee/lib/api/geo_replication.rb | 2 +- @@ -50,19 +68,23 @@ Remove use of Virtus since this has been removed from Grape. ee/lib/api/project_packages.rb | 2 +- ee/lib/api/project_push_rule.rb | 2 +- ee/lib/api/protected_environments.rb | 2 +- + ee/lib/api/pypi_packages.rb | 2 +- ee/lib/api/scim.rb | 2 +- ee/lib/api/unleash.rb | 2 +- ee/lib/api/v3/github.rb | 2 +- ee/lib/api/visual_review_discussions.rb | 2 +- ee/lib/api/vulnerabilities.rb | 2 +- - ee/lib/api/vulnerability_findings.rb | 2 +- + ee/lib/api/vulnerability_exports.rb | 2 +- + ee/lib/api/vulnerability_findings.rb | 8 ++- ee/lib/api/vulnerability_issue_links.rb | 2 +- ee/lib/ee/api/boards.rb | 2 +- ee/lib/ee/api/group_boards.rb | 2 +- + ee/lib/ee/api/helpers/settings_helpers.rb | 6 +- ee/spec/lib/ee/api/helpers_spec.rb | 2 +- lib/api/access_requests.rb | 2 +- lib/api/admin/sidekiq.rb | 2 +- lib/api/api.rb | 2 +- + lib/api/api_guard.rb | 11 +++- lib/api/appearance.rb | 2 +- lib/api/applications.rb | 2 +- lib/api/avatar.rb | 2 +- @@ -73,8 +95,9 @@ Remove use of Virtus since this has been removed from Grape. lib/api/broadcast_messages.rb | 2 +- lib/api/commit_statuses.rb | 2 +- lib/api/commits.rb | 2 +- + lib/api/container_registry_event.rb | 2 +- lib/api/deploy_keys.rb | 2 +- - lib/api/deploy_tokens.rb | 2 +- + lib/api/deploy_tokens.rb | 6 +- lib/api/deployments.rb | 2 +- lib/api/discussions.rb | 2 +- lib/api/environments.rb | 2 +- @@ -90,11 +113,13 @@ Remove use of Virtus since this has been removed from Grape. lib/api/group_labels.rb | 2 +- lib/api/group_milestones.rb | 2 +- lib/api/group_variables.rb | 2 +- - lib/api/groups.rb | 2 +- + lib/api/groups.rb | 4 +- + lib/api/helpers/merge_requests_helpers.rb | 2 +- + lib/api/helpers/projects_helpers.rb | 2 +- lib/api/import_github.rb | 2 +- lib/api/internal/base.rb | 2 +- lib/api/internal/pages.rb | 2 +- - lib/api/issues.rb | 2 +- + lib/api/issues.rb | 10 ++-- lib/api/job_artifacts.rb | 2 +- lib/api/jobs.rb | 2 +- lib/api/keys.rb | 2 +- @@ -102,9 +127,11 @@ Remove use of Virtus since this has been removed from Grape. lib/api/lint.rb | 2 +- lib/api/lsif_data.rb | 2 +- lib/api/markdown.rb | 2 +- - lib/api/members.rb | 2 +- + lib/api/members.rb | 6 +- lib/api/merge_request_diffs.rb | 2 +- - lib/api/merge_requests.rb | 2 +- + lib/api/merge_requests.rb | 8 +-- + lib/api/metrics/dashboard/annotations.rb | 2 +- + lib/api/milestone_responses.rb | 2 +- lib/api/namespaces.rb | 2 +- lib/api/notes.rb | 2 +- lib/api/notification_settings.rb | 2 +- @@ -124,19 +151,19 @@ Remove use of Virtus since this has been removed from Grape. lib/api/project_snippets.rb | 2 +- lib/api/project_statistics.rb | 2 +- lib/api/project_templates.rb | 2 +- - lib/api/projects.rb | 2 +- + lib/api/projects.rb | 4 +- lib/api/protected_branches.rb | 2 +- lib/api/protected_tags.rb | 2 +- lib/api/release/links.rb | 2 +- lib/api/releases.rb | 2 +- lib/api/remote_mirrors.rb | 2 +- - lib/api/repositories.rb | 2 +- + lib/api/repositories.rb | 4 +- lib/api/resource_label_events.rb | 2 +- - lib/api/runner.rb | 2 +- - lib/api/runners.rb | 2 +- + lib/api/runner.rb | 4 +- + lib/api/runners.rb | 12 ++-- lib/api/search.rb | 2 +- lib/api/services.rb | 2 +- - lib/api/settings.rb | 2 +- + lib/api/settings.rb | 11 ++-- lib/api/sidekiq_metrics.rb | 2 +- lib/api/snippets.rb | 2 +- lib/api/statistics.rb | 2 +- @@ -146,37 +173,41 @@ Remove use of Virtus since this has been removed from Grape. lib/api/system_hooks.rb | 2 +- lib/api/tags.rb | 2 +- lib/api/templates.rb | 2 +- + lib/api/terraform/state.rb | 2 +- lib/api/todos.rb | 2 +- lib/api/triggers.rb | 2 +- lib/api/user_counts.rb | 2 +- lib/api/users.rb | 2 +- - lib/api/validations/types/workhorse_file.rb | 2 +- + .../types/comma_separated_to_array.rb | 2 +- + .../types/comma_separated_to_integer_array.rb | 15 +++++ + lib/api/validations/types/labels_list.rb | 24 -------- + lib/api/validations/types/safe_file.rb | 15 ----- + lib/api/validations/types/workhorse_file.rb | 13 ++--- lib/api/variables.rb | 2 +- lib/api/version.rb | 2 +- - lib/api/wikis.rb | 2 +- + lib/api/wikis.rb | 4 +- + spec/requests/api/settings_spec.rb | 10 +++- spec/rubocop/cop/code_reuse/worker_spec.rb | 2 +- - 145 files changed, 187 insertions(+), 171 deletions(-) + 164 files changed, 282 insertions(+), 264 deletions(-) create mode 100644 changelogs/unreleased/sh-update-grape-gem.yml + create mode 100644 lib/api/validations/types/comma_separated_to_integer_array.rb + delete mode 100644 lib/api/validations/types/labels_list.rb + delete mode 100644 lib/api/validations/types/safe_file.rb +--- a/Gemfile ++++ b/Gemfile +@@ -19,7 +19,7 @@ + gem 'pg', '~> 1.1' + + gem 'rugged', '~> 0.28' +-gem 'grape-path-helpers', '~> 1.2' ++gem 'grape-path-helpers', '~> 1.3' + + gem 'faraday', '~> 0.12' + gem 'marginalia', '~> 1.8' --- a/Gemfile.lock +++ b/Gemfile.lock -@@ -1,3 +1,15 @@ -+GIT -+ remote: https://github.com/ruby-grape/grape.git -+ revision: cd03bcf1054a2cba2f8c6a31ca2f1563409f728b -+ specs: -+ grape (1.3.2) -+ activesupport -+ builder -+ dry-types (>= 1.1) -+ mustermann-grape (~> 1.0.0) -+ rack (>= 1.3.0) -+ rack-accept -+ - GEM - remote: https://rubygems.org/ - specs: -@@ -103,10 +115,6 @@ +@@ -103,10 +103,6 @@ aws-sdk-core (= 2.11.374) aws-sigv4 (1.1.0) aws-eventstream (~> 1.0, >= 1.0.2) @@ -187,7 +218,7 @@ Remove use of Virtus since this has been removed from Grape. babosa (1.0.2) base32 (0.3.2) batch-loader (1.4.0) -@@ -165,8 +173,6 @@ +@@ -165,8 +161,6 @@ nap open4 (~> 1.3) coderay (1.1.2) @@ -196,11 +227,20 @@ Remove use of Virtus since this has been removed from Grape. colored2 (3.1.2) commonmarker (0.20.1) ruby-enum (~> 0.5) -@@ -239,6 +245,28 @@ +@@ -222,8 +216,6 @@ + ruby-statistics (>= 2.1) + thor (>= 0.19, < 2) + unicode_plot (>= 0.0.4, < 1.0.0) +- descendants_tracker (0.0.4) +- thread_safe (~> 0.3, >= 0.3.1) + device_detector (1.0.0) + devise (4.7.1) + bcrypt (~> 3.0) +@@ -250,6 +242,28 @@ doorkeeper-openid_connect (1.6.3) doorkeeper (>= 5.0, < 5.2) json-jwt (~> 1.6) -+ dry-configurable (0.11.4) ++ dry-configurable (0.11.5) + concurrent-ruby (~> 1.0) + dry-core (~> 0.4, >= 0.4.7) + dry-equalizer (~> 0.2) @@ -225,16 +265,18 @@ Remove use of Virtus since this has been removed from Grape. ed25519 (1.2.4) elasticsearch (6.8.0) elasticsearch-api (= 6.8.0) -@@ -436,19 +464,12 @@ +@@ -439,19 +453,19 @@ signet (~> 0.7) gpgme (2.0.20) mini_portile2 (~> 2.3) - grape (1.1.0) -- activesupport -- builder -- mustermann-grape (~> 1.0.0) -- rack (>= 1.3.0) -- rack-accept ++ grape (1.3.2) + activesupport + builder ++ dry-types (>= 1.1) + mustermann-grape (~> 1.0.0) + rack (>= 1.3.0) + rack-accept - virtus (>= 1.0.0) grape-entity (0.7.1) activesupport (>= 4.0) @@ -247,7 +289,7 @@ Remove use of Virtus since this has been removed from Grape. rake (~> 12) grape_logging (1.8.3) grape -@@ -640,9 +661,10 @@ +@@ -645,9 +659,10 @@ multi_xml (0.6.0) multipart-post (2.1.1) murmurhash3 (0.1.6) @@ -261,7 +303,15 @@ Remove use of Virtus since this has been removed from Grape. nakayoshi_fork (0.0.4) nap (1.1.0) nenv (0.3.0) -@@ -1112,11 +1134,6 @@ +@@ -960,6 +975,7 @@ + ruby-saml (1.7.2) + nokogiri (>= 1.5.10) + ruby-statistics (2.1.2) ++ ruby2_keywords (0.0.2) + ruby_dep (1.5.0) + ruby_parser (3.13.1) + sexp_processor (~> 4.9) +@@ -1118,11 +1134,6 @@ activerecord (>= 3.0) activesupport (>= 3.0) version_sorter (2.2.4) @@ -273,34 +323,51 @@ Remove use of Virtus since this has been removed from Grape. vmstat (2.3.0) warden (1.2.8) rack (>= 2.0.6) -@@ -1247,9 +1264,9 @@ +@@ -1253,9 +1264,9 @@ google-api-client (~> 0.23) google-protobuf (~> 3.8.0) gpgme (~> 2.0.19) - grape (~> 1.1.0) -+ grape! ++ grape (~> 1.3.2) grape-entity (~> 0.7.1) - grape-path-helpers (~> 1.2) + grape-path-helpers (~> 1.3) grape_logging (~> 1.7) graphiql-rails (~> 1.4.10) - graphql (~> 1.9.19) + graphql (~> 1.10.5) --- /dev/null +++ b/changelogs/unreleased/sh-update-grape-gem.yml @@ -0,0 +1,5 @@ +--- -+title: Upgrade Grape v1.1.0 to v1.3.1 ++title: Upgrade Grape v1.1.0 to v1.3.2 +merge_request: 27276 +author: +type: other +--- a/doc/development/api_styleguide.md ++++ b/doc/development/api_styleguide.md +@@ -98,6 +98,14 @@ + Model.create(foo: params[:foo]) + ``` + ++## Array types ++ ++With Grape v1.3+, Array types must be defined with a `coerce_with` ++block, or parameters will fail to validate when passed a string from an ++API request. See the [Grape upgrading ++documentation](https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions) ++for more details. ++ + ## Using HTTP status helpers + + For non-200 HTTP responses, use the provided helpers in `lib/api/helpers.rb` to ensure correct behaviour (`not_found!`, `no_content!` etc.). These will `throw` inside Grape and abort the execution of your endpoint. --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md -@@ -516,12 +516,12 @@ +@@ -513,12 +513,12 @@ interface first here. - For example, suppose we have a few more optional params for EE. We can move the --params out of the `Grape::API` class to a helper module, so we can inject it -+params out of the `Grape::API::Instance` class to a helper module, so we can inject it + For example, suppose we have a few more optional parameters for EE. We can move the +-paramters out of the `Grape::API` class to a helper module, so we can inject it ++parameters out of the `Grape::API::Instance` class to a helper module, so we can inject it before it would be used in the class. ```ruby @@ -310,7 +377,7 @@ Remove use of Virtus since this has been removed from Grape. helpers Helpers::ProjectsHelpers end end -@@ -582,7 +582,7 @@ +@@ -579,7 +579,7 @@ ```ruby module API @@ -319,7 +386,7 @@ Remove use of Virtus since this has been removed from Grape. # EE::API::JobArtifacts would override the following helpers helpers do def authorize_download_artifacts! -@@ -626,7 +626,7 @@ +@@ -623,7 +623,7 @@ ```ruby module API @@ -328,7 +395,7 @@ Remove use of Virtus since this has been removed from Grape. helpers do # EE::API::MergeRequests would override the following helpers def update_merge_request_ee(merge_request) -@@ -695,7 +695,7 @@ +@@ -692,7 +692,7 @@ ```ruby # api/merge_requests/parameters.rb module API @@ -337,7 +404,7 @@ Remove use of Virtus since this has been removed from Grape. module Parameters def self.update_params_at_least_one_of %i[ -@@ -711,7 +711,7 @@ +@@ -708,7 +708,7 @@ # api/merge_requests.rb module API @@ -357,6 +424,17 @@ Remove use of Virtus since this has been removed from Grape. include PaginationParams before { authenticate! } +--- a/lib/api/admin/sidekiq.rb ++++ b/lib/api/admin/sidekiq.rb +@@ -2,7 +2,7 @@ + + module API + module Admin +- class Sidekiq < Grape::API ++ class Sidekiq < Grape::API::Instance + before { authenticated_as_admin! } + + namespace 'admin' do --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -1,7 +1,7 @@ @@ -368,6 +446,26 @@ Remove use of Virtus since this has been removed from Grape. include APIGuard LOG_FILENAME = Rails.root.join("log", "api_json.log") +--- a/lib/api/api_guard.rb ++++ b/lib/api/api_guard.rb +@@ -148,7 +148,16 @@ + { scope: e.scopes }) + end + +- response.finish ++ finished_response = nil ++ response.finish do |rack_response| ++ # Grape expects a Rack::Response ++ # (https://github.com/ruby-grape/grape/commit/c117bff7d22971675f4b34367d3a98bc31c8fc02), ++ # and we need to retrieve it here: ++ # https://github.com/nov/rack-oauth2/blob/40c9a99fd80486ccb8de0e4869ae384547c0d703/lib/rack/oauth2/server/abstract/error.rb#L28 ++ finished_response = rack_response ++ end ++ ++ finished_response + end + end + end --- a/lib/api/appearance.rb +++ b/lib/api/appearance.rb @@ -1,7 +1,7 @@ @@ -379,19 +477,6 @@ Remove use of Virtus since this has been removed from Grape. before { authenticated_as_admin! } helpers do -@@ -24,9 +24,9 @@ - optional :title, type: String, desc: 'Instance title on the sign in / sign up page' - optional :description, type: String, desc: 'Markdown text shown on the sign in / sign up page' - # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 -- optional :logo, type: File, desc: 'Instance image used on the sign in / sign up page' # rubocop:disable Scalability/FileUploads -- optional :header_logo, type: File, desc: 'Instance image used for the main navigation bar' # rubocop:disable Scalability/FileUploads -- optional :favicon, type: File, desc: 'Instance favicon in .ico/.png format' # rubocop:disable Scalability/FileUploads -+ optional :logo, types: [File, ::API::Validations::Types::WorkhorseFile], desc: 'Instance image used on the sign in / sign up page' # rubocop:disable Scalability/FileUploads -+ optional :header_logo, types: [File, ::API::Validations::Types::WorkhorseFile], desc: 'Instance image used for the main navigation bar' # rubocop:disable Scalability/FileUploads -+ optional :favicon, types: [File, ::API::Validations::Types::WorkhorseFile], desc: 'Instance favicon in .ico/.png format' # rubocop:disable Scalability/FileUploads - optional :new_project_guidelines, type: String, desc: 'Markmarkdown text shown on the new project page' - optional :header_message, type: String, desc: 'Message within the system header bar' - optional :footer_message, type: String, desc: 'Message within the system footer bar' --- a/lib/api/applications.rb +++ b/lib/api/applications.rb @@ -2,7 +2,7 @@ @@ -491,6 +576,17 @@ Remove use of Virtus since this has been removed from Grape. include PaginationParams before do +--- a/lib/api/container_registry_event.rb ++++ b/lib/api/container_registry_event.rb +@@ -1,7 +1,7 @@ + # frozen_string_literal: true + + module API +- class ContainerRegistryEvent < Grape::API ++ class ContainerRegistryEvent < Grape::API::Instance + DOCKER_DISTRIBUTION_EVENTS_V1_JSON = 'application/vnd.docker.distribution.events.v1+json' + + before { authenticate_registry_notification! } --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -1,7 +1,7 @@ @@ -502,6 +598,35 @@ Remove use of Virtus since this has been removed from Grape. include PaginationParams before { authenticate! } +--- a/lib/api/deploy_tokens.rb ++++ b/lib/api/deploy_tokens.rb +@@ -1,7 +1,7 @@ + # frozen_string_literal: true + + module API +- class DeployTokens < Grape::API ++ class DeployTokens < Grape::API::Instance + include PaginationParams + + helpers do +@@ -54,7 +54,7 @@ + + params do + requires :name, type: String, desc: "New deploy token's name" +- requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), ++ requires :scopes, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), + desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository", "read_registry", or "write_registry".' + optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.' + optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`' +@@ -117,7 +117,7 @@ + + params do + requires :name, type: String, desc: 'The name of the deploy token' +- requires :scopes, type: Array[String], values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), ++ requires :scopes, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, values: ::DeployToken::AVAILABLE_SCOPES.map(&:to_s), + desc: 'Indicates the deploy token scopes. Must be at least one of "read_repository", "read_registry", or "write_registry".' + optional :expires_at, type: DateTime, desc: 'Expiration date for the deploy token. Does not expire if no value is provided.' + optional :username, type: String, desc: 'Username for deploy token. Default is `gitlab+deploy-token-{n}`' --- a/lib/api/deployments.rb +++ b/lib/api/deployments.rb @@ -2,7 +2,7 @@ @@ -532,9 +657,9 @@ Remove use of Virtus since this has been removed from Grape. # Environments RESTfull API endpoints - class Environments < Grape::API + class Environments < Grape::API::Instance - include ::API::Helpers::CustomValidators include PaginationParams + before { authenticate! } --- a/lib/api/error_tracking.rb +++ b/lib/api/error_tracking.rb @@ -1,7 +1,7 @@ @@ -687,6 +812,28 @@ Remove use of Virtus since this has been removed from Grape. optional :all_available, type: Boolean, desc: 'Show all group that you have access to' optional :search, type: String, desc: 'Search for a specific group' optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' +--- a/lib/api/helpers/merge_requests_helpers.rb ++++ b/lib/api/helpers/merge_requests_helpers.rb +@@ -24,7 +24,7 @@ + optional :milestone, type: String, desc: 'Return merge requests for a specific milestone' + optional :labels, + type: Array[String], +- coerce_with: Validations::Types::LabelsList.coerce, ++ coerce_with: Validations::Types::CommaSeparatedToArray.coerce, + desc: 'Comma-separated list of label names' + optional :with_labels_details, type: Boolean, desc: 'Return titles of labels and other details', default: false + optional :created_after, type: DateTime, desc: 'Return merge requests created after the specified time' +--- a/lib/api/helpers/projects_helpers.rb ++++ b/lib/api/helpers/projects_helpers.rb +@@ -44,7 +44,7 @@ + optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' + optional :only_allow_merge_if_pipeline_succeeds, type: Boolean, desc: 'Only allow to merge if builds succeed' + optional :only_allow_merge_if_all_discussions_are_resolved, type: Boolean, desc: 'Only allow to merge if all discussions are resolved' +- optional :tag_list, type: Array[String], desc: 'The list of tags for a project' ++ optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The list of tags for a project' + # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 + optional :avatar, type: File, desc: 'Avatar image for project' # rubocop:disable Scalability/FileUploads + optional :printing_merge_request_link_enabled, type: Boolean, desc: 'Show link to create/view merge request when pushing from the command line' --- a/lib/api/import_github.rb +++ b/lib/api/import_github.rb @@ -1,7 +1,7 @@ @@ -718,8 +865,8 @@ Remove use of Virtus since this has been removed from Grape. - class Pages < Grape::API + class Pages < Grape::API::Instance before do - not_found! unless Feature.enabled?(:pages_internal_api) authenticate_gitlab_pages_request! + end --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -1,7 +1,7 @@ @@ -730,20 +877,20 @@ Remove use of Virtus since this has been removed from Grape. + class Issues < Grape::API::Instance include PaginationParams helpers Helpers::IssuesHelpers - helpers ::Gitlab::IssuableMetadata -@@ -10,9 +10,9 @@ + helpers Helpers::RateLimiter +@@ -11,9 +11,9 @@ helpers do params :negatable_issue_filter_params do - optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names' -+ optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' ++ optional :labels, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' optional :milestone, type: String, desc: 'Milestone title' - optional :iids, type: Array[Integer], desc: 'The IID array of issues' + optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IID array of issues' optional :search, type: String, desc: 'Search issues for text present in the title, description, or any combination of these' optional :in, type: String, desc: '`title`, `description`, or a string joining them with comma' -@@ -62,10 +62,10 @@ +@@ -63,10 +63,10 @@ params :issue_params do optional :description, type: String, desc: 'The description of an issue' @@ -752,7 +899,7 @@ Remove use of Virtus since this has been removed from Grape. optional :assignee_id, type: Integer, desc: '[Deprecated] The ID of a user to assign issue' optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue' - optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names' -+ optional :labels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' ++ optional :labels, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'Comma-separated list of label names' optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY' optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential' optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked" @@ -905,6 +1052,28 @@ Remove use of Virtus since this has been removed from Grape. end get ":id/merge_requests" do authorize! :read_merge_request, user_project +--- a/lib/api/metrics/dashboard/annotations.rb ++++ b/lib/api/metrics/dashboard/annotations.rb +@@ -3,7 +3,7 @@ + module API + module Metrics + module Dashboard +- class Annotations < Grape::API ++ class Annotations < Grape::API::Instance + desc 'Create a new monitoring dashboard annotation' do + success Entities::Metrics::Dashboard::Annotation + end +--- a/lib/api/milestone_responses.rb ++++ b/lib/api/milestone_responses.rb +@@ -15,7 +15,7 @@ + params :list_params do + optional :state, type: String, values: %w[active closed all], default: 'all', + desc: 'Return "active", "closed", or "all" milestones' +- optional :iids, type: Array[Integer], desc: 'The IIDs of the milestones' ++ optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IIDs of the milestones' + optional :title, type: String, desc: 'The title of the milestones' + optional :search, type: String, desc: 'The search criteria for the title or description of the milestone' + use :pagination --- a/lib/api/namespaces.rb +++ b/lib/api/namespaces.rb @@ -1,7 +1,7 @@ @@ -1034,9 +1203,9 @@ Remove use of Virtus since this has been removed from Grape. module API - class ProjectExport < Grape::API + class ProjectExport < Grape::API::Instance - helpers do - def throttled?(action) - rate_limiter.throttled?(action, scope: [current_user, action, user_project]) + helpers Helpers::RateLimiter + + before do --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -1,7 +1,7 @@ @@ -1102,7 +1271,7 @@ Remove use of Virtus since this has been removed from Grape. + class ProjectStatistics < Grape::API::Instance before do authenticate! - not_found! unless user_project.daily_statistics_enabled? + authorize! :daily_statistics, user_project --- a/lib/api/project_templates.rb +++ b/lib/api/project_templates.rb @@ -1,7 +1,7 @@ @@ -1125,16 +1294,7 @@ Remove use of Virtus since this has been removed from Grape. include PaginationParams include Helpers::CustomAttributes -@@ -504,7 +504,7 @@ - desc 'Upload a file' - params do - # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 -- requires :file, type: File, desc: 'The file to be uploaded' # rubocop:disable Scalability/FileUploads -+ requires :file, types: [File, ::API::Validations::Types::WorkhorseFile], desc: 'The file to be uploaded' # rubocop:disable Scalability/FileUploads - end - post ":id/uploads" do - upload = UploadService.new(user_project, params[:file]).execute -@@ -517,7 +517,7 @@ +@@ -520,7 +520,7 @@ end params do optional :search, type: String, desc: 'Return list of users matching the search criteria' @@ -1292,6 +1452,15 @@ Remove use of Virtus since this has been removed from Grape. optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES, desc: 'The status of the runners to show' - optional :tag_list, type: Array[String], desc: 'The tags of the runners to show' ++ optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The tags of the runners to show' + use :pagination + end + get ':id/runners' do +@@ -208,7 +208,7 @@ + desc: 'The type of the runners to show' + optional :status, type: String, values: Ci::Runner::AVAILABLE_STATUSES, + desc: 'The status of the runners to show' +- optional :tag_list, type: Array[String], desc: 'The tags of the runners to show' + optional :tag_list, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The tags of the runners to show' use :pagination end @@ -1328,6 +1497,40 @@ Remove use of Virtus since this has been removed from Grape. before { authenticated_as_admin! } helpers Helpers::SettingsHelpers +@@ -49,7 +49,7 @@ + optional :default_project_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default project visibility' + optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' + optional :default_snippet_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default snippet visibility' +- optional :disabled_oauth_sign_in_sources, type: Array[String], desc: 'Disable certain OAuth sign-in sources' ++ optional :disabled_oauth_sign_in_sources, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Disable certain OAuth sign-in sources' + optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups' + optional :domain_blacklist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' + optional :domain_whitelist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' +@@ -79,7 +79,8 @@ + requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." + end + optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' +- optional :import_sources, type: Array[String], values: %w[github bitbucket bitbucket_server gitlab google_code fogbugz git gitlab_project gitea manifest phabricator], ++ optional :import_sources, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, ++ values: %w[github bitbucket bitbucket_server gitlab google_code fogbugz git gitlab_project gitea manifest phabricator], + desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com' + optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts" + optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB' +@@ -121,12 +122,12 @@ + requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' + end + optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." +- optional :repository_storages, type: Array[String], desc: 'Storage paths for new projects' ++ optional :repository_storages, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Storage paths for new projects' + optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to set up Two-factor authentication' + given require_two_factor_authentication: ->(val) { val } do + requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' + end +- optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' ++ optional :restricted_visibility_levels, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' + optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up' + optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.' + optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects' --- a/lib/api/sidekiq_metrics.rb +++ b/lib/api/sidekiq_metrics.rb @@ -3,7 +3,7 @@ @@ -1427,6 +1630,17 @@ Remove use of Virtus since this has been removed from Grape. include PaginationParams GLOBAL_TEMPLATE_TYPES = { +--- a/lib/api/terraform/state.rb ++++ b/lib/api/terraform/state.rb +@@ -4,7 +4,7 @@ + + module API + module Terraform +- class State < Grape::API ++ class State < Grape::API::Instance + include ::Gitlab::Utils::StrongMemoize + + default_format :json --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -1,7 +1,7 @@ @@ -1471,15 +1685,80 @@ Remove use of Virtus since this has been removed from Grape. include PaginationParams include APIGuard include Helpers::CustomAttributes -@@ -51,7 +51,7 @@ - optional :can_create_group, type: Boolean, desc: 'Flag indicating the user can create groups' - optional :external, type: Boolean, desc: 'Flag indicating the user is an external user' - # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 -- optional :avatar, type: File, desc: 'Avatar image for user' # rubocop:disable Scalability/FileUploads -+ optional :avatar, types: [File, ::API::Validations::Types::WorkhorseFile], desc: 'Avatar image for user' # rubocop:disable Scalability/FileUploads - optional :theme_id, type: Integer, desc: 'The GitLab theme for the user' - optional :color_scheme_id, type: Integer, desc: 'The color scheme for the file viewer' - optional :private_profile, type: Boolean, desc: 'Flag indicating the user has a private profile' +--- a/lib/api/validations/types/comma_separated_to_array.rb ++++ b/lib/api/validations/types/comma_separated_to_array.rb +@@ -10,7 +10,7 @@ + when String + value.split(',').map(&:strip) + when Array +- value.map { |v| v.to_s.split(',').map(&:strip) }.flatten ++ value.flat_map { |v| v.to_s.split(',').map(&:strip) } + else + [] + end +--- /dev/null ++++ b/lib/api/validations/types/comma_separated_to_integer_array.rb +@@ -0,0 +1,15 @@ ++# frozen_string_literal: true ++ ++module API ++ module Validations ++ module Types ++ class CommaSeparatedToIntegerArray < CommaSeparatedToArray ++ def self.coerce ++ lambda do |value| ++ super.call(value).map(&:to_i) ++ end ++ end ++ end ++ end ++ end ++end +--- a/lib/api/validations/types/labels_list.rb ++++ /dev/null +@@ -1,24 +0,0 @@ +-# frozen_string_literal: true +- +-module API +- module Validations +- module Types +- class LabelsList +- def self.coerce +- lambda do |value| +- case value +- when String +- value.split(',').map(&:strip) +- when Array +- value.flat_map { |v| v.to_s.split(',').map(&:strip) } +- when LabelsList +- value +- else +- [] +- end +- end +- end +- end +- end +- end +-end +--- a/lib/api/validations/types/safe_file.rb ++++ /dev/null +@@ -1,15 +0,0 @@ +-# frozen_string_literal: true +- +-# This module overrides the Grape type validator defined in +-# https://github.com/ruby-grape/grape/blob/master/lib/grape/validations/types/file.rb +-module API +- module Validations +- module Types +- class SafeFile < ::Grape::Validations::Types::File +- def value_coerced?(value) +- super && value[:tempfile].is_a?(Tempfile) +- end +- end +- end +- end +-end --- a/lib/api/validations/types/workhorse_file.rb +++ b/lib/api/validations/types/workhorse_file.rb @@ -3,15 +3,14 @@ @@ -1546,6 +1825,25 @@ Remove use of Virtus since this has been removed from Grape. optional :branch, type: String, desc: 'The name of the branch' end post ":id/wikis/attachments" do +--- a/spec/requests/api/settings_spec.rb ++++ b/spec/requests/api/settings_spec.rb +@@ -60,14 +60,14 @@ + default_projects_limit: 3, + default_project_creation: 2, + password_authentication_enabled_for_web: false, +- repository_storages: ['custom'], ++ repository_storages: 'custom', + plantuml_enabled: true, + plantuml_url: 'http://plantuml.example.com', + sourcegraph_enabled: true, + sourcegraph_url: 'https://sourcegraph.com', + sourcegraph_public_only: false, + default_snippet_visibility: 'internal', +- restricted_visibility_levels: ['public'], ++ restricted_visibility_levels: 'public', + default_artifacts_expire_in: '2 days', + help_page_text: 'custom help text', + help_page_hide_commercial_content: true, --- a/spec/rubocop/cop/code_reuse/worker_spec.rb +++ b/spec/rubocop/cop/code_reuse/worker_spec.rb @@ -31,7 +31,7 @@ @@ -1557,119 +1855,256 @@ Remove use of Virtus since this has been removed from Grape. resource :projects do get '/' do FooWorker.perform_async ---- a/lib/api/api_guard.rb -+++ b/lib/api/api_guard.rb -@@ -144,7 +144,16 @@ - { scope: e.scopes }) - end +--- a/.rubocop.yml ++++ b/.rubocop.yml +@@ -274,6 +274,18 @@ + - 'spec/**/*' + - 'ee/spec/**/*' -- response.finish -+ finished_response = nil -+ response.finish do |rack_response| -+ # Grape expects a Rack::Response -+ # (https://github.com/ruby-grape/grape/commit/c117bff7d22971675f4b34367d3a98bc31c8fc02), -+ # and we need to retrieve it here: -+ # https://github.com/nov/rack-oauth2/blob/40c9a99fd80486ccb8de0e4869ae384547c0d703/lib/rack/oauth2/server/abstract/error.rb#L28 -+ finished_response = rack_response -+ end ++API/GrapeAPIInstance: ++ Enabled: true ++ Include: ++ - 'lib/**/api/**/*.rb' ++ - 'ee/**/api/**/*.rb' + -+ finished_response - end - end - end ---- a/lib/api/validations/types/safe_file.rb -+++ /dev/null -@@ -1,15 +0,0 @@ --# frozen_string_literal: true -- --# This module overrides the Grape type validator defined in --# https://github.com/ruby-grape/grape/blob/master/lib/grape/validations/types/file.rb --module API -- module Validations -- module Types -- class SafeFile < ::Grape::Validations::Types::File -- def value_coerced?(value) -- super && value[:tempfile].is_a?(Tempfile) -- end -- end -- end -- end --end ---- a/lib/api/milestone_responses.rb -+++ b/lib/api/milestone_responses.rb -@@ -15,7 +15,7 @@ - params :list_params do - optional :state, type: String, values: %w[active closed all], default: 'all', - desc: 'Return "active", "closed", or "all" milestones' -- optional :iids, type: Array[Integer], desc: 'The IIDs of the milestones' -+ optional :iids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The IIDs of the milestones' - optional :title, type: String, desc: 'The title of the milestones' - optional :search, type: String, desc: 'The search criteria for the title or description of the milestone' - use :pagination ++API/GrapeArrayMissingCoerce: ++ Enabled: true ++ Include: ++ - 'lib/**/api/**/*.rb' ++ - 'ee/**/api/**/*.rb' ++ + Cop/SidekiqOptionsQueue: + Enabled: true + Exclude: --- /dev/null -+++ b/lib/api/validations/types/comma_separated_to_integer_array.rb -@@ -0,0 +1,15 @@ ++++ b/rubocop/cop/api/grape_api_instance.rb +@@ -0,0 +1,42 @@ +# frozen_string_literal: true + -+module API -+ module Validations -+ module Types -+ class CommaSeparatedToIntegerArray < CommaSeparatedToArray -+ def self.coerce -+ lambda do |value| -+ super.call(value).map(&:to_i) ++module RuboCop ++ module Cop ++ module API ++ class GrapeAPIInstance < RuboCop::Cop::Cop ++ # This cop checks that APIs subclass Grape::API::Instance with Grape v1.3+. ++ # ++ # @example ++ # ++ # # bad ++ # module API ++ # class Projects < Grape::API ++ # end ++ # end ++ # ++ # # good ++ # module API ++ # class Projects < Grape::API::Instance ++ # end ++ # end ++ MSG = 'Inherit from Grape::API::Instance instead of Grape::API. ' \ ++ 'For more details check the https://gitlab.com/gitlab-org/gitlab/-/issues/215230.' ++ ++ def_node_matcher :grape_api_definition, <<~PATTERN ++ (class ++ (const _ _) ++ (const ++ (const nil? :Grape) :API) ++ ... ++ ) ++ PATTERN ++ ++ def on_class(node) ++ grape_api_definition(node) do ++ add_offense(node.children[1]) + end + end + end + end + end +end ---- a/lib/api/helpers/projects_helpers.rb -+++ b/lib/api/helpers/projects_helpers.rb -@@ -45,7 +45,7 @@ - optional :only_allow_merge_if_all_discussions_are_resolved, type: Boolean, desc: 'Only allow to merge if all discussions are resolved' - optional :tag_list, type: Array[String], desc: 'The list of tags for a project' - # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 -- optional :avatar, type: File, desc: 'Avatar image for project' # rubocop:disable Scalability/FileUploads -+ optional :avatar, types: [File, ::API::Validations::Types::WorkhorseFile], desc: 'Avatar image for project' # rubocop:disable Scalability/FileUploads - optional :printing_merge_request_link_enabled, type: Boolean, desc: 'Show link to create/view merge request when pushing from the command line' - optional :merge_method, type: String, values: %w(ff rebase_merge merge), desc: 'The merge method used when merging merge requests' - optional :suggestion_commit_message, type: String, desc: 'The commit message used to apply merge request suggestions' ---- a/lib/api/helpers/merge_requests_helpers.rb -+++ b/lib/api/helpers/merge_requests_helpers.rb -@@ -25,7 +25,7 @@ - optional :milestone, type: String, desc: 'Return merge requests for a specific milestone' - optional :labels, - type: Array[String], -- coerce_with: Validations::Types::LabelsList.coerce, -+ coerce_with: Validations::Types::CommaSeparatedToArray.coerce, - desc: 'Comma-separated list of label names' - optional :with_labels_details, type: Boolean, desc: 'Return titles of labels and other details', default: false - optional :created_after, type: DateTime, desc: 'Return merge requests created after the specified time' ---- a/lib/api/validations/types/labels_list.rb -+++ /dev/null -@@ -1,24 +0,0 @@ --# frozen_string_literal: true -- --module API -- module Validations -- module Types -- class LabelsList -- def self.coerce -- lambda do |value| -- case value -- when String -- value.split(',').map(&:strip) -- when Array -- value.flat_map { |v| v.to_s.split(',').map(&:strip) } -- when LabelsList -- value -- else -- [] -- end -- end -- end -- end -- end -- end --end +--- /dev/null ++++ b/rubocop/cop/api/grape_array_missing_coerce.rb +@@ -0,0 +1,83 @@ ++# frozen_string_literal: true ++ ++module RuboCop ++ module Cop ++ module API ++ class GrapeArrayMissingCoerce < RuboCop::Cop::Cop ++ # This cop checks that Grape API parameters using an Array type ++ # implement a coerce_with method: ++ # ++ # https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions ++ # ++ # @example ++ # ++ # # bad ++ # requires :values, type: Array[String] ++ # ++ # # good ++ # requires :values, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce ++ # ++ # end ++ MSG = 'This Grape parameter defines an Array but is missing a coerce_with definition. ' \ ++ 'For more details, see https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions' ++ ++ def_node_matcher :grape_api_instance?, <<~PATTERN ++ (class ++ (const _ _) ++ (const ++ (const ++ (const nil? :Grape) :API) :Instance) ++ ... ++ ) ++ PATTERN ++ ++ def_node_matcher :grape_api_param_block?, <<~PATTERN ++ (send _ {:requires :optional} ++ (sym _) ++ $_) ++ PATTERN ++ ++ def_node_matcher :grape_type_def?, <<~PATTERN ++ (sym :type) ++ PATTERN ++ ++ def_node_matcher :grape_array_type?, <<~PATTERN ++ (send ++ (const nil? :Array) :[] ++ (const nil? _)) ++ PATTERN ++ ++ def_node_matcher :grape_coerce_with?, <<~PATTERN ++ (sym :coerce_with) ++ PATTERN ++ ++ def on_class(node) ++ @grape_api ||= grape_api_instance?(node) ++ end ++ ++ def on_send(node) ++ return unless @grape_api ++ ++ match = grape_api_param_block?(node) ++ ++ return unless match.is_a?(RuboCop::AST::HashNode) ++ ++ is_array_type = false ++ has_coerce_method = false ++ ++ match.each_pair do |first, second| ++ has_coerce_method ||= grape_coerce_with?(first) ++ ++ if grape_type_def?(first) && grape_array_type?(second) ++ is_array_type = true ++ end ++ end ++ ++ if is_array_type && !has_coerce_method ++ add_offense(node) ++ end ++ end ++ end ++ end ++ end ++end +--- /dev/null ++++ b/spec/rubocop/cop/api/grape_api_instance_spec.rb +@@ -0,0 +1,31 @@ ++# frozen_string_literal: true ++ ++require 'fast_spec_helper' ++require 'rubocop' ++require_relative '../../../support/helpers/expect_offense' ++require_relative '../../../../rubocop/cop/api/grape_api_instance' ++ ++describe RuboCop::Cop::API::GrapeAPIInstance do ++ include CopHelper ++ include ExpectOffense ++ ++ subject(:cop) { described_class.new } ++ ++ it 'adds an offense when inheriting from Grape::API' do ++ inspect_source(<<~CODE.strip_indent) ++ class SomeAPI < Grape::API ++ end ++ CODE ++ ++ expect(cop.offenses.size).to eq(1) ++ end ++ ++ it 'adds an offense when inheriting from Grape::API::Instance' do ++ inspect_source(<<~CODE.strip_indent) ++ class SomeAPI < Grape::API::Instance ++ end ++ CODE ++ ++ expect(cop.offenses.size).to be_zero ++ end ++end +--- /dev/null ++++ b/spec/rubocop/cop/api/grape_array_missing_coerce_spec.rb +@@ -0,0 +1,64 @@ ++# frozen_string_literal: true ++ ++require 'fast_spec_helper' ++require 'rubocop' ++require_relative '../../../support/helpers/expect_offense' ++require_relative '../../../../rubocop/cop/api/grape_array_missing_coerce' ++ ++describe RuboCop::Cop::API::GrapeArrayMissingCoerce do ++ include CopHelper ++ include ExpectOffense ++ ++ subject(:cop) { described_class.new } ++ ++ it 'adds an offense with a required parameter' do ++ inspect_source(<<~CODE.strip_indent) ++ class SomeAPI < Grape::API::Instance ++ params do ++ requires :values, type: Array[String] ++ end ++ end ++ CODE ++ ++ expect(cop.offenses.size).to eq(1) ++ end ++ ++ it 'adds an offense with an optional parameter' do ++ inspect_source(<<~CODE.strip_indent) ++ class SomeAPI < Grape::API::Instance ++ params do ++ optional :values, type: Array[String] ++ end ++ end ++ CODE ++ ++ expect(cop.offenses.size).to eq(1) ++ end ++ ++ it 'does not add an offense' do ++ inspect_source(<<~CODE.strip_indent) ++ class SomeAPI < Grape::API::Instance ++ params do ++ requires :values, type: Array[String], coerce_with: ->(val) { val.split(',').map(&:strip) } ++ requires :milestone, type: String, desc: 'Milestone title' ++ optional :assignee_id, types: [Integer, String], integer_none_any: true, ++ desc: 'Return issues which are assigned to the user with the given ID' ++ end ++ end ++ CODE ++ ++ expect(cop.offenses.size).to be_zero ++ end ++ ++ it 'does not add an offense for unrelated classes' do ++ inspect_source(<<~CODE.strip_indent) ++ class SomeClass ++ params do ++ requires :values, type: Array[String] ++ end ++ end ++ CODE ++ ++ expect(cop.offenses.size).to be_zero ++ end ++end diff --git a/debian/patches/0790-protobuf-compat.patch b/debian/patches/0790-protobuf-compat.patch index 000ca0e3dc..06f5c441fd 100644 --- a/debian/patches/0790-protobuf-compat.patch +++ b/debian/patches/0790-protobuf-compat.patch @@ -9,7 +9,7 @@ Subject: [PATCH 2/2] Fix protobuf compatibility (Thanks to Stan Hu) --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb -@@ -110,7 +110,7 @@ +@@ -117,7 +117,7 @@ return unless %w[git-receive-pack git-upload-pack git-upload-archive].include?(action) { diff --git a/debian/patches/series b/debian/patches/series index 636600e0b6..acae3ae0ce 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -6,7 +6,6 @@ 0350-relax-method-source.patch 0430-remove-gitlab-markup.patch 0440-remove-puma.patch -0460-embed-derailed-benchmarks.patch 0480-embed-elasticsearch-model.patch 0480-embed-elasticsearch-rails.patch 0480-embed-faraday-middleware-aws-signers-v4.patch