From 0c4afb9f5620604d74f88b52bfff6a3dce459429 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 28 Oct 2021 00:10:09 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../merge_request_templates/Deprecations.md | 28 +++++-- Gemfile | 2 +- Gemfile.lock | 4 +- app/finders/packages/group_packages_finder.rb | 2 +- app/finders/packages/package_finder.rb | 2 +- app/finders/packages/packages_finder.rb | 3 +- app/models/packages/package.rb | 10 ++- app/models/packages/package_file.rb | 3 +- ...ckages_remove_cross_joins_to_pipelines.yml | 8 ++ .../templates/_deprecation_template.md.erb | 6 +- doc/update/deprecations.md | 5 +- .../merge_requests/approvals/settings.md | 10 ++- lib/api/package_files.rb | 5 +- lib/gitlab/sidekiq_logging/json_formatter.rb | 1 - .../sidekiq_logging/structured_logger.rb | 2 - lib/gitlab/sidekiq_middleware/monitor.rb | 2 - lib/gitlab/x509/certificate.rb | 20 +++-- scripts/gitaly-test-build | 2 + scripts/gitaly-test-spawn | 1 + .../packages/group_packages_finder_spec.rb | 24 ++++++ spec/finders/packages/package_finder_spec.rb | 24 ++++++ spec/finders/packages/packages_finder_spec.rb | 25 +++++++ spec/lib/gitlab/x509/certificate_spec.rb | 5 ++ spec/models/packages/package_file_spec.rb | 23 +++++- spec/models/packages/package_spec.rb | 74 ++++++++++++++++++- spec/requests/api/package_files_spec.rb | 28 +++++++ .../support/database/cross-join-allowlist.yml | 16 ---- spec/support/helpers/gitaly_setup.rb | 30 +++----- .../requests/packages_shared_context.rb | 6 ++ .../package_details_shared_examples.rb | 37 +++++++++- 30 files changed, 335 insertions(+), 73 deletions(-) create mode 100644 config/feature_flags/development/packages_remove_cross_joins_to_pipelines.yml diff --git a/.gitlab/merge_request_templates/Deprecations.md b/.gitlab/merge_request_templates/Deprecations.md index 0898ea4709e..1449246b9bc 100644 --- a/.gitlab/merge_request_templates/Deprecations.md +++ b/.gitlab/merge_request_templates/Deprecations.md @@ -6,16 +6,19 @@ **Be sure to link this MR to the relevant deprecation issue(s).** +Deprecation announcements can and should be created and merged into Docs at any time, to optimize user awareness and planning. We encourage confirmed deprecations to be merged as soon as the required reviews are complete, even if weeks ahead of the target milestone's release post. For the announcement to be included in a specific release post and that release's documentation packages, this MR must be reviewed/merged per the due dates below: + **By the 10th**: Assign this MR to these team members as Reviewer and for Approval (optional unless noted as required): - Product Marketing: `@PMM` - Product Designer(s): `@ProductDesigners` - Group Manager or Director: `@manager` - Engineering Manager: `@EM` - Required +- Technical writer: `@TW` - Required -**By 8:00 AM PDT 15th**: PM will assign this MR to the TW reviewer: `@PM` +**By 11:59 AM PDT 15th**: PM assigns this MR to the TW reviewer for final review and merge: `@PM` -**By 11:59 PM PDT 15th**: TW Reviewer will perform final review and merge this MR to Master: `@TW` +**By 11:59 PM PDT 17th**: TW Reviewer updates Docs by merging this MR to `master`: `@TW` --- @@ -31,9 +34,9 @@ They are frequently updated, and everyone should make sure they are aware of the ## PM release post item checklist - [ ] Set yourself as the Assignee. -- [ ] If the deprecation is a [breaking change](https://about.gitlab.com/handbook/product/gitlab-the-product/#breaking-change), add label `breaking change` +- [ ] If the deprecation is a [breaking change](https://about.gitlab.com/handbook/product/gitlab-the-product/#breaking-change), add label `breaking change`. - [ ] Follow the process to [create a deprecation YAML file](https://about.gitlab.com/handbook/marketing/blog/release-posts/#creating-a-deprecation-entry). -- [ ] Add reviewers by the 10th +- [ ] Add reviewers by the 10th. - [ ] When ready to be merged and not later than the 15th, add the ~ready label and @ message the TW for final review and merge. ## Reviewers @@ -79,5 +82,18 @@ yourself as a reviewer if it's not ready for merge yet. -When the PM indicates it is ready for merge, all issues have been addressed merge this MR. - - You must merge this MR by the 15th so the Release Post TW lead can run the [deprecations in Docs rake task](https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-doc) on the 16th +When the PM indicates it is ready for merge and all issues have been addressed, start the merge process. + +#### Technical writer merge process + +The [deprecations doc's `.md` file](https://gitlab.com/gitlab-org/gitlab/blob/master/doc/update/deprecations.md) +must be updated before this MR is merged: + +1. Check out the MR's branch (in the [`gitlab-org/gitlab`](https://gitlab.com/gitlab-org/gitlab) project). +1. From the command line (in the branch), run `bin/rake gitlab:docs:compile_deprecations`. + If you want to double check that it worked, you can run `bin/rake gitlab:docs:check_deprecations` + to verify that the doc is up to date. +1. Commit the updated file and push the changes. +1. Set the MR to merge when the pipeline succeeds (or merge if the pipeline is already complete). + +If you have trouble running the rake task, check the [troubleshooting steps](https://about.gitlab.com/handbook/marketing/blog/release-posts/#deprecation-rake-task-troubleshooting). diff --git a/Gemfile b/Gemfile index 237ac191e68..3a04f1e1395 100644 --- a/Gemfile +++ b/Gemfile @@ -196,7 +196,7 @@ gem 'acts-as-taggable-on', '~> 7.0' # Background jobs gem 'sidekiq', '~> 6.2.2' -gem 'sidekiq-cron', '~> 1.2' +gem 'sidekiq-cron', '~> 1.0' gem 'redis-namespace', '~> 1.8.1' gem 'gitlab-sidekiq-fetcher', '0.8.0', require: 'sidekiq-reliable-fetch' diff --git a/Gemfile.lock b/Gemfile.lock index 59e134c4170..7497734d6dc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1172,7 +1172,7 @@ GEM connection_pool (>= 2.2.2) rack (~> 2.0) redis (>= 4.2.0) - sidekiq-cron (1.2.0) + sidekiq-cron (1.0.4) fugit (~> 1.1) sidekiq (>= 4.2.1) signet (0.14.0) @@ -1611,7 +1611,7 @@ DEPENDENCIES settingslogic (~> 2.0.9) shoulda-matchers (~> 4.0.1) sidekiq (~> 6.2.2) - sidekiq-cron (~> 1.2) + sidekiq-cron (~> 1.0) simple_po_parser (~> 1.1.2) simplecov (~> 0.18.5) simplecov-cobertura (~> 1.3.1) diff --git a/app/finders/packages/group_packages_finder.rb b/app/finders/packages/group_packages_finder.rb index 3ac5f00d518..093deb43c41 100644 --- a/app/finders/packages/group_packages_finder.rb +++ b/app/finders/packages/group_packages_finder.rb @@ -22,7 +22,7 @@ module Packages def packages_for_group_projects(installable_only: false) packages = ::Packages::Package - .including_build_info + .load_pipelines .including_project_route .including_tags .for_projects(group_projects_visible_to_current_user.select(:id)) diff --git a/app/finders/packages/package_finder.rb b/app/finders/packages/package_finder.rb index ee96896e350..f4463bf0726 100644 --- a/app/finders/packages/package_finder.rb +++ b/app/finders/packages/package_finder.rb @@ -9,7 +9,7 @@ module Packages def execute @project .packages - .including_build_info + .load_pipelines .including_project_route .including_tags .displayable diff --git a/app/finders/packages/packages_finder.rb b/app/finders/packages/packages_finder.rb index 552468ecfd1..29ec3645633 100644 --- a/app/finders/packages/packages_finder.rb +++ b/app/finders/packages/packages_finder.rb @@ -14,9 +14,10 @@ module Packages def execute packages = project.packages - .including_build_info + .load_pipelines .including_project_route .including_tags + packages = filter_with_version(packages) packages = filter_by_package_type(packages) packages = filter_by_package_name(packages) diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index 34eae6ab5dc..ff9e7ef4cf5 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -40,7 +40,7 @@ class Packages::Package < ApplicationRecord has_one :composer_metadatum, inverse_of: :package, class_name: 'Packages::Composer::Metadatum' has_one :rubygems_metadatum, inverse_of: :package, class_name: 'Packages::Rubygems::Metadatum' has_many :build_infos, inverse_of: :package - has_many :pipelines, through: :build_infos + has_many :pipelines, through: :build_infos, disable_joins: -> { disable_cross_joins_to_pipelines? } has_one :debian_publication, inverse_of: :package, class_name: 'Packages::Debian::Publication' has_one :debian_distribution, through: :debian_publication, source: :distribution, inverse_of: :packages, class_name: 'Packages::Debian::ProjectDistribution' @@ -131,9 +131,11 @@ class Packages::Package < ApplicationRecord scope :has_version, -> { where.not(version: nil) } scope :preload_files, -> { preload(:package_files) } + scope :preload_pipelines, -> { preload(pipelines: :user) } scope :last_of_each_version, -> { where(id: all.select('MAX(id) AS id').group(:version)) } scope :limit_recent, ->(limit) { order_created_desc.limit(limit) } scope :select_distinct_name, -> { select(:name).distinct } + scope :load_pipelines, -> { disable_cross_joins_to_pipelines? ? preload_pipelines : including_build_info } # Sorting scope :order_created, -> { reorder(created_at: :asc) } @@ -160,6 +162,10 @@ class Packages::Package < ApplicationRecord joins(:project).reorder(keyset_order) end + def self.disable_cross_joins_to_pipelines? + ::Feature.enabled?(:packages_remove_cross_joins_to_pipelines, default_enabled: :yaml) + end + def self.only_maven_packages_with_path(path, use_cte: false) if use_cte # This is an optimization fence which assumes that looking up the Metadatum record by path (globally) @@ -245,7 +251,7 @@ class Packages::Package < ApplicationRecord def versions project.packages - .including_build_info + .load_pipelines .including_tags .with_name(name) .where.not(version: version) diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 14701b8a800..554c5e64f7c 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -15,7 +15,7 @@ class Packages::PackageFile < ApplicationRecord has_one :conan_file_metadatum, inverse_of: :package_file, class_name: 'Packages::Conan::FileMetadatum' has_many :package_file_build_infos, inverse_of: :package_file, class_name: 'Packages::PackageFileBuildInfo' - has_many :pipelines, through: :package_file_build_infos + has_many :pipelines, through: :package_file_build_infos, disable_joins: -> { Packages::Package.disable_cross_joins_to_pipelines? } has_one :debian_file_metadatum, inverse_of: :package_file, class_name: 'Packages::Debian::FileMetadatum' has_one :helm_file_metadatum, inverse_of: :package_file, class_name: 'Packages::Helm::FileMetadatum' @@ -38,6 +38,7 @@ class Packages::PackageFile < ApplicationRecord scope :with_format, ->(format) { where(::Packages::PackageFile.arel_table[:file_name].matches("%.#{format}")) } scope :preload_package, -> { preload(:package) } + scope :preload_pipelines, -> { preload(pipelines: :user) } scope :preload_conan_file_metadata, -> { preload(:conan_file_metadatum) } scope :preload_debian_file_metadata, -> { preload(:debian_file_metadatum) } scope :preload_helm_file_metadata, -> { preload(:helm_file_metadatum) } diff --git a/config/feature_flags/development/packages_remove_cross_joins_to_pipelines.yml b/config/feature_flags/development/packages_remove_cross_joins_to_pipelines.yml new file mode 100644 index 00000000000..5f3b4490cf7 --- /dev/null +++ b/config/feature_flags/development/packages_remove_cross_joins_to_pipelines.yml @@ -0,0 +1,8 @@ +--- +name: packages_remove_cross_joins_to_pipelines +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70624 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/342921 +milestone: '14.5' +type: development +group: group::package +default_enabled: false diff --git a/data/deprecations/templates/_deprecation_template.md.erb b/data/deprecations/templates/_deprecation_template.md.erb index a037151c6ac..82f9c25386b 100644 --- a/data/deprecations/templates/_deprecation_template.md.erb +++ b/data/deprecations/templates/_deprecation_template.md.erb @@ -22,8 +22,10 @@ located at `lib/tasks/gitlab/docs/compile_deprecations.rake`. Do not edit this page directly. -To add a deprecation, use the example.yml file in `/data/deprecations/templates` as a template, -then run `bin/rake gitlab:docs:compile_deprecations`. +To add a deprecation, use the example.yml file in `/data/deprecations/templates` as a template. + +To update this doc, run `bin/rake gitlab:docs:compile_deprecations`. +To verify this doc is up to date, run `bin/rake gitlab:docs:check_deprecations` --> <% if milestones.any? -%> <%- milestones.each do |milestone| %> diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index e2af4f453c0..f6b847c1c9d 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -22,8 +22,9 @@ located at `lib/tasks/gitlab/docs/compile_deprecations.rake`. Do not edit this page directly. -To add a deprecation, use the example.yml file in `/data/deprecations/templates` as a template, -then run `bin/rake gitlab:docs:compile_deprecations`. +To add a deprecation, follow the [deprecation item guidance](https://about.gitlab.com/handbook/marketing/blog/release-posts/#creating-a-deprecation-entry) and use the [`/data/deprecations/templates/example.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/data/deprecations/templates/example.yml) file as a template. + +To update this doc, follow the instructions in [Update the deprecations doc](https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-doc). --> ## 14.4 diff --git a/doc/user/project/merge_requests/approvals/settings.md b/doc/user/project/merge_requests/approvals/settings.md index c5440c3358c..dab95212110 100644 --- a/doc/user/project/merge_requests/approvals/settings.md +++ b/doc/user/project/merge_requests/approvals/settings.md @@ -70,8 +70,14 @@ their own. To do this: it can't be changed at the project level. 1. Select **Save changes**. -Even with this configuration, [code owners](../../code_owners.md) who contribute -to a merge request can approve merge requests that affect files they own. +Depending on your version of GitLab, [code owners](../../code_owners.md) who commit +to a merge request may or may not be able to approve the work: + +- In GitLab 13.10 and earlier, [code owners](../../code_owners.md) who commit + to a merge request can approve it, even if the merge request affects files they own. +- In [GitLab 13.11 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/331548), + [code owners](../../code_owners.md) who commit + to a merge request cannot approve it, when the merge request affects files they own. To learn more about the [differences between authors and committers](https://git-scm.com/book/en/v2/Git-Basics-Viewing-the-Commit-History), read the official Git documentation for an explanation. diff --git a/lib/api/package_files.rb b/lib/api/package_files.rb index 6d0c1f44a36..a4d298d94e8 100644 --- a/lib/api/package_files.rb +++ b/lib/api/package_files.rb @@ -28,7 +28,10 @@ module API package = ::Packages::PackageFinder .new(user_project, params[:package_id]).execute - present paginate(package.package_files), with: ::API::Entities::PackageFile + files = package.package_files + files = files.preload_pipelines if Packages::Package.disable_cross_joins_to_pipelines? + + present paginate(files), with: ::API::Entities::PackageFile end desc 'Remove a package file' do diff --git a/lib/gitlab/sidekiq_logging/json_formatter.rb b/lib/gitlab/sidekiq_logging/json_formatter.rb index d22f0851927..8894b48417c 100644 --- a/lib/gitlab/sidekiq_logging/json_formatter.rb +++ b/lib/gitlab/sidekiq_logging/json_formatter.rb @@ -2,7 +2,6 @@ # This is needed for sidekiq-cluster require 'json' -require 'sidekiq/job_retry' module Gitlab module SidekiqLogging diff --git a/lib/gitlab/sidekiq_logging/structured_logger.rb b/lib/gitlab/sidekiq_logging/structured_logger.rb index a9bfcce2e0a..3438bc0f3ef 100644 --- a/lib/gitlab/sidekiq_logging/structured_logger.rb +++ b/lib/gitlab/sidekiq_logging/structured_logger.rb @@ -2,8 +2,6 @@ require 'active_record' require 'active_record/log_subscriber' -require 'sidekiq/job_logger' -require 'sidekiq/job_retry' module Gitlab module SidekiqLogging diff --git a/lib/gitlab/sidekiq_middleware/monitor.rb b/lib/gitlab/sidekiq_middleware/monitor.rb index d38fed3b768..ed825dbfd60 100644 --- a/lib/gitlab/sidekiq_middleware/monitor.rb +++ b/lib/gitlab/sidekiq_middleware/monitor.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'sidekiq/job_retry' - module Gitlab module SidekiqMiddleware class Monitor diff --git a/lib/gitlab/x509/certificate.rb b/lib/gitlab/x509/certificate.rb index 81a50433be2..752f3c6b004 100644 --- a/lib/gitlab/x509/certificate.rb +++ b/lib/gitlab/x509/certificate.rb @@ -19,6 +19,10 @@ module Gitlab ca_certs.map(&:to_pem).join('\n') unless ca_certs.blank? end + class << self + include ::Gitlab::Utils::StrongMemoize + end + def self.from_strings(key_string, cert_string, ca_certs_string = nil) key = OpenSSL::PKey::RSA.new(key_string) cert = OpenSSL::X509::Certificate.new(cert_string) @@ -44,13 +48,17 @@ module Gitlab # Returns a concatenated array of Strings, each being a PEM-coded CA certificate. def self.ca_certs_bundle - return @certs if @certs + strong_memoize(:ca_certs_bundle) do + ca_certs_paths.flat_map do |cert_file| + load_ca_certs_bundle(File.read(cert_file)) + rescue OpenSSL::OpenSSLError => e + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, cert_file: cert_file) + end.uniq.join("\n") + end + end - @certs = ca_certs_paths.flat_map do |cert_file| - load_ca_certs_bundle(File.read(cert_file)) - rescue OpenSSL::OpenSSLError => e - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e, cert_file: cert_file) - end.uniq.join("\n") + def self.reset_ca_certs_bundle + clear_memoization(:ca_certs_bundle) end # Returns an array of OpenSSL::X509::Certificate objects, empty array if none found diff --git a/scripts/gitaly-test-build b/scripts/gitaly-test-build index adc9b56ca4f..e6afadccc7e 100755 --- a/scripts/gitaly-test-build +++ b/scripts/gitaly-test-build @@ -13,6 +13,8 @@ class GitalyTestBuild include GitalySetup def run + set_bundler_config + # If we have the binaries from the cache, we can skip building them again if File.exist?(tmp_tests_gitaly_bin_dir) GitalySetup::LOGGER.debug "Gitaly binary already built. Skip building...\n" diff --git a/scripts/gitaly-test-spawn b/scripts/gitaly-test-spawn index 2e0e51124af..e7e25a217b2 100755 --- a/scripts/gitaly-test-spawn +++ b/scripts/gitaly-test-spawn @@ -9,6 +9,7 @@ class GitalyTestSpawn include GitalySetup def run + set_bundler_config install_gitaly_gems if ENV['CI'] check_gitaly_config! diff --git a/spec/finders/packages/group_packages_finder_spec.rb b/spec/finders/packages/group_packages_finder_spec.rb index 3254c436674..7da1bfa58aa 100644 --- a/spec/finders/packages/group_packages_finder_spec.rb +++ b/spec/finders/packages/group_packages_finder_spec.rb @@ -160,6 +160,30 @@ RSpec.describe Packages::GroupPackagesFinder do end end + context 'with pipelines' do + let_it_be(:build_info_1) { create(:package_build_info, :with_pipeline, package: package1) } + let_it_be(:build_info_2) { create(:package_build_info, :with_pipeline, package: package2) } + + it 'preloads the pipelines' do + expect(::Packages::Package).to receive(:preload_pipelines).and_call_original + expect(::Packages::Package).not_to receive(:including_build_info) + + expect(subject).to match_array([package1, package2]) + end + + context 'with packages_remove_cross_joins_to_pipelines disabled' do + before do + stub_feature_flags(packages_remove_cross_joins_to_pipelines: false) + end + + it 'includes the pipelines' do + expect(::Packages::Package).to receive(:including_build_info).and_call_original + expect(::Packages::Package).not_to receive(:preload_pipelines) + expect(subject).to match_array([package1, package2]) + end + end + end + it_behaves_like 'concerning versionless param' it_behaves_like 'concerning package statuses' end diff --git a/spec/finders/packages/package_finder_spec.rb b/spec/finders/packages/package_finder_spec.rb index 1b0c88a4771..dfa1a65b322 100644 --- a/spec/finders/packages/package_finder_spec.rb +++ b/spec/finders/packages/package_finder_spec.rb @@ -32,5 +32,29 @@ RSpec.describe ::Packages::PackageFinder do expect { subject }.to raise_exception(ActiveRecord::RecordNotFound) end end + + context 'with pipelines' do + let_it_be(:build_info) { create(:package_build_info, :with_pipeline, package: maven_package) } + + it 'preloads the pipelines' do + expect(::Packages::Package).to receive(:preload_pipelines).and_call_original + expect(::Packages::Package).not_to receive(:including_build_info) + + expect(subject).to eq(maven_package) + end + + context 'with packages_remove_cross_joins_to_pipelines disabled' do + before do + stub_feature_flags(packages_remove_cross_joins_to_pipelines: false) + end + + it 'includes the pipelines' do + expect(::Packages::Package).to receive(:including_build_info).and_call_original + expect(::Packages::Package).not_to receive(:preload_pipelines) + + expect(subject).to eq(maven_package) + end + end + end end end diff --git a/spec/finders/packages/packages_finder_spec.rb b/spec/finders/packages/packages_finder_spec.rb index b72f4aab3ec..8d67c96e815 100644 --- a/spec/finders/packages/packages_finder_spec.rb +++ b/spec/finders/packages/packages_finder_spec.rb @@ -81,6 +81,31 @@ RSpec.describe ::Packages::PackagesFinder do it { is_expected.to match_array([conan_package, maven_package]) } end + context 'with pipelines' do + let_it_be(:build_info1) { create(:package_build_info, :with_pipeline, package: conan_package) } + let_it_be(:build_info2) { create(:package_build_info, :with_pipeline, package: maven_package) } + + it 'preloads the pipelines' do + expect(::Packages::Package).to receive(:preload_pipelines).and_call_original + expect(::Packages::Package).not_to receive(:including_build_info) + + expect(subject).to match_array([conan_package, maven_package]) + end + + context 'with packages_remove_cross_joins_to_pipelines disabled' do + before do + stub_feature_flags(packages_remove_cross_joins_to_pipelines: false) + end + + it 'includes the pipelines' do + expect(::Packages::Package).to receive(:including_build_info).and_call_original + expect(::Packages::Package).not_to receive(:preload_pipelines) + + expect(subject).to match_array([conan_package, maven_package]) + end + end + end + it_behaves_like 'concerning versionless param' it_behaves_like 'concerning package statuses' end diff --git a/spec/lib/gitlab/x509/certificate_spec.rb b/spec/lib/gitlab/x509/certificate_spec.rb index 4e1a269eecf..2dc30cc871d 100644 --- a/spec/lib/gitlab/x509/certificate_spec.rb +++ b/spec/lib/gitlab/x509/certificate_spec.rb @@ -19,6 +19,11 @@ RSpec.describe Gitlab::X509::Certificate do before do stub_const("OpenSSL::X509::DEFAULT_CERT_DIR", sample_ca_certs_path) stub_const("OpenSSL::X509::DEFAULT_CERT_FILE", sample_cert) + described_class.reset_ca_certs_bundle + end + + after(:context) do + described_class.reset_ca_certs_bundle end describe 'testing environment setup' do diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index 450656e3e9c..8617793f41d 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -14,7 +14,6 @@ RSpec.describe Packages::PackageFile, type: :model do it { is_expected.to belong_to(:package) } it { is_expected.to have_one(:conan_file_metadatum) } it { is_expected.to have_many(:package_file_build_infos).inverse_of(:package_file) } - it { is_expected.to have_many(:pipelines).through(:package_file_build_infos) } it { is_expected.to have_one(:debian_file_metadatum).inverse_of(:package_file).class_name('Packages::Debian::FileMetadatum') } it { is_expected.to have_one(:helm_file_metadatum).inverse_of(:package_file).class_name('Packages::Helm::FileMetadatum') } end @@ -206,6 +205,28 @@ RSpec.describe Packages::PackageFile, type: :model do end end + describe '#pipelines' do + let_it_be_with_refind(:package_file) { create(:package_file) } + + subject { package_file.pipelines } + + context 'package_file without pipeline' do + it { is_expected.to be_empty } + end + + context 'package_file with pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:pipeline2) { create(:ci_pipeline) } + + before do + package_file.package_file_build_infos.create!(pipeline: pipeline) + package_file.package_file_build_infos.create!(pipeline: pipeline2) + end + + it { is_expected.to contain_exactly(pipeline, pipeline2) } + end + end + describe '#update_file_store callback' do let_it_be(:package_file) { build(:package_file, :nuget, size: nil) } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 2573c01d686..12a2a2a94a6 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -14,7 +14,6 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to have_many(:dependency_links).inverse_of(:package) } it { is_expected.to have_many(:tags).inverse_of(:package) } it { is_expected.to have_many(:build_infos).inverse_of(:package) } - it { is_expected.to have_many(:pipelines).through(:build_infos) } it { is_expected.to have_one(:conan_metadatum).inverse_of(:package) } it { is_expected.to have_one(:maven_metadatum).inverse_of(:package) } it { is_expected.to have_one(:debian_publication).inverse_of(:package).class_name('Packages::Debian::Publication') } @@ -962,6 +961,33 @@ RSpec.describe Packages::Package, type: :model do end end + describe '.load_pipelines' do + let_it_be(:package) { create(:maven_package) } + let_it_be(:build_info) { create(:package_build_info, :with_pipeline, package: package) } + + subject { described_class.load_pipelines } + + it 'uses preload', :aggregate_failures do + expect(described_class).to receive(:preload_pipelines).and_call_original + expect(described_class).not_to receive(:including_build_info) + + subject + end + + context 'with packages_remove_cross_joins_to_pipelines disabled' do + before do + stub_feature_flags(packages_remove_cross_joins_to_pipelines: false) + end + + it 'uses includes', :aggregate_failures do + expect(described_class).to receive(:including_build_info).and_call_original + expect(described_class).not_to receive(:preload_pipelines) + + subject + end + end + end + describe '#versions' do let_it_be(:project) { create(:project) } let_it_be(:package) { create(:maven_package, project: project) } @@ -975,6 +1001,30 @@ RSpec.describe Packages::Package, type: :model do it 'does not return different packages' do expect(package.versions).not_to include(package3) end + + context 'with pipelines' do + let_it_be(:build_info) { create(:package_build_info, :with_pipeline, package: package2) } + + it 'preloads the pipelines' do + expect(::Packages::Package).to receive(:preload_pipelines).and_call_original + expect(::Packages::Package).not_to receive(:including_build_info) + + expect(package.versions).to contain_exactly(package2) + end + + context 'with packages_remove_cross_joins_to_pipelines disabled' do + before do + stub_feature_flags(packages_remove_cross_joins_to_pipelines: false) + end + + it 'includes the pipelines' do + expect(::Packages::Package).to receive(:including_build_info).and_call_original + expect(::Packages::Package).not_to receive(:preload_pipelines) + + expect(package.versions).to contain_exactly(package2) + end + end + end end describe '#pipeline' do @@ -999,6 +1049,28 @@ RSpec.describe Packages::Package, type: :model do end end + describe '#pipelines' do + let_it_be_with_refind(:package) { create(:maven_package) } + + subject { package.pipelines } + + context 'package without pipeline' do + it { is_expected.to be_empty } + end + + context 'package with pipeline' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:pipeline2) { create(:ci_pipeline) } + + before do + package.build_infos.create!(pipeline: pipeline) + package.build_infos.create!(pipeline: pipeline2) + end + + it { is_expected.to contain_exactly(pipeline, pipeline2) } + end + end + describe '#tag_names' do let_it_be(:package) { create(:nuget_package) } diff --git a/spec/requests/api/package_files_spec.rb b/spec/requests/api/package_files_spec.rb index eb1f04d193e..c170c151057 100644 --- a/spec/requests/api/package_files_spec.rb +++ b/spec/requests/api/package_files_spec.rb @@ -27,6 +27,34 @@ RSpec.describe API::PackageFiles do expect(response).to have_gitlab_http_status(:not_found) end + + context 'with pipelines' do + let(:package_file_build_info) { create(:package_file_build_info, :with_pipeline, package_file: package.package_files.first) } + + it 'preloads the pipelines' do + expect(::Packages::PackageFile).to receive(:preload_pipelines).and_call_original + + get api(url) + + expect(response).to have_gitlab_http_status(:ok) + end + + context 'with packages_remove_cross_joins_to_pipelines disabled' do + before do + stub_feature_flags(packages_remove_cross_joins_to_pipelines: false) + end + + it 'does not preload the pipelines' do + expect(::Packages::PackageFile).not_to receive(:preload_pipelines) + + ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/342921') do + get api(url) + end + + expect(response).to have_gitlab_http_status(:ok) + end + end + end end context 'project is private' do diff --git a/spec/support/database/cross-join-allowlist.yml b/spec/support/database/cross-join-allowlist.yml index 6d9b002e026..bfb0cf0f45a 100644 --- a/spec/support/database/cross-join-allowlist.yml +++ b/spec/support/database/cross-join-allowlist.yml @@ -2,26 +2,10 @@ - "./ee/spec/services/ci/minutes/additional_packs/change_namespace_service_spec.rb" - "./ee/spec/services/ci/minutes/additional_packs/create_service_spec.rb" - "./ee/spec/services/ci/minutes/refresh_cached_data_service_spec.rb" -- "./spec/features/groups/packages_spec.rb" - "./spec/features/projects/infrastructure_registry_spec.rb" -- "./spec/features/projects/packages_spec.rb" -- "./spec/lib/api/entities/package_spec.rb" - "./spec/lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans_spec.rb" - "./spec/lib/gitlab/background_migration/migrate_pages_metadata_spec.rb" - "./spec/migrations/20210907211557_finalize_ci_builds_bigint_conversion_spec.rb" - "./spec/migrations/associate_existing_dast_builds_with_variables_spec.rb" - "./spec/migrations/disable_job_token_scope_when_unused_spec.rb" - "./spec/migrations/schedule_copy_ci_builds_columns_to_security_scans2_spec.rb" -- "./spec/presenters/packages/detail/package_presenter_spec.rb" -- "./spec/requests/api/graphql/group/packages_spec.rb" -- "./spec/requests/api/graphql/packages/composer_spec.rb" -- "./spec/requests/api/graphql/packages/conan_spec.rb" -- "./spec/requests/api/graphql/packages/maven_spec.rb" -- "./spec/requests/api/graphql/packages/nuget_spec.rb" -- "./spec/requests/api/graphql/packages/package_spec.rb" -- "./spec/requests/api/graphql/packages/pypi_spec.rb" -- "./spec/requests/api/graphql/project/packages_spec.rb" -- "./spec/requests/api/package_files_spec.rb" -- "./spec/services/packages/conan/create_package_file_service_spec.rb" -- "./spec/services/packages/create_package_file_service_spec.rb" -- "./spec/services/packages/generic/create_package_file_service_spec.rb" diff --git a/spec/support/helpers/gitaly_setup.rb b/spec/support/helpers/gitaly_setup.rb index 99afc8b364e..8a329c2f9dd 100644 --- a/spec/support/helpers/gitaly_setup.rb +++ b/spec/support/helpers/gitaly_setup.rb @@ -9,7 +9,6 @@ require 'securerandom' require 'socket' require 'logger' -require 'bundler' module GitalySetup LOGGER = begin @@ -47,26 +46,13 @@ module GitalySetup File.join(tmp_tests_gitlab_shell_dir, '.gitlab_shell_secret') end - # Return the path of the vendored gems in /gitaly, if exists - def gdk_gitaly_ruby_gem_path - gitaly_ruby_path = File.expand_path('../../../../gitaly/ruby/', __dir__) - bundle_config_path = File.join(gitaly_ruby_path, '.bundle') - bundle_path = Bundler::Settings.new(bundle_config_path).path - - return if bundle_path.use_system_gems? - - File.expand_path(bundle_path.explicit_path, gitaly_ruby_path) - end - def env { 'HOME' => File.expand_path('tmp/tests'), 'GEM_PATH' => Gem.path.join(':'), - 'BUNDLE_IGNORE_CONFIG' => '1', + 'BUNDLE_APP_CONFIG' => File.join(gemfile_dir, '.bundle'), + 'BUNDLE_INSTALL_FLAGS' => nil, 'BUNDLE_GEMFILE' => gemfile, - 'BUNDLE_PATH' => bundle_path, - 'BUNDLE_JOBS' => '4', - 'BUNDLE_RETRY' => '3', 'RUBYOPT' => nil, # Git hooks can't run during tests as the internal API is not running. @@ -75,13 +61,17 @@ module GitalySetup } end - def bundle_path + # rubocop:disable GitlabSecurity/SystemCommandInjection + def set_bundler_config + system('bundle config set --local jobs 4', chdir: gemfile_dir) + system('bundle config set --local retry 3', chdir: gemfile_dir) + if ENV['CI'] - File.expand_path('../../../vendor/gitaly-ruby', __dir__) - else - gdk_gitaly_ruby_gem_path || File.expand_path(Bundler.configured_bundle_path.base_path) + bundle_path = File.expand_path('../../../vendor/gitaly-ruby', __dir__) + system('bundle', 'config', 'set', '--local', 'path', bundle_path, chdir: gemfile_dir) end end + # rubocop:enable GitlabSecurity/SystemCommandInjection def config_path(service) case service diff --git a/spec/support/shared_contexts/graphql/requests/packages_shared_context.rb b/spec/support/shared_contexts/graphql/requests/packages_shared_context.rb index 645ea742f07..9ac3d4a04f9 100644 --- a/spec/support/shared_contexts/graphql/requests/packages_shared_context.rb +++ b/spec/support/shared_contexts/graphql/requests/packages_shared_context.rb @@ -10,6 +10,7 @@ RSpec.shared_context 'package details setup' do let(:excluded) { %w[metadata apiFuzzingCiConfiguration pipeline packageFiles] } let(:package_files) { all_graphql_fields_for('PackageFile') } let(:dependency_links) { all_graphql_fields_for('PackageDependencyLink') } + let(:pipelines) { all_graphql_fields_for('Pipeline', max_depth: 1) } let(:user) { project.owner } let(:package_details) { graphql_data_at(:package) } let(:metadata_response) { graphql_data_at(:package, :metadata) } @@ -34,6 +35,11 @@ RSpec.shared_context 'package details setup' do #{dependency_links} } } + pipelines { + nodes { + #{pipelines} + } + } FIELDS end end diff --git a/spec/support/shared_examples/requests/api/graphql/packages/package_details_shared_examples.rb b/spec/support/shared_examples/requests/api/graphql/packages/package_details_shared_examples.rb index 41a61ba5fd7..2faeb1a534b 100644 --- a/spec/support/shared_examples/requests/api/graphql/packages/package_details_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/graphql/packages/package_details_shared_examples.rb @@ -2,9 +2,42 @@ RSpec.shared_examples 'a package detail' do it_behaves_like 'a working graphql query' do - it 'matches the JSON schema' do - expect(package_details).to match_schema('graphql/packages/package_details') + it_behaves_like 'matching the package details schema' + end + + context 'with pipelines' do + let_it_be(:build_info1) { create(:package_build_info, :with_pipeline, package: package) } + let_it_be(:build_info2) { create(:package_build_info, :with_pipeline, package: package) } + let_it_be(:build_info3) { create(:package_build_info, :with_pipeline, package: package) } + + it_behaves_like 'a working graphql query' do + it_behaves_like 'matching the package details schema' end + + context 'with packages_remove_cross_joins_to_pipelines disabled' do + # subject is called in a before callback that is executed before the one below + # the callback below MUST be executed before the subject + # solution: nullify subject and manually call #post_graphql + subject {} + + before do + stub_feature_flags(packages_remove_cross_joins_to_pipelines: false) + + ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/342921') do + post_graphql(query, current_user: user) + end + end + + it_behaves_like 'a working graphql query' do + it_behaves_like 'matching the package details schema' + end + end + end +end + +RSpec.shared_examples 'matching the package details schema' do + it 'matches the JSON schema' do + expect(package_details).to match_schema('graphql/packages/package_details') end end