diff options
74 files changed, 1627 insertions, 303 deletions
diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index b301a54e6ac..6904aea4f09 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -156,11 +156,7 @@ Rails/SaveBang: - 'spec/lib/gitlab/shard_health_cache_spec.rb' - 'spec/mailers/notify_spec.rb' - 'spec/models/clusters/applications/helm_spec.rb' - - 'spec/models/container_repository_spec.rb' - 'spec/models/design_management/version_spec.rb' - - 'spec/models/diff_discussion_spec.rb' - - 'spec/models/diff_note_spec.rb' - - 'spec/models/email_spec.rb' - 'spec/models/environment_spec.rb' - 'spec/models/event_spec.rb' - 'spec/models/fork_network_spec.rb' @@ -1681,7 +1677,6 @@ Gitlab/NamespacedClass: - 'app/models/project_services/mattermost_slash_commands_service.rb' - 'app/models/project_services/microsoft_teams_service.rb' - 'app/models/project_services/mock_ci_service.rb' - - 'app/models/project_services/mock_deployment_service.rb' - 'app/models/project_services/mock_monitoring_service.rb' - 'app/models/project_services/monitoring_service.rb' - 'app/models/project_services/open_project_service.rb' diff --git a/app/assets/javascripts/sidebar/components/participants/sidebar_participants.vue b/app/assets/javascripts/sidebar/components/participants/sidebar_participants.vue deleted file mode 100644 index 4ac515e552a..00000000000 --- a/app/assets/javascripts/sidebar/components/participants/sidebar_participants.vue +++ /dev/null @@ -1,31 +0,0 @@ -<script> -import Store from '../../stores/sidebar_store'; -import participants from './participants.vue'; - -export default { - components: { - participants, - }, - props: { - mediator: { - type: Object, - required: true, - }, - }, - data() { - return { - store: new Store(), - }; - }, -}; -</script> - -<template> - <div class="block participants"> - <participants - :loading="store.isFetching.participants" - :participants="store.participants" - :number-of-less-participants="7" - /> - </div> -</template> diff --git a/app/assets/javascripts/sidebar/components/participants/sidebar_participants_widget.vue b/app/assets/javascripts/sidebar/components/participants/sidebar_participants_widget.vue index d3043e6f6aa..9927a0f9114 100644 --- a/app/assets/javascripts/sidebar/components/participants/sidebar_participants_widget.vue +++ b/app/assets/javascripts/sidebar/components/participants/sidebar_participants_widget.vue @@ -64,5 +64,6 @@ export default { :loading="isLoading" :participants="participants" :number-of-less-participants="7" + class="block participants" /> </template> diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index 3f24fdc75dc..0de7a647c0a 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -14,6 +14,7 @@ import CollapsedAssigneeList from '~/sidebar/components/assignees/collapsed_assi import SidebarAssigneesWidget from '~/sidebar/components/assignees/sidebar_assignees_widget.vue'; import SidebarConfidentialityWidget from '~/sidebar/components/confidential/sidebar_confidentiality_widget.vue'; import SidebarDueDateWidget from '~/sidebar/components/date/sidebar_date_widget.vue'; +import SidebarParticipantsWidget from '~/sidebar/components/participants/sidebar_participants_widget.vue'; import SidebarReferenceWidget from '~/sidebar/components/reference/sidebar_reference_widget.vue'; import { apolloProvider } from '~/sidebar/graphql'; import Translate from '../vue_shared/translate'; @@ -21,7 +22,6 @@ import SidebarAssignees from './components/assignees/sidebar_assignees.vue'; import CopyEmailToClipboard from './components/copy_email_to_clipboard.vue'; import SidebarLabels from './components/labels/sidebar_labels.vue'; import IssuableLockForm from './components/lock/issuable_lock_form.vue'; -import sidebarParticipants from './components/participants/sidebar_participants.vue'; import SidebarReviewers from './components/reviewers/sidebar_reviewers.vue'; import SidebarSeverity from './components/severity/sidebar_severity.vue'; import SidebarSubscriptionsWidget from './components/subscriptions/sidebar_subscriptions_widget.vue'; @@ -314,21 +314,29 @@ function mountLockComponent() { }); } -function mountParticipantsComponent(mediator) { +function mountParticipantsComponent() { const el = document.querySelector('.js-sidebar-participants-entry-point'); if (!el) return; + const { fullPath, iid } = getSidebarOptions(); + // eslint-disable-next-line no-new new Vue({ el, + apolloProvider, components: { - sidebarParticipants, + SidebarParticipantsWidget, }, render: (createElement) => - createElement('sidebar-participants', { + createElement('sidebar-participants-widget', { props: { - mediator, + iid: String(iid), + fullPath, + issuableType: + isInIssuePage() || isInIncidentPage() || isInDesignPage() + ? IssuableType.Issue + : IssuableType.MergeRequest, }, }), }); @@ -435,7 +443,7 @@ export function mountSidebar(mediator) { mountDueDateComponent(mediator); mountReferenceComponent(mediator); mountLockComponent(); - mountParticipantsComponent(mediator); + mountParticipantsComponent(); mountSubscriptionsComponent(); mountCopyEmailComponent(); diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index 603d28a8395..ceccec8c5cb 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -200,10 +200,6 @@ @include btn-red; } - &.btn-cancel { - float: right; - } - &.btn-grouped { @include btn-with-margin; } diff --git a/app/graphql/mutations/labels/create.rb b/app/graphql/mutations/labels/create.rb index 4da628d53ea..683d0b44586 100644 --- a/app/graphql/mutations/labels/create.rb +++ b/app/graphql/mutations/labels/create.rb @@ -20,10 +20,6 @@ module Mutations required: false, description: 'Description of the label.' - argument :remove_on_close, GraphQL::BOOLEAN_TYPE, - required: false, - description: copy_field_description(Types::LabelType, :remove_on_close) - argument :color, GraphQL::STRING_TYPE, required: false, default_value: Label::DEFAULT_COLOR, diff --git a/app/graphql/types/label_type.rb b/app/graphql/types/label_type.rb index cb6b0312aa3..4e8718a80da 100644 --- a/app/graphql/types/label_type.rb +++ b/app/graphql/types/label_type.rb @@ -23,7 +23,5 @@ module Types description: 'When this label was created.' field :updated_at, Types::TimeType, null: false, description: 'When this label was last updated.' - field :remove_on_close, GraphQL::BOOLEAN_TYPE, null: false, - description: 'Whether the label should be removed from an issue when the issue is closed.' end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 46fc87a6ea8..b8edbbfed24 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -11,6 +11,7 @@ module Ci include Importable include Ci::HasRef include IgnorableColumns + include TaggableQueries BuildArchivedError = Class.new(StandardError) @@ -379,6 +380,33 @@ module Ci end end + def self.build_matchers(project) + unique_params = [ + :protected, + Arel.sql("(#{arel_tag_names_array.to_sql})") + ] + + group(*unique_params).pluck('array_agg(id)', *unique_params).map do |values| + Gitlab::Ci::Matching::BuildMatcher.new({ + build_ids: values[0], + protected: values[1], + tag_list: values[2], + project: project + }) + end + end + + def build_matcher + strong_memoize(:build_matcher) do + Gitlab::Ci::Matching::BuildMatcher.new({ + protected: protected?, + tag_list: tag_list, + build_ids: [id], + project: project + }) + end + end + def auto_retry_allowed? auto_retry.allowed? end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 8c877c2b818..04074eac383 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -10,6 +10,8 @@ module Ci include TokenAuthenticatable include IgnorableColumns include FeatureGate + include Gitlab::Utils::StrongMemoize + include TaggableQueries add_authentication_token_field :token, encrypted: -> { Feature.enabled?(:ci_runners_tokens_optional_encryption, default_enabled: true) ? :optional : :required } @@ -197,6 +199,42 @@ module Ci end end + def self.runner_matchers + unique_params = [ + :runner_type, + :public_projects_minutes_cost_factor, + :private_projects_minutes_cost_factor, + :run_untagged, + :access_level, + Arel.sql("(#{arel_tag_names_array.to_sql})") + ] + + # we use distinct to de-duplicate data + distinct.pluck(*unique_params).map do |values| + Gitlab::Ci::Matching::RunnerMatcher.new({ + runner_type: values[0], + public_projects_minutes_cost_factor: values[1], + private_projects_minutes_cost_factor: values[2], + run_untagged: values[3], + access_level: values[4], + tag_list: values[5] + }) + end + end + + def runner_matcher + strong_memoize(:runner_matcher) do + Gitlab::Ci::Matching::RunnerMatcher.new({ + runner_type: runner_type, + public_projects_minutes_cost_factor: public_projects_minutes_cost_factor, + private_projects_minutes_cost_factor: private_projects_minutes_cost_factor, + run_untagged: run_untagged, + access_level: access_level, + tag_list: tag_list + }) + end + end + def assign_to(project, current_user = nil) if instance_type? self.runner_type = :project_type diff --git a/app/models/concerns/packages/debian/distribution.rb b/app/models/concerns/packages/debian/distribution.rb index 267c7a4d201..ec69ff830d8 100644 --- a/app/models/concerns/packages/debian/distribution.rb +++ b/app/models/concerns/packages/debian/distribution.rb @@ -18,6 +18,10 @@ module Packages belongs_to container_type belongs_to :creator, class_name: 'User' + has_one :key, + class_name: "Packages::Debian::#{container_type.capitalize}DistributionKey", + foreign_key: :distribution_id, + inverse_of: :distribution # component_files must be destroyed by ruby code in order to properly remove carrierwave uploads has_many :components, class_name: "Packages::Debian::#{container_type.capitalize}Component", diff --git a/app/models/concerns/packages/debian/distribution_key.rb b/app/models/concerns/packages/debian/distribution_key.rb new file mode 100644 index 00000000000..7023e2dcd37 --- /dev/null +++ b/app/models/concerns/packages/debian/distribution_key.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Packages + module Debian + module DistributionKey + extend ActiveSupport::Concern + + included do + belongs_to :distribution, class_name: "Packages::Debian::#{container_type.capitalize}Distribution", inverse_of: :key + validates :distribution, + presence: true + + validates :private_key, presence: true, length: { maximum: 512.kilobytes } + validates :passphrase, presence: true, length: { maximum: 255 } + validates :public_key, presence: true, length: { maximum: 512.kilobytes } + validates :fingerprint, presence: true, length: { maximum: 255 } + + validate :private_key_armored, :public_key_armored + + attr_encrypted :private_key, + mode: :per_attribute_iv, + key: Settings.attr_encrypted_db_key_base_32, + algorithm: 'aes-256-gcm' + attr_encrypted :passphrase, + mode: :per_attribute_iv, + key: Settings.attr_encrypted_db_key_base_32, + algorithm: 'aes-256-gcm' + + private + + def private_key_armored + if private_key.present? && !private_key.start_with?('-----BEGIN PGP PRIVATE KEY BLOCK-----') + errors.add(:private_key, 'must be ASCII armored') + end + end + + def public_key_armored + if public_key.present? && !public_key.start_with?('-----BEGIN PGP PUBLIC KEY BLOCK-----') + errors.add(:public_key, 'must be ASCII armored') + end + end + end + end + end +end diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb index afebc426762..86280097d19 100644 --- a/app/models/concerns/prometheus_adapter.rb +++ b/app/models/concerns/prometheus_adapter.rb @@ -38,7 +38,7 @@ module PrometheusAdapter # This is a heavy-weight check if a prometheus is properly configured and accessible from GitLab. # This actually sends a request to an external service and often it could take a long time, - # Please consider using `configured?` instead if the process is running on unicorn/puma threads. + # Please consider using `configured?` instead if the process is running on Puma threads. def can_query? prometheus_client.present? end diff --git a/app/models/concerns/taggable_queries.rb b/app/models/concerns/taggable_queries.rb new file mode 100644 index 00000000000..2897e5e6420 --- /dev/null +++ b/app/models/concerns/taggable_queries.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module TaggableQueries + extend ActiveSupport::Concern + + class_methods do + # context is a name `acts_as_taggable context` + def arel_tag_names_array(context = :tags) + ActsAsTaggableOn::Tagging + .joins(:tag) + .where("taggings.taggable_id=#{quoted_table_name}.id") # rubocop:disable GitlabSecurity/SqlInjection + .where(taggings: { context: context, taggable_type: polymorphic_name }) + .select('COALESCE(array_agg(tags.name ORDER BY name), ARRAY[]::text[])') + end + end +end diff --git a/app/models/label_link.rb b/app/models/label_link.rb index a466fe69300..4fb5fd8c58a 100644 --- a/app/models/label_link.rb +++ b/app/models/label_link.rb @@ -11,5 +11,4 @@ class LabelLink < ApplicationRecord validates :label, presence: true, unless: :importing? scope :for_target, -> (target_id, target_type) { where(target_id: target_id, target_type: target_type) } - scope :with_remove_on_close_labels, -> { joins(:label).where(labels: { remove_on_close: true }) } end diff --git a/app/models/packages/debian/group_distribution_key.rb b/app/models/packages/debian/group_distribution_key.rb new file mode 100644 index 00000000000..a60ddca32e2 --- /dev/null +++ b/app/models/packages/debian/group_distribution_key.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class Packages::Debian::GroupDistributionKey < ApplicationRecord + def self.container_type + :group + end + + include Packages::Debian::DistributionKey +end diff --git a/app/models/packages/debian/project_distribution_key.rb b/app/models/packages/debian/project_distribution_key.rb new file mode 100644 index 00000000000..69cf2791b02 --- /dev/null +++ b/app/models/packages/debian/project_distribution_key.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class Packages::Debian::ProjectDistributionKey < ApplicationRecord + def self.container_type + :project + end + + include Packages::Debian::DistributionKey +end diff --git a/app/models/project.rb b/app/models/project.rb index 9d572b7e2f8..238be813c4b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -213,7 +213,6 @@ class Project < ApplicationRecord has_one :external_wiki_service has_one :prometheus_service, inverse_of: :project has_one :mock_ci_service - has_one :mock_deployment_service has_one :mock_monitoring_service has_one :microsoft_teams_service has_one :packagist_service diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 1700d1d8586..cc4ad1a9c85 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -25,7 +25,6 @@ module Issues end if project.issues_enabled? && issue.close(current_user) - remove_on_close_labels_from(issue) event_service.close_issue(issue, current_user) create_note(issue, closed_via) if system_note @@ -52,18 +51,6 @@ module Issues private - def remove_on_close_labels_from(issue) - old_labels = issue.labels.to_a - - issue.label_links.with_remove_on_close_labels.delete_all - issue.labels.reset - - Issuable::CommonSystemNotesService.new(project: project, current_user: current_user).execute( - issue, - old_labels: old_labels - ) - end - def close_external_issue(issue, closed_via) return unless project.external_issue_tracker&.support_close_issue? diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml index e7d8171d276..a1457688278 100644 --- a/app/views/dashboard/todos/_todo.html.haml +++ b/app/views/dashboard/todos/_todo.html.haml @@ -3,7 +3,7 @@ .todo-avatar.gl-display-none.gl-sm-display-inline-block = author_avatar(todo, size: 40) - .todo-item.gl-w-full.gl-align-self-center{ data: { qa_selector: "todo_item_container" } } + .todo-item.flex-fill.gl-overflow-auto.gl-align-self-center{ data: { qa_selector: "todo_item_container" } } .todo-title.gl-mb-3.gl-md-mb-0 - if todo_author_display?(todo) = todo_target_state_pill(todo) diff --git a/app/workers/jira_connect/sync_branch_worker.rb b/app/workers/jira_connect/sync_branch_worker.rb index b8211286d1c..4e8566d86c9 100644 --- a/app/workers/jira_connect/sync_branch_worker.rb +++ b/app/workers/jira_connect/sync_branch_worker.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module JiraConnect - class SyncBranchWorker # rubocop:disable Scalability/IdempotentWorker + class SyncBranchWorker include ApplicationWorker sidekiq_options retry: 3 diff --git a/app/workers/jira_connect/sync_merge_request_worker.rb b/app/workers/jira_connect/sync_merge_request_worker.rb index 6b3a6ae84ad..bf31df2271f 100644 --- a/app/workers/jira_connect/sync_merge_request_worker.rb +++ b/app/workers/jira_connect/sync_merge_request_worker.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module JiraConnect - class SyncMergeRequestWorker # rubocop:disable Scalability/IdempotentWorker + class SyncMergeRequestWorker include ApplicationWorker sidekiq_options retry: 3 diff --git a/changelogs/unreleased/17461-remove-labels-on-issue-close.yml b/changelogs/unreleased/17461-remove-labels-on-issue-close.yml deleted file mode 100644 index 3f6f401ce5e..00000000000 --- a/changelogs/unreleased/17461-remove-labels-on-issue-close.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Add option to remove labels on issue close in the REST and GraphQL API -merge_request: 61286 -author: -type: added diff --git a/changelogs/unreleased/331057-follow-up-from-audit-and-update-buttons-on-projects-tagscontroller.yml b/changelogs/unreleased/331057-follow-up-from-audit-and-update-buttons-on-projects-tagscontroller.yml new file mode 100644 index 00000000000..5614bccc788 --- /dev/null +++ b/changelogs/unreleased/331057-follow-up-from-audit-and-update-buttons-on-projects-tagscontroller.yml @@ -0,0 +1,5 @@ +--- +title: Left-align certain application-wide cancel buttons to conform to the GitLab Pajamas style guide +merge_request: 61858 +author: +type: changed diff --git a/changelogs/unreleased/debian_distribution_keys.yml b/changelogs/unreleased/debian_distribution_keys.yml new file mode 100644 index 00000000000..fdc4ae45a4c --- /dev/null +++ b/changelogs/unreleased/debian_distribution_keys.yml @@ -0,0 +1,5 @@ +--- +title: Debian Group and Project Distribution Keys schema +merge_request: 60993 +author: Mathieu Parent +type: added diff --git a/changelogs/unreleased/issue-220040-fix-robocop-savebang-spec-models-1.yml b/changelogs/unreleased/issue-220040-fix-robocop-savebang-spec-models-1.yml new file mode 100644 index 00000000000..8ecada2a9d0 --- /dev/null +++ b/changelogs/unreleased/issue-220040-fix-robocop-savebang-spec-models-1.yml @@ -0,0 +1,5 @@ +--- +title: Fixed Rails Save Bang offenses in few spec/models/* files +merge_request: 61961 +author: Suraj Tripathi @surajtripathy07 +type: fixed diff --git a/config/initializers/macos.rb b/config/initializers/macos.rb index f410af6ed47..1edd6c0a730 100644 --- a/config/initializers/macos.rb +++ b/config/initializers/macos.rb @@ -5,7 +5,7 @@ if /darwin/ =~ RUBY_PLATFORM require 'fiddle' # Dynamically load Foundation.framework, ~implicitly~ initialising - # the Objective-C runtime before any forking happens in Unicorn + # the Objective-C runtime before any forking happens in webserver # # From https://bugs.ruby-lang.org/issues/14009 Fiddle.dlopen '/System/Library/Frameworks/Foundation.framework/Foundation' diff --git a/config/initializers/rack_timeout.rb b/config/initializers/rack_timeout.rb index e217398ee7d..d5027cae08d 100644 --- a/config/initializers/rack_timeout.rb +++ b/config/initializers/rack_timeout.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# Unicorn terminates any request which runs longer than 60 seconds. # Puma doesn't have any timeout mechanism for terminating long-running # requests, to make sure that server is not paralyzed by long-running # or stuck queries, we add a request timeout which terminates the diff --git a/config/initializers/tracing.rb b/config/initializers/tracing.rb index 3f066953c23..10b59a9e3d8 100644 --- a/config/initializers/tracing.rb +++ b/config/initializers/tracing.rb @@ -15,7 +15,7 @@ if Labkit::Tracing.enabled? Labkit::Tracing::Rails::ActionViewSubscriber.instrument Labkit::Tracing::Rails::ActiveSupportSubscriber.instrument - # In multi-processed clustered architectures (puma, unicorn) don't + # In multi-processed clustered architectures (e.g. Puma cluster) don't # start tracing until the worker processes are spawned. This works # around issues when the opentracing implementation spawns threads Gitlab::Cluster::LifecycleEvents.on_worker_start do diff --git a/db/migrate/20210505070612_create_packages_debian_group_distribution_keys.rb b/db/migrate/20210505070612_create_packages_debian_group_distribution_keys.rb new file mode 100644 index 00000000000..1e79b4e29b0 --- /dev/null +++ b/db/migrate/20210505070612_create_packages_debian_group_distribution_keys.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class CreatePackagesDebianGroupDistributionKeys < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + INDEX_DISTRIBUTION = 'idx_pkgs_debian_group_distribution_keys_on_distribution_id' + + disable_ddl_transaction! + + def up + create_table_with_constraints :packages_debian_group_distribution_keys do |t| + t.timestamps_with_timezone + t.references :distribution, + foreign_key: { to_table: :packages_debian_group_distributions, on_delete: :cascade }, + index: { name: INDEX_DISTRIBUTION }, + null: false + + t.text :encrypted_private_key, null: false + t.text :encrypted_private_key_iv, null: false + t.text :encrypted_passphrase, null: false + t.text :encrypted_passphrase_iv, null: false + t.text :public_key, null: false + t.text :fingerprint, null: false + + t.text_limit :public_key, 512.kilobytes + t.text_limit :fingerprint, 255 + end + end + + def down + with_lock_retries do + drop_table :packages_debian_group_distribution_keys + end + end +end diff --git a/db/migrate/20210505070812_create_packages_debian_project_distribution_keys.rb b/db/migrate/20210505070812_create_packages_debian_project_distribution_keys.rb new file mode 100644 index 00000000000..f5ec01ec0d6 --- /dev/null +++ b/db/migrate/20210505070812_create_packages_debian_project_distribution_keys.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class CreatePackagesDebianProjectDistributionKeys < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + INDEX_DISTRIBUTION = 'idx_pkgs_debian_project_distribution_keys_on_distribution_id' + + disable_ddl_transaction! + + def up + create_table_with_constraints :packages_debian_project_distribution_keys do |t| + t.timestamps_with_timezone + t.references :distribution, + foreign_key: { to_table: :packages_debian_project_distributions, on_delete: :cascade }, + index: { name: INDEX_DISTRIBUTION }, + null: false + + t.text :encrypted_private_key, null: false + t.text :encrypted_private_key_iv, null: false + t.text :encrypted_passphrase, null: false + t.text :encrypted_passphrase_iv, null: false + t.text :public_key, null: false + t.text :fingerprint, null: false + + t.text_limit :public_key, 512.kilobytes + t.text_limit :fingerprint, 255 + end + end + + def down + with_lock_retries do + drop_table :packages_debian_project_distribution_keys + end + end +end diff --git a/db/migrate/20210507191949_add_remove_on_issue_close_to_labels.rb b/db/migrate/20210507191949_add_remove_on_issue_close_to_labels.rb index 107388fe533..131edb20164 100644 --- a/db/migrate/20210507191949_add_remove_on_issue_close_to_labels.rb +++ b/db/migrate/20210507191949_add_remove_on_issue_close_to_labels.rb @@ -1,17 +1,11 @@ # frozen_string_literal: true class AddRemoveOnIssueCloseToLabels < ActiveRecord::Migration[6.0] - include Gitlab::Database::MigrationHelpers + # This migration was reverted in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62056 def up - with_lock_retries do - add_column :labels, :remove_on_close, :boolean, null: false, default: false - end end def down - with_lock_retries do - remove_column :labels, :remove_on_close, :boolean - end end end diff --git a/db/schema_migrations/20210505070612 b/db/schema_migrations/20210505070612 new file mode 100644 index 00000000000..454634cb74b --- /dev/null +++ b/db/schema_migrations/20210505070612 @@ -0,0 +1 @@ +52cc795e577a6de524cc55ce8d11f140e5d919f1164bb9983f7dd2c1ef2f0859
\ No newline at end of file diff --git a/db/schema_migrations/20210505070812 b/db/schema_migrations/20210505070812 new file mode 100644 index 00000000000..ca53caa49af --- /dev/null +++ b/db/schema_migrations/20210505070812 @@ -0,0 +1 @@ +7d57e1fea3652c0c04d29261d3c21b314ed443c9e61b882d22ca7f59807c312b
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 5d33a61e942..0d36b68edec 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14280,8 +14280,7 @@ CREATE TABLE labels ( description_html text, type character varying, group_id integer, - cached_markdown_version integer, - remove_on_close boolean DEFAULT false NOT NULL + cached_markdown_version integer ); CREATE SEQUENCE labels_id_seq @@ -15538,6 +15537,30 @@ CREATE SEQUENCE packages_debian_group_components_id_seq ALTER SEQUENCE packages_debian_group_components_id_seq OWNED BY packages_debian_group_components.id; +CREATE TABLE packages_debian_group_distribution_keys ( + id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + distribution_id bigint NOT NULL, + encrypted_private_key text NOT NULL, + encrypted_private_key_iv text NOT NULL, + encrypted_passphrase text NOT NULL, + encrypted_passphrase_iv text NOT NULL, + public_key text NOT NULL, + fingerprint text NOT NULL, + CONSTRAINT check_bc95dc3fbe CHECK ((char_length(fingerprint) <= 255)), + CONSTRAINT check_f708183491 CHECK ((char_length(public_key) <= 524288)) +); + +CREATE SEQUENCE packages_debian_group_distribution_keys_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE packages_debian_group_distribution_keys_id_seq OWNED BY packages_debian_group_distribution_keys.id; + CREATE TABLE packages_debian_group_distributions ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, @@ -15640,6 +15663,30 @@ CREATE SEQUENCE packages_debian_project_components_id_seq ALTER SEQUENCE packages_debian_project_components_id_seq OWNED BY packages_debian_project_components.id; +CREATE TABLE packages_debian_project_distribution_keys ( + id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + distribution_id bigint NOT NULL, + encrypted_private_key text NOT NULL, + encrypted_private_key_iv text NOT NULL, + encrypted_passphrase text NOT NULL, + encrypted_passphrase_iv text NOT NULL, + public_key text NOT NULL, + fingerprint text NOT NULL, + CONSTRAINT check_9e8a5eef0a CHECK ((char_length(fingerprint) <= 255)), + CONSTRAINT check_d188f6547f CHECK ((char_length(public_key) <= 524288)) +); + +CREATE SEQUENCE packages_debian_project_distribution_keys_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE packages_debian_project_distribution_keys_id_seq OWNED BY packages_debian_project_distribution_keys.id; + CREATE TABLE packages_debian_project_distributions ( id bigint NOT NULL, created_at timestamp with time zone NOT NULL, @@ -19951,6 +19998,8 @@ ALTER TABLE ONLY packages_debian_group_component_files ALTER COLUMN id SET DEFAU ALTER TABLE ONLY packages_debian_group_components ALTER COLUMN id SET DEFAULT nextval('packages_debian_group_components_id_seq'::regclass); +ALTER TABLE ONLY packages_debian_group_distribution_keys ALTER COLUMN id SET DEFAULT nextval('packages_debian_group_distribution_keys_id_seq'::regclass); + ALTER TABLE ONLY packages_debian_group_distributions ALTER COLUMN id SET DEFAULT nextval('packages_debian_group_distributions_id_seq'::regclass); ALTER TABLE ONLY packages_debian_project_architectures ALTER COLUMN id SET DEFAULT nextval('packages_debian_project_architectures_id_seq'::regclass); @@ -19959,6 +20008,8 @@ ALTER TABLE ONLY packages_debian_project_component_files ALTER COLUMN id SET DEF ALTER TABLE ONLY packages_debian_project_components ALTER COLUMN id SET DEFAULT nextval('packages_debian_project_components_id_seq'::regclass); +ALTER TABLE ONLY packages_debian_project_distribution_keys ALTER COLUMN id SET DEFAULT nextval('packages_debian_project_distribution_keys_id_seq'::regclass); + ALTER TABLE ONLY packages_debian_project_distributions ALTER COLUMN id SET DEFAULT nextval('packages_debian_project_distributions_id_seq'::regclass); ALTER TABLE ONLY packages_debian_publications ALTER COLUMN id SET DEFAULT nextval('packages_debian_publications_id_seq'::regclass); @@ -21423,6 +21474,9 @@ ALTER TABLE ONLY packages_debian_group_component_files ALTER TABLE ONLY packages_debian_group_components ADD CONSTRAINT packages_debian_group_components_pkey PRIMARY KEY (id); +ALTER TABLE ONLY packages_debian_group_distribution_keys + ADD CONSTRAINT packages_debian_group_distribution_keys_pkey PRIMARY KEY (id); + ALTER TABLE ONLY packages_debian_group_distributions ADD CONSTRAINT packages_debian_group_distributions_pkey PRIMARY KEY (id); @@ -21435,6 +21489,9 @@ ALTER TABLE ONLY packages_debian_project_component_files ALTER TABLE ONLY packages_debian_project_components ADD CONSTRAINT packages_debian_project_components_pkey PRIMARY KEY (id); +ALTER TABLE ONLY packages_debian_project_distribution_keys + ADD CONSTRAINT packages_debian_project_distribution_keys_pkey PRIMARY KEY (id); + ALTER TABLE ONLY packages_debian_project_distributions ADD CONSTRAINT packages_debian_project_distributions_pkey PRIMARY KEY (id); @@ -22212,6 +22269,10 @@ CREATE INDEX idx_packages_debian_project_component_files_on_architecture_id ON p CREATE INDEX idx_packages_packages_on_project_id_name_version_package_type ON packages_packages USING btree (project_id, name, version, package_type); +CREATE INDEX idx_pkgs_debian_group_distribution_keys_on_distribution_id ON packages_debian_group_distribution_keys USING btree (distribution_id); + +CREATE INDEX idx_pkgs_debian_project_distribution_keys_on_distribution_id ON packages_debian_project_distribution_keys USING btree (distribution_id); + CREATE UNIQUE INDEX idx_pkgs_dep_links_on_pkg_id_dependency_id_dependency_type ON packages_dependency_links USING btree (package_id, dependency_id, dependency_type); CREATE INDEX idx_proj_feat_usg_on_jira_dvcs_cloud_last_sync_at_and_proj_id ON project_feature_usages USING btree (jira_dvcs_cloud_last_sync_at, project_id) WHERE (jira_dvcs_cloud_last_sync_at IS NOT NULL); @@ -26272,6 +26333,9 @@ ALTER TABLE ONLY group_merge_request_approval_settings ALTER TABLE ONLY analytics_cycle_analytics_project_stages ADD CONSTRAINT fk_rails_3829e49b66 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY packages_debian_project_distribution_keys + ADD CONSTRAINT fk_rails_3834a11264 FOREIGN KEY (distribution_id) REFERENCES packages_debian_project_distributions(id) ON DELETE CASCADE; + ALTER TABLE ONLY issue_user_mentions ADD CONSTRAINT fk_rails_3861d9fefa FOREIGN KEY (note_id) REFERENCES notes(id) ON DELETE CASCADE; @@ -26680,6 +26744,9 @@ ALTER TABLE ONLY packages_debian_publications ALTER TABLE ONLY boards_epic_user_preferences ADD CONSTRAINT fk_rails_76c4e9732d FOREIGN KEY (epic_id) REFERENCES epics(id) ON DELETE CASCADE; +ALTER TABLE ONLY packages_debian_group_distribution_keys + ADD CONSTRAINT fk_rails_779438f163 FOREIGN KEY (distribution_id) REFERENCES packages_debian_group_distributions(id) ON DELETE CASCADE; + ALTER TABLE ONLY ci_subscriptions_projects ADD CONSTRAINT fk_rails_7871f9a97b FOREIGN KEY (upstream_project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 792e10e85b7..b86c4ac9352 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2700,7 +2700,6 @@ Input type: `LabelCreateInput` | <a id="mutationlabelcreatedescription"></a>`description` | [`String`](#string) | Description of the label. | | <a id="mutationlabelcreategrouppath"></a>`groupPath` | [`ID`](#id) | Full path of the group with which the resource is associated. | | <a id="mutationlabelcreateprojectpath"></a>`projectPath` | [`ID`](#id) | Full path of the project with which the resource is associated. | -| <a id="mutationlabelcreateremoveonclose"></a>`removeOnClose` | [`Boolean`](#boolean) | Whether the label should be removed from an issue when the issue is closed. | | <a id="mutationlabelcreatetitle"></a>`title` | [`String!`](#string) | Title of the label. | #### Fields @@ -5200,6 +5199,29 @@ The edge type for [`EpicList`](#epiclist). | <a id="epiclistedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. | | <a id="epiclistedgenode"></a>`node` | [`EpicList`](#epiclist) | The item at the end of the edge. | +#### `EscalationPolicyTypeConnection` + +The connection type for [`EscalationPolicyType`](#escalationpolicytype). + +##### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="escalationpolicytypeconnectionedges"></a>`edges` | [`[EscalationPolicyTypeEdge]`](#escalationpolicytypeedge) | A list of edges. | +| <a id="escalationpolicytypeconnectionnodes"></a>`nodes` | [`[EscalationPolicyType]`](#escalationpolicytype) | A list of nodes. | +| <a id="escalationpolicytypeconnectionpageinfo"></a>`pageInfo` | [`PageInfo!`](#pageinfo) | Information to aid in pagination. | + +#### `EscalationPolicyTypeEdge` + +The edge type for [`EscalationPolicyType`](#escalationpolicytype). + +##### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="escalationpolicytypeedgecursor"></a>`cursor` | [`String!`](#string) | A cursor for use in pagination. | +| <a id="escalationpolicytypeedgenode"></a>`node` | [`EscalationPolicyType`](#escalationpolicytype) | The item at the end of the edge. | + #### `EventConnection` The connection type for [`Event`](#event). @@ -8610,6 +8632,32 @@ Check permissions for the current user on an epic. | <a id="epicpermissionsreadepiciid"></a>`readEpicIid` | [`Boolean!`](#boolean) | Indicates the user can perform `read_epic_iid` on this resource. | | <a id="epicpermissionsupdateepic"></a>`updateEpic` | [`Boolean!`](#boolean) | Indicates the user can perform `update_epic` on this resource. | +### `EscalationPolicyType` + +Represents an escalation policy. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="escalationpolicytypedescription"></a>`description` | [`String`](#string) | The description of the escalation policy. | +| <a id="escalationpolicytypeid"></a>`id` | [`IncidentManagementEscalationPolicyID`](#incidentmanagementescalationpolicyid) | ID of the escalation policy. | +| <a id="escalationpolicytypename"></a>`name` | [`String`](#string) | The name of the escalation policy. | +| <a id="escalationpolicytyperules"></a>`rules` | [`[EscalationRuleType!]`](#escalationruletype) | Steps of the escalation policy. | + +### `EscalationRuleType` + +Represents an escalation rule for an escalation policy. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="escalationruletypeelapsedtimeseconds"></a>`elapsedTimeSeconds` | [`Int`](#int) | The time in seconds before the rule is activated. | +| <a id="escalationruletypeid"></a>`id` | [`IncidentManagementEscalationRuleID`](#incidentmanagementescalationruleid) | ID of the escalation policy. | +| <a id="escalationruletypeoncallschedule"></a>`oncallSchedule` | [`IncidentManagementOncallSchedule`](#incidentmanagementoncallschedule) | The on-call schedule to notify. | +| <a id="escalationruletypestatus"></a>`status` | [`EscalationRuleStatus`](#escalationrulestatus) | The status required to prevent the rule from activating. | + ### `Event` Representing an event. @@ -9743,7 +9791,6 @@ four standard [pagination arguments](#connection-pagination-arguments): | <a id="labeldescription"></a>`description` | [`String`](#string) | Description of the label (Markdown rendered as HTML for caching). | | <a id="labeldescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. | | <a id="labelid"></a>`id` | [`ID!`](#id) | Label ID. | -| <a id="labelremoveonclose"></a>`removeOnClose` | [`Boolean!`](#boolean) | Whether the label should be removed from an issue when the issue is closed. | | <a id="labeltextcolor"></a>`textColor` | [`String!`](#string) | Text color of the label. | | <a id="labeltitle"></a>`title` | [`String!`](#string) | Content of the label. | | <a id="labelupdatedat"></a>`updatedAt` | [`Time!`](#time) | When this label was last updated. | @@ -10911,6 +10958,7 @@ Represents vulnerability finding of a security report on the pipeline. | <a id="projecthttpurltorepo"></a>`httpUrlToRepo` | [`String`](#string) | URL to connect to the project via HTTPS. | | <a id="projectid"></a>`id` | [`ID!`](#id) | ID of the project. | | <a id="projectimportstatus"></a>`importStatus` | [`String`](#string) | Status of import background job of the project. | +| <a id="projectincidentmanagementescalationpolicies"></a>`incidentManagementEscalationPolicies` | [`EscalationPolicyTypeConnection`](#escalationpolicytypeconnection) | Incident Management escalation policies of the project. (see [Connections](#connections)) | | <a id="projectissuesenabled"></a>`issuesEnabled` | [`Boolean`](#boolean) | Indicates if Issues are enabled for the current user. | | <a id="projectjiraimportstatus"></a>`jiraImportStatus` | [`String`](#string) | Status of Jira import background job of the project. | | <a id="projectjiraimports"></a>`jiraImports` | [`JiraImportConnection`](#jiraimportconnection) | Jira imports into the project. (see [Connections](#connections)) | @@ -11184,6 +11232,18 @@ four standard [pagination arguments](#connection-pagination-arguments): | <a id="projectenvironmentssearch"></a>`search` | [`String`](#string) | Search query for environment name. | | <a id="projectenvironmentsstates"></a>`states` | [`[String!]`](#string) | States of environments that should be included in result. | +##### `Project.incidentManagementEscalationPolicy` + +Incident Management escalation policy of the project. + +Returns [`EscalationPolicyType`](#escalationpolicytype). + +###### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="projectincidentmanagementescalationpolicyid"></a>`id` | [`IncidentManagementEscalationPolicyID!`](#incidentmanagementescalationpolicyid) | ID of the escalation policy. | + ##### `Project.incidentManagementOncallSchedules` Incident Management On-call schedules of the project. @@ -13968,6 +14028,15 @@ Epic ID wildcard values. | <a id="epicwildcardidany"></a>`ANY` | Any epic is assigned. | | <a id="epicwildcardidnone"></a>`NONE` | No epic is assigned. | +### `EscalationRuleStatus` + +Escalation rule statuses. + +| Value | Description | +| ----- | ----------- | +| <a id="escalationrulestatusacknowledged"></a>`ACKNOWLEDGED` | . | +| <a id="escalationrulestatusresolved"></a>`RESOLVED` | . | + ### `EventAction` Event action. @@ -15015,6 +15084,18 @@ Represents a unique identifier that is Base64 obfuscated. It is often used to re An ISO 8601-encoded date. +### `IncidentManagementEscalationPolicyID` + +A `IncidentManagementEscalationPolicyID` is a global ID. It is encoded as a string. + +An example `IncidentManagementEscalationPolicyID` is: `"gid://gitlab/IncidentManagement::EscalationPolicy/1"`. + +### `IncidentManagementEscalationRuleID` + +A `IncidentManagementEscalationRuleID` is a global ID. It is encoded as a string. + +An example `IncidentManagementEscalationRuleID` is: `"gid://gitlab/IncidentManagement::EscalationRule/1"`. + ### `IncidentManagementOncallParticipantID` A `IncidentManagementOncallParticipantID` is a global ID. It is encoded as a string. diff --git a/doc/api/labels.md b/doc/api/labels.md index a9f2698a270..963d320a384 100644 --- a/doc/api/labels.md +++ b/doc/api/labels.md @@ -46,8 +46,7 @@ Example response: "open_merge_requests_count": 1, "subscribed": false, "priority": 10, - "is_project_label": true, - "remove_on_close": false + "is_project_label": true }, { "id" : 4, @@ -61,8 +60,7 @@ Example response: "open_merge_requests_count": 0, "subscribed": false, "priority": null, - "is_project_label": true, - "remove_on_close": false + "is_project_label": true }, { "id" : 7, @@ -76,8 +74,7 @@ Example response: "open_merge_requests_count": 1, "subscribed": false, "priority": null, - "is_project_label": true, - "remove_on_close": true + "is_project_label": true }, { "id" : 8, @@ -91,8 +88,7 @@ Example response: "open_merge_requests_count": 2, "subscribed": false, "priority": null, - "is_project_label": false, - "remove_on_close": false + "is_project_label": false }, { "id" : 9, @@ -106,8 +102,7 @@ Example response: "open_merge_requests_count": 1, "subscribed": true, "priority": null, - "is_project_label": true, - "remove_on_close": false + "is_project_label": true } ] ``` @@ -145,8 +140,7 @@ Example response: "open_merge_requests_count": 1, "subscribed": false, "priority": 10, - "is_project_label": true, - "remove_on_close": true + "is_project_label": true } ``` @@ -165,7 +159,6 @@ POST /projects/:id/labels | `color` | string | yes | The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the [CSS color names](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords) | | `description` | string | no | The description of the label | | `priority` | integer | no | The priority of the label. Must be greater or equal than zero or `null` to remove the priority. | -| `remove_on_close` | boolean | no | Whether the label should be removed from an issue when the issue is closed. _([Introduced in GitLab 13.12](https://gitlab.com/gitlab-org/gitlab/-/issues/17461))_ | ```shell curl --data "name=feature&color=#5843AD" --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/labels" @@ -186,8 +179,7 @@ Example response: "open_merge_requests_count": 0, "subscribed": false, "priority": null, - "is_project_label": true, - "remove_on_close": true + "is_project_label": true } ``` @@ -228,7 +220,6 @@ PUT /projects/:id/labels/:label_id | `color` | string | yes if `new_name` is not provided | The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the [CSS color names](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords) | | `description` | string | no | The new description of the label | | `priority` | integer | no | The new priority of the label. Must be greater or equal than zero or `null` to remove the priority. | -| `remove_on_close` | boolean | no | Boolean option specifying whether the label should be removed from issues when they are closed. | ```shell curl --request PUT --data "new_name=docs&color=#8E44AD&description=Documentation" --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/labels/documentation" @@ -249,8 +240,7 @@ Example response: "open_merge_requests_count": 2, "subscribed": false, "priority": null, - "is_project_label": true, - "remove_on_close": true + "is_project_label": true } ``` @@ -291,8 +281,7 @@ Example response: "open_issues_count": 1, "closed_issues_count": 0, "open_merge_requests_count": 2, - "subscribed": false, - "remove_on_close": true + "subscribed": false } ``` @@ -333,8 +322,7 @@ Example response: "open_merge_requests_count": 1, "subscribed": true, "priority": null, - "is_project_label": true, - "remove_on_close": true + "is_project_label": true } ``` diff --git a/doc/architecture/blueprints/composable_codebase_using_rails_engines/index.md b/doc/architecture/blueprints/composable_codebase_using_rails_engines/index.md new file mode 100644 index 00000000000..69661aa2be0 --- /dev/null +++ b/doc/architecture/blueprints/composable_codebase_using_rails_engines/index.md @@ -0,0 +1,614 @@ +--- +stage: none +group: unassigned +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +comments: false +description: 'Making a GitLab codebase composable - allowing to run parts of the application' +--- + +# Composable GitLab codebase - using Rails Engines + +The one of the major risks of a single codebase is an infinite growth of the whole +application. The more code being added results in not only ever increasing resource requirements +for running the application, but increased application coupling and explosion of the complexity. + +## Executive summary + +This blueprint discusses an impact of introducing **Application Layers** as a way to reduce and improve the application +codebase. This discusses the positive and negative outcomes of the proposed solution and tries to estimate the impact +on GitLab.com and smaller installations. + +**Application Layers** tries to split GitLab Rails codebase horizontally following the pattern of how we actually +run GitLab instead of vertical split. This follows the idea that a single feature needs to run in many different ways +(CI for example has Web interface, uses API, and performs background processing), and we are not able to easily +run only a given feature separate to the rest application (like CI) due to coupling. + +The proposal itself does allow us to disconnect some aspects of the features. These aspects could be treated +as components that are run separately from the rest of the stack, but still sharing a large portion of core. +This model could be implemented to provide an API interface for external tooling (Runners API, Packages API, Feature Flags Unleash API) +and allow us to have much better resiliency and much easier way to scale application in the future. + +The actual split was tested with the usage of [Rails Engines](https://guides.rubyonrails.org/engines.html) +implementing separate gems in a single repository. The [Rails Engines](https://guides.rubyonrails.org/engines.html) +allowed us to well describe the indivdual components with its dependencies and run an application +consisting of many Rails Engines. + +The blueprint aims to retain all key aspects of GitLab success: single and monolithic codebase (with a [single data-store](https://about.gitlab.com/handbook/product/single-application/#single-data-store)), +but allows us to better model application and make our codebase more composable. + +## Challenges of the Monolith (a current state) + +Today the usage of monolith proves to be challenging in many cases. A single big monolith +codebase without clear boundaries results in a number of problems and inefficiencies, some of them being: + +- Deep coupling makes application harder to develop in longer term, as it leads to a spaghetti implementation + instead of considering building more interface-based architecture +- Deep coupling between parts of the codebase making it harder to test. To test only a small portion of application + we usually need to run a whole test suite to confidently know which parts are affected. This to + some extent can be improved by building a heuristic to aid this process, but it is prone to errors and hard + to keep accurate at all times +- All components need to be loaded at all times in order to run only parts of the application +- Increased resource usage, as we load parts of the application that are rarely used in a given context +- The high memory usage results in slowing the whole application as it increases GC cycles duration + creating significantly longer latency for processing requests or worse cache usage of CPUs +- Increased application boot-up times as we need to load and parse significantly more files +- Longer boot-up times slows down the development, as running application or tests takes singificantly longer + reducing velocity and amount of iterations + +## Composable codebase dimensions + +In general, we can think about two ways how codebase can be modeled: + +- **vertically** in Bounded Contexts, each representing a domain of the application, ex.: All features related to CI are in a given context +- **horizontally** in Application Layers: Sidekiq, GraphQL, REST API, Web Controllers, all Domain Models and Services that interface with DB directly + +This blueprint explicitly talks about **horizontal** split and **Application Layers**. + +## Current state of Bounded Contexts (**vertical** split) + +The Bounded Contexts is a topic that was discussed extensively number of times for a couple of years. +Reflected in number of issues: + +- [Create new models / classes within a module / namespace](https://gitlab.com/gitlab-org/gitlab/-/issues/212156) +- [Make teams to be maintainers of their code](https://gitlab.com/gitlab-org/gitlab/-/issues/25872) +- [Use nested structure to organize CI classes](https://gitlab.com/gitlab-org/gitlab/-/issues/209745) +- [WIP: Make it simple to build and use "Decoupled Services"](https://gitlab.com/gitlab-org/gitlab/-/issues/31121) + +We are partially executing a **Bounded Contexts** idea: + +- Make each team to own their own namespace, namespace which is defined as a `module` in a codebase +- Make each team to own their own tests, as namespaces would define a clear boundaries +- Since we use namespaces, individual contributor or reviewer can know who to reach from domain experts about help with + the given context + +The module namespaces are actively being used today to model codebase around team boundaries. Currently, the most +prominent namespaces being used today are `Ci::` and `Packages::`. They provide a good way to contain the code owned +by a group in a well-defined structure. + +However, the **Bounded Contexts** while it helps development, it does not help with the above stated goals. This is purely +a logical split of the code. This does not prevent deep-coupling. It is still possible to create a circular dependency (and it often happens) +between a background processing of CI pipeline and Runner API interface. +API can call Sidekiq Worker, Sidekiq can use API to create an endpoint path. + +The **Bounded Contexts** do not make our codebase smarter to know what depends on what, as the whole codebase +is treated as single package that needs to be loaded and executed. + +Possible additional considerations to the disadvantages of Bounded Context: + +- It can lead to tribal knowledge and duplicate code +- The deep coupling can make it difficult to iterate and make minimal changes +- Changes may have cascading effects that are difficult to isolate due to the vertical split + +## The Application Layers (**horizontal* split) + +While we continue leveraging **Bounded Contexts** in form of namespace separation that aids development and review process +the **Application Layers** can provide a way to create a clean separation between different functional parts. + +Our main codebase (`GitLab Rails` after a GitLab running on Ruby on Rails) consists many of implicit **Application Layers**. +There are no clear boundaries between each layer which results in a deep coupling. + +The concept of **Application Layers** looks at the application from the perspective of how we run the application +instead of perspective of individual features (like CI or Packages). GitLab application today can be decomposed into the following +application layers. This list is not exhaustive, but shows a general list of the different parts of a single monolithic codebase: + +- Web Controllers: process Web requests coming from users visiting web interface +- Web API: API calls coming from the automated tooling, in some cases also users visiting web interface +- Web Runners API: API calls from the Runners, that allows Runner to fetch new jobs, or update trace log +- Web GraphQL: provide a flexible API interface, allowing the Web frontend to fetch only the data needed thereby reducing the amount of compute and data transfer +- Web ActionCable: provide bi-directional connection to enable real-time features for Users visiting web interface +- Web Feature Flags Unleash Backend: provide an Unleash-compatible Server that uses GitLab API +- Web Packages API: provide a REST API compatible with the packaging tools: Debian, Maven, Container Registry Proxy, etc. +- Git nodes: all code required to authorize `git pull/push` over `SSH` or `HTTPS` +- Sidekiq: run background jobs +- Services/Models/DB: all code required to maintain our database structure, data validation, business logic and policies models that needs to be shared with other components + +The best way to likely describe how the actual GitLab Rails split would look like. It is a satellite model. +Where we have a single core, that is shared across all satelitte components. The design of that implies +that satelitte components have a limited way to communicate with each other. In a single monolithic application +in most of cases application would communicate with a code. In a satellite model the communication needs +to be performed externally to the component. This can be via Database, Redis or using a well defined exposed API. + +```mermaid +flowchart TD + subgraph Data Store + D[Database] + R[Redis] + end + subgraph Rails Engines + subgraph Data Access Layer + C[Core] + end + subgraph Web Processing + W[Web] + end + subgraph Background Processing + S[Sidekiq] + end + end + C --> D & R + W & S -- using application models --> C + R -- push background job --> S + W -- via async schedule --> S + S -- via Web API --> W +``` + +### Application Layers for on-premise installations + +The on-premise installations are significantly smaller and they usually run GitLab Rails in two main flavors: + +```mermaid +graph LR + gitlab_node[GitLab Node with Load Balancer] + + gitlab_node_web[Web running Puma] + gitlab_node_sidekiq[Background jobs running Sidekiq] + gitlab_node_git[Git running Puma and SSH] + + subgraph GitLab Rails + gitlab_rails_web_controllers[Controllers] + gitlab_rails_api[API] + gitlab_rails_api_runners[API Runner] + gitlab_rails_graphql[GraphQL] + gitlab_rails_actioncable[ActionCable] + gitlab_rails_services[Services] + gitlab_rails_models[Models] + gitlab_rails_sidekiq[Sidekiq Workers] + end + + postgresql_db[(PostgreSQL Database)] + redis_db[(Redis Database)] + + gitlab_node --> gitlab_node_web + gitlab_node --> gitlab_node_sidekiq + gitlab_node --> gitlab_node_git + + gitlab_node_web --> gitlab_rails_web_controllers + gitlab_node_web --> gitlab_rails_api + gitlab_node_web --> gitlab_rails_api_runners + gitlab_node_web --> gitlab_rails_graphql + gitlab_node_web --> gitlab_rails_actioncable + gitlab_node_git --> gitlab_rails_api + gitlab_node_sidekiq --> gitlab_rails_sidekiq + + gitlab_rails_web_controllers --> gitlab_rails_services + gitlab_rails_api --> gitlab_rails_services + gitlab_rails_api_runners --> gitlab_rails_services + gitlab_rails_graphql --> gitlab_rails_services + gitlab_rails_actioncable --> gitlab_rails_services + gitlab_rails_sidekiq --> gitlab_rails_services + + gitlab_rails_services --> gitlab_rails_models + + gitlab_rails_models --> postgresql_db + gitlab_rails_models --> redis_db +``` + +### Application Layers on GitLab.com + +Due to its scale, GitLab.com requires much more attention to run. This is needed in order to better manage resources +and provide SLAs for different functional parts. The chart below provides a simplistic view of GitLab.com application layers. +It does not include all components, like Object Storage nor Gitaly nodes, but shows the GitLab Rails dependencies between +different components and how they are configured on GitLab.com today: + +```mermaid +graph LR + gitlab_com_lb[GitLab.com Load Balancer] + + gitlab_com_web[Web Nodes running Puma] + gitlab_com_api[API Nodes running Puma] + gitlab_com_websockets[WebSockets Nodes running Puma] + gitlab_com_sidekiq[Background Jobs running Sidekiq] + gitlab_com_git[Git Nodes running Puma and SSH] + + subgraph GitLab Rails + gitlab_rails_web_controllers[Controllers] + gitlab_rails_api[API] + gitlab_rails_api_runners[API Runner] + gitlab_rails_graphql[GraphQL] + gitlab_rails_actioncable[ActionCable] + gitlab_rails_services[Services] + gitlab_rails_models[Models] + gitlab_rails_sidekiq[Sidekiq Workers] + end + + postgresql_db[(PostgreSQL Database)] + redis_db[(Redis Database)] + + gitlab_com_lb --> gitlab_com_web + gitlab_com_lb --> gitlab_com_api + gitlab_com_lb --> gitlab_com_websockets + gitlab_com_lb --> gitlab_com_git + + gitlab_com_web --> gitlab_rails_web_controllers + gitlab_com_api --> gitlab_rails_api + gitlab_com_api --> gitlab_rails_api_runners + gitlab_com_api --> gitlab_rails_graphql + gitlab_com_websockets --> gitlab_rails_actioncable + gitlab_com_git --> gitlab_rails_api + gitlab_com_sidekiq --> gitlab_rails_sidekiq + + gitlab_rails_web_controllers --> gitlab_rails_services + gitlab_rails_api --> gitlab_rails_services + gitlab_rails_api_runners --> gitlab_rails_services + gitlab_rails_graphql --> gitlab_rails_services + gitlab_rails_actioncable --> gitlab_rails_services + gitlab_rails_sidekiq --> gitlab_rails_services + + gitlab_rails_services --> gitlab_rails_models + + gitlab_rails_models --> postgresql_db + gitlab_rails_models --> redis_db +``` + +### Layer dependencies + +The differences in how GitLab is run for on-premise versus how we run GitLab.com does show a main division line in GitLab Rails: + +- Web: containing all API, all Controllers, all GraphQL and ActionCable functionality +- Sidekiq: containing all background processing jobs +- Core: containing all database, models and services that needs to be shared between Web and Sidekiq + +Each of these top-level application layers do depend only on a fraction of the codebase with all relevant dependencies: + +- In all cases we need the underlying database structure and application models +- In some cases we need dependent services +- We only need a part of the application common library +- We need gems to support the requested functionality +- Individual layers should not use another sibling layer (tight coupling), rather connect via API, Redis or DB to share data (loose coupling) + +## Proposal + +The Memory team group conducted a Proof-of-Concept phase to understand the impact of introducing **Application Layers**. +We did this to understand the complexity, impact, and needed iterations to execute this proposal. + +The proposals here should be treated as evaluation of the impact of this blueprint, +but not a final solution to be implemented. The PoC as defined is not something that should be merged, +but serves as a basis for future work. + +### PoC using Rails Engines + +We decided to use Rails Engines by modeling a Web Application Layer. The Web Engine contained Controllers, API, GraphQL. +This allowed us to run Web Nodes with all dependencies, but measure the impact on Sidekiq not having these components loaded. + +All work can be found in these merge requests: + +- [Provide mechanism to load GraphQL with all dependencies only when needed](https://gitlab.com/gitlab-org/gitlab/-/issues/288044) +- [Draft: PoC - Move Graphql to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50180) +- [Draft: PoC - Move Controllers and Grape API:API to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53720) +- [Draft: PoC - Move only Grape API:API to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53982) +- [Measure performance impact for proposed web_engine](https://gitlab.com/gitlab-org/gitlab/-/issues/300548) + +What was done? + +- We used [Rails Engines](https://guides.rubyonrails.org/engines.html) +- The 99% of changes as visible in the above MRs is moving files as-is +- We moved all GraphQL code and specs into `engines/web_engine/` as-is +- We moved all API and Controllers code and specs into `engines/web_engine` +- We adapted CI to test `engines/web_engine/` as a self-sufficient component of stack +- We configured GitLab to load `gem web_engine` running Web nodes (Puma web server) +- We disabled loading `web_engine` when running Background processing nodes (Sidekiq) + +#### Implementation details for proposed solution + +1. Introduce new Rails Engine for each application layer. + + We created `engines` folder, which could contain different engines for each application layer we introduce in the future. + + In the above PoCs we introuced the new Web Application Layer, located in `engines/web_engine` folder. + +1. Move all code and specs into `engines/web_engine/` + + - We moved all Graphql code and specs into `engines/web_engine/` without changing files itself + - We moved all Grape API and Controllers code into `engines/web_engine/` without changing files itself + +1. Move gems to the `engines/web_engine/` + + - We moved all Graphql gems to the actual web_engine Gemfile + - We moved Grape API gem to the actual web_engine Gemfile + + ```ruby + Gem::Specification.new do |spec| + spec.add_dependency 'apollo_upload_server' + spec.add_dependency 'graphql' + spec.add_dependency 'graphiql-rails' + + spec.add_dependency 'graphql-docs' + spec.add_dependency 'grape' + end + ``` + +1. Move routes to the `engines/web_engine/config/routes.rb` file + + - We moved Graphql routes to the web_engine routes. + - We moved API routes to the web_engine routes. + - We moved most of the controller routes to the web_engine routes. + + ```ruby + Rails.application.routes.draw do + post '/api/graphql', to: 'graphql#execute' + mount GraphiQL::Rails::Engine, at: '/-/graphql-explorer', graphql_path: + Gitlab::Utils.append_path(Gitlab.config.gitlab.relative_url_root, '/api/graphql') + + draw :api + + #... + end + ``` + +1. Move initializers to the `engines/web_engine/config/initializers` folder + + - We moved graphql.rb initializer to the `web_engine` initializers folder + - We moved grape_patch.rb and graphe_validators to the `web_engine` initializers folder + +1. Connect GitLab application with the WebEngine + + In GitLab Gemfile.rb, add web_engine to the engines group + + ```ruby + # Gemfile + group :engines, :test do + gem 'web_engine', path: 'engines/web_engine' + end + ``` + + Since the gem is inside :engines group, it will not be automatically required by default. + +1. Configure GitLab when to load the engine. + + In GitLab `config/engines.rb`, we can configure when do we want to load our engines by relying on our `Gitlab::Runtime` + + ```ruby + # config/engines.rb + # Load only in case we are running web_server or rails console + if Gitlab::Runtime.web_server? || Gitlab::Runtime.console? + require 'web_engine' + end + ``` + +1. Configure Engine + + Our Engine inherits from the `Rails::Engine` class. This way this gem notifies Rails that + there's an engine at the specified path so it will correctly mount the engine inside + the application, performing tasks such as adding the app directory of the engine to + the load path for models, mailers, controllers, and views. + A file at `lib/web_engine/engine.rb`, is identical in function to a standard Rails + application's `config/application.rb` file. This way engines can access a config + object which contains configuration shared by all railties and the application. + Additionally, each engine can access autoload_paths, eager_load_paths, and autoload_once_paths + settings which are scoped to that engine. + + ```ruby + module WebEngine + class Engine < ::Rails::Engine + config.eager_load_paths.push(*%W[#{config.root}/lib + #{config.root}/app/graphql/resolvers/concerns + #{config.root}/app/graphql/mutations/concerns + #{config.root}/app/graphql/types/concerns]) + + if Gitlab.ee? + ee_paths = config.eager_load_paths.each_with_object([]) do |path, memo| + ee_path = config.root + .join('ee', Pathname.new(path).relative_path_from(config.root)) + memo << ee_path.to_s + end + # Eager load should load CE first + config.eager_load_paths.push(*ee_paths) + end + end + end + ``` + +1. Testing + + We adapted CI to test `engines/web_engine/` as a self-sufficient component of stack. + + - We moved spec as-is files to the `engines/web_engine/spec` folder + - We moved ee/spec as-is files to the `engines/web_engine/ee/spec` folder + - We control specs from main application using environment variable `TEST_WEB_ENGINE` + - We added new CI job that will run `engines/web_engine/spec` tests separately using `TEST_WEB_ENGINE` env variable. + - We added new CI job that will run `engines/web_engine/ee/spec` tests separately using `TEST_WEB_ENGINE` env variable. + - We are running all whitebox frontend tests with `TEST_WEB_ENGINE=true` + +#### Results + +The effect on introducing these changes: + +- Savings for RSS +- 61.06 MB (7.76%) - Sidekiq without GraphQL +- 100.11 MB (12.73%) - Sidekiq without GraphQL and API +- 208.83 MB (26.56%) - Sidekiq without GraphQL, API, Controllers +- The size of Web nodes (running Puma) stayed the same as before + +Savings on Sidekiq `start-up` event, for a single Sidekiq cluster without GraphQL, API, Controllers + +- We saved 264.13 MB RSS (28.69%) +- We saved 264.09 MB USS (29.36%) +- Boot-up time was reduced from 45.31 to 21.80 seconds. It was 23.51 seconds faster (51.89%) +- We have 805,772 less live objects, 4,587,535 less allocated objects, 2,866 less allocated pages and 3.65 MB less allocated space for objects outside of the heap +- We loaded 2,326 less code files (15.64%) +- We reduced the duration of a single full GC cycle from 0.80s to 0.70 (12.64%) + +Puma single, showed very little difference as expected. + +More details can be found in the [issue](https://gitlab.com/gitlab-org/gitlab/-/issues/300548#note_516323444). + +#### Impact on GitLab.com + +Estimating the results for the scale of running GitLab.com, today we use: + +- Currently individual GC cycle takes around [130ms for Web](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=avg(rate(ruby_gc_duration_seconds_sum%7Bstage%3D%22main%22%2Ctype%3D%22web%22%7D%5B5m%5D)%2Frate(ruby_gc_duration_seconds_count%5B5m%5D))&g0.tab=0) + and [200ms for Sidekiq](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=avg(rate(ruby_gc_duration_seconds_sum%7Bstage%3D%22main%22%2Ctype%3D%22sidekiq%22%7D%5B5m%5D)%2Frate(ruby_gc_duration_seconds_count%5B5m%5D))&g0.tab=0) on GitLab.com +- On average we do around [2 GC cycles per-second](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.end_input=2021-02-17%2017%3A56&g0.max_source_resolution=0s&g0.expr=avg(rate(ruby_gc_duration_seconds_count%7Bstage%3D%22main%22%2Ctype%3D%22web%22%7D%5B5m%5D))&g0.tab=0) + or [0.12 cycles per second for Sidekiq](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.end_input=2021-02-17%2017%3A56&g0.max_source_resolution=0s&g0.expr=avg(rate(ruby_gc_duration_seconds_count%7Bstage%3D%22main%22%2Ctype%3D%22sidekiq%22%7D%5B5m%5D))&g0.tab=0) +- This translates to using [around 9.5 vCPUs per-second for Web](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=sum(rate(ruby_gc_duration_seconds_sum%7Bstage%3D%22main%22%2Ctype%3D%22web%22%7D%5B5m%5D))&g0.tab=0) + and [around 8 vCPUs per-second for Sidekiq](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=sum(rate(ruby_gc_duration_seconds_sum%7Bstage%3D%22main%22%2Ctype%3D%22sidekiq%22%7D%5B5m%5D))&g0.tab=0) of spend on GC alone +- Sidekiq [uses 2.1GB on average](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=max(ruby_process_unique_memory_bytes%7Btype%3D%22sidekiq%22%7D)%2F1024%2F1024%2F1024&g0.tab=1) + or [550GB in total](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=sum(ruby_process_unique_memory_bytes%7Btype%3D%22sidekiq%22%7D)%2F1024%2F1024%2F1024&g0.tab=0) of memory on GitLab.com + +We estimate the possible maximum savings for introducing `web_engine`: + +- Reduce a GC cycle time by 20%, from to 200ms to 160ms +- The amount of GC cycles per-second would stay the same, but due to GC cycle time reduction we would use around 6 vCPUs instead of 8 vCPUs +- In the best case we would be looking at Sidekiq alone we would be estimating to save up-to 137GB of memory on GitLab.com + +This model could be extended to introduce `sidekiq_engine` giving a similar benefits +(even more important due to visible impact on users) for Web nodes. + +#### Outcome + +We achieved a number of benefits introducing these changes. + +Pros: + +- Significantly lower memory usage +- Significantly shorter application load time for Sidekiq +- Significantly improved responsivness of Sidekiq service due to much shorter GC cycles +- Significantly easier testing of a portion of application, ex. changing `web_engines/` does require + re-running test only for this application layer +- We retained a monolithic architecture of the codebase, but sharing database and application models +- A significant saving from the infrastracture side +- Ability to comfortably run on constrainted environments by reducing application footprint + +Cons: + +- It is harder to implement GraphQL subscriptions as in case of Sidekiq as we need another way to pass subscriptions +- `api_v4` paths can be used in some services that are used by Sidekiq (e.g. `api_v4_projects_path`) +- url_helpers paths are used in models and services, that could be used by Sidekiq (e.g. `Gitlab::Routing.url_helpers.project_pipelines_path` is used by [ExpirePipelineCacheService](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/ci/expire_pipeline_cache_service.rb#L20) in [ExpirePipelineCacheWorker](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/workers/expire_pipeline_cache_worker.rb#L18)) + +#### Example: GraphQL + +[Draft: PoC - Move Graphql to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50180) + +- The [99% of changes](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50180/diffs?commit_id=49c9881c6696eb620dccac71532a3173f5702ea8) as visible in the above MRs is moving files as-is. +- The [actual work](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50180/diffs?commit_id=1d9a9edfa29ea6638e7d8a6712ddf09f5be77a44) on fixing cross-dependencies, specs, and configuring web_engine +- We [adapted](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50180/diffs?commit_id=d7f862cc209ce242000b2aec88ff7f4485acdd92) CI to test `engines/web_engine/` as a self-sufficient component of stack + +Today, loading GraphQL requires a bunch of [dependencies](https://gitlab.com/gitlab-org/gitlab/-/issues/288044): + +> We also discovered that we load/require 14480 files, [memory-team-2gb-week#9](https://gitlab.com/gitlab-org/memory-team/memory-team-2gb-week/-/issues/9#note_452530513) +> when we start GitLab. 1274 files belong to Graphql. This means that if we don't load 1274 application files +> and all related Graphql gems when we don't need them (Sidekiq), we could save a lot of memory. + +GraphQL only needs to run in a specific context. If we could limit when it is being loaded we could effectively improve application efficiency, by reducing application load time and required memory. This, for example, is applicable for every size installation. + +A potential challenge with GraphQL and Websockets is that at some point we might want to use Action Cable subscriptions and push GraphQL/API payload from Sidekiq to clients. This would likely utilize Redis to pass data through. Where Sidekiq would publish information on Redis and ActionCable Node would pass through that information to connected clients. This way of working is possible in the above model, but we would have to use GraphQL or API (over HTTP endpoint) to calculate what should be sent. + +An alternative way is to use a notification system that would always make an `ActionCable` node (the one handling WebSockets) generate a payload based on a send query instead of performing passthrough. This could be applicable since `ActionCable` is the one handling a given connection for a client. This could have a downside of having to recalculate the same payload if many clients would be watching the same resource. However, this behavior of system might still be desired for security purposes, as generated payload might be dependent on permission of watching client (we would show different for anonymous, and different for the member of the project). + +#### Example: API + +[Draft: PoC - Move only Grape API:API to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53982) + +- [99% of the changes](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53982/diffs?commit_id=c8b72249b6e8f875ed4c713f0668207377604043), as visible in the above MRs, are moving the files as-is. +- The [actual work](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53982/diffs?commit_id=00d9b54ba952c85ff4d158a18205c2fac13eaf8d) on fixing cross-dependencies, specs, configuring initializers, gems and routes. + +Grape::API is another example that only needs to run only in a web server context. + +Potential chalenges with Grape API: + +- Currently there are some API::API dependencies in the models (e.g. `API::Helpers::Version` dependency in [project model](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/project.rb#L2019) or API::API dependency in GeoNode model for [geo_retrieve_url](https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/app/models/geo_node.rb#L183)) +- `api_v4` paths are used in helpers, presenters, and views (e.g. `api_v4_projects_path` in [PackagesHelper](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/helpers/packages_helper.rb#L17)) + +#### Example: Controllers + +[Draft: PoC - Move Controllers and Grape API:API to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53720) + +- [99% of the changes](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53720/diffs?commit_id=17174495cf3263c8e69a0420092d9fa759170aa6), as visible in the above MRs, are moving files as-is. +- The [actual work](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53720/diffs?commit_id=39cc4bb1e0ce47f66605d06eb1b0d6b89ba174e6) on fixing cross-dependencies, specs, configuring initializers, gems and routes. + +Controllers, Serializers, some presenters and some of the Grape:Entities are also good examples that only need to be run in web server context. + +Potential chalenges with moving Controllers: + +- We needed to extend `Gitlab::Patch::DrawRoute` in order to support `engines/web_engine/config/routes` and `engines/web_engine/ee/config/routes` in case when `web_engine` is loaded. Here is potential [solution](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53720#note_506957398). +- `Gitlab::Routing.url_helpers` paths are used in models and services, that could be used by Sidekiq (e.g. `Gitlab::Routing.url_helpers.project_pipelines_path` is used by [ExpirePipelineCacheService](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/ci/expire_pipeline_cache_service.rb#L20) in [ExpirePipelineCacheWorker](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/workers/expire_pipeline_cache_worker.rb#L18))) + +### Packwerk + +NOTE: +Packwerk is currently accepting bug fixes only, and it is not being actively developed. Check for [more details](https://github.com/Shopify/packwerk#note-packwerk-is-considered-to-be-feature-complete-for-shopifys-uses-we-are-currently-accepting-bug-fixes-only-and-it-is-not-being-actively-developed-please-fork-this-project-if-you-are-interested-in-adding-new-features) + +## Future impact + +**Application Layers** and this proposal currently defines only `web_engine`. Following the same pattern we could easily introduce +additional engines dedicated for supporting that would allow us to maintain much better separation, lower memory usage +and much better maintanability of GitLab Rails into the future. + +This would be a framework for introducing all new interfaces for features that do not need to be part of the core codebase, +like support for additional Package services. Allowing us to better scale application in the future, but retaining a single codebase +and monolithic architecture of GitLab. + +As of today, it seems reasonable to define three **application layers**: + +- `gitlab-core`: a core functionality: DB structure, models, services, common library. It models a data access layer, and initially all services needed to run GitLab. This might be potentially be split in the future into smaller aspects +- `gitlab-web`: a Controllers/API/GraphQL/ActionCable functionality needed to run in a web server context (depends on `gitlab-core`) +- `gitlab-sidekiq`: a background jobs functionality needed to run Sidekiq Workers (depends on `gitlab-core`) + +This model is best described today as a shared core with satellite. The shared core defines data access layer, where as satellites define a way to present and process this data. Satelites can only talk with Core. They cannot directly load or talk to another satelitte unless they use a well defined interface in form of API, GraphQL or Redis (as for scheduling Sidekiq jobs). + +It is reasonable to assume that we limit how many `engines` we allow. Initial proposal is to allow up to 5 engines +to be created to ensure that we do not have explosion of engines. + +## Issues and Merge Requests + +- [Split application into functional parts to ensure that only needed code is loaded with all dependencies](https://gitlab.com/gitlab-org/gitlab/-/issues/290935) +- [Provide mechanism to load GraphQL with all dependencies only when needed](https://gitlab.com/gitlab-org/gitlab/-/issues/288044) +- [Draft: PoC - Move Graphql to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50180) +- [Draft: PoC - Move Controllers and Grape API:API to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53720) +- [Draft: PoC - Move only Grape API:API to the WebEngine](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53982) +- [Measure performance impact for proposed web_engine](https://gitlab.com/gitlab-org/gitlab/-/issues/300548) +- [Create new models / classes within a module / namespace](https://gitlab.com/gitlab-org/gitlab/-/issues/212156) +- [Make teams to be maintainers of their code](https://gitlab.com/gitlab-org/gitlab/-/issues/25872) +- [Use nested structure to organize CI classes](https://gitlab.com/gitlab-org/gitlab/-/issues/209745) +- [WIP: Make it simple to build and use "Decoupled Services"](https://gitlab.com/gitlab-org/gitlab/-/issues/31121) +- [Rails takes awhile to boot, let's see if we can improve this](https://gitlab.com/gitlab-org/gitlab/-/issues/213992) + +## Who + +Proposal: + +<!-- vale gitlab.Spelling = NO --> + +| Role | Who +|------------------------------|-------------------------| +| Author | Kamil Trzciński | +| Architecture Evolution Coach | ? | +| Engineering Leader | ? | + +DRIs: + +| Role | Who +|------------------------------|------------------------| +| Product | ? | +| Leadership | Craig Gomes | +| Engineering | ? | + +Domain Experts: + +| Role | Who +|------------------------------|------------------------| +| Domain Expert | Nikola Milojevic | +| Domain Expert | ? | +| Domain Expert | ? | + +<!-- vale gitlab.Spelling = YES --> diff --git a/lib/api/entities/label_basic.rb b/lib/api/entities/label_basic.rb index 00ecea26ec3..ed52688638e 100644 --- a/lib/api/entities/label_basic.rb +++ b/lib/api/entities/label_basic.rb @@ -3,7 +3,7 @@ module API module Entities class LabelBasic < Grape::Entity - expose :id, :name, :color, :description, :description_html, :text_color, :remove_on_close + expose :id, :name, :color, :description, :description_html, :text_color end end end diff --git a/lib/api/helpers/label_helpers.rb b/lib/api/helpers/label_helpers.rb index 796b8928243..da0ee8f207e 100644 --- a/lib/api/helpers/label_helpers.rb +++ b/lib/api/helpers/label_helpers.rb @@ -5,34 +5,27 @@ module API module LabelHelpers extend Grape::API::Helpers - params :optional_label_params do - optional :description, type: String, desc: 'The description of the label' - optional :remove_on_close, type: Boolean, desc: 'Whether the label should be removed from an issue when the issue is closed' - end - params :label_create_params do requires :name, type: String, desc: 'The name of the label to be created' requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the allowed CSS color names" - - use :optional_label_params + optional :description, type: String, desc: 'The description of label to be created' end params :label_update_params do optional :new_name, type: String, desc: 'The new name of the label' optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the allowed CSS color names" - - use :optional_label_params + optional :description, type: String, desc: 'The new description of label' end params :project_label_update_params do use :label_update_params optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true - at_least_one_of :new_name, :color, :description, :priority, :remove_on_close + at_least_one_of :new_name, :color, :description, :priority end params :group_label_update_params do use :label_update_params - at_least_one_of :new_name, :color, :description, :remove_on_close + at_least_one_of :new_name, :color, :description end def find_label(parent, id_or_title, params = { include_ancestor_groups: true }) diff --git a/lib/gitlab/ci/matching/build_matcher.rb b/lib/gitlab/ci/matching/build_matcher.rb new file mode 100644 index 00000000000..dff7d9141d9 --- /dev/null +++ b/lib/gitlab/ci/matching/build_matcher.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Matching + class BuildMatcher + ATTRIBUTES = %i[ + protected + tag_list + build_ids + project + ].freeze + + attr_reader(*ATTRIBUTES) + alias_method :protected?, :protected + + def initialize(params) + ATTRIBUTES.each do |attribute| + instance_variable_set("@#{attribute}", params.fetch(attribute)) + end + end + + def has_tags? + tag_list.present? + end + end + end + end +end diff --git a/lib/gitlab/ci/matching/runner_matcher.rb b/lib/gitlab/ci/matching/runner_matcher.rb new file mode 100644 index 00000000000..63642674936 --- /dev/null +++ b/lib/gitlab/ci/matching/runner_matcher.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Matching + ### + # This class is used to check if a build can be picked by a runner: + # + # runner = Ci::Runner.find(id) + # build = Ci::Build.find(id) + # runner.runner_matcher.matches?(build.build_matcher) + # + # There are also class level methods to build matchers: + # + # `project.builds.build_matchers(project)` returns a distinct collection + # of build matchers. + # `Ci::Runner.runner_matchers` returns a distinct collection of runner matchers. + # + class RunnerMatcher + ATTRIBUTES = %i[ + runner_type + public_projects_minutes_cost_factor + private_projects_minutes_cost_factor + run_untagged + access_level + tag_list + ].freeze + + attr_reader(*ATTRIBUTES) + + def initialize(params) + ATTRIBUTES.each do |attribute| + instance_variable_set("@#{attribute}", params.fetch(attribute)) + end + end + + def matches?(build_matcher) + ensure_build_matcher_instance!(build_matcher) + return false if ref_protected? && !build_matcher.protected? + + accepting_tags?(build_matcher) + end + + def instance_type? + runner_type.to_sym == :instance_type + end + + private + + def ref_protected? + access_level.to_sym == :ref_protected + end + + def accepting_tags?(build_matcher) + (run_untagged || build_matcher.has_tags?) && (build_matcher.tag_list - tag_list).empty? + end + + def ensure_build_matcher_instance!(build_matcher) + return if build_matcher.is_a?(Matching::BuildMatcher) + + raise ArgumentError, 'only Gitlab::Ci::Matching::BuildMatcher are allowed' + end + end + end + end +end + +Gitlab::Ci::Matching::RunnerMatcher.prepend_mod_with('Gitlab::Ci::Matching::RunnerMatcher') diff --git a/lib/gitlab/cluster/lifecycle_events.rb b/lib/gitlab/cluster/lifecycle_events.rb index b3dc59466ec..6b7b9b09269 100644 --- a/lib/gitlab/cluster/lifecycle_events.rb +++ b/lib/gitlab/cluster/lifecycle_events.rb @@ -8,13 +8,13 @@ module Gitlab # LifecycleEvents lets Rails initializers register application startup hooks # that are sensitive to forking. For example, to defer the creation of # watchdog threads. This lets us abstract away the Unix process - # lifecycles of Unicorn, Sidekiq, Puma, Puma Cluster, etc. + # lifecycles of Sidekiq, Puma, Puma Cluster, etc. # # We have the following lifecycle events. # # - on_before_fork (on master process): # - # Unicorn/Puma Cluster: This will be called exactly once, + # Puma Cluster: This will be called exactly once, # on startup, before the workers are forked. This is # called in the PARENT/MASTER process. # @@ -22,7 +22,7 @@ module Gitlab # # - on_master_start (on master process): # - # Unicorn/Puma Cluster: This will be called exactly once, + # Puma Cluster: This will be called exactly once, # on startup, before the workers are forked. This is # called in the PARENT/MASTER process. # @@ -30,7 +30,7 @@ module Gitlab # # - on_before_blackout_period (on master process): # - # Unicorn/Puma Cluster: This will be called before a blackout + # Puma Cluster: This will be called before a blackout # period when performing graceful shutdown of master. # This is called on `master` process. # @@ -38,7 +38,7 @@ module Gitlab # # - on_before_graceful_shutdown (on master process): # - # Unicorn/Puma Cluster: This will be called before a graceful + # Puma Cluster: This will be called before a graceful # shutdown of workers starts happening, but after blackout period. # This is called on `master` process. # @@ -46,11 +46,6 @@ module Gitlab # # - on_before_master_restart (on master process): # - # Unicorn: This will be called before a new master is spun up. - # This is called on forked master before `execve` to become - # a new masterfor Unicorn. This means that this does not really - # affect old master process. - # # Puma Cluster: This will be called before a new master is spun up. # This is called on `master` process. # @@ -58,7 +53,7 @@ module Gitlab # # - on_worker_start (on worker process): # - # Unicorn/Puma Cluster: This is called in the worker process + # Puma Cluster: This is called in the worker process # exactly once before processing requests. # # Sidekiq/Puma Single: This is called immediately. @@ -114,7 +109,7 @@ module Gitlab end # - # Lifecycle integration methods (called from unicorn.rb, puma.rb, etc.) + # Lifecycle integration methods (called from puma.rb, etc.) # def do_worker_start call(:worker_start_hooks, @worker_start_hooks) diff --git a/lib/gitlab/cluster/puma_worker_killer_initializer.rb b/lib/gitlab/cluster/puma_worker_killer_initializer.rb index fd9f58a34f3..e634291f894 100644 --- a/lib/gitlab/cluster/puma_worker_killer_initializer.rb +++ b/lib/gitlab/cluster/puma_worker_killer_initializer.rb @@ -12,12 +12,9 @@ module Gitlab require 'puma_worker_killer' PumaWorkerKiller.config do |config| - # Note! ram is expressed in megabytes (whereas GITLAB_UNICORN_MEMORY_MAX is in bytes) - # Importantly RAM is for _all_workers (ie, the cluster), - # not each worker as is the case with GITLAB_UNICORN_MEMORY_MAX worker_count = puma_options[:workers] || 1 - # The Puma Worker Killer checks the total RAM used by both the master - # and worker processes. + # The Puma Worker Killer checks the total memory used by the cluster, + # i.e. both primary and worker processes. # https://github.com/schneems/puma_worker_killer/blob/v0.1.0/lib/puma_worker_killer/puma_memory.rb#L57 # # Additional memory is added when running in `development` diff --git a/lib/gitlab/stack_prof.rb b/lib/gitlab/stack_prof.rb index 4b7d93c91ce..97f52491e9e 100644 --- a/lib/gitlab/stack_prof.rb +++ b/lib/gitlab/stack_prof.rb @@ -118,7 +118,6 @@ module Gitlab # # see also: # * https://github.com/puma/puma/blob/master/docs/signals.md#puma-signals - # * https://github.com/phusion/unicorn/blob/master/SIGNALS # * https://github.com/mperham/sidekiq/wiki/Signals Signal.trap('SIGUSR2') do write.write('.') diff --git a/lib/support/init.d/gitlab.default.example b/lib/support/init.d/gitlab.default.example index 1b499467ad6..53bebe55fa3 100644 --- a/lib/support/init.d/gitlab.default.example +++ b/lib/support/init.d/gitlab.default.example @@ -5,9 +5,6 @@ # Normal values are "production", "test" and "development". RAILS_ENV="production" -# Uncomment the line below to enable the Unicorn web server instead of Puma. -# use_web_server="unicorn" - # app_user defines the user that GitLab is run as. # The default is "git". app_user="git" @@ -43,7 +40,7 @@ gitlab_workhorse_pid_path="$pid_path/gitlab-workhorse.pid" # socket. To listen on TCP connections (needed by Apache) change to: # '-listenNetwork tcp -listenAddr 127.0.0.1:8181' # -# The -authBackend setting tells gitlab-workhorse where it can reach Unicorn. +# The -authBackend setting tells gitlab-workhorse where it can reach the GitLab Rails application. # For relative URL support change to: # '-authBackend http://127.0.0.1/8080/gitlab' # Read more in http://doc.gitlab.com/ce/install/relative_url.html diff --git a/lib/tasks/gitlab/setup.rake b/lib/tasks/gitlab/setup.rake index 31bd80e78d4..705519d1741 100644 --- a/lib/tasks/gitlab/setup.rake +++ b/lib/tasks/gitlab/setup.rake @@ -40,7 +40,7 @@ namespace :gitlab do end # If there are any clients connected to the DB, PostgreSQL won't let - # you drop the database. It's possible that Sidekiq, Unicorn, or + # you drop the database. It's possible that Sidekiq, Puma, or # some other client will be hanging onto a connection, preventing # the DROP DATABASE from working. To workaround this problem, this # method terminates all the connections so that a subsequent DROP diff --git a/qa/qa/specs/features/browser_ui/5_package/maven_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/maven_repository_spec.rb index fb019290d4c..09cc1bf58bc 100644 --- a/qa/qa/specs/features/browser_ui/5_package/maven_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/maven_repository_spec.rb @@ -3,7 +3,7 @@ require 'securerandom' module QA - RSpec.describe 'Package', :orchestrated, :packages do + RSpec.describe 'Package', :orchestrated, :packages, :reliable do describe 'Maven Repository' do include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb b/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb index 4d714affd65..0f5406bda57 100644 --- a/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/npm_registry_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :orchestrated, :packages do + RSpec.describe 'Package', :orchestrated, :packages, :reliable do describe 'npm registry' do include Runtime::Fixtures diff --git a/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb index 3db5b9671d9..4fb321f60dd 100644 --- a/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/nuget_repository_spec.rb @@ -3,7 +3,7 @@ require 'securerandom' module QA - RSpec.describe 'Package', :orchestrated, :packages do + RSpec.describe 'Package', :orchestrated, :packages, :reliable do describe 'NuGet Repository' do include Runtime::Fixtures let(:project) do diff --git a/spec/factories/packages/debian/distribution_key.rb b/spec/factories/packages/debian/distribution_key.rb new file mode 100644 index 00000000000..6bd457c50d0 --- /dev/null +++ b/spec/factories/packages/debian/distribution_key.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :debian_project_distribution_key, class: 'Packages::Debian::ProjectDistributionKey' do + distribution { association(:debian_project_distribution) } + + private_key { '-----BEGIN PGP PRIVATE KEY BLOCK-----' } + passphrase { '12345' } + public_key { '-----BEGIN PGP PUBLIC KEY BLOCK-----' } + fingerprint { '12345' } + + factory :debian_group_distribution_key, class: 'Packages::Debian::GroupDistributionKey' do + distribution { association(:debian_group_distribution) } + end + end +end diff --git a/spec/fixtures/api/schemas/public_api/v4/label_basic.json b/spec/fixtures/api/schemas/public_api/v4/label_basic.json index a4653c67ed2..a501bc2ec56 100644 --- a/spec/fixtures/api/schemas/public_api/v4/label_basic.json +++ b/spec/fixtures/api/schemas/public_api/v4/label_basic.json @@ -6,8 +6,7 @@ "color", "description", "description_html", - "text_color", - "remove_on_close" + "text_color" ], "properties": { "id": { "type": "integer" }, @@ -21,8 +20,7 @@ "text_color": { "type": "string", "pattern": "^#[0-9A-Fa-f]{3}{1,2}$" - }, - "remove_on_close": { "type": "boolean" } + } }, "additionalProperties": false } diff --git a/spec/graphql/mutations/issues/update_spec.rb b/spec/graphql/mutations/issues/update_spec.rb index 6d6a5b94219..bd780477658 100644 --- a/spec/graphql/mutations/issues/update_spec.rb +++ b/spec/graphql/mutations/issues/update_spec.rb @@ -69,33 +69,17 @@ RSpec.describe Mutations::Issues::Update do context 'when changing state' do let_it_be_with_refind(:issue) { create(:issue, project: project, state: :opened) } - before do - mutation_params[:state_event] = state_event - end - - context 'when state_event is close' do - let_it_be(:removable_label) { create(:label, project: project, remove_on_close: true, issues: [issue]) } + it 'closes issue' do + mutation_params[:state_event] = 'close' - let(:state_event) { 'close' } - - it 'closes issue' do - expect do - subject - issue.reload - end.to change(issue, :state).from('opened').to('closed').and( - change { issue.label_ids }.from([removable_label.id]).to([]) - ) - end + expect { subject }.to change { issue.reload.state }.from('opened').to('closed') end - context 'when state_event is reopen' do - let(:state_event) { 'reopen' } - - it 'reopens issue' do - issue.close + it 'reopens issue' do + issue.close + mutation_params[:state_event] = 'reopen' - expect { subject }.to change { issue.reload.state }.from('closed').to('opened') - end + expect { subject }.to change { issue.reload.state }.from('closed').to('opened') end end diff --git a/spec/graphql/types/label_type_spec.rb b/spec/graphql/types/label_type_spec.rb index 475b2a2ad34..427b5d2dcef 100644 --- a/spec/graphql/types/label_type_spec.rb +++ b/spec/graphql/types/label_type_spec.rb @@ -11,7 +11,6 @@ RSpec.describe GitlabSchema.types['Label'] do :color, :text_color, :created_at, - :remove_on_close, :updated_at ] diff --git a/spec/lib/gitlab/ci/matching/build_matcher_spec.rb b/spec/lib/gitlab/ci/matching/build_matcher_spec.rb new file mode 100644 index 00000000000..f12e85da9c2 --- /dev/null +++ b/spec/lib/gitlab/ci/matching/build_matcher_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Ci::Matching::BuildMatcher do + let(:dummy_attributes) do + { + protected: true, + tag_list: %w[tag1 tag2], + build_ids: [1, 2, 3], + project: :my_project + } + end + + subject(:matcher) { described_class.new(attributes) } + + describe '.new' do + context 'when attributes are missing' do + let(:attributes) { {} } + + it { expect { matcher }.to raise_error(KeyError) } + end + + context 'with attributes' do + let(:attributes) { dummy_attributes } + + it { expect(matcher.protected).to eq(true) } + + it { expect(matcher.tag_list).to eq(%w[tag1 tag2]) } + + it { expect(matcher.build_ids).to eq([1, 2, 3]) } + + it { expect(matcher.project).to eq(:my_project) } + end + end + + describe '#protected?' do + context 'when protected is set to true' do + let(:attributes) { dummy_attributes } + + it { expect(matcher.protected?).to be_truthy } + end + + context 'when protected is set to false' do + let(:attributes) { dummy_attributes.merge(protected: false) } + + it { expect(matcher.protected?).to be_falsey } + end + end + + describe '#has_tags?' do + context 'when tags are present' do + let(:attributes) { dummy_attributes } + + it { expect(matcher.has_tags?).to be_truthy } + end + + context 'when tags are empty' do + let(:attributes) { dummy_attributes.merge(tag_list: []) } + + it { expect(matcher.has_tags?).to be_falsey } + end + end +end diff --git a/spec/lib/gitlab/ci/matching/runner_matcher_spec.rb b/spec/lib/gitlab/ci/matching/runner_matcher_spec.rb new file mode 100644 index 00000000000..d6492caa31a --- /dev/null +++ b/spec/lib/gitlab/ci/matching/runner_matcher_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Matching::RunnerMatcher do + let(:dummy_attributes) do + { + runner_type: 'instance_type', + public_projects_minutes_cost_factor: 0, + private_projects_minutes_cost_factor: 1, + run_untagged: false, + access_level: 'ref_protected', + tag_list: %w[tag1 tag2] + } + end + + subject(:matcher) { described_class.new(attributes) } + + describe '.new' do + context 'when attributes are missing' do + let(:attributes) { {} } + + it { expect { matcher }.to raise_error(KeyError) } + end + + context 'with attributes' do + let(:attributes) { dummy_attributes } + + it { expect(matcher.runner_type).to eq('instance_type') } + + it { expect(matcher.public_projects_minutes_cost_factor).to eq(0) } + + it { expect(matcher.private_projects_minutes_cost_factor).to eq(1) } + + it { expect(matcher.run_untagged).to eq(false) } + + it { expect(matcher.access_level).to eq('ref_protected') } + + it { expect(matcher.tag_list).to eq(%w[tag1 tag2]) } + end + end + + describe '#instance_type?' do + let(:attributes) { dummy_attributes } + + it { expect(matcher.instance_type?).to be_truthy } + + context 'context with private runners' do + let(:attributes) { dummy_attributes.merge(runner_type: 'project_type') } + + it { expect(matcher.instance_type?).to be_falsey } + end + end + + describe '#matches?' do + let(:build) { build_stubbed(:ci_build, build_attributes) } + let(:runner_matcher) { described_class.new(dummy_attributes.merge(runner_attributes)) } + + subject { runner_matcher.matches?(record) } + + context 'with an instance of BuildMatcher' do + using RSpec::Parameterized::TableSyntax + + where(:ref_protected, :build_protected, :run_untagged, :runner_tags, :build_tags, :result) do + # the `ref_protected? && !build.protected?` part: + true | true | true | [] | [] | true + true | false | true | [] | [] | false + false | true | true | [] | [] | true + false | false | true | [] | [] | true + # `accepting_tags?(build)` bit: + true | true | true | [] | [] | true + true | true | true | [] | ['a'] | false + true | true | true | %w[a b] | ['a'] | true + true | true | true | ['a'] | %w[a b] | false + true | true | true | ['a'] | ['a'] | true + true | true | false | ['a'] | ['a'] | true + true | true | false | ['b'] | ['a'] | false + true | true | false | %w[a b] | ['a'] | true + end + + with_them do + let(:build_attributes) do + { + tag_list: build_tags, + protected: build_protected + } + end + + let(:runner_attributes) do + { + access_level: ref_protected ? 'ref_protected' : 'not_protected', + run_untagged: run_untagged, + tag_list: runner_tags + } + end + + let(:record) { build.build_matcher } + + it { is_expected.to eq(result) } + end + end + + context 'with an instance of Ci::Build' do + let(:runner_attributes) { {} } + let(:build_attributes) { {} } + let(:record) { build } + + it 'raises ArgumentError' do + expect { subject }.to raise_error ArgumentError, /BuildMatcher are allowed/ + end + end + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index f81db1413c2..c7e77a34582 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -395,7 +395,6 @@ project: - ewm_service - external_wiki_service - mock_ci_service -- mock_deployment_service - mock_monitoring_service - forked_to_members - forked_from_project @@ -571,6 +570,7 @@ project: - exported_protected_branches - incident_management_oncall_schedules - incident_management_oncall_rotations +- incident_management_escalation_policies - debian_distributions - merge_request_metrics - security_orchestration_policy_configuration diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 70ebff2a54e..0be1b09851f 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -102,7 +102,6 @@ ProjectLabel: - template - description - priority -- remove_on_close Milestone: - id - title diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 66d2f5f4ee9..27b0ed0c994 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4987,4 +4987,67 @@ RSpec.describe Ci::Build do it { is_expected.to be_truthy } end end + + describe '.build_matchers' do + let_it_be(:pipeline) { create(:ci_pipeline, :protected) } + + subject(:matchers) { pipeline.builds.build_matchers(pipeline.project) } + + context 'when the pipeline is empty' do + it 'does not throw errors' do + is_expected.to eq([]) + end + end + + context 'when the pipeline has builds' do + let_it_be(:build_without_tags) do + create(:ci_build, pipeline: pipeline) + end + + let_it_be(:build_with_tags) do + create(:ci_build, pipeline: pipeline, tag_list: %w[tag1 tag2]) + end + + let_it_be(:other_build_with_tags) do + create(:ci_build, pipeline: pipeline, tag_list: %w[tag2 tag1]) + end + + it { expect(matchers.size).to eq(2) } + + it 'groups build ids' do + expect(matchers.map(&:build_ids)).to match_array([ + [build_without_tags.id], + match_array([build_with_tags.id, other_build_with_tags.id]) + ]) + end + + it { expect(matchers.map(&:tag_list)).to match_array([[], %w[tag1 tag2]]) } + + it { expect(matchers.map(&:protected?)).to all be_falsey } + + context 'when the builds are protected' do + before do + pipeline.builds.update_all(protected: true) + end + + it { expect(matchers).to all be_protected } + end + end + end + + describe '#build_matcher' do + let_it_be(:build) do + build_stubbed(:ci_build, tag_list: %w[tag1 tag2]) + end + + subject(:matcher) { build.build_matcher } + + it { expect(matcher.build_ids).to eq([build.id]) } + + it { expect(matcher.tag_list).to match_array(%w[tag1 tag2]) } + + it { expect(matcher.protected?).to eq(build.protected?) } + + it { expect(matcher.project).to eq(build.project) } + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ffe0b0d0b19..4fd76390e49 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -975,6 +975,108 @@ RSpec.describe Ci::Runner do end end + describe '.runner_matchers' do + subject(:matchers) { described_class.all.runner_matchers } + + context 'deduplicates on runner_type' do + before do + create_list(:ci_runner, 2, :instance) + create_list(:ci_runner, 2, :project) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:runner_type)).to match_array(%w[instance_type project_type]) + end + end + + context 'deduplicates on public_projects_minutes_cost_factor' do + before do + create_list(:ci_runner, 2, public_projects_minutes_cost_factor: 5) + create_list(:ci_runner, 2, public_projects_minutes_cost_factor: 10) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:public_projects_minutes_cost_factor)).to match_array([5, 10]) + end + end + + context 'deduplicates on private_projects_minutes_cost_factor' do + before do + create_list(:ci_runner, 2, private_projects_minutes_cost_factor: 5) + create_list(:ci_runner, 2, private_projects_minutes_cost_factor: 10) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:private_projects_minutes_cost_factor)).to match_array([5, 10]) + end + end + + context 'deduplicates on run_untagged' do + before do + create_list(:ci_runner, 2, run_untagged: true, tag_list: ['a']) + create_list(:ci_runner, 2, run_untagged: false, tag_list: ['a']) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:run_untagged)).to match_array([true, false]) + end + end + + context 'deduplicates on access_level' do + before do + create_list(:ci_runner, 2, access_level: :ref_protected) + create_list(:ci_runner, 2, access_level: :not_protected) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:access_level)).to match_array(%w[ref_protected not_protected]) + end + end + + context 'deduplicates on tag_list' do + before do + create_list(:ci_runner, 2, tag_list: %w[tag1 tag2]) + create_list(:ci_runner, 2, tag_list: %w[tag3 tag4]) + end + + it 'creates two matchers' do + expect(matchers.size).to eq(2) + + expect(matchers.map(&:tag_list)).to match_array([%w[tag1 tag2], %w[tag3 tag4]]) + end + end + end + + describe '#runner_matcher' do + let(:runner) do + build_stubbed(:ci_runner, :instance_type, tag_list: %w[tag1 tag2]) + end + + subject(:matcher) { runner.runner_matcher } + + it { expect(matcher.runner_type).to eq(runner.runner_type) } + + it { expect(matcher.public_projects_minutes_cost_factor).to eq(runner.public_projects_minutes_cost_factor) } + + it { expect(matcher.private_projects_minutes_cost_factor).to eq(runner.private_projects_minutes_cost_factor) } + + it { expect(matcher.run_untagged).to eq(runner.run_untagged) } + + it { expect(matcher.access_level).to eq(runner.access_level) } + + it { expect(matcher.tag_list).to match_array(runner.tag_list) } + end + describe '#uncached_contacted_at' do let(:contacted_at_stored) { 1.hour.ago.change(usec: 0) } let(:runner) { create(:ci_runner, contacted_at: contacted_at_stored) } diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index abaae5b059a..e43cd837bed 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -320,7 +320,7 @@ RSpec.describe ContainerRepository do before do group.parent = test_group - group.save + group.save! end it { is_expected.to contain_exactly(repository, another_repository) } diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index 26b311fe629..2a2663149d0 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -21,9 +21,9 @@ RSpec.describe DiffDiscussion do describe '#merge_request_version_params' do let(:merge_request) { create(:merge_request, source_project: project, target_project: project, importing: true) } - let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } - let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) } - let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } + let!(:merge_request_diff1) { merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff2) { merge_request.merge_request_diffs.create!(head_commit_sha: nil) } + let!(:merge_request_diff3) { merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') } context 'when the discussion is active' do it 'returns an empty hash, which will end up showing the latest version' do diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 215c733f26b..2731eadecc0 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -167,7 +167,7 @@ RSpec.describe DiffNote do end it 'creates a diff note file' do - subject.save + subject.save! expect(subject.note_diff_file).to be_present end end @@ -188,7 +188,7 @@ RSpec.describe DiffNote do end it 'raises an error' do - expect { subject.save }.to raise_error(::DiffNote::NoteDiffFileCreationError, + expect { subject.save! }.to raise_error(::DiffNote::NoteDiffFileCreationError, "Failed to find diff line for: #{diff_file.file_path}, "\ "old_line: #{position.old_line}"\ ", new_line: #{position.new_line}") @@ -201,7 +201,7 @@ RSpec.describe DiffNote do end it 'creates a diff note file' do - subject.save + subject.save! expect(subject.reload.note_diff_file).to be_present end end @@ -544,7 +544,7 @@ RSpec.describe DiffNote do it "does not update the position" do expect(subject).not_to receive(:update_position) - subject.save + subject.save! end end diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index cd0938682db..2b09ee5c190 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -24,7 +24,7 @@ RSpec.describe Email do let(:user) { create(:user) } it 'synchronizes the gpg keys when the email is updated' do - email = user.emails.create(email: 'new@email.com') + email = user.emails.create!(email: 'new@email.com') expect(user).to receive(:update_invalid_gpg_signatures) diff --git a/spec/models/label_link_spec.rb b/spec/models/label_link_spec.rb index e5753b34e72..c6bec215145 100644 --- a/spec/models/label_link_spec.rb +++ b/spec/models/label_link_spec.rb @@ -13,27 +13,14 @@ RSpec.describe LabelLink do let(:invalid_items_for_bulk_insertion) { [] } # class does not have any validations defined end - describe 'scopes' do - describe '.for_target' do - it 'returns the label links for a given target' do - label_link = create(:label_link, target: create(:merge_request)) + describe '.for_target' do + it 'returns the label links for a given target' do + label_link = create(:label_link, target: create(:merge_request)) - create(:label_link, target: create(:issue)) + create(:label_link, target: create(:issue)) - expect(described_class.for_target(label_link.target_id, label_link.target_type)) - .to contain_exactly(label_link) - end - end - - describe '.with_remove_on_close_labels' do - it 'responds with label_links that can be removed when an issue is closed' do - issue = create(:issue) - removable_label = create(:label, project: issue.project, remove_on_close: true) - create(:label_link, target: issue) - removable_issue_label_link = create(:label_link, label: removable_label, target: issue) - - expect(described_class.with_remove_on_close_labels).to contain_exactly(removable_issue_label_link) - end + expect(described_class.for_target(label_link.target_id, label_link.target_type)) + .to contain_exactly(label_link) end end end diff --git a/spec/models/packages/debian/group_distribution_key_spec.rb b/spec/models/packages/debian/group_distribution_key_spec.rb new file mode 100644 index 00000000000..9ba163012b0 --- /dev/null +++ b/spec/models/packages/debian/group_distribution_key_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::GroupDistributionKey do + it_behaves_like 'Debian Distribution Key', :group +end diff --git a/spec/models/packages/debian/project_distribution_key_spec.rb b/spec/models/packages/debian/project_distribution_key_spec.rb new file mode 100644 index 00000000000..3dd723423f1 --- /dev/null +++ b/spec/models/packages/debian/project_distribution_key_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::ProjectDistributionKey do + it_behaves_like 'Debian Distribution Key', :project +end diff --git a/spec/requests/api/graphql/mutations/labels/create_spec.rb b/spec/requests/api/graphql/mutations/labels/create_spec.rb index ca3ccc8e06c..28284408306 100644 --- a/spec/requests/api/graphql/mutations/labels/create_spec.rb +++ b/spec/requests/api/graphql/mutations/labels/create_spec.rb @@ -11,8 +11,7 @@ RSpec.describe Mutations::Labels::Create do { 'title' => 'foo', 'description' => 'some description', - 'color' => '#FF0000', - 'removeOnClose' => true + 'color' => '#FF0000' } end diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb index 900ffe6dfc7..c677e68b285 100644 --- a/spec/requests/api/group_labels_spec.rb +++ b/spec/requests/api/group_labels_spec.rb @@ -290,7 +290,7 @@ RSpec.describe API::GroupLabels do put api("/groups/#{group.id}/labels", user), params: { name: group_label1.name } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('new_name, color, description, remove_on_close are missing, '\ + expect(json_response['error']).to eq('new_name, color, description are missing, '\ 'at least one parameter must be provided') end end @@ -337,7 +337,7 @@ RSpec.describe API::GroupLabels do put api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}", user) expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('new_name, color, description, remove_on_close are missing, '\ + expect(json_response['error']).to eq('new_name, color, description are missing, '\ 'at least one parameter must be provided') end end diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 38c080059c4..dac721cbea0 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -402,17 +402,6 @@ RSpec.describe API::Issues do expect(response).to have_gitlab_http_status(:ok) expect(json_response['state']).to eq 'opened' end - - it 'removes labels marked to be removed on issue closed' do - removable_label = create(:label, project: project, remove_on_close: true) - create(:label_link, target: issue, label: removable_label) - - put api_for_user, params: { state_event: 'close' } - - expect(issue.reload.label_ids).not_to include(removable_label.id) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['state']).to eq 'closed' - end end describe 'PUT /projects/:id/issues/:issue_iid to update updated_at param' do diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index f2ceedf6dbd..26377c40b73 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -57,7 +57,7 @@ RSpec.describe API::Labels do put_labels_api(route_type, user, spec_params) expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq('new_name, color, description, priority, remove_on_close are missing, '\ + expect(json_response['error']).to eq('new_name, color, description, priority are missing, '\ 'at least one parameter must be provided') end @@ -112,14 +112,6 @@ RSpec.describe API::Labels do expect(json_response['id']).to eq(expected_response_label_id) expect(json_response['priority']).to eq(10) end - - it "returns 200 if remove_on_close is changed (#{route_type} route)" do - put_labels_api(route_type, user, spec_params, remove_on_close: true) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['id']).to eq(expected_response_label_id) - expect(json_response['remove_on_close']).to eq(true) - end end it 'returns 200 if a priority is removed (deprecated route)' do @@ -309,8 +301,7 @@ RSpec.describe API::Labels do name: valid_label_title_2, color: '#FFAABB', description: 'test', - priority: 2, - remove_on_close: true + priority: 2 } expect(response).to have_gitlab_http_status(:created) @@ -318,7 +309,6 @@ RSpec.describe API::Labels do expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to eq('test') expect(json_response['priority']).to eq(2) - expect(json_response['remove_on_close']).to eq(true) end it 'returns created label when only required params' do diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 8950bdd465f..f18c72d3991 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -3,50 +3,22 @@ require 'spec_helper' RSpec.describe Issues::CloseService do - subject(:close_issue) { described_class.new(project: project, current_user: user).close_issue(issue) } - - let_it_be(:project, refind: true) { create(:project, :repository) } - let_it_be(:label1) { create(:label, project: project) } - let_it_be(:label2) { create(:label, project: project, remove_on_close: true) } - let_it_be(:author) { create(:user) } - let_it_be(:user) { create(:user, email: "user@example.com") } - let_it_be(:user2) { create(:user, email: "user2@example.com") } - let_it_be(:guest) { create(:user) } - let_it_be(:closing_merge_request) { create(:merge_request, source_project: project) } - + let(:project) { create(:project, :repository) } + let(:user) { create(:user, email: "user@example.com") } + let(:user2) { create(:user, email: "user2@example.com") } + let(:guest) { create(:user) } + let(:issue) { create(:issue, title: "My issue", project: project, assignees: [user2], author: create(:user)) } let(:external_issue) { ExternalIssue.new('JIRA-123', project) } - let!(:issue) { create(:issue, title: "My issue", project: project, assignees: [user2], author: author) } + let(:closing_merge_request) { create(:merge_request, source_project: project) } + let(:closing_commit) { create(:commit, project: project) } + let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } - before_all do + before do project.add_maintainer(user) project.add_developer(user2) project.add_guest(guest) end - shared_examples 'removes labels marked for removal from issue when closed' do - before do - issue.update!(label_ids: [label1.id, label2.id]) - end - - it 'removes labels marked for removal' do - expect do - close_issue - end.to change { issue.reload.label_ids }.from(containing_exactly(label1.id, label2.id)).to(containing_exactly(label1.id)) - end - - it 'creates system notes for the removed labels' do - expect do - close_issue - end.to change(ResourceLabelEvent, :count).by(1) - - expect(ResourceLabelEvent.last.slice(:action, :issue_id, :label_id)).to eq( - 'action' => 'remove', - 'issue_id' => issue.id, - 'label_id' => label2.id - ) - end - end - describe '#execute' do let(:service) { described_class.new(project: project, current_user: user) } @@ -149,8 +121,6 @@ RSpec.describe Issues::CloseService do end end - it_behaves_like 'removes labels marked for removal from issue when closed' - it 'mentions closure via a merge request' do close_issue @@ -214,18 +184,10 @@ RSpec.describe Issues::CloseService do end context "closed by a commit", :sidekiq_might_not_need_inline do - subject(:close_issue) do + it 'mentions closure via a commit' do perform_enqueued_jobs do described_class.new(project: project, current_user: user).close_issue(issue, closed_via: closing_commit) end - end - - let(:closing_commit) { create(:commit, project: project) } - - it_behaves_like 'removes labels marked for removal from issue when closed' - - it 'mentions closure via a commit' do - close_issue email = ActionMailer::Base.deliveries.last @@ -237,8 +199,9 @@ RSpec.describe Issues::CloseService do context 'when user cannot read the commit' do it 'does not mention the commit id' do project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED) - - close_issue + perform_enqueued_jobs do + described_class.new(project: project, current_user: user).close_issue(issue, closed_via: closing_commit) + end email = ActionMailer::Base.deliveries.last body_text = email.body.parts.map(&:body).join(" ") @@ -259,14 +222,12 @@ RSpec.describe Issues::CloseService do it 'verifies the number of queries' do recorded = ActiveRecord::QueryRecorder.new { close_issue } - expected_queries = 32 + expected_queries = 23 expect(recorded.count).to be <= expected_queries expect(recorded.cached_count).to eq(0) end - it_behaves_like 'removes labels marked for removal from issue when closed' - it 'closes the issue' do close_issue @@ -296,8 +257,6 @@ RSpec.describe Issues::CloseService do end it 'marks todos as done' do - todo = create(:todo, :assigned, user: user, project: project, target: issue, author: user2) - close_issue expect(todo.reload).to be_done @@ -362,32 +321,26 @@ RSpec.describe Issues::CloseService do end context 'when issue is not confidential' do - it_behaves_like 'removes labels marked for removal from issue when closed' - it 'executes issue hooks' do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) - close_issue + described_class.new(project: project, current_user: user).close_issue(issue) end end context 'when issue is confidential' do - let(:issue) { create(:issue, :confidential, project: project) } - - it_behaves_like 'removes labels marked for removal from issue when closed' - it 'executes confidential issue hooks' do + issue = create(:issue, :confidential, project: project) + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) - close_issue + described_class.new(project: project, current_user: user).close_issue(issue) end end context 'internal issues disabled' do - let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } - before do project.issues_enabled = false project.save! diff --git a/spec/support/shared_examples/models/packages/debian/distribution_key_shared_examples.rb b/spec/support/shared_examples/models/packages/debian/distribution_key_shared_examples.rb new file mode 100644 index 00000000000..26794c83736 --- /dev/null +++ b/spec/support/shared_examples/models/packages/debian/distribution_key_shared_examples.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'Debian Distribution Key' do |container| + let_it_be_with_refind(:distribution_key) { create("debian_#{container}_distribution_key") } # rubocop:disable Rails/SaveBang + + subject { distribution_key } + + describe 'relationships' do + it { is_expected.to belong_to(:distribution).class_name("Packages::Debian::#{container.capitalize}Distribution").inverse_of(:key) } + end + + describe 'validations' do + describe "#distribution" do + it { is_expected.to validate_presence_of(:distribution) } + end + + describe '#private_key' do + it { is_expected.to validate_presence_of(:private_key) } + + it { is_expected.to allow_value("-----BEGIN PGP PRIVATE KEY BLOCK-----\n...").for(:private_key) } + it { is_expected.not_to allow_value('A').for(:private_key).with_message('must be ASCII armored') } + end + + describe '#passphrase' do + it { is_expected.to validate_presence_of(:passphrase) } + + it { is_expected.to allow_value('P@$$w0rd').for(:passphrase) } + it { is_expected.to allow_value('A' * 255).for(:passphrase) } + it { is_expected.not_to allow_value('A' * 256).for(:passphrase) } + end + + describe '#public_key' do + it { is_expected.to validate_presence_of(:public_key) } + + it { is_expected.to allow_value("-----BEGIN PGP PUBLIC KEY BLOCK-----\n...").for(:public_key) } + it { is_expected.not_to allow_value('A').for(:public_key).with_message('must be ASCII armored') } + end + + describe '#fingerprint' do + it { is_expected.to validate_presence_of(:passphrase) } + + it { is_expected.to allow_value('abc').for(:passphrase) } + it { is_expected.to allow_value('A' * 255).for(:passphrase) } + it { is_expected.not_to allow_value('A' * 256).for(:passphrase) } + end + end +end diff --git a/spec/support/shared_examples/models/packages/debian/distribution_shared_examples.rb b/spec/support/shared_examples/models/packages/debian/distribution_shared_examples.rb index 8693d6868e9..5459d17b1df 100644 --- a/spec/support/shared_examples/models/packages/debian/distribution_shared_examples.rb +++ b/spec/support/shared_examples/models/packages/debian/distribution_shared_examples.rb @@ -17,6 +17,7 @@ RSpec.shared_examples 'Debian Distribution' do |factory, container, can_freeze| it { is_expected.to belong_to(container) } it { is_expected.to belong_to(:creator).class_name('User') } + it { is_expected.to have_one(:key).class_name("Packages::Debian::#{container.capitalize}DistributionKey").with_foreign_key(:distribution_id).inverse_of(:distribution) } it { is_expected.to have_many(:components).class_name("Packages::Debian::#{container.capitalize}Component").inverse_of(:distribution) } it { is_expected.to have_many(:architectures).class_name("Packages::Debian::#{container.capitalize}Architecture").inverse_of(:distribution) } end diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go index d8e2a7b0d9f..129de1fd7b3 100644 --- a/workhorse/internal/api/api.go +++ b/workhorse/internal/api/api.go @@ -308,7 +308,7 @@ func (api *API) PreAuthorizeHandler(next HandleFunc, suffix string) http.Handler return } - httpResponse.Body.Close() // Free up the Unicorn worker + httpResponse.Body.Close() // Free up the Puma thread copyAuthHeader(httpResponse, w) @@ -347,7 +347,7 @@ func copyAuthHeader(httpResponse *http.Response, w http.ResponseWriter) { func passResponseBack(httpResponse *http.Response, w http.ResponseWriter, r *http.Request) { // NGINX response buffering is disabled on this path (with - // X-Accel-Buffering: no) but we still want to free up the Unicorn worker + // X-Accel-Buffering: no) but we still want to free up the Puma thread // that generated httpResponse as fast as possible. To do this we buffer // the entire response body in memory before sending it on. responseBody, err := bufferResponse(httpResponse.Body) @@ -355,7 +355,7 @@ func passResponseBack(httpResponse *http.Response, w http.ResponseWriter, r *htt helper.Fail500(w, r, err) return } - httpResponse.Body.Close() // Free up the Unicorn worker + httpResponse.Body.Close() // Free up the Puma thread bytesTotal.Add(float64(responseBody.Len())) for k, v := range httpResponse.Header { |