diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-07-26 09:09:00 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-07-26 09:09:00 +0000 |
commit | ec3483bd1831dfca0c7baec0f8aad9079d28907d (patch) | |
tree | 67b6f8eed91c44ab4054191d2a2fbf9d18cc695e | |
parent | d10e03ba6f8d5d425e53931d306a099404a5958b (diff) | |
download | gitlab-ce-ec3483bd1831dfca0c7baec0f8aad9079d28907d.tar.gz |
Add latest changes from gitlab-org/gitlab@master
37 files changed, 277 insertions, 606 deletions
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index afa383b28d9..eda1ba49254 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -641,36 +641,33 @@ rspec-ee unit pg12 geo: - .rails:rules:ee-only-unit - .rspec-ee-unit-geo-parallel -# FIXME: Temporarily disable geo minimal rspec jobs https://gitlab.com/gitlab-org/gitlab/-/issues/294212 -#rspec-ee unit pg12 geo minimal: -# extends: -# - rspec-ee unit pg12 geo -# - .minimal-rspec-tests -# - .rails:rules:ee-only-unit:minimal +rspec-ee unit pg12 geo minimal: + extends: + - rspec-ee unit pg12 geo + - .minimal-rspec-tests + - .rails:rules:ee-only-unit:minimal rspec-ee integration pg12 geo: extends: - .rspec-ee-base-geo-pg12 - .rails:rules:ee-only-integration -# FIXME: Temporarily disable geo minimal rspec jobs https://gitlab.com/gitlab-org/gitlab/-/issues/294212 -#rspec-ee integration pg12 geo minimal: -# extends: -# - rspec-ee integration pg12 geo -# - .minimal-rspec-tests -# - .rails:rules:ee-only-integration:minimal +rspec-ee integration pg12 geo minimal: + extends: + - rspec-ee integration pg12 geo + - .minimal-rspec-tests + - .rails:rules:ee-only-integration:minimal rspec-ee system pg12 geo: extends: - .rspec-ee-base-geo-pg12 - .rails:rules:ee-only-system -# FIXME: Temporarily disable geo minimal rspec jobs https://gitlab.com/gitlab-org/gitlab/-/issues/294212 -#rspec-ee system pg12 geo minimal: -# extends: -# - rspec-ee system pg12 geo -# - .minimal-rspec-tests -# - .rails:rules:ee-only-system:minimal +rspec-ee system pg12 geo minimal: + extends: + - rspec-ee system pg12 geo + - .minimal-rspec-tests + - .rails:rules:ee-only-system:minimal db:rollback geo: extends: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index c74865b9fc5..82611ee713f 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -34,6 +34,15 @@ .if-merge-request: &if-merge-request if: '$CI_MERGE_REQUEST_IID' +.if-merge-request-approved: &if-merge-request-approved + if: '$CI_MERGE_REQUEST_IID && $CI_MERGE_REQUEST_APPROVED' + +.if-merge-request-not-approved: &if-merge-request-not-approved + if: '$CI_MERGE_REQUEST_IID && $CI_MERGE_REQUEST_APPROVED != "true"' + +.if-automated-merge-request: &if-automated-merge-request + if: '$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME == "release-tools/update-gitaly" || $CI_MERGE_REQUEST_TARGET_BRANCH_NAME =~ /stable-ee$/' + .if-merge-request-title-as-if-foss: &if-merge-request-title-as-if-foss if: '$CI_MERGE_REQUEST_TITLE =~ /RUN AS-IF-FOSS/' @@ -70,9 +79,6 @@ .if-cache-credentials-schedule: &if-cache-credentials-schedule if: '$CI_REPO_CACHE_CREDENTIALS && $CI_PIPELINE_SOURCE == "schedule"' -.if-merge-request-rspec-minimal-disabled: &if-merge-request-rspec-minimal-disabled - if: '$CI_MERGE_REQUEST_IID && $RSPEC_MINIMAL_ENABLED != "true"' - .if-rspec-fail-fast-disabled: &if-rspec-fail-fast-disabled if: '$RSPEC_FAIL_FAST_ENABLED != "true"' @@ -612,12 +618,18 @@ ############### .rails:rules:ee-and-foss-migration: rules: - - changes: *db-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *db-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *db-patterns .rails:rules:ee-and-foss-migration:minimal: rules: - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -643,12 +655,18 @@ .rails:rules:ee-and-foss-unit: rules: - - changes: *backend-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *backend-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *backend-patterns .rails:rules:ee-and-foss-unit:minimal: rules: - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -660,12 +678,18 @@ .rails:rules:ee-and-foss-integration: rules: - - changes: *backend-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *backend-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *backend-patterns .rails:rules:ee-and-foss-integration:minimal: rules: - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -677,12 +701,18 @@ .rails:rules:ee-and-foss-system: rules: - - changes: *code-backstage-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *code-backstage-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *code-backstage-patterns .rails:rules:ee-and-foss-system:minimal: rules: - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -694,12 +724,18 @@ .rails:rules:ee-and-foss-fast_spec_helper: rules: - - changes: ["config/**/*"] - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: ["config/**/*"] + - <<: *if-merge-request-not-approved + when: never + - changes: ["config/**/*"] .rails:rules:ee-and-foss-fast_spec_helper:minimal: rules: - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -718,14 +754,20 @@ rules: - <<: *if-not-ee when: never - - changes: *db-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *db-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *db-patterns .rails:rules:ee-only-migration:minimal: rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -739,14 +781,20 @@ rules: - <<: *if-not-ee when: never - - changes: *backend-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *backend-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *backend-patterns .rails:rules:ee-only-unit:minimal: rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -760,14 +808,20 @@ rules: - <<: *if-not-ee when: never - - changes: *backend-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *backend-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *backend-patterns .rails:rules:ee-only-integration:minimal: rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -781,14 +835,20 @@ rules: - <<: *if-not-ee when: never - - changes: *code-backstage-patterns - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *code-backstage-patterns + - <<: *if-merge-request-not-approved + when: never + - changes: *code-backstage-patterns .rails:rules:ee-only-system:minimal: rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request-title-run-all-rspec when: never @@ -802,11 +862,15 @@ rules: - <<: *if-not-ee when: never + - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *db-patterns + - <<: *if-merge-request-not-approved + when: never - <<: *if-security-merge-request changes: *db-patterns - <<: *if-merge-request-title-as-if-foss changes: *db-patterns - - <<: *if-merge-request-title-run-all-rspec - <<: *if-merge-request changes: *ci-patterns @@ -814,7 +878,9 @@ rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request changes: *ci-patterns @@ -823,17 +889,20 @@ changes: *db-patterns - <<: *if-merge-request-title-as-if-foss changes: *db-patterns - - <<: *if-merge-request-title-run-all-rspec .rails:rules:as-if-foss-unit: rules: - <<: *if-not-ee when: never + - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *backend-patterns + - <<: *if-merge-request-not-approved + when: never - <<: *if-security-merge-request changes: *backend-patterns - <<: *if-merge-request-title-as-if-foss changes: *backend-patterns - - <<: *if-merge-request-title-run-all-rspec - <<: *if-merge-request changes: *ci-patterns @@ -841,7 +910,9 @@ rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request changes: *ci-patterns @@ -850,17 +921,20 @@ changes: *backend-patterns - <<: *if-merge-request-title-as-if-foss changes: *backend-patterns - - <<: *if-merge-request-title-run-all-rspec .rails:rules:as-if-foss-integration: rules: - <<: *if-not-ee when: never + - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *backend-patterns + - <<: *if-merge-request-not-approved + when: never - <<: *if-security-merge-request changes: *backend-patterns - <<: *if-merge-request-title-as-if-foss changes: *backend-patterns - - <<: *if-merge-request-title-run-all-rspec - <<: *if-merge-request changes: *ci-patterns @@ -868,7 +942,9 @@ rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request changes: *ci-patterns @@ -877,17 +953,20 @@ changes: *backend-patterns - <<: *if-merge-request-title-as-if-foss changes: *backend-patterns - - <<: *if-merge-request-title-run-all-rspec .rails:rules:as-if-foss-system: rules: - <<: *if-not-ee when: never + - <<: *if-merge-request-title-run-all-rspec + - <<: *if-automated-merge-request + changes: *code-backstage-patterns + - <<: *if-merge-request-not-approved + when: never - <<: *if-security-merge-request changes: *code-backstage-patterns - <<: *if-merge-request-title-as-if-foss changes: *code-backstage-patterns - - <<: *if-merge-request-title-run-all-rspec - <<: *if-merge-request changes: *ci-patterns @@ -895,7 +974,9 @@ rules: - <<: *if-not-ee when: never - - <<: *if-merge-request-rspec-minimal-disabled + - <<: *if-merge-request-approved + when: never + - <<: *if-automated-merge-request when: never - <<: *if-merge-request changes: *ci-patterns @@ -904,7 +985,6 @@ changes: *code-backstage-patterns - <<: *if-merge-request-title-as-if-foss changes: *code-backstage-patterns - - <<: *if-merge-request-title-run-all-rspec .rails:rules:ee-and-foss-db-library-code: rules: diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 7c861ff4f25..4f8358977e3 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -88,57 +88,6 @@ Graphql/OldTypes: - 'ee/app/graphql/ee/types/merge_request_type.rb' - 'ee/app/graphql/ee/types/namespace_type.rb' - 'ee/app/graphql/ee/types/project_type.rb' - - 'ee/app/graphql/mutations/app_sec/fuzzing/api/ci_configuration/create.rb' - - 'ee/app/graphql/mutations/boards/epic_boards/create.rb' - - 'ee/app/graphql/mutations/boards/epics/create.rb' - - 'ee/app/graphql/mutations/boards/lists/update_limit_metrics.rb' - - 'ee/app/graphql/mutations/boards/scoped_issue_board_arguments.rb' - - 'ee/app/graphql/mutations/boards/update_epic_user_preferences.rb' - - 'ee/app/graphql/mutations/clusters/agent_tokens/create.rb' - - 'ee/app/graphql/mutations/clusters/agents/create.rb' - - 'ee/app/graphql/mutations/compliance_management/frameworks/create.rb' - - 'ee/app/graphql/mutations/concerns/mutations/shared_epic_arguments.rb' - - 'ee/app/graphql/mutations/dast/profiles/create.rb' - - 'ee/app/graphql/mutations/dast/profiles/run.rb' - - 'ee/app/graphql/mutations/dast/profiles/update.rb' - - 'ee/app/graphql/mutations/dast_on_demand_scans/create.rb' - - 'ee/app/graphql/mutations/dast_scanner_profiles/create.rb' - - 'ee/app/graphql/mutations/dast_scanner_profiles/delete.rb' - - 'ee/app/graphql/mutations/dast_scanner_profiles/update.rb' - - 'ee/app/graphql/mutations/dast_site_profiles/create.rb' - - 'ee/app/graphql/mutations/dast_site_profiles/delete.rb' - - 'ee/app/graphql/mutations/dast_site_profiles/update.rb' - - 'ee/app/graphql/mutations/dast_site_tokens/create.rb' - - 'ee/app/graphql/mutations/dast_site_validations/create.rb' - - 'ee/app/graphql/mutations/dast_site_validations/revoke.rb' - - 'ee/app/graphql/mutations/epics/add_issue.rb' - - 'ee/app/graphql/mutations/epics/base.rb' - - 'ee/app/graphql/mutations/epics/set_subscription.rb' - - 'ee/app/graphql/mutations/gitlab_subscriptions/activate.rb' - - 'ee/app/graphql/mutations/incident_management/escalation_policy/create.rb' - - 'ee/app/graphql/mutations/incident_management/escalation_policy/update.rb' - - 'ee/app/graphql/mutations/incident_management/oncall_rotation/create.rb' - - 'ee/app/graphql/mutations/incident_management/oncall_rotation/destroy.rb' - - 'ee/app/graphql/mutations/incident_management/oncall_rotation/update.rb' - - 'ee/app/graphql/mutations/incident_management/oncall_schedule/create.rb' - - 'ee/app/graphql/mutations/incident_management/oncall_schedule/destroy.rb' - - 'ee/app/graphql/mutations/incident_management/oncall_schedule/update.rb' - - 'ee/app/graphql/mutations/issues/common_ee_mutation_arguments.rb' - - 'ee/app/graphql/mutations/issues/promote_to_epic.rb' - - 'ee/app/graphql/mutations/issues/set_weight.rb' - - 'ee/app/graphql/mutations/iterations/cadences/create.rb' - - 'ee/app/graphql/mutations/iterations/cadences/update.rb' - - 'ee/app/graphql/mutations/iterations/create.rb' - - 'ee/app/graphql/mutations/iterations/update.rb' - - 'ee/app/graphql/mutations/quality_management/test_cases/create.rb' - - 'ee/app/graphql/mutations/requirements_management/base_requirement.rb' - - 'ee/app/graphql/mutations/requirements_management/export_requirements.rb' - - 'ee/app/graphql/mutations/requirements_management/update_requirement.rb' - - 'ee/app/graphql/mutations/security/ci_configuration/configure_dependency_scanning.rb' - - 'ee/app/graphql/mutations/security_policy/assign_security_policy_project.rb' - - 'ee/app/graphql/mutations/security_policy/commit_scan_execution_policy.rb' - - 'ee/app/graphql/mutations/security_policy/create_security_policy_project.rb' - - 'ee/app/graphql/mutations/vulnerabilities/dismiss.rb' - 'ee/app/graphql/resolvers/alert_management/payload_alert_field_resolver.rb' - 'ee/app/graphql/resolvers/clusters/agents_resolver.rb' - 'ee/app/graphql/resolvers/concerns/common_requirement_arguments.rb' diff --git a/app/assets/javascripts/sidebar/components/severity/sidebar_severity.vue b/app/assets/javascripts/sidebar/components/severity/sidebar_severity.vue index fdf63c23552..5dc93476120 100644 --- a/app/assets/javascripts/sidebar/components/severity/sidebar_severity.vue +++ b/app/assets/javascripts/sidebar/components/severity/sidebar_severity.vue @@ -23,6 +23,7 @@ export default { GlLink, SeverityToken, }, + inject: ['canUpdate'], props: { projectPath: { type: String, @@ -153,6 +154,7 @@ export default { > {{ $options.i18n.SEVERITY }} <gl-link + v-if="canUpdate" data-testid="editButton" href="#" @click="toggleFormDropdown" diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index dd1b439c482..8837f3601e4 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -493,7 +493,7 @@ function mountSeverityComponent() { return false; } - const { fullPath, iid, severity } = getSidebarOptions(); + const { fullPath, iid, severity, editable } = getSidebarOptions(); return new Vue({ el: severityContainerEl, @@ -501,6 +501,9 @@ function mountSeverityComponent() { components: { SidebarSeverity, }, + provide: { + canUpdate: editable, + }, render: (createElement) => createElement('sidebar-severity', { props: { diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 7d34198653a..5d381ae0885 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -5,6 +5,11 @@ class ApplicationSetting < ApplicationRecord include CacheMarkdownField include TokenAuthenticatable include ChronicDurationAttribute + include IgnorableColumns + + ignore_columns %i[elasticsearch_shards elasticsearch_replicas], remove_with: '14.4', remove_after: '2021-09-22' + ignore_column :seat_link_enabled, remove_with: '14.4', remove_after: '2021-09-22' + ignore_column :cloud_license_enabled, remove_with: '14.4', remove_after: '2021-09-22' INSTANCE_REVIEW_MIN_USERS = 50 GRAFANA_URL_ERROR_MESSAGE = 'Please check your Grafana URL setting in ' \ diff --git a/app/models/project.rb b/app/models/project.rb index f0926a044a2..24ff0ed3b8c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -45,6 +45,9 @@ class Project < ApplicationRecord BoardLimitExceeded = Class.new(StandardError) + ignore_columns :mirror_last_update_at, :mirror_last_successful_update_at, remove_after: '2021-09-22', remove_with: '14.4' + ignore_columns :pull_mirror_branch_prefix, remove_after: '2021-09-22', remove_with: '14.4' + STATISTICS_ATTRIBUTE = 'repositories_count' UNKNOWN_IMPORT_URL = 'http://unknown.git' # Hashed Storage versions handle rolling out new storage to project and dependents models: diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index a700f104150..4937f2e862b 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -25,11 +25,8 @@ class RemoteMirror < ApplicationRecord before_save :set_new_remote_name, if: :mirror_url_changed? after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.mirror_available } - after_save :refresh_remote, if: :saved_change_to_mirror_url? after_update :reset_fields, if: :saved_change_to_mirror_url? - after_commit :remove_remote, on: :destroy - before_validation :store_credentials scope :enabled, -> { where(enabled: true) } @@ -100,11 +97,11 @@ class RemoteMirror < ApplicationRecord update_status == 'started' end - def update_repository(inmemory_remote:) + def update_repository Gitlab::Git::RemoteMirror.new( project.repository.raw, remote_name, - inmemory_remote ? remote_url : nil, + remote_url, **options_for_update ).update end @@ -227,15 +224,6 @@ class RemoteMirror < ApplicationRecord Gitlab::UrlSanitizer.new(read_attribute(:url)).full_url end - def ensure_remote! - return unless project - return unless remote_name && remote_url - - # If this fails or the remote already exists, we won't know due to - # https://gitlab.com/gitlab-org/gitaly/issues/1317 - project.repository.add_remote(remote_name, remote_url) - end - def after_sent_notification update_column(:error_notification_sent, true) end @@ -312,25 +300,6 @@ class RemoteMirror < ApplicationRecord self.remote_name = "remote_mirror_#{SecureRandom.hex}" end - def refresh_remote - return unless project - - # Before adding a new remote we have to delete the data from - # the previous remote name - prev_remote_name = remote_name_before_last_save || fallback_remote_name - run_after_commit do - project.repository.async_remove_remote(prev_remote_name) - end - - project.repository.add_remote(remote_name, remote_url) - end - - def remove_remote - return unless project # could be pending to delete so don't need to touch the git repository - - project.repository.async_remove_remote(remote_name) - end - def mirror_url_changed? url_changed? || attribute_changed?(:credentials) end diff --git a/app/models/repository.rb b/app/models/repository.rb index a77aaf02e06..1b7663c2973 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -939,32 +939,7 @@ class Repository end def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, prune: true) - return fetch_remote(remote_name, url: url, refmap: refmap, forced: forced, prune: prune) if Feature.enabled?(:fetch_remote_params, project, default_enabled: :yaml) - - unless remote_name - remote_name = "tmp-#{SecureRandom.hex}" - tmp_remote_name = true - end - - add_remote(remote_name, url, mirror_refmap: refmap) - fetch_remote(remote_name, forced: forced, prune: prune) - ensure - async_remove_remote(remote_name) if tmp_remote_name - end - - def async_remove_remote(remote_name) - return unless remote_name - return unless project - - job_id = RepositoryRemoveRemoteWorker.perform_async(project.id, remote_name) - - if job_id - Gitlab::AppLogger.info("Remove remote job scheduled for #{project.id} with remote name: #{remote_name} job ID #{job_id}.") - else - Gitlab::AppLogger.info("Remove remote job failed to create for #{project.id} with remote name #{remote_name}.") - end - - job_id + fetch_remote(remote_name, url: url, refmap: refmap, forced: forced, prune: prune) end def fetch_source_branch!(source_repository, source_branch, local_ref) diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index 6c29ba81910..898125c181c 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -43,12 +43,7 @@ module Projects # LFS objects must be sent first, or the push has dangling pointers send_lfs_objects!(remote_mirror) - response = if Feature.enabled?(:update_remote_mirror_inmemory, project, default_enabled: :yaml) - remote_mirror.update_repository(inmemory_remote: true) - else - remote_mirror.ensure_remote! - remote_mirror.update_repository(inmemory_remote: false) - end + response = remote_mirror.update_repository if response.divergent_refs.any? message = "Some refs have diverged and have not been updated on the remote:" diff --git a/app/views/admin/application_settings/_package_registry.html.haml b/app/views/admin/application_settings/_package_registry.html.haml index 8de65f267d2..7cdadaaf37b 100644 --- a/app/views/admin/application_settings/_package_registry.html.haml +++ b/app/views/admin/application_settings/_package_registry.html.haml @@ -6,7 +6,7 @@ %button.btn.gl-button.btn-default.js-settings-toggle{ type: 'button' } = expanded_by_default? ? _('Collapse') : _('Expand') %p - = _("Settings related to the use and experience of using GitLab's Package Registry.") + = _("Control how the GitLab Package Registry functions.") = render_if_exists 'admin/application_settings/ee_package_registry' diff --git a/app/workers/repository_remove_remote_worker.rb b/app/workers/repository_remove_remote_worker.rb index da611786bc4..c95393e7d21 100644 --- a/app/workers/repository_remove_remote_worker.rb +++ b/app/workers/repository_remove_remote_worker.rb @@ -16,22 +16,13 @@ class RepositoryRemoveRemoteWorker # rubocop:disable Scalability/IdempotentWorke attr_reader :project, :remote_name def perform(project_id, remote_name) - @remote_name = remote_name - @project = Project.find_by_id(project_id) - - return unless @project - - logger.info("Removing remote #{remote_name} from project #{project.id}") - - try_obtain_lease do - remove_remote = @project.repository.remove_remote(remote_name) - - if remove_remote - logger.info("Remote #{remote_name} was successfully removed from project #{project.id}") - else - logger.error("Could not remove remote #{remote_name} from project #{project.id}") - end - end + # On-disk remotes are slated for removal, and GitLab doesn't create any of + # them anymore. For backwards compatibility, we need to keep the worker + # though such that we can be sure to drain all jobs on an update. Making + # this a no-op is fine though: the worst that can happen is that we still + # have old remotes lingering in the repository's config, but Gitaly will + # start to clean these up in repository maintenance. + # https://gitlab.com/gitlab-org/gitlab/-/issues/336745 end def lease_timeout diff --git a/config/feature_flags/development/fetch_remote_params.yml b/config/feature_flags/development/fetch_remote_params.yml deleted file mode 100644 index db6b1a1775d..00000000000 --- a/config/feature_flags/development/fetch_remote_params.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: fetch_remote_params -introduced_by_url: -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/325528 -milestone: '13.12' -type: development -group: group::gitaly -default_enabled: true diff --git a/config/feature_flags/development/update_remote_mirror_inmemory.yml b/config/feature_flags/development/update_remote_mirror_inmemory.yml deleted file mode 100644 index 38c54c3cfc7..00000000000 --- a/config/feature_flags/development/update_remote_mirror_inmemory.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: update_remote_mirror_inmemory -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63962 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/333517 -milestone: '14.0' -type: development -group: group::gitaly -default_enabled: true diff --git a/config/initializers/0_inject_enterprise_edition_module.rb b/config/initializers/0_inject_enterprise_edition_module.rb index ea3fbda9186..a00075990eb 100644 --- a/config/initializers/0_inject_enterprise_edition_module.rb +++ b/config/initializers/0_inject_enterprise_edition_module.rb @@ -64,7 +64,31 @@ module InjectEnterpriseEditionModule # After we moved everything over to zeitwerk we can avoid rescuing # NameError and just check if const_defined? # mod && mod.const_defined?(name, false) && mod.const_get(name, false) - mod && mod.const_get(name, false) + result = mod && mod.const_get(name, false) + + if result.name == "#{mod}::#{name}" + result + else + # This may hit into a Rails issue that when we try to load + # `EE::API::Appearance`, Rails might load `::Appearance` the first time + # when `mod.const_get(name, false)` is called if `::Appearance` is not + # loaded yet. This can be demonstrated as the following: + # + # EE.const_get('API::Appearance', false) # => Appearance + # EE.const_get('API::Appearance', false) # => raise NameError + # + # Getting a `NameError` is what we're expecting here, because + # `EE::API::Appearance` doesn't exist. + # + # This is because Rails will attempt to load constants from all the + # parent namespaces, and if it finds one it'll load it and return it. + # However, the second time when it's called, since the top-level class + # is already loaded, then Rails will skip this process. This weird + # behaviour can be worked around by calling this the second time. + # The particular line is at: + # https://github.com/rails/rails/blob/v6.1.3.2/activesupport/lib/active_support/dependencies.rb#L569-L570 + mod.const_get(name, false) + end rescue NameError false end diff --git a/config/puma.rb.example b/config/puma.rb.example index f659529f23f..c70baf6570e 100644 --- a/config/puma.rb.example +++ b/config/puma.rb.example @@ -82,3 +82,12 @@ json_formatter = Gitlab::PumaLogging::JSONFormatter.new log_formatter do |str| json_formatter.call(str) end + +lowlevel_error_handler do |ex, env| + if Raven.configuration.capture_allowed? + Raven.capture_exception(ex, tags: { 'handler': 'puma_low_level' }, extra: { puma_env: env }) + end + + # note the below is just a Rack response + [500, {}, ["An error has occurred and reported in the system's low-level error handler."]] +end diff --git a/doc/ci/yaml/gitlab_ci_yaml.md b/doc/ci/yaml/gitlab_ci_yaml.md index 2723cb19c1f..6cd900123e0 100644 --- a/doc/ci/yaml/gitlab_ci_yaml.md +++ b/doc/ci/yaml/gitlab_ci_yaml.md @@ -4,11 +4,8 @@ group: Pipeline Execution info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers type: reference --- -<!-- markdownlint-disable MD044 --> -<!-- vale gitlab.Spelling = NO --> -# The .gitlab-ci.yml file **(FREE)** -<!-- vale gitlab.Spelling = YES --> -<!-- markdownlint-enable MD044 --> + +# The `.gitlab-ci.yml` file **(FREE)** To use GitLab CI/CD, you need: diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 86522e571a3..4d2007f548f 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -423,6 +423,7 @@ WARNING: do not merge the merge request** except for [very specific cases](https://about.gitlab.com/handbook/engineering/workflow/#criteria-for-merging-during-broken-master). For other cases, follow these [handbook instructions](https://about.gitlab.com/handbook/engineering/workflow/#merging-during-broken-master). + - If the latest pipeline was created before the merge request was approved, start a new pipeline to ensure that full RSpec suite has been run. You may skip this step only if the merge request does not contain any backend change. - If the **latest [Pipeline for Merged Results](../ci/pipelines/pipelines_for_merged_results.md)** finished less than 2 hours ago, you might merge without starting a new pipeline as the merge request is close enough to `main`. diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 0fe48fe8b9e..be544c02b2b 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -421,58 +421,18 @@ This experiment is only enabled when the CI/CD variable `RSPEC_FAIL_FAST_ENABLED The test files related to the Merge Request are determined using the [`test_file_finder`](https://gitlab.com/gitlab-org/ci-cd/test_file_finder) gem. We are using a custom mapping between source file to test files, maintained in the `tests.yml` file. -### RSpec minimal job experiment +### RSpec minimal jobs -As part of the objective to improve overall pipeline duration, we are experimenting with a minimal set of RSpec tests. -The purpose of this experiment is to verify if we are able to run a minimal set of RSpec tests in a Merge Request pipeline, -without resulting in increased number of broken main branch. +Before a merge request is approved, the pipeline will run a minimal set of RSpec tests that are related to the merge request changes. +This is to reduce the pipeline cost and shorten the job duration. To identify the minimal set of tests needed, we use [Crystalball gem](https://github.com/toptal/crystalball) to create a test mapping. The test mapping contains a map of each source files to a list of test files which is dependent of the source file. This mapping is currently generated using a combination of test coverage tracing and a static mapping. In the `detect-tests` job, we use this mapping to identify the minimal tests needed for the current Merge Request. -In this experiment, each `rspec` job is accompanied with a `minimal` version. -For example, `rspec unit` job has a corresponding `rspec unit minimal` job. -During the experiment, each Merge Request pipeline will contain both versions of the job, running in parallel. - -To illustrate this: - -```mermaid -graph LR - A --"artifact: list of test files"--> C1 & D1 & E1 & F1 - - subgraph "prepare stage"; - A["detect-tests"]; - end - - subgraph "test stage"; - C["rspec migration"]; - C1["rspec migration minimal"]; - D["rspec unit"]; - D1["rspec unit minimal"]; - E["rspec integration"]; - E1["rspec integration minimal"]; - F["rspec system"]; - F1["rspec system minimal"]; - end -``` - -The result of both set of jobs in the pipeline is then compared to identify any false positive. -A list of such pipeline can be found in [Sisense](https://app.periscopedata.com/app/gitlab/496118/Engineering-Productivity-Sandbox?widget=10492739&udv=833427). - -A false positive is defined as a pipeline where the `minimal` jobs passed, but the non-`minimal` jobs failed. -This indicates that the changeset resulted in a test failure, which was not detected by the `minimal` jobs. -Consequently, this signifies a gap in the test mapping used, which would need to be rectified. - -#### Findings - -After a round of the experiment done in December 2020, -we discovered that it was challenging to achieve a mapping that gives high confidence at the moment, -because of 2 reasons: - -- Each identified gap in the test mapping is unique, each needing its own investigation and improvement to the creation of the test mapping. -- There is a large number of flaky tests which added a lot of noise in the data, slowing down the investigation process. +After a merge request has been approved, the pipeline would contain the full RSpec tests. This will ensure that all tests +have been run before a merge request is merged. ### PostgreSQL versions testing diff --git a/doc/development/service_ping/metrics_dictionary.md b/doc/development/service_ping/metrics_dictionary.md index b64b2234fa9..9d668f37e79 100644 --- a/doc/development/service_ping/metrics_dictionary.md +++ b/doc/development/service_ping/metrics_dictionary.md @@ -37,7 +37,7 @@ Each metric is defined in a separate YAML file consisting of a number of fields: | `status` | yes | `string`; [status](#metric-statuses) of the metric, may be set to `data_available`, `implemented`, `not_used`, `deprecated`, `removed`, `broken`. | | `time_frame` | yes | `string`; may be set to a value like `7d`, `28d`, `all`, `none`. | | `data_source` | yes | `string`; may be set to a value like `database`, `redis`, `redis_hll`, `prometheus`, `system`. | -| `data_category` | yes | `string`; [categories](#data-category) of the metric, may be set to `Operational`, `Optional`, `Subscription`, `Standard`. | +| `data_category` | yes | `string`; [categories](#data-category) of the metric, may be set to `Operational`, `Optional`, `Subscription`, `Standard`. The default value is `Optional`.| | `instrumentation_class` | no | `string`; [the class that implements the metric](metrics_instrumentation.md). | | `distribution` | yes | `array`; may be set to one of `ce, ee` or `ee`. The [distribution](https://about.gitlab.com/handbook/marketing/strategic-marketing/tiers/#definitions) where the tracked feature is available. | | `tier` | yes | `array`; may be set to one of `free, premium, ultimate`, `premium, ultimate` or `ultimate`. The [tier]( https://about.gitlab.com/handbook/marketing/strategic-marketing/tiers/) where the tracked feature is available. | @@ -99,7 +99,7 @@ should be changed. We use the following categories to classify a metric: - `Operational`: Required data for operational purposes. -- `Optional`: Data that is optional to collect. This can be [enabled or disabled](../service_ping/index.md#disable-service-ping) in the Admin Area. +- `Optional`: Default value for a metric. Data that is optional to collect. This can be [enabled or disabled](../service_ping/index.md#disable-service-ping) in the Admin Area. - `Subscription`: Data related to licensing. - `Standard`: Standard set of identifiers that are included when collecting data. diff --git a/lib/api/appearance.rb b/lib/api/appearance.rb index fe498bf611b..1eaa4167a7d 100644 --- a/lib/api/appearance.rb +++ b/lib/api/appearance.rb @@ -48,3 +48,5 @@ module API end end end + +API::Appearance.prepend_mod diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 70d072e8082..b60a6160066 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -703,19 +703,6 @@ module Gitlab write_ref(ref, start_point) end - # If `mirror_refmap` is present the remote is set as mirror with that mapping - def add_remote(remote_name, url, mirror_refmap: nil) - wrapped_gitaly_errors do - gitaly_remote_client.add_remote(remote_name, url, mirror_refmap) - end - end - - def remove_remote(remote_name) - wrapped_gitaly_errors do - gitaly_remote_client.remove_remote(remote_name) - end - end - def find_remote_root_ref(remote_name, remote_url, authorization = nil) return unless remote_name.present? && remote_url.present? diff --git a/lib/gitlab/gitaly_client/remote_service.rb b/lib/gitlab/gitaly_client/remote_service.rb index 487127b7b74..652b28c9290 100644 --- a/lib/gitlab/gitaly_client/remote_service.rb +++ b/lib/gitlab/gitaly_client/remote_service.rb @@ -26,23 +26,6 @@ module Gitlab @storage = repository.storage end - def add_remote(name, url, mirror_refmaps) - request = Gitaly::AddRemoteRequest.new( - repository: @gitaly_repo, - name: name, - url: url, - mirror_refmaps: Array.wrap(mirror_refmaps).map(&:to_s) - ) - - GitalyClient.call(@storage, :remote_service, :add_remote, request, timeout: GitalyClient.fast_timeout) - end - - def remove_remote(name) - request = Gitaly::RemoveRemoteRequest.new(repository: @gitaly_repo, name: name) - - GitalyClient.call(@storage, :remote_service, :remove_remote, request, timeout: GitalyClient.long_timeout).result - end - # The remote_name parameter is deprecated and will be removed soon. def find_remote_root_ref(remote_name, remote_url, authorization) request = Gitaly::FindRemoteRootRefRequest.new(repository: @gitaly_repo, diff --git a/lib/gitlab/github_import/importer/pull_requests_importer.rb b/lib/gitlab/github_import/importer/pull_requests_importer.rb index b2f099761b1..8b894140923 100644 --- a/lib/gitlab/github_import/importer/pull_requests_importer.rb +++ b/lib/gitlab/github_import/importer/pull_requests_importer.rb @@ -40,11 +40,7 @@ module Gitlab # updating the timestamp. project.update_column(:last_repository_updated_at, Time.zone.now) - if Feature.enabled?(:fetch_remote_params, project, default_enabled: :yaml) - project.repository.fetch_remote('github', url: project.import_url, refmap: Gitlab::GithubImport.refmap, forced: false) - else - project.repository.fetch_remote('github', forced: false) - end + project.repository.fetch_remote('github', url: project.import_url, refmap: Gitlab::GithubImport.refmap, forced: false) pname = project.path_with_namespace diff --git a/locale/gitlab.pot b/locale/gitlab.pot index de72d782daf..68c8a5efbdc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8892,6 +8892,9 @@ msgstr "" msgid "Control emails linked to your account" msgstr "" +msgid "Control how the GitLab Package Registry functions." +msgstr "" + msgid "Control whether to display third-party offers in GitLab." msgstr "" @@ -30005,9 +30008,6 @@ msgstr "" msgid "Settings" msgstr "" -msgid "Settings related to the use and experience of using GitLab's Package Registry." -msgstr "" - msgid "Setup" msgstr "" @@ -36913,9 +36913,6 @@ msgstr "" msgid "When disabled, an external authentication provider must be used." msgstr "" -msgid "When enabled, if an npm package isn't found in the GitLab Registry, we will attempt to pull from the global npm registry." -msgstr "" - msgid "When enabled, users cannot use GitLab until the terms have been accepted." msgstr "" diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 12ae644cab8..c9aa2a80187 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -13,7 +13,8 @@ module QA :initialize_with_readme, :auto_devops_enabled, :github_personal_access_token, - :github_repository_path + :github_repository_path, + :gitlab_repository_path attributes :id, :name, diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb index 80e94fa1628..2b308c9080e 100644 --- a/spec/factories_spec.rb +++ b/spec/factories_spec.rb @@ -30,7 +30,6 @@ RSpec.describe 'factories' do [:pages_domain, :with_trusted_expired_chain], [:pages_domain, :explicit_ecdsa], [:project_member, :blocked], - [:project, :remote_mirror], [:remote_mirror, :ssh], [:user_preference, :only_comments], [:ci_pipeline_artifact, :remote_store] diff --git a/spec/frontend/sidebar/components/severity/sidebar_severity_spec.js b/spec/frontend/sidebar/components/severity/sidebar_severity_spec.js index 747d370e1cf..6116bc68927 100644 --- a/spec/frontend/sidebar/components/severity/sidebar_severity_spec.js +++ b/spec/frontend/sidebar/components/severity/sidebar_severity_spec.js @@ -1,5 +1,6 @@ import { GlDropdown, GlDropdownItem, GlLoadingIcon, GlTooltip, GlSprintf } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import createFlash from '~/flash'; import { INCIDENT_SEVERITY, ISSUABLE_TYPES } from '~/sidebar/components/severity/constants'; @@ -15,6 +16,7 @@ describe('SidebarSeverity', () => { const projectPath = 'gitlab-org/gitlab-test'; const iid = '1'; const severity = 'CRITICAL'; + let canUpdate = true; function createComponent(props = {}) { const propsData = { @@ -25,8 +27,11 @@ describe('SidebarSeverity', () => { ...props, }; mutate = jest.fn(); - wrapper = shallowMount(SidebarSeverity, { + wrapper = shallowMountExtended(SidebarSeverity, { propsData, + provide: { + canUpdate, + }, mocks: { $apollo: { mutate, @@ -45,22 +50,34 @@ describe('SidebarSeverity', () => { afterEach(() => { if (wrapper) { wrapper.destroy(); - wrapper = null; } }); - const findSeverityToken = () => wrapper.findAll(SeverityToken); - const findEditBtn = () => wrapper.find('[data-testid="editButton"]'); - const findDropdown = () => wrapper.find(GlDropdown); - const findCriticalSeverityDropdownItem = () => wrapper.find(GlDropdownItem); - const findLoadingIcon = () => wrapper.find(GlLoadingIcon); - const findTooltip = () => wrapper.find(GlTooltip); + const findSeverityToken = () => wrapper.findAllComponents(SeverityToken); + const findEditBtn = () => wrapper.findByTestId('editButton'); + const findDropdown = () => wrapper.findComponent(GlDropdown); + const findCriticalSeverityDropdownItem = () => wrapper.findComponent(GlDropdownItem); + const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const findTooltip = () => wrapper.findComponent(GlTooltip); const findCollapsedSeverity = () => wrapper.find({ ref: 'severity' }); - it('renders severity widget', () => { - expect(findEditBtn().exists()).toBe(true); - expect(findSeverityToken().exists()).toBe(true); - expect(findDropdown().exists()).toBe(true); + describe('Severity widget', () => { + it('renders severity dropdown and token', () => { + expect(findSeverityToken().exists()).toBe(true); + expect(findDropdown().exists()).toBe(true); + }); + + describe('edit button', () => { + it('is rendered when `canUpdate` provided as `true`', () => { + expect(findEditBtn().exists()).toBe(true); + }); + + it('is NOT rendered when `canUpdate` provided as `false`', () => { + canUpdate = false; + createComponent(); + expect(findEditBtn().exists()).toBe(false); + }); + }); }); describe('Update severity', () => { @@ -100,7 +117,7 @@ describe('SidebarSeverity', () => { ); findCriticalSeverityDropdownItem().vm.$emit('click'); - await wrapper.vm.$nextTick(); + await nextTick(); expect(findLoadingIcon().exists()).toBe(true); resolvePromise(); @@ -128,27 +145,29 @@ describe('SidebarSeverity', () => { it('should expand the dropdown on collapsed icon click', async () => { wrapper.vm.isDropdownShowing = false; - await wrapper.vm.$nextTick(); + await nextTick(); expect(findDropdown().classes(HIDDDEN_CLASS)).toBe(true); findCollapsedSeverity().trigger('click'); - await wrapper.vm.$nextTick(); + await nextTick(); expect(findDropdown().classes(SHOWN_CLASS)).toBe(true); }); }); describe('expanded', () => { it('toggles dropdown with edit button', async () => { + canUpdate = true; + createComponent(); wrapper.vm.isDropdownShowing = false; - await wrapper.vm.$nextTick(); + await nextTick(); expect(findDropdown().classes(HIDDDEN_CLASS)).toBe(true); findEditBtn().vm.$emit('click'); - await wrapper.vm.$nextTick(); + await nextTick(); expect(findDropdown().classes(SHOWN_CLASS)).toBe(true); findEditBtn().vm.$emit('click'); - await wrapper.vm.$nextTick(); + await nextTick(); expect(findDropdown().classes(HIDDDEN_CLASS)).toBe(true); }); }); diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 706bcdea291..9b3d8c8ef3a 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -2001,47 +2001,6 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do end end - describe 'remotes' do - let(:repository) { mutable_repository } - let(:remote_name) { 'my-remote' } - let(:url) { 'http://my-repo.git' } - - after do - ensure_seeds - end - - describe '#add_remote' do - let(:mirror_refmap) { '+refs/*:refs/*' } - - it 'added the remote' do - begin - repository_rugged.remotes.delete(remote_name) - rescue Rugged::ConfigError - end - - repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) - - expect(repository_rugged.remotes[remote_name]).not_to be_nil - expect(repository_rugged.config["remote.#{remote_name}.mirror"]).to eq('true') - expect(repository_rugged.config["remote.#{remote_name}.prune"]).to eq('true') - expect(repository_rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) - end - end - - describe '#remove_remote' do - it 'removes the remote' do - repository_rugged.remotes.create(remote_name, url) - - expect(repository.remove_remote(remote_name)).to be true - - # Since we deleted the remote via Gitaly, Rugged doesn't know - # this changed underneath it. Let's refresh the Rugged repo. - repository_rugged = Rugged::Repository.new(repository_path) - expect(repository_rugged.remotes[remote_name]).to be_nil - end - end - end - describe '#bundle_to_disk' do let(:save_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } diff --git a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb index 2ec5f70be76..cdbd48e106a 100644 --- a/spec/lib/gitlab/gitaly_client/remote_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/remote_service_spec.rb @@ -9,31 +9,6 @@ RSpec.describe Gitlab::GitalyClient::RemoteService do let(:remote_name) { 'my-remote' } let(:client) { described_class.new(project.repository) } - describe '#add_remote' do - let(:url) { 'http://my-repo.git' } - let(:mirror_refmap) { :all_refs } - - it 'sends an add_remote message' do - expect_any_instance_of(Gitaly::RemoteService::Stub) - .to receive(:add_remote) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) - .and_return(double(:add_remote_response)) - - client.add_remote(remote_name, url, mirror_refmap) - end - end - - describe '#remove_remote' do - it 'sends an remove_remote message and returns the result value' do - expect_any_instance_of(Gitaly::RemoteService::Stub) - .to receive(:remove_remote) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) - .and_return(double(result: true)) - - expect(client.remove_remote(remote_name)).to be(true) - end - end - describe '#find_remote_root_ref' do let(:remote) { 'origin' } let(:url) { 'http://git.example.com/my-repo.git' } diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb index 133d515246a..3fa2ca1f57c 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb @@ -148,7 +148,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do end end - shared_examples '#update_repository' do + describe '#update_repository' do it 'updates the repository' do importer = described_class.new(project, client) @@ -162,34 +162,16 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do .to receive(:increment) .and_call_original - freeze_time do - importer.update_repository - - expect(project.last_repository_updated_at).to be_like_time(Time.zone.now) - end - end - end - - describe '#update_repository with :fetch_remote_params enabled' do - before do - stub_feature_flags(fetch_remote_params: true) expect(project.repository) .to receive(:fetch_remote) .with('github', forced: false, url: url, refmap: Gitlab::GithubImport.refmap) - end - it_behaves_like '#update_repository' - end + freeze_time do + importer.update_repository - describe '#update_repository with :fetch_remote_params disabled' do - before do - stub_feature_flags(fetch_remote_params: false) - expect(project.repository) - .to receive(:fetch_remote) - .with('github', forced: false) + expect(project.last_repository_updated_at).to be_like_time(Time.zone.now) + end end - - it_behaves_like '#update_repository' end describe '#update_repository?' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 70a396b3a14..ec99e094a03 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2914,10 +2914,6 @@ RSpec.describe Project, factory_default: :keep do subject { project.has_remote_mirror? } - before do - allow_any_instance_of(RemoteMirror).to receive(:refresh_remote) - end - it 'returns true when a remote mirror is enabled' do is_expected.to be_truthy end @@ -2934,10 +2930,6 @@ RSpec.describe Project, factory_default: :keep do delegate :update_remote_mirrors, to: :project - before do - allow_any_instance_of(RemoteMirror).to receive(:refresh_remote) - end - it 'syncs enabled remote mirror' do expect_any_instance_of(RemoteMirror).to receive(:sync) diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index a64b01967ef..058cc2c5925 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -93,22 +93,14 @@ RSpec.describe RemoteMirror, :mailer do expect(mirror.credentials).to eq({ user: 'foo', password: 'bar' }) end - it 'updates the remote config if credentials changed' do + it 'does not update the repository config if credentials changed' do mirror = create_mirror(url: 'http://foo:bar@test.com') repo = mirror.project.repository + old_config = rugged_repo(repo).config mirror.update_attribute(:url, 'http://foo:baz@test.com') - config = rugged_repo(repo).config - expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com') - end - - it 'removes previous remote' do - mirror = create_mirror(url: 'http://foo:bar@test.com') - - expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original - - mirror.update(url: 'http://test.com') + expect(rugged_repo(repo).config.to_hash).to eq(old_config.to_hash) end end end @@ -157,34 +149,20 @@ RSpec.describe RemoteMirror, :mailer do end describe '#update_repository' do - shared_examples 'an update' do - it 'performs update including options' do - git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy) - mirror = build(:remote_mirror) - - expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true) - mirror.update_repository(inmemory_remote: inmemory) - - expect(git_remote_mirror).to have_received(:new).with( - mirror.project.repository.raw, - mirror.remote_name, - inmemory ? mirror.url : nil, - keep_divergent_refs: true - ) - expect(git_remote_mirror).to have_received(:update) - end - end + it 'performs update including options' do + git_remote_mirror = stub_const('Gitlab::Git::RemoteMirror', spy) + mirror = build(:remote_mirror) - context 'with inmemory remote' do - let(:inmemory) { true } + expect(mirror).to receive(:options_for_update).and_return(keep_divergent_refs: true) + mirror.update_repository - it_behaves_like 'an update' - end - - context 'with on-disk remote' do - let(:inmemory) { false } - - it_behaves_like 'an update' + expect(git_remote_mirror).to have_received(:new).with( + mirror.project.repository.raw, + mirror.remote_name, + mirror.url, + keep_divergent_refs: true + ) + expect(git_remote_mirror).to have_received(:update) end end @@ -303,10 +281,10 @@ RSpec.describe RemoteMirror, :mailer do end context 'when remote mirror gets destroyed' do - it 'removes remote' do + it 'does not remove the remote' do mirror = create_mirror(url: 'http://foo:bar@test.com') - expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original + expect(RepositoryRemoveRemoteWorker).not_to receive(:perform_async) mirror.destroy! end @@ -402,30 +380,6 @@ RSpec.describe RemoteMirror, :mailer do end end - describe '#ensure_remote!' do - let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } - let(:project) { remote_mirror.project } - let(:repository) { project.repository } - - it 'adds a remote multiple times with no errors' do - expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, remote_mirror.url).twice.and_call_original - - 2.times do - remote_mirror.ensure_remote! - end - end - - context 'SSH public-key authentication' do - it 'omits the password from the URL' do - remote_mirror.update!(auth_method: 'ssh_public_key', url: 'ssh://git:pass@example.com') - - expect(repository).to receive(:add_remote).with(remote_mirror.remote_name, 'ssh://git@example.com') - - remote_mirror.ensure_remote! - end - end - end - describe '#url=' do let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 452eafe733f..7e68886a0cb 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1094,99 +1094,17 @@ RSpec.describe Repository do end end - describe '#async_remove_remote' do - before do - masterrev = repository.find_branch('master').dereferenced_target - create_remote_branch('joe', 'remote_branch', masterrev) - end - - context 'when worker is scheduled successfully' do - before do - masterrev = repository.find_branch('master').dereferenced_target - create_remote_branch('remote_name', 'remote_branch', masterrev) - - allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return('1234') - end - - it 'returns job_id' do - expect(repository.async_remove_remote('joe')).to eq('1234') - end - end - - context 'when worker does not schedule successfully' do - before do - allow(RepositoryRemoveRemoteWorker).to receive(:perform_async).and_return(nil) - end - - it 'returns nil' do - expect(Gitlab::AppLogger).to receive(:info).with("Remove remote job failed to create for #{project.id} with remote name joe.") - - expect(repository.async_remove_remote('joe')).to be_nil - end - end - end - describe '#fetch_as_mirror' do let(:url) { "http://example.com" } + let(:remote_name) { "remote-name" } - context 'when :fetch_remote_params is enabled' do - let(:remote_name) { "remote-name" } - - before do - stub_feature_flags(fetch_remote_params: true) - end - - it 'fetches the URL without creating a remote' do - expect(repository).not_to receive(:add_remote) - expect(repository) - .to receive(:fetch_remote) - .with(remote_name, url: url, forced: false, prune: true, refmap: :all_refs) - .and_return(nil) - - repository.fetch_as_mirror(url, remote_name: remote_name) - end - end - - context 'when :fetch_remote_params is disabled' do - before do - stub_feature_flags(fetch_remote_params: false) - end - - shared_examples 'a fetch' do - it 'adds and fetches a remote' do - expect(repository) - .to receive(:add_remote) - .with(expected_remote, url, mirror_refmap: :all_refs) - .and_return(nil) - expect(repository) - .to receive(:fetch_remote) - .with(expected_remote, forced: false, prune: true) - .and_return(nil) - - repository.fetch_as_mirror(url, remote_name: remote_name) - end - end - - context 'with temporary remote' do - let(:remote_name) { nil } - let(:expected_remote_suffix) { "123456" } - let(:expected_remote) { "tmp-#{expected_remote_suffix}" } - - before do - expect(repository) - .to receive(:async_remove_remote).with(expected_remote).and_return(nil) - allow(SecureRandom).to receive(:hex).and_return(expected_remote_suffix) - end - - it_behaves_like 'a fetch' - end - - context 'with remote name' do - let(:remote_name) { "foo" } - let(:expected_remote) { "foo" } + it 'fetches the URL without creating a remote' do + expect(repository) + .to receive(:fetch_remote) + .with(remote_name, url: url, forced: false, prune: true, refmap: :all_refs) + .and_return(nil) - it_behaves_like 'a fetch' - end + repository.fetch_as_mirror(url, remote_name: remote_name) end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index ac82e4c025f..4e2a056a641 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -3368,10 +3368,6 @@ RSpec.describe NotificationService, :mailer do project.add_maintainer(u_maintainer2) project.add_developer(u_developer) - # Mock remote update - allow(project.repository).to receive(:async_remove_remote) - allow(project.repository).to receive(:add_remote) - reset_delivered_emails! end diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index feb70ddaa46..686b204c726 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -13,38 +13,17 @@ RSpec.describe Projects::UpdateRemoteMirrorService do describe '#execute' do let(:retries) { 0 } - let(:inmemory) { true } subject(:execute!) { service.execute(remote_mirror, retries) } before do - stub_feature_flags(update_remote_mirror_inmemory: inmemory) project.repository.add_branch(project.owner, 'existing-branch', 'master') allow(remote_mirror) .to receive(:update_repository) - .with(inmemory_remote: inmemory) .and_return(double(divergent_refs: [])) end - context 'with in-memory remote disabled' do - let(:inmemory) { false } - - it 'ensures the remote exists' do - expect(remote_mirror).to receive(:ensure_remote!) - - execute! - end - end - - context 'with in-memory remote enabled' do - it 'does not ensure the remote exists' do - expect(remote_mirror).not_to receive(:ensure_remote!) - - execute! - end - end - it 'does not fetch the remote repository' do # See https://gitlab.com/gitlab-org/gitaly/-/issues/2670 expect(project.repository).not_to receive(:fetch_remote) diff --git a/spec/workers/repository_remove_remote_worker_spec.rb b/spec/workers/repository_remove_remote_worker_spec.rb index 758f7f75e03..11081ec9b37 100644 --- a/spec/workers/repository_remove_remote_worker_spec.rb +++ b/spec/workers/repository_remove_remote_worker_spec.rb @@ -24,37 +24,25 @@ RSpec.describe RepositoryRemoveRemoteWorker do .and_return(project) end - it 'does not remove remote when cannot obtain lease' do + it 'does nothing when cannot obtain lease' do stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) expect(project.repository) .not_to receive(:remove_remote) - expect(subject) - .to receive(:log_error) - .with("Cannot obtain an exclusive lease for #{lease_key}. There must be another instance already in execution.") + .not_to receive(:log_error) subject.perform(project.id, remote_name) end - it 'removes remote from repository when obtain a lease' do + it 'does nothing when obtain a lease' do stub_exclusive_lease(lease_key, timeout: lease_timeout) - masterrev = project.repository.find_branch('master').dereferenced_target - create_remote_branch(remote_name, 'remote_branch', masterrev) expect(project.repository) - .to receive(:remove_remote) - .with(remote_name) - .and_call_original + .not_to receive(:remove_remote) subject.perform(project.id, remote_name) end end end - - def create_remote_branch(remote_name, branch_name, target) - rugged = rugged_repo(project.repository) - - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) - end end |