diff --git a/.gitlab/ci/review-apps/dast.gitlab-ci.yml b/.gitlab/ci/review-apps/dast.gitlab-ci.yml index 512c850b7da..d0ad4d23a82 100644 --- a/.gitlab/ci/review-apps/dast.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/dast.gitlab-ci.yml @@ -45,7 +45,6 @@ # 10019, 10021 Missing security headers # 10023, 10024, 10025, 10037 Information Disclosure # 10040 Secure Pages Include Mixed Content -# 10055 CSP # 10056 X-Debug-Token Information Leak # Duration: 14 minutes 20 seconds @@ -54,7 +53,7 @@ dast:secureHeaders-csp-infoLeak: - .dast_conf variables: DAST_USERNAME: "user1" - DAST_ONLY_INCLUDE_RULES: "10019,10021,10023,10024,10025,10037,10040,10055,10056" + DAST_ONLY_INCLUDE_RULES: "10019,10021,10023,10024,10025,10037,10040,10056" script: - /analyze diff --git a/app/controllers/groups/dependency_proxy_for_containers_controller.rb b/app/controllers/groups/dependency_proxy_for_containers_controller.rb index 142512cc2a9..00839583ecc 100644 --- a/app/controllers/groups/dependency_proxy_for_containers_controller.rb +++ b/app/controllers/groups/dependency_proxy_for_containers_controller.rb @@ -145,8 +145,7 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy end def dependency_proxy - @dependency_proxy ||= - group.dependency_proxy_setting || group.create_dependency_proxy_setting + @dependency_proxy ||= group.dependency_proxy_setting end def ensure_group diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index ef6c10d43cd..c71134e0547 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -26,9 +26,13 @@ module Projects ).to_json end - # @assignable_runners is using ci_owned_runners - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336436') do + if current_user.ci_owned_runners_cross_joins_fix_enabled? render + else + # @assignable_runners is using ci_owned_runners + ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336436') do + render + end end end diff --git a/app/models/ci/namespace_mirror.rb b/app/models/ci/namespace_mirror.rb index cff63c00e07..ce3faf3546b 100644 --- a/app/models/ci/namespace_mirror.rb +++ b/app/models/ci/namespace_mirror.rb @@ -10,6 +10,10 @@ module Ci where('traversal_ids @> ARRAY[?]::int[]', id) end + scope :contains_any_of_namespaces, -> (ids) do + where('traversal_ids && ARRAY[?]::int[]', ids) + end + scope :by_namespace_id, -> (namespace_id) { where(namespace_id: namespace_id) } class << self diff --git a/app/models/group.rb b/app/models/group.rb index 92d7fddf736..4e675d3eb8a 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -776,6 +776,10 @@ class Group < Namespace super || build_dependency_proxy_image_ttl_policy end + def dependency_proxy_setting + super || build_dependency_proxy_setting + end + def crm_enabled? crm_settings&.enabled? end diff --git a/app/models/user.rb b/app/models/user.rb index 3f9f5b39922..8e954aac1ba 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1605,23 +1605,32 @@ class User < ApplicationRecord def ci_owned_runners @ci_owned_runners ||= begin - project_runners = Ci::RunnerProject - .where(project: authorized_projects(Gitlab::Access::MAINTAINER)) - .joins(:runner) - .select('ci_runners.*') - - group_runners = Ci::RunnerNamespace - .where(namespace_id: owned_groups.self_and_descendant_ids) - .joins(:runner) - .select('ci_runners.*') - - Ci::Runner.from_union([project_runners, group_runners]).allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336436') + if ci_owned_runners_cross_joins_fix_enabled? + Ci::Runner + .from_union([ci_owned_project_runners_from_project_members, + ci_owned_project_runners_from_group_members, + ci_owned_group_runners]) + else + Ci::Runner + .from_union([ci_legacy_owned_project_runners, ci_legacy_owned_group_runners]) + .allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336436') + end end end def owns_runner?(runner) - ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336436') do + if ci_owned_runners_cross_joins_fix_enabled? ci_owned_runners.exists?(runner.id) + else + ::Gitlab::Database.allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/336436') do + ci_owned_runners.exists?(runner.id) + end + end + end + + def ci_owned_runners_cross_joins_fix_enabled? + strong_memoize(:ci_owned_runners_cross_joins_fix_enabled) do + Feature.enabled?(:ci_owned_runners_cross_joins_fix, self, default_enabled: :yaml) end end @@ -2199,6 +2208,50 @@ class User < ApplicationRecord ::Gitlab::Auth::Ldap::Access.allowed?(self) end + + def ci_legacy_owned_project_runners + Ci::RunnerProject + .select('ci_runners.*') + .joins(:runner) + .where(project: authorized_projects(Gitlab::Access::MAINTAINER)) + end + + def ci_legacy_owned_group_runners + Ci::RunnerNamespace + .select('ci_runners.*') + .joins(:runner) + .where(namespace_id: owned_groups.self_and_descendant_ids) + end + + def ci_owned_project_runners_from_project_members + Ci::RunnerProject + .select('ci_runners.*') + .joins(:runner) + .where(project: project_members.where('access_level >= ?', Gitlab::Access::MAINTAINER).pluck(:source_id)) + end + + def ci_owned_project_runners_from_group_members + Ci::RunnerProject + .select('ci_runners.*') + .joins(:runner) + .joins('JOIN ci_project_mirrors ON ci_project_mirrors.project_id = ci_runner_projects.project_id') + .joins('JOIN ci_namespace_mirrors ON ci_namespace_mirrors.namespace_id = ci_project_mirrors.namespace_id') + .merge(ci_namespace_mirrors_for_group_members(Gitlab::Access::MAINTAINER)) + end + + def ci_owned_group_runners + Ci::RunnerNamespace + .select('ci_runners.*') + .joins(:runner) + .joins('JOIN ci_namespace_mirrors ON ci_namespace_mirrors.namespace_id = ci_runner_namespaces.namespace_id') + .merge(ci_namespace_mirrors_for_group_members(Gitlab::Access::OWNER)) + end + + def ci_namespace_mirrors_for_group_members(level) + Ci::NamespaceMirror.contains_any_of_namespaces( + group_members.where('access_level >= ?', level).pluck(:source_id) + ) + end end User.prepend_mod_with('User') diff --git a/app/services/import/validate_remote_git_endpoint_service.rb b/app/services/import/validate_remote_git_endpoint_service.rb index afccb5373a9..1b8fa45e979 100644 --- a/app/services/import/validate_remote_git_endpoint_service.rb +++ b/app/services/import/validate_remote_git_endpoint_service.rb @@ -23,6 +23,8 @@ module Import return ServiceResponse.error(message: "#{@params[:url]} is not a valid URL") unless uri + return ServiceResponse.success if uri.scheme == 'git' + uri.fragment = nil url = Gitlab::Utils.append_path(uri.to_s, "/info/refs?service=#{GIT_SERVICE_NAME}") diff --git a/config/feature_flags/development/ci_owned_runners_cross_joins_fix.yml b/config/feature_flags/development/ci_owned_runners_cross_joins_fix.yml new file mode 100644 index 00000000000..aacb188ba35 --- /dev/null +++ b/config/feature_flags/development/ci_owned_runners_cross_joins_fix.yml @@ -0,0 +1,8 @@ +--- +name: ci_owned_runners_cross_joins_fix +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78216 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350322 +milestone: '14.8' +type: development +group: group::pipeline execution +default_enabled: false diff --git a/config/feature_flags/development/rate_limit_gitlab_shell.yml b/config/feature_flags/development/rate_limit_gitlab_shell.yml new file mode 100644 index 00000000000..ceb9e86b01c --- /dev/null +++ b/config/feature_flags/development/rate_limit_gitlab_shell.yml @@ -0,0 +1,8 @@ +--- +name: rate_limit_gitlab_shell +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78373 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350465 +milestone: '14.7' +type: development +group: group::source code +default_enabled: false diff --git a/config/feature_flags/development/sourcegraph.yml b/config/feature_flags/development/sourcegraph.yml index 12170aec869..f9aa76f6c7c 100644 --- a/config/feature_flags/development/sourcegraph.yml +++ b/config/feature_flags/development/sourcegraph.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/292199 milestone: '12.5' type: development group: group::editor -default_enabled: false +default_enabled: true diff --git a/doc/administration/geo/replication/troubleshooting.md b/doc/administration/geo/replication/troubleshooting.md index 1bc23d6434f..a6ea41171a9 100644 --- a/doc/administration/geo/replication/troubleshooting.md +++ b/doc/administration/geo/replication/troubleshooting.md @@ -432,7 +432,16 @@ If large repositories are affected by this problem, their resync may take a long time and cause significant load on your Geo nodes, storage and network systems. -If you get the error message `Synchronization failed - Error syncing repository` along with the following log messages, this indicates that the expected `geo` remote is not present in the `.git/config` file +If you see the error message `Synchronization failed - Error syncing repository` along with `fatal: fsck error in packed object`, this indicates +a consistency check error when syncing the repository. + +One example of a consistency error is: `error: object f4a87a3be694fbbd6e50a668a31a8513caeaafe3: hasDotgit: contains '.git`. + +Removing the malformed objects causing consistency errors require rewriting the repository history, which is not always an option. However, +it's possible to override the consistency checks instead. To do that, follow +[the instructions in the Gitaly docs](../../gitaly/configure_gitaly.md#repository-consistency-checks). + +You can also get the error message `Synchronization failed - Error syncing repository` along with the following log messages, this indicates that the expected `geo` remote is not present in the `.git/config` file of a repository on the secondary Geo node's file system: ```json diff --git a/doc/ci/interactive_web_terminal/index.md b/doc/ci/interactive_web_terminal/index.md index 5724c56b096..f49fdd6c39f 100644 --- a/doc/ci/interactive_web_terminal/index.md +++ b/doc/ci/interactive_web_terminal/index.md @@ -32,10 +32,18 @@ Two things need to be configured for the interactive web terminal to work: - If you are using a reverse proxy with your GitLab instance, web terminals need to be [enabled](../../administration/integration/terminal.md#enabling-and-disabling-terminal-support) -NOTE: -Interactive web terminals are not yet supported by -[`gitlab-runner` Helm chart](https://docs.gitlab.com/charts/charts/gitlab/gitlab-runner/index.html). -Support is tracked [in this issue](https://gitlab.com/gitlab-org/charts/gitlab-runner/-/issues/79). +### Partial support for Helm chart + +Interactive web terminals are partially supported in `gitlab-runner` Helm chart. +They are enabled when: + +- The number of replica is one +- You use the `loadBalancer` service + +Support for fixing these limitations is tracked in the following issues: + +- [Support of more than one replica](https://gitlab.com/gitlab-org/charts/gitlab-runner/-/issues/323) +- [Support of more service types](https://gitlab.com/gitlab-org/charts/gitlab-runner/-/issues/324) ## Debugging a running job diff --git a/doc/integration/sourcegraph.md b/doc/integration/sourcegraph.md index 6f0027aedc7..b2e5f7b4b7d 100644 --- a/doc/integration/sourcegraph.md +++ b/doc/integration/sourcegraph.md @@ -7,8 +7,13 @@ type: reference, how-to # Sourcegraph integration **(FREE)** -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/16556) in GitLab 12.5. -> - Note that this integration is in BETA and deployed [behind a feature flag](#enable-the-sourcegraph-feature-flag) disabled by default. Self-managed instances can opt to enable it. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/16556) in GitLab 12.5 [with a flag](../administration/feature_flags.md) named `sourcegraph`. Disabled by default. +> - Enabled on GitLab.com in GitLab 12.5. +> - [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73116) in GitLab 14.8. + +FLAG: +On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the feature flag](../administration/feature_flags.md) named `sourcegraph`. +On GitLab.com, this feature is available for public projects only. [Sourcegraph](https://sourcegraph.com) provides code intelligence features, natively integrated into the GitLab UI. @@ -26,41 +31,9 @@ you can choose to enable Sourcegraph [through your user preferences](#enable-sou ## Set up for self-managed GitLab instances **(FREE SELF)** Before you can enable Sourcegraph code intelligence in GitLab you must: +configure a Sourcegraph instance with your GitLab instance as an external service. -- Enable the `sourcegraph` feature flag for your GitLab instance. -- Configure a Sourcegraph instance with your GitLab instance as an external service. - -### Enable the Sourcegraph feature flag - -NOTE: -If you are running a self-managed instance, the Sourcegraph integration is unavailable -unless the feature flag `sourcegraph` is enabled. This can be done from the Rails console -by instance administrators. - -Use these commands to start the Rails console: - -```shell -# Omnibus GitLab -gitlab-rails console - -# Installation from source -cd /home/git/gitlab -sudo -u git -H bin/rails console -e production -``` - -Then run the following command to enable the feature flag: - -```ruby -Feature.enable(:sourcegraph) -``` - -You can also enable the feature flag only for specific projects with: - -```ruby -Feature.enable(:sourcegraph, Project.find_by_full_path('my_group/my_project')) -``` - -### Set up a self-managed Sourcegraph instance +### Set up a self-managed Sourcegraph instance **(FREE SELF)** If you are new to Sourcegraph, head over to the [Sourcegraph installation documentation](https://docs.sourcegraph.com/admin) and get your instance up and running. diff --git a/doc/subscriptions/gitlab_com/index.md b/doc/subscriptions/gitlab_com/index.md index 8db1285b88b..b72fad02b3d 100644 --- a/doc/subscriptions/gitlab_com/index.md +++ b/doc/subscriptions/gitlab_com/index.md @@ -267,10 +267,17 @@ previous period), log in to the [Customers Portal](https://customers.gitlab.com/ If you have difficulty during the renewal process, contact the [Support team](https://support.gitlab.com/hc/en-us/requests/new?ticket_form_id=360000071293) for assistance. -## Change the contact person for your subscription +## Add or change the contacts for your subscription -To change the contact person who manages your subscription, -contact the GitLab [Support team](https://support.gitlab.com/hc/en-us/requests/new?ticket_form_id=360000071293). +Contacts can renew a subscription, cancel a subscription, or transfer the subscription to a different namespace. + +To change the contacts: + +1. Ensure an account exists in the + [Customers Portal](https://customers.gitlab.com/customers/sign_in) for the user you want to add. +1. Verify you have access to at least one of + [these requirements](https://about.gitlab.com/handbook/support/license-and-renewals/workflows/customersdot/associating_purchases.html). +1. [Create a ticket with the Support team](https://support.gitlab.com/hc/en-us/requests/new?ticket_form_id=360000071293). Include any relevant material in your request. ## CI/CD minutes diff --git a/doc/subscriptions/self_managed/index.md b/doc/subscriptions/self_managed/index.md index 24ae97e185d..5ac2c5a5037 100644 --- a/doc/subscriptions/self_managed/index.md +++ b/doc/subscriptions/self_managed/index.md @@ -419,6 +419,18 @@ The following is emailed to you: [Upload the new license](../../user/admin_area/license.md#upload-your-license) to your instance. The new tier takes effect when the new license is uploaded. +## Add or change the contacts for your subscription + +Contacts can renew a subscription, cancel a subscription, or transfer the subscription to a different namespace. + +To change the contacts: + +1. Ensure an account exists in the + [Customers Portal](https://customers.gitlab.com/customers/sign_in) for the user you want to add. +1. Verify you have access to at least one of + [these requirements](https://about.gitlab.com/handbook/support/license-and-renewals/workflows/customersdot/associating_purchases.html). +1. [Create a ticket with the Support team](https://support.gitlab.com/hc/en-us/requests/new?ticket_form_id=360000071293). Include any relevant material in your request. + ## Subscription expiry When your license expires, GitLab locks down features, like Git pushes diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index d8e39d089e4..48157a91477 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -43,6 +43,10 @@ module API # This is a separate method so that EE can alter its behaviour more # easily. + if Feature.enabled?(:rate_limit_gitlab_shell, default_enabled: :yaml) + check_rate_limit!(:gitlab_shell_operation, scope: [params[:action], params[:project], actor.key_or_user]) + end + # Stores some Git-specific env thread-safely env = parse_env Gitlab::Git::HookEnv.set(gl_repository, env) if container diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index b90f1f4da0d..12f1b15f820 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -56,7 +56,8 @@ module Gitlab profile_update_username: { threshold: 10, interval: 1.minute }, update_environment_canary_ingress: { threshold: 1, interval: 1.minute }, auto_rollback_deployment: { threshold: 1, interval: 3.minutes }, - user_email_lookup: { threshold: -> { application_settings.user_email_lookup_limit }, interval: 1.minute } + user_email_lookup: { threshold: -> { application_settings.user_email_lookup_limit }, interval: 1.minute }, + gitlab_shell_operation: { threshold: 600, interval: 1.minute } }.freeze end @@ -64,7 +65,7 @@ module Gitlab # be throttled. # # @param key [Symbol] Key attribute registered in `.rate_limits` - # @param scope [Array] Array of ActiveRecord models to scope throttling to a specific request (e.g. per user per project) + # @param scope [Array] Array of ActiveRecord models, Strings or Symbols to scope throttling to a specific request (e.g. per user per project) # @param threshold [Integer] Optional threshold value to override default one registered in `.rate_limits` # @param users_allowlist [Array] Optional list of usernames to exclude from the limit. This param will only be functional if Scope includes a current user. # @param peek [Boolean] Optional. When true the key will not be incremented but the current throttled state will be returned. diff --git a/lib/gitlab/sourcegraph.rb b/lib/gitlab/sourcegraph.rb index 7ef6ab32bd4..892c4468107 100644 --- a/lib/gitlab/sourcegraph.rb +++ b/lib/gitlab/sourcegraph.rb @@ -8,13 +8,14 @@ module Gitlab end def feature_available? - # The sourcegraph_bundle feature could be conditionally applied, so check if `!off?` - !feature.off? + # The sourcegraph feature could be conditionally applied, so check if `!off?` + # We also can't just check !off? because the ActiveRecord might not exist yet + self.feature_enabled? || !feature.off? end def feature_enabled?(actor = nil) # Some CI jobs grep for Feature.enabled? in our codebase, so it is important this reference stays around. - Feature.enabled?(:sourcegraph, actor) + Feature.enabled?(:sourcegraph, actor, default_enabled: :yaml) end private diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index b01a7902bf2..18c68615637 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -2,41 +2,6 @@ namespace :gitlab do namespace :gitaly do - desc 'Installs gitaly for running tests within gitlab-development-kit' - task :test_install, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args| - inside_gdk = Rails.env.test? && File.exist?(Rails.root.join('../GDK_ROOT')) - - if ENV['FORCE_GITALY_INSTALL'] || !inside_gdk - Rake::Task["gitlab:gitaly:install"].invoke(*args) - - next - end - - gdk_gitaly_dir = ENV.fetch('GDK_GITALY', Rails.root.join('../gitaly')) - - # Our test setup expects a git repo, so clone rather than copy - clone_repo(gdk_gitaly_dir, args.dir, clone_opts: %w[--depth 1]) unless Dir.exist?(args.dir) - - # We assume the GDK gitaly already compiled binaries - build_dir = File.join(gdk_gitaly_dir, '_build') - FileUtils.cp_r(build_dir, args.dir) - - # We assume the GDK gitaly already ran bundle install - bundle_dir = File.join(gdk_gitaly_dir, 'ruby', '.bundle') - FileUtils.cp_r(bundle_dir, File.join(args.dir, 'ruby')) - - # For completeness we copy this for gitaly's make target - ruby_bundle_file = File.join(gdk_gitaly_dir, '.ruby-bundle') - FileUtils.cp_r(ruby_bundle_file, args.dir) - - gitaly_binary = File.join(build_dir, 'bin', 'gitaly') - warn_gitaly_out_of_date!(gitaly_binary, Gitlab::GitalyClient.expected_server_version) - rescue Errno::ENOENT => e - puts "Could not copy files, did you run `gdk update`? Error: #{e.message}" - - raise - end - desc 'GitLab | Gitaly | Clone and checkout gitaly' task :clone, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args| warn_user_is_not_gitlab @@ -60,9 +25,6 @@ Usage: rake "gitlab:gitaly:install[/installation/dir,/storage/path]") storage_paths = { 'default' => args.storage_path } Gitlab::SetupHelper::Gitaly.create_configuration(args.dir, storage_paths) - # In CI we run scripts/gitaly-test-build - next if ENV['CI'].present? - Dir.chdir(args.dir) do Bundler.with_original_env do env = { "RUBYOPT" => nil, "BUNDLE_GEMFILE" => nil } diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_instance_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_instance_level_spec.rb index c58cdec622d..70b31c1beca 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_instance_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_instance_level_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package Registry', :orchestrated, :packages, :reliable, :object_storage do + RSpec.describe 'Package Registry', :orchestrated, :packages, :object_storage do describe 'npm instance level endpoint' do using RSpec::Parameterized::TableSyntax include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_project_level_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_project_level_spec.rb index cec902e073a..e25a742493b 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_project_level_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/npm/npm_project_level_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package Registry', :orchestrated, :packages, :reliable, :object_storage do + RSpec.describe 'Package Registry', :orchestrated, :packages, :object_storage do describe 'npm project level endpoint' do using RSpec::Parameterized::TableSyntax include Runtime::Fixtures diff --git a/scripts/gitaly-test-build b/scripts/gitaly-test-build index e6afadccc7e..adc9b56ca4f 100755 --- a/scripts/gitaly-test-build +++ b/scripts/gitaly-test-build @@ -13,8 +13,6 @@ 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 e7e25a217b2..eed79f75224 100755 --- a/scripts/gitaly-test-spawn +++ b/scripts/gitaly-test-spawn @@ -9,17 +9,11 @@ class GitalyTestSpawn include GitalySetup def run - set_bundler_config - install_gitaly_gems if ENV['CI'] - check_gitaly_config! + install_gitaly_gems - # # Uncomment line below to see all gitaly logs merged into CI trace - # spawn('sleep 1; tail -f log/gitaly-test.log') - - # In local development this pid file is used by rspec. - IO.write(File.expand_path('../tmp/tests/gitaly.pid', __dir__), start_gitaly) - IO.write(File.expand_path('../tmp/tests/gitaly2.pid', __dir__), start_gitaly2) - IO.write(File.expand_path('../tmp/tests/praefect.pid', __dir__), start_praefect) + # Optionally specify the path to the gitaly config toml as first argument. + # Used by workhorse in test. + spawn_gitaly(ARGV[0]) end end diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb index 4ee213eeacf..f438be534fa 100644 --- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb +++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb @@ -178,10 +178,6 @@ RSpec.describe Groups::DependencyProxyForContainersController do subject { get_manifest(tag) } context 'feature enabled' do - before do - enable_dependency_proxy - end - it_behaves_like 'without a token' it_behaves_like 'without permission' it_behaves_like 'feature flag disabled with private group' @@ -270,7 +266,6 @@ RSpec.describe Groups::DependencyProxyForContainersController do let_it_be_with_reload(:group) { create(:group, parent: parent_group) } before do - parent_group.create_dependency_proxy_setting!(enabled: true) group_deploy_token.update_column(:group_id, parent_group.id) end @@ -294,10 +289,6 @@ RSpec.describe Groups::DependencyProxyForContainersController do subject { get_blob } context 'feature enabled' do - before do - enable_dependency_proxy - end - it_behaves_like 'without a token' it_behaves_like 'without permission' it_behaves_like 'feature flag disabled with private group' @@ -341,7 +332,6 @@ RSpec.describe Groups::DependencyProxyForContainersController do let_it_be_with_reload(:group) { create(:group, parent: parent_group) } before do - parent_group.create_dependency_proxy_setting!(enabled: true) group_deploy_token.update_column(:group_id, parent_group.id) end @@ -474,10 +464,6 @@ RSpec.describe Groups::DependencyProxyForContainersController do end end - def enable_dependency_proxy - group.create_dependency_proxy_setting!(enabled: true) - end - def disable_dependency_proxy group.create_dependency_proxy_setting!(enabled: false) end diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index d50f1aa1dd8..7e96e99640a 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -25,6 +25,19 @@ RSpec.describe Projects::Settings::CiCdController do expect(response).to render_template(:show) end + context 'when the FF ci_owned_runners_cross_joins_fix is disabled' do + before do + stub_feature_flags(ci_owned_runners_cross_joins_fix: false) + end + + it 'renders show with 200 status code' do + get :show, params: { namespace_id: project.namespace, project_id: project } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end + end + context 'with CI/CD disabled' do before do project.project_feature.update_attribute(:builds_access_level, ProjectFeature::DISABLED) diff --git a/spec/graphql/mutations/ci/runner/delete_spec.rb b/spec/graphql/mutations/ci/runner/delete_spec.rb index 27e8236d593..9f30c95edd5 100644 --- a/spec/graphql/mutations/ci/runner/delete_spec.rb +++ b/spec/graphql/mutations/ci/runner/delete_spec.rb @@ -5,9 +5,9 @@ require 'spec_helper' RSpec.describe Mutations::Ci::Runner::Delete do include GraphqlHelpers - let_it_be(:user) { create(:user) } let_it_be(:runner) { create(:ci_runner) } + let(:user) { create(:user) } let(:current_ctx) { { current_user: user } } let(:mutation_params) do @@ -46,10 +46,10 @@ RSpec.describe Mutations::Ci::Runner::Delete do end context 'when user can delete owned runner' do - let_it_be(:project) { create(:project, creator_id: user.id) } - let_it_be(:project_runner, reload: true) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } + let!(:project) { create(:project, creator_id: user.id) } + let!(:project_runner) { create(:ci_runner, :project, description: 'Project runner', projects: [project]) } - before_all do + before do project.add_maintainer(user) end @@ -63,10 +63,10 @@ RSpec.describe Mutations::Ci::Runner::Delete do end context 'with more than one associated project' do - let_it_be(:project2) { create(:project, creator_id: user.id) } - let_it_be(:two_projects_runner) { create(:ci_runner, :project, description: 'Two projects runner', projects: [project, project2]) } + let!(:project2) { create(:project, creator_id: user.id) } + let!(:two_projects_runner) { create(:ci_runner, :project, description: 'Two projects runner', projects: [project, project2]) } - before_all do + before do project2.add_maintainer(user) end diff --git a/spec/lib/gitlab/sourcegraph_spec.rb b/spec/lib/gitlab/sourcegraph_spec.rb index 6bebd1ca3e6..e2c1e959cbf 100644 --- a/spec/lib/gitlab/sourcegraph_spec.rb +++ b/spec/lib/gitlab/sourcegraph_spec.rb @@ -37,6 +37,12 @@ RSpec.describe Gitlab::Sourcegraph do it { is_expected.to be_truthy } end + + context 'when feature is disabled' do + let(:feature_scope) { false } + + it { is_expected.to be_falsey } + end end describe '.feature_enabled?' do diff --git a/spec/models/ci/namespace_mirror_spec.rb b/spec/models/ci/namespace_mirror_spec.rb index fd0581341b7..a9d916115fc 100644 --- a/spec/models/ci/namespace_mirror_spec.rb +++ b/spec/models/ci/namespace_mirror_spec.rb @@ -30,6 +30,20 @@ RSpec.describe Ci::NamespaceMirror do end end + describe '.contains_any_of_namespaces' do + let!(:other_group1) { create(:group) } + let!(:other_group2) { create(:group, parent: other_group1) } + let!(:other_group3) { create(:group, parent: other_group2) } + + subject(:result) { described_class.contains_any_of_namespaces([group2.id, other_group2.id]) } + + it 'returns groups having group2.id in traversal_ids' do + expect(result.pluck(:namespace_id)).to contain_exactly( + group2.id, group3.id, group4.id, other_group2.id, other_group3.id + ) + end + end + describe '.by_namespace_id' do subject(:result) { described_class.by_namespace_id(group2.id) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 67db822a6e8..332b6705c14 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2823,6 +2823,26 @@ RSpec.describe Group do end end + describe '#dependency_proxy_setting' do + subject(:setting) { group.dependency_proxy_setting } + + it 'builds a new policy if one does not exist', :aggregate_failures do + expect(setting.enabled).to eq(true) + expect(setting).not_to be_persisted + end + + context 'with existing policy' do + before do + group.dependency_proxy_setting.update!(enabled: false) + end + + it 'returns the policy if it already exists', :aggregate_failures do + expect(setting.enabled).to eq(false) + expect(setting).to be_persisted + end + end + end + describe '#crm_enabled?' do it 'returns false where no crm_settings exist' do expect(group.crm_enabled?).to be_falsey diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ad4873950e1..93039a2118a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3967,7 +3967,7 @@ RSpec.describe User do end end - describe '#ci_owned_runners' do + shared_context '#ci_owned_runners' do let(:user) { create(:user) } shared_examples :nested_groups_owner do @@ -4274,6 +4274,16 @@ RSpec.describe User do end end + it_behaves_like '#ci_owned_runners' + + context 'when FF ci_owned_runners_cross_joins_fix is disabled' do + before do + stub_feature_flags(ci_owned_runners_cross_joins_fix: false) + end + + it_behaves_like '#ci_owned_runners' + end + describe '#projects_with_reporter_access_limited_to' do let(:project1) { create(:project) } let(:project2) { create(:project) } diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index cbb27575ea9..d7f688bdf9c 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -909,7 +909,6 @@ RSpec.describe GroupPolicy do context 'feature enabled' do before do stub_config(dependency_proxy: { enabled: true }) - group.create_dependency_proxy_setting!(enabled: true) end context 'reporter' do @@ -955,7 +954,6 @@ RSpec.describe GroupPolicy do before do stub_config(dependency_proxy: { enabled: true }) - group.create_dependency_proxy_setting!(enabled: true) end it { is_expected.to be_allowed(:read_dependency_proxy) } diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 0a71eb43f81..9aa8aaafc68 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -372,7 +372,38 @@ RSpec.describe API::Internal::Base do end end - describe "POST /internal/allowed", :clean_gitlab_redis_shared_state do + describe "POST /internal/allowed", :clean_gitlab_redis_shared_state, :clean_gitlab_redis_rate_limiting do + shared_examples 'rate limited request' do + let(:action) { 'git-upload-pack' } + let(:actor) { key } + + it 'is throttled by rate limiter' do + allow(::Gitlab::ApplicationRateLimiter).to receive(:threshold).and_return(1) + expect(::Gitlab::ApplicationRateLimiter).to receive(:throttled?).with(:gitlab_shell_operation, scope: [action, project.full_path, actor]).twice.and_call_original + + request + + expect(response).to have_gitlab_http_status(:ok) + + request + + expect(response).to have_gitlab_http_status(:too_many_requests) + expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') + end + + context 'when rate_limit_gitlab_shell feature flag is disabled' do + before do + stub_feature_flags(rate_limit_gitlab_shell: false) + end + + it 'is not throttled by rate limiter' do + expect(::Gitlab::ApplicationRateLimiter).not_to receive(:throttled?) + + subject + end + end + end + context "access granted" do let(:env) { {} } @@ -530,6 +561,32 @@ RSpec.describe API::Internal::Base do expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true') expect(user.reload.last_activity_on).to eql(Date.today) end + + it_behaves_like 'rate limited request' do + def request + pull(key, project) + end + end + + context 'when user_id is passed' do + it_behaves_like 'rate limited request' do + let(:actor) { user } + + def request + post( + api("/internal/allowed"), + params: { + user_id: user.id, + project: full_path_for(project), + gl_repository: gl_repository_for(project), + action: 'git-upload-pack', + secret_token: secret_token, + protocol: 'ssh' + } + ) + end + end + end end context "with a feature flag enabled for a project" do @@ -576,6 +633,14 @@ RSpec.describe API::Internal::Base do expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) expect(user.reload.last_activity_on).to be_nil end + + it_behaves_like 'rate limited request' do + let(:action) { 'git-receive-pack' } + + def request + push(key, project) + end + end end context 'when receive_max_input_size has been updated' do @@ -838,6 +903,14 @@ RSpec.describe API::Internal::Base do expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) end + + it_behaves_like 'rate limited request' do + let(:action) { 'git-upload-archive' } + + def request + archive(key, project) + end + end end context "not added to project" do diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index d953d37221c..793438808a5 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -499,9 +499,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac before do group.add_owner(user) - group.create_dependency_proxy_setting!(enabled: true) other_group.add_owner(other_user) - other_group.create_dependency_proxy_setting!(enabled: true) allow(Gitlab.config.dependency_proxy) .to receive(:enabled).and_return(true) diff --git a/spec/services/import/validate_remote_git_endpoint_service_spec.rb b/spec/services/import/validate_remote_git_endpoint_service_spec.rb index fbd8a3cb323..9dc862b6ca3 100644 --- a/spec/services/import/validate_remote_git_endpoint_service_spec.rb +++ b/spec/services/import/validate_remote_git_endpoint_service_spec.rb @@ -24,6 +24,17 @@ RSpec.describe Import::ValidateRemoteGitEndpointService do expect(Gitlab::HTTP).to have_received(:get).with(endpoint_url, basic_auth: nil, stream_body: true, follow_redirects: false) end + context 'when uri is using git:// protocol' do + subject { described_class.new(url: 'git://demo.host/repo')} + + it 'returns success' do + result = subject.execute + + expect(result).to be_a(ServiceResponse) + expect(result.success?).to be(true) + end + end + context 'when receiving HTTP response' do subject { described_class.new(url: base_url) } diff --git a/spec/support/helpers/gitaly_setup.rb b/spec/support/helpers/gitaly_setup.rb index 923051a2e04..905c439f4d9 100644 --- a/spec/support/helpers/gitaly_setup.rb +++ b/spec/support/helpers/gitaly_setup.rb @@ -9,8 +9,13 @@ require 'securerandom' require 'socket' require 'logger' +require 'bundler' module GitalySetup + extend self + + REPOS_STORAGE = 'default' + LOGGER = begin default_name = ENV['CI'] ? 'DEBUG' : 'WARN' level_name = ENV['GITLAB_TESTING_LOG_LEVEL']&.upcase @@ -52,11 +57,13 @@ module GitalySetup def env { - 'HOME' => expand_path('tmp/tests'), 'GEM_PATH' => Gem.path.join(':'), - 'BUNDLE_APP_CONFIG' => File.join(gemfile_dir, '.bundle'), 'BUNDLE_INSTALL_FLAGS' => nil, + 'BUNDLE_IGNORE_CONFIG' => '1', + 'BUNDLE_PATH' => bundle_path, 'BUNDLE_GEMFILE' => gemfile, + 'BUNDLE_JOBS' => '4', + 'BUNDLE_RETRY' => '3', 'RUBYOPT' => nil, # Git hooks can't run during tests as the internal API is not running. @@ -65,17 +72,20 @@ module GitalySetup } end - # 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) + def bundle_path + # Allow the user to override BUNDLE_PATH if they need to + return ENV['GITALY_TEST_BUNDLE_PATH'] if ENV['GITALY_TEST_BUNDLE_PATH'] if ENV['CI'] - bundle_path = expand_path('vendor/gitaly-ruby') - system('bundle', 'config', 'set', '--local', 'path', bundle_path, chdir: gemfile_dir) + expand_path('vendor/gitaly-ruby') + else + explicit_path = Bundler.configured_bundle_path.explicit_path + + return unless explicit_path + + expand_path(explicit_path) end end - # rubocop:enable GitlabSecurity/SystemCommandInjection def config_path(service) case service @@ -88,6 +98,10 @@ module GitalySetup end end + def repos_path(storage = REPOS_STORAGE) + Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path + end + def service_binary(service) case service when :gitaly, :gitaly2 @@ -97,16 +111,20 @@ module GitalySetup end end + def run_command(cmd, env: {}) + system(env, *cmd, exception: true, chdir: tmp_tests_gitaly_dir) + end + def install_gitaly_gems - system(env, "make #{tmp_tests_gitaly_dir}/.ruby-bundle", chdir: tmp_tests_gitaly_dir) # rubocop:disable GitlabSecurity/SystemCommandInjection + run_command(%W[make #{tmp_tests_gitaly_dir}/.ruby-bundle], env: env) end def build_gitaly - system(env.merge({ 'GIT_VERSION' => nil }), 'make all git', chdir: tmp_tests_gitaly_dir) # rubocop:disable GitlabSecurity/SystemCommandInjection + run_command(%w[make all git], env: env.merge('GIT_VERSION' => nil)) end - def start_gitaly - start(:gitaly) + def start_gitaly(toml = nil) + start(:gitaly, toml) end def start_gitaly2 @@ -117,14 +135,20 @@ module GitalySetup start(:praefect) end - def start(service) + def start(service, toml = nil) + toml ||= config_path(service) args = ["#{tmp_tests_gitaly_bin_dir}/#{service_binary(service)}"] args.push("-config") if service == :praefect - args.push(config_path(service)) + args.push(toml) + + # Ensure user configuration does not affect Git + # Context: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58776#note_547613780 + env = self.env.merge('HOME' => nil, 'XDG_CONFIG_HOME' => nil) + pid = spawn(env, *args, [:out, :err] => "log/#{service}-test.log") begin - try_connect!(service) + try_connect!(service, toml) rescue StandardError Process.kill('TERM', pid) raise @@ -161,29 +185,37 @@ module GitalySetup abort 'bundle check failed' unless system(env, 'bundle', 'check', out: out, chdir: gemfile_dir) end - def read_socket_path(service) + def connect_proc(toml) # This code needs to work in an environment where we cannot use bundler, # so we cannot easily use the toml-rb gem. This ad-hoc parser should be # good enough. - config_text = IO.read(config_path(service)) + config_text = IO.read(toml) config_text.lines.each do |line| - match_data = line.match(/^\s*socket_path\s*=\s*"([^"]*)"$/) + match_data = line.match(/^\s*(socket_path|listen_addr)\s*=\s*"([^"]*)"$/) - return match_data[1] if match_data + next unless match_data + + case match_data[1] + when 'socket_path' + return -> { UNIXSocket.new(match_data[2]) } + when 'listen_addr' + addr, port = match_data[2].split(':') + return -> { TCPSocket.new(addr, port.to_i) } + end end - raise "failed to find socket_path in #{config_path(service)}" + raise "failed to find socket_path or listen_addr in #{toml}" end - def try_connect!(service) + def try_connect!(service, toml) LOGGER.debug "Trying to connect to #{service}: " timeout = 20 delay = 0.1 - socket = read_socket_path(service) + connect = connect_proc(toml) Integer(timeout / delay).times do - UNIXSocket.new(socket) + connect.call LOGGER.debug " OK\n" return @@ -194,6 +226,128 @@ module GitalySetup LOGGER.warn " FAILED to connect to #{service}\n" - raise "could not connect to #{socket}" + raise "could not connect to #{service}" + end + + def gitaly_socket_path + Gitlab::GitalyClient.address(REPOS_STORAGE).delete_prefix('unix:') + end + + def gitaly_dir + socket_path = gitaly_socket_path + socket_path = File.expand_path(gitaly_socket_path) if expand_path_for_socket? + + File.dirname(socket_path) + end + + # Linux fails with "bind: invalid argument" if a UNIX socket path exceeds 108 characters: + # https://github.com/golang/go/issues/6895. We use absolute paths in CI to ensure + # that changes in the current working directory don't affect GRPC reconnections. + def expand_path_for_socket? + !!ENV['CI'] + end + + def setup_gitaly + unless ENV['CI'] + # In CI Gitaly is built in the setup-test-env job and saved in the + # artifacts. So when tests are started, there's no need to build Gitaly. + build_gitaly + end + + Gitlab::SetupHelper::Gitaly.create_configuration( + gitaly_dir, + { 'default' => repos_path }, + force: true, + options: { + prometheus_listen_addr: 'localhost:9236' + } + ) + Gitlab::SetupHelper::Gitaly.create_configuration( + gitaly_dir, + { 'default' => repos_path }, + force: true, + options: { + internal_socket_dir: File.join(gitaly_dir, "internal_gitaly2"), + gitaly_socket: "gitaly2.socket", + config_filename: "gitaly2.config.toml" + } + ) + Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true) + end + + def socket_path(service) + File.join(tmp_tests_gitaly_dir, "#{service}.socket") + end + + def praefect_socket_path + "unix:" + socket_path(:praefect) + end + + def stop(pid) + Process.kill('KILL', pid) + rescue Errno::ESRCH + # The process can already be gone if the test run was INTerrupted. + end + + def spawn_gitaly(toml = nil) + check_gitaly_config! + + pids = [] + + if toml + pids << start_gitaly(toml) + else + pids << start_gitaly + pids << start_gitaly2 + pids << start_praefect + end + + Kernel.at_exit do + # In CI, this function is called by scripts/gitaly-test-spawn, triggered + # in a before_script. Gitaly needs to remain running until the container + # is stopped. + next if ENV['CI'] + # In Workhorse tests (locally or in CI), this function is called by + # scripts/gitaly-test-spawn during `make test`. Gitaly needs to remain + # running until `make test` cleans it up. + next if ENV['GITALY_PID_FILE'] + + pids.each { |pid| stop(pid) } + end + rescue StandardError + raise gitaly_failure_message + end + + def gitaly_failure_message + message = "gitaly spawn failed\n\n" + + message += "- The `gitaly` binary does not exist: #{gitaly_binary}\n" unless File.exist?(gitaly_binary) + message += "- The `praefect` binary does not exist: #{praefect_binary}\n" unless File.exist?(praefect_binary) + message += "- The `git` binary does not exist: #{git_binary}\n" unless File.exist?(git_binary) + + message += "\nCheck log/gitaly-test.log for errors.\n" + + unless ci? + message += "\nIf binaries are missing, try running `make -C tmp/tests/gitaly build git.`\n" + message += "\nOtherwise, try running `rm -rf #{tmp_tests_gitaly_dir}`." + end + + message + end + + def git_binary + File.join(tmp_tests_gitaly_dir, "_build", "deps", "git", "install", "bin", "git") + end + + def gitaly_binary + File.join(tmp_tests_gitaly_dir, "_build", "bin", "gitaly") + end + + def praefect_binary + File.join(tmp_tests_gitaly_dir, "_build", "bin", "praefect") + end + + def git_binary_exists? + File.exist?(git_binary) end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index c571962f739..5c3ca92c4d0 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'parallel' +require_relative 'gitaly_setup' module TestEnv extend self @@ -93,7 +94,6 @@ module TestEnv }.freeze TMP_TEST_PATH = Rails.root.join('tmp', 'tests').freeze - REPOS_STORAGE = 'default' SECOND_STORAGE_PATH = Rails.root.join('tmp', 'tests', 'second_storage') SETUP_METHODS = %i[setup_gitaly setup_gitlab_shell setup_workhorse setup_factory_repo setup_forked_repo].freeze @@ -128,7 +128,7 @@ module TestEnv # Can be overriden def post_init - start_gitaly(gitaly_dir) + start_gitaly end # Clean /tmp/tests @@ -142,7 +142,7 @@ module TestEnv end FileUtils.mkdir_p( - Gitlab::GitalyClient::StorageSettings.allow_disk_access { TestEnv.repos_path } + Gitlab::GitalyClient::StorageSettings.allow_disk_access { GitalySetup.repos_path } ) FileUtils.mkdir_p(SECOND_STORAGE_PATH) FileUtils.mkdir_p(backup_path) @@ -159,109 +159,28 @@ module TestEnv def setup_gitaly component_timed_setup('Gitaly', - install_dir: gitaly_dir, + install_dir: GitalySetup.gitaly_dir, version: Gitlab::GitalyClient.expected_server_version, - task: "gitlab:gitaly:test_install", - task_args: [gitaly_dir, repos_path, gitaly_url].compact) do - Gitlab::SetupHelper::Gitaly.create_configuration( - gitaly_dir, - { 'default' => repos_path }, - force: true, - options: { - prometheus_listen_addr: 'localhost:9236' - } - ) - Gitlab::SetupHelper::Gitaly.create_configuration( - gitaly_dir, - { 'default' => repos_path }, - force: true, - options: { - internal_socket_dir: File.join(gitaly_dir, "internal_gitaly2"), - gitaly_socket: "gitaly2.socket", - config_filename: "gitaly2.config.toml" - } - ) - Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true) - end + task: "gitlab:gitaly:clone", + fresh_install: ENV.key?('FORCE_GITALY_INSTALL'), + task_args: [GitalySetup.gitaly_dir, GitalySetup.repos_path, gitaly_url].compact) do + GitalySetup.setup_gitaly + end end - def gitaly_socket_path - Gitlab::GitalyClient.address('default').sub(/\Aunix:/, '') - end - - def gitaly_dir - socket_path = gitaly_socket_path - socket_path = File.expand_path(gitaly_socket_path) if expand_path? - - File.dirname(socket_path) - end - - # Linux fails with "bind: invalid argument" if a UNIX socket path exceeds 108 characters: - # https://github.com/golang/go/issues/6895. We use absolute paths in CI to ensure - # that changes in the current working directory don't affect GRPC reconnections. - def expand_path? - !!ENV['CI'] - end - - def start_gitaly(gitaly_dir) + def start_gitaly if ci? # Gitaly has been spawned outside this process already return end - spawn_script = Rails.root.join('scripts/gitaly-test-spawn').to_s - Bundler.with_original_env do - unless system(spawn_script) - raise gitaly_failure_message - end - end - - gitaly_pid = Integer(File.read(TMP_TEST_PATH.join('gitaly.pid'))) - gitaly2_pid = Integer(File.read(TMP_TEST_PATH.join('gitaly2.pid'))) - praefect_pid = Integer(File.read(TMP_TEST_PATH.join('praefect.pid'))) - - Kernel.at_exit do - pids = [gitaly_pid, gitaly2_pid, praefect_pid] - pids.each { |pid| stop(pid) } - end - - wait('gitaly') - wait('praefect') - end - - def stop(pid) - Process.kill('KILL', pid) - rescue Errno::ESRCH - # The process can already be gone if the test run was INTerrupted. + GitalySetup.spawn_gitaly end def gitaly_url ENV.fetch('GITALY_REPO_URL', nil) end - def socket_path(service) - TMP_TEST_PATH.join('gitaly', "#{service}.socket").to_s - end - - def praefect_socket_path - "unix:" + socket_path(:praefect) - end - - def wait(service) - sleep_time = 10 - sleep_interval = 0.1 - socket = socket_path(service) - - Integer(sleep_time / sleep_interval).times do - Socket.unix(socket) - return - rescue StandardError - sleep sleep_interval - end - - raise "could not connect to #{service} at #{socket.inspect} after #{sleep_time} seconds" - end - # Feature specs are run through Workhorse def setup_workhorse # Always rebuild the config file @@ -377,8 +296,7 @@ module TestEnv def rm_storage_dir(storage, dir) Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path - target_repo_refs_path = File.join(repos_path, dir) + target_repo_refs_path = File.join(GitalySetup.repos_path(storage), dir) FileUtils.remove_dir(target_repo_refs_path) end rescue Errno::ENOENT @@ -386,8 +304,7 @@ module TestEnv def storage_dir_exists?(storage, dir) Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repos_path = Gitlab.config.repositories.storages[storage].legacy_disk_path - File.exist?(File.join(repos_path, dir)) + File.exist?(File.join(GitalySetup.repos_path(storage), dir)) end end @@ -400,7 +317,7 @@ module TestEnv end def repos_path - @repos_path ||= Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path + @repos_path ||= GitalySetup.repos_path end def backup_path @@ -525,7 +442,7 @@ module TestEnv end end - def component_timed_setup(component, install_dir:, version:, task:, task_args: []) + def component_timed_setup(component, install_dir:, version:, task:, fresh_install: true, task_args: []) start = Time.now ensure_component_dir_name_is_correct!(component, install_dir) @@ -535,7 +452,7 @@ module TestEnv if component_needs_update?(install_dir, version) # Cleanup the component entirely to ensure we start fresh - FileUtils.rm_rf(install_dir) + FileUtils.rm_rf(install_dir) if fresh_install if ENV['SKIP_RAILS_ENV_IN_RAKE'] # When we run `scripts/setup-test-env`, we take care of loading the necessary dependencies @@ -614,39 +531,6 @@ module TestEnv expected_version == sha.chomp end - - def gitaly_failure_message - message = "gitaly spawn failed\n\n" - - message += "- The `gitaly` binary does not exist: #{gitaly_binary}\n" unless File.exist?(gitaly_binary) - message += "- The `praefect` binary does not exist: #{praefect_binary}\n" unless File.exist?(praefect_binary) - message += "- The `git` binary does not exist: #{git_binary}\n" unless File.exist?(git_binary) - - message += "\nCheck log/gitaly-test.log for errors.\n" - - unless ci? - message += "\nIf binaries are missing, try running `make -C tmp/tests/gitaly build git.`\n" - message += "\nOtherwise, try running `rm -rf #{gitaly_dir}`." - end - - message - end - - def git_binary - File.join(gitaly_dir, "_build", "deps", "git", "install", "bin", "git") - end - - def gitaly_binary - File.join(gitaly_dir, "_build", "bin", "gitaly") - end - - def praefect_binary - File.join(gitaly_dir, "_build", "bin", "praefect") - end - - def git_binary_exists? - File.exist?(git_binary) - end end require_relative('../../../ee/spec/support/helpers/ee/test_env') if Gitlab.ee? diff --git a/spec/support/praefect.rb b/spec/support/praefect.rb index 3218275c2aa..451b47cc83c 100644 --- a/spec/support/praefect.rb +++ b/spec/support/praefect.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -require_relative 'helpers/test_env' +require_relative 'helpers/gitaly_setup' RSpec.configure do |config| config.before(:each, :praefect) do allow(Gitlab.config.repositories.storages['default']).to receive(:[]).and_call_original allow(Gitlab.config.repositories.storages['default']).to receive(:[]).with('gitaly_address') - .and_return(TestEnv.praefect_socket_path) + .and_return(GitalySetup.praefect_socket_path) end end diff --git a/workhorse/Makefile b/workhorse/Makefile index 890d460adbc..031fe581d28 100644 --- a/workhorse/Makefile +++ b/workhorse/Makefile @@ -106,7 +106,7 @@ run-gitaly: $(GITALY_PID_FILE) $(GITALY_PID_FILE): gitaly.toml $(call message, "Starting gitaly") - cd ..; GITALY_TESTING_NO_GIT_HOOKS=1 GITALY_PID_FILE=workhorse/$(GITALY_PID_FILE) $(GITALY) workhorse/gitaly.toml & + cd ..; GITALY_TESTING_NO_GIT_HOOKS=1 GITALY_PID_FILE=workhorse/$(GITALY_PID_FILE) scripts/gitaly-test-spawn workhorse/gitaly.toml gitaly.toml: ../tmp/tests/gitaly/config.toml sed -e 's/^socket_path.*$$/listen_addr = "0.0.0.0:8075"/;s/^\[auth\]$$//;s/^token.*$$//;s/^internal_socket_dir.*$$//' \