diff options
24 files changed, 384 insertions, 92 deletions
diff --git a/app/helpers/timeboxes_helper.rb b/app/helpers/timeboxes_helper.rb index 39993bbfb44..11d09a79dcf 100644 --- a/app/helpers/timeboxes_helper.rb +++ b/app/helpers/timeboxes_helper.rb @@ -172,18 +172,19 @@ module TimeboxesHelper def timebox_date_range(timebox) if timebox.start_date && timebox.due_date - "#{timebox.start_date.to_s(:medium)}–#{timebox.due_date.to_s(:medium)}" + s_("DateRange|%{start_date}–%{end_date}") % { start_date: l(timebox.start_date, format: Date::DATE_FORMATS[:medium]), + end_date: l(timebox.due_date, format: Date::DATE_FORMATS[:medium]) } elsif timebox.due_date if timebox.due_date.past? - _("expired on %{timebox_due_date}") % { timebox_due_date: timebox.due_date.to_s(:medium) } + _("expired on %{timebox_due_date}") % { timebox_due_date: l(timebox.due_date, format: Date::DATE_FORMATS[:medium]) } else - _("expires on %{timebox_due_date}") % { timebox_due_date: timebox.due_date.to_s(:medium) } + _("expires on %{timebox_due_date}") % { timebox_due_date: l(timebox.due_date, format: Date::DATE_FORMATS[:medium]) } end elsif timebox.start_date if timebox.start_date.past? - _("started on %{timebox_start_date}") % { timebox_start_date: timebox.start_date.to_s(:medium) } + _("started on %{timebox_start_date}") % { timebox_start_date: l(timebox.start_date, format: Date::DATE_FORMATS[:medium]) } else - _("starts on %{timebox_start_date}") % { timebox_start_date: timebox.start_date.to_s(:medium) } + _("starts on %{timebox_start_date}") % { timebox_start_date: l(timebox.start_date, format: Date::DATE_FORMATS[:medium]) } end end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b332d702c0a..8f6ee62f16d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -67,6 +67,8 @@ class MergeRequest < ApplicationRecord has_one :merge_head_diff, -> { merge_head }, inverse_of: :merge_request, class_name: 'MergeRequestDiff' has_one :cleanup_schedule, inverse_of: :merge_request + has_one :predictions, inverse_of: :merge_request + delegate :suggested_reviewers, to: :predictions belongs_to :latest_merge_request_diff, class_name: 'MergeRequestDiff' manual_inverse_association :latest_merge_request_diff, :merge_request diff --git a/app/models/merge_request/predictions.rb b/app/models/merge_request/predictions.rb new file mode 100644 index 00000000000..ef9e00b5f74 --- /dev/null +++ b/app/models/merge_request/predictions.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class MergeRequest::Predictions < ApplicationRecord # rubocop:disable Style/ClassAndModuleChildren + belongs_to :merge_request, inverse_of: :predictions + + validates :suggested_reviewers, json_schema: { filename: 'merge_request_predictions_suggested_reviewers' } +end diff --git a/app/validators/json_schemas/merge_request_predictions_suggested_reviewers.json b/app/validators/json_schemas/merge_request_predictions_suggested_reviewers.json new file mode 100644 index 00000000000..70112d7e414 --- /dev/null +++ b/app/validators/json_schemas/merge_request_predictions_suggested_reviewers.json @@ -0,0 +1,10 @@ +{ + "description": "Merge request predictions suggested reviewers", + "type": "object", + "properties": { + "top_n": { "type": "number" }, + "version": { "type": "string" }, + "changes": { "type": "array" } + }, + "additionalProperties": true +} diff --git a/config/feature_flags/development/maven_central_request_forwarding.yml b/config/feature_flags/development/maven_central_request_forwarding.yml new file mode 100644 index 00000000000..756a931b3a1 --- /dev/null +++ b/config/feature_flags/development/maven_central_request_forwarding.yml @@ -0,0 +1,8 @@ +--- +name: maven_central_request_forwarding +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85299 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/359553 +milestone: '15.4' +type: development +group: group::package +default_enabled: false diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 40f3b6a2abd..4527efe5a1c 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -299,6 +299,8 @@ - 1 - - merge_requests_execute_approval_hooks - 1 +- - merge_requests_fetch_suggested_reviewers + - 1 - - merge_requests_handle_assignees_change - 1 - - merge_requests_resolve_todos diff --git a/db/docs/merge_request_predictions.yml b/db/docs/merge_request_predictions.yml new file mode 100644 index 00000000000..7495f0934a4 --- /dev/null +++ b/db/docs/merge_request_predictions.yml @@ -0,0 +1,9 @@ +--- +table_name: merge_request_predictions +classes: +- MergeRequest::Prediction +feature_categories: +- workflow_automation +description: Includes machine learning model predictions +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97622 +milestone: '15.4' diff --git a/db/migrate/20220406193806_add_maven_package_requests_forwarding_to_application_settings.rb b/db/migrate/20220406193806_add_maven_package_requests_forwarding_to_application_settings.rb new file mode 100644 index 00000000000..60b2efd3e9c --- /dev/null +++ b/db/migrate/20220406193806_add_maven_package_requests_forwarding_to_application_settings.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddMavenPackageRequestsForwardingToApplicationSettings < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def up + add_column(:application_settings, :maven_package_requests_forwarding, :boolean, default: true, null: false) + end + + def down + remove_column(:application_settings, :maven_package_requests_forwarding) + end +end diff --git a/db/migrate/20220915140802_create_merge_request_predictions.rb b/db/migrate/20220915140802_create_merge_request_predictions.rb new file mode 100644 index 00000000000..20cd7e58092 --- /dev/null +++ b/db/migrate/20220915140802_create_merge_request_predictions.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class CreateMergeRequestPredictions < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def up + create_table :merge_request_predictions, id: false do |t| + t.references :merge_request, + primary_key: true, null: false, type: :bigint, + index: false, foreign_key: { on_delete: :cascade } + + t.timestamps_with_timezone null: false + t.jsonb :suggested_reviewers, null: false, default: {} + end + end + + def down + drop_table :merge_request_predictions + end +end diff --git a/db/schema_migrations/20220406193806 b/db/schema_migrations/20220406193806 new file mode 100644 index 00000000000..a5dfed18303 --- /dev/null +++ b/db/schema_migrations/20220406193806 @@ -0,0 +1 @@ +f2ed979f3af7aec03defc737add2e5d5bf4aad080d501003744ee42f902074d5
\ No newline at end of file diff --git a/db/schema_migrations/20220915140802 b/db/schema_migrations/20220915140802 new file mode 100644 index 00000000000..676e295aa14 --- /dev/null +++ b/db/schema_migrations/20220915140802 @@ -0,0 +1 @@ +9b0f19a59e104f0df6abac7d58012701dcf9a031116f5cc643e407506e186cc2
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 076030ee7f8..7cece2e2828 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11468,6 +11468,7 @@ CREATE TABLE application_settings ( inactive_projects_min_size_mb integer DEFAULT 0 NOT NULL, inactive_projects_send_warning_email_after_months integer DEFAULT 1 NOT NULL, delayed_group_deletion boolean DEFAULT true NOT NULL, + maven_package_requests_forwarding boolean DEFAULT true NOT NULL, arkose_labs_namespace text DEFAULT 'client'::text NOT NULL, max_export_size integer DEFAULT 0, encrypted_slack_app_signing_secret bytea, @@ -17491,6 +17492,22 @@ CREATE SEQUENCE merge_request_metrics_id_seq ALTER SEQUENCE merge_request_metrics_id_seq OWNED BY merge_request_metrics.id; +CREATE TABLE merge_request_predictions ( + merge_request_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + suggested_reviewers jsonb DEFAULT '{}'::jsonb NOT NULL +); + +CREATE SEQUENCE merge_request_predictions_merge_request_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_request_predictions_merge_request_id_seq OWNED BY merge_request_predictions.merge_request_id; + CREATE TABLE merge_request_reviewers ( id bigint NOT NULL, user_id bigint NOT NULL, @@ -23730,6 +23747,8 @@ ALTER TABLE ONLY merge_request_diffs ALTER COLUMN id SET DEFAULT nextval('merge_ ALTER TABLE ONLY merge_request_metrics ALTER COLUMN id SET DEFAULT nextval('merge_request_metrics_id_seq'::regclass); +ALTER TABLE ONLY merge_request_predictions ALTER COLUMN merge_request_id SET DEFAULT nextval('merge_request_predictions_merge_request_id_seq'::regclass); + ALTER TABLE ONLY merge_request_reviewers ALTER COLUMN id SET DEFAULT nextval('merge_request_reviewers_id_seq'::regclass); ALTER TABLE ONLY merge_request_user_mentions ALTER COLUMN id SET DEFAULT nextval('merge_request_user_mentions_id_seq'::regclass); @@ -25753,6 +25772,9 @@ ALTER TABLE ONLY merge_request_diffs ALTER TABLE ONLY merge_request_metrics ADD CONSTRAINT merge_request_metrics_pkey PRIMARY KEY (id); +ALTER TABLE ONLY merge_request_predictions + ADD CONSTRAINT merge_request_predictions_pkey PRIMARY KEY (merge_request_id); + ALTER TABLE ONLY merge_request_reviewers ADD CONSTRAINT merge_request_reviewers_pkey PRIMARY KEY (id); @@ -34347,6 +34369,9 @@ ALTER TABLE ONLY issues_prometheus_alert_events ALTER TABLE ONLY merge_trains ADD CONSTRAINT fk_rails_b374b5225d FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_request_predictions + ADD CONSTRAINT fk_rails_b3b78cbcd0 FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; + ALTER TABLE ONLY incident_management_escalation_rules ADD CONSTRAINT fk_rails_b3c9c17bd4 FOREIGN KEY (oncall_schedule_id) REFERENCES incident_management_oncall_schedules(id) ON DELETE CASCADE; diff --git a/doc/api/settings.md b/doc/api/settings.md index c3578f53c32..467fa6bfc37 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -89,6 +89,7 @@ Example response: "asset_proxy_url": "https://assets.example.com", "asset_proxy_whitelist": ["example.com", "*.example.com", "your-instance.com"], "asset_proxy_allowlist": ["example.com", "*.example.com", "your-instance.com"], + "maven_package_requests_forwarding": true, "npm_package_requests_forwarding": true, "pypi_package_requests_forwarding": true, "snippet_size_limit": 52428800, @@ -201,6 +202,7 @@ Example response: "allow_local_requests_from_hooks_and_services": true, "allow_local_requests_from_web_hooks_and_services": true, "allow_local_requests_from_system_hooks": false, + "maven_package_requests_forwarding": true, "npm_package_requests_forwarding": true, "pypi_package_requests_forwarding": true, "snippet_size_limit": 52428800, @@ -390,6 +392,7 @@ listed in the descriptions of the relevant settings. | `mirror_capacity_threshold` **(PREMIUM)** | integer | no | Minimum capacity to be available before scheduling more mirrors preemptively. | | `mirror_max_capacity` **(PREMIUM)** | integer | no | Maximum number of mirrors that can be synchronizing at the same time. | | `mirror_max_delay` **(PREMIUM)** | integer | no | Maximum time (in minutes) between updates that a mirror can have when scheduled to synchronize. | +| `maven_package_requests_forwarding` **(PREMIUM)** | boolean | no | Use repo.maven.apache.org as a default remote repository when the package is not found in the GitLab Package Registry for Maven. | | `npm_package_requests_forwarding` **(PREMIUM)** | boolean | no | Use npmjs.org as a default remote repository when the package is not found in the GitLab Package Registry for npm. | | `pypi_package_requests_forwarding` **(PREMIUM)** | boolean | no | Use pypi.org as a default remote repository when the package is not found in the GitLab Package Registry for PyPI. | | `outbound_local_requests_whitelist` | array of strings | no | Define a list of trusted domains or IP addresses to which local requests are allowed when local requests for hooks and services are disabled. diff --git a/doc/user/admin_area/settings/continuous_integration.md b/doc/user/admin_area/settings/continuous_integration.md index 9e711b8f5e2..dab3c78d9d1 100644 --- a/doc/user/admin_area/settings/continuous_integration.md +++ b/doc/user/admin_area/settings/continuous_integration.md @@ -275,6 +275,18 @@ To select a CI/CD template for the required pipeline configuration: ## Package Registry configuration +### Maven Forwarding **(PREMIUM SELF)** + +GitLab administrators can disable the forwarding of Maven requests to [Maven Central](https://search.maven.org/). + +To disable forwarding Maven requests: + +1. On the top bar, select **Menu > Admin**. +1. On the left sidebar, select **Settings > CI/CD**. +1. Expand the **Package Registry** section. +1. Clear the checkbox **Forward Maven package requests to the Maven Registry if the packages are not found in the GitLab Package Registry**. +1. Select **Save changes**. + ### npm Forwarding **(PREMIUM SELF)** GitLab administrators can disable the forwarding of npm requests to [npmjs.com](https://www.npmjs.com/). diff --git a/doc/user/packages/maven_repository/index.md b/doc/user/packages/maven_repository/index.md index eae0c886013..957374245d2 100644 --- a/doc/user/packages/maven_repository/index.md +++ b/doc/user/packages/maven_repository/index.md @@ -703,6 +703,67 @@ dependencies { } ``` +### Request forwarding to Maven Central + +> [Introduced](<https://gitlab.com/gitlab-org/gitlab/-/issues/362657>) behind a [feature flag](../../feature_flags.md), disabled by default in GitLab 15.4 + +FLAG: +On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `maven_central_request_forwarding`. +On GitLab.com, this feature is not available. + +When a Maven package is not found in the Package Registry, the request is forwarded +to [Maven Central](https://search.maven.org/). + +When the feature flag is enabled, administrators can disable this behavior in the +[Continuous Integration settings](../../admin_area/settings/continuous_integration.md). + +There are many ways to configure your Maven project so that it will request packages +in Maven Central from GitLab. Maven repositories are queried in a +[specific order](https://maven.apache.org/guides/mini/guide-multiple-repositories.html#repository-order). +By default, maven-central is usually checked first through the +[Super POM](https://maven.apache.org/guides/introduction/introduction-to-the-pom.html#Super_POM), so +GitLab needs to be configured to be queried before maven-central. + +[Using GitLab as a mirror of the central proxy](#setting-gitlab-as-a-mirror-for-the-central-proxy) is one +way to force GitLab to be queried in place of maven-central. + +Maven forwarding is restricted to only the [project level](#project-level-maven-endpoint) and +[group level](#group-level-maven-endpoint) endpoints. The [instance level endpoint](#instance-level-maven-endpoint) +has naming restrictions that prevent it from being used for packages that don't follow that convention and also +introduces too much security risk for supply-chain style attacks. + +#### Setting GitLab as a mirror for the central proxy + +To ensure all package requests are sent to GitLab instead of Maven Central, +you can override Maven Central as the central repository by adding a `<mirror>` +section to your `settings.xml`: + +```xml +<settings> + <servers> + <server> + <id>central-proxy</id> + <configuration> + <httpHeaders> + <property> + <name>Private-Token</name> + <value>{personal_access_token}</value> + </property> + </httpHeaders> + </configuration> + </server> + </servers> + <mirrors> + <mirror> + <id>central-proxy</id> + <name>GitLab proxy of central repo</name> + <url>https://gitlab.example.com/api/v4/projects/{project_id}/packages/maven</url> + <mirrorOf>central</mirrorOf> + </mirror> + </mirrors> +</settings> +``` + ## Remove a package For your project, go to **Packages and registries > Package Registry**. diff --git a/lib/api/helpers/packages/dependency_proxy_helpers.rb b/lib/api/helpers/packages/dependency_proxy_helpers.rb index b8ae1dddd7e..a09499e00d7 100644 --- a/lib/api/helpers/packages/dependency_proxy_helpers.rb +++ b/lib/api/helpers/packages/dependency_proxy_helpers.rb @@ -6,16 +6,18 @@ module API module DependencyProxyHelpers REGISTRY_BASE_URLS = { npm: 'https://registry.npmjs.org/', - pypi: 'https://pypi.org/simple/' + pypi: 'https://pypi.org/simple/', + maven: 'https://repo.maven.apache.org/maven2/' }.freeze APPLICATION_SETTING_NAMES = { npm: 'npm_package_requests_forwarding', - pypi: 'pypi_package_requests_forwarding' + pypi: 'pypi_package_requests_forwarding', + maven: 'maven_package_requests_forwarding' }.freeze def redirect_registry_request(forward_to_registry, package_type, options) - if forward_to_registry && redirect_registry_request_available?(package_type) + if forward_to_registry && redirect_registry_request_available?(package_type) && maven_forwarding_ff_enabled?(package_type, options[:target]) ::Gitlab::Tracking.event(self.options[:for].name, "#{package_type}_request_forward") redirect(registry_url(package_type, options)) else @@ -33,6 +35,8 @@ module API "#{base_url}#{options[:package_name]}" when :pypi "#{base_url}#{options[:package_name]}/" + when :maven + "#{base_url}#{options[:path]}/#{options[:file_name]}" end end @@ -46,6 +50,16 @@ module API .attributes .fetch(application_setting_name, false) end + + private + + def maven_forwarding_ff_enabled?(package_type, target) + return true unless package_type == :maven + return true if Feature.enabled?(:maven_central_request_forwarding) + return false unless target + + Feature.enabled?(:maven_central_request_forwarding, target.root_ancestor) + end end end end diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index e38c88d7424..a3a25ec1696 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -22,6 +22,7 @@ module API end helpers ::API::Helpers::PackagesHelpers + helpers ::API::Helpers::Packages::DependencyProxyHelpers helpers do def path_exists?(path) @@ -113,7 +114,31 @@ module API project || group, path: params[:path], order_by_package_file: order_by_package_file - ).execute! + ).execute + end + + def find_and_present_package_file(package, file_name, format, params) + project = package&.project + package_file = nil + + package_file = ::Packages::PackageFileFinder.new(package, file_name).execute if package + + no_package_found = package_file ? false : true + + redirect_registry_request(no_package_found, :maven, path: params[:path], file_name: params[:file_name], target: params[:target]) do + not_found!('Package') if no_package_found + + case format + when 'md5' + package_file.file_md5 + when 'sha1' + package_file.file_sha1 + else + track_package_event('pull_package', :maven, project: project, namespace: project&.namespace) if jar_file?(format) + + present_carrierwave_file_with_head_support!(package_file) + end + end end end @@ -141,6 +166,8 @@ module API package = fetch_package(file_name: file_name, project: project) + not_found!('Package') unless package + package_file = ::Packages::PackageFileFinder .new(package, file_name).execute! @@ -169,31 +196,20 @@ module API route_setting :authentication, job_token_allowed: true, deploy_token_allowed: true get ':id/-/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do # return a similar failure to group = find_group(params[:id]) - not_found!('Group') unless path_exists?(params[:path]) - - file_name, format = extract_format(params[:file_name]) - group = find_group(params[:id]) + if Feature.disabled?(:maven_central_request_forwarding, group&.root_ancestor) + not_found!('Group') unless path_exists?(params[:path]) + end + not_found!('Group') unless can?(current_user, :read_group, group) + file_name, format = extract_format(params[:file_name]) package = fetch_package(file_name: file_name, group: group) - authorize_read_package!(package.project) + authorize_read_package!(package.project) if package - package_file = ::Packages::PackageFileFinder - .new(package, file_name).execute! - - case format - when 'md5' - package_file.file_md5 - when 'sha1' - package_file.file_sha1 - else - track_package_event('pull_package', :maven, project: package.project, namespace: package.project.namespace) if jar_file?(format) - - present_carrierwave_file_with_head_support!(package_file) - end + find_and_present_package_file(package, file_name, format, params.merge(target: group)) end end @@ -211,7 +227,9 @@ module API route_setting :authentication, job_token_allowed: true, deploy_token_allowed: true get ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do # return a similar failure to user_project - not_found!('Project') unless path_exists?(params[:path]) + unless Feature.enabled?(:maven_central_request_forwarding, user_project&.root_ancestor) + not_found!('Project') unless path_exists?(params[:path]) + end authorize_read_package!(user_project) @@ -219,19 +237,7 @@ module API package = fetch_package(file_name: file_name, project: user_project) - package_file = ::Packages::PackageFileFinder - .new(package, file_name).execute! - - case format - when 'md5' - package_file.file_md5 - when 'sha1' - package_file.file_sha1 - else - track_package_event('pull_package', :maven, project: user_project, namespace: user_project.namespace) if jar_file?(format) - - present_carrierwave_file_with_head_support!(package_file) - end + find_and_present_package_file(package, file_name, format, params.merge(target: user_project)) end desc 'Workhorse authorize the maven package file upload' do diff --git a/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_with_corrected_regex.rb b/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_with_corrected_regex.rb index b9151343d6a..2d64b7378be 100644 --- a/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_with_corrected_regex.rb +++ b/lib/gitlab/background_migration/backfill_draft_status_on_merge_requests_with_corrected_regex.rb @@ -9,6 +9,7 @@ module Gitlab # Migration only version of MergeRequest table class MergeRequest < ::ApplicationRecord include EachBatch + validates :suggested_reviewers, json_schema: { filename: 'merge_request_suggested_reviewers' } CORRECTED_REGEXP_STR = "^(\\[draft\\]|\\(draft\\)|draft:|draft|\\[WIP\\]|WIP:|WIP)" diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index 6d290a14896..5725d7a4503 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -318,6 +318,7 @@ merge_request_diff_details: :gitlab_main merge_request_diff_files: :gitlab_main merge_request_diffs: :gitlab_main merge_request_metrics: :gitlab_main +merge_request_predictions: :gitlab_main merge_request_reviewers: :gitlab_main merge_requests_closing_issues: :gitlab_main merge_requests: :gitlab_main diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0b8fbbf9605..7d657de9621 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -12390,6 +12390,9 @@ msgstr "" msgid "Date range must be shorter than %{max_range} days." msgstr "" +msgid "DateRange|%{start_date}–%{end_date}" +msgstr "" + msgid "Day of month" msgstr "" diff --git a/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb b/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb index 1aedba63057..aa4b0a137cd 100644 --- a/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb +++ b/spec/lib/api/helpers/packages/dependency_proxy_helpers_spec.rb @@ -8,6 +8,8 @@ RSpec.describe API::Helpers::Packages::DependencyProxyHelpers do describe '#redirect_registry_request' do using RSpec::Parameterized::TableSyntax + let_it_be(:project) { create(:project) } + let(:options) { {} } subject { helper.redirect_registry_request(forward_to_registry, package_type, options) { helper.fallback } } @@ -38,11 +40,12 @@ RSpec.describe API::Helpers::Packages::DependencyProxyHelpers do end end - %i[npm pypi].each do |forwardable_package_type| + %i[maven npm pypi].each do |forwardable_package_type| context "with #{forwardable_package_type} packages" do include_context 'dependency proxy helpers context' let(:package_type) { forwardable_package_type } + let(:options) { { project: project } } where(:application_setting, :forward_to_registry, :example_name) do true | true | 'executing redirect' @@ -59,17 +62,41 @@ RSpec.describe API::Helpers::Packages::DependencyProxyHelpers do it_behaves_like params[:example_name] end end + + context 'when maven_central_request_forwarding is disabled' do + let(:package_type) { :maven } + let(:options) { { project: project } } + + include_context 'dependency proxy helpers context' + + where(:application_setting, :forward_to_registry) do + true | true + true | false + false | true + false | false + end + + with_them do + before do + stub_feature_flags(maven_central_request_forwarding: false) + allow_fetch_application_setting(attribute: "maven_package_requests_forwarding", return_value: application_setting) + end + + it_behaves_like 'executing fallback' + end + end end context 'with non-forwardable package type' do let(:forward_to_registry) { true } before do + stub_application_setting(maven_package_requests_forwarding: true) stub_application_setting(npm_package_requests_forwarding: true) stub_application_setting(pypi_package_requests_forwarding: true) end - Packages::Package.package_types.keys.without('npm', 'pypi').each do |pkg_type| + Packages::Package.package_types.keys.without('maven', 'npm', 'pypi').each do |pkg_type| context "#{pkg_type}" do let(:package_type) { pkg_type.to_sym } @@ -81,18 +108,21 @@ RSpec.describe API::Helpers::Packages::DependencyProxyHelpers do end describe '#registry_url' do - subject { helper.registry_url(package_type, package_name: 'test') } + subject { helper.registry_url(package_type, options) } - where(:package_type, :expected_result) do - :npm | 'https://registry.npmjs.org/test' - :pypi | 'https://pypi.org/simple/test/' + where(:package_type, :expected_result, :params) do + :maven | 'https://repo.maven.apache.org/maven2/test/123' | { path: 'test', file_name: '123', project: project } + :npm | 'https://registry.npmjs.org/test' | { package_name: 'test' } + :pypi | 'https://pypi.org/simple/test/' | { package_name: 'test' } end with_them do + let(:options) { params } + it { is_expected.to eq(expected_result) } end - Packages::Package.package_types.keys.without('npm', 'pypi').each do |pkg_type| + Packages::Package.package_types.keys.without('maven', 'npm', 'pypi').each do |pkg_type| context "with non-forwardable package type #{pkg_type}" do let(:package_type) { pkg_type } diff --git a/spec/lib/gitlab/import/merge_request_creator_spec.rb b/spec/lib/gitlab/import/merge_request_creator_spec.rb index 9aedca40f1b..8f502216294 100644 --- a/spec/lib/gitlab/import/merge_request_creator_spec.rb +++ b/spec/lib/gitlab/import/merge_request_creator_spec.rb @@ -8,10 +8,13 @@ RSpec.describe Gitlab::Import::MergeRequestCreator do subject { described_class.new(project) } describe '#execute' do + let(:attributes) do + HashWithIndifferentAccess.new(merge_request.attributes.except('merge_params', 'suggested_reviewers')) + end + context 'merge request already exists' do let(:merge_request) { create(:merge_request, target_project: project, source_project: project) } let(:commits) { merge_request.merge_request_diffs.first.commits } - let(:attributes) { HashWithIndifferentAccess.new(merge_request.attributes.except("merge_params")) } it 'updates the data' do commits_count = commits.count @@ -31,7 +34,6 @@ RSpec.describe Gitlab::Import::MergeRequestCreator do context 'new merge request' do let(:merge_request) { build(:merge_request, target_project: project, source_project: project) } - let(:attributes) { HashWithIndifferentAccess.new(merge_request.attributes.except("merge_params")) } it 'creates a new merge request' do attributes.delete(:id) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index a9c03fb0f32..e270ca9ec6a 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -212,6 +212,7 @@ merge_requests: - cleanup_schedule - compliance_violations - created_environments +- predictions external_pull_requests: - project merge_request_diff: diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index 33a722793e3..d7cc6991ef4 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' RSpec.describe API::MavenPackages do + using RSpec::Parameterized::TableSyntax include WorkhorseHelpers include_context 'workhorse headers' @@ -40,15 +41,15 @@ RSpec.describe API::MavenPackages do project.add_developer(user) end - shared_examples 'handling groups and subgroups for' do |shared_example_name, visibilities: %i[public]| + shared_examples 'handling groups and subgroups for' do |shared_example_name, visibilities: { public: :redirect }| context 'within a group' do - visibilities.each do |visibility| + visibilities.each do |visibility, not_found_response| context "that is #{visibility}" do before do group.update!(visibility_level: Gitlab::VisibilityLevel.level_value(visibility.to_s)) end - it_behaves_like shared_example_name + it_behaves_like shared_example_name, not_found_response end end end @@ -60,20 +61,20 @@ RSpec.describe API::MavenPackages do move_project_to_namespace(subgroup) end - visibilities.each do |visibility| + visibilities.each do |visibility, not_found_response| context "that is #{visibility}" do before do subgroup.update!(visibility_level: Gitlab::VisibilityLevel.level_value(visibility.to_s)) group.update!(visibility_level: Gitlab::VisibilityLevel.level_value(visibility.to_s)) end - it_behaves_like shared_example_name + it_behaves_like shared_example_name, not_found_response end end end end - shared_examples 'handling groups, subgroups and user namespaces for' do |shared_example_name, visibilities: %i[public]| + shared_examples 'handling groups, subgroups and user namespaces for' do |shared_example_name, visibilities: { public: :redirect }| it_behaves_like 'handling groups and subgroups for', shared_example_name, visibilities: visibilities context 'within a user namespace' do @@ -103,16 +104,6 @@ RSpec.describe API::MavenPackages do end end - shared_examples 'rejecting the request for non existing maven path' do |expected_status: :not_found| - it 'rejects the request' do - expect(::Packages::Maven::PackageFinder).not_to receive(:new) - - subject - - expect(response).to have_gitlab_http_status(expected_status) - end - end - shared_examples 'processing HEAD requests' do |instance_level: false| subject { head api(url) } @@ -162,7 +153,7 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do let(:path) { 'foo/bar/1.2.3' } - it_behaves_like 'rejecting the request for non existing maven path', expected_status: instance_level ? :forbidden : :not_found + it_behaves_like 'returning response status', instance_level ? :forbidden : :redirect end end end @@ -238,6 +229,59 @@ RSpec.describe API::MavenPackages do end end + shared_examples 'forwarding package requests' do + context 'request forwarding' do + include_context 'dependency proxy helpers context' + + subject { download_file(file_name: package_name) } + + shared_examples 'redirecting the request' do + it_behaves_like 'returning response status', :redirect + end + + shared_examples 'package not found' do + it_behaves_like 'returning response status', :not_found + end + + where(:forward, :package_in_project, :shared_examples_name) do + true | true | 'successfully returning the file' + true | false | 'redirecting the request' + false | true | 'successfully returning the file' + false | false | 'package not found' + end + + with_them do + let(:package_name) { package_in_project ? package_file.file_name : 'foo' } + + before do + allow_fetch_application_setting(attribute: 'maven_package_requests_forwarding', return_value: forward) + end + + it_behaves_like params[:shared_examples_name] + end + + context 'with maven_central_request_forwarding disabled' do + where(:forward, :package_in_project, :shared_examples_name) do + true | true | 'successfully returning the file' + true | false | 'package not found' + false | true | 'successfully returning the file' + false | false | 'package not found' + end + + with_them do + let(:package_name) { package_in_project ? package_file.file_name : 'foo' } + + before do + stub_feature_flags(maven_central_request_forwarding: false) + allow_fetch_application_setting(attribute: 'maven_package_requests_forwarding', return_value: forward) + end + + it_behaves_like params[:shared_examples_name] + end + end + end + end + describe 'GET /api/v4/packages/maven/*path/:file_name' do context 'a public project' do subject { download_file(file_name: package_file.file_name) } @@ -259,7 +303,16 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path', expected_status: :forbidden + it_behaves_like 'returning response status', :forbidden + end + + it 'returns not found when a package is not found' do + finder = double('finder', execute: nil) + expect(::Packages::Maven::PackageFinder).to receive(:new).and_return(finder) + + subject + + expect(response).to have_gitlab_http_status(:not_found) end end @@ -291,11 +344,11 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file_with_token(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path', expected_status: :forbidden + it_behaves_like 'returning response status', :forbidden end end - it_behaves_like 'handling groups, subgroups and user namespaces for', 'getting a file', visibilities: %i[public internal] + it_behaves_like 'handling groups, subgroups and user namespaces for', 'getting a file', visibilities: { public: :redirect, internal: :not_found } end context 'private project' do @@ -349,11 +402,11 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file_with_token(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path', expected_status: :forbidden + it_behaves_like 'returning response status', :forbidden end end - it_behaves_like 'handling groups, subgroups and user namespaces for', 'getting a file', visibilities: %i[public internal private] + it_behaves_like 'handling groups, subgroups and user namespaces for', 'getting a file', visibilities: { public: :redirect, internal: :not_found, private: :not_found } end context 'project name is different from a package name' do @@ -408,6 +461,8 @@ RSpec.describe API::MavenPackages do group.add_developer(user) end + it_behaves_like 'forwarding package requests' + context 'a public project' do subject { download_file(file_name: package_file.file_name) } @@ -428,7 +483,7 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path' + it_behaves_like 'returning response status', :redirect end end @@ -443,15 +498,15 @@ RSpec.describe API::MavenPackages do subject { download_file_with_token(file_name: package_file.file_name) } - shared_examples 'getting a file for a group' do + shared_examples 'getting a file for a group' do |not_found_response| it_behaves_like 'tracking the file download event' it_behaves_like 'bumping the package last downloaded at field' it_behaves_like 'successfully returning the file' - it 'denies download when no private token' do + it 'forwards download when no private token' do download_file(file_name: package_file.file_name) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(not_found_response) end it_behaves_like 'downloads with a job token' @@ -460,11 +515,11 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file_with_token(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path' + it_behaves_like 'returning response status', :redirect end end - it_behaves_like 'handling groups and subgroups for', 'getting a file for a group', visibilities: %i[internal public] + it_behaves_like 'handling groups and subgroups for', 'getting a file for a group', visibilities: { internal: :not_found, public: :redirect } end context 'private project' do @@ -474,7 +529,7 @@ RSpec.describe API::MavenPackages do subject { download_file_with_token(file_name: package_file.file_name) } - shared_examples 'getting a file for a group' do + shared_examples 'getting a file for a group' do |not_found_response| it_behaves_like 'tracking the file download event' it_behaves_like 'bumping the package last downloaded at field' it_behaves_like 'successfully returning the file' @@ -484,13 +539,13 @@ RSpec.describe API::MavenPackages do subject - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:redirect) end it 'denies download when no private token' do download_file(file_name: package_file.file_name) - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(not_found_response) end it_behaves_like 'downloads with a job token' @@ -499,7 +554,7 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file_with_token(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path' + it_behaves_like 'returning response status', :redirect end context 'with group deploy token' do @@ -519,12 +574,12 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file_with_token(file_name: package_file.file_name, path: 'foo/bar/1.2.3', request_headers: group_deploy_token_headers) } - it_behaves_like 'rejecting the request for non existing maven path' + it_behaves_like 'returning response status', :redirect end end end - it_behaves_like 'handling groups and subgroups for', 'getting a file for a group', visibilities: %i[private internal public] + it_behaves_like 'handling groups and subgroups for', 'getting a file for a group', visibilities: { private: :not_found, internal: :not_found, public: :redirect } context 'with a reporter from a subgroup accessing the root group' do let_it_be(:root_group) { create(:group, :private) } @@ -542,7 +597,7 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file_with_token(file_name: package_file.file_name, path: 'foo/bar/1.2.3', request_headers: headers_with_token, group_id: root_group.id) } - it_behaves_like 'rejecting the request for non existing maven path' + it_behaves_like 'returning response status', :redirect end end end @@ -638,12 +693,14 @@ RSpec.describe API::MavenPackages do it_behaves_like 'successfully returning the file' it_behaves_like 'file download in FIPS mode' - it 'returns sha1 of the file' do - download_file(file_name: package_file.file_name + '.sha1') + %w[sha1 md5].each do |format| + it "returns #{format} of the file" do + download_file(file_name: package_file.file_name + ".#{format}") - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq('text/plain') - expect(response.body).to eq(package_file.file_sha1) + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type).to eq('text/plain') + expect(response.body).to eq(package_file.send("file_#{format}".to_sym)) + end end context 'when the repository is disabled' do @@ -662,7 +719,7 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path' + it_behaves_like 'returning response status', :redirect end end @@ -697,10 +754,12 @@ RSpec.describe API::MavenPackages do context 'with a non existing maven path' do subject { download_file_with_token(file_name: package_file.file_name, path: 'foo/bar/1.2.3') } - it_behaves_like 'rejecting the request for non existing maven path' + it_behaves_like 'returning response status', :redirect end end + it_behaves_like 'forwarding package requests' + def download_file(file_name:, params: {}, request_headers: headers, path: maven_metadatum.path) get api("/projects/#{project.id}/packages/maven/" \ "#{path}/#{file_name}"), params: params, headers: request_headers |