diff --git a/.gitlab/ci/dast.gitlab-ci.yml b/.gitlab/ci/dast.gitlab-ci.yml index 1b45fff7ec7..93f64930822 100644 --- a/.gitlab/ci/dast.gitlab-ci.yml +++ b/.gitlab/ci/dast.gitlab-ci.yml @@ -35,6 +35,8 @@ stage: dast # Default job timeout set to 90m and dast rules needs 2h to so that it won't timeout. timeout: 2h + # Add retry because of intermittent connection problems. See https://gitlab.com/gitlab-org/gitlab/-/issues/244313 + retry: 1 artifacts: paths: - gl-dast-report.json # GitLab-specific diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 813a55eb3b5..53abdf5bb38 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -505,6 +505,22 @@ rspec-ee system pg12 geo: ################################################## # EE: Canonical MR pipelines +rspec fail-fast: + extends: + - .rspec-base-pg11 + - .rails:rules:rspec fail-fast + stage: test + needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets", "detect-tests"] + script: + - run_timed_command "scripts/gitaly-test-build" + - run_timed_command "scripts/gitaly-test-spawn" + - source scripts/rspec_helpers.sh + - rspec_fail_fast tmp/matching_tests.txt "--tag ~quarantine" + artifacts: + expire_in: 7d + paths: + - tmp/capybara/ + rspec foss-impact: extends: - .rspec-base-pg11-as-if-foss @@ -520,5 +536,19 @@ rspec foss-impact: paths: - tmp/capybara/ +fail-pipeline-early: + extends: + - .rails:rules:fail-pipeline-early + stage: post-test + needs: + - job: rspec fail-fast + artifacts: false + variables: + GIT_DEPTH: 1 + before_script: + - source scripts/utils.sh + - install_api_client_dependencies_with_apt + script: + - fail_pipeline_early # EE: Canonical MR pipelines ################################################## diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 5b9b5e09444..f6e606e0534 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -67,6 +67,12 @@ .if-cache-credentials-schedule: &if-cache-credentials-schedule if: '$CI_REPO_CACHE_CREDENTIALS && $CI_PIPELINE_SOURCE == "schedule"' +.if-rspec-fail-fast-disabled: &if-rspec-fail-fast-disabled + if: '$RSPEC_FAIL_FAST_ENABLED != "true"' + +.if-rspec-fail-fast-skipped: &if-rspec-fail-fast-skipped + if: '$CI_MERGE_REQUEST_TITLE =~ /SKIP RSPEC FAIL-FAST/' + #################### # Changes patterns # #################### @@ -579,6 +585,34 @@ - <<: *if-dot-com-gitlab-org-merge-request changes: *code-backstage-patterns +.rails:rules:rspec fail-fast: + rules: + - <<: *if-rspec-fail-fast-disabled + when: never + - <<: *if-rspec-fail-fast-skipped + when: never + - <<: *if-not-ee + when: never + - <<: *if-security-merge-request + changes: *code-backstage-patterns + - <<: *if-dot-com-gitlab-org-merge-request + changes: *code-backstage-patterns + +.rails:rules:fail-pipeline-early: + rules: + - <<: *if-rspec-fail-fast-disabled + when: never + - <<: *if-rspec-fail-fast-skipped + when: never + - <<: *if-not-ee + when: never + - <<: *if-security-merge-request + changes: *code-backstage-patterns + when: on_failure + - <<: *if-dot-com-gitlab-org-merge-request + changes: *code-backstage-patterns + when: on_failure + .rails:rules:downtime_check: rules: - <<: *if-merge-request diff --git a/app/assets/javascripts/lib/utils/axios_startup_calls.js b/app/assets/javascripts/lib/utils/axios_startup_calls.js index a047cebc8ab..642bd948b61 100644 --- a/app/assets/javascripts/lib/utils/axios_startup_calls.js +++ b/app/assets/javascripts/lib/utils/axios_startup_calls.js @@ -33,6 +33,9 @@ const setupAxiosStartupCalls = axios => { fetchHeaders[key] = val; }); + // We can delete it as it anyhow should only be called once + delete startupCalls[fullUrl]; + // eslint-disable-next-line promise/no-nesting return res .clone() diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index cfe674f9c52..54fcf41ca50 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -397,7 +397,7 @@ export default { :secondary-button-text="__('Cancel')" variant="warning" :dismissible="false" - @primaryAction="forceCloseIssue" + @primaryAction="toggleBlockedIssueWarning(false) && forceCloseIssue()" @secondaryAction="toggleBlockedIssueWarning(false) && enableButton()" >

diff --git a/app/graphql/resolvers/group_members_resolver.rb b/app/graphql/resolvers/group_members_resolver.rb new file mode 100644 index 00000000000..f34c873a8a9 --- /dev/null +++ b/app/graphql/resolvers/group_members_resolver.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Resolvers + class GroupMembersResolver < MembersResolver + authorize :read_group_member + + private + + def preloads + { + user: [:user, :source] + } + end + + def finder_class + GroupMembersFinder + end + end +end diff --git a/app/graphql/resolvers/members_resolver.rb b/app/graphql/resolvers/members_resolver.rb new file mode 100644 index 00000000000..88a1ab71c45 --- /dev/null +++ b/app/graphql/resolvers/members_resolver.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Resolvers + class MembersResolver < BaseResolver + include Gitlab::Graphql::Authorize::AuthorizeResource + include LooksAhead + + argument :search, GraphQL::STRING_TYPE, + required: false, + description: 'Search query' + + def resolve_with_lookahead(**args) + authorize!(object) + + apply_lookahead(finder_class.new(object, current_user, params: args).execute) + end + + private + + def finder_class + # override in subclass + end + end +end diff --git a/app/graphql/resolvers/project_members_resolver.rb b/app/graphql/resolvers/project_members_resolver.rb index 7ee635014f4..1ca4e81f397 100644 --- a/app/graphql/resolvers/project_members_resolver.rb +++ b/app/graphql/resolvers/project_members_resolver.rb @@ -1,25 +1,15 @@ # frozen_string_literal: true module Resolvers - class ProjectMembersResolver < BaseResolver - include Gitlab::Graphql::Authorize::AuthorizeResource - - argument :search, GraphQL::STRING_TYPE, - required: false, - description: 'Search query' - + class ProjectMembersResolver < MembersResolver type Types::MemberInterface, null: true authorize :read_project_member - alias_method :project, :object - - def resolve(**args) - authorize!(project) + private + def finder_class MembersFinder - .new(project, current_user, params: args) - .execute end end end diff --git a/app/graphql/types/base_enum.rb b/app/graphql/types/base_enum.rb index 94f6c47e876..159443641bc 100644 --- a/app/graphql/types/base_enum.rb +++ b/app/graphql/types/base_enum.rb @@ -2,9 +2,12 @@ module Types class BaseEnum < GraphQL::Schema::Enum + extend GitlabStyleDeprecations + class << self def value(*args, **kwargs, &block) enum[args[0].downcase] = kwargs[:value] || args[0] + kwargs = gitlab_deprecation(kwargs) super(*args, **kwargs, &block) end diff --git a/app/graphql/types/base_field.rb b/app/graphql/types/base_field.rb index c8ec4b3f897..90f5004dd8f 100644 --- a/app/graphql/types/base_field.rb +++ b/app/graphql/types/base_field.rb @@ -3,6 +3,7 @@ module Types class BaseField < GraphQL::Schema::Field prepend Gitlab::Graphql::Authorize + include GitlabStyleDeprecations DEFAULT_COMPLEXITY = 1 @@ -12,7 +13,7 @@ module Types kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity]) @feature_flag = kwargs[:feature_flag] kwargs = check_feature_flag(kwargs) - kwargs = handle_deprecated(kwargs) + kwargs = gitlab_deprecation(kwargs) super(*args, **kwargs, &block) end @@ -52,28 +53,6 @@ module Types args end - def handle_deprecated(kwargs) - if kwargs[:deprecation_reason].present? - raise ArgumentError, 'Use `deprecated` property instead of `deprecation_reason`. ' \ - 'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields' - end - - deprecation = kwargs.delete(:deprecated) - return kwargs unless deprecation - - milestone, reason = deprecation.values_at(:milestone, :reason).map(&:presence) - - raise ArgumentError, 'Please provide a `milestone` within `deprecated`' unless milestone - raise ArgumentError, 'Please provide a `reason` within `deprecated`' unless reason - raise ArgumentError, '`milestone` must be a `String`' unless milestone.is_a?(String) - - deprecated_in = "Deprecated in #{milestone}" - kwargs[:deprecation_reason] = "#{reason}. #{deprecated_in}" - kwargs[:description] += ". #{deprecated_in}: #{reason}" if kwargs[:description] - - kwargs - end - def field_complexity(resolver_class, current) return current if current.present? && current > 0 diff --git a/app/graphql/types/concerns/gitlab_style_deprecations.rb b/app/graphql/types/concerns/gitlab_style_deprecations.rb new file mode 100644 index 00000000000..2c932f4214b --- /dev/null +++ b/app/graphql/types/concerns/gitlab_style_deprecations.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# Concern for handling deprecation arguments. +# https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields-and-enum-values +module GitlabStyleDeprecations + extend ActiveSupport::Concern + + private + + def gitlab_deprecation(kwargs) + if kwargs[:deprecation_reason].present? + raise ArgumentError, 'Use `deprecated` property instead of `deprecation_reason`. ' \ + 'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields-and-enum-values' + end + + deprecation = kwargs.delete(:deprecated) + return kwargs unless deprecation + + milestone, reason = deprecation.values_at(:milestone, :reason).map(&:presence) + + raise ArgumentError, 'Please provide a `milestone` within `deprecated`' unless milestone + raise ArgumentError, 'Please provide a `reason` within `deprecated`' unless reason + raise ArgumentError, '`milestone` must be a `String`' unless milestone.is_a?(String) + + deprecated_in = "Deprecated in #{milestone}" + kwargs[:deprecation_reason] = "#{reason}. #{deprecated_in}" + kwargs[:description] += ". #{deprecated_in}: #{reason}" if kwargs[:description] + + kwargs + end +end diff --git a/app/graphql/types/group_type.rb b/app/graphql/types/group_type.rb index cc8cd7c01f9..60b2e3c7b6e 100644 --- a/app/graphql/types/group_type.rb +++ b/app/graphql/types/group_type.rb @@ -75,6 +75,12 @@ module Types description: 'Title of the label' end + field :group_members, + Types::GroupMemberType.connection_type, + description: 'A membership of a user within this group', + extras: [:lookahead], + resolver: Resolvers::GroupMembersResolver + def label(title:) BatchLoader::GraphQL.for(title).batch(key: group) do |titles, loader, args| LabelsFinder diff --git a/app/graphql/types/member_interface.rb b/app/graphql/types/member_interface.rb index fed7272c0e3..615a45413cb 100644 --- a/app/graphql/types/member_interface.rb +++ b/app/graphql/types/member_interface.rb @@ -23,8 +23,7 @@ module Types description: 'Date and time the membership expires' field :user, Types::UserType, null: false, - description: 'User that is associated with the member object', - resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, obj.user_id).find } + description: 'User that is associated with the member object' definition_methods do def resolve_type(object, context) diff --git a/app/models/analytics/instance_statistics/measurement.rb b/app/models/analytics/instance_statistics/measurement.rb index fd917632e15..eaaf9e999b3 100644 --- a/app/models/analytics/instance_statistics/measurement.rb +++ b/app/models/analytics/instance_statistics/measurement.rb @@ -12,6 +12,15 @@ module Analytics pipelines: 6 } + IDENTIFIER_QUERY_MAPPING = { + identifiers[:projects] => -> { Project }, + identifiers[:users] => -> { User }, + identifiers[:issues] => -> { Issue }, + identifiers[:merge_requests] => -> { MergeRequest }, + identifiers[:groups] => -> { Group }, + identifiers[:pipelines] => -> { Ci::Pipeline } + }.freeze + validates :recorded_at, :identifier, :count, presence: true validates :recorded_at, uniqueness: { scope: :identifier } diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index a5218ac33d7..b660c814881 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -22,7 +22,9 @@ class ApplicationSetting < ApplicationRecord belongs_to :push_rule alias_attribute :self_monitoring_project_id, :instance_administration_project_id - belongs_to :instance_administrators_group, class_name: "Group" + belongs_to :instance_group, class_name: "Group", foreign_key: 'instance_administrators_group_id' + alias_attribute :instance_group_id, :instance_administrators_group_id + alias_attribute :instance_administrators_group, :instance_group def self.repository_storages_weighted_attributes @repository_storages_weighted_atributes ||= Gitlab.config.repositories.storages.keys.map { |k| "repository_storages_weighted_#{k}".to_sym }.freeze diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 4da752fe474..c98e82efef7 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -80,6 +80,7 @@ class GroupPolicy < BasePolicy enable :read_list enable :read_label enable :read_board + enable :read_group_member end rule { ~can?(:read_group) }.policy do diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index 7206eaf51b2..e616aeec798 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -18,7 +18,6 @@ module IncidentManagement current_user, title: title, description: description, - label_ids: [find_or_create_incident_label.id], issue_type: ISSUE_TYPE ).execute @@ -31,13 +30,6 @@ module IncidentManagement attr_reader :title, :description - def find_or_create_incident_label - IncidentManagement::CreateIncidentLabelService - .new(project, current_user) - .execute - .payload[:label] - end - def success(issue) ServiceResponse.success(payload: { issue: issue }) end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 943879e12c1..0ed2b08b7b1 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -61,6 +61,22 @@ module Issues Milestones::IssuesCountService.new(milestone).delete_cache end + + # Applies label "incident" (creates it if missing) to incident issues. + # Please use in "after" hooks only to ensure we are not appyling + # labels prematurely. + def add_incident_label(issue) + return unless issue.incident? + + label = ::IncidentManagement::CreateIncidentLabelService + .new(project, current_user) + .execute + .payload[:label] + + return if issue.label_ids.include?(label.id) + + issue.labels << label + end end end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 474fd974f90..a9be38ee7e6 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -25,12 +25,13 @@ module Issues end end - def after_create(issuable) - todo_service.new_issue(issuable, current_user) + def after_create(issue) + add_incident_label(issue) + todo_service.new_issue(issue, current_user) user_agent_detail_service.create - resolve_discussions_with_issue(issuable) - delete_milestone_total_issue_counter_cache(issuable.milestone) - track_incident_action(current_user, issuable, :incident_created) + resolve_discussions_with_issue(issue) + delete_milestone_total_issue_counter_cache(issue.milestone) + track_incident_action(current_user, issue, :incident_created) super end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index cc71b306ade..bff8501d7d4 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -22,6 +22,7 @@ module Issues end def after_update(issue) + add_incident_label(issue) IssuesChannel.broadcast_to(issue, event: 'updated') if Gitlab::ActionCable::Config.in_app? || Feature.enabled?(:broadcast_issue_updates, issue.project) end diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 24605949499..09ec49a040a 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -4,6 +4,7 @@ - issuable_type = issuable_sidebar[:type] - signed_in = !!issuable_sidebar.dig(:current_user, :id) - can_edit_issuable = issuable_sidebar.dig(:current_user, :can_edit) +- add_page_startup_api_call "#{issuable_sidebar[:issuable_json_path]}?serializer=sidebar_extras" - if Feature.enabled?(:vue_issuable_sidebar, @project.group) %aside#js-vue-issuable-sidebar{ data: { signed_in: signed_in, diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index de81d7e5fe6..451decce9fb 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -115,6 +115,14 @@ :weight: 1 :idempotent: :tags: [] +- :name: cronjob:analytics_instance_statistics_count_job_trigger + :feature_category: :instance_statistics + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: cronjob:authorized_project_update_periodic_recalculate :feature_category: :source_code_management :has_external_dependencies: @@ -1204,6 +1212,14 @@ :weight: 1 :idempotent: true :tags: [] +- :name: analytics_instance_statistics_counter_job + :feature_category: :instance_statistics + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: authorized_keys :feature_category: :source_code_management :has_external_dependencies: diff --git a/app/workers/analytics/instance_statistics/count_job_trigger_worker.rb b/app/workers/analytics/instance_statistics/count_job_trigger_worker.rb new file mode 100644 index 00000000000..8580802fb7e --- /dev/null +++ b/app/workers/analytics/instance_statistics/count_job_trigger_worker.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Analytics + module InstanceStatistics + class CountJobTriggerWorker + include ApplicationWorker + include CronjobQueue # rubocop:disable Scalability/CronWorkerContext + + DEFAULT_DELAY = 3.minutes.freeze + + feature_category :instance_statistics + urgency :low + + idempotent! + + def perform + return if Feature.disabled?(:store_instance_statistics_measurements) + + recorded_at = Time.zone.now + measurement_identifiers = Analytics::InstanceStatistics::Measurement.identifiers + + worker_arguments = Gitlab::Analytics::InstanceStatistics::WorkersArgumentBuilder.new( + measurement_identifiers: measurement_identifiers.values, + recorded_at: recorded_at + ).execute + + perform_in = DEFAULT_DELAY.minutes.from_now + worker_arguments.each do |args| + CounterJobWorker.perform_in(perform_in, *args) + + perform_in += DEFAULT_DELAY + end + end + end + end +end diff --git a/app/workers/analytics/instance_statistics/counter_job_worker.rb b/app/workers/analytics/instance_statistics/counter_job_worker.rb new file mode 100644 index 00000000000..062b5ccc207 --- /dev/null +++ b/app/workers/analytics/instance_statistics/counter_job_worker.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Analytics + module InstanceStatistics + class CounterJobWorker + include ApplicationWorker + + feature_category :instance_statistics + urgency :low + + idempotent! + + def perform(measurement_identifier, min_id, max_id, recorded_at) + query_scope = ::Analytics::InstanceStatistics::Measurement::IDENTIFIER_QUERY_MAPPING[measurement_identifier].call + + count = if min_id.nil? || max_id.nil? # table is empty + 0 + else + Gitlab::Database::BatchCount.batch_count(query_scope, start: min_id, finish: max_id) + end + + return if count == Gitlab::Database::BatchCounter::FALLBACK + + InstanceStatistics::Measurement.insert_all([{ recorded_at: recorded_at, count: count, identifier: measurement_identifier }]) + end + end + end +end diff --git a/changelogs/unreleased/233856-cablett-group-users.yml b/changelogs/unreleased/233856-cablett-group-users.yml new file mode 100644 index 00000000000..047e29c97a7 --- /dev/null +++ b/changelogs/unreleased/233856-cablett-group-users.yml @@ -0,0 +1,5 @@ +--- +title: Expose group memberships under group via GraphQL +merge_request: 39331 +author: +type: added diff --git a/changelogs/unreleased/pl-add-incident-label-for-incidents.yml b/changelogs/unreleased/pl-add-incident-label-for-incidents.yml new file mode 100644 index 00000000000..c8998570a54 --- /dev/null +++ b/changelogs/unreleased/pl-add-incident-label-for-incidents.yml @@ -0,0 +1,5 @@ +--- +title: Add incident label for manually created incident issues +merge_request: 41598 +author: +type: fixed diff --git a/changelogs/unreleased/rp-change-instance-administrators-name.yml b/changelogs/unreleased/rp-change-instance-administrators-name.yml new file mode 100644 index 00000000000..37cf711c3fa --- /dev/null +++ b/changelogs/unreleased/rp-change-instance-administrators-name.yml @@ -0,0 +1,5 @@ +--- +title: Change name of GitLab Instance Administrators group to GitLab Instance +merge_request: 41684 +author: +type: changed diff --git a/config/application.rb b/config/application.rb index 397066bd292..4d2f3745b52 100644 --- a/config/application.rb +++ b/config/application.rb @@ -49,7 +49,8 @@ module Gitlab #{config.root}/app/models/members #{config.root}/app/models/project_services #{config.root}/app/graphql/resolvers/concerns - #{config.root}/app/graphql/mutations/concerns]) + #{config.root}/app/graphql/mutations/concerns + #{config.root}/app/graphql/types/concerns]) config.generators.templates.push("#{config.root}/generator_templates") diff --git a/config/feature_categories.yml b/config/feature_categories.yml index e41d6d86e46..7b85f910d85 100644 --- a/config/feature_categories.yml +++ b/config/feature_categories.yml @@ -65,6 +65,7 @@ - integrations - interactive_application_security_testing - internationalization +- instance_statistics - issue_tracking - jenkins_importer - jira_importer diff --git a/config/feature_flags/development/store_instance_statistics_measurements.yml b/config/feature_flags/development/store_instance_statistics_measurements.yml new file mode 100644 index 00000000000..bc7b3400694 --- /dev/null +++ b/config/feature_flags/development/store_instance_statistics_measurements.yml @@ -0,0 +1,7 @@ +--- +name: store_instance_statistics_measurements +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41300 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/247871 +group: group::analytics +type: development +default_enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 9c069ef7796..0334d37a8da 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -514,6 +514,9 @@ Settings.cron_jobs['postgres_dynamic_partitions_creator']['job_class'] ||= 'Part Settings.cron_jobs['ci_platform_metrics_update_cron_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['ci_platform_metrics_update_cron_worker']['cron'] ||= '47 9 * * *' Settings.cron_jobs['ci_platform_metrics_update_cron_worker']['job_class'] = 'CiPlatformMetricsUpdateCronWorker' +Settings.cron_jobs['analytics_instance_statistics_count_job_trigger_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['analytics_instance_statistics_count_job_trigger_worker']['cron'] ||= '50 23 */1 * *' +Settings.cron_jobs['analytics_instance_statistics_count_job_trigger_worker']['job_class'] ||= 'Analytics::InstanceStatistics::CountJobTriggerWorker' Gitlab.ee do Settings.cron_jobs['adjourned_group_deletion_worker'] ||= Settingslogic.new({}) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index adf2e46869e..cb85b5b6d1e 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -30,6 +30,8 @@ - 1 - - analytics_code_review_metrics - 1 +- - analytics_instance_statistics_counter_job + - 1 - - authorized_keys - 2 - - authorized_project_update diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index c60da31f093..16999e21a2a 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -6711,6 +6711,36 @@ type Group { """ fullPath: ID! + """ + A membership of a user within this group + """ + groupMembers( + """ + Returns the elements in the list that come after the specified cursor. + """ + after: String + + """ + Returns the elements in the list that come before the specified cursor. + """ + before: String + + """ + Returns the first _n_ elements from the list. + """ + first: Int + + """ + Returns the last _n_ elements from the list. + """ + last: Int + + """ + Search query + """ + search: String + ): GroupMemberConnection + """ Indicates if Group timelogs are enabled for namespace """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index dd88ba59a04..d3a9d721ebb 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -18665,6 +18665,69 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "groupMembers", + "description": "A membership of a user within this group", + "args": [ + { + "name": "search", + "description": "Search query", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "after", + "description": "Returns the elements in the list that come after the specified cursor.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "before", + "description": "Returns the elements in the list that come before the specified cursor.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "first", + "description": "Returns the first _n_ elements from the list.", + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "last", + "description": "Returns the last _n_ elements from the list.", + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "defaultValue": null + } + ], + "type": { + "kind": "OBJECT", + "name": "GroupMemberConnection", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "groupTimelogsEnabled", "description": "Indicates if Group timelogs are enabled for namespace", diff --git a/doc/ci/examples/authenticating-with-hashicorp-vault/index.md b/doc/ci/examples/authenticating-with-hashicorp-vault/index.md index 9a87786ec70..91342655aa5 100644 --- a/doc/ci/examples/authenticating-with-hashicorp-vault/index.md +++ b/doc/ci/examples/authenticating-with-hashicorp-vault/index.md @@ -9,6 +9,12 @@ type: tutorial This tutorial demonstrates how to authenticate, configure, and read secrets with HashiCorp's Vault from GitLab CI/CD. +NOTE: **Note:** +[GitLab Premium](https://about.gitlab.com/pricing/) supports read access to a +Hashicorp Vault, and enables you to +[use Vault secrets in a CI job](../../secrets/index.md#use-vault-secrets-in-a-ci-job-premium). +To learn more, read [Using external secrets in CI](../../secrets/index.md). + ## Requirements This tutorial assumes you are familiar with GitLab CI/CD and Vault. diff --git a/doc/ci/secrets/index.md b/doc/ci/secrets/index.md new file mode 100644 index 00000000000..7a9743503c6 --- /dev/null +++ b/doc/ci/secrets/index.md @@ -0,0 +1,157 @@ +--- +stage: Release +group: Release Management +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +type: concepts, howto +--- + +# Using external secrets in CI + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/218746) in GitLab 13.4 and GitLab Runner 13.4. + +Secrets represent sensitive information your CI job needs to complete work. This +sensitive information can be items like API tokens, database credentials, or private keys. +Secrets are sourced from your secrets provider. + +Unlike CI variables, which are always presented to a job, secrets must be explicitly +required by a job. Read [GitLab CI/CD pipeline configuration reference](../yaml/README.md#secrets) +for more information about the syntax. + +GitLab has selected [Vault by Hashicorp](https://www.vaultproject.io) as the +first supported provider, and [KV-V2](https://www.vaultproject.io/docs/secrets/kv/kv-v2) +as the first supported secrets engine. + +GitLab authenticates using Vault's +[JWT Auth method](https://www.vaultproject.io/docs/auth/jwt#jwt-authentication), using +the [JSON Web Token](https://gitlab.com/gitlab-org/gitlab/-/issues/207125) (`CI_JOB_JWT`) +introduced in GitLab 12.10. + +You must [configure your Vault server](#configure-your-vault-server) before you +can use [use Vault secrets in a CI job](#use-vault-secrets-in-a-ci-job-premium). + +NOTE: **Note:** +Read the [Authenticating and Reading Secrets With Hashicorp Vault](../examples/authenticating-with-hashicorp-vault/index.md) +tutorial for a version of this feature that is available to all +subscription levels, supports writing secrets to and deleting secrets from Vault, +and multiple secrets engines. + +## Configure your Vault server + +To configure your Vault server: + +1. Enable the authentication method by running these commands. They provide your Vault + server the [JSON Web Key Set](https://tools.ietf.org/html/rfc7517) (JWKS) endpoint for your GitLab instance, so Vault + can fetch the public signing key and verify the JSON Web Token (JWT) when authenticating: + + ```shell + $ vault auth enable jwt + + $ vault write auth/jwt/config \ + jwks_url="https://gitlab.example.com/-/jwks" \ + bound_issuer="gitlab.example.com" + ``` + +1. Configure policies on your Vault server to grant or forbid access to certain + paths and operations. This example grants read access to the set of secrets + required by your production environment: + + ```shell + vault policy write myproject-production - < - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/28321) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.4 and GitLab Runner 13.4. + +After [configuring your Vault server](#configure-your-vault-server), you can use +the secrets stored in Vault by defining them with the `vault` keyword: + +```yaml +secrets: + DATABASE_PASSWORD: + vault: production/db/password@ops # translates to secret `ops/data/production/db`, field `password` +``` + +In this example: + +- `production/db` - The secret. +- `password` The field. +- `ops` - The path where the secrets engine is mounted. + +After GitLab fetches the secret from Vault, the value is saved in a temporary file. +The path to this file is stored in environment variable named `DATABASE_PASSWORD`, +similar to [CI variables of type `file`](../variables/README.md#custom-environment-variables-of-type-file). + +For more information about the supported syntax, read the +[`.gitlab-ci.yml` reference](../yaml/README.md#secretsvault-premium). + +## Configure Vault server roles + +When a CI job attempts to authenticate, it specifies a role. You can use roles to group +different policies together. If authentication is successful, these policies are +attached to the resulting Vault token. + +[Bound claims](https://www.vaultproject.io/docs/auth/jwt#bound-claims) are predefined +values that are matched to the JWT's claims. With bounded claims, you can restrict access +to specific GitLab users, specific projects, or even jobs running for specific Git +references. You can have as many bounded claims you need, but they must *all* match +for authentication to be successful. + +Combining bounded claims with GitLab features like [user roles](../../user/permissions.md) +and [protected branches](../../user/project/protected_branches.md), you can tailor +these rules to fit your specific use case. In this example, authentication is allowed +only for jobs running for protected tags with names matching the pattern used for +production releases: + +```shell +$ vault write auth/jwt/role/myproject-production - < [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33014) in GitLab 13.4. + +`secrets` indicates the [CI Secrets](../secrets/index.md) this job needs. It should be a hash, +and the keys should be the names of the environment variables the job needs to access the secrets. + +#### `secrets:vault` **(PREMIUM)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/28321) in GitLab 13.4. + +`vault` keyword specifies secrets provided by [Hashicorp's Vault](https://www.vaultproject.io/). +This syntax has multiple forms. The shortest form asssumes the use of the +[KV-V2](https://www.vaultproject.io/docs/secrets/kv/kv-v2) secrets engine, +mounted at the default path `kv-v2`. The last part of the secret's path is the +field to fetch the value for: + +```yaml +job: + secrets: + DATABASE_PASSWORD: + vault: production/db/password # translates to secret `kv-v2/data/production/db`, field `password` +``` + +You can specify a custom secrets engine path by adding a suffix starting with `@`: + +```yaml +job: + secrets: + DATABASE_PASSWORD: + vault: production/db/password@ops # translates to secret `ops/data/production/db`, field `password` +``` + +In the detailed form of the syntax, you can specify all details explicitly: + +```yaml +job: + secrets: + DATABASE_PASSWORD: # translates to secret `ops/data/production/db`, field `password` + vault: + engine: + name: kv-v2 + path: ops + path: production/db + field: password +``` + ### `pages` `pages` is a special job that is used to upload static content to GitLab that diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index c7610996019..76fb895cc5e 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -366,16 +366,16 @@ def foo end ``` -## Deprecating fields +## Deprecating fields and enum values GitLab's GraphQL API is versionless, which means we maintain backwards compatibility with older versions of the API with every change. Rather -than removing a field, we need to _deprecate_ the field instead. In -future, GitLab -[may remove deprecated fields](https://gitlab.com/gitlab-org/gitlab/-/issues/32292). +than removing a field or [enum value](#enums), we need to _deprecate_ it instead. +In future, GitLab +[may remove deprecated parts of the schema](https://gitlab.com/gitlab-org/gitlab/-/issues/32292). -Fields are deprecated using the `deprecated` property. The value -of the property is a `Hash` of: +Fields and enum values are deprecated using the `deprecated` property. +The value of the property is a `Hash` of: - `reason` - Reason for the deprecation. - `milestone` - Milestone that the field was deprecated. @@ -388,13 +388,14 @@ field :token, GraphQL::STRING_TYPE, null: true, description: 'Token for login' ``` -The original `description:` of the field should be maintained, and should -_not_ be updated to mention the deprecation. +The original `description` of the things being deprecated should be maintained, +and should _not_ be updated to mention the deprecation. Instead, the `reason` will +be appended to the `description`. ### Deprecation reason style guide -Where the reason for deprecation is due to the field being replaced -with another field, the `reason` must be: +Where the reason for deprecation is due to the field or enum value being +replaced, the `reason` must be: ```plaintext Use `otherFieldName` @@ -408,9 +409,22 @@ field :designs, ::Types::DesignManagement::DesignCollectionType, null: true, description: 'The designs associated with this issue', ``` +```ruby +module Types + class TodoStateEnum < BaseEnum + value 'pending', deprecated: { reason: 'Use PENDING', milestone: '10.0' } + value 'done', deprecated: { reason: 'Use DONE', milestone: '10.0' } + value 'PENDING', value: 'pending' + value 'DONE', value: 'done' + end +end +``` + If the field is not being replaced by another field, a descriptive deprecation `reason` should be given. +See also [Aliasing and deprecating mutations](#aliasing-and-deprecating-mutations). + ## Enums GitLab GraphQL enums are defined in `app/graphql/types`. When defining new enums, the @@ -455,6 +469,9 @@ module Types end ``` +Enum values can be deprecated using the +[`deprecated` keyword](#deprecating-fields-and-enum-values). + ## JSON When data to be returned by GraphQL is stored as @@ -1155,7 +1172,8 @@ mount_aliased_mutation 'BarMutation', Mutations::FooMutation ``` This allows us to rename a mutation and continue to support the old name, -when coupled with the [`deprecated`](#deprecating-fields) argument. +when coupled with the [`deprecated`](#deprecating-fields-and-enum-values) +argument. Example: diff --git a/doc/development/documentation/structure.md b/doc/development/documentation/structure.md index e13b2f4d031..6ddf6405067 100644 --- a/doc/development/documentation/structure.md +++ b/doc/development/documentation/structure.md @@ -1,45 +1,46 @@ --- +stage: none +group: Style Guide +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers description: What to include in GitLab documentation pages. --- # Documentation structure and template -This document will help you determine how to structure a page within GitLab's -documentation and what content to include. These standards help ensure consistency -and completeness throughout the documentation, and they make it easier to contribute. +Use these standards to contribute content to the GitLab documentation. Before getting started, familiarize yourself with [GitLab's Documentation guidelines](index.md) -and the section on Content in the [Style Guide](styleguide.md). +and the [Documentation Style Guide](styleguide.md). ## Components of a documentation page -Most pages will be dedicated to a specific GitLab feature or to a use case that involves -one or more features, potentially in conjunction with third-party tools. +Most pages are dedicated to a specific GitLab feature or to a use case that +involves one or more features, potentially in conjunction with third-party tools. -Every feature or use case document should include the following content in the following sequence, -with exceptions and details noted below and in the template included on this page. +In general, each topic should include the following content, in this sequence: -- **Title**: Top-level heading with the feature name, or a use case name, which would start with - a verb, like "Configure", "Enable", and so on. -- **Introduction**: A couple sentences about the subject matter and what's to be found -on this page. Describe what the feature or topic is, what it does, and in what context it should -be used. There is no need to add a title called "Introduction" or "Overview," because people rarely - search for these terms. Just put this information after the title. -- **Use cases**: describes real use case scenarios for that feature/configuration. -- **Requirements**: describes what software, configuration, account, or knowledge is required. -- **Instructions**: one or more sets of detailed instructions to follow. -- **Troubleshooting** guide (recommended but not required). +- *Metadata*: Information about the stage, group, and how to find the technical + writer for the topic. This information isn't visible in the published help. +- *Title*: A top-level heading with the feature or use case name. Choose a term + that defines the functionality and use the same term in all the resources + where the feature is mentioned. +- *Introduction*: In a few sentences beneath the title, describe what the + feature or topic is, what it does, and in what context it should be used. +- *Use cases*: Describe real user scenarios. +- *Prerequisites*: Describe the software, configuration, account, permissions, + or knowledge required to use this functionality. +- *Tasks*: Present detailed step-by-step instructions on how to use the feature. +- *Troubleshooting*: List errors and how to address them. Recommended but not + required. -For additional details on each, see the [template for new docs](#template-for-new-docs), -below. - -Note that you can include additional subsections, as appropriate, such as 'How it Works', 'Architecture', -and other logical divisions such as pre-deployment and post-deployment steps. +You can include additional subsections, as appropriate, such as *How it Works*, +or *Architecture*. You can also include other logical divisions, such as +pre-deployment and post-deployment tasks. ## Template for new docs -To start a new document, respect the file tree and file name guidelines, -as well as the style guidelines. Use the following template: +Follow the [folder structure and file name guidelines](styleguide.md#folder-structure-overview) +and create a new topic by using this template: ```markdown --- -description: "Short document description." # Up to ~200 chars long. They will be displayed +description: "Short document description." # Up to ~200 chars long. This information is displayed in Google Search snippets. It may help to write the page intro first, and then reuse it here. -stage: "Add the stage name here, and remove the quotation marks" -group: "Add the group name here, and remove the quotation marks" +stage: Add the stage name here +group: Add the group name here info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers --- -# Feature Name or Use Case Name **[TIER]** (1) - > [Introduced](link_to_issue_or_mr) in GitLab (Tier) X.Y (2). -An introduction -- without its own additional header -- goes here. -Offer a description of the feature or use case, and what to expect on this page. -(You can reuse this content, or part of it, for the front matter's `description` at the top -of this file). - -The introduction should answer the following questions: +Write a description of the feature or use case. This introduction should answer +these questions: - What is this feature or use case? - Who is it for? -- What is the context in which it is used and are there any prerequisites/requirements? -- What can the audience do with this? (Be sure to consider all applicable audiences, like - GitLab admin and developer-user.) -- What are the benefits to using this over any alternatives? +- What is the context in which it is used and are there any prerequisites or + requirements? +- What can the audience do with this? (Be sure to consider all applicable + audiences, such as GitLab admin and developer-user.) +- What are the benefits of using this over any existing alternatives? + +You can reuse this content, or part of it, for the front matter's `description` +at the top of this file. ## Use cases -Describe some use cases, typically in bulleted form. Include real-life examples for each. +Describe common use cases, typically in bulleted form. Include real-life examples +for each. -If the page itself is dedicated to a use case, this section can usually include more specific -scenarios for use (for example, variations on the main use case), but if that's not applicable, -the section can be omitted. +If the page itself is dedicated to a use case, this section usually includes more +specific scenarios for use (for example, variations on the main use case), but if +that's not applicable, you can omit this section. Examples of use cases on feature pages: + - CE and EE: [Issues](../../user/project/issues/index.md#use-cases) - CE and EE: [Merge Requests](../../user/project/merge_requests/index.md) - EE-only: [Geo](../../administration/geo/replication/index.md) - EE-only: [Jenkins integration](../../integration/jenkins.md) -## Requirements +## Prerequisites -State any requirements for using the feature and/or following along with the instructions. +State any prerequisites for using the feature. These might include: -These can include both: -- technical requirements (for example, an account on a third party service, an amount of storage space, - prior configuration of another feature) -- prerequisite knowledge (for example, familiarity with certain GitLab features, cloud technologies) +- Technical prereqs (for example, an account on a third-party service, an amount + of storage space, or prior configuration of another feature) +- Prerequisite knowledge (for example, familiarity with certain GitLab features + or other products and technologies). Link each one to an appropriate place for more information. -## Instructions +## Tasks -This is the part of the document where you can include one or more sets of instructions. Each topic should help users accomplish a specific task. -Headers should describe the task the reader will achieve by following the instructions within, -typically starting with a verb. For example, `Create a package` or `Configure a pipeline`. +The heading should: -Larger instruction sets may have subsections covering specific phases of the process. -Where appropriate, provide examples of code or configuration files to better clarify -intended usage. +- Describe the task and start with a verb. For example, `Create a package` or + `Configure a pipeline`. +- Be short and descriptive (up to ~50 chars). +- Start from an `h2` (`##`), then go over `h3`, `h4`, `h5`, and `h6` as needed. + Never skip a hierarchy level (like `h2` > `h4`). It breaks the table of + contents and can affect the breadcrumbs. -- Write a step-by-step guide, with no gaps between the steps. -- Include example code or configurations as part of the relevant step. - Use appropriate Markdown to wrap code blocks with - [syntax highlighting](../../user/markdown.md#colored-code-and-syntax-highlighting). -- Start with an h2 (`##`), break complex steps into small steps using - subheadings h3 > h4 > h5 > h6. _Never skip a hierarchy level, such - as h2 > h4_, as it will break the TOC and may affect the breadcrumbs. -- Use short and descriptive headings (up to ~50 chars). You can use one - single heading like `## Configure X` for instructions when the feature - is simple and the document is short. +Bigger tasks can have subsections that explain specific phases of the process. + +Include example code or configurations when needed. Use Markdown to wrap code +blocks with [syntax highlighting](../../user/markdown.md#colored-code-and-syntax-highlighting). Example topic: ## Create a teddy bear -Start by writing a sentence or two about _why_ someone would want to perform this task. -It's not always possible, but is a good practice. For example: - -Create a teddy bear when you need something to hug. - -Follow this information with the task steps. +Create a teddy bear when you need something to hug. (Include the reason why you +might do the task.) To create a teddy bear: @@ -142,40 +136,40 @@ To create a teddy bear: 1. Expand **This** and click **This**. 1. Do another step. -After the numbered list, add a sentence with the expected result, if it -is not obvious, and any next steps. For example: +The teddy bear is now in the kitchen, in the cupboard above the sink. _(This is the result.)_ -The teddy bear is now in the kitchen, in the cupboard above the sink. +You can retrieve the teddy bear and put it on the couch with the other animals. _(These are next steps.)_ -You can retrieve the teddy bear and put it on the couch with the other animals. - -Screenshots are not necessary. They are difficult to keep up-to-date and can clutter the page. +Screenshots are not necessary. They are difficult to keep up-to-date and can +clutter the page. +If you have none to add when creating a doc, leave this section in place but +commented out to help encourage others to add to it in the future. --> --- Notes: -- (1): Apply the [tier badges](styleguide.md#product-badges) accordingly +- (1): Apply the [tier badges](styleguide.md#product-badges) accordingly. - (2): Apply the correct format for the - [GitLab version that introduces the feature](styleguide.md#gitlab-versions-and-tiers) + [GitLab version that introduces the feature](styleguide.md#gitlab-versions-and-tiers). ``` ## Help and feedback section -The "help and feedback" section (introduced by [!319](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/319)) displayed at the end of each document -can be omitted from the doc by adding a key into the its front matter: +This section ([introduced](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/319) in GitLab 11.4) +is displayed at the end of each document and can be omitted by adding a key into +the front matter: ```yaml --- @@ -183,8 +177,8 @@ feedback: false --- ``` -The default is to leave it there. If you want to omit it from a document, -you must check with a technical writer before doing so. +The default is to leave it there. If you want to omit it from a document, you +must check with a technical writer before doing so. ### Disqus @@ -192,8 +186,8 @@ We also have integrated the docs site with Disqus (introduced by [!151](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/151)), allowing our users to post comments. -To omit only the comments from the feedback section, use the following -key on the front matter: +To omit only the comments from the feedback section, use the following key in +the front matter: ```yaml --- @@ -201,36 +195,42 @@ comments: false --- ``` -We are only hiding comments in main index pages, such as [the main documentation index](../../README.md), since its content is too broad to comment on. Before omitting Disqus, -you must check with a technical writer. +We're hiding comments only in main index pages, such as [the main documentation index](../../README.md), +since its content is too broad to comment on. Before omitting Disqus, you must +check with a technical writer. -Note that once `feedback: false` is added to the front matter, it will automatically omit +Note that after adding `feedback: false` to the front matter, it will omit Disqus, therefore, don't add both keys to the same document. -The click events in the feedback section are tracked with Google Tag Manager. The -conversions can be viewed on Google Analytics by navigating to **Behavior > Events > Top events > docs**. +The click events in the feedback section are tracked with Google Tag Manager. +The conversions can be viewed on Google Analytics by navigating to +**Behavior > Events > Top events > docs**. ## Guidelines for good practices > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36576/) in GitLab 13.2 as GitLab Development documentation. -"Good practice" examples demonstrate encouraged ways of writing code while comparing with examples of practices to avoid. -These examples are labeled as "Bad" or "Good". -In GitLab development guidelines, when presenting the cases, it is recommended -to follow a **first-bad-then-good** strategy. First demonstrate the "Bad" practice (how things _could_ be done, which is often still working code), -and then how things _should_ be done better, using a "Good" example. This is typically an improved example of the same code. +*Good practice* examples demonstrate encouraged ways of writing code while +comparing with examples of practices to avoid. These examples are labeled as +*Bad* or *Good*. In GitLab development guidelines, when presenting the cases, +it's recommended to follow a *first-bad-then-good* strategy. First demonstrate +the *Bad* practice (how things *could* be done, which is often still working +code), and then how things *should* be done better, using a *Good* example. This +is typically an improved example of the same code. Consider the following guidelines when offering examples: -- First, offer the "Bad" example, then the "Good" one. +- First, offer the *Bad* example, and then the *Good* one. - When only one bad case and one good case is given, use the same code block. -- When more than one bad case or one good case is offered, use separated code blocks for each. -With many examples being presented, a clear separation helps the reader to go directly to the good part. -Consider offering an explanation (for example, a comment, a link to a resource, etc.) on why something is bad practice. +- When more than one bad case or one good case is offered, use separated code + blocks for each. With many examples being presented, a clear separation helps + the reader to go directly to the good part. Consider offering an explanation + (for example, a comment, or a link to a resource) on why something is bad + practice. - Better and best cases can be considered part of the good case(s) code block. -In the same code block, precede each with comments: `# Better` and `# Best`. + In the same code block, precede each with comments: `# Better` and `# Best`. NOTE: **Note:** -While the bad-then-good approach is acceptable for the GitLab development guidelines, do not use it -for user documentation. For user documentation, use "Do" and "Don't." For example, see the -[Pajamas Design System](https://design.gitlab.com/content/punctuation/). +Although the bad-then-good approach is acceptable for the GitLab development +guidelines, do not use it for user documentation. For user documentation, use +*Do* and *Don't*. For examples, see the [Pajamas Design System](https://design.gitlab.com/content/punctuation/). diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 1aa6cc7c570..a6afef668bd 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -357,6 +357,65 @@ graph RL; end ``` +### Fail-fast pipeline in Merge Requests + +To provide faster feedback when a Merge Request breaks existing tests, we are experimenting with a +fail-fast mechanism. + +An `rspec fail-fast` job is added in parallel to all other `rspec` jobs in a Merge +Request pipeline. This job runs the tests that are directly related to the changes +in the Merge Request. + +If any of these tests fail, the `rspec fail-fast` job fails, triggering a +`fail-pipeline-early` job to run. The `fail-pipeline-early` job: + +- Cancels the currently running pipeline and all in-progress jobs. +- Sets pipeline to have status `failed`. + +For example: + +```mermaid +graph LR + subgraph "prepare stage"; + A["detect-tests"] + end + + subgraph "test stage"; + B["jest"]; + C["rspec migration"]; + D["rspec unit"]; + E["rspec integration"]; + F["rspec system"]; + G["rspec fail-fast"]; + end + + subgraph "post-test stage"; + Z["fail-pipeline-early"]; + end + + A --"artifact: list of test files"--> G + G --"on failure"--> Z +``` + +A Merge Request author may choose to opt-out of the fail fast mechanism by doing one of the following: + +- Including `[SKIP RSPEC FAIL-FAST]` in the Merge Request title. +- Starting the `dont-interrupt-me` job found in the `sync` stage of a Merge Request pipeline. + +The `rspec fail-fast` is a no-op if there are more than 10 test files related to the +Merge Request. This prevents `rspec fail-fast` duration from exceeding the average +`rspec` job duration and defeating its purpose. + +This number can be overridden by setting a CI variable named `RSPEC_FAIL_FAST_TEST_FILE_COUNT_THRESHOLD`. + +NOTE: **Note:** +This experiment is only enabled when the CI variable `RSPEC_FAIL_FAST_ENABLED=true` is set. + +#### Determining related test files in a Merge Request + +The test files related to the Merge Request are determined using the [`test_file_finder`](https://gitlab.com/gitlab-org/ci-cd/test_file_finder) gem. +We are using a custom mapping between source file to test files, maintained in the `tests.yml` file. + ### PostgreSQL versions testing #### Current versions testing diff --git a/doc/gitlab-basics/start-using-git.md b/doc/gitlab-basics/start-using-git.md index 95b3a59ef6e..3812fd3b92a 100644 --- a/doc/gitlab-basics/start-using-git.md +++ b/doc/gitlab-basics/start-using-git.md @@ -1,10 +1,9 @@ --- stage: Create group: Source Code -info: "To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers" +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers type: howto, tutorial description: "Introduction to using Git through the command line." -last_updated: 2020-06-30 --- # Start using Git on the command line diff --git a/doc/topics/git/how_to_install_git/index.md b/doc/topics/git/how_to_install_git/index.md index 7b842b6a409..a90ccbf9eaf 100644 --- a/doc/topics/git/how_to_install_git/index.md +++ b/doc/topics/git/how_to_install_git/index.md @@ -1,15 +1,9 @@ --- stage: Create group: Source Code -info: "To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers" -author: Sean Packham -author_gitlab: SeanPackham -level: beginner -article_type: user guide -date: 2017-05-15 +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers description: 'This article describes how to install Git on macOS, Ubuntu Linux and Windows.' type: howto -last_updated: 2020-04-22 --- # Installing Git diff --git a/doc/topics/git/numerous_undo_possibilities_in_git/index.md b/doc/topics/git/numerous_undo_possibilities_in_git/index.md index b59bdf12371..77732e0da3e 100644 --- a/doc/topics/git/numerous_undo_possibilities_in_git/index.md +++ b/doc/topics/git/numerous_undo_possibilities_in_git/index.md @@ -1,14 +1,8 @@ --- stage: Create group: Source Code -info: "To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers" -author: Crt Mori -author_gitlab: Letme -level: intermediary -article_type: tutorial -date: 2017-05-15 +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers type: howto -last_updated: 2019-05-31 --- # Numerous undo possibilities in Git diff --git a/doc/user/packages/composer_repository/index.md b/doc/user/packages/composer_repository/index.md index 2d974a6321f..89e02b4847c 100644 --- a/doc/user/packages/composer_repository/index.md +++ b/doc/user/packages/composer_repository/index.md @@ -172,4 +172,4 @@ CAUTION: **Important:** Make sure to never commit the `auth.json` file to your repository. To install packages from a CI job, consider using the [`composer config`](https://getcomposer.org/doc/articles/handling-private-packages-with-satis.md#authentication) tool with your personal access token stored in a [GitLab CI/CD environment variable](../../../ci/variables/README.md) or in -[Hashicorp Vault](../../../ci/examples/authenticating-with-hashicorp-vault/index.md). +[Hashicorp Vault](../../../ci/secrets/index.md). diff --git a/doc/user/project/file_lock.md b/doc/user/project/file_lock.md index 3ff6530ec28..6fd33901621 100644 --- a/doc/user/project/file_lock.md +++ b/doc/user/project/file_lock.md @@ -1,9 +1,8 @@ --- stage: Create group: Source Code -info: "To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers" +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers type: reference, howto -last_updated: 2020-09-07 --- # File Locking **(CORE)** diff --git a/lib/gitlab/analytics/instance_statistics/workers_argument_builder.rb b/lib/gitlab/analytics/instance_statistics/workers_argument_builder.rb new file mode 100644 index 00000000000..636bba22c23 --- /dev/null +++ b/lib/gitlab/analytics/instance_statistics/workers_argument_builder.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Analytics + module InstanceStatistics + class WorkersArgumentBuilder + def initialize(measurement_identifiers: [], recorded_at: Time.zone.now) + @measurement_identifiers = measurement_identifiers + @recorded_at = recorded_at + end + + def execute + measurement_identifiers.map do |measurement_identifier| + query_scope = ::Analytics::InstanceStatistics::Measurement::IDENTIFIER_QUERY_MAPPING[measurement_identifier]&.call + + next if query_scope.nil? + + # Determining the query range (id range) as early as possible in order to get more accurate counts. + start = query_scope.minimum(:id) + finish = query_scope.maximum(:id) + + [measurement_identifier, start, finish, recorded_at] + end.compact + end + + private + + attr_reader :measurement_identifiers, :recorded_at + end + end + end +end diff --git a/lib/gitlab/database_importers/instance_administrators/create_group.rb b/lib/gitlab/database_importers/instance_administrators/create_group.rb index 5bf0e5a320d..d9425810405 100644 --- a/lib/gitlab/database_importers/instance_administrators/create_group.rb +++ b/lib/gitlab/database_importers/instance_administrators/create_group.rb @@ -6,6 +6,8 @@ module Gitlab class CreateGroup < ::BaseService include Stepable + NAME = 'GitLab Instance' + PATH_PREFIX = 'gitlab-instance' VISIBILITY_LEVEL = Gitlab::VisibilityLevel::INTERNAL steps :validate_application_settings, @@ -117,12 +119,12 @@ module Gitlab def create_group_params { - name: 'GitLab Instance Administrators', + name: NAME, visibility_level: VISIBILITY_LEVEL, # The 8 random characters at the end are so that the path does not # clash with any existing group that the user might have created. - path: "gitlab-instance-administrators-#{SecureRandom.hex(4)}" + path: "#{PATH_PREFIX}-#{SecureRandom.hex(4)}" } end end diff --git a/lib/gitlab/database_importers/self_monitoring/project/create_service.rb b/lib/gitlab/database_importers/self_monitoring/project/create_service.rb index 07a4c3bf5e6..88f035c2d1b 100644 --- a/lib/gitlab/database_importers/self_monitoring/project/create_service.rb +++ b/lib/gitlab/database_importers/self_monitoring/project/create_service.rb @@ -9,7 +9,7 @@ module Gitlab include SelfMonitoring::Helpers VISIBILITY_LEVEL = Gitlab::VisibilityLevel::INTERNAL - PROJECT_NAME = 'GitLab self monitoring' + PROJECT_NAME = 'Monitoring' steps :validate_application_settings, :create_group, diff --git a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb index a91ab17887d..871e9c4b7c8 100644 --- a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb +++ b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb @@ -24,9 +24,11 @@ module Gitlab return unless entry.is_a?(Hash) + exception_type = entry[:type] raw_message = entry[:value] - return unless raw_message.start_with?('GRPC::') + return unless exception_type&.start_with?('GRPC::') + return unless raw_message.present? message, debug_str = split_debug_error_string(raw_message) diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh index 3b0c7d973de..3812a8b8ef7 100644 --- a/scripts/rspec_helpers.sh +++ b/scripts/rspec_helpers.sh @@ -111,6 +111,27 @@ function rspec_paralellized_job() { date } +function rspec_fail_fast() { + local test_file_count_threshold=${RSPEC_FAIL_FAST_TEST_FILE_COUNT_THRESHOLD:-10} + local matching_tests_file=${1} + local rspec_opts=${2} + local test_files="$(cat "${matching_tests_file}")" + local test_file_count=$(wc -w "${matching_tests_file}" | awk {'print $1'}) + + if [[ "${test_file_count}" -gt "${test_file_count_threshold}" ]]; then + echo "This job is intentionally skipped because there are more than ${test_file_count_threshold} test files matched," + echo "which would take too long to run in this job." + echo "All the tests would be run in other rspec jobs." + exit 0 + fi + + if [[ -n $test_files ]]; then + rspec_simple_job "${rspec_opts} ${test_files}" + else + echo "No rspec fail-fast tests to run" + fi +} + function rspec_matched_foss_tests() { local test_file_count_threshold=20 local matching_tests_file=${1} @@ -131,6 +152,6 @@ function rspec_matched_foss_tests() { if [[ -n $test_files ]]; then rspec_simple_job "${rspec_opts} ${test_files}" else - echo "No FOSS test files to run" + echo "No impacted FOSS rspec tests to run" fi } diff --git a/scripts/utils.sh b/scripts/utils.sh index 98d9dca74c0..5218c1e2d98 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -137,3 +137,15 @@ function play_job() { job_url=$(curl --silent --show-error --request POST --header "PRIVATE-TOKEN: ${api_token}" "${url}" | jq ".web_url") echoinfo "Manual job '${job_name}' started at: ${job_url}" } + +function fail_pipeline_early() { + local dont_interrupt_me_job_id + dont_interrupt_me_job_id=$(get_job_id 'dont-interrupt-me' 'scope=success') + + if [[ "${dont_interrupt_me_job_id}" != "" ]]; then + echoinfo "This pipeline cannot be interrupted due to \`dont-interrupt-me\` job ${dont_interrupt_me_job_id}" + else + echoinfo "Failing pipeline early for fast feedback due to test failures in rspec fail-fast." + curl --request POST --header "PRIVATE-TOKEN: ${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}" "https://${CI_SERVER_HOST}/api/v4/projects/${CI_PROJECT_ID}/pipelines/${CI_PIPELINE_ID}/cancel" + fi +} diff --git a/spec/factories/labels.rb b/spec/factories/labels.rb index 6725b571f19..a9a9416c48b 100644 --- a/spec/factories/labels.rb +++ b/spec/factories/labels.rb @@ -18,6 +18,13 @@ FactoryBot.define do title { "#{prefix}::#{generate(:label_title)}" } end + trait :incident do + properties = IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES + title { properties.fetch(:title) } + description { properties.fetch(:description) } + color { properties.fetch(:color) } + end + factory :label, traits: [:base_label], class: 'ProjectLabel' do project diff --git a/spec/factories/usage_data.rb b/spec/factories/usage_data.rb index 8b0df81422b..5b20205a235 100644 --- a/spec/factories/usage_data.rb +++ b/spec/factories/usage_data.rb @@ -51,12 +51,11 @@ FactoryBot.define do create(:protected_branch, name: 'main', project: projects[0]) # Incident Labeled Issues - incident_label_attrs = IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES - incident_label = create(:label, project: projects[0], **incident_label_attrs) + incident_label = create(:label, :incident, project: projects[0]) create(:labeled_issue, project: projects[0], labels: [incident_label]) incident_group = create(:group) - incident_label_scoped_to_project = create(:label, project: projects[1], **incident_label_attrs) - incident_label_scoped_to_group = create(:group_label, group: incident_group, **incident_label_attrs) + incident_label_scoped_to_project = create(:label, :incident, project: projects[1]) + incident_label_scoped_to_group = create(:group_label, :incident, group: incident_group) create(:labeled_issue, project: projects[1], labels: [incident_label_scoped_to_project]) create(:labeled_issue, project: projects[1], labels: [incident_label_scoped_to_group]) diff --git a/spec/graphql/resolvers/group_members_resolver_spec.rb b/spec/graphql/resolvers/group_members_resolver_spec.rb new file mode 100644 index 00000000000..bbfea575492 --- /dev/null +++ b/spec/graphql/resolvers/group_members_resolver_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Resolvers::GroupMembersResolver do + include GraphqlHelpers + + it_behaves_like 'querying members with a group' do + let_it_be(:resource_member) { create(:group_member, user: user_1, group: group_1) } + let_it_be(:resource) { group_1 } + end +end diff --git a/spec/graphql/resolvers/project_members_resolver_spec.rb b/spec/graphql/resolvers/project_members_resolver_spec.rb index 91406d5c749..2f4145b3215 100644 --- a/spec/graphql/resolvers/project_members_resolver_spec.rb +++ b/spec/graphql/resolvers/project_members_resolver_spec.rb @@ -5,62 +5,9 @@ require 'spec_helper' RSpec.describe Resolvers::ProjectMembersResolver do include GraphqlHelpers - context "with a group" do - let_it_be(:root_group) { create(:group) } - let_it_be(:group_1) { create(:group, parent: root_group) } - let_it_be(:group_2) { create(:group, parent: root_group) } - let_it_be(:project) { create(:project, group: group_1) } - - let_it_be(:user_1) { create(:user, name: 'test user') } - let_it_be(:user_2) { create(:user, name: 'test user 2') } - let_it_be(:user_3) { create(:user, name: 'another user 1') } - let_it_be(:user_4) { create(:user, name: 'another user 2') } - - let_it_be(:project_member) { create(:project_member, user: user_1, project: project) } - let_it_be(:group_1_member) { create(:group_member, user: user_2, group: group_1) } - let_it_be(:group_2_member) { create(:group_member, user: user_3, group: group_2) } - let_it_be(:root_group_member) { create(:group_member, user: user_4, group: root_group) } - - let(:args) { {} } - - subject do - resolve(described_class, obj: project, args: args, ctx: { current_user: user_4 }) - end - - describe '#resolve' do - it 'finds all project members' do - expect(subject).to contain_exactly(project_member, group_1_member, root_group_member) - end - - context 'with search' do - context 'when the search term matches a user' do - let(:args) { { search: 'test' } } - - it 'searches users by user name' do - expect(subject).to contain_exactly(project_member, group_1_member) - end - end - - context 'when the search term does not match any user' do - let(:args) { { search: 'nothing' } } - - it 'is empty' do - expect(subject).to be_empty - end - end - end - - context 'when user can not see project members' do - let_it_be(:other_user) { create(:user) } - - subject do - resolve(described_class, obj: project, args: args, ctx: { current_user: other_user }) - end - - it 'raises an error' do - expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) - end - end - end + it_behaves_like 'querying members with a group' do + let_it_be(:project) { create(:project, group: group_1) } + let_it_be(:resource_member) { create(:project_member, user: user_1, project: project) } + let_it_be(:resource) { project } end end diff --git a/spec/graphql/types/base_enum_spec.rb b/spec/graphql/types/base_enum_spec.rb index 0d0f6346f2d..b7adcf217f6 100644 --- a/spec/graphql/types/base_enum_spec.rb +++ b/spec/graphql/types/base_enum_spec.rb @@ -21,4 +21,16 @@ RSpec.describe Types::BaseEnum do expect(enum.enum).to be_a(HashWithIndifferentAccess) end end + + include_examples 'Gitlab-style deprecations' do + def subject(args = {}) + enum = Class.new(described_class) do + graphql_name 'TestEnum' + + value 'TEST_VALUE', **args + end + + enum.to_graphql.values['TEST_VALUE'] + end + end end diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb index 73bb54e7ad0..bcfbd7f2480 100644 --- a/spec/graphql/types/base_field_spec.rb +++ b/spec/graphql/types/base_field_spec.rb @@ -167,70 +167,23 @@ RSpec.describe Types::BaseField do end end - describe '`deprecated` property' do - def test_field(args = {}) + include_examples 'Gitlab-style deprecations' do + def subject(args = {}) base_args = { name: 'test', type: GraphQL::STRING_TYPE, null: true } described_class.new(**base_args.merge(args)) end - describe 'validations' do - it 'raises an informative error if `deprecation_reason` is used' do - expect { test_field(deprecation_reason: 'foo') }.to raise_error( - ArgumentError, - 'Use `deprecated` property instead of `deprecation_reason`. ' \ - 'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields' - ) - end - - it 'raises an error if a required property is missing', :aggregate_failures do - expect { test_field(deprecated: { milestone: '1.10' }) }.to raise_error( - ArgumentError, - 'Please provide a `reason` within `deprecated`' - ) - expect { test_field(deprecated: { reason: 'Deprecation reason' }) }.to raise_error( - ArgumentError, - 'Please provide a `milestone` within `deprecated`' - ) - end - - it 'raises an error if milestone is not a String', :aggregate_failures do - expect { test_field(deprecated: { milestone: 1.10, reason: 'Deprecation reason' }) }.to raise_error( - ArgumentError, - '`milestone` must be a `String`' - ) - end - end - - it 'adds a formatted `deprecated_reason` to the field' do - field = test_field(deprecated: { milestone: '1.10', reason: 'Deprecation reason' }) - - expect(field.deprecation_reason).to eq('Deprecation reason. Deprecated in 1.10') - end - - it 'appends to the description if given' do - field = test_field( - deprecated: { milestone: '1.10', reason: 'Deprecation reason' }, - description: 'Field description' - ) - - expect(field.description).to eq('Field description. Deprecated in 1.10: Deprecation reason') - end - - it 'does not append to the description if it is absent' do - field = test_field(deprecated: { milestone: '1.10', reason: 'Deprecation reason' }) - - expect(field.description).to be_nil - end - it 'interacts well with the `feature_flag` property' do - field = test_field( + field = subject( deprecated: { milestone: '1.10', reason: 'Deprecation reason' }, description: 'Field description', feature_flag: 'foo_flag' ) - expect(field.description).to eq('Field description. Available only when feature flag `foo_flag` is enabled. Deprecated in 1.10: Deprecation reason') + expectation = 'Field description. Available only when feature flag `foo_flag` is enabled. Deprecated in 1.10: Deprecation reason' + + expect(field.description).to eq(expectation) end end end diff --git a/spec/graphql/types/group_type_spec.rb b/spec/graphql/types/group_type_spec.rb index 0b87805c2ef..bf7ddebaf5b 100644 --- a/spec/graphql/types/group_type_spec.rb +++ b/spec/graphql/types/group_type_spec.rb @@ -16,7 +16,7 @@ RSpec.describe GitlabSchema.types['Group'] do web_url avatar_url share_with_group_lock project_creation_level subgroup_creation_level require_two_factor_authentication two_factor_grace_period auto_devops_enabled emails_disabled - mentions_disabled parent boards milestones + mentions_disabled parent boards milestones group_members ] expect(described_class).to include_graphql_fields(*expected_fields) @@ -30,5 +30,12 @@ RSpec.describe GitlabSchema.types['Group'] do end end + describe 'members field' do + subject { described_class.fields['groupMembers'] } + + it { is_expected.to have_graphql_type(Types::GroupMemberType.connection_type) } + it { is_expected.to have_graphql_resolver(Resolvers::GroupMembersResolver) } + end + it_behaves_like 'a GraphQL type with labels' end diff --git a/spec/lib/gitlab/analytics/instance_statistics/workers_argument_builder_spec.rb b/spec/lib/gitlab/analytics/instance_statistics/workers_argument_builder_spec.rb new file mode 100644 index 00000000000..d232e509e00 --- /dev/null +++ b/spec/lib/gitlab/analytics/instance_statistics/workers_argument_builder_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Analytics::InstanceStatistics::WorkersArgumentBuilder do + context 'when no measurement identifiers are given' do + it 'returns empty array' do + expect(described_class.new(measurement_identifiers: []).execute).to be_empty + end + end + + context 'when measurement identifiers are given' do + let_it_be(:user_1) { create(:user) } + let_it_be(:project_1) { create(:project, namespace: user_1.namespace, creator: user_1) } + let_it_be(:project_2) { create(:project, namespace: user_1.namespace, creator: user_1) } + let_it_be(:project_3) { create(:project, namespace: user_1.namespace, creator: user_1) } + + let(:recorded_at) { 2.days.ago } + let(:projects_measurement_identifier) { ::Analytics::InstanceStatistics::Measurement.identifiers.fetch(:projects) } + let(:users_measurement_identifier) { ::Analytics::InstanceStatistics::Measurement.identifiers.fetch(:users) } + let(:measurement_identifiers) { [projects_measurement_identifier, users_measurement_identifier] } + + subject { described_class.new(measurement_identifiers: measurement_identifiers, recorded_at: recorded_at).execute } + + it 'returns worker arguments' do + expect(subject).to eq([ + [projects_measurement_identifier, project_1.id, project_3.id, recorded_at], + [users_measurement_identifier, user_1.id, user_1.id, recorded_at] + ]) + end + + context 'when bogus measurement identifiers are given' do + before do + measurement_identifiers << 'bogus1' + measurement_identifiers << 'bogus2' + end + + it 'skips bogus measurement identifiers' do + expect(subject).to eq([ + [projects_measurement_identifier, project_1.id, project_3.id, recorded_at], + [users_measurement_identifier, user_1.id, user_1.id, recorded_at] + ]) + end + end + end +end diff --git a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb index a3661bbe49a..39029322e25 100644 --- a/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb +++ b/spec/lib/gitlab/database_importers/instance_administrators/create_group_spec.rb @@ -65,8 +65,8 @@ RSpec.describe Gitlab::DatabaseImporters::InstanceAdministrators::CreateGroup do it 'creates group' do expect(result[:status]).to eq(:success) expect(group).to be_persisted - expect(group.name).to eq('GitLab Instance Administrators') - expect(group.path).to start_with('gitlab-instance-administrators') + expect(group.name).to eq('GitLab Instance') + expect(group.path).to start_with('gitlab-instance') expect(group.path.split('-').last.length).to eq(8) expect(group.visibility_level).to eq(described_class::VISIBILITY_LEVEL) end diff --git a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb index dc821709815..797707114a1 100644 --- a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb @@ -20,7 +20,8 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do exception: { values: [ { - value: "GRPC::DeadlineExceeded: 4:DeadlineExceeded. debug_error_string:{\"hello\":1}" + type: "GRPC::DeadlineExceeded", + value: "4:DeadlineExceeded. debug_error_string:{\"hello\":1}" } ] }, @@ -43,7 +44,8 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do exception: { values: [ { - value: "GRPC::DeadlineExceeded: 4:DeadlineExceeded." + type: "GRPC::DeadlineExceeded", + value: "4:DeadlineExceeded." } ] }, diff --git a/spec/lib/gitlab/graphql/docs/renderer_spec.rb b/spec/lib/gitlab/graphql/docs/renderer_spec.rb index 0f0127819fa..d1be962a4f8 100644 --- a/spec/lib/gitlab/graphql/docs/renderer_spec.rb +++ b/spec/lib/gitlab/graphql/docs/renderer_spec.rb @@ -99,7 +99,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do graphql_name 'MyEnum' value 'BAZ', description: 'A description of BAZ' - value 'BAR', description: 'A description of BAR', deprecation_reason: 'This is deprecated' + value 'BAR', description: 'A description of BAR', deprecated: { reason: 'This is deprecated', milestone: '1.10' } end Class.new(Types::BaseObject) do @@ -115,7 +115,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do | Value | Description | | ----- | ----------- | - | `BAR` **{warning-solid}** | **Deprecated:** This is deprecated | + | `BAR` **{warning-solid}** | **Deprecated:** This is deprecated. Deprecated in 1.10 | | `BAZ` | A description of BAZ | DOC diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb new file mode 100644 index 00000000000..84b2fd63d46 --- /dev/null +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting group members information' do + include GraphqlHelpers + + let_it_be(:group) { create(:group, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:user_1) { create(:user, username: 'user') } + let_it_be(:user_2) { create(:user, username: 'test') } + + let(:member_data) { graphql_data['group']['groupMembers']['edges'] } + + before do + [user_1, user_2].each { |user| group.add_guest(user) } + end + + context 'when the request is correct' do + it_behaves_like 'a working graphql query' do + before do + fetch_members(user) + end + end + + it 'returns group members successfully' do + fetch_members(user) + + expect(graphql_errors).to be_nil + expect_array_response(user_1.to_global_id.to_s, user_2.to_global_id.to_s) + end + + it 'returns members that match the search query' do + fetch_members(user, { search: 'test' }) + + expect(graphql_errors).to be_nil + expect_array_response(user_2.to_global_id.to_s) + end + end + + def fetch_members(user = nil, args = {}) + post_graphql(members_query(args), current_user: user) + end + + def members_query(args = {}) + members_node = <<~NODE + edges { + node { + user { + id + } + } + } + NODE + + graphql_query_for("group", + { full_path: group.full_path }, + [query_graphql_field("groupMembers", args, members_node)] + ) + end + + def expect_array_response(*items) + expect(response).to have_gitlab_http_status(:success) + expect(member_data).to be_an Array + expect(member_data.map { |node| node["node"]["user"]["id"] }).to match_array(items) + end +end diff --git a/spec/requests/api/graphql/group_query_spec.rb b/spec/requests/api/graphql/group_query_spec.rb index d99bff2e349..83180c7d7a5 100644 --- a/spec/requests/api/graphql/group_query_spec.rb +++ b/spec/requests/api/graphql/group_query_spec.rb @@ -89,18 +89,13 @@ RSpec.describe 'getting group information', :do_not_mock_admin_mode do end it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do - post_graphql(group_query(group1), current_user: admin) - end.count + pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/245272') queries = [{ query: group_query(group1) }, { query: group_query(group2) }] - expect do - post_multiplex(queries, current_user: admin) - end.not_to exceed_query_limit(control_count) - - expect(graphql_errors).to contain_exactly(nil, nil) + expect { post_multiplex(queries, current_user: admin) } + .to issue_same_number_of_queries_as { post_graphql(group_query(group1), current_user: admin) } end end diff --git a/spec/services/incident_management/create_incident_label_service_spec.rb b/spec/services/incident_management/create_incident_label_service_spec.rb index 18a7c019497..4771dfc9e64 100644 --- a/spec/services/incident_management/create_incident_label_service_spec.rb +++ b/spec/services/incident_management/create_incident_label_service_spec.rb @@ -10,9 +10,10 @@ RSpec.describe IncidentManagement::CreateIncidentLabelService do subject(:execute) { service.execute } describe 'execute' do - let(:title) { described_class::LABEL_PROPERTIES[:title] } - let(:color) { described_class::LABEL_PROPERTIES[:color] } - let(:description) { described_class::LABEL_PROPERTIES[:description] } + let(:incident_label_attributes) { attributes_for(:label, :incident) } + let(:title) { incident_label_attributes[:title] } + let(:color) { incident_label_attributes[:color] } + let(:description) { incident_label_attributes[:description] } shared_examples 'existing label' do it 'returns the existing label' do diff --git a/spec/services/incident_management/incidents/create_service_spec.rb b/spec/services/incident_management/incidents/create_service_spec.rb index 404c428cd94..23170a0bf17 100644 --- a/spec/services/incident_management/incidents/create_service_spec.rb +++ b/spec/services/incident_management/incidents/create_service_spec.rb @@ -13,7 +13,7 @@ RSpec.describe IncidentManagement::Incidents::CreateService do context 'when incident has title and description' do let(:title) { 'Incident title' } let(:new_issue) { Issue.last! } - let(:label_title) { IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES[:title] } + let(:label_title) { attributes_for(:label, :incident)[:title] } it 'responds with success' do expect(create_incident).to be_success @@ -23,15 +23,20 @@ RSpec.describe IncidentManagement::Incidents::CreateService do expect { create_incident }.to change(Issue, :count).by(1) end - it 'created issue has correct attributes' do + it 'created issue has correct attributes', :aggregate_failures do create_incident - aggregate_failures do - expect(new_issue.title).to eq(title) - expect(new_issue.description).to eq(description) - expect(new_issue.author).to eq(user) - expect(new_issue.issue_type).to eq('incident') - expect(new_issue.labels.map(&:title)).to eq([label_title]) + + expect(new_issue.title).to eq(title) + expect(new_issue.description).to eq(description) + expect(new_issue.author).to eq(user) + end + + it_behaves_like 'incident issue' do + before do + create_incident end + + let(:issue) { new_issue } end context 'when incident label does not exists' do diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 96ab5343cb7..c7fa569c936 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -3,18 +3,18 @@ require 'spec_helper' RSpec.describe Issues::CreateService do - let(:project) { create(:project) } - let(:user) { create(:user) } + let_it_be_with_reload(:project) { create(:project) } + let_it_be(:user) { create(:user) } describe '#execute' do + let_it_be(:assignee) { create(:user) } + let_it_be(:milestone) { create(:milestone, project: project) } let(:issue) { described_class.new(project, user, opts).execute } - let(:assignee) { create(:user) } - let(:milestone) { create(:milestone, project: project) } context 'when params are valid' do - let(:labels) { create_pair(:label, project: project) } + let_it_be(:labels) { create_pair(:label, project: project) } - before do + before_all do project.add_maintainer(user) project.add_maintainer(assignee) end @@ -49,16 +49,35 @@ RSpec.describe Issues::CreateService do end end + it_behaves_like 'not an incident issue' + context 'issue is incident type' do before do - opts[:issue_type] = 'incident' + opts.merge!(issue_type: 'incident') end let(:current_user) { user } + let(:incident_label_attributes) { attributes_for(:label, :incident) } subject { issue } + it_behaves_like 'incident issue' it_behaves_like 'an incident management tracked event', :incident_management_incident_created + + it 'does create an incident label' do + expect { subject } + .to change { Label.where(incident_label_attributes).count }.by(1) + end + + context 'when invalid' do + before do + opts.merge!(title: '') + end + + it 'does not create an incident label prematurely' do + expect { subject }.not_to change(Label, :count) + end + end end it 'refreshes the number of open issues', :use_clean_rails_memory_store_caching do @@ -66,9 +85,9 @@ RSpec.describe Issues::CreateService do end context 'when current user cannot admin issues in the project' do - let(:guest) { create(:user) } + let_it_be(:guest) { create(:user) } - before do + before_all do project.add_guest(guest) end @@ -263,7 +282,7 @@ RSpec.describe Issues::CreateService do context 'issue create service' do context 'assignees' do - before do + before_all do project.add_maintainer(user) end @@ -329,7 +348,7 @@ RSpec.describe Issues::CreateService do } end - before do + before_all do project.add_maintainer(user) project.add_maintainer(assignee) end @@ -343,11 +362,11 @@ RSpec.describe Issues::CreateService do end context 'resolving discussions' do - let(:discussion) { create(:diff_note_on_merge_request).to_discussion } - let(:merge_request) { discussion.noteable } - let(:project) { merge_request.source_project } + let_it_be(:discussion) { create(:diff_note_on_merge_request).to_discussion } + let_it_be(:merge_request) { discussion.noteable } + let_it_be(:project) { merge_request.source_project } - before do + before_all do project.add_maintainer(user) end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index dade8f41ab7..f0092c35fda 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -78,6 +78,12 @@ RSpec.describe Issues::UpdateService, :mailer do expect(issue.severity).to eq(IssuableSeverity::DEFAULT) end + + it_behaves_like 'not an incident issue' do + before do + update_issue(opts) + end + end end context 'when issue type is incident' do @@ -88,6 +94,27 @@ RSpec.describe Issues::UpdateService, :mailer do expect(issue.severity).to eq('low') end + + it_behaves_like 'incident issue' do + before do + update_issue(opts) + end + end + + context 'with existing incident label' do + let_it_be(:incident_label) { create(:label, :incident, project: project) } + + before do + opts.delete(:label_ids) # don't override but retain existing labels + issue.labels << incident_label + end + + it_behaves_like 'incident issue' do + before do + update_issue(opts) + end + end + end end it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do @@ -113,15 +140,37 @@ RSpec.describe Issues::UpdateService, :mailer do end context 'issue in incident type' do + let(:current_user) { user } + let(:incident_label_attributes) { attributes_for(:label, :incident) } + before do - opts[:issue_type] = 'incident' + opts.merge!(issue_type: 'incident', confidential: true) end - let(:current_user) { user } - - subject { update_issue(confidential: true) } + subject { update_issue(opts) } it_behaves_like 'an incident management tracked event', :incident_management_incident_change_confidential + + it_behaves_like 'incident issue' do + before do + subject + end + end + + it 'does create an incident label' do + expect { subject } + .to change { Label.where(incident_label_attributes).count }.by(1) + end + + context 'when invalid' do + before do + opts.merge!(title: '') + end + + it 'does not create an incident label prematurely' do + expect { subject }.not_to change(Label, :count) + end + end end it 'updates open issue counter for assignees when issue is reassigned' do diff --git a/spec/support/shared_examples/graphql/members_shared_examples.rb b/spec/support/shared_examples/graphql/members_shared_examples.rb index a58e716efd2..3a046c3feec 100644 --- a/spec/support/shared_examples/graphql/members_shared_examples.rb +++ b/spec/support/shared_examples/graphql/members_shared_examples.rb @@ -20,3 +20,64 @@ RSpec.shared_examples 'a working membership object query' do |model_option| ).to eq('DEVELOPER') end end + +RSpec.shared_examples 'querying members with a group' do + let_it_be(:root_group) { create(:group, :private) } + let_it_be(:group_1) { create(:group, :private, parent: root_group, name: 'Main Group') } + let_it_be(:group_2) { create(:group, :private, parent: root_group) } + + let_it_be(:user_1) { create(:user, name: 'test user') } + let_it_be(:user_2) { create(:user, name: 'test user 2') } + let_it_be(:user_3) { create(:user, name: 'another user 1') } + let_it_be(:user_4) { create(:user, name: 'another user 2') } + + let_it_be(:root_group_member) { create(:group_member, user: user_4, group: root_group) } + let_it_be(:group_1_member) { create(:group_member, user: user_2, group: group_1) } + let_it_be(:group_2_member) { create(:group_member, user: user_3, group: group_2) } + + let(:args) { {} } + + subject do + resolve(described_class, obj: resource, args: args, ctx: { current_user: user_4 }) + end + + describe '#resolve' do + before do + group_1.add_maintainer(user_4) + end + + it 'finds all resource members' do + expect(subject).to contain_exactly(resource_member, group_1_member, root_group_member) + end + + context 'with search' do + context 'when the search term matches a user' do + let(:args) { { search: 'test' } } + + it 'searches users by user name' do + expect(subject).to contain_exactly(resource_member, group_1_member) + end + end + + context 'when the search term does not match any user' do + let(:args) { { search: 'nothing' } } + + it 'is empty' do + expect(subject).to be_empty + end + end + end + + context 'when user can not see resource members' do + let_it_be(:other_user) { create(:user) } + + subject do + resolve(described_class, obj: resource, args: args, ctx: { current_user: other_user }) + end + + it 'raises an error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + end +end diff --git a/spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb b/spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb new file mode 100644 index 00000000000..ed139e638bf --- /dev/null +++ b/spec/support/shared_examples/graphql/types/gitlab_style_deprecations_shared_examples.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'Gitlab-style deprecations' do + describe 'validations' do + it 'raises an informative error if `deprecation_reason` is used' do + expect { subject(deprecation_reason: 'foo') }.to raise_error( + ArgumentError, + 'Use `deprecated` property instead of `deprecation_reason`. ' \ + 'See https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#deprecating-fields-and-enum-values' + ) + end + + it 'raises an error if a required property is missing', :aggregate_failures do + expect { subject(deprecated: { milestone: '1.10' }) }.to raise_error( + ArgumentError, + 'Please provide a `reason` within `deprecated`' + ) + expect { subject(deprecated: { reason: 'Deprecation reason' }) }.to raise_error( + ArgumentError, + 'Please provide a `milestone` within `deprecated`' + ) + end + + it 'raises an error if milestone is not a String', :aggregate_failures do + expect { subject(deprecated: { milestone: 1.10, reason: 'Deprecation reason' }) }.to raise_error( + ArgumentError, + '`milestone` must be a `String`' + ) + end + end + + it 'adds a formatted `deprecated_reason` to the subject' do + deprecable = subject(deprecated: { milestone: '1.10', reason: 'Deprecation reason' }) + + expect(deprecable.deprecation_reason).to eq('Deprecation reason. Deprecated in 1.10') + end + + it 'appends to the description if given' do + deprecable = subject( + deprecated: { milestone: '1.10', reason: 'Deprecation reason' }, + description: 'Deprecable description' + ) + + expect(deprecable.description).to eq('Deprecable description. Deprecated in 1.10: Deprecation reason') + end + + it 'does not append to the description if it is absent' do + deprecable = subject(deprecated: { milestone: '1.10', reason: 'Deprecation reason' }) + + expect(deprecable.description).to be_nil + end +end diff --git a/spec/support/shared_examples/services/incident_shared_examples.rb b/spec/support/shared_examples/services/incident_shared_examples.rb new file mode 100644 index 00000000000..d6e79931df5 --- /dev/null +++ b/spec/support/shared_examples/services/incident_shared_examples.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# This shared_example requires the following variables: +# - issue (required) +# +# Usage: +# +# it_behaves_like 'incident issue' do +# let(:issue) { ... } +# end +# +# include_examples 'incident issue' +RSpec.shared_examples 'incident issue' do + let(:label_properties) { attributes_for(:label, :incident) } + + it 'has incident as issue type' do + expect(issue.issue_type).to eq('incident') + end + + it 'has exactly one incident label' do + expect(issue.labels).to be_one do |label| + label.slice(*label_properties.keys).symbolize_keys == label_properties + end + end +end + +# This shared_example requires the following variables: +# - issue (required) +# +# Usage: +# +# it_behaves_like 'not an incident issue' do +# let(:issue) { ... } +# end +# +# include_examples 'not an incident issue' +RSpec.shared_examples 'not an incident issue' do + let(:label_properties) { attributes_for(:label, :incident) } + + it 'has not incident as issue type' do + expect(issue.issue_type).not_to eq('incident') + end + + it 'has not an incident label' do + expect(issue.labels).not_to include(have_attributes(label_properties)) + end +end diff --git a/spec/workers/analytics/instance_statistics/count_job_trigger_worker_spec.rb b/spec/workers/analytics/instance_statistics/count_job_trigger_worker_spec.rb new file mode 100644 index 00000000000..620900b3402 --- /dev/null +++ b/spec/workers/analytics/instance_statistics/count_job_trigger_worker_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Analytics::InstanceStatistics::CountJobTriggerWorker do + it_behaves_like 'an idempotent worker' + + context 'triggers a job for each measurement identifiers' do + let(:expected_count) { Analytics::InstanceStatistics::Measurement.identifiers.size } + + it 'triggers CounterJobWorker jobs' do + subject.perform + + expect(Analytics::InstanceStatistics::CounterJobWorker.jobs.count).to eq(expected_count) + end + end + + context 'when the `store_instance_statistics_measurements` feature flag is off' do + before do + stub_feature_flags(store_instance_statistics_measurements: false) + end + + it 'does not trigger any CounterJobWorker job' do + subject.perform + + expect(Analytics::InstanceStatistics::CounterJobWorker.jobs.count).to eq(0) + end + end +end diff --git a/spec/workers/analytics/instance_statistics/counter_job_worker_spec.rb b/spec/workers/analytics/instance_statistics/counter_job_worker_spec.rb new file mode 100644 index 00000000000..8db86071dc4 --- /dev/null +++ b/spec/workers/analytics/instance_statistics/counter_job_worker_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Analytics::InstanceStatistics::CounterJobWorker do + let_it_be(:user_1) { create(:user) } + let_it_be(:user_2) { create(:user) } + + let(:users_measurement_identifier) { ::Analytics::InstanceStatistics::Measurement.identifiers.fetch(:users) } + let(:recorded_at) { Time.zone.now } + let(:job_args) { [users_measurement_identifier, user_1.id, user_2.id, recorded_at] } + + before do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) + end + + include_examples 'an idempotent worker' do + it 'counts a scope and stores the result' do + subject + + measurement = Analytics::InstanceStatistics::Measurement.first + expect(measurement.recorded_at).to be_like_time(recorded_at) + expect(measurement.identifier).to eq('users') + expect(measurement.count).to eq(2) + end + end + + context 'when no records are in the database' do + let(:users_measurement_identifier) { ::Analytics::InstanceStatistics::Measurement.identifiers.fetch(:groups) } + + subject { described_class.new.perform(users_measurement_identifier, nil, nil, recorded_at) } + + it 'sets 0 as the count' do + subject + + measurement = Analytics::InstanceStatistics::Measurement.first + expect(measurement.recorded_at).to be_like_time(recorded_at) + expect(measurement.identifier).to eq('groups') + expect(measurement.count).to eq(0) + end + end + + it 'does not raise error when inserting duplicated measurement' do + subject + + expect { subject }.not_to raise_error + end + + it 'does not insert anything when BatchCount returns error' do + allow(Gitlab::Database::BatchCount).to receive(:batch_count).and_return(Gitlab::Database::BatchCounter::FALLBACK) + + expect { subject }.not_to change { Analytics::InstanceStatistics::Measurement.count } + end +end