diff options
23 files changed, 284 insertions, 74 deletions
diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/details/npm_installation.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/details/npm_installation.vue index 47081e23318..2448324549e 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/components/details/npm_installation.vue +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/details/npm_installation.vue @@ -1,5 +1,5 @@ <script> -import { GlLink, GlSprintf } from '@gitlab/ui'; +import { GlLink, GlSprintf, GlFormRadioGroup } from '@gitlab/ui'; import { s__ } from '~/locale'; import InstallationTitle from '~/packages_and_registries/package_registry/components/details/installation_title.vue'; @@ -11,6 +11,8 @@ import { TRACKING_LABEL_CODE_INSTRUCTION, NPM_PACKAGE_MANAGER, YARN_PACKAGE_MANAGER, + PROJECT_PACKAGE_ENDPOINT_TYPE, + INSTANCE_PACKAGE_ENDPOINT_TYPE, } from '~/packages_and_registries/package_registry/constants'; import CodeInstruction from '~/vue_shared/components/registry/code_instruction.vue'; @@ -21,8 +23,9 @@ export default { CodeInstruction, GlLink, GlSprintf, + GlFormRadioGroup, }, - inject: ['npmHelpPath', 'npmPath'], + inject: ['npmHelpPath', 'npmPath', 'npmProjectPath'], props: { packageEntity: { type: Object, @@ -32,6 +35,7 @@ export default { data() { return { instructionType: NPM_PACKAGE_MANAGER, + packageEndpointType: INSTANCE_PACKAGE_ENDPOINT_TYPE, }; }, computed: { @@ -39,13 +43,13 @@ export default { return this.npmInstallationCommand(NPM_PACKAGE_MANAGER); }, npmSetup() { - return this.npmSetupCommand(NPM_PACKAGE_MANAGER); + return this.npmSetupCommand(NPM_PACKAGE_MANAGER, this.packageEndpointType); }, yarnCommand() { return this.npmInstallationCommand(YARN_PACKAGE_MANAGER); }, yarnSetupCommand() { - return this.npmSetupCommand(YARN_PACKAGE_MANAGER); + return this.npmSetupCommand(YARN_PACKAGE_MANAGER, this.packageEndpointType); }, showNpm() { return this.instructionType === NPM_PACKAGE_MANAGER; @@ -58,14 +62,16 @@ export default { return `${instruction} ${this.packageEntity.name}`; }, - npmSetupCommand(type) { + npmSetupCommand(type, endpointType) { const scope = this.packageEntity.name.substring(0, this.packageEntity.name.indexOf('/')); + const npmPathForEndpoint = + endpointType === INSTANCE_PACKAGE_ENDPOINT_TYPE ? this.npmPath : this.npmProjectPath; if (type === NPM_PACKAGE_MANAGER) { - return `echo ${scope}:registry=${this.npmPath}/ >> .npmrc`; + return `echo ${scope}:registry=${npmPathForEndpoint}/ >> .npmrc`; } - return `echo \\"${scope}:registry\\" \\"${this.npmPath}/\\" >> .yarnrc`; + return `echo \\"${scope}:registry\\" \\"${npmPathForEndpoint}/\\" >> .yarnrc`; }, }, packageManagers: { @@ -87,6 +93,10 @@ export default { { value: NPM_PACKAGE_MANAGER, label: s__('PackageRegistry|Show NPM commands') }, { value: YARN_PACKAGE_MANAGER, label: s__('PackageRegistry|Show Yarn commands') }, ], + packageEndpointTypeOptions: [ + { value: INSTANCE_PACKAGE_ENDPOINT_TYPE, text: s__('PackageRegistry|Instance-level') }, + { value: PROJECT_PACKAGE_ENDPOINT_TYPE, text: s__('PackageRegistry|Project-level') }, + ], }; </script> @@ -116,6 +126,12 @@ export default { <h3 class="gl-font-lg">{{ __('Registry setup') }}</h3> + <gl-form-radio-group + :options="$options.packageEndpointTypeOptions" + :checked="packageEndpointType" + @change="packageEndpointType = $event" + /> + <code-instruction v-if="showNpm" :instruction="npmSetup" diff --git a/app/assets/javascripts/packages_and_registries/package_registry/constants.js b/app/assets/javascripts/packages_and_registries/package_registry/constants.js index 5f0ff89000d..38f335d16e2 100644 --- a/app/assets/javascripts/packages_and_registries/package_registry/constants.js +++ b/app/assets/javascripts/packages_and_registries/package_registry/constants.js @@ -86,3 +86,6 @@ export const PACKAGE_PROCESSING_STATUS = 'PROCESSING'; export const NPM_PACKAGE_MANAGER = 'npm'; export const YARN_PACKAGE_MANAGER = 'yarn'; + +export const PROJECT_PACKAGE_ENDPOINT_TYPE = 'project'; +export const INSTANCE_PACKAGE_ENDPOINT_TYPE = 'instance'; diff --git a/app/assets/javascripts/pages/shared/mount_runner_instructions.js b/app/assets/javascripts/pages/shared/mount_runner_instructions.js index e83c73edfde..1cb7259be64 100644 --- a/app/assets/javascripts/pages/shared/mount_runner_instructions.js +++ b/app/assets/javascripts/pages/shared/mount_runner_instructions.js @@ -9,7 +9,12 @@ export function initInstallRunner(componentId = 'js-install-runner') { const installRunnerEl = document.getElementById(componentId); if (installRunnerEl) { - const defaultClient = createDefaultClient(); + const defaultClient = createDefaultClient( + {}, + { + assumeImmutableResults: true, + }, + ); const apolloProvider = new VueApollo({ defaultClient, diff --git a/app/assets/javascripts/releases/mount_show.js b/app/assets/javascripts/releases/mount_show.js index 7272880197a..686f9e294b7 100644 --- a/app/assets/javascripts/releases/mount_show.js +++ b/app/assets/javascripts/releases/mount_show.js @@ -6,7 +6,12 @@ import ReleaseShowApp from './components/app_show.vue'; Vue.use(VueApollo); const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient(), + defaultClient: createDefaultClient( + {}, + { + assumeImmutableResults: true, + }, + ), }); export default () => { diff --git a/app/helpers/packages_helper.rb b/app/helpers/packages_helper.rb index ebf30fb3538..70d9927cf7f 100644 --- a/app/helpers/packages_helper.rb +++ b/app/helpers/packages_helper.rb @@ -70,6 +70,7 @@ module PackagesHelper can_delete: can?(current_user, :destroy_package, project).to_s, svg_path: image_path('illustrations/no-packages.svg'), npm_path: package_registry_instance_url(:npm), + npm_project_path: package_registry_project_url(project.id, :npm), npm_help_path: help_page_path('user/packages/npm_registry/index'), maven_path: package_registry_project_url(project.id, :maven), maven_help_path: help_page_path('user/packages/maven_repository/index'), diff --git a/app/models/user.rb b/app/models/user.rb index ae75e0f8011..03347ee3a40 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2098,7 +2098,7 @@ class User < ApplicationRecord def check_username_format return if username.blank? || Mime::EXTENSION_LOOKUP.keys.none? { |type| username.end_with?(".#{type}") } - errors.add(:username, _('ending with a file extension is not allowed.')) + errors.add(:username, _('ending with a reserved file extension is not allowed.')) end def groups_with_developer_maintainer_project_access diff --git a/doc/.vale/gitlab/Acronyms.yml b/doc/.vale/gitlab/Acronyms.yml index 1a1de67b72e..0edba1852e7 100644 --- a/doc/.vale/gitlab/Acronyms.yml +++ b/doc/.vale/gitlab/Acronyms.yml @@ -17,6 +17,7 @@ exceptions: - AJAX - ANSI - API + - APM - ARM - ARN - ASCII @@ -24,6 +25,7 @@ exceptions: - BSD - CAS - CDN + - CIDR - CLI - CNA - CNAME diff --git a/doc/administration/geo/replication/object_storage.md b/doc/administration/geo/replication/object_storage.md index 1f799b30125..3a10d3bad58 100644 --- a/doc/administration/geo/replication/object_storage.md +++ b/doc/administration/geo/replication/object_storage.md @@ -73,7 +73,7 @@ GitLab does not currently support the case where both: ## Third-party replication services When using Amazon S3, you can use -[CRR](https://docs.aws.amazon.com/AmazonS3/latest/dev/crr.html) to +[Cross-Region Replication (CRR)](https://docs.aws.amazon.com/AmazonS3/latest/dev/crr.html) to have automatic replication between the bucket used by the **primary** site and the bucket used by **secondary** sites. diff --git a/doc/user/clusters/management_project_template.md b/doc/user/clusters/management_project_template.md index 9e2b00a0f54..6fd75f5c2ce 100644 --- a/doc/user/clusters/management_project_template.md +++ b/doc/user/clusters/management_project_template.md @@ -91,10 +91,6 @@ the pipeline runs, Helmfile tries to either install or update your apps accordin cluster and Helm releases. If you change this attribute to `installed: false`, Helmfile tries try to uninstall this app from your cluster. [Read more](https://github.com/roboll/helmfile) about how Helmfile works. -Furthermore, each app has an `applications/{app}/values.yaml` file (`applicaton/{app}/values.yaml.gotmpl` in case of GitLab Runner). This is the -place where you can define default values for your app's Helm chart. Some apps already have defaults -pre-defined by GitLab. - ### Built-in applications The [built-in supported applications](https://gitlab.com/gitlab-org/project-templates/cluster-management/-/tree/master/applications) are: @@ -110,3 +106,9 @@ The [built-in supported applications](https://gitlab.com/gitlab-org/project-temp - [Prometheus](../infrastructure/clusters/manage/management_project_applications/prometheus.md) - [Sentry](../infrastructure/clusters/manage/management_project_applications/sentry.md) - [Vault](../infrastructure/clusters/manage/management_project_applications/vault.md) + +#### How to customize your applications + +Each app has an `applications/{app}/values.yaml` file (`applicaton/{app}/values.yaml.gotmpl` in case of GitLab Runner). This is the +place where you can define default values for your app's Helm chart. Some apps already have defaults +pre-defined by GitLab. diff --git a/doc/user/packages/npm_registry/index.md b/doc/user/packages/npm_registry/index.md index fe7e6a0ea46..08118aa1750 100644 --- a/doc/user/packages/npm_registry/index.md +++ b/doc/user/packages/npm_registry/index.md @@ -370,13 +370,26 @@ in a JavaScript project. You can install a package from the scope of a project o If multiple packages have the same name and version, when you install a package, the most recently-published package is retrieved. -1. Set the URL for scoped packages by running: +1. Set the URL for scoped packages. + + For [instance-level endpoints](#use-the-gitlab-endpoint-for-npm-packages) run: ```shell npm config set @foo:registry https://gitlab.example.com/api/v4/packages/npm/ ``` - Replace `@foo` with your scope. + - Replace `@foo` with your scope. + - Replace `gitlab.example.com` with your domain name. + + For [project-level endpoints](#use-the-gitlab-endpoint-for-npm-packages) run: + + ```shell + npm config set @foo:registry https://gitlab.example.com/api/v4/projects/<your_project_id>/packages/npm/ + ``` + + - Replace `@foo` with your scope. + - Replace `gitlab.example.com` with your domain name. + - Replace `<your_project_id>` with your project ID, found on the project's home page. 1. Ensure [authentication](#authenticate-to-the-package-registry) is configured. diff --git a/doc/user/project/clusters/serverless/index.md b/doc/user/project/clusters/serverless/index.md index fb32579f40e..f6598f8846b 100644 --- a/doc/user/project/clusters/serverless/index.md +++ b/doc/user/project/clusters/serverless/index.md @@ -282,7 +282,7 @@ Explanation of the fields used above: |-----------|-------------| | `name` | Indicates which provider is used to execute the `serverless.yml` file. In this case, the TriggerMesh middleware. | | `envs` | Includes the environment variables to be passed as part of function execution for **all** functions in the file, where `FOO` is the variable name and `BAR` are the variable contents. You may replace this with your own variables. | -| `secrets` | Includes the contents of the Kubernetes secret as environment variables accessible to be passed as part of function execution for **all** functions in the file. The secrets are expected in INI format. | +| `secrets` | Includes the contents of the Kubernetes secret as environment variables accessible to be passed as part of function execution for **all** functions in the file. The secrets are expected in `INI` format. | ### `functions` @@ -296,7 +296,7 @@ subsequent lines contain the function attributes. | `runtime` (optional)| The runtime to be used to execute the function. This can be a runtime alias (see [Runtime aliases](#runtime-aliases)), or it can be a full URL to a custom runtime repository. When the runtime is not specified, we assume that `Dockerfile` is present in the function directory specified by `source`. | | `description` | A short description of the function. | | `envs` | Sets an environment variable for the specific function only. | -| `secrets` | Includes the contents of the Kubernetes secret as environment variables accessible to be passed as part of function execution for the specific function only. The secrets are expected in INI format. | +| `secrets` | Includes the contents of the Kubernetes secret as environment variables accessible to be passed as part of function execution for the specific function only. The secrets are expected in `INI` format. | ### Deployment diff --git a/lib/gitlab/database/load_balancing/load_balancer.rb b/lib/gitlab/database/load_balancing/load_balancer.rb index 9b00b323301..a51626b895a 100644 --- a/lib/gitlab/database/load_balancing/load_balancer.rb +++ b/lib/gitlab/database/load_balancing/load_balancer.rb @@ -235,7 +235,7 @@ module Gitlab @configuration.model.connection_specification_name, role: ActiveRecord::Base.writing_role, shard: ActiveRecord::Base.default_shard - ) + ) || raise(::ActiveRecord::ConnectionNotEstablished) end private diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 89630cdbf34..13f4ce69d01 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -24145,6 +24145,9 @@ msgstr "" msgid "PackageRegistry|Install package version" msgstr "" +msgid "PackageRegistry|Instance-level" +msgstr "" + msgid "PackageRegistry|Invalid Package: failed metadata extraction" msgstr "" @@ -24190,6 +24193,9 @@ msgstr "" msgid "PackageRegistry|Pip Command" msgstr "" +msgid "PackageRegistry|Project-level" +msgstr "" + msgid "PackageRegistry|Publish and share packages for a variety of common package managers. %{docLinkStart}More information%{docLinkEnd}" msgstr "" @@ -39909,7 +39915,7 @@ msgstr "" msgid "encrypted: needs to be a :required, :optional or :migrating!" msgstr "" -msgid "ending with a file extension is not allowed." +msgid "ending with a reserved file extension is not allowed." msgstr "" msgid "entries cannot be larger than 255 characters" diff --git a/spec/frontend/packages_and_registries/package_registry/components/details/__snapshots__/npm_installation_spec.js.snap b/spec/frontend/packages_and_registries/package_registry/components/details/__snapshots__/npm_installation_spec.js.snap index 6a7f14dc33f..d5649e39561 100644 --- a/spec/frontend/packages_and_registries/package_registry/components/details/__snapshots__/npm_installation_spec.js.snap +++ b/spec/frontend/packages_and_registries/package_registry/components/details/__snapshots__/npm_installation_spec.js.snap @@ -21,6 +21,15 @@ exports[`NpmInstallation renders all the messages 1`] = ` Registry setup </h3> + <gl-form-radio-group-stub + checked="instance" + disabledfield="disabled" + htmlfield="html" + options="[object Object],[object Object]" + textfield="text" + valuefield="value" + /> + <code-instruction-stub copytext="Copy npm setup command" instruction="echo @gitlab-org:registry=npmPath/ >> .npmrc" diff --git a/spec/frontend/packages_and_registries/package_registry/components/details/npm_installation_spec.js b/spec/frontend/packages_and_registries/package_registry/components/details/npm_installation_spec.js index 083c6858ad0..b89410ede13 100644 --- a/spec/frontend/packages_and_registries/package_registry/components/details/npm_installation_spec.js +++ b/spec/frontend/packages_and_registries/package_registry/components/details/npm_installation_spec.js @@ -1,3 +1,4 @@ +import { GlFormRadioGroup } from '@gitlab/ui'; import { nextTick } from 'vue'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; @@ -12,6 +13,8 @@ import { PACKAGE_TYPE_NPM, NPM_PACKAGE_MANAGER, YARN_PACKAGE_MANAGER, + PROJECT_PACKAGE_ENDPOINT_TYPE, + INSTANCE_PACKAGE_ENDPOINT_TYPE, } from '~/packages_and_registries/package_registry/constants'; import CodeInstructions from '~/vue_shared/components/registry/code_instruction.vue'; @@ -25,12 +28,14 @@ describe('NpmInstallation', () => { const findCodeInstructions = () => wrapper.findAllComponents(CodeInstructions); const findInstallationTitle = () => wrapper.findComponent(InstallationTitle); + const findEndPointTypeSector = () => wrapper.findComponent(GlFormRadioGroup); function createComponent({ data = {} } = {}) { wrapper = shallowMountExtended(NpmInstallation, { provide: { npmHelpPath: 'npmHelpPath', npmPath: 'npmPath', + npmProjectPath: 'npmProjectPath', }, propsData: { packageEntity, @@ -53,6 +58,19 @@ describe('NpmInstallation', () => { expect(wrapper.element).toMatchSnapshot(); }); + describe('endpoint type selector', () => { + it('has the endpoint type selector', () => { + expect(findEndPointTypeSector().exists()).toBe(true); + expect(findEndPointTypeSector().vm.$attrs.checked).toBe(INSTANCE_PACKAGE_ENDPOINT_TYPE); + expect(findEndPointTypeSector().props()).toMatchObject({ + options: [ + { value: INSTANCE_PACKAGE_ENDPOINT_TYPE, text: 'Instance-level' }, + { value: PROJECT_PACKAGE_ENDPOINT_TYPE, text: 'Project-level' }, + ], + }); + }); + }); + describe('install command switch', () => { it('has the installation title component', () => { expect(findInstallationTitle().exists()).toBe(true); @@ -96,6 +114,28 @@ describe('NpmInstallation', () => { trackingAction: TRACKING_ACTION_COPY_NPM_SETUP_COMMAND, }); }); + + it('renders the correct setup command for different endpoint types', async () => { + findEndPointTypeSector().vm.$emit('change', PROJECT_PACKAGE_ENDPOINT_TYPE); + + await nextTick(); + + expect(findCodeInstructions().at(1).props()).toMatchObject({ + instruction: `echo @gitlab-org:registry=npmProjectPath/ >> .npmrc`, + multiline: false, + trackingAction: TRACKING_ACTION_COPY_NPM_SETUP_COMMAND, + }); + + findEndPointTypeSector().vm.$emit('change', INSTANCE_PACKAGE_ENDPOINT_TYPE); + + await nextTick(); + + expect(findCodeInstructions().at(1).props()).toMatchObject({ + instruction: `echo @gitlab-org:registry=npmPath/ >> .npmrc`, + multiline: false, + trackingAction: TRACKING_ACTION_COPY_NPM_SETUP_COMMAND, + }); + }); }); describe('yarn', () => { @@ -118,5 +158,27 @@ describe('NpmInstallation', () => { trackingAction: TRACKING_ACTION_COPY_YARN_SETUP_COMMAND, }); }); + + it('renders the correct setup command for different endpoint types', async () => { + findEndPointTypeSector().vm.$emit('change', PROJECT_PACKAGE_ENDPOINT_TYPE); + + await nextTick(); + + expect(findCodeInstructions().at(1).props()).toMatchObject({ + instruction: `echo \\"@gitlab-org:registry\\" \\"npmProjectPath/\\" >> .yarnrc`, + multiline: false, + trackingAction: TRACKING_ACTION_COPY_YARN_SETUP_COMMAND, + }); + + findEndPointTypeSector().vm.$emit('change', INSTANCE_PACKAGE_ENDPOINT_TYPE); + + await nextTick(); + + expect(findCodeInstructions().at(1).props()).toMatchObject({ + instruction: 'echo \\"@gitlab-org:registry\\" \\"npmPath/\\" >> .yarnrc', + multiline: false, + trackingAction: TRACKING_ACTION_COPY_YARN_SETUP_COMMAND, + }); + }); }); }); diff --git a/spec/initializers/database_config_spec.rb b/spec/initializers/database_config_spec.rb index 5ddfbd64c23..23f7fd06254 100644 --- a/spec/initializers/database_config_spec.rb +++ b/spec/initializers/database_config_spec.rb @@ -2,19 +2,11 @@ require 'spec_helper' -RSpec.describe 'Database config initializer' do +RSpec.describe 'Database config initializer', :reestablished_active_record_base do subject do load Rails.root.join('config/initializers/database_config.rb') end - around do |example| - original_config = ActiveRecord::Base.connection_db_config - - example.run - - ActiveRecord::Base.establish_connection(original_config) - end - before do allow(Gitlab::Runtime).to receive(:max_threads).and_return(max_threads) end diff --git a/spec/lib/gitlab/database/bulk_update_spec.rb b/spec/lib/gitlab/database/bulk_update_spec.rb index 33dea809d7d..9a6463c99fa 100644 --- a/spec/lib/gitlab/database/bulk_update_spec.rb +++ b/spec/lib/gitlab/database/bulk_update_spec.rb @@ -91,45 +91,38 @@ RSpec.describe Gitlab::Database::BulkUpdate do .to eq(['MR a', 'Issue a', 'Issue b']) end - shared_examples 'basic functionality' do - it 'sets multiple values' do - create_default(:user) - create_default(:project) - - i_a, i_b = create_list(:issue, 2) + context 'validates prepared_statements support', :reestablished_active_record_base do + using RSpec::Parameterized::TableSyntax - mapping = { - i_a => { title: 'Issue a' }, - i_b => { title: 'Issue b' } - } + where(:prepared_statements) do + [false, true] + end - described_class.execute(%i[title], mapping) + before do + configuration_hash = ActiveRecord::Base.connection_db_config.configuration_hash - expect([i_a, i_b].map { |x| x.reset.title }) - .to eq(['Issue a', 'Issue b']) + ActiveRecord::Base.establish_connection( + configuration_hash.merge(prepared_statements: prepared_statements) + ) end - end - include_examples 'basic functionality' + with_them do + it 'sets multiple values' do + create_default(:user) + create_default(:project) - context 'when prepared statements are configured differently to the normal test environment' do - before do - klass = Class.new(ActiveRecord::Base) do - def self.abstract_class? - true # So it gets its own connection - end - end + i_a, i_b = create_list(:issue, 2) - stub_const('ActiveRecordBasePreparedStatementsInverted', klass) + mapping = { + i_a => { title: 'Issue a' }, + i_b => { title: 'Issue b' } + } - c = ActiveRecord::Base.retrieve_connection.instance_variable_get(:@config) - inverted = c.merge(prepared_statements: !ActiveRecord::Base.connection.prepared_statements) - ActiveRecordBasePreparedStatementsInverted.establish_connection(inverted) + described_class.execute(%i[title], mapping) - allow(ActiveRecord::Base).to receive(:connection_specification_name) - .and_return(ActiveRecordBasePreparedStatementsInverted.connection_specification_name) + expect([i_a, i_b].map { |x| x.reset.title }) + .to eq(['Issue a', 'Issue b']) + end end - - include_examples 'basic functionality' end end diff --git a/spec/lib/gitlab/database/connection_spec.rb b/spec/lib/gitlab/database/connection_spec.rb index 7f94d7af4a9..ee1df141cd6 100644 --- a/spec/lib/gitlab/database/connection_spec.rb +++ b/spec/lib/gitlab/database/connection_spec.rb @@ -126,15 +126,7 @@ RSpec.describe Gitlab::Database::Connection do end end - describe '#disable_prepared_statements' do - around do |example| - original_config = connection.scope.connection.pool.db_config - - example.run - - connection.scope.establish_connection(original_config) - end - + describe '#disable_prepared_statements', :reestablished_active_record_base do it 'disables prepared statements' do connection.scope.establish_connection( ::Gitlab::Database.main.config.merge(prepared_statements: true) diff --git a/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb b/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb index 2a1f91b5b21..399fcae2fa0 100644 --- a/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb +++ b/spec/lib/gitlab/database/postgresql_adapter/force_disconnectable_mixin_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::PostgresqlAdapter::ForceDisconnectableMixin do +RSpec.describe Gitlab::Database::PostgresqlAdapter::ForceDisconnectableMixin, :reestablished_active_record_base do describe 'checking in a connection to the pool' do let(:model) do Class.new(ActiveRecord::Base) do diff --git a/spec/lib/gitlab/database/schema_migrations/context_spec.rb b/spec/lib/gitlab/database/schema_migrations/context_spec.rb index a79e6706149..c9fdcdb079c 100644 --- a/spec/lib/gitlab/database/schema_migrations/context_spec.rb +++ b/spec/lib/gitlab/database/schema_migrations/context_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Gitlab::Database::SchemaMigrations::Context do end end - context 'multiple databases' do + context 'multiple databases', :reestablished_active_record_base do let(:connection_class) do Class.new(::ApplicationRecord) do self.abstract_class = true @@ -34,8 +34,6 @@ RSpec.describe Gitlab::Database::SchemaMigrations::Context do end end - let(:configuration_overrides) { {} } - before do connection_class.establish_connection( ActiveRecord::Base diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4cbd702cb8a..3ceb460651c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -401,7 +401,7 @@ RSpec.describe User do user = build(:user, username: "test.#{type}") expect(user).not_to be_valid - expect(user.errors.full_messages).to include('Username ending with a file extension is not allowed.') + expect(user.errors.full_messages).to include('Username ending with a reserved file extension is not allowed.') expect(build(:user, username: "test#{type}")).to be_valid end end diff --git a/spec/support/database/multiple_databases.rb b/spec/support/database/multiple_databases.rb index 8ce642a682c..5e1ae60536f 100644 --- a/spec/support/database/multiple_databases.rb +++ b/spec/support/database/multiple_databases.rb @@ -5,5 +5,57 @@ module Database def skip_if_multiple_databases_not_setup skip 'Skipping because multiple databases not set up' unless Gitlab::Database.has_config?(:ci) end + + # The usage of this method switches temporarily used `connection_handler` + # allowing full manipulation of ActiveRecord::Base connections without + # having side effects like: + # - misaligned transactions since this is managed by `BeforeAllAdapter` + # - removal of primary connections + # + # The execution within a block ensures safe cleanup of all allocated resources. + # + # rubocop:disable Database/MultipleDatabases + def with_reestablished_active_record_base(reconnect: true) + connection_classes = ActiveRecord::Base.connection_handler.connection_pool_names.map(&:constantize).to_h do |klass| + [klass, klass.connection_db_config] + end + + original_handler = ActiveRecord::Base.connection_handler + new_handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new + ActiveRecord::Base.connection_handler = new_handler + + if reconnect + connection_classes.each { |klass, db_config| klass.establish_connection(db_config) } + end + + yield + ensure + ActiveRecord::Base.connection_handler = original_handler + new_handler&.clear_all_connections! + end + # rubocop:enable Database/MultipleDatabases + end + + module ActiveRecordBaseEstablishConnection + def establish_connection(*args) + # rubocop:disable Database/MultipleDatabases + if connected? && connection&.transaction_open? && ActiveRecord::Base.connection_handler == ActiveRecord::Base.default_connection_handler + raise "Cannot re-establish '#{self}.establish_connection' within an open transaction (#{connection&.open_transactions.to_i}). " \ + "Use `with_reestablished_active_record_base` instead or add `:reestablished_active_record_base` to rspec context." + end + # rubocop:enable Database/MultipleDatabases + + super + end end end + +RSpec.configure do |config| + config.around(:each, :reestablished_active_record_base) do |example| + with_reestablished_active_record_base(reconnect: example.metadata.fetch(:reconnect, true)) do + example.run + end + end +end + +ActiveRecord::Base.singleton_class.prepend(::Database::ActiveRecordBaseEstablishConnection) # rubocop:disable Database/MultipleDatabases diff --git a/spec/support_specs/database/multiple_databases_spec.rb b/spec/support_specs/database/multiple_databases_spec.rb new file mode 100644 index 00000000000..6ad15fd6594 --- /dev/null +++ b/spec/support_specs/database/multiple_databases_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Database::MultipleDatabases' do + describe '.with_reestablished_active_record_base' do + context 'when doing establish_connection' do + context 'on ActiveRecord::Base' do + it 'raises exception' do + expect { ActiveRecord::Base.establish_connection(:main) }.to raise_error /Cannot re-establish/ + end + + context 'when using with_reestablished_active_record_base' do + it 'does not raise exception' do + with_reestablished_active_record_base do + expect { ActiveRecord::Base.establish_connection(:main) }.not_to raise_error + end + end + end + end + + context 'on Ci::CiDatabaseRecord' do + before do + skip_if_multiple_databases_not_setup + end + + it 'raises exception' do + expect { Ci::CiDatabaseRecord.establish_connection(:ci) }.to raise_error /Cannot re-establish/ + end + + context 'when using with_reestablished_active_record_base' do + it 'does not raise exception' do + with_reestablished_active_record_base do + expect { Ci::CiDatabaseRecord.establish_connection(:main) }.not_to raise_error + end + end + end + end + end + + context 'when trying to access connection' do + context 'when reconnect is true' do + it 'does not raise exception' do + with_reestablished_active_record_base(reconnect: true) do + expect { ActiveRecord::Base.connection.execute("SELECT 1") }.not_to raise_error # rubocop:disable Database/MultipleDatabases + end + end + end + + context 'when reconnect is false' do + it 'does raise exception' do + with_reestablished_active_record_base(reconnect: false) do + expect { ActiveRecord::Base.connection.execute("SELECT 1") }.to raise_error(ActiveRecord::ConnectionNotEstablished) # rubocop:disable Database/MultipleDatabases + end + end + end + end + end +end |