diff options
83 files changed, 1572 insertions, 393 deletions
diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index af8306d944f..2ff4c3dc07f 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -970,17 +970,7 @@ Layout/ArgumentAlignment: - 'ee/app/graphql/types/work_items/widgets/status_filter_input_type.rb' - 'ee/app/graphql/types/work_items/widgets/status_input_type.rb' - 'ee/app/graphql/types/work_items/widgets/weight_input_type.rb' - - 'ee/app/models/approval_wrapped_rule.rb' - - 'ee/app/models/dast/pre_scan_verification.rb' - - 'ee/app/models/deployments/approval.rb' - - 'ee/app/models/ee/application_setting.rb' - 'ee/app/models/ee/ci/secure_file.rb' - - 'ee/app/models/ee/deployment.rb' - - 'ee/app/models/ee/epic.rb' - - 'ee/app/models/ee/group.rb' - - 'ee/app/models/ee/identity.rb' - - 'ee/app/models/ee/member.rb' - - 'ee/app/models/ee/merge_request.rb' - 'ee/app/models/ee/namespace.rb' - 'ee/app/models/ee/namespace/storage/notification.rb' - 'ee/app/models/gitlab_subscriptions/upcoming_reconciliation.rb' diff --git a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue index d2db61e096a..e6c3a0cba58 100644 --- a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue +++ b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue @@ -54,7 +54,7 @@ export default { // whenever a item in the autocomplete dropdown is clicked const originalClickOutHandler = this.$refs.submitDropdown.$refs.dropdown.clickOutHandler; this.$refs.submitDropdown.$refs.dropdown.clickOutHandler = (e) => { - if (!e.target.closest('.atwho-container')) { + if (!e.composedPath().includes(this.$el)) { originalClickOutHandler(e); } }; diff --git a/app/assets/javascripts/crm/components/crm_form.vue b/app/assets/javascripts/crm/components/crm_form.vue index ea6a6892bbd..0b61ffa091e 100644 --- a/app/assets/javascripts/crm/components/crm_form.vue +++ b/app/assets/javascripts/crm/components/crm_form.vue @@ -14,6 +14,8 @@ import { MountingPortal } from 'portal-vue'; import { __ } from '~/locale'; import { logError } from '~/lib/logger'; import { getFirstPropertyValue } from '~/lib/utils/common_utils'; +import { DRAWER_Z_INDEX } from '~/lib/utils/constants'; +import { getContentWrapperHeight } from '~/lib/utils/dom_utils'; import { INDEX_ROUTE_NAME } from '../constants'; const MSG_SAVE_CHANGES = __('Save changes'); @@ -241,17 +243,12 @@ export default { return data[keys[0]]; }, getDrawerHeaderHeight() { - const wrapperEl = document.querySelector('.content-wrapper'); - - if (wrapperEl) { - return `${wrapperEl.offsetTop}px`; - } - - return ''; + return getContentWrapperHeight(); }, }, MSG_CANCEL, INDEX_ROUTE_NAME, + DRAWER_Z_INDEX, }; </script> @@ -261,6 +258,7 @@ export default { :header-height="getDrawerHeaderHeight()" class="gl-drawer-responsive" :open="drawerOpen" + :z-index="$options.DRAWER_Z_INDEX" @close="close(false)" > <template #title> diff --git a/app/controllers/admin/background_migrations_controller.rb b/app/controllers/admin/background_migrations_controller.rb index a5211961d81..d1b87e67800 100644 --- a/app/controllers/admin/background_migrations_controller.rb +++ b/app/controllers/admin/background_migrations_controller.rb @@ -18,7 +18,7 @@ module Admin @current_tab = @relations_by_tab.key?(params[:tab]) ? params[:tab] : 'queued' @migrations = @relations_by_tab[@current_tab].page(params[:page]) @successful_rows_counts = batched_migration_class.successful_rows_counts(@migrations.map(&:id)) - @databases = Gitlab::Database.db_config_names + @databases = Gitlab::Database.db_config_names(with_schema: :gitlab_shared) end def show diff --git a/app/graphql/mutations/dependency_proxy/group_settings/update.rb b/app/graphql/mutations/dependency_proxy/group_settings/update.rb index 6be07edd883..ee510373f34 100644 --- a/app/graphql/mutations/dependency_proxy/group_settings/update.rb +++ b/app/graphql/mutations/dependency_proxy/group_settings/update.rb @@ -8,10 +8,11 @@ module Mutations include Mutations::ResolvesGroup - description 'These settings can be adjusted by the group Owner or Maintainer. However, in GitLab 16.0, we ' \ - 'will be limiting this to the Owner role. ' \ - '[GitLab-#364441](https://gitlab.com/gitlab-org/gitlab/-/issues/364441) proposes making ' \ - 'this change to match the permissions level in the user interface.' + description <<~DESC + These settings can be adjusted by the group Owner or Maintainer. + [Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting + this to Owners only to match the permissions level in the user interface. + DESC authorize :admin_dependency_proxy diff --git a/app/graphql/mutations/dependency_proxy/image_ttl_group_policy/update.rb b/app/graphql/mutations/dependency_proxy/image_ttl_group_policy/update.rb index 79d7a93c4e2..0759b8e1beb 100644 --- a/app/graphql/mutations/dependency_proxy/image_ttl_group_policy/update.rb +++ b/app/graphql/mutations/dependency_proxy/image_ttl_group_policy/update.rb @@ -8,6 +8,12 @@ module Mutations include Mutations::ResolvesGroup + description <<~DESC + These settings can be adjusted by the group Owner or Maintainer. + [Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting + this to Owners only to match the permissions level in the user interface. + DESC + authorize :admin_dependency_proxy argument :group_path, diff --git a/app/graphql/mutations/namespace/package_settings/update.rb b/app/graphql/mutations/namespace/package_settings/update.rb index ea72b71715c..96bee693a1e 100644 --- a/app/graphql/mutations/namespace/package_settings/update.rb +++ b/app/graphql/mutations/namespace/package_settings/update.rb @@ -8,6 +8,12 @@ module Mutations include Mutations::ResolvesNamespace + description <<~DESC + These settings can be adjusted by the group Owner or Maintainer. + [Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting + this to Owners only to match the permissions level in the user interface. + DESC + authorize :admin_package argument :namespace_path, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index a63159dc4af..907d578eb37 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -734,6 +734,7 @@ class ApplicationSetting < MainClusterwide::ApplicationRecord attr_encrypted :tofa_response_json_keys, encryption_options_base_32_aes_256_gcm.merge(encode: false, encode_iv: false) attr_encrypted :tofa_url, encryption_options_base_32_aes_256_gcm.merge(encode: false, encode_iv: false) attr_encrypted :tofa_access_token_expires_in, encryption_options_base_32_aes_256_gcm.merge(encode: false, encode_iv: false) + attr_encrypted :ai_access_token, encryption_options_base_32_aes_256_gcm.merge(encode: false, encode_iv: false) validates :disable_feed_token, inclusion: { in: [true, false], message: N_('must be a boolean value') } diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 285721de387..5b22851a98c 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -109,6 +109,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy @subject.runner_registration_enabled? end + condition(:raise_admin_package_to_owner_enabled) do + Feature.enabled?(:raise_group_admin_package_permission_to_owner, @subject) + end + rule { can?(:read_group) & design_management_enabled }.policy do enable :read_design_activity end @@ -202,7 +206,6 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy rule { maintainer }.policy do enable :destroy_package - enable :admin_package enable :create_projects enable :import_projects enable :admin_pipeline @@ -304,7 +307,11 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy rule { dependency_proxy_access_allowed & dependency_proxy_available } .enable :read_dependency_proxy - rule { maintainer & dependency_proxy_available }.policy do + rule { maintainer & dependency_proxy_available & ~raise_admin_package_to_owner_enabled }.policy do + enable :admin_dependency_proxy + end + + rule { owner & dependency_proxy_available & raise_admin_package_to_owner_enabled }.policy do enable :admin_dependency_proxy end @@ -370,6 +377,9 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy # Should be matched with ProjectPolicy#read_internal_note rule { admin | reporter }.enable :read_internal_note + rule { maintainer & ~raise_admin_package_to_owner_enabled }.enable :admin_package + rule { owner & raise_admin_package_to_owner_enabled }.enable :admin_package + def access_level(for_any_session: false) return GroupMember::NO_ACCESS if @user.nil? return GroupMember::NO_ACCESS unless user_is_user? diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb index 6058c89d347..26dc748ad51 100644 --- a/app/serializers/note_entity.rb +++ b/app/serializers/note_entity.rb @@ -109,8 +109,6 @@ class NoteEntity < API::Entities::Note end def external_author - return unless Feature.enabled?(:external_note_author_service_desk) - return unless object.note_metadata&.external_author if can?(current_user, :read_external_emails, object.project) diff --git a/config/feature_flags/development/external_note_author_service_desk.yml b/config/feature_flags/development/raise_group_admin_package_permission_to_owner.yml index bd5b5d9d350..7136e9e2ad0 100644 --- a/config/feature_flags/development/external_note_author_service_desk.yml +++ b/config/feature_flags/development/raise_group_admin_package_permission_to_owner.yml @@ -1,8 +1,8 @@ --- -name: external_note_author_service_desk -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117149 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/408932 -milestone: '16.0' +name: raise_group_admin_package_permission_to_owner +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118360 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/409642 +milestone: '16.1' type: development -group: group::respond +group: group::package registry default_enabled: false diff --git a/config/initializers/validate_database_config.rb b/config/initializers/validate_database_config.rb index d381dbac2ed..d1fa9bb345d 100644 --- a/config/initializers/validate_database_config.rb +++ b/config/initializers/validate_database_config.rb @@ -10,11 +10,12 @@ if configurations = ActiveRecord::Base.configurations.configurations "The `main:` database needs to be defined as a first configuration item instead of `#{configurations.first.name}`." end - rejected_config_names = configurations.map(&:name).to_set - Gitlab::Database.all_database_names + all_database_names = Gitlab::Database.all_database_names + rejected_config_names = configurations.map(&:name).to_set - all_database_names if rejected_config_names.any? raise "ERROR: This installation of GitLab uses unsupported database names " \ "in 'config/database.yml': #{rejected_config_names.to_a.join(", ")}. The only supported ones are " \ - "#{Gitlab::Database.all_database_names.join(", ")}." + "#{all_database_names.join(", ")}." end replicas_config_names = configurations.select(&:replica?).map(&:name) diff --git a/db/database_connections/ci.yaml b/db/database_connections/ci.yaml new file mode 100644 index 00000000000..daa155dcb00 --- /dev/null +++ b/db/database_connections/ci.yaml @@ -0,0 +1,10 @@ +name: ci +description: Cell-local GitLab database holding all CI pipelines, builds, etc. +gitlab_schemas: + - gitlab_internal + - gitlab_shared + - gitlab_ci +klass: Ci::ApplicationRecord +# if CI database is not configured, use this database +fallback_database: main +uses_load_balancing: true diff --git a/db/database_connections/main.yaml b/db/database_connections/main.yaml new file mode 100644 index 00000000000..f6bd50a2ad8 --- /dev/null +++ b/db/database_connections/main.yaml @@ -0,0 +1,14 @@ +name: main +description: Main GitLab database holding all projects, issues, etc. +gitlab_schemas: + - gitlab_internal + - gitlab_shared + - gitlab_main + - gitlab_pm +# Note that we use ActiveRecord::Base here and not ApplicationRecord. +# This is deliberate, as: +# - the load balancer must be enabled for _all_ models +# - other models outside of Rails that are using `ActiveRecord::Base` +# needs to use `main:` +klass: ActiveRecord::Base +uses_load_balancing: true diff --git a/db/database_connections/main_clusterwide.yaml b/db/database_connections/main_clusterwide.yaml new file mode 100644 index 00000000000..d55b2ddce40 --- /dev/null +++ b/db/database_connections/main_clusterwide.yaml @@ -0,0 +1,10 @@ +name: main_clusterwide +description: Cluster-wide GitLab database holding only data shared globally across Cells. +gitlab_schemas: + - gitlab_internal + - gitlab_shared + - gitlab_main_clusterwide +klass: MainClusterwide::ApplicationRecord +# if Cluster-wide database is not configured, fallback to using main +fallback_database: main +uses_load_balancing: true diff --git a/db/gitlab_schemas/gitlab_ci.yaml b/db/gitlab_schemas/gitlab_ci.yaml new file mode 100644 index 00000000000..ee429fbf19c --- /dev/null +++ b/db/gitlab_schemas/gitlab_ci.yaml @@ -0,0 +1,2 @@ +name: gitlab_ci +description: Schema for all Cell-local CI tables, ex. ci_builds, etc. diff --git a/db/gitlab_schemas/gitlab_internal.yaml b/db/gitlab_schemas/gitlab_internal.yaml new file mode 100644 index 00000000000..7093c980daa --- /dev/null +++ b/db/gitlab_schemas/gitlab_internal.yaml @@ -0,0 +1,2 @@ +name: gitlab_internal +description: Schema for all internal tables, like `pg_*` diff --git a/db/gitlab_schemas/gitlab_main.yaml b/db/gitlab_schemas/gitlab_main.yaml new file mode 100644 index 00000000000..ef94d9f8668 --- /dev/null +++ b/db/gitlab_schemas/gitlab_main.yaml @@ -0,0 +1,2 @@ +name: gitlab_main +description: Schema for all Cell-local tables, ex. projects, issues, etc. diff --git a/db/gitlab_schemas/gitlab_main_clusterwide.yaml b/db/gitlab_schemas/gitlab_main_clusterwide.yaml new file mode 100644 index 00000000000..d7b54489573 --- /dev/null +++ b/db/gitlab_schemas/gitlab_main_clusterwide.yaml @@ -0,0 +1,2 @@ +name: gitlab_main_clusterwide +description: Schema for all Cluster-wide tables, ex. application_settings, etc. diff --git a/db/gitlab_schemas/gitlab_pm.yaml b/db/gitlab_schemas/gitlab_pm.yaml new file mode 100644 index 00000000000..5cef6188c3a --- /dev/null +++ b/db/gitlab_schemas/gitlab_pm.yaml @@ -0,0 +1,2 @@ +name: gitlab_pm +description: Schema for all Cell-local package management features. diff --git a/db/gitlab_schemas/gitlab_shared.yaml b/db/gitlab_schemas/gitlab_shared.yaml new file mode 100644 index 00000000000..f28adf4f1cb --- /dev/null +++ b/db/gitlab_schemas/gitlab_shared.yaml @@ -0,0 +1,4 @@ +name: gitlab_shared +description: + Schema for all tables implementing shared features, + ex. loose foreign keys, re-indexing, etc. diff --git a/db/migrate/20230523122242_add_encrypted_ai_access_token.rb b/db/migrate/20230523122242_add_encrypted_ai_access_token.rb new file mode 100644 index 00000000000..66035dc9c02 --- /dev/null +++ b/db/migrate/20230523122242_add_encrypted_ai_access_token.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class AddEncryptedAiAccessToken < Gitlab::Database::Migration[2.1] + def change + add_column :application_settings, :encrypted_ai_access_token, :binary + add_column :application_settings, :encrypted_ai_access_token_iv, :binary + end +end diff --git a/db/schema_migrations/20230523122242 b/db/schema_migrations/20230523122242 new file mode 100644 index 00000000000..36316e88777 --- /dev/null +++ b/db/schema_migrations/20230523122242 @@ -0,0 +1 @@ +16c5eed18bf8beb8200fc2eaae9ffe532439e463cdb417f84eabc29992779558
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 19d71325370..63c2d188b14 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11844,6 +11844,8 @@ CREATE TABLE application_settings ( wiki_asciidoc_allow_uri_includes boolean DEFAULT false NOT NULL, namespace_aggregation_schedule_lease_duration_in_seconds integer DEFAULT 300 NOT NULL, container_registry_data_repair_detail_worker_max_concurrency integer DEFAULT 2 NOT NULL, + encrypted_ai_access_token bytea, + encrypted_ai_access_token_iv bytea, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_container_registry_pre_import_tags_rate_positive CHECK ((container_registry_pre_import_tags_rate >= (0)::numeric)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), diff --git a/doc/administration/geo/replication/multiple_servers.md b/doc/administration/geo/replication/multiple_servers.md index 155fefb2dbf..ca4f1c36e69 100644 --- a/doc/administration/geo/replication/multiple_servers.md +++ b/doc/administration/geo/replication/multiple_servers.md @@ -79,6 +79,14 @@ The following steps enable a GitLab site to serve as the Geo **primary** site. After making these changes, [reconfigure GitLab](../../restart_gitlab.md#omnibus-gitlab-reconfigure) so the changes take effect. +### Step 2: Define the site as the **primary** site + +1. Execute the following command on one of the frontend nodes: + + ```shell + sudo gitlab-ctl set-geo-primary-node + ``` + NOTE: PostgreSQL and Redis should have already been disabled on the application nodes during typical GitLab multi-node setup. Connections diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 772b10ed863..7577b260f6b 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -36,7 +36,7 @@ The following metrics are available: | Metric | Type | Since | Description | Labels | | :--------------------------------------------------------------- | :---------- | ------: | :-------------------------------------------------------------------------------------------------------------------- | :-------------------------------------------------------- | -| `gitlab_cache_misses_total` | Counter | 10.2 | Cache read miss | `controller`, `action` | +| `gitlab_cache_misses_total` | Counter | 10.2 | Cache read miss | `controller`, `action`, `store` | | `gitlab_cache_operation_duration_seconds` | Histogram | 10.2 | Cache access time | `operation`, `store` | | `gitlab_cache_operations_total` | Counter | 12.2 | Cache operations by controller or action | `controller`, `action`, `operation`, `store` | | `gitlab_cache_read_multikey_count` | Histogram | 15.7 | Count of keys in multi-key cache read operations | `controller`, `action`, `store` | @@ -63,8 +63,8 @@ The following metrics are available: | `gitlab_transaction_cache_<key>_duration_total` | Counter | 10.2 | Counter for total time (seconds) spent in Rails cache calls (per key) | | | `gitlab_transaction_cache_count_total` | Counter | 10.2 | Counter for total Rails cache calls (aggregate) | | | `gitlab_transaction_cache_duration_total` | Counter | 10.2 | Counter for total time (seconds) spent in Rails cache calls (aggregate) | | -| `gitlab_transaction_cache_read_hit_count_total` | Counter | 10.2 | Counter for cache hits for Rails cache calls | `controller`, `action` | -| `gitlab_transaction_cache_read_miss_count_total` | Counter | 10.2 | Counter for cache misses for Rails cache calls | `controller`, `action` | +| `gitlab_transaction_cache_read_hit_count_total` | Counter | 10.2 | Counter for cache hits for Rails cache calls | `controller`, `action`, `store` | +| `gitlab_transaction_cache_read_miss_count_total` | Counter | 10.2 | Counter for cache misses for Rails cache calls | `controller`, `action`, `store` | | `gitlab_transaction_duration_seconds` | Histogram | 10.2 | Duration for successful requests (`gitlab_transaction_*` metrics) | `controller`, `action` | | `gitlab_transaction_event_build_found_total` | Counter | 9.4 | Counter for build found for API /jobs/request | | | `gitlab_transaction_event_build_invalid_total` | Counter | 9.4 | Counter for build invalid due to concurrency conflict for API /jobs/request | | diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 938d8828cfc..73ec3ed8781 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -6347,6 +6347,10 @@ Input type: `UpdateContainerExpirationPolicyInput` ### `Mutation.updateDependencyProxyImageTtlGroupPolicy` +These settings can be adjusted by the group Owner or Maintainer. +[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting +this to Owners only to match the permissions level in the user interface. + Input type: `UpdateDependencyProxyImageTtlGroupPolicyInput` #### Arguments @@ -6368,7 +6372,9 @@ Input type: `UpdateDependencyProxyImageTtlGroupPolicyInput` ### `Mutation.updateDependencyProxySettings` -These settings can be adjusted by the group Owner or Maintainer. However, in GitLab 16.0, we will be limiting this to the Owner role. [GitLab-#364441](https://gitlab.com/gitlab-org/gitlab/-/issues/364441) proposes making this change to match the permissions level in the user interface. +These settings can be adjusted by the group Owner or Maintainer. +[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting +this to Owners only to match the permissions level in the user interface. Input type: `UpdateDependencyProxySettingsInput` @@ -6529,6 +6535,10 @@ Input type: `UpdateIterationInput` ### `Mutation.updateNamespacePackageSettings` +These settings can be adjusted by the group Owner or Maintainer. +[Issue 370471](https://gitlab.com/gitlab-org/gitlab/-/issues/370471) proposes limiting +this to Owners only to match the permissions level in the user interface. + Input type: `UpdateNamespacePackageSettingsInput` #### Arguments @@ -11754,7 +11764,7 @@ Information about a connected Agent. | Name | Type | Description | | ---- | ---- | ----------- | -| <a id="aicachedmessagetypecontent"></a>`content` | [`String`](#string) | Content of the message. Can be null for user requests or failed responses. | +| <a id="aicachedmessagetypecontent"></a>`content` | [`String`](#string) | Content of the message. Can be null for failed responses. | | <a id="aicachedmessagetypeerrors"></a>`errors` | [`[String!]!`](#string) | Errors that occurred while asynchronously fetching an AI (assistant) response. | | <a id="aicachedmessagetypeid"></a>`id` | [`ID`](#id) | UUID of the message. | | <a id="aicachedmessagetyperequestid"></a>`requestId` | [`ID`](#id) | UUID of the original request message. | diff --git a/doc/api/users.md b/doc/api/users.md index fc441ab1925..809d1474803 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -128,6 +128,7 @@ GET /users | `without_projects` | boolean | no | Filter users without projects. Default is `false`, which means that all users are returned, with and without projects. | | `admins` | boolean | no | Return only administrators. Default is `false` | | `saml_provider_id` **(PREMIUM)** | number | no | Return only users created by the specified SAML provider ID. If not included, it returns all users. | +| `skip_ldap` **(PREMIUM)** | boolean | no | Skip LDAP users. | ```json [ diff --git a/doc/architecture/blueprints/remote_development/img/remote_dev_15_7.png b/doc/architecture/blueprints/remote_development/img/remote_dev_15_7.png Binary files differdeleted file mode 100644 index f36bfa24998..00000000000 --- a/doc/architecture/blueprints/remote_development/img/remote_dev_15_7.png +++ /dev/null diff --git a/doc/architecture/blueprints/remote_development/img/remote_dev_15_7_1.png b/doc/architecture/blueprints/remote_development/img/remote_dev_15_7_1.png Binary files differdeleted file mode 100644 index aab946923e2..00000000000 --- a/doc/architecture/blueprints/remote_development/img/remote_dev_15_7_1.png +++ /dev/null diff --git a/doc/architecture/blueprints/remote_development/index.md b/doc/architecture/blueprints/remote_development/index.md index e2647551a95..1539a0878f0 100644 --- a/doc/architecture/blueprints/remote_development/index.md +++ b/doc/architecture/blueprints/remote_development/index.md @@ -10,27 +10,25 @@ participating-stages: [] <!-- vale gitlab.FutureTense = NO --> -# Remote Development +# Remote development ## Summary -Remote Development is a new architecture for our software-as-a-service platform that provides a more consistent user experience writing code hosted in GitLab. It may also provide additional features in the future, such as a purely browser-based workspace and the ability to connect to an already running VM/Container or to use a GitLab-hosted VM/Container. +Remote development is a new architecture for our software-as-a-service platform that provides a more consistent user experience writing code hosted in GitLab. It may also provide additional features in the future, such as a purely browser-based workspace and the ability to connect to an already running VM/Container or to use a GitLab-hosted VM/Container. -## Web IDE and Remote Development +## Web IDE and remote development -It is important to note that `Remote Development !== Web IDE`, and this is something we want to be explicit about in this document as the terms can become conflated when they shouldn't. Our new Web IDE is a separate ongoing effort that is running in parallel to Remote Development. +It is important to note that `remote development !== Web IDE`, and this is something we want to be explicit about in this document as the terms can become conflated when they shouldn't. Our new Web IDE is a separate ongoing effort that is running in parallel to remote development. These two separate categories do have some overlap as it is a goal to allow a user to connect a running workspace to the Web IDE, **but** this does not mean the two are dependent on one another. -You can use the [Web IDE](../../../user/project/web_ide/index.md) to commit changes to a project directly from your web browser without installing any dependencies or cloning any repositories. The Web IDE, however, lacks a native runtime environment on which you would compile code, run tests, or generate real-time feedback in the IDE. For a more complete IDE experience, you can pair the Web IDE with a Remote Development workspace that has been properly configured to run as a host. - - +You can use the [Web IDE](../../../user/project/web_ide/index.md) to commit changes to a project directly from your web browser without installing any dependencies or cloning any repositories. The Web IDE, however, lacks a native runtime environment on which you would compile code, run tests, or generate real-time feedback in the IDE. For a more complete IDE experience, you can pair the Web IDE with a remote development workspace that has been properly configured to run as a host. ## Long-term vision As a [new Software Developer to a team such as Sasha](https://about.gitlab.com/handbook/product/personas/#sasha-software-developer) with no local development environment, I should be able to: -- Navigate to a repository on GitLab.com or self-managed. +- Go to a repository on GitLab.com or self-managed. - Click a button that will provide a list of current workspaces for this repository. - Click a button that will create a new workspace or select an existing workspace from a list. - Go through a configuration wizard that will let me select various options for my workspace (memory/CPU). @@ -38,14 +36,137 @@ As a [new Software Developer to a team such as Sasha](https://about.gitlab.com/h - Make code changes, run tests, troubleshoot based on the terminal output, and commit new changes. - Submit MRs of any kind without having to clone the repository locally or to manually update a local development environment. -## User Flow Diagram +## Terminology + +We use the following terms to describe components and properties of the remote development architecture. + +### Remote development + +Remote development allows you to use a secure development environment in the cloud that you can connect to from your local machine through a web browser or a client-based solution with the purpose of developing a software product there. + +#### Remote development properties + +- Separate your development environment to avoid impacting your local machine configuration. +- Make it easy for new contributors to get started and keep everyone on a consistent environment. +- Use tools or runtimes not available on your local OS or manage multiple versions of them. +- Access an existing development environment from multiple machines or locations. + +Discouraged synonyms: VS Code for web, remote development Extension, browser-only WebIDE, Client only WebIDE + +### Workspace + +Container/VM-based developer machines providing all the tools and dependencies needed to code, build, test, run, and debug applications. + +#### Workspace properties + +- Workspaces should be isolated from each other by default and are responsible for managing the lifecycle of their components. This isolation can be multi-layered: namespace isolation, network isolation, resources isolation, node isolation, sandboxing containers, etc. ([reference](https://kubernetes.io/docs/concepts/security/multi-tenancy/)). +- A workspace should contain project components as well as editor components. +- A workspace should be a combination of resources that support cloud-based development environment. +- Workspaces are constrained by the amount of resources provided to them. + +### Web IDE + +VS Code for web - replacement of our current legacy Web IDE. + +#### Web IDE properties + +A package for bootstrapping GitLab context-aware Web IDE that: + +- Is built on top of Microsoft's VS Code. We customize and add VS Code features in the [GitLab fork of the VS Code project](https://gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork). +- Can be configured in a way that it connects to the workspace rather than only using the browser. When connected to a workspace, a user should be able to do the following from the Web IDE: + - Edit, build, or debug on a different OS than they are running locally. + - Make use of larger or more specialized hardware than their local machine for development. + - Separate developer environments to avoid conflicts, improve security, and speed up onboarding. + +### Remote development extension for desktop + +Something that plugs into the desktop IDE and connects you to the workspace. + +#### Remote development extension for desktop properties + +- Allows you to open any folder in a workspace. +- Should be desktop IDE agnostic. +- Should have access to local files or APIs. + +## Goals + +### A consistent experience + +Organizations should have the same user experience on our SaaS platform as they do on a self-managed GitLab instance. We want to abstract away the user's development environment to avoid impacting their local machine configuration. We also want to provide support for developing on the same operating system you deploy to or use larger or more specialized hardware. + +A major goal is that each member of a development team should have the same development experience minus any specialized local configuration. This will also make it easy for new contributors to get started and keep everyone on a consistent environment. + +### Increased availability + +A workspace should allow access to an existing development environment from multiple machines and locations across a single or multiple teams. It should also allow a user to make use of tools or runtimes not available on their local OS or manage multiple versions of them. + +Additionally, remote development workspaces could provide a way to implement disaster recovery if we are able to leverage the capabilities of [Cells](../../../architecture/blueprints/pods/index.md). + +### Scalability + +As an organization begins to scale, they quickly realize the need to support additional types of projects that might require extensive workflows. Remote development workspaces aim to solve that issue by abstracting away the burden of complex machine configuration, dependency management, and possible data-seeding issues. + +To facilitate working on different features across different projects, remote development should allow each user to provision multiple workspaces to enable quick context switching. + +Eventually, we should be able to allow users to vertically scale their workspaces with more compute cores, memory, and other resources. If a user is currently working against a 2 CPU and 4 GB RAM workspace but comes to find they need more CPU, they should be able to upgrade their compute layer to something more suitable with a click or CLI command in the workspace. + +### Built-in security and enterprise readiness + +As remote development becomes a viable replacement for virtual desktop infrastructure solutions, it must be secure and support enterprise requirements. These include role-based access control and the ability to remove all source code from developer machines. + +### Faster project and developer onboarding + +As a zero-install development environment that runs in your browser, remote development makes it easy for anyone to join your team and contribute to a project. + +### Regions + +GitLab.com is only hosted in the United States of America. Organizations located in other regions have voiced demand for local SaaS offerings. BYO infrastructure helps work in conjunction with [GitLab Regions](https://gitlab.com/groups/gitlab-org/-/epics/6037) because a user's workspace may be deployed in different geographies. The ability to deploy workspaces to different geographies might also help to solve data residency and compliance problems. + +## Market analysis + +We have conducted a market analysis to understand the broader market and what others can offer us by way of open-source libraries, integrations, or partnership opportunities. We have broken down the effort into a set of issues where we investigate each potential competitor/pathway/partnership as a spike. + +- [Market analysis](https://gitlab.com/groups/gitlab-org/-/epics/8131) +- [YouTube results](https://www.youtube.com/playlist?list=PL05JrBw4t0KrRQhnSYRNh1s1mEUypx67-) + +## Che vs DevWorkspace Operator vs custom-built solution + +After an investigation into using [Che](https://gitlab.com/gitlab-org/gitlab/-/issues/366052) as our backend to accelerate remote development, we ultimately opted to [write our own custom-built solution using DevWorkspace Operator](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97449#note_1131215629). + +Some advantages of us opting to write our own custom-built solution are: + +- We can still use the core DevWorkspace Operator and build on top of it. +- It is easier to add support for other configurations apart from `devfile` in the future if the need arises. +- We have the ability to choose which tech stack to use (for example, instead of using `traefik`, which is used in Che, explore NGINX itself or use the GitLab Agent for Kubernetes). + +After writing our own custom-built solution using DevWorkspace Operator, +we decided to [remove the dependency on DevWorkspace Operator](https://gitlab.com/groups/gitlab-org/-/epics/9895) +and thus the transitive dependency of Cert Manager. + +## Architecture details + +Remote development is delivered as a module in the +[GitLab Agent for Kubernetes](../../../user/clusters/agent/index.md) project. +The overall goal of this architecture is to ensure that the **actual state** of all +remote development workspaces running in the Kubernetes clusters is reconciled with the **desired state** of the +workspaces as set by the user. + +This is accomplished as follows: - +1. The desired state of the workspaces is obtained from user actions in the GitLab UI or API and persisted in the Rails database. +1. There is a reconciliation loop between the agent and Rails, which: + - Retrieves the actual state of the workspaces from the Kubernetes clusters through the agent and sends it to Rails to be persisted. + - Rails compares the actual state with the desired state and responds with actions to bring the actual state in line with the desired state for all workspaces. -## Architecture +### System design ```plantuml @startuml + +title + System design for Remote Development +end title + node "Kubernetes" { [Ingress Controller] --> [GitLab Workspaces Proxy] : Decrypt Traffic @@ -64,11 +185,11 @@ node "Kubernetes" { [GitLab Workspaces Proxy] ..> [Workspace 2] : Forward traffic\nfor workspace 2 [GitLab Workspaces Proxy] --> [Workspace 1] : Forward traffic\nfor workspace 1 - [Agentk] .up.> [Workspace n] : Applies kubernetes resources\nfor workspace n - [Agentk] .up.> [Workspace 2] : Applies kubernetes resources\nfor workspace 2 - [Agentk] .up.> [Workspace 1] : Applies kubernetes resources\nfor workspace 1 + [Agent] .up.> [Workspace n] : Applies kubernetes resources\nfor workspace n + [Agent] .up.> [Workspace 2] : Applies kubernetes resources\nfor workspace 2 + [Agent] .up.> [Workspace 1] : Applies kubernetes resources\nfor workspace 1 - [Agentk] --> [Kubernetes API Server] : Interact and get/apply\nKubernetes resources + [Agent] --> [Kubernetes API Server] : Interact and get/apply\nKubernetes resources } node "GitLab" { @@ -78,12 +199,12 @@ node "GitLab" { [KAS] -up-> [GitLab Rails] : Proxy } -[Agentk] -up-> [KAS] : Initiate reconciliation loop +[Agent] -up-> [KAS] : Initiate reconciliation loop "Load Balancer IP" --> [Ingress Controller] [Browser] --> [Nginx] : Browse GitLab [Browser] -right-> "Domain IP" : Browse workspace URL "Domain IP" .right.> "Load Balancer IP" -[GitLab Workspaces Proxy] ..> [GitLab Rails] : Authenticate and authorize\nthe user accessing the workspace. +[GitLab Workspaces Proxy] ..> [GitLab Rails] : Authenticate and authorize\nthe user accessing the workspace note top of "Domain IP" For local development, workspace URL @@ -100,132 +221,495 @@ end note @enduml ``` -## Terminology +### Remote development with the GitLab Agent for Kubernetes topology -We use the following terms to describe components and properties of the Remote Development architecture. +- The Kubernetes API is not shown in this diagram, but it is assumed that it is managing the workspaces through the agent. +- The numbers of components in each Kubernetes cluster are arbitrary. -### Remote Development +```plantuml +@startuml -Remote Development allows you to use a secure development environment in the cloud that you can connect to from your local machine through a web browser or a client-based solution with the purpose of developing a software product there. +title + Remote Development with GitLab Agent for Kubernetes topology +end title -#### Remote Development properties +node "GitLab Monolith" as gitlab { + rectangle "kas deployment" as kas_deployment { + collections kas1..kas8 + } + rectangle rails + database postgres -- Separate your development environment to avoid impacting your local machine configuration. -- Make it easy for new contributors to get started and keep everyone on a consistent environment. -- Use tools or runtimes not available on your local OS or manage multiple versions of them. -- Access an existing development environment from multiple machines or locations. + kas_deployment - rails + rails -left- postgres +} -Discouraged synonyms: VS Code for web, Remote Development Extension, browser-only WebIDE, Client only WebIDE +node "kubernetes cluster 1" as kubernetes1 { + rectangle "agent A workspaces" as agent_a_workspaces { + collections workspace2..workspace8 + rectangle workspace1 + } -### Workspace + rectangle "agent B workspaces" as agent_b..agent_b_8_workspaces { + collections workspace10..workspace16 + rectangle workspace9 + } -Container/VM-based developer machines providing all the tools and dependencies needed to code, build, test, run, and debug applications. + rectangle "agent A deployment" as agent_a_deployment { + rectangle agent_a_1 + } -#### Workspace properties + rectangle "agent B deployment" as agent_b_deployment { + collections agent_b_1..agent_b_8 + } -- Workspaces should be isolated from each other by default and are responsible for managing the lifecycle of their components. This isolation can be multi-layered: namespace isolation, network isolation, resources isolation, node isolation, sandboxing containers, etc. ([reference](https://kubernetes.io/docs/concepts/security/multi-tenancy/)). -- A workspace should contain project components as well as editor components. -- A workspace should be a combination of resources that support cloud-based development environment. -- Workspaces are constrained by the amount of resources provided to them. + agent_a_1 - agent_a_workspaces + agent_b_1..agent_b_8 - agent_b..agent_b_8_workspaces +} -### Web IDE +node "kubernetes cluster 2" as kubernetes2 { + rectangle "agent C workspaces" as agent_c_workspaces { + collections workspace18..workspace24 + rectangle workspace17 + } -VS Code for web - replacement of our current legacy Web IDE. + rectangle "agent C deployment" as agent_c_deployment { + rectangle agent_c_1 + } -#### Web IDE properties + agent_c_1 -down- agent_c_workspaces +} -A package for bootstrapping GitLab context-aware Web IDE that: -- Is built on top of Microsoft's VS Code. We customize and add VS Code features in the [GitLab fork of the VS Code project](https://gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork). -- Can be configured in a way that it connects to the workspace rather than only using the browser. When connected to a workspace, a user should be able to do the following from the Web IDE: - - Edit, build, or debug on a different OS than they are running locally. - - Make use of larger or more specialized hardware than their local machine for development. - - Separate developer environments to avoid conflicts, improve security, and speed up onboarding. +cloud cloud -### Remote Development Extension for Desktop +cloud - kas1..kas8 +cloud - agent_a_1 +cloud - agent_b_1..agent_b_8 +cloud - agent_c_1 -Something that plugs into the desktop IDE and connects you to the workspace. -#### Remote Development Extension for Desktop properties +'the following hidden line is a hack to get the diagram to render correctly +agent_a_1 -[hidden]- agent_b_1..agent_b_8 +gitlab -[hidden]d- kubernetes2 -- Allows you to open any folder in a workspace. -- Should be desktop IDE agnostic. -- Should have access to local files or APIs. +@enduml +``` -## Goals +### High-level overview of the communication between Rails and the agent -### A consistent experience +```plantuml +@startuml +!pragma teoz true + +title + High level overview of the communication between rails and agent +end title + +box gitlab monolith #Beige +participant rails order 20 +box kas #Bisque +participant "kas" as kas order 40 +end box +end box + +box Kubernetes cluster #Beige +box agent #Bisque +participant "agent remote_development\nmodule" as agent_rd_mod order 50 +end box +participant kubernetes order 60 +end box + +loop forever + agent_rd_mod -> kubernetes: Subscribe to Kubernetes for\nworkspace changes associated with the agent + activate agent_rd_mod + + autoactivate on + agent_rd_mod -> kas: POST request with\nupdated workspace information + note right + Any updated workspace information from + Kubernetes is pushed with next reconciliation. + end note + kas -> rails: proxy POST request from agent to rails -Organizations should have the same user experience on our SaaS platform as they do on a self-managed GitLab instance. We want to abstract away the user's development environment to avoid impacting their local machine configuration. We also want to provide support for developing on the same operating system you deploy to or use larger or more specialized hardware. + return Respond with all workspaces to be created/updated/terminated\nalong with an acknowledgement that all information sent by\nagent have been persisted into the database successfully + return proxy workspace information to agent + autoactivate off -A major goal is that each member of a development team should have the same development experience minus any specialized local configuration. This will also make it easy for new contributors to get started and keep everyone on a consistent environment. + agent_rd_mod -> kubernetes: Apply any received workspace information to kubernetes + deactivate agent_rd_mod +end loop -### Increased availability +@enduml +``` -A workspace should allow access to an existing development environment from multiple machines and locations across a single or multiple teams. It should also allow a user to make use of tools or runtimes not available on their local OS or manage multiple versions of them. +### Types of messages between Rails and the agent + +The agent can send different types of messages to Rails to capture different information. Depending on what type of message the agent sends, Rails will respond accordingly. + +Different types of messages are: + +- `prerequisites` (yet to be implemented) - This is the first message the agent sends to Rails after the agent starts or restarts or after a leader-election. + - Actions performed by the agent: + - Fetch Kubernetes resources that are required to be available in the Kubernetes cluster + - Actions performed by Rails: + - Send the Kubernetes manifests for `gitlab-workspaces-proxy` that need to be available in the Kubernetes cluster. +- `reconcile` - Messages sent to Rails to persist the current state of the workspaces. There are two types of updates specified by the `update_type` field with the following possible values: `full` and `partial`. The payload schema remains the same for both update types. + - `full` + - Actions performed by the agent: + - Send the current state of all the workspaces in the Kubernetes cluster managed by the agent. + - To keep things consistent between the agent and Rails, the agent will send this message every time agent undergoes a full reconciliation cycle that occurs + - when an agent starts or restarts + - after a leader-election + - periodically, as set using the `full_sync_interval` configuration (default: once every hour) + - whenever the agent configuration is updated + - Actions performed by Rails: + - Update Postgres with the current state and respond with all the workspaces managed by the agent and their last resource version that Rails has persisted in Postgres. + - Returning the persisted resource version back to the agent gives it a confirmation that the updates for that workspace have been successfully processed on the Rails end. + - This persisted resource version will also help with sending only the latest workspaces changes from the agent to Rails for `reconcile` message with `partial` update type. + - `partial` + - Actions performed by the agent: + - Send the latest workspace changes to Rails that are not yet persisted in Postgres. This persisted resource version will help with sending only the latest workspaces changes from the agent to Rails. + - Actions performed by Rails: + - Update Postgres with the current state and respond with the workspaces to be created/updated/deleted in the Kubernetes cluster and their last resource version that Rails has persisted in Postgres. + - The workspaces to be created/updated/deleted are calculated by using the filter `desired state updated at >= agent info reported at`. + - Returning the persisted resource version back to the agent gives it a confirmation that the updates for that workspace have been successfully processed on the Rails end. + +### Event-driven polling vs full or partial reconciliation + +It was initially considered desirable to be able to tell the agent to not wait for the next reconciliation loop but instead poll immediately. This would grant the following benefits: + +1. This would grant the ability to trigger a full reconciliation on demand that would allow on-demand recovery/resetting of module state in the agent. +1. Apart from making the architecture more event-driven and real-time it would also help to increase the interval between reconciliation polls, thus reducing the load on the infrastructure. + +However, as the prospective solutions were evaluated, it was concluded that there are very few/rare cases that would merit this capability, especially given the complexity of the viable options. An eventual reconciliation of state would suffice for most cases and it could be simply achieved through full reconciliation that is carried out periodically (with a longer interval as compared to partial reconciliation). + +You can read more in this [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/387090) and [conclusion comment](https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/-/merge_requests/13#note_1282495106) + +## Workspace states + +- `CreationRequested` - Initial state of a Workspace; Creation requested by user but hasn't yet been acted on +- `Starting` - In the process of being ready for use +- `Running` - Ready for use +- `Stopping` - In the process of scaling down +- `Stopped` - Persistent storage is still available but workspace has been scaled down +- `Failed` - Kubernetes resources have been applied by `agentk` but are not ready due to various reasons (for example, crashing container) +- `Error` - Kubernetes resources failed to get applied by `agentk` +- `RestartRequested` - User has requested a restart of the workspace but the restart has not yet successfully happened +- `Terminating` - User has requested the termination of the workspace and the action has been initiated but not yet completed. +- `Terminated` - Persistent storage has been deleted and the workspace has been scaled down +- `Unknown` - Not able to understand the actual state of the workspace + +### Possible `actual_state` values + +The `actual_state` values are determined from the `status` attribute in the Kubernetes deployment changes, which the agent listens to and sends to Rails. + +The following diagram represents the typical flow of the `actual_state` values for a `Workspace` record based on the +`status` values received from the agent. The `status` is parsed to derive the `actual_state` of the workspace based on different conditions. + +However, any of these states can be skipped if there have been any +transitional `status` updates that were not received from the agent for some reason (a quick transition, a +failure to send the event, etc). -Additionally, Remote Development workspaces could provide a way to implement disaster recovery if we are able to leverage the capabilities of [Pods](../../../architecture/blueprints/pods/index.md). +```plantuml +[*] --> CreationRequested +CreationRequested : Initial state before\nworkspace creation\nrequest is sent\nto kubernetes +CreationRequested -right-> Starting : status=Starting +CreationRequested -right-> Error : Could not create\nworkspace + +Starting : Workspace config is being\napplied to kubernetes +Starting -right-> Running : status=Running +Starting -down-> Failed : status=Failed\n(container crashing) + +Running : Workspace is running +Running -down-> Stopping : status=Stopping +Running -down-> Failed : status=Failed\n(container crashing) +Running -down-> Terminated : status=Terminated +Running -right-> Error : Could not\nstop/terminate\nworkspace + +Stopping : Workspace is stopping +Stopping -down-> Stopped : status=Stopped +Stopping -left-> Failed : status=Failed\n(could not\nunmount volume\nand stop workspace) + +Stopped : Workspace is Stopped\nby user request +Stopped -left-> Failed : status=Failed\n(could not\nunmount volume\nterminate workspace) +Stopped -right-> Error : Could not\nstart/terminate\nworkspace +Stopped -down-> Terminated : status=Terminated +Stopped -up-> Starting : status=Starting + +Terminated: Workspace has been deleted + +Failed: Workspace is not ready due to\nvarious reasons(e.g. crashing container) +Failed -up-> Starting : status=Starting\n(container\nnot crashing) +Failed -right-> Stopped : status=Stopped +Failed -down-> Terminated : status=Terminated +Failed -down-> Error : Could not\nstop/terminate\nworkspace + +Error: Kubernetes resources failed to get applied +Error -up-> Terminated : status=Terminated + +Unknown: Unable to understand the actual state of the workspace +``` -### Scalability +### Possible `desired_state` values -As an organization begins to scale, they quickly realize the need to support additional types of projects that might require extensive workflows. Remote Development workspaces aim to solve that issue by abstracting away the burden of complex machine configuration, dependency management, and possible data-seeding issues. +The `desired_state` values are determined from the user's request to Rails and are sent to the agent by Rails. -To facilitate working on different features across different projects, Remote Development should allow each user to provision multiple workspaces to enable quick context switching. +`desired_state` is a subset of the `actual_state` with only `Running`, `Stopped`, `Terminated` and `RestartRequested` values. +The state reconciliation logic in Rails will +continually attempt to transition the `actual_state` to the `desired_state` value, unless the workspace is in an unrecoverable state. -Eventually, we should be able to allow users to vertically scale their workspaces with more compute cores, memory, and other resources. If a user is currently working against a 2 CPU and 4 GB RAM workspace but comes to find they need more CPU, they should be able to upgrade their compute layer to something more suitable with a click or CLI command within the workspace. +There is also an additional supported state of `RestartRequested` which is only valid for `desired_state`. +This value is not a valid value for `actual_state`. It is required in order for Rails to +initiate a restart of a started workspace. It will only persist until a `status` of `Stopped` is received +from the agent, indicating that the restart request was successful and in progress or completed. +At this point, the `desired_state` will be automatically changed to `Running` to trigger the workspace to restart again. +If there is a failure to restart the workspace, and a `Stopped` status is never received, the +`desired_state` will remain `RestartRequested` until a new `desired_state` is specified. -### Provide built-in security and enterprise readiness +```plantuml +[*] --> Running +Running : Workspace is running +Running -down-> Stopped : status=Stopped +Running -left-> Terminated : status=Terminated -As Remote Development becomes a viable replacement for Virtual Desktop Infrastructure solutions, they must be secure and support enterprise requirements, such as role-based access control and the ability to remove all source code from developer machines. +Stopped : Workspace is Stopped\nby user request +Stopped -up-> Running : status=Running +Stopped -down-> Terminated : status=Terminated -### Accelerate project and developer onboarding +Terminated: Workspace has been deleted -As a zero-install development environment that runs in your browser, Remote Development makes it easy for anyone to join your team and contribute to a project. +RestartRequested : User has requested a workspace restart.\n**desired_state** will automatically change\nto **'Running'** if actual state\nof **'Stopped'** is received. +RestartRequested -left-> Running : status=Running +``` -### Regions +## Workspace user traffic authentication and authorization + +We need to only allow certain users to access workspaces. Currently, we are restricting this to the creator/owner of the workspace. After the workspace is created, it needs to be exposed to the network so that the user can connect to it. +Thus, any traffic incoming to the workspace needs to be authenticated and authorized. +[`gitlab-workspaces-proxy`](https://gitlab.com/gitlab-org/remote-development/gitlab-workspaces-proxy) handles discovery, authentication and authorization of the workspaces running in a Kubernetes cluster. + +It will proxy all HTTP and WebSocket calls to the correct workspace. It will perform the following tasks: + +1. Workspace discovery - The proxy will auto discover workspaces based on labels of Kubernetes service resources. The proxy will watch the Kubernetes API for the creation/updation/deletion of Kubernetes service resources. When an service resource is created, the proxy will automatically configure itself to use corresponding service as an upstream. Thus it will require a Kubernetes service account and a role that allows it to watch, list and get service resources. +1. Authentication - It will use the [OAuth 2.0 flow](../../../api/oauth2.md) with GitLab to authenticate the user. GitLab will act as the identity provider. If the customer uses a third party SSO service to log into GitLab, the flow would automatically delegate authentication to that provider. One of the complexities with authentication is the fact that each workspace is served on its own domain, and therefore we can't set the redirect URI on the GitLab app to a specific workspace. We need to set a state in the OAuth 2.0 flow to redirect to the correct workspace. +1. Authorization - The proxy will make a call to a GitLab GraphQL endpoint with the user's credentials obtained in the authentication phase. The endpoint will validate if the user has access to the workspace and accordingly return either a 404 or a 200. +1. Session Management - The proxy will be stateless and be deployed without additional third party software such as a cache or database, and therefore will be using a signed JWT to manage sessions. The JWT is signed using a key provided to the proxy during startup. + +All traffic incoming to the Kubernetes cluster on a given domain is forwarded to `gitlab-workspaces-proxy`, which then decides how to serve that traffic. + +```mermaid +flowchart TB + UserWorkspaceTraffic[User Workspace Traffic] --> Ingress + subgraph WorkspaceCluster[Workspace Cluster] + Ingress --> GitLabWorkspacesProxy[GitLab Workspaces Proxy] + GitLabWorkspacesProxy --Proxy--> Workspace1[Workspace 1] + GitLabWorkspacesProxy --Proxy--> Workspace2[Workspace 2] + GitLabWorkspacesProxy --Proxy--> Workspace3[Workspace 3] + end + GitLabWorkspacesProxy --OAuth 2--> GitLab + GitLab --Redirect--> GitLabWorkspacesProxy + GitLabWorkspacesProxy --Authz API--> GitLab +``` -GitLab.com is only hosted within the United States of America. Organizations located in other regions have voiced demand for local SaaS offerings. BYO infrastructure helps work in conjunction with [GitLab Regions](https://gitlab.com/groups/gitlab-org/-/epics/6037) because a user's workspace may be deployed within different geographies. The ability to deploy workspaces to different geographies might also help to solve data residency and compliance problems. +- Advantages + - Single instance of proxy, and therefore it is easy to manage and get metrics from. + - Easy to upgrade as a single instance exists - workspaces do not need to be restarted. +- Disadvantages + - Single point of failure + - It will have to scale with traffic + - New component (other than the GitLab Agent) that would have to be deployed in the Kubernetes cluster by the customer + - Does need Kubernetes privileges to list service resources. + +### Other options considered + +#### Sidecar proxy + +A sidecar will be injected into each workspace and all traffic to the workspace will flow through the sidecar. The sidecar will only handle the traffic for a single workspace. The sidecar can communicate with the workspace over the loopback interface (localhost) because the two share a network namespace. + +```mermaid +flowchart TB + UserWorkspaceTraffic[User Workspace Traffic] --> Ingress + subgraph WorkspaceCluster[Workspace Cluster] + Ingress --> Workspace1Proxy + Ingress --> Workspace2Proxy + Ingress --> Workspace3Proxy + subgraph workspace1[Workspace 1] + Workspace1Proxy[Workspace Sidecar Proxy] --Proxy--> Workspace1[Workspace 1] + end + subgraph workspace2[Workspace 2] + Workspace2Proxy[Workspace Sidecar Proxy] --Proxy--> Workspace2[Workspace 2] + end + subgraph workspace3[Workspace 3] + Workspace3Proxy[Workspace Sidecar Proxy] --Proxy--> Workspace3[Workspace 3] + end + end + + Workspace3Proxy --OAuth 2--> GitLab + GitLab --Redirect--> Workspace3Proxy + Workspace3Proxy --Authz API--> GitLab +``` -## Market analysis +- Advantages + - Does not need to handle a large volume of traffic + - If a sidecar stops working it does not hamper the working of other workspaces +- Disadvantages + - Inefficient usage of resources as a sidecar will have to be deployed for every workspace + - Workspace might take slightly longer to come up because of the additional proxy element + +#### Auth annotations on the Ingress resource + +Use auth annotations on the Ingress resource to allow Ingress controllers(for example, `ingress-nginx`) to delegate authentication and authorization to a separate process. The challenge is that these annotations are not standardized (that is, not part of the [Ingress specification](https://kubernetes.io/docs/concepts/services-networking/ingress/)) and may not be supported across different Ingress controllers. We would need to document the process to setup our Auth provider for each of the Ingress controllers. However, if they do become a part of the new [Gateway API](https://gateway-api.sigs.k8s.io/concepts/security-model/), we will reconsider this decision. + +For `ingress-nginx`, the auth annotations would be: + +```yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + nginx.ingress.kubernetes.io/auth-url: "https://$host/oauth2/auth" + nginx.ingress.kubernetes.io/auth-signin: "https://$host/oauth2/start?rd=$escaped_request_uri" + name: example-ingress + namespace: example-namespace +spec: + ingressClassName: nginx + rules: + - host: "*.workspaces.example.dev" + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: example-workspace + port: + number: 80 +``` -We have conducted a market analysis to understand the broader market and what others can offer us by way of open-source libraries, integrations, or partnership opportunities. We have broken down the effort into a set of issues where we investigate each potential competitor/pathway/partnership as a spike. +For `traefik`, the auth annotations would be: + +```yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: example-ingress + namespace: example-namespace + annotations: + kubernetes.io/ingress.class: traefik + ingress.kubernetes.io/auth-type: forward + ingress.kubernetes.io/auth-url: http://traefik-forward-auth:4181 + ingress.kubernetes.io/auth-response-headers: X-Forwarded-User +spec: + ingressClassName: traefik + rules: + - host: "*.workspaces.example.dev" + http: + paths: + - backend: + name: example-workspace + servicePort: http +``` -- [Market analysis](https://gitlab.com/groups/gitlab-org/-/epics/8131) -- [YouTube results](https://www.youtube.com/playlist?list=PL05JrBw4t0KrRQhnSYRNh1s1mEUypx67-) +For `Google Cloud Load Balancer using IAP`, the auth annotations would be: + +```yaml +apiVersion: cloud.google.com/v1 +kind: BackendConfig +metadata: + name: example-backend-config + namespace: example-namespace +spec: + iap: + enabled: true + oauthclientCredentials: + secretName: example-secret +``` -### Implementation +## Accessing the Web IDE from the workspace -- [Viable Maturity Epic](https://gitlab.com/groups/gitlab-org/-/epics/9190) to track progress. -- [Documentation](https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs) -explaining the architecture and implementation details. +Currently, we only support GitLab fork of VS Code as the editor that is injected inside a workspace during runtime. +The [editor injector](https://gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/-/tree/main/scripts/gl/editor-injector) is a container image that contains GitLab fork of VS Code server. +The editor injector contains scripts for copying this server into a workspace and starting the server. -## Che vs. DevWorkspace Operatoor vs. Custom-Built Solution +The editor injector packages both the WebUI and the Extension Host (VS Code backend). [Currently](https://gitlab.com/gitlab-org/gitlab/-/issues/393006), we also package the WebUI in the workspace. That means that the GitLab fork of VS Code editor can be used two ways: -After an investigation into using [Che](https://gitlab.com/gitlab-org/gitlab/-/issues/366052) as our backend to accelerate Remote Development, we ultimately opted to [write our own custom-built solution using DevWorkspace Operator](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97449#note_1131215629). +- Access the Workspace URL **directly** and use the bundled WebUI +- Access the Workspace through **WebIDE** (ignore the bundled WebUI) -Some advantages of us opting to write our own custom-built solution are: +```plantuml +@startuml -- We can still use the core DevWorkspace Operator and build on top of it. -- It is easier to add support for other configurations apart from `devfile` in the future if the need arises. -- We have the ability to choose which tech stack to use (for example, instead of using Traefik which is used in Che, explore NGINX itself or use GitLab Agent for Kubernetes). +title + Accessing the editor inside a workspace +end title -After writing our own custom-built solution using DevWorkspace Operator, -we decided to [remove the dependency on DevWorkspace Operator](https://gitlab.com/groups/gitlab-org/-/epics/9895) -and thus the transitive dependency of Cert Manager. +node "Workspace" { + node "GitLab fork of VS Code" { + [HTTP Server] .[#blue]up.> [Static assets of the WebUI] + [HTTP Server] -[#blue]-> [Extension Server (VS Code backend)] + [HTTP Server] -[#green]-> [Extension Server (VS Code backend)] + } +} + +[Workspace URL in browser] -[#blue]right-> [HTTP Server] +[Workspace URL in browser] .[#blue]right.> [HTTP Server] +[WebIDE URL in browser] -[#green]left-> [HTTP Server] + +note right of "Static assets of the WebUI" + We build WebIDE on top of these assets +end note + +note bottom of "Workspace URL in browser" + Accessing the workspace directly + uses the packaged WebUI +end note + +note bottom of "WebIDE URL in browser" + Accessing the workspace through WebIDE + ignores the web assets in the workspace + and only uses the WebSocket connection +end note + +@enduml +``` + +## Building container images for workspaces + +We rely on file group permissions to be able to modify and run any file in a container. +Thus, while creating the workspace, we use an arbitrary Linux user to run the container. +If the container image you want to use does not support arbitrary user IDs, you can build you own by using the snippet below. This snippet is provided only for reference. If there are other locations in the container that should have write access to the Linux user running the container, make sure those files and folders have the desired root Linux group permissions that we rely on. + +```dockerfile +FROM IMAGE_OF_YOUR_CHOICE + +RUN useradd -l -u 33333 -G sudo -md /home/gitlab-workspaces -s /bin/bash -p gitlab-workspaces gitlab-workspaces + +ENV HOME=/home/gitlab-workspaces + +WORKDIR $HOME + +RUN mkdir -p /home/gitlab-workspaces && chgrp -R 0 /home && chmod -R g=u /etc/passwd /etc/group /home + +USER gitlab-workspaces +``` + +You can read more about this decision in this [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/396300#note_1375061754). ## Links +- [Remote Development direction](https://about.gitlab.com/direction/create/editor/remote_development) - [Remote Development presentation](https://docs.google.com/presentation/d/1XHH_ZilZPufQoWVWViv3evipI-BnAvRQrdvzlhBuumw/edit#slide=id.g131f2bb72e4_0_8) - [Category Strategy epic](https://gitlab.com/groups/gitlab-org/-/epics/7419) - [Minimal Maturity epic](https://gitlab.com/groups/gitlab-org/-/epics/9189) - [Viable Maturity epic](https://gitlab.com/groups/gitlab-org/-/epics/9190) - [Complete Maturity epic](https://gitlab.com/groups/gitlab-org/-/epics/9191) -- [Remote Development sync](https://docs.google.com/document/d/1hWVvksIc7VzZjG-0iSlzBnLpyr-OjwBVCYMxsBB3h_E/edit#) +- [Remote Development Engineering Sync](https://docs.google.com/document/d/1hWVvksIc7VzZjG-0iSlzBnLpyr-OjwBVCYMxsBB3h_E/edit#) - [Market analysis and architecture](https://gitlab.com/groups/gitlab-org/-/epics/8131) -- [GA4K Architecture](https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/-/blob/main/doc/architecture.md) +- [Developer Documentation](https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/) - [BYO infrastructure](https://gitlab.com/groups/gitlab-org/-/epics/8290) - [Browser runtime](https://gitlab.com/groups/gitlab-org/-/epics/8291) - [GitLab-hosted infrastructure](https://gitlab.com/groups/gitlab-org/-/epics/8292) -- [Browser runtime spike](https://gitlab.com/gitlab-org/gitlab-web-ide/-/merge_requests/58). -- [Remote Development direction](https://about.gitlab.com/direction/create/editor/remote_development) +- [Browser runtime spike](https://gitlab.com/gitlab-org/gitlab-web-ide/-/merge_requests/58) - [Ideal user journey](https://about.gitlab.com/direction/create/editor/remote_development/#ideal-user-journey) +- [Building container images for workspaces](https://gitlab.com/gitlab-org/gitlab/-/issues/396300#note_1375061754) diff --git a/doc/user/packages/dependency_proxy/index.md b/doc/user/packages/dependency_proxy/index.md index d7cf33cc2a4..62c63c0e42a 100644 --- a/doc/user/packages/dependency_proxy/index.md +++ b/doc/user/packages/dependency_proxy/index.md @@ -35,6 +35,8 @@ For a list of planned additions, view the ## Enable or turn off the Dependency Proxy for a group +> Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) from Developer to Maintainer in GitLab 15.0. + To enable or turn off the Dependency Proxy for a group: 1. On the top bar, select **Main menu > Groups** and find your group. diff --git a/doc/user/packages/dependency_proxy/reduce_dependency_proxy_storage.md b/doc/user/packages/dependency_proxy/reduce_dependency_proxy_storage.md index 3aaaf0c0186..4c625499d07 100644 --- a/doc/user/packages/dependency_proxy/reduce_dependency_proxy_storage.md +++ b/doc/user/packages/dependency_proxy/reduce_dependency_proxy_storage.md @@ -26,7 +26,7 @@ image or tag from Docker Hub. ## Cleanup policies -> [Required permissions](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) changed from developer to maintainer in GitLab 15.0. +> Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) from Developer to Maintainer in GitLab 15.0. ### Enable cleanup policies from within GitLab diff --git a/doc/user/packages/generic_packages/index.md b/doc/user/packages/generic_packages/index.md index bad099e6733..e6ee4caa5d4 100644 --- a/doc/user/packages/generic_packages/index.md +++ b/doc/user/packages/generic_packages/index.md @@ -118,7 +118,7 @@ API or the UI. #### Do not allow duplicate Generic packages > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/293755) in GitLab 13.12. -> - [Required permissions](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) changed from developer to maintainer in GitLab 15.0. +> - Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) from Developer to Maintainer in GitLab 15.0. To prevent users from publishing duplicate generic packages, you can use the [GraphQL API](../../../api/graphql/reference/index.md#packagesettings) or the UI. diff --git a/doc/user/packages/maven_repository/index.md b/doc/user/packages/maven_repository/index.md index fd8049d9b75..bcc55af65bc 100644 --- a/doc/user/packages/maven_repository/index.md +++ b/doc/user/packages/maven_repository/index.md @@ -474,6 +474,8 @@ To delete older package versions, consider using the Packages API or the UI. ### Do not allow duplicate Maven packages +> Required role [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/350682) from Developer to Maintainer in GitLab 15.0. + To prevent users from publishing duplicate Maven packages, you can use the [GraphQl API](../../../api/graphql/reference/index.md#packagesettings) or the UI. In the UI: diff --git a/lib/api/users.rb b/lib/api/users.rb index dc77dc5c157..40453ed2384 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -99,6 +99,7 @@ module API optional :exclude_internal, as: :non_internal, type: Boolean, default: false, desc: 'Filters only non internal users' optional :without_project_bots, type: Boolean, default: false, desc: 'Filters users without project bots' optional :admins, type: Boolean, default: false, desc: 'Filters only admin users' + optional :two_factor, type: String, desc: 'Filter users by Two-factor authentication.' all_or_none_of :extern_uid, :provider use :sort_params @@ -108,10 +109,12 @@ module API end # rubocop: disable CodeReuse/ActiveRecord get feature_category: :user_profile, urgency: :low do - authenticated_as_admin! if params[:extern_uid].present? && params[:provider].present? + index_params = declared_params(include_missing: false) + + authenticated_as_admin! if index_params[:extern_uid].present? && index_params[:provider].present? unless current_user&.can_read_all_resources? - params.except!(:created_after, :created_before, :order_by, :sort, :two_factor, :without_projects) + index_params.except!(:created_after, :created_before, :order_by, :sort, :two_factor, :without_projects) end authorized = can?(current_user, :read_users_list) @@ -121,11 +124,11 @@ module API # a list of all the users on the GitLab instance. `UsersFinder` performs # an exact match on the `username` parameter, so we are guaranteed to # get either 0 or 1 `users` here. - authorized &&= params[:username].present? if current_user.blank? + authorized &&= index_params[:username].present? if current_user.blank? forbidden!("Not authorized to access /api/v4/users") unless authorized - users = UsersFinder.new(current_user, params).execute + users = UsersFinder.new(current_user, index_params).execute users = reorder_users(users) entity = current_user&.can_read_all_resources? ? Entities::UserWithAdmin : Entities::UserBasic diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 4197c87f51f..da9ebf4ab0f 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -2,8 +2,6 @@ module Gitlab module Database - DATABASE_NAMES = %w[main ci main_clusterwide].freeze - MAIN_DATABASE_NAME = 'main' CI_DATABASE_NAME = 'ci' DEFAULT_POOL_HEADROOM = 10 @@ -56,67 +54,78 @@ module Gitlab MODE_SINGLE_DATABASE_CI_CONNECTION = "single-database-ci-connection" MODE_MULTIPLE_DATABASES = "multiple-databases" + def self.all_database_connection_files + Dir.glob(Rails.root.join("db/database_connections/*.yaml")) + end + + def self.all_gitlab_schema_files + Dir.glob(Rails.root.join("db/gitlab_schemas/*.yaml")) + end + + def self.all_database_connections + @all_database_connections ||= + all_database_connection_files + .map { |file| DatabaseConnectionInfo.load_file(file) } + .sort_by(&:order) + .index_by(&:name) + .with_indifferent_access.freeze + end + + def self.all_database_names + all_database_connections.keys.map(&:to_s) + end + + def self.all_gitlab_schemas + @all_gitlab_schemas ||= + all_gitlab_schema_files + .map { |file| GitlabSchemaInfo.load_file(file) } + .index_by(&:name) + .with_indifferent_access.freeze + end + def self.database_base_models - @database_base_models ||= { - # Note that we use ActiveRecord::Base here and not ApplicationRecord. - # This is deliberate, as we also use these classes to apply load - # balancing to, and the load balancer must be enabled for _all_ models - # that inherit from ActiveRecord::Base; not just our own models that - # inherit from ApplicationRecord. - main: ::ActiveRecord::Base, - main_clusterwide: ::MainClusterwide::ApplicationRecord.connection_class? ? ::MainClusterwide::ApplicationRecord : nil, - ci: ::Ci::ApplicationRecord.connection_class? ? ::Ci::ApplicationRecord : nil - }.compact.with_indifferent_access.freeze + # Note that we use ActiveRecord::Base here and not ApplicationRecord. + # This is deliberate, as we also use these classes to apply load + # balancing to, and the load balancer must be enabled for _all_ models + # that inherit from ActiveRecord::Base; not just our own models that + # inherit from ApplicationRecord. + @database_base_models ||= + all_database_connections + .transform_values(&:connection_class) + .compact.with_indifferent_access.freeze end # This returns a list of databases that contains all the gitlab_shared schema - # tables. We can't reuse database_base_models because Geo does not support - # the gitlab_shared tables yet. + # tables. def self.database_base_models_with_gitlab_shared - @database_base_models_with_gitlab_shared ||= { - # Note that we use ActiveRecord::Base here and not ApplicationRecord. - # This is deliberate, as we also use these classes to apply load - # balancing to, and the load balancer must be enabled for _all_ models - # that inher from ActiveRecord::Base; not just our own models that - # inherit from ApplicationRecord. - main: ::ActiveRecord::Base, - main_clusterwide: ::MainClusterwide::ApplicationRecord.connection_class? ? ::MainClusterwide::ApplicationRecord : nil, - ci: ::Ci::ApplicationRecord.connection_class? ? ::Ci::ApplicationRecord : nil - }.compact.with_indifferent_access.freeze + @database_base_models_with_gitlab_shared ||= + all_database_connections + .select { |_, db| db.has_gitlab_shared? } + .transform_values(&:connection_class) + .compact.with_indifferent_access.freeze end # This returns a list of databases whose connection supports database load - # balancing. We can't reuse the database_base_models method because the Geo - # database does not support load balancing yet. - # - # TODO: https://gitlab.com/gitlab-org/geo-team/discussions/-/issues/5032 + # balancing. We can't reuse the database_base_models since not all connections + # do support load balancing. def self.database_base_models_using_load_balancing - @database_base_models_using_load_balancing ||= { - # Note that we use ActiveRecord::Base here and not ApplicationRecord. - # This is deliberate, as we also use these classes to apply load - # balancing to, and the load balancer must be enabled for _all_ models - # that inher from ActiveRecord::Base; not just our own models that - # inherit from ApplicationRecord. - main: ::ActiveRecord::Base, - main_clusterwide: ::MainClusterwide::ApplicationRecord.connection_class? ? ::MainClusterwide::ApplicationRecord : nil, - ci: ::Ci::ApplicationRecord.connection_class? ? ::Ci::ApplicationRecord : nil - }.compact.with_indifferent_access.freeze + @database_base_models_using_load_balancing ||= + all_database_connections + .select { |_, db| db.uses_load_balancing? } + .transform_values(&:connection_class) + .compact.with_indifferent_access.freeze end # This returns a list of base models with connection associated for a given gitlab_schema def self.schemas_to_base_models - @schemas_to_base_models ||= { - gitlab_main: [self.database_base_models.fetch(:main)], - gitlab_ci: [self.database_base_models[:ci] || self.database_base_models.fetch(:main)], # use CI or fallback to main - gitlab_shared: database_base_models_with_gitlab_shared.values, # all models - gitlab_internal: database_base_models.values, # all models - gitlab_pm: [self.database_base_models.fetch(:main)], # package metadata models - gitlab_main_clusterwide: [self.database_base_models[:main_clusterwide] || self.database_base_models.fetch(:main)] - }.with_indifferent_access.freeze - end - - def self.all_database_names - DATABASE_NAMES + @schemas_to_base_models ||= + all_gitlab_schemas.transform_values do |schema| + all_database_connections + .values + .select { |db| db.gitlab_schemas.include?(schema.name) } + .filter_map { |db| db.connection_class_or_fallback(all_database_connections) } + .uniq + end.compact.with_indifferent_access.freeze end # We configure the database connection pool size automatically based on the @@ -255,8 +264,16 @@ module Gitlab end end - def self.db_config_names - ::ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).map(&:name) - ['geo'] + def self.db_config_names(with_schema:) + db_config_names = ::ActiveRecord::Base.configurations + .configs_for(env_name: Rails.env).map(&:name) + return db_config_names unless with_schema + + schema_models = schemas_to_base_models.fetch(with_schema) + db_config_names.select do |db_config_name| + db_info = all_database_connections.fetch(db_config_name) + schema_models.include?(db_info.connection_class) + end end # This returns all matching schemas that a given connection can use diff --git a/lib/gitlab/database/database_connection_info.rb b/lib/gitlab/database/database_connection_info.rb new file mode 100644 index 00000000000..57ecbcd64ae --- /dev/null +++ b/lib/gitlab/database/database_connection_info.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module Gitlab + module Database + DatabaseConnectionInfo = Struct.new( + :name, + :description, + :gitlab_schemas, + :klass, + :fallback_database, + :db_dir, + :uses_load_balancing, + :file_path, + keyword_init: true + ) do + include Gitlab::Utils::StrongMemoize + + def initialize(*) + super + self.name = name.to_sym + self.gitlab_schemas = gitlab_schemas.map(&:to_sym) + self.klass = klass.constantize + self.fallback_database = fallback_database&.to_sym + self.db_dir = Rails.root.join(db_dir || 'db') + end + + def self.load_file(yaml_file) + content = YAML.load_file(yaml_file) + new(**content.deep_symbolize_keys.merge(file_path: yaml_file)) + end + + def active_record_base? + klass == ActiveRecord::Base + end + private :active_record_base? + + strong_memoize_attr def connection_class + klass.connection_class || active_record_base? ? klass : nil + end + + strong_memoize_attr def order + # Retain order of configurations as they are defined in `config/database.yml` + ActiveRecord::Base.configurations + .configs_for(env_name: Rails.env) + .map(&:name) + .index(name.to_s) || 1_000 + end + + def connection_class_or_fallback(all_databases) + if connection_class + connection_class + elsif fallback_database + all_databases.fetch(fallback_database) + .connection_class_or_fallback(all_databases) + end + end + + def has_gitlab_shared? + gitlab_schemas.include?(:gitlab_shared) + end + + def uses_load_balancing? + !!uses_load_balancing + end + + def db_docs_dir + db_dir.join('docs') + end + end + end +end diff --git a/lib/gitlab/database/gitlab_schema.rb b/lib/gitlab/database/gitlab_schema.rb index 3d2e9f3f243..bae94267f14 100644 --- a/lib/gitlab/database/gitlab_schema.rb +++ b/lib/gitlab/database/gitlab_schema.rb @@ -3,13 +3,15 @@ # This module gathers information about table to schema mapping # to understand table affinity # -# Each table / view needs to have assigned gitlab_schema. Names supported today: +# Each table / view needs to have assigned gitlab_schema. For example: # # - gitlab_shared - defines a set of tables that are found on all databases (data accessed is dependent on connection) # - gitlab_main / gitlab_ci - defines a set of tables that can only exist on a given application database # - gitlab_geo - defines a set of tables that can only exist on the geo database # - gitlab_internal - defines all internal tables of Rails and PostgreSQL # +# All supported GitLab schemas can be viewed in `db/gitlab_schemas/` and `ee/db/gitlab_schemas/` +# # Tables for the purpose of tests should be prefixed with `_test_my_table_name` module Gitlab @@ -17,8 +19,6 @@ module Gitlab module GitlabSchema UnknownSchemaError = Class.new(StandardError) - DICTIONARY_PATH = 'db/docs/' - def self.table_schemas!(tables) tables.map { |table| table_schema!(table) }.to_set end @@ -70,26 +70,6 @@ module Gitlab end # rubocop:enable Metrics/CyclomaticComplexity - def self.dictionary_path_globs - [Rails.root.join(DICTIONARY_PATH, '*.yml')] - end - - def self.view_path_globs - [Rails.root.join(DICTIONARY_PATH, 'views', '*.yml')] - end - - def self.deleted_views_path_globs - [Rails.root.join(DICTIONARY_PATH, 'deleted_views', '*.yml')] - end - - def self.deleted_tables_path_globs - [Rails.root.join(DICTIONARY_PATH, 'deleted_tables', '*.yml')] - end - - def self.views_and_tables_to_schema - @views_and_tables_to_schema ||= self.tables_to_schema.merge(self.views_to_schema) - end - def self.table_schema!(name) # rubocop:disable Gitlab/DocUrl self.table_schema(name) || raise( @@ -100,32 +80,45 @@ module Gitlab # rubocop:enable Gitlab/DocUrl end + def self.dictionary_paths + Gitlab::Database.all_database_connections + .values.map(&:db_docs_dir).uniq + end + + def self.dictionary_path_globs(scope) + self.dictionary_paths.map { |path| Rails.root.join(path, scope, '*.yml') } + end + + def self.views_and_tables_to_schema + @views_and_tables_to_schema ||= self.tables_to_schema.merge(self.views_to_schema) + end + def self.deleted_views_and_tables_to_schema @deleted_views_and_tables_to_schema ||= self.deleted_tables_to_schema.merge(self.deleted_views_to_schema) end def self.deleted_tables_to_schema - @deleted_tables_to_schema ||= self.build_dictionary(self.deleted_tables_path_globs) + @deleted_tables_to_schema ||= self.build_dictionary('deleted_tables').to_h end def self.deleted_views_to_schema - @deleted_views_to_schema ||= self.build_dictionary(self.deleted_views_path_globs) + @deleted_views_to_schema ||= self.build_dictionary('deleted_views').to_h end def self.tables_to_schema - @tables_to_schema ||= self.build_dictionary(self.dictionary_path_globs) + @tables_to_schema ||= self.build_dictionary('').to_h end def self.views_to_schema - @views_to_schema ||= self.build_dictionary(self.view_path_globs) + @views_to_schema ||= self.build_dictionary('views').to_h end def self.schema_names @schema_names ||= self.views_and_tables_to_schema.values.to_set end - private_class_method def self.build_dictionary(path_globs) - Dir.glob(path_globs).each_with_object({}) do |file_path, dic| + def self.build_dictionary(scope) + Dir.glob(dictionary_path_globs(scope)).map do |file_path| data = YAML.load_file(file_path) key_name = data['table_name'] || data['view_name'] @@ -140,7 +133,7 @@ module Gitlab end # rubocop:enable Gitlab/DocUrl - dic[key_name] = data['gitlab_schema'].to_sym + [key_name, data['gitlab_schema'].to_sym] end end end diff --git a/lib/gitlab/database/gitlab_schema_info.rb b/lib/gitlab/database/gitlab_schema_info.rb new file mode 100644 index 00000000000..7593d252975 --- /dev/null +++ b/lib/gitlab/database/gitlab_schema_info.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Gitlab + module Database + GitlabSchemaInfo = Struct.new( + :name, + :description, + :file_path, + keyword_init: true + ) do + def initialize(*) + super + self.name = name.to_sym + end + + def self.load_file(yaml_file) + content = YAML.load_file(yaml_file) + new(**content.deep_symbolize_keys.merge(file_path: yaml_file)) + end + end + end +end diff --git a/lib/gitlab/github_import/representation/diff_note.rb b/lib/gitlab/github_import/representation/diff_note.rb index 0408b34bb02..191e15962a6 100644 --- a/lib/gitlab/github_import/representation/diff_note.rb +++ b/lib/gitlab/github_import/representation/diff_note.rb @@ -12,7 +12,7 @@ module Gitlab expose_attribute :noteable_id, :commit_id, :file_path, :diff_hunk, :author, :created_at, :updated_at, :original_commit_id, :note_id, :end_line, :start_line, - :side, :in_reply_to_id, :discussion_id + :side, :in_reply_to_id, :discussion_id, :subject_type # Builds a diff note from a GitHub API response. # @@ -43,7 +43,8 @@ module Gitlab start_line: note[:start_line], side: note[:side], in_reply_to_id: note[:in_reply_to_id], - discussion_id: DiffNotes::DiscussionId.new(note).find_or_generate + discussion_id: DiffNotes::DiscussionId.new(note).find_or_generate, + subject_type: note[:subject_type] } new(hash) @@ -84,8 +85,14 @@ module Gitlab end def line_code - diff_line = Gitlab::Diff::Parser.new.parse(diff_hunk.lines).to_a.last + # on the GitHub side it is possible to leave a comment on a file + # or on a line. When the comment is left on a file there is no + # diff hunk, but LegacyDiffNote requires line_code to be always present + # and DiffFile requires it for text files + # so it is set as the first line for any type of file (image, binary, text) + return Gitlab::Git.diff_line_code(file_path, 1, 1) if on_file? + diff_line = Gitlab::Diff::Parser.new.parse(diff_hunk.lines).to_a.last Gitlab::Git.diff_line_code(file_path, diff_line.new_pos, diff_line.old_pos) end @@ -141,6 +148,10 @@ module Gitlab def addition? side == 'RIGHT' end + + def on_file? + subject_type == 'file' + end end end end diff --git a/lib/gitlab/hotlinking_detector.rb b/lib/gitlab/hotlinking_detector.rb index b5000777010..e81983cd014 100644 --- a/lib/gitlab/hotlinking_detector.rb +++ b/lib/gitlab/hotlinking_detector.rb @@ -25,6 +25,11 @@ module Gitlab return true if INVALID_FORMATS.include?(request_accepts.first) false + + rescue ActionDispatch::Http::MimeNegotiation::InvalidType, Mime::Type::InvalidMimeType + # Malformed requests with invalid MIME types prevent the checks from + # being executed correctly, so we should intercept those requests. + true end private diff --git a/lib/gitlab/metrics/loose_foreign_keys_slis.rb b/lib/gitlab/metrics/loose_foreign_keys_slis.rb index 5d8245aa609..c0db709fe13 100644 --- a/lib/gitlab/metrics/loose_foreign_keys_slis.rb +++ b/lib/gitlab/metrics/loose_foreign_keys_slis.rb @@ -26,7 +26,7 @@ module Gitlab private def possible_labels - ::Gitlab::Database.db_config_names.map do |db_config_name| + ::Gitlab::Database.db_config_names(with_schema: :gitlab_shared).map do |db_config_name| { db_config_name: db_config_name, feature_category: :database diff --git a/lib/gitlab/metrics/subscribers/rails_cache.rb b/lib/gitlab/metrics/subscribers/rails_cache.rb index b4e9e85a012..e2cdd6c5358 100644 --- a/lib/gitlab/metrics/subscribers/rails_cache.rb +++ b/lib/gitlab/metrics/subscribers/rails_cache.rb @@ -13,7 +13,7 @@ module Gitlab return unless current_transaction - labels = { store: event.payload[:store].split('::').last } + labels = { store: extract_store_name(event) } current_transaction.observe(:gitlab_cache_read_multikey_count, event.payload[:key].size, labels) do buckets [10, 50, 100, 1000] docstring 'Number of keys for mget in read_multi/fetch_multi' @@ -26,11 +26,7 @@ module Gitlab return unless current_transaction return if event.payload[:super_operation] == :fetch - unless event.payload[:hit] - current_transaction.increment(:gitlab_cache_misses_total, 1) do - docstring 'Cache read miss' - end - end + track_cache_miss(event) unless event.payload[:hit] end def cache_write(event) @@ -48,23 +44,23 @@ module Gitlab def cache_fetch_hit(event) return unless current_transaction - current_transaction.increment(:gitlab_transaction_cache_read_hit_count_total, 1) + labels = { store: extract_store_name(event) } + current_transaction.increment(:gitlab_transaction_cache_read_hit_count_total, 1, labels) end def cache_generate(event) return unless current_transaction - current_transaction.increment(:gitlab_cache_misses_total, 1) do - docstring 'Cache read miss' - end + track_cache_miss(event) - current_transaction.increment(:gitlab_transaction_cache_read_miss_count_total, 1) + labels = { store: extract_store_name(event) } + current_transaction.increment(:gitlab_transaction_cache_read_miss_count_total, 1, labels) end def observe(key, event) return unless current_transaction - labels = { operation: key, store: event.payload[:store].split('::').last } + labels = { operation: key, store: extract_store_name(event) } current_transaction.increment(:gitlab_cache_operations_total, 1, labels) do docstring 'Cache operations' @@ -76,6 +72,20 @@ module Gitlab private + def track_cache_miss(event) + # avoid passing in labels to ensure metric has consistent set of labels + labels = { store: extract_store_name(event) } + + current_transaction.increment(:gitlab_cache_misses_total, 1, labels) do + docstring 'Cache read miss' + end + end + + def extract_store_name(event) + # see payload documentation in https://guides.rubyonrails.org/active_support_instrumentation.html#active-support + event.payload[:store].to_s.split('::').last + end + def current_transaction ::Gitlab::Metrics::WebTransaction.current end diff --git a/lib/tasks/gitlab/background_migrations.rake b/lib/tasks/gitlab/background_migrations.rake index eca51c345d1..a4e14af22bd 100644 --- a/lib/tasks/gitlab/background_migrations.rake +++ b/lib/tasks/gitlab/background_migrations.rake @@ -6,7 +6,7 @@ namespace :gitlab do namespace :background_migrations do desc 'Synchronously finish executing a batched background migration' task :finalize, [:job_class_name, :table_name, :column_name, :job_arguments] => :environment do |_, args| - if Gitlab::Database.db_config_names.size > 1 + if Gitlab::Database.db_config_names(with_schema: :gitlab_shared).size > 1 puts "Please specify the database".color(:red) exit 1 end diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 79121d432e5..d51926e0e08 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -483,18 +483,12 @@ namespace :gitlab do end namespace :dictionary do - DB_DOCS_PATH = Rails.root.join('db', 'docs') - desc 'Generate database docs yaml' task generate: :environment do next if Gitlab.jh? - FileUtils.mkdir_p(DB_DOCS_PATH) - - if Gitlab.ee? - Gitlab::Database::EE_DATABASES_NAME_TO_DIR.each do |_, ee_db_dir| - FileUtils.mkdir_p(Rails.root.join(ee_db_dir, 'docs')) - end + Gitlab::Database.all_database_connections.values.map(&:db_docs_dir).each do |db_dir| + FileUtils.mkdir_p(db_dir) end Rails.application.eager_load! @@ -575,12 +569,7 @@ namespace :gitlab do def dictionary_file_path(source_name, views, database) sub_directory = views.include?(source_name) ? 'views' : '' - path = if Gitlab.ee? && Gitlab::Database::EE_DATABASES_NAME_TO_DIR.key?(database.to_s) - Rails.root.join(Gitlab::Database::EE_DATABASES_NAME_TO_DIR[database.to_s], 'docs') - else - DB_DOCS_PATH - end - + path = Gitlab::Database.all_database_connections.fetch(database).db_docs_dir File.join(path, sub_directory, "#{source_name}.yml") end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9c38dba5784..58d24b3ef68 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48183,9 +48183,6 @@ msgstr "" msgid "Unable to suggest a path. Please refresh and try again." msgstr "" -msgid "Unable to summarize your review. No draft comments found." -msgstr "" - msgid "Unable to update label prioritization at this time" msgstr "" diff --git a/spec/features/admin/admin_sees_background_migrations_spec.rb b/spec/features/admin/admin_sees_background_migrations_spec.rb index 77266e65e4c..7d4d3deb6d8 100644 --- a/spec/features/admin/admin_sees_background_migrations_spec.rb +++ b/spec/features/admin/admin_sees_background_migrations_spec.rb @@ -200,7 +200,7 @@ RSpec.describe "Admin > Admin sees background migrations", feature_category: :da before do skip_if_multiple_databases_are_setup - allow(Gitlab::Database).to receive(:db_config_names).and_return(['main']) + allow(Gitlab::Database).to receive(:db_config_names).with(with_schema: :gitlab_shared).and_return(['main']) end it 'does not render the database listbox' do @@ -214,7 +214,7 @@ RSpec.describe "Admin > Admin sees background migrations", feature_category: :da before do skip_if_multiple_databases_not_setup(:ci) - allow(Gitlab::Database).to receive(:db_config_names).and_return(%w[main ci]) + allow(Gitlab::Database).to receive(:db_config_names).with(with_schema: :gitlab_shared).and_return(%w[main ci]) end it 'renders the database listbox' do diff --git a/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb b/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb index ae368e4d37e..084876ba90c 100644 --- a/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb +++ b/spec/graphql/mutations/dependency_proxy/group_settings/update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mutations::DependencyProxy::GroupSettings::Update do +RSpec.describe Mutations::DependencyProxy::GroupSettings::Update, feature_category: :dependency_proxy do using RSpec::Parameterized::TableSyntax let_it_be_with_reload(:group) { create(:group) } @@ -36,7 +36,8 @@ RSpec.describe Mutations::DependencyProxy::GroupSettings::Update do end where(:user_role, :shared_examples_name) do - :maintainer | 'updating the dependency proxy group settings' + :owner | 'updating the dependency proxy group settings' + :maintainer | 'denying access to dependency proxy group settings' :developer | 'denying access to dependency proxy group settings' :reporter | 'denying access to dependency proxy group settings' :guest | 'denying access to dependency proxy group settings' @@ -50,6 +51,14 @@ RSpec.describe Mutations::DependencyProxy::GroupSettings::Update do end it_behaves_like params[:shared_examples_name] + + context 'with disabled admin_package feature flag' do + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it_behaves_like 'updating the dependency proxy group settings' if params[:user_role] == :maintainer + end end end end diff --git a/spec/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb b/spec/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb index 1e5059d7ef7..9a6215417ef 100644 --- a/spec/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb +++ b/spec/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update do +RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update, feature_category: :dependency_proxy do using RSpec::Parameterized::TableSyntax let_it_be_with_reload(:group) { create(:group) } @@ -58,6 +58,15 @@ RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update do end end + # To be removed when raise_group_admin_package_permission_to_owner FF is removed + shared_examples 'disabling admin_package feature flag' do |action:| + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it_behaves_like "#{action} the dependency proxy image ttl policy" + end + before do stub_config(dependency_proxy: { enabled: true }) end @@ -71,7 +80,8 @@ RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update do end where(:user_role, :shared_examples_name) do - :maintainer | 'updating the dependency proxy image ttl policy' + :owner | 'updating the dependency proxy image ttl policy' + :maintainer | 'denying access to dependency proxy image ttl policy' :developer | 'denying access to dependency proxy image ttl policy' :reporter | 'denying access to dependency proxy image ttl policy' :guest | 'denying access to dependency proxy image ttl policy' @@ -84,6 +94,7 @@ RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update do end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :updating if params[:user_role] == :maintainer end end @@ -91,7 +102,8 @@ RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update do let_it_be(:ttl_policy) { group.dependency_proxy_image_ttl_policy } where(:user_role, :shared_examples_name) do - :maintainer | 'creating the dependency proxy image ttl policy' + :owner | 'creating the dependency proxy image ttl policy' + :maintainer | 'denying access to dependency proxy image ttl policy' :developer | 'denying access to dependency proxy image ttl policy' :reporter | 'denying access to dependency proxy image ttl policy' :guest | 'denying access to dependency proxy image ttl policy' @@ -104,6 +116,7 @@ RSpec.describe Mutations::DependencyProxy::ImageTtlGroupPolicy::Update do end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :creating if params[:user_role] == :maintainer end end end diff --git a/spec/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/graphql/mutations/namespace/package_settings/update_spec.rb index 09ac1c99b10..576f514183f 100644 --- a/spec/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/graphql/mutations/namespace/package_settings/update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Mutations::Namespace::PackageSettings::Update do +RSpec.describe Mutations::Namespace::PackageSettings::Update, feature_category: :package_registry do using RSpec::Parameterized::TableSyntax let_it_be_with_reload(:namespace) { create(:group) } @@ -77,6 +77,15 @@ RSpec.describe Mutations::Namespace::PackageSettings::Update do end end + # To be removed when raise_group_admin_package_permission_to_owner FF is removed + RSpec.shared_examples 'disabling admin_package feature flag' do |action:| + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it_behaves_like "#{action} the namespace package setting" + end + context 'with existing namespace package setting' do let_it_be(:package_settings) { create(:namespace_package_setting, namespace: namespace) } let_it_be(:params) do @@ -96,7 +105,8 @@ RSpec.describe Mutations::Namespace::PackageSettings::Update do end where(:user_role, :shared_examples_name) do - :maintainer | 'updating the namespace package setting' + :owner | 'updating the namespace package setting' + :maintainer | 'denying access to namespace package setting' :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' @@ -109,6 +119,7 @@ RSpec.describe Mutations::Namespace::PackageSettings::Update do end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :updating if params[:user_role] == :maintainer end end @@ -116,7 +127,8 @@ RSpec.describe Mutations::Namespace::PackageSettings::Update do let_it_be(:package_settings) { namespace.package_settings } where(:user_role, :shared_examples_name) do - :maintainer | 'creating the namespace package setting' + :owner | 'creating the namespace package setting' + :maintainer | 'denying access to namespace package setting' :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' @@ -129,6 +141,7 @@ RSpec.describe Mutations::Namespace::PackageSettings::Update do end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :creating if params[:user_role] == :maintainer end end end diff --git a/spec/graphql/types/dependency_proxy/image_ttl_group_policy_type_spec.rb b/spec/graphql/types/dependency_proxy/image_ttl_group_policy_type_spec.rb index af0f91a844e..2a81e9508e7 100644 --- a/spec/graphql/types/dependency_proxy/image_ttl_group_policy_type_spec.rb +++ b/spec/graphql/types/dependency_proxy/image_ttl_group_policy_type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GitlabSchema.types['DependencyProxyImageTtlGroupPolicy'] do +RSpec.describe GitlabSchema.types['DependencyProxyImageTtlGroupPolicy'], feature_category: :dependency_proxy do it { expect(described_class.graphql_name).to eq('DependencyProxyImageTtlGroupPolicy') } it { expect(described_class.description).to eq('Group-level Dependency Proxy TTL policy settings') } diff --git a/spec/graphql/types/namespace/package_settings_type_spec.rb b/spec/graphql/types/namespace/package_settings_type_spec.rb index 5039f2d6153..40048b7dfa6 100644 --- a/spec/graphql/types/namespace/package_settings_type_spec.rb +++ b/spec/graphql/types/namespace/package_settings_type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GitlabSchema.types['PackageSettings'] do +RSpec.describe GitlabSchema.types['PackageSettings'], feature_category: :package_registry do specify { expect(described_class.graphql_name).to eq('PackageSettings') } specify { expect(described_class.description).to eq('Namespace-level Package Registry settings') } diff --git a/spec/lib/gitlab/database/database_connection_info_spec.rb b/spec/lib/gitlab/database/database_connection_info_spec.rb new file mode 100644 index 00000000000..c87fd61268d --- /dev/null +++ b/spec/lib/gitlab/database/database_connection_info_spec.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::DatabaseConnectionInfo, feature_category: :cell do + let(:default_attributes) do + { + name: 'main', + gitlab_schemas: ['gitlab_main'], + klass: 'ActiveRecord::Base' + } + end + + let(:attributes) { default_attributes } + + subject { described_class.new(attributes) } + + describe '.new' do + let(:attributes) { default_attributes.merge(fallback_database: 'fallback') } + + it 'does convert attributes into symbols and objects' do + expect(subject.name).to be_a(Symbol) + expect(subject.gitlab_schemas).to all(be_a(Symbol)) + expect(subject.klass).to be(ActiveRecord::Base) + expect(subject.fallback_database).to be_a(Symbol) + expect(subject.db_dir).to be_a(Pathname) + end + + it 'does raise error when using invalid argument' do + expect { described_class.new(invalid: 'aa') }.to raise_error ArgumentError, /unknown keywords: invalid/ + end + end + + describe '.load_file' do + it 'does load YAML file and has file_path specified' do + file_path = Rails.root.join('db/database_connections/main.yaml') + db_info = described_class.load_file(file_path) + + expect(db_info).not_to be_nil + expect(db_info.file_path).to eq(file_path) + end + end + + describe '#connection_class' do + context 'when klass is "ActiveRecord::Base"' do + let(:attributes) { default_attributes.merge(klass: 'ActiveRecord::Base') } + + it 'does always return "ActiveRecord::Base"' do + expect(subject.connection_class).to eq(ActiveRecord::Base) + end + end + + context 'when klass is "Ci::ApplicationRecord"' do + let(:attributes) { default_attributes.merge(klass: 'Ci::ApplicationRecord') } + + it 'does return "Ci::ApplicationRecord" when it is connection_class' do + expect(Ci::ApplicationRecord).to receive(:connection_class).and_return(true) + + expect(subject.connection_class).to eq(Ci::ApplicationRecord) + end + + it 'does return nil when it is not connection_class' do + expect(Ci::ApplicationRecord).to receive(:connection_class).and_return(false) + + expect(subject.connection_class).to eq(nil) + end + end + end + + describe '#order' do + using RSpec::Parameterized::TableSyntax + + let(:configs_for) { %w[main ci geo] } + + before do + hash_configs = configs_for.map do |x| + instance_double(ActiveRecord::DatabaseConfigurations::HashConfig, name: x) + end + allow(::ActiveRecord::Base).to receive(:configurations).and_return( + instance_double(ActiveRecord::DatabaseConfigurations, configs_for: hash_configs) + ) + end + + where(:name, :order) do + :main | 0 + :ci | 1 + :undefined | 1000 + end + + with_them do + let(:attributes) { default_attributes.merge(name: name) } + + it { expect(subject.order).to eq(order) } + end + end + + describe '#connection_class_or_fallback' do + let(:all_databases) do + { + main: described_class.new( + name: 'main', gitlab_schemas: [], klass: 'ActiveRecord::Base'), + ci: described_class.new( + name: 'ci', gitlab_schemas: [], klass: 'Ci::ApplicationRecord', fallback_database: 'main') + } + end + + context 'for "main"' do + it 'does return ActiveRecord::Base' do + expect(all_databases[:main].connection_class_or_fallback(all_databases)) + .to eq(ActiveRecord::Base) + end + end + + context 'for "ci"' do + it 'does return "Ci::ApplicationRecord" when it is connection_class' do + expect(Ci::ApplicationRecord).to receive(:connection_class).and_return(true) + + expect(all_databases[:ci].connection_class_or_fallback(all_databases)) + .to eq(Ci::ApplicationRecord) + end + + it 'does return "ActiveRecord::Base" (fallback to "main") when it is not connection_class' do + expect(Ci::ApplicationRecord).to receive(:connection_class).and_return(false) + + expect(all_databases[:ci].connection_class_or_fallback(all_databases)) + .to eq(ActiveRecord::Base) + end + end + end + + describe '#has_gitlab_shared?' do + using RSpec::Parameterized::TableSyntax + + where(:gitlab_schemas, :result) do + %w[gitlab_main] | false + %w[gitlab_main gitlab_shared] | true + end + + with_them do + let(:attributes) { default_attributes.merge(gitlab_schemas: gitlab_schemas) } + + it { expect(subject.has_gitlab_shared?).to eq(result) } + end + end + + describe 'db_docs_dir' do + let(:attributes) { default_attributes.merge(db_dir: db_dir) } + + context 'when db_dir is specified' do + let(:db_dir) { 'ee/my/db' } + + it { expect(subject.db_docs_dir).to eq(Rails.root.join(db_dir, 'docs')) } + end + + context 'when db_dir is not specified fallbacks to "db/docs"' do + let(:db_dir) { nil } + + it { expect(subject.db_docs_dir).to eq(Rails.root.join('db/docs')) } + end + end +end diff --git a/spec/lib/gitlab/database/each_database_spec.rb b/spec/lib/gitlab/database/each_database_spec.rb index 75b543bee85..2653297c81a 100644 --- a/spec/lib/gitlab/database/each_database_spec.rb +++ b/spec/lib/gitlab/database/each_database_spec.rb @@ -70,11 +70,13 @@ RSpec.describe Gitlab::Database::EachDatabase do # Clear the memoization because the return of Gitlab::Database#schemas_to_base_models depends stubbed value clear_memoization(:@schemas_to_base_models) - clear_memoization(:@schemas_to_base_models_ee) end it 'only yields the unshared connections' do - expect(Gitlab::Database).to receive(:db_config_share_with).exactly(3).times.and_return(nil, 'main', 'main') + # if this is `non-main` connection make it shared with `main` + allow(Gitlab::Database).to receive(:db_config_share_with) do |db_config| + db_config.name != 'main' ? 'main' : nil + end expect { |b| described_class.each_database_connection(include_shared: false, &b) } .to yield_successive_args([ActiveRecord::Base.connection, 'main']) diff --git a/spec/lib/gitlab/database/gitlab_schema_info_spec.rb b/spec/lib/gitlab/database/gitlab_schema_info_spec.rb new file mode 100644 index 00000000000..b37aec46de8 --- /dev/null +++ b/spec/lib/gitlab/database/gitlab_schema_info_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::GitlabSchemaInfo, feature_category: :cell do + describe '.new' do + it 'does ensure that name is always symbol' do + schema_info = described_class.new(name: 'gitlab_main') + expect(schema_info.name).to eq(:gitlab_main) + end + + it 'does raise error when using invalid argument' do + expect { described_class.new(invalid: 'aa') }.to raise_error ArgumentError, /unknown keywords: invalid/ + end + end + + describe '.load_file' do + it 'does load YAML file and has file_path specified' do + file_path = Rails.root.join('db/gitlab_schemas/gitlab_main.yaml') + schema_info = described_class.load_file(file_path) + + expect(schema_info).not_to be_nil + expect(schema_info.file_path).to eq(file_path) + end + end +end diff --git a/spec/lib/gitlab/database/gitlab_schema_spec.rb b/spec/lib/gitlab/database/gitlab_schema_spec.rb index 0499605ee2e..3cb2d91e227 100644 --- a/spec/lib/gitlab/database/gitlab_schema_spec.rb +++ b/spec/lib/gitlab/database/gitlab_schema_spec.rb @@ -45,53 +45,72 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do describe '.views_and_tables_to_schema' do include_examples 'validate schema data', described_class.views_and_tables_to_schema - # This being run across different databases indirectly also tests - # a general consistency of structure across databases - Gitlab::Database.database_base_models.except(:geo).each do |db_config_name, db_class| - context "for #{db_config_name} using #{db_class}" do - let(:db_data_sources) { db_class.connection.data_sources } - - # The embedding and Geo databases do not share the same structure as all decomposed databases - subject do - described_class.views_and_tables_to_schema.reject { |_, v| v == :gitlab_embedding || v == :gitlab_geo } + # group configurations by db_docs_dir, since then we expect all sharing this + # to contain exactly those tables + Gitlab::Database.all_database_connections.values.group_by(&:db_docs_dir).each do |db_docs_dir, db_infos| + context "for #{db_docs_dir}" do + let(:all_gitlab_schemas) { db_infos.flat_map(&:gitlab_schemas).to_set } + + let(:tables_for_gitlab_schemas) do + described_class.views_and_tables_to_schema.select do |_, gitlab_schema| + all_gitlab_schemas.include?(gitlab_schema) + end end - it 'new data sources are added' do - missing_data_sources = db_data_sources.to_set - subject.keys - - expect(missing_data_sources).to be_empty, \ - "Missing table/view(s) #{missing_data_sources.to_a} not found in " \ - "#{described_class}.views_and_tables_to_schema. " \ - "Any new tables or views must be added to the database dictionary. " \ - "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" - end - - it 'non-existing data sources are removed' do - extra_data_sources = subject.keys.to_set - db_data_sources - - expect(extra_data_sources).to be_empty, \ - "Extra table/view(s) #{extra_data_sources.to_a} found in #{described_class}.views_and_tables_to_schema. " \ - "Any removed or renamed tables or views must be removed from the database dictionary. " \ - "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" + db_infos.to_h { |db_info| [db_info.name, db_info.connection_class] } + .compact.each do |db_config_name, connection_class| + context "validates '#{db_config_name}' using '#{connection_class}'" do + let(:data_sources) { connection_class.connection.data_sources } + + it 'new data sources are added' do + missing_data_sources = data_sources.to_set - tables_for_gitlab_schemas.keys + + expect(missing_data_sources).to be_empty, \ + "Missing table/view(s) #{missing_data_sources.to_a} not found in " \ + "#{described_class}.views_and_tables_to_schema. " \ + "Any new tables or views must be added to the database dictionary. " \ + "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" + end + + it 'non-existing data sources are removed' do + extra_data_sources = tables_for_gitlab_schemas.keys.to_set - data_sources + + expect(extra_data_sources).to be_empty, \ + "Extra table/view(s) #{extra_data_sources.to_a} found in " \ + "#{described_class}.views_and_tables_to_schema. " \ + "Any removed or renamed tables or views must be removed from the database dictionary. " \ + "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" + end + end end end end - end - describe '.dictionary_path_globs' do - include_examples 'validate path globs', described_class.dictionary_path_globs - end + it 'all tables and views are unique' do + table_and_view_names = described_class.build_dictionary('') + table_and_view_names += described_class.build_dictionary('views') - describe '.view_path_globs' do - include_examples 'validate path globs', described_class.view_path_globs - end + # ignore gitlab_internal due to `ar_internal_metadata`, `schema_migrations` + table_and_view_names = table_and_view_names + .reject { |_, gitlab_schema| gitlab_schema == :gitlab_internal } + + duplicated_tables = table_and_view_names + .group_by(&:first) + .select { |_, schemas| schemas.count > 1 } + .keys - describe '.deleted_tables_path_globs' do - include_examples 'validate path globs', described_class.deleted_tables_path_globs + expect(duplicated_tables).to be_empty, \ + "Duplicated table(s) #{duplicated_tables.to_a} found in #{described_class}.views_and_tables_to_schema. " \ + "Any duplicated table must be removed from db/docs/ or ee/db/docs/. " \ + "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" + end end - describe '.deleted_views_path_globs' do - include_examples 'validate path globs', described_class.deleted_views_path_globs + describe '.dictionary_path_globs' do + include_examples 'validate path globs', described_class.dictionary_path_globs('') + include_examples 'validate path globs', described_class.dictionary_path_globs('views') + include_examples 'validate path globs', described_class.dictionary_path_globs('deleted_views') + include_examples 'validate path globs', described_class.dictionary_path_globs('deleted_tables') end describe '.tables_to_schema' do @@ -121,7 +140,7 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do end describe '.table_schemas!' do - let(:tables) { %w[users projects ci_builds] } + let(:tables) { %w[namespaces projects ci_builds] } subject { described_class.table_schemas!(tables) } @@ -130,7 +149,7 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do end context 'when one of the tables does not have a matching table schema' do - let(:tables) { %w[users projects unknown ci_builds] } + let(:tables) { %w[namespaces projects unknown ci_builds] } it 'raises error' do expect { subject }.to raise_error(/Could not find gitlab schema for table unknown/) diff --git a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb index f5ce207773f..82f77d2bb19 100644 --- a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb @@ -428,21 +428,24 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d describe '#ensure_batched_background_migration_is_finished' do let(:job_class_name) { 'CopyColumnUsingBackgroundMigrationJob' } - let(:table) { :events } + let(:table_name) { 'events' } let(:column_name) { :id } let(:job_arguments) { [["id"], ["id_convert_to_bigint"], nil] } + let(:gitlab_schema) { Gitlab::Database::GitlabSchema.table_schema!(table_name) } let(:configuration) do { job_class_name: job_class_name, - table_name: table, + table_name: table_name, column_name: column_name, job_arguments: job_arguments } end let(:migration_attributes) do - configuration.merge(gitlab_schema: Gitlab::Database.gitlab_schemas_for_connection(migration.connection).first) + configuration.merge( + gitlab_schema: gitlab_schema + ) end before do @@ -457,7 +460,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d create(:batched_background_migration, :active, migration_attributes) allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| - allow(runner).to receive(:finalize).with(job_class_name, table, column_name, job_arguments).and_return(false) + allow(runner).to receive(:finalize).with(job_class_name, table_name, column_name, job_arguments).and_return(false) end expect { ensure_batched_background_migration_is_finished } @@ -530,7 +533,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d migration = create(:batched_background_migration, :active, configuration) allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| - expect(runner).to receive(:finalize).with(job_class_name, table, column_name, job_arguments).and_return(migration.finish!) + expect(runner).to receive(:finalize).with(job_class_name, table_name, column_name, job_arguments).and_return(migration.finish!) end ensure_batched_background_migration_is_finished @@ -543,7 +546,7 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d create(:batched_background_migration, :active, configuration) allow_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |runner| - expect(runner).not_to receive(:finalize).with(job_class_name, table, column_name, job_arguments) + expect(runner).not_to receive(:finalize).with(job_class_name, table_name, column_name, job_arguments) end expect { migration.ensure_batched_background_migration_is_finished(**configuration.merge(finalize: false)) }.to raise_error(RuntimeError) diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb index d5f4afd7ba4..5f1e8842f18 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb @@ -228,6 +228,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::ForeignKeyHelpers end it 'validates FK for each partition' do + allow(migration).to receive(:statement_timeout_disabled?).and_return(false) expect(migration).to receive(:execute).with(/SET statement_timeout TO 0/).twice expect(migration).to receive(:execute).with(/RESET statement_timeout/).twice expect(migration).to receive(:execute) diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index f2be888e6eb..ab3cd8fa5e6 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -15,6 +15,68 @@ RSpec.describe Gitlab::Database, feature_category: :database do end end + describe '.all_database_connections' do + it 'the first entry is always main' do + expect(described_class.all_database_connections.keys).to start_with('main') + end + + it 'contains as many entries as YAML files' do + expect(described_class.all_database_connections.values.map(&:file_path)) + .to contain_exactly(*described_class.all_database_connection_files) + end + end + + describe '.database_base_models' do + subject { described_class.database_base_models } + + it 'contains "main"' do + is_expected.to include("main" => ActiveRecord::Base) + end + + it 'does not contain "ci" when not running CI database' do + skip_if_multiple_databases_are_setup(:ci) + + is_expected.not_to include("ci") + end + + it 'contains "ci" pointing to Ci::ApplicationRecord when running CI database' do + skip_if_multiple_databases_not_setup(:ci) + + is_expected.to include("ci" => Ci::ApplicationRecord) + end + end + + describe '.all_gitlab_schemas' do + it 'contains as many entries as YAML files' do + expect(described_class.all_gitlab_schemas.values.map(&:file_path)) + .to contain_exactly(*described_class.all_gitlab_schema_files) + end + end + + describe '.schemas_to_base_models' do + subject { described_class.schemas_to_base_models } + + it 'contains gitlab_main' do + is_expected.to include(gitlab_main: [ActiveRecord::Base]) + end + + it 'contains gitlab_shared' do + is_expected.to include(gitlab_main: include(ActiveRecord::Base)) + end + + it 'contains gitlab_ci pointing to ActiveRecord::Base when not running CI database' do + skip_if_multiple_databases_are_setup(:ci) + + is_expected.to include(gitlab_ci: [ActiveRecord::Base]) + end + + it 'contains gitlab_ci pointing to Ci::ApplicationRecord when running CI database' do + skip_if_multiple_databases_not_setup(:ci) + + is_expected.to include(gitlab_ci: [Ci::ApplicationRecord]) + end + end + describe '.default_pool_size' do before do allow(Gitlab::Runtime).to receive(:max_threads).and_return(7) @@ -250,22 +312,35 @@ RSpec.describe Gitlab::Database, feature_category: :database do end describe '.db_config_names' do - let(:expected) { %w[foo bar] } + using RSpec::Parameterized::TableSyntax - it 'includes only main by default' do - allow(::ActiveRecord::Base).to receive(:configurations).and_return( - double(configs_for: %w[foo bar].map { |x| double(name: x) }) - ) - - expect(described_class.db_config_names).to eq(expected) + where(:configs_for, :gitlab_schema, :expected_main, :expected_main_ci) do + %i[main] | :gitlab_shared | %i[main] | %i[main] + %i[main ci] | :gitlab_shared | %i[main] | %i[main ci] + %i[main ci] | :gitlab_ci | %i[main] | %i[ci] end - it 'excludes geo when that is included' do - allow(::ActiveRecord::Base).to receive(:configurations).and_return( - double(configs_for: %w[foo bar geo].map { |x| double(name: x) }) - ) + with_them do + before do + hash_configs = configs_for.map do |x| + instance_double(ActiveRecord::DatabaseConfigurations::HashConfig, name: x) + end + allow(::ActiveRecord::Base).to receive(:configurations).and_return( + instance_double(ActiveRecord::DatabaseConfigurations, configs_for: hash_configs) + ) + end - expect(described_class.db_config_names).to eq(expected) + if ::Gitlab::Database.has_config?(:ci) + it 'when main and CI database are configured' do + expect(described_class.db_config_names(with_schema: gitlab_schema)) + .to eq(expected_main_ci) + end + else + it 'when only main database is configured' do + expect(described_class.db_config_names(with_schema: gitlab_schema)) + .to eq(expected_main) + end + end end end diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb index 8595ea6d495..be202733a89 100644 --- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb @@ -146,6 +146,14 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote, :clean_gitlab_red expect(note.line_code).to eq('8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_2_2') end + + context 'when comment on file' do + it 'generates line code for first line' do + note = described_class.new(diff_hunk: '', file_path: 'README.md', subject_type: 'file') + + expect(note.line_code).to eq('8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_1_1') + end + end end describe '#note and #contains_suggestion?' do diff --git a/spec/lib/gitlab/hotlinking_detector_spec.rb b/spec/lib/gitlab/hotlinking_detector_spec.rb index 536d744c197..809c4a3c244 100644 --- a/spec/lib/gitlab/hotlinking_detector_spec.rb +++ b/spec/lib/gitlab/hotlinking_detector_spec.rb @@ -39,6 +39,9 @@ RSpec.describe Gitlab::HotlinkingDetector do true | "text/css,*/*;q=0.1" true | "text/css" true | "text/css,*/*;q=0.1" + + # Invalid MIME definition + true | "text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2" end with_them do diff --git a/spec/lib/gitlab/metrics/loose_foreign_keys_slis_spec.rb b/spec/lib/gitlab/metrics/loose_foreign_keys_slis_spec.rb index 58740278425..0d6ce68a7f8 100644 --- a/spec/lib/gitlab/metrics/loose_foreign_keys_slis_spec.rb +++ b/spec/lib/gitlab/metrics/loose_foreign_keys_slis_spec.rb @@ -7,7 +7,7 @@ RSpec.describe ::Gitlab::Metrics::LooseForeignKeysSlis do # config/database.yml and the specs need to work for all configurations. That # means this assertion is a copy of the implementation. let(:possible_labels) do - ::Gitlab::Database.db_config_names.map do |db_config_name| + ::Gitlab::Database.db_config_names(with_schema: :gitlab_shared).map do |db_config_name| { db_config_name: db_config_name, feature_category: :database diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb index 2d4c6d1cc56..fe5264a1ccb 100644 --- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb @@ -10,6 +10,25 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do let(:store_label) { 'CustomStore' } let(:event) { double(:event, duration: 15.2, payload: { key: %w[a b c], store: store }) } + context 'when receiving multiple instrumentation hits in a transaction' do + before do + allow(subscriber).to receive(:current_transaction) + .and_return(transaction) + end + + it 'does not raise InvalidLabelSetError error' do + expect do + subscriber.cache_read(event) + subscriber.cache_read_multi(event) + subscriber.cache_write(event) + subscriber.cache_delete(event) + subscriber.cache_exist?(event) + subscriber.cache_fetch_hit(event) + subscriber.cache_generate(event) + end.not_to raise_error + end + end + describe '#cache_read' do it 'increments the cache_read duration' do expect(subscriber).to receive(:observe) @@ -32,7 +51,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do it 'does not increment cache read miss total' do expect(transaction).not_to receive(:increment) - .with(:gitlab_cache_misses_total, 1) + .with(:gitlab_cache_misses_total, 1, { store: store_label }) subscriber.cache_read(event) end @@ -44,7 +63,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do it 'increments the cache_read_miss total' do expect(transaction).to receive(:increment) - .with(:gitlab_cache_misses_total, 1) + .with(:gitlab_cache_misses_total, 1, { store: store_label }) expect(transaction).to receive(:increment) .with(any_args).at_least(1) # Other calls @@ -56,7 +75,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do it 'does not increment cache read miss total' do expect(transaction).not_to receive(:increment) - .with(:gitlab_cache_misses_total, 1) + .with(:gitlab_cache_misses_total, 1, { store: store_label }) subscriber.cache_read(event) end @@ -145,7 +164,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do it 'increments the cache_read_hit count' do expect(transaction).to receive(:increment) - .with(:gitlab_transaction_cache_read_hit_count_total, 1) + .with(:gitlab_transaction_cache_read_hit_count_total, 1, { store: store_label }) subscriber.cache_fetch_hit(event) end @@ -168,9 +187,9 @@ RSpec.describe Gitlab::Metrics::Subscribers::RailsCache do end it 'increments the cache_fetch_miss count and cache_read_miss total' do - expect(transaction).to receive(:increment).with(:gitlab_cache_misses_total, 1) + expect(transaction).to receive(:increment).with(:gitlab_cache_misses_total, 1, { store: store_label }) expect(transaction).to receive(:increment) - .with(:gitlab_transaction_cache_read_miss_count_total, 1) + .with(:gitlab_transaction_cache_read_miss_count_total, 1, { store: store_label }) subscriber.cache_generate(event) end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 5e85a6e187b..bca38fa5638 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -1167,6 +1167,14 @@ RSpec.describe GroupPolicy, feature_category: :system_access do end describe 'dependency proxy' do + RSpec.shared_examples 'disabling admin_package feature flag' do + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it { is_expected.to be_allowed(:admin_dependency_proxy) } + end + context 'feature disabled' do let(:current_user) { owner } @@ -1197,7 +1205,18 @@ RSpec.describe GroupPolicy, feature_category: :system_access do let(:current_user) { maintainer } it { is_expected.to be_allowed(:read_dependency_proxy) } + it { is_expected.to be_disallowed(:admin_dependency_proxy) } + + it_behaves_like 'disabling admin_package feature flag' + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:read_dependency_proxy) } it { is_expected.to be_allowed(:admin_dependency_proxy) } + + it_behaves_like 'disabling admin_package feature flag' end end end @@ -1760,4 +1779,38 @@ RSpec.describe GroupPolicy, feature_category: :system_access do specify { is_expected.to be_allowed(:read_achievement) } end end + + describe 'admin_package ability' do + context 'with maintainer' do + let(:current_user) { maintainer } + + context 'with feature flag enabled' do + specify { is_expected.to be_disallowed(:admin_package) } + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + specify { is_expected.to be_allowed(:admin_package) } + end + end + + context 'with owner' do + let(:current_user) { owner } + + context 'with feature flag enabled' do + specify { is_expected.to be_allowed(:admin_package) } + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + specify { is_expected.to be_allowed(:admin_package) } + end + end + end end diff --git a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb index aca8527ba0a..c8745fcbb62 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_group_setting_spec.rb @@ -46,12 +46,15 @@ RSpec.describe 'getting dependency proxy settings for a group', feature_category context 'with different permissions' do where(:group_visibility, :role, :access_granted) do - :private | :maintainer | true + :private | :owner | true + :private | :maintainer | false :private | :developer | false :private | :reporter | false :private | :guest | false :private | :anonymous | false - :public | :maintainer | true + + :public | :owner | true + :public | :maintainer | false :public | :developer | false :public | :reporter | false :public | :guest | false @@ -73,6 +76,20 @@ RSpec.describe 'getting dependency proxy settings for a group', feature_category expect(dependency_proxy_group_setting_response).to be_blank end end + + context 'with disabled admin_package feature flag' do + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + if params[:role] == :maintainer + it 'return the proper response' do + subject + + expect(dependency_proxy_group_setting_response).to eq('enabled' => true) + end + end + end end end end diff --git a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb index edff4dc1dae..8365cece4a3 100644 --- a/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb +++ b/spec/requests/api/graphql/group/dependency_proxy_image_ttl_policy_spec.rb @@ -45,12 +45,15 @@ RSpec.describe 'getting dependency proxy image ttl policy for a group', feature_ context 'with different permissions' do where(:group_visibility, :role, :access_granted) do - :private | :maintainer | true + :private | :owner | true + :private | :maintainer | false :private | :developer | false :private | :reporter | false :private | :guest | false :private | :anonymous | false - :public | :maintainer | true + + :public | :owner | true + :public | :maintainer | false :public | :developer | false :public | :reporter | false :public | :guest | false @@ -72,6 +75,20 @@ RSpec.describe 'getting dependency proxy image ttl policy for a group', feature_ expect(dependency_proxy_image_ttl_policy_response).to be_blank end end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + if params[:role] == :maintainer + it 'returns the proper response' do + subject + + expect(dependency_proxy_image_ttl_policy_response).to eq("createdAt" => nil, "enabled" => false, "ttl" => 90, "updatedAt" => nil) + end + end + end end end end diff --git a/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb b/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb index 5d5696d3f66..86ea77a8f35 100644 --- a/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/dependency_proxy/group_settings/update_spec.rb @@ -49,16 +49,21 @@ RSpec.describe 'Updating the dependency proxy group settings', feature_category: end context 'with permission' do - before do - group.add_maintainer(user) - end + %i[owner maintainer].each do |role| + context "for #{role}" do + before do + group.send("add_#{role}", user) + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end - it 'returns the updated dependency proxy settings', :aggregate_failures do - subject + it 'returns the updated dependency proxy settings', :aggregate_failures do + subject - expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['errors']).to be_empty - expect(group_settings[:enabled]).to eq(false) + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['errors']).to be_empty + expect(group_settings[:enabled]).to eq(false) + end + end end end end diff --git a/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb b/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb index 66ee17f356c..bc8b2da84b9 100644 --- a/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb +++ b/spec/requests/api/graphql/mutations/dependency_proxy/image_ttl_group_policy/update_spec.rb @@ -51,19 +51,24 @@ RSpec.describe 'Updating the dependency proxy image ttl policy', feature_categor end context 'with permission' do - before do - group.add_maintainer(user) - end + %i[owner maintainer].each do |role| + context "for #{role}" do + before do + group.send("add_#{role}", user) + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end - it 'returns the updated dependency proxy image ttl policy', :aggregate_failures do - subject + it 'returns the updated dependency proxy image ttl policy', :aggregate_failures do + subject - expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['errors']).to be_empty - expect(ttl_policy_response).to include( - 'enabled' => params[:enabled], - 'ttl' => params[:ttl] - ) + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['errors']).to be_empty + expect(ttl_policy_response).to include( + 'enabled' => params[:enabled], + 'ttl' => params[:ttl] + ) + end + end end end end diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb index f4f4f34fe29..2f26a2f92b2 100644 --- a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -139,6 +139,15 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis end end + # To be removed when raise_group_admin_package_permission_to_owner FF is removed + RSpec.shared_examples 'disabling admin_package feature flag' do |action:| + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it_behaves_like "accepting the mutation request #{action} the package settings" + end + describe 'post graphql mutation' do subject { post_graphql_mutation(mutation, current_user: user) } @@ -147,7 +156,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis let_it_be(:namespace, reload: true) { package_settings.namespace } where(:user_role, :shared_examples_name) do - :maintainer | 'accepting the mutation request updating the package settings' + :owner | 'accepting the mutation request updating the package settings' + :maintainer | 'denying the mutation request' :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' @@ -160,6 +170,7 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :updating if params[:user_role] == :maintainer end end @@ -169,7 +180,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis let(:package_settings) { namespace.package_settings } where(:user_role, :shared_examples_name) do - :maintainer | 'accepting the mutation request creating the package settings' + :owner | 'accepting the mutation request creating the package settings' + :maintainer | 'denying the mutation request' :developer | 'denying the mutation request' :reporter | 'denying the mutation request' :guest | 'denying the mutation request' @@ -182,6 +194,7 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :creating if params[:user_role] == :maintainer end end end diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index caf62c251b6..644f26af006 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -53,16 +53,6 @@ RSpec.describe 'merge requests discussions', feature_category: :source_code_mana let(:notes_metadata_threshold) { 1 } it_behaves_like 'N+1 queries' - - context 'when external_note_author_service_desk feature flag is disabled' do - let(:notes_metadata_threshold) { 0 } - - before do - stub_feature_flags(external_note_author_service_desk: false) - end - - it_behaves_like 'N+1 queries' - end end it 'limits Gitaly queries', :request_store do diff --git a/spec/serializers/note_entity_spec.rb b/spec/serializers/note_entity_spec.rb index bbb1d2ca164..d5c4a31a937 100644 --- a/spec/serializers/note_entity_spec.rb +++ b/spec/serializers/note_entity_spec.rb @@ -59,22 +59,11 @@ RSpec.describe NoteEntity do subject { entity.as_json[:external_author] } - context 'when external_note_author_service_desk feature flag is enabled' do + context 'with external note author' do let(:obfuscated_email) { 'em*****@e*****.c**' } let(:email) { 'email@example.com' } it_behaves_like 'external author' end - - context 'when external_note_author_service_desk feature flag is disabled' do - let(:email) { nil } - let(:obfuscated_email) { nil } - - before do - stub_feature_flags(external_note_author_service_desk: false) - end - - it_behaves_like 'external author' - end end end diff --git a/spec/services/dependency_proxy/group_settings/update_service_spec.rb b/spec/services/dependency_proxy/group_settings/update_service_spec.rb index 38f837a828a..101eee35ca5 100644 --- a/spec/services/dependency_proxy/group_settings/update_service_spec.rb +++ b/spec/services/dependency_proxy/group_settings/update_service_spec.rb @@ -41,7 +41,8 @@ RSpec.describe ::DependencyProxy::GroupSettings::UpdateService, feature_category end where(:user_role, :shared_examples_name) do - :maintainer | 'updating the dependency proxy group settings' + :owner | 'updating the dependency proxy group settings' + :maintainer | 'denying access to dependency proxy group settings' :developer | 'denying access to dependency proxy group settings' :reporter | 'denying access to dependency proxy group settings' :guest | 'denying access to dependency proxy group settings' @@ -55,6 +56,14 @@ RSpec.describe ::DependencyProxy::GroupSettings::UpdateService, feature_category end it_behaves_like params[:shared_examples_name] + + context 'with disabled admin_package feature flag' do + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it_behaves_like 'updating the dependency proxy group settings' if params[:user_role] == :maintainer + end end end end diff --git a/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb b/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb index f58434222a5..6a5df7358eb 100644 --- a/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb +++ b/spec/services/dependency_proxy/image_ttl_group_policies/update_service_spec.rb @@ -62,6 +62,15 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService, feature_ end end + # To be removed when raise_group_admin_package_permission_to_owner FF is removed + shared_examples 'disabling admin_package feature flag' do |action:| + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it_behaves_like "#{action} the dependency proxy image ttl policy" + end + before do stub_config(dependency_proxy: { enabled: true }) end @@ -71,7 +80,8 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService, feature_ let_it_be(:params) { { enabled: false, ttl: 2 } } where(:user_role, :shared_examples_name) do - :maintainer | 'updating the dependency proxy image ttl policy' + :owner | 'updating the dependency proxy image ttl policy' + :maintainer | 'denying access to dependency proxy image ttl policy' :developer | 'denying access to dependency proxy image ttl policy' :reporter | 'denying access to dependency proxy image ttl policy' :guest | 'denying access to dependency proxy image ttl policy' @@ -84,6 +94,7 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService, feature_ end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :updating if params[:user_role] == :maintainer end end @@ -91,7 +102,8 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService, feature_ let_it_be(:ttl_policy) { group.dependency_proxy_image_ttl_policy } where(:user_role, :shared_examples_name) do - :maintainer | 'creating the dependency proxy image ttl policy' + :owner | 'creating the dependency proxy image ttl policy' + :maintainer | 'denying access to dependency proxy image ttl policy' :developer | 'denying access to dependency proxy image ttl policy' :reporter | 'denying access to dependency proxy image ttl policy' :guest | 'denying access to dependency proxy image ttl policy' @@ -104,15 +116,21 @@ RSpec.describe ::DependencyProxy::ImageTtlGroupPolicies::UpdateService, feature_ end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :creating if params[:user_role] == :maintainer end context 'when the policy is not found' do - before do - group.add_maintainer(user) - expect(group).to receive(:dependency_proxy_image_ttl_policy).and_return nil + %i[owner maintainer].each do |role| + context "when user is #{role}" do + before do + group.send("add_#{role}", user) + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + expect(group).to receive(:dependency_proxy_image_ttl_policy).and_return nil + end + + it_behaves_like 'returning an error', 'Dependency proxy image TTL Policy not found', 404 + end end - - it_behaves_like 'returning an error', 'Dependency proxy image TTL Policy not found', 404 end end end diff --git a/spec/services/namespaces/package_settings/update_service_spec.rb b/spec/services/namespaces/package_settings/update_service_spec.rb index e21c9a8f1b9..385fd7c130e 100644 --- a/spec/services/namespaces/package_settings/update_service_spec.rb +++ b/spec/services/namespaces/package_settings/update_service_spec.rb @@ -81,6 +81,15 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService, feature_category: : end end + # To be removed when raise_group_admin_package_permission_to_owner FF is removed + shared_examples 'disabling admin_package feature flag' do |action:| + before do + stub_feature_flags(raise_group_admin_package_permission_to_owner: false) + end + + it_behaves_like "#{action} the namespace package setting" + end + context 'with existing namespace package setting' do let_it_be(:package_settings) { create(:namespace_package_setting, namespace: namespace) } let_it_be(:params) do @@ -99,7 +108,8 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService, feature_category: : end where(:user_role, :shared_examples_name) do - :maintainer | 'updating the namespace package setting' + :owner | 'updating the namespace package setting' + :maintainer | 'denying access to namespace package setting' :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' @@ -112,6 +122,7 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService, feature_category: : end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :updating if params[:user_role] == :maintainer end end @@ -119,7 +130,8 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService, feature_category: : let_it_be(:package_settings) { namespace.package_settings } where(:user_role, :shared_examples_name) do - :maintainer | 'creating the namespace package setting' + :owner | 'creating the namespace package setting' + :maintainer | 'denying access to namespace package setting' :developer | 'denying access to namespace package setting' :reporter | 'denying access to namespace package setting' :guest | 'denying access to namespace package setting' @@ -132,6 +144,7 @@ RSpec.describe ::Namespaces::PackageSettings::UpdateService, feature_category: : end it_behaves_like params[:shared_examples_name] + it_behaves_like 'disabling admin_package feature flag', action: :creating if params[:user_role] == :maintainer end end end diff --git a/spec/support/helpers/database/multiple_databases_helpers.rb b/spec/support/helpers/database/multiple_databases_helpers.rb index 806956ca648..fcdf820642d 100644 --- a/spec/support/helpers/database/multiple_databases_helpers.rb +++ b/spec/support/helpers/database/multiple_databases_helpers.rb @@ -2,7 +2,7 @@ module Database module MultipleDatabasesHelpers - EXTRA_DBS = ::Gitlab::Database::DATABASE_NAMES.map(&:to_sym) - [:main] + EXTRA_DBS = ::Gitlab::Database.all_database_names.map(&:to_sym) - [:main] def database_exists?(database_name) ::Gitlab::Database.has_database?(database_name) diff --git a/spec/support/helpers/migrations_helpers.rb b/spec/support/helpers/migrations_helpers.rb index 1b8c3388051..dcf61d57af7 100644 --- a/spec/support/helpers/migrations_helpers.rb +++ b/spec/support/helpers/migrations_helpers.rb @@ -4,7 +4,7 @@ module MigrationsHelpers def active_record_base(database: nil) database_name = database || self.class.metadata[:database] || :main - unless Gitlab::Database::DATABASE_NAMES.include?(database_name.to_s) + unless ::Gitlab::Database.all_database_connections.include?(database_name) raise ArgumentError, "#{database_name} is not a valid argument" end diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index 111fd3dc7df..108acdfd4e5 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -52,7 +52,6 @@ RSpec.shared_context 'GroupPolicy context' do let(:maintainer_permissions) do %i[ destroy_package - admin_package create_projects create_cluster update_cluster admin_cluster add_cluster destroy_upload @@ -67,6 +66,7 @@ RSpec.shared_context 'GroupPolicy context' do admin_group admin_namespace admin_group_member + admin_package change_visibility_level set_note_created_at create_subgroup diff --git a/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb index 93a394387a3..59bdc4da174 100644 --- a/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb +++ b/spec/support/shared_examples/controllers/hotlink_interceptor_shared_examples.rb @@ -35,6 +35,9 @@ RSpec.shared_examples "hotlink interceptor" do :not_acceptable | "text/css,*/*;q=0.1" :not_acceptable | "text/css" :not_acceptable | "text/css,*/*;q=0.1" + + # Invalid MIME definition + :not_acceptable | "text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2" end with_them do diff --git a/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb b/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb index c07d1552ba2..dc92e56d013 100644 --- a/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb +++ b/spec/support/shared_examples/metrics/active_record_subscriber_shared_examples.rb @@ -2,7 +2,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| let(:db_config_name) do - db_config_name = ::Gitlab::Database.db_config_names.first + db_config_name = ::Gitlab::Database.db_config_names(with_schema: :gitlab_shared).first db_config_name += "_replica" if db_role == :secondary db_config_name end @@ -96,7 +96,7 @@ end RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do |db_role| let(:db_config_name) do - db_config_name = ::Gitlab::Database.db_config_names.first + db_config_name = ::Gitlab::Database.db_config_names(with_schema: :gitlab_shared).first db_config_name += "_replica" if db_role == :secondary db_config_name end diff --git a/spec/tasks/gitlab/background_migrations_rake_spec.rb b/spec/tasks/gitlab/background_migrations_rake_spec.rb index 04be713e0d4..0a7ab214cc1 100644 --- a/spec/tasks/gitlab/background_migrations_rake_spec.rb +++ b/spec/tasks/gitlab/background_migrations_rake_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gi before do allow(Gitlab::Database).to receive(:database_base_models).and_return(base_models) - allow(Gitlab::Database).to receive(:db_config_names).and_return(databases) + allow(Gitlab::Database).to receive(:db_config_names).with(with_schema: :gitlab_shared).and_return(databases) end context 'without the proper arguments' do diff --git a/spec/tasks/gitlab/db/decomposition/connection_status_rake_spec.rb b/spec/tasks/gitlab/db/decomposition/connection_status_rake_spec.rb index 78f86049ebb..4c161faf733 100644 --- a/spec/tasks/gitlab/db/decomposition/connection_status_rake_spec.rb +++ b/spec/tasks/gitlab/db/decomposition/connection_status_rake_spec.rb @@ -49,7 +49,7 @@ RSpec.describe 'gitlab:db:decomposition:connection_status', feature_category: :c context 'when separate ci database is configured' do before do - skip_if_multiple_databases_not_setup + skip_if_multiple_databases_not_setup(:ci) end it "does not show connection information" do diff --git a/spec/tasks/gitlab/db_rake_spec.rb b/spec/tasks/gitlab/db_rake_spec.rb index 95730f62b28..bc2fd0b22eb 100644 --- a/spec/tasks/gitlab/db_rake_spec.rb +++ b/spec/tasks/gitlab/db_rake_spec.rb @@ -563,8 +563,8 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor end if Gitlab.ee? - allow(File).to receive(:open).with(Rails.root.join(Gitlab::Database::EMBEDDING_DATABASE_DIR, 'structure.sql').to_s, any_args).and_yield(output) - allow(File).to receive(:open).with(Rails.root.join(Gitlab::Database::GEO_DATABASE_DIR, 'structure.sql').to_s, any_args).and_yield(output) + allow(File).to receive(:open).with(Rails.root.join('ee/db/geo/structure.sql').to_s, any_args).and_yield(output) + allow(File).to receive(:open).with(Rails.root.join('ee/db/embedding/structure.sql').to_s, any_args).and_yield(output) end end @@ -1018,7 +1018,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor end where(:db) do - Gitlab::Database::DATABASE_NAMES.map(&:to_sym) + ::Gitlab::Database.db_config_names(with_schema: :gitlab_shared).map(&:to_sym) end with_them do |
