diff options
41 files changed, 439 insertions, 255 deletions
diff --git a/.markdownlint.json b/.markdownlint.json index f1749e79bae..897e096591c 100644 --- a/.markdownlint.json +++ b/.markdownlint.json @@ -59,6 +59,7 @@ "GitLab Shell", "GitLab Workhorse", "GitLab", + "Gitleaks", "Gmail", "Google", "Grafana", @@ -114,6 +115,7 @@ "Shibboleth", "Slack", "SMTP", + "SpotBugs", "SSH", "Tiller", "TOML", diff --git a/app/assets/javascripts/filtered_search/constants.js b/app/assets/javascripts/filtered_search/constants.js index d7264e96b13..7e7a2588951 100644 --- a/app/assets/javascripts/filtered_search/constants.js +++ b/app/assets/javascripts/filtered_search/constants.js @@ -4,3 +4,8 @@ export const DROPDOWN_TYPE = { hint: 'hint', operator: 'operator', }; + +export const FILTER_TYPE = { + none: 'none', + any: 'any', +}; diff --git a/app/assets/javascripts/filtered_search/dropdown_operator.js b/app/assets/javascripts/filtered_search/dropdown_operator.js index 0c8c8140ee9..1bbd33b6258 100644 --- a/app/assets/javascripts/filtered_search/dropdown_operator.js +++ b/app/assets/javascripts/filtered_search/dropdown_operator.js @@ -47,13 +47,17 @@ export default class DropdownOperator extends FilteredSearchDropdown { title: '=', help: __('is'), }, - { + ]; + + if (gon.features?.notIssuableQueries) { + dropdownData.push({ tag: 'not-equal', type: 'string', title: '!=', help: __('is not'), - }, - ]; + }); + } + this.droplab.changeHookList(this.hookId, this.dropdown, [Filter], this.config); this.droplab.setData(this.hookId, dropdownData); super.renderContent(forceShowList); diff --git a/app/assets/javascripts/filtered_search/filtered_search_dropdown.js b/app/assets/javascripts/filtered_search/filtered_search_dropdown.js index 2b6e1f25dc6..f7ce2ea01e0 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown.js +++ b/app/assets/javascripts/filtered_search/filtered_search_dropdown.js @@ -1,6 +1,7 @@ import DropdownUtils from './dropdown_utils'; import FilteredSearchDropdownManager from './filtered_search_dropdown_manager'; import FilteredSearchVisualTokens from './filtered_search_visual_tokens'; +import { FILTER_TYPE } from './constants'; const DATA_DROPDOWN_TRIGGER = 'data-dropdown-trigger'; @@ -74,6 +75,9 @@ export default class FilteredSearchDropdown { renderContent(forceShowList = false) { const currentHook = this.getCurrentHook(); + + FilteredSearchDropdown.hideDropdownItemsforNotOperator(currentHook); + if (forceShowList && currentHook && currentHook.list.hidden) { currentHook.list.show(); } @@ -138,4 +142,41 @@ export default class FilteredSearchDropdown { hook.list.render(results); } } + + /** + * Hide None & Any options from the current dropdown. + * Hiding happens only for NOT operator. + */ + static hideDropdownItemsforNotOperator(currentHook) { + const lastOperator = FilteredSearchVisualTokens.getLastTokenOperator(); + + if (lastOperator === '!=') { + const { list: dropdownEl } = currentHook.list; + + let shouldHideDivider = true; + + // Iterate over all the static dropdown values, + // then hide `None` and `Any` items. + Array.from(dropdownEl.querySelectorAll('li[data-value]')).forEach(itemEl => { + const { + dataset: { value }, + } = itemEl; + + if (value.toLowerCase() === FILTER_TYPE.none || value.toLowerCase() === FILTER_TYPE.any) { + itemEl.classList.add('hidden'); + } else { + // If we encountered any element other than None/Any, then + // we shouldn't hide the divider + shouldHideDivider = false; + } + }); + + if (shouldHideDivider) { + const divider = dropdownEl.querySelector('li.divider'); + if (divider) { + divider.classList.add('hidden'); + } + } + } + } } diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index a18c80b996e..bad55b34fb3 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -20,6 +20,9 @@ module Boards skip_before_action :authenticate_user!, only: [:index] before_action :validate_id_list, only: [:bulk_move] before_action :can_move_issues?, only: [:bulk_move] + before_action do + push_frontend_feature_flag(:not_issuable_queries, board.group, default_enabled: true) + end def index list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params) diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index ca43bf42580..0b1b3f2bcba 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -11,6 +11,9 @@ module IssuableActions before_action only: :show do push_frontend_feature_flag(:scoped_labels, default_enabled: true) end + before_action do + push_frontend_feature_flag(:not_issuable_queries, @project, default_enabled: true) + end end def permitted_keys diff --git a/app/controllers/concerns/issuable_collections_action.rb b/app/controllers/concerns/issuable_collections_action.rb index 0a6f684a9fc..78b3c6771b3 100644 --- a/app/controllers/concerns/issuable_collections_action.rb +++ b/app/controllers/concerns/issuable_collections_action.rb @@ -32,6 +32,10 @@ module IssuableCollectionsAction private + def set_not_query_feature_flag(object = nil) + push_frontend_feature_flag(:not_issuable_queries, object, default_enabled: true) + end + def sorting_field case action_name when 'issues' diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 1668cf004f8..dd9e6488bc5 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -10,6 +10,7 @@ class DashboardController < Dashboard::ApplicationController before_action :projects, only: [:issues, :merge_requests] before_action :set_show_full_reference, only: [:issues, :merge_requests] before_action :check_filters_presence!, only: [:issues, :merge_requests] + before_action :set_not_query_feature_flag respond_to :html diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 44120fda17c..3dbf57285b0 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -31,6 +31,10 @@ class GroupsController < Groups::ApplicationController push_frontend_feature_flag(:vue_issuables_list, @group) end + before_action do + set_not_query_feature_flag(@group) + end + before_action :export_rate_limit, only: [:export, :download_export] skip_cross_project_access_check :index, :new, :create, :edit, :update, diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 5687b375cf0..7014f2ec205 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -40,7 +40,7 @@ class IssuableFinder requires_cross_project_access unless: -> { params.project? } - NEGATABLE_PARAMS_HELPER_KEYS = %i[include_subgroups in].freeze + NEGATABLE_PARAMS_HELPER_KEYS = %i[project_id scope status include_subgroups].freeze attr_accessor :current_user, :params @@ -68,7 +68,7 @@ class IssuableFinder # This should not be used in controller strong params! def negatable_scalar_params - @negatable_scalar_params ||= scalar_params + %i[project_id group_id] + @negatable_scalar_params ||= scalar_params - %i[search in] end # This should not be used in controller strong params! @@ -100,7 +100,7 @@ class IssuableFinder items = filter_items(items) # Let's see if we have to negate anything - items = by_negation(items) + items = filter_negated_items(items) # This has to be last as we use a CTE as an optimization fence # for counts by passing the force_cte param and enabling the @@ -132,6 +132,22 @@ class IssuableFinder by_my_reaction_emoji(items) end + # Negates all params found in `negatable_params` + def filter_negated_items(items) + return items unless Feature.enabled?(:not_issuable_queries, params.group || params.project, default_enabled: true) + + # API endpoints send in `nil` values so we test if there are any non-nil + return items unless not_params.present? && not_params.values.any? + + items = by_negated_author(items) + items = by_negated_assignee(items) + items = by_negated_label(items) + items = by_negated_milestone(items) + items = by_negated_release(items) + items = by_negated_my_reaction_emoji(items) + by_negated_iids(items) + end + def row_count Gitlab::IssuablesCountForState.new(self).for_state_or_opened(params[:state]) end @@ -189,6 +205,21 @@ class IssuableFinder private + def not_params + strong_memoize(:not_params) do + params_class.new(params[:not].dup, current_user, klass).tap do |not_params| + next unless not_params.present? + + # These are "helper" params that modify the results, like :in and :search. They usually come in at the top-level + # params, but if they do come in inside the `:not` params, the inner ones should take precedence. + not_helpers = params.slice(*NEGATABLE_PARAMS_HELPER_KEYS).merge(params[:not].slice(*NEGATABLE_PARAMS_HELPER_KEYS)) + not_helpers.each do |key, value| + not_params[key] = value unless not_params[key].present? + end + end + end + end + def force_cte? !!params[:force_cte] end @@ -215,33 +246,6 @@ class IssuableFinder klass.available_states.key(value) end - # Negates all params found in `negatable_params` - # rubocop: disable CodeReuse/ActiveRecord - def by_negation(items) - not_params = params[:not].dup - # API endpoints send in `nil` values so we test if there are any non-nil - return items unless not_params.present? && not_params.values.any? - - not_params.keep_if { |_k, v| v.present? }.each do |(key, value)| - # These aren't negatable params themselves, but rather help other searches, so we skip them. - # They will be added into all the NOT searches. - next if NEGATABLE_PARAMS_HELPER_KEYS.include?(key.to_sym) - next unless self.class.negatable_params.include?(key.to_sym) - - # These are "helper" params that are required inside the NOT to get the right results. They usually come in - # at the top-level params, but if they do come in inside the `:not` params, they should take precedence. - not_helpers = params.slice(*NEGATABLE_PARAMS_HELPER_KEYS).merge(params[:not].slice(*NEGATABLE_PARAMS_HELPER_KEYS)) - not_param = { key => value }.with_indifferent_access.merge(not_helpers).merge(not_query: true) - - items_to_negate = self.class.new(current_user, not_param).execute - - items = items.where.not(id: items_to_negate) - end - - items - end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def by_scope(items) return items.none if params.current_user_related? && !current_user @@ -326,6 +330,12 @@ class IssuableFinder # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord + def by_negated_iids(items) + not_params[:iids].present? ? items.where.not(iid: not_params[:iids]) : items + end + # rubocop: enable CodeReuse/ActiveRecord + + # rubocop: disable CodeReuse/ActiveRecord def sort(items) # Ensure we always have an explicit sort order (instead of inheriting # multiple orders when combining ActiveRecord::Relation objects). @@ -347,9 +357,19 @@ class IssuableFinder end # rubocop: enable CodeReuse/ActiveRecord - def by_assignee(items) - return items.assigned_to(params.assignees) if not_query? && params.assignees.any? + # rubocop: disable CodeReuse/ActiveRecord + def by_negated_author(items) + if not_params.author + items.where.not(author_id: not_params.author.id) + elsif not_params.author_id? || not_params.author_username? # author not found + items.none + else + items + end + end + # rubocop: enable CodeReuse/ActiveRecord + def by_assignee(items) if params.filter_by_no_assignee? items.unassigned elsif params.filter_by_any_assignee? @@ -363,6 +383,17 @@ class IssuableFinder end end + def by_negated_assignee(items) + # We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB" + if not_params.assignees.present? + items.not_assigned_to(not_params.assignees) + elsif not_params.assignee_id? || not_params.assignee_username? # assignee not found + items.none + else + items + end + end + # rubocop: disable CodeReuse/ActiveRecord def by_milestone(items) return items unless params.milestones? @@ -382,6 +413,20 @@ class IssuableFinder end # rubocop: enable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord + def by_negated_milestone(items) + return items unless not_params.milestones? + + if not_params.filter_by_upcoming_milestone? + items.joins(:milestone).merge(Milestone.not_upcoming) + elsif not_params.filter_by_started_milestone? + items.joins(:milestone).merge(Milestone.not_started) + else + items.without_particular_milestone(not_params[:milestone_title]) + end + end + # rubocop: enable CodeReuse/ActiveRecord + def by_release(items) return items unless params.releases? @@ -394,6 +439,12 @@ class IssuableFinder end end + def by_negated_release(items) + return items unless not_params.releases? + + items.without_particular_release(not_params[:release_tag], not_params[:project_id]) + end + def by_label(items) return items unless params.labels? @@ -402,10 +453,16 @@ class IssuableFinder elsif params.filter_by_any_label? items.any_label else - items.with_label(params.label_names, params[:sort], not_query: not_query?) + items.with_label(params.label_names, params[:sort]) end end + def by_negated_label(items) + return items unless not_params.labels? + + items.without_particular_labels(not_params.label_names) + end + def by_my_reaction_emoji(items) return items unless params[:my_reaction_emoji] && current_user @@ -418,11 +475,13 @@ class IssuableFinder end end - def by_non_archived(items) - params[:non_archived].present? ? items.non_archived : items + def by_negated_my_reaction_emoji(items) + return items unless not_params[:my_reaction_emoji] && current_user + + items.not_awarded(current_user, not_params[:my_reaction_emoji]) end - def not_query? - !!params[:not_query] + def by_non_archived(items) + params[:non_archived].present? ? items.non_archived : items end end diff --git a/app/finders/issuable_finder/params.rb b/app/finders/issuable_finder/params.rb index 120ef364368..adf9f1ca9d8 100644 --- a/app/finders/issuable_finder/params.rb +++ b/app/finders/issuable_finder/params.rb @@ -132,6 +132,8 @@ class IssuableFinder def project strong_memoize(:project) do + next nil unless params[:project_id].present? + project = Project.find(params[:project_id]) project = nil unless Ability.allowed?(current_user, :"read_#{klass.to_ability_name}", project) diff --git a/app/finders/issues_finder/params.rb b/app/finders/issues_finder/params.rb index aaeead7c709..cd92b79265d 100644 --- a/app/finders/issues_finder/params.rb +++ b/app/finders/issues_finder/params.rb @@ -50,4 +50,4 @@ class IssuesFinder end end -IssuableFinder::Params.prepend_if_ee('EE::IssuesFinder::Params') +IssuesFinder::Params.prepend_if_ee('EE::IssuesFinder::Params') diff --git a/app/graphql/types/snippet_type.rb b/app/graphql/types/snippet_type.rb index 4ebdbd5766c..b23c4f71ffa 100644 --- a/app/graphql/types/snippet_type.rb +++ b/app/graphql/types/snippet_type.rb @@ -14,7 +14,7 @@ module Types expose_permissions Types::PermissionTypes::Snippet field :id, GraphQL::ID_TYPE, - description: 'Id of the snippet', + description: 'ID of the snippet', null: false field :title, GraphQL::STRING_TYPE, diff --git a/app/graphql/types/todo_type.rb b/app/graphql/types/todo_type.rb index 5ce5093c55e..08e7fabeb74 100644 --- a/app/graphql/types/todo_type.rb +++ b/app/graphql/types/todo_type.rb @@ -10,7 +10,7 @@ module Types authorize :read_todo field :id, GraphQL::ID_TYPE, - description: 'Id of the todo', + description: 'ID of the todo', null: false field :project, Types::ProjectType, diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb index 0f2a389f0a3..f1b2bebfb47 100644 --- a/app/models/concerns/awardable.rb +++ b/app/models/concerns/awardable.rb @@ -14,32 +14,29 @@ module Awardable class_methods do def awarded(user, name = nil) - sql = <<~EOL - EXISTS ( - SELECT TRUE - FROM award_emoji - WHERE user_id = :user_id AND - #{"name = :name AND" if name.present?} - awardable_type = :awardable_type AND - awardable_id = #{self.arel_table.name}.id - ) - EOL + award_emoji_table = Arel::Table.new('award_emoji') + inner_query = award_emoji_table + .project('true') + .where(award_emoji_table[:user_id].eq(user.id)) + .where(award_emoji_table[:awardable_type].eq(self.name)) + .where(award_emoji_table[:awardable_id].eq(self.arel_table[:id])) + + inner_query = inner_query.where(award_emoji_table[:name].eq(name)) if name.present? - where(sql, user_id: user.id, name: name, awardable_type: self.name) + where(inner_query.exists) end - def not_awarded(user) - sql = <<~EOL - NOT EXISTS ( - SELECT TRUE - FROM award_emoji - WHERE user_id = :user_id AND - awardable_type = :awardable_type AND - awardable_id = #{self.arel_table.name}.id - ) - EOL + def not_awarded(user, name = nil) + award_emoji_table = Arel::Table.new('award_emoji') + inner_query = award_emoji_table + .project('true') + .where(award_emoji_table[:user_id].eq(user.id)) + .where(award_emoji_table[:awardable_type].eq(self.name)) + .where(award_emoji_table[:awardable_id].eq(self.arel_table[:id])) + + inner_query = inner_query.where(award_emoji_table[:name].eq(name)) if name.present? - where(sql, user_id: user.id, awardable_type: self.name) + where(inner_query.exists.not) end def order_upvotes_desc diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 37f2209b9d2..a1b14dca4ac 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -115,9 +115,31 @@ module Issuable end # rubocop:enable GitlabSecurity/SqlInjection + scope :not_assigned_to, ->(users) do + assignees_table = Arel::Table.new("#{to_ability_name}_assignees") + sql = assignees_table.project('true') + .where(assignees_table[:user_id].in(users)) + .where(Arel::Nodes::SqlLiteral.new("#{to_ability_name}_id = #{to_ability_name}s.id")) + where(sql.exists.not) + end + + scope :without_particular_labels, ->(label_names) do + labels_table = Label.arel_table + label_links_table = LabelLink.arel_table + issuables_table = klass.arel_table + inner_query = label_links_table.project('true') + .join(labels_table, Arel::Nodes::InnerJoin).on(labels_table[:id].eq(label_links_table[:label_id])) + .where(label_links_table[:target_type].eq(name) + .and(label_links_table[:target_id].eq(issuables_table[:id])) + .and(labels_table[:title].in(label_names))) + .exists.not + + where(inner_query) + end + scope :without_label, -> { joins("LEFT OUTER JOIN label_links ON label_links.target_type = '#{name}' AND label_links.target_id = #{table_name}.id").where(label_links: { id: nil }) } scope :with_label_ids, ->(label_ids) { joins(:label_links).where(label_links: { label_id: label_ids }) } - scope :any_label, -> { joins(:label_links).group(:id) } + scope :any_label, -> { joins(:label_links).distinct } scope :join_project, -> { joins(:project) } scope :inc_notes_with_associations, -> { includes(notes: [:project, :author, :award_emoji]) } scope :references_project, -> { references(:project) } @@ -286,9 +308,8 @@ module Issuable .reorder(Gitlab::Database.nulls_last_order('highest_priority', direction)) end - def with_label(title, sort = nil, not_query: false) - multiple_labels = title.is_a?(Array) && title.size > 1 - if multiple_labels && !not_query + def with_label(title, sort = nil) + if title.is_a?(Array) && title.size > 1 joins(:labels).where(labels: { title: title }).group(*grouping_columns(sort)).having("COUNT(DISTINCT labels.title) = #{title.size}") else joins(:labels).where(labels: { title: title }) diff --git a/app/models/concerns/milestoneable.rb b/app/models/concerns/milestoneable.rb index 3ffb32f94fc..8f8494a9678 100644 --- a/app/models/concerns/milestoneable.rb +++ b/app/models/concerns/milestoneable.rb @@ -17,8 +17,10 @@ module Milestoneable scope :of_milestones, ->(ids) { where(milestone_id: ids) } scope :any_milestone, -> { where('milestone_id IS NOT NULL') } scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) } + scope :without_particular_milestone, ->(title) { left_outer_joins(:milestone).where("milestones.title != ? OR milestone_id IS NULL", title) } scope :any_release, -> { joins_milestone_releases } scope :with_release, -> (tag, project_id) { joins_milestone_releases.where( milestones: { releases: { tag: tag, project_id: project_id } } ) } + scope :without_particular_release, -> (tag, project_id) { joins_milestone_releases.where.not( milestones: { releases: { tag: tag, project_id: project_id } } ) } scope :left_joins_milestones, -> { joins("LEFT OUTER JOIN milestones ON #{table_name}.milestone_id = milestones.id") } scope :order_milestone_due_desc, -> { left_joins_milestones.reorder(Arel.sql('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date DESC')) } diff --git a/app/models/milestone.rb b/app/models/milestone.rb index b1cac9af30f..da6490611fd 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -19,6 +19,12 @@ class Milestone < ApplicationRecord has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent scope :started, -> { active.where('milestones.start_date <= CURRENT_DATE') } + scope :not_started, -> { active.where('milestones.start_date > CURRENT_DATE') } + scope :not_upcoming, -> do + active + .where('milestones.due_date <= CURRENT_DATE') + .order(:project_id, :group_id, :due_date) + end scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) } scope :reorder_by_due_date_asc, -> { reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) } diff --git a/changelogs/unreleased/198324-avoid_subqueries.yml b/changelogs/unreleased/198324-avoid_subqueries.yml new file mode 100644 index 00000000000..4aa37fd4c0d --- /dev/null +++ b/changelogs/unreleased/198324-avoid_subqueries.yml @@ -0,0 +1,5 @@ +--- +title: Speed up NOT Issue filters +merge_request: 27639 +author: +type: performance diff --git a/changelogs/unreleased/33200-add-empty-needs-to-auto-devops-test.yml b/changelogs/unreleased/33200-add-empty-needs-to-auto-devops-test.yml new file mode 100644 index 00000000000..81f02558134 --- /dev/null +++ b/changelogs/unreleased/33200-add-empty-needs-to-auto-devops-test.yml @@ -0,0 +1,5 @@ +--- +title: Allow Auto DevOps Test stage to start immediately +merge_request: 31185 +author: +type: other diff --git a/doc/.vale/gitlab/spelling-exceptions.txt b/doc/.vale/gitlab/spelling-exceptions.txt index bf78eaaee8c..1cabfe303fa 100644 --- a/doc/.vale/gitlab/spelling-exceptions.txt +++ b/doc/.vale/gitlab/spelling-exceptions.txt @@ -101,6 +101,7 @@ failovers failsafe favicon firewalled +Flawfinder Flowdock Fluentd Forgerock @@ -112,9 +113,11 @@ Gitea GitHub GitLab gitlabsos +Gitleaks Gitter Gmail Google +Gosec Gradle Grafana gravatar @@ -160,6 +163,7 @@ Kibana Knative Kramdown Kubernetes +Kubesec Laravel LDAP Libravatar @@ -302,12 +306,15 @@ sanitization serializer serializers serializing +Sitespeed Slack Slony SMTP +Sobelow Sourcegraph spidering Splunk +SpotBugs SSH storable strace diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 79dd7a34c26..baf257eb2ff 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -9017,7 +9017,7 @@ type Snippet implements Noteable { httpUrlToRepo: String """ - Id of the snippet + ID of the snippet """ id: ID! @@ -9463,7 +9463,7 @@ type Todo { group: Group """ - Id of the todo + ID of the todo """ id: ID! diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 7db78b5d34b..83809db6a9b 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -26985,7 +26985,7 @@ }, { "name": "id", - "description": "Id of the snippet", + "description": "ID of the snippet", "args": [ ], @@ -28412,7 +28412,7 @@ }, { "name": "id", - "description": "Id of the todo", + "description": "ID of the todo", "args": [ ], diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 390e09a7cef..3259ce316e7 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1365,7 +1365,7 @@ Represents a snippet entry | `descriptionHtml` | String | The GitLab Flavored Markdown rendering of `description` | | `fileName` | String | File Name of the snippet | | `httpUrlToRepo` | String | HTTP URL to the snippet repository | -| `id` | ID! | Id of the snippet | +| `id` | ID! | ID of the snippet | | `project` | Project | The project the snippet is associated with | | `rawUrl` | String! | Raw URL of the snippet | | `sshUrlToRepo` | String | SSH URL to the snippet repository | @@ -1460,7 +1460,7 @@ Representing a todo entry | `body` | String! | Body of the todo | | `createdAt` | Time! | Timestamp this todo was created | | `group` | Group | Group this todo is associated with | -| `id` | ID! | Id of the todo | +| `id` | ID! | ID of the todo | | `project` | Project | The project this todo is associated with | | `state` | TodoStateEnum! | State of the todo | | `targetType` | TodoTargetEnum! | Target type of the todo | diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index 0f42e062901..9299d096851 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -13,7 +13,7 @@ to learn how to protect your organization. ## Overview -If you are using [GitLab CI/CD](../../../ci/README.md), you can analyze your source code for known +If you're using [GitLab CI/CD](../../../ci/README.md), you can analyze your source code for known vulnerabilities using Static Application Security Testing (SAST). You can take advantage of SAST by doing one of the following: @@ -37,7 +37,7 @@ The results are sorted by the priority of the vulnerability: 1. Everything else NOTE: **Note:** -A pipeline consists of multiple jobs, including SAST and DAST scanning. If any job fails to finish for any reason, the security dashboard will not show SAST scanner output. For example, if the SAST job finishes but the DAST job fails, the security dashboard will not show SAST results. The analyzer will output an [exit code](../../../development/integrations/secure.md#exit-code) on failure. +A pipeline consists of multiple jobs, including SAST and DAST scanning. If any job fails to finish for any reason, the security dashboard won't show SAST scanner output. For example, if the SAST job finishes but the DAST job fails, the security dashboard won't show SAST results. The analyzer will output an [exit code](../../../development/integrations/secure.md#exit-code) on failure. ## Use cases @@ -55,12 +55,12 @@ executor running in privileged mode. If you're using the shared Runners on GitLa this is enabled by default. Privileged mode is not necessary if you've [disabled Docker in Docker -for SAST](#disabling-docker-in-docker-for-sast) +for SAST](#disabling-docker-in-docker-for-sast). CAUTION: **Caution:** Our SAST jobs currently expect a Linux container type. Windows containers are not yet supported. CAUTION: **Caution:** -If you use your own Runners, make sure that the Docker version you have installed +If you use your own Runners, make sure the Docker version installed is **not** `19.03.0`. See [troubleshooting information](#error-response-from-daemon-error-processing-tar-file-docker-tar-relocation-error) for details. ## Supported languages and frameworks @@ -71,7 +71,7 @@ The following table shows which languages, package managers and frameworks are s |-----------------------------------------------------------------------------|----------------------------------------------------------------------------------------|------------------------------| | .NET | [Security Code Scan](https://security-code-scan.github.io) | 11.0 | | Any | [Gitleaks](https://github.com/zricethezav/gitleaks) and [TruffleHog](https://github.com/dxa4481/truffleHog) | 11.9 | -| Apex (Salesforce) | [pmd](https://pmd.github.io/pmd/index.html) | 12.1 | +| Apex (Salesforce) | [PMD](https://pmd.github.io/pmd/index.html) | 12.1 | | C/C++ | [Flawfinder](https://dwheeler.com/flawfinder/) | 10.7 | | Elixir (Phoenix) | [Sobelow](https://github.com/nccgroup/sobelow) | 11.10 | | Go | [Gosec](https://github.com/securego/gosec) | 10.7 | @@ -85,7 +85,7 @@ The following table shows which languages, package managers and frameworks are s | React | [ESLint react plugin](https://github.com/yannickcr/eslint-plugin-react) | 12.5 | | Ruby on Rails | [brakeman](https://brakemanscanner.org) | 10.3 | | Scala ([Ant](https://ant.apache.org/), [Gradle](https://gradle.org/), [Maven](https://maven.apache.org/) and [SBT](https://www.scala-sbt.org/)) | [SpotBugs](https://spotbugs.github.io/) with the [find-sec-bugs](https://find-sec-bugs.github.io/) plugin | 11.0 (SBT) & 11.9 (Ant, Gradle, Maven) | -| TypeScript | [TSLint config security](https://github.com/webschik/tslint-config-security/) | 11.9 | +| TypeScript | [`tslint-config-security`](https://github.com/webschik/tslint-config-security/) | 11.9 | NOTE: **Note:** The Java analyzers can also be used for variants like the @@ -104,7 +104,7 @@ provided by [Auto DevOps](../../../topics/autodevops/index.md). For GitLab 11.9 and later, to enable SAST you must [include](../../../ci/yaml/README.md#includetemplate) the [`SAST.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml) -that is provided as a part of your GitLab installation. For GitLab versions earlier than 11.9, you +provided as a part of your GitLab installation. For GitLab versions earlier than 11.9, you can copy and use the job as defined that template. Add the following to your `.gitlab-ci.yml` file: @@ -122,14 +122,13 @@ The results will be saved as a that you can later download and analyze. Due to implementation limitations, we always take the latest SAST artifact available. Behind the scenes, the [GitLab SAST Docker image](https://gitlab.com/gitlab-org/security-products/sast) -is used to detect the languages/frameworks and in turn runs the matching scan tools. +is used to detect the languages or frameworks used, and in turn runs the matching scan tools. ### Customizing the SAST settings The SAST settings can be changed through [environment variables](#available-variables) by using the [`variables`](../../../ci/yaml/README.md#variables) parameter in `.gitlab-ci.yml`. - In the following example, we include the SAST template and at the same time we set the `SAST_GOSEC_LEVEL` variable to `2`: @@ -142,7 +141,7 @@ variables: ``` Because the template is [evaluated before](../../../ci/yaml/README.md#include) -the pipeline configuration, the last mention of the variable will take precedence. +the pipeline configuration, the last mention of the variable takes precedence. ### Overriding the SAST template @@ -173,10 +172,10 @@ it via [custom environment variables](#custom-environment-variables). #### Using a variable to pass username and password to a private Maven repository -If you have a private Maven repository which requires login credentials, +If your private Maven repository requires login credentials, you can use the `MAVEN_CLI_OPTS` environment variable. -Read more on [how to use private Maven repos](../index.md#using-private-maven-repos). +Read more on [how to use private Maven repositories](../index.md#using-private-maven-repos). ### Disabling Docker in Docker for SAST @@ -198,11 +197,11 @@ to start relevant analyzers depending on the detected repository language(s) ins [orchestrator](https://gitlab.com/gitlab-org/security-products/dependency-scanning/). However, there are some differences in the way repository languages are detected between DIND and non-DIND. You can observe these differences by checking both Linguist and the common library. For instance, Linguist -looks for `*.java` files to spin up the [spotbugs](https://gitlab.com/gitlab-org/security-products/analyzers/spotbugs) +looks for `*.java` files to spin up the [SpotBugs](https://gitlab.com/gitlab-org/security-products/analyzers/spotbugs) image, while orchestrator only looks for the existence of `pom.xml`, `build.xml`, `gradlew`, -`grailsw`, or `mvnw`. GitLab uses Linguist to detect new file types in the default branch. This -means that when introducing files or dependencies for a new language or package manager, the -corresponding scans won't be triggered in the MR and will only run on the default branch once the +`grailsw`, or `mvnw`. GitLab uses Linguist to detect new file types in the default branch. +When introducing files or dependencies for a new language or package manager, the +corresponding scans won't be triggered in the MR, and will only run on the default branch once the MR is merged. This will be addressed by [#211702](https://gitlab.com/gitlab-org/gitlab/-/issues/211702). NOTE: **Note:** @@ -210,13 +209,13 @@ With the current language detection logic, any new languages or frameworks intro context of a merge request don't trigger a corresponding scan. These scans only occur once the code is committed to the default branch. -#### Enabling kubesec analyzer +#### Enabling Kubesec analyzer > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12752) in GitLab Ultimate 12.6. When [Docker in Docker is disabled](#disabling-docker-in-docker-for-sast), you will need to set `SCAN_KUBERNETES_MANIFESTS` to `"true"` to enable the -kubesec analyzer. In `.gitlab-ci.yml`, define: +Kubesec analyzer. In `.gitlab-ci.yml`, define: ```yaml include: @@ -310,10 +309,10 @@ Some analyzers make it possible to filter out vulnerabilities under a given thre | `SAST_BRAKEMAN_LEVEL` | 1 | Ignore Brakeman vulnerabilities under given confidence level. Integer, 1=Low 3=High. | | `SAST_FLAWFINDER_LEVEL` | 1 | Ignore Flawfinder vulnerabilities under given risk level. Integer, 0=No risk, 5=High risk. | | `SAST_GITLEAKS_ENTROPY_LEVEL` | 8.0 | Minimum entropy for secret detection. Float, 0.0 = low, 8.0 = high. | -| `SAST_GOSEC_LEVEL` | 0 | Ignore gosec vulnerabilities under given confidence level. Integer, 0=Undefined, 1=Low, 2=Medium, 3=High. | -| `SAST_GITLEAKS_COMMIT_FROM` | - | The commit a gitleaks scan starts at. | -| `SAST_GITLEAKS_COMMIT_TO` | - | The commit a gitleaks scan ends at. | -| `SAST_GITLEAKS_HISTORIC_SCAN` | false | Flag to enable a historic gitleaks scan. | +| `SAST_GOSEC_LEVEL` | 0 | Ignore Gosec vulnerabilities under given confidence level. Integer, 0=Undefined, 1=Low, 2=Medium, 3=High. | +| `SAST_GITLEAKS_COMMIT_FROM` | - | The commit a Gitleaks scan starts at. | +| `SAST_GITLEAKS_COMMIT_TO` | - | The commit a Gitleaks scan ends at. | +| `SAST_GITLEAKS_HISTORIC_SCAN` | false | Flag to enable a historic Gitleaks scan. | #### Docker-in-Docker orchestrator @@ -323,9 +322,9 @@ The following variables configure the Docker-in-Docker orchestrator. |------------------------------------------|---------------|-------------| | `SAST_ANALYZER_IMAGES` | | Comma-separated list of custom images. Default images are still enabled. Read more about [customizing analyzers](analyzers.md). Not available when [Docker-in-Docker is disabled](#disabling-docker-in-docker-for-sast). | | `SAST_PULL_ANALYZER_IMAGES` | 1 | Pull the images from the Docker registry (set to 0 to disable). Read more about [customizing analyzers](analyzers.md). Not available when [Docker-in-Docker is disabled](#disabling-docker-in-docker-for-sast). | -| `SAST_DOCKER_CLIENT_NEGOTIATION_TIMEOUT` | 2m | Time limit for Docker client negotiation. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". For example, "300ms", "1.5h" or "2h45m". | -| `SAST_PULL_ANALYZER_IMAGE_TIMEOUT` | 5m | Time limit when pulling the image of an analyzer. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". For example, "300ms", "1.5h" or "2h45m". | -| `SAST_RUN_ANALYZER_TIMEOUT` | 20m | Time limit when running an analyzer. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". For example, "300ms", "1.5h" or "2h45m".| +| `SAST_DOCKER_CLIENT_NEGOTIATION_TIMEOUT` | 2m | Time limit for Docker client negotiation. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. For example, `300ms`, `1.5h` or `2h45m`. | +| `SAST_PULL_ANALYZER_IMAGE_TIMEOUT` | 5m | Time limit when pulling the image of an analyzer. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. For example, `300ms`, `1.5h` or `2h45m`. | +| `SAST_RUN_ANALYZER_TIMEOUT` | 20m | Time limit when running an analyzer. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. For example, `300ms`, `1.5h` or `2h45m`.| NOTE: **Note:** Timeout variables are not applicable for setups with [disabled Docker In Docker](index.md#disabling-docker-in-docker-for-sast). @@ -336,18 +335,18 @@ Some analyzers can be customized with environment variables. | Environment variable | Analyzer | Description | |-----------------------------|----------|-------------| -| `SCAN_KUBERNETES_MANIFESTS` | kubesec | Set to `"true"` to scan Kubernetes manifests when [Docker in Docker](#disabling-docker-in-docker-for-sast) is disabled. | -| `ANT_HOME` | spotbugs | The `ANT_HOME` environment variable. | -| `ANT_PATH` | spotbugs | Path to the `ant` executable. | -| `GRADLE_PATH` | spotbugs | Path to the `gradle` executable. | -| `JAVA_OPTS` | spotbugs | Additional arguments for the `java` executable. | -| `JAVA_PATH` | spotbugs | Path to the `java` executable. | -| `SAST_JAVA_VERSION` | spotbugs | Which Java version to use. Supported versions are `8` and `11`. Defaults to `8`. | -| `MAVEN_CLI_OPTS` | spotbugs | Additional arguments for the `mvn` or `mvnw` executable. | -| `MAVEN_PATH` | spotbugs | Path to the `mvn` executable. | -| `MAVEN_REPO_PATH` | spotbugs | Path to the Maven local repository (shortcut for the `maven.repo.local` property). | -| `SBT_PATH` | spotbugs | Path to the `sbt` executable. | -| `FAIL_NEVER` | spotbugs | Set to `1` to ignore compilation failure. | +| `SCAN_KUBERNETES_MANIFESTS` | Kubesec | Set to `"true"` to scan Kubernetes manifests when [Docker in Docker](#disabling-docker-in-docker-for-sast) is disabled. | +| `ANT_HOME` | SpotBugs | The `ANT_HOME` environment variable. | +| `ANT_PATH` | SpotBugs | Path to the `ant` executable. | +| `GRADLE_PATH` | SpotBugs | Path to the `gradle` executable. | +| `JAVA_OPTS` | SpotBugs | Additional arguments for the `java` executable. | +| `JAVA_PATH` | SpotBugs | Path to the `java` executable. | +| `SAST_JAVA_VERSION` | SpotBugs | Which Java version to use. Supported versions are `8` and `11`. Defaults to `8`. | +| `MAVEN_CLI_OPTS` | SpotBugs | Additional arguments for the `mvn` or `mvnw` executable. | +| `MAVEN_PATH` | SpotBugs | Path to the `mvn` executable. | +| `MAVEN_REPO_PATH` | SpotBugs | Path to the Maven local repository (shortcut for the `maven.repo.local` property). | +| `SBT_PATH` | SpotBugs | Path to the `sbt` executable. | +| `FAIL_NEVER` | SpotBugs | Set to `1` to ignore compilation failure. | #### Custom environment variables @@ -454,8 +453,8 @@ the report JSON unless stated otherwise. Presence of optional fields depends on | `version` | Report syntax version used to generate this JSON. | | `vulnerabilities` | Array of vulnerability objects. | | `vulnerabilities[].id` | Unique identifier of the vulnerability. | -| `vulnerabilities[].category` | Where this vulnerability belongs (SAST, Dependency Scanning etc.). For SAST, it will always be `sast`. | -| `vulnerabilities[].name` | Name of the vulnerability, this must not include the occurrence's specific information. Optional. | +| `vulnerabilities[].category` | Where this vulnerability belongs (such as SAST, Dependency Scanning). For SAST, it will always be `sast`. | +| `vulnerabilities[].name` | Name of the vulnerability. Must not include the occurrence's specific information. Optional. | | `vulnerabilities[].message` | A short text that describes the vulnerability, it may include the occurrence's specific information. Optional. | | `vulnerabilities[].description` | A long text that describes the vulnerability. Optional. | | `vulnerabilities[].cve` | (**DEPRECATED - use `vulnerabilities[].id` instead**) A fingerprint string value that represents a concrete occurrence of the vulnerability. It's used to determine whether two vulnerability occurrences are same or different. May not be 100% accurate. **This is NOT a [CVE](https://cve.mitre.org/)**. | @@ -471,8 +470,8 @@ the report JSON unless stated otherwise. Presence of optional fields depends on | `vulnerabilities[].location.end_line` | The last line of the code affected by the vulnerability. Optional. | | `vulnerabilities[].location.class` | If specified, provides the name of the class where the vulnerability is located. Optional. | | `vulnerabilities[].location.method` | If specified, provides the name of the method where the vulnerability is located. Optional. | -| `vulnerabilities[].identifiers` | An ordered array of references that identify a vulnerability on internal or external DBs. | -| `vulnerabilities[].identifiers[].type` | Type of the identifier. Possible values: common identifier types (among `cve`, `cwe`, `osvdb`, and `usn`) or analyzer-dependent ones (e.g., `bandit_test_id` for [Bandit analyzer](https://wiki.openstack.org/wiki/Security/Projects/Bandit)). | +| `vulnerabilities[].identifiers` | An ordered array of references that identify a vulnerability on internal or external databases. | +| `vulnerabilities[].identifiers[].type` | Type of the identifier. Possible values: common identifier types (among `cve`, `cwe`, `osvdb`, and `usn`) or analyzer-dependent ones (like `bandit_test_id` for [Bandit analyzer](https://wiki.openstack.org/wiki/Security/Projects/Bandit)). | | `vulnerabilities[].identifiers[].name` | Name of the identifier for display purposes. | | `vulnerabilities[].identifiers[].value` | Value of the identifier for matching purposes. | | `vulnerabilities[].identifiers[].url` | URL to identifier's documentation. Optional. | @@ -480,8 +479,8 @@ the report JSON unless stated otherwise. Presence of optional fields depends on ## Secret detection GitLab is also able to detect secrets and credentials that have been unintentionally pushed to the -repository (for example, an API key that allows write access to third-party deployment -environments). +repository, such as an API key that allows write access to third-party deployment +environments. This check is performed by a specific analyzer during the `sast` job. It runs regardless of the programming language of your app, and you don't need to change anything to your @@ -492,7 +491,7 @@ GitLab currently includes [Gitleaks](https://github.com/zricethezav/gitleaks) an NOTE: **Note:** The secrets analyzer will ignore "Password in URL" vulnerabilities if the password begins with a dollar sign (`$`) as this likely indicates the password being used is an environment -variable. For example, `https://username:$password@example.com/path/to/repo` will not be +variable. For example, `https://username:$password@example.com/path/to/repo` won't be detected, whereas `https://username:password@example.com/path/to/repo` would be detected. ## Security Dashboard @@ -515,13 +514,13 @@ For more information about the vulnerabilities database update, check the For self-managed GitLab instances in an environment with limited, restricted, or intermittent access to external resources through the internet, some adjustments are required for the SAST job to -successfully run. For more information, see [Offline environments](../offline_deployments/index.md). +run successfully. For more information, see [Offline environments](../offline_deployments/index.md). ### Requirements for offline SAST To use SAST in an offline environment, you need: -- [Disable Docker-In-Docker](#disabling-docker-in-docker-for-sast) +- To [disable Docker-In-Docker](#disabling-docker-in-docker-for-sast). - GitLab Runner with the [`docker` or `kubernetes` executor](#requirements). - Docker Container Registry with locally available copies of SAST [analyzer](https://gitlab.com/gitlab-org/security-products/analyzers) images. @@ -560,7 +559,7 @@ registry.gitlab.com/gitlab-org/security-products/analyzers/tslint:2 The process for importing Docker images into a local offline Docker registry depends on **your network security policy**. Please consult your IT staff to find an accepted and approved process by which external resources can be imported or temporarily accessed. Note that these scanners are [updated periodically](../index.md#maintenance-and-update-of-the-vulnerabilities-database) -with new definitions, so consider if you are able to make periodic updates yourself. +with new definitions, so consider if you're able to make periodic updates yourself. For details on saving and transporting Docker images as a file, see Docker's documentation on [`docker save`](https://docs.docker.com/engine/reference/commandline/save/), [`docker load`](https://docs.docker.com/engine/reference/commandline/load/), @@ -588,6 +587,6 @@ security reports without requiring internet access. ### Error response from daemon: error processing tar file: docker-tar: relocation error This error occurs when the Docker version used to run the SAST job is `19.03.0`. -You are advised to update to Docker `19.03.1` or greater. Older versions are not +Consider updating to Docker `19.03.1` or greater. Older versions are not affected. Read more in [this issue](https://gitlab.com/gitlab-org/gitlab/issues/13830#note_211354992 "Current SAST container fails"). diff --git a/doc/user/project/merge_requests/browser_performance_testing.md b/doc/user/project/merge_requests/browser_performance_testing.md index 3dd87fcc8f5..b76c6397204 100644 --- a/doc/user/project/merge_requests/browser_performance_testing.md +++ b/doc/user/project/merge_requests/browser_performance_testing.md @@ -6,55 +6,50 @@ type: reference, howto > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/3507) in [GitLab Premium](https://about.gitlab.com/pricing/) 10.3. -If your application offers a web interface and you are using +If your application offers a web interface and you're using [GitLab CI/CD](../../../ci/README.md), you can quickly determine the performance impact of pending code changes. ## Overview GitLab uses [Sitespeed.io](https://www.sitespeed.io), a free and open source -tool for measuring the performance of web sites, and has built a simple -[Sitespeed plugin](https://gitlab.com/gitlab-org/gl-performance) -which outputs the results in a file called `performance.json`. This plugin -outputs the performance score for each page that is analyzed. - +tool, for measuring the performance of web sites. GitLab has built a simple +[Sitespeed plugin](https://gitlab.com/gitlab-org/gl-performance) which outputs +the performance score for each page analyzed in a file called `performance.json`. The [Sitespeed.io performance score](https://examples.sitespeed.io/6.0/2017-11-23-23-43-35/help.html) -is a composite value based on best practices, and we will be expanding support -for [additional metrics](https://gitlab.com/gitlab-org/gitlab/issues/4370) -in a future release. +is a composite value based on best practices. -Going a step further, GitLab can show the Performance report right -in the merge request widget area (see below). +GitLab can [show the Performance report](#how-browser-performance-testing-works) +in the merge request widget area. ## Use cases -For instance, consider the following workflow: +Consider the following workflow: 1. A member of the marketing team is attempting to track engagement by adding a new tool. 1. With browser performance metrics, they see how their changes are impacting the usability of the page for end users. -1. The metrics show that after their changes the performance score of the page has gone down. -1. When looking at the detailed report, they see that the new JavaScript library was - included in `<head>` which affects loading page speed. -1. They ask a front end developer to help them, who sets the library to load asynchronously. -1. The frontend developer approves the merge request and authorizes its deployment to production. +1. The metrics show that after their changes, the performance score of the page has gone down. +1. When looking at the detailed report, they see the new JavaScript library was + included in `<head>`, which affects loading page speed. +1. They ask for help from a front end developer, who sets the library to load asynchronously. +1. The frontend developer approves the merge request, and authorizes its deployment to production. -## How it works +## How browser performance testing works -First of all, you need to define a job in your `.gitlab-ci.yml` file that generates the +First, define a job in your `.gitlab-ci.yml` file that generates the [Performance report artifact](../../../ci/pipelines/job_artifacts.md#artifactsreportsperformance-premium). -For more information on how the Performance job should look like, check the -example on [Configuring Browser Performance Testing](#configuring-browser-performance-testing). - GitLab then checks this report, compares key performance metrics for each page -between the source and target branches, and shows the information right on the merge request. +between the source and target branches, and shows the information in the merge request. + +For an example Performance job, see +[Configuring Browser Performance Testing](#configuring-browser-performance-testing). NOTE: **Note:** -If the Performance report doesn't have anything to compare to, no information -will be displayed in the merge request area. That is the case when you add the -Performance job in your `.gitlab-ci.yml` for the very first time. -Consecutive merge requests will have something to compare to, and the Performance -report will be shown properly. +If the Performance report has no data to compare, such as when you add the +Performance job in your `.gitlab-ci.yml` for the very first time, no information +displays in the merge request widget area. Consecutive merge requests will have data for +comparison, and the Performance report will be shown properly.  @@ -64,52 +59,51 @@ This example shows how to run the [sitespeed.io container](https://hub.docker.co on your code by using GitLab CI/CD and [sitespeed.io](https://www.sitespeed.io) using Docker-in-Docker. -First, you need GitLab Runner with -[docker-in-docker build](../../../ci/docker/using_docker_build.md#use-docker-in-docker-workflow-with-docker-executor). +1. First, set up GitLab Runner with a + [docker-in-docker build](../../../ci/docker/using_docker_build.md#use-docker-in-docker-workflow-with-docker-executor). +1. After configuring the Runner, add a new job to `.gitlab-ci.yml` that generates + the expected report. +1. Define the `performance` job according to your version of GitLab: -Once you set up the Runner, add a new job to `.gitlab-ci.yml` that generates the -expected report. + - For GitLab 12.4 and later - [include](../../../ci/yaml/README.md#includetemplate) the + [`Browser-Performance.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Verify/Browser-Performance.gitlab-ci.yml) provided as a part of your GitLab installation. + - For GitLab versions earlier than 12.4 - Copy and use the job as defined in the + [`Browser-Performance.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Verify/Browser-Performance.gitlab-ci.yml). -For GitLab 12.4 and later, to define the `performance` job, you must -[include](../../../ci/yaml/README.md#includetemplate) the -[`Browser-Performance.gitlab-ci.yml` template](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Verify/Browser-Performance.gitlab-ci.yml) -that's provided as a part of your GitLab installation. -For GitLab versions earlier than 12.4, you can copy and use the job as defined -in that template. + CAUTION: **Caution:** + The job definition provided by the template does not support Kubernetes yet. + For a complete example of a more complex setup that works in Kubernetes, see + [`Browser-Performance-Testing.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml). -CAUTION: **Caution:** -The job definition provided by the template does not support Kubernetes yet. For a complete example of a more complex setup -that works in Kubernetes, see [here](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml). +1. Add the following to your `.gitlab-ci.yml` file: -Add the following to your `.gitlab-ci.yml` file: + ```yaml + include: + template: Verify/Browser-Performance.gitlab-ci.yml -```yaml -include: - template: Verify/Browser-Performance.gitlab-ci.yml + performance: + variables: + URL: https://example.com + ``` -performance: - variables: - URL: https://example.com -``` - -CAUTION: **Caution:** -The job definition provided by the template is supported in GitLab 11.5 and later versions. -It also requires GitLab Runner 11.5 or later. For earlier versions, use the -[previous job definitions](#previous-job-definitions). + CAUTION: **Caution:** + The job definition provided by the template is supported in GitLab 11.5 and later versions. + It also requires GitLab Runner 11.5 or later. For earlier versions, use the + [previous job definitions](#previous-job-definitions). -The above example will create a `performance` job in your CI/CD pipeline and will run +The above example creates a `performance` job in your CI/CD pipeline and runs sitespeed.io against the webpage you defined in `URL` to gather key metrics. The [GitLab plugin for sitespeed.io](https://gitlab.com/gitlab-org/gl-performance) -is downloaded in order to save the report as a [Performance report artifact](../../../ci/pipelines/job_artifacts.md#artifactsreportsperformance-premium) -that you can later download and analyze. Due to implementation limitations we always +is downloaded to save the report as a [Performance report artifact](../../../ci/pipelines/job_artifacts.md#artifactsreportsperformance-premium) +that you can later download and analyze. Due to implementation limitations, we always take the latest Performance artifact available. -The full HTML sitespeed.io report will also be saved as an artifact, and if you have -[GitLab Pages](../pages/index.md) enabled, it can be viewed directly in your browser. +The full HTML sitespeed.io report is saved as an artifact, and if +[GitLab Pages](../pages/index.md) is enabled, it can be viewed directly in your browser. -It is also possible to customize options by setting the `SITESPEED_OPTIONS` variable. -For example, this is how to override the number of runs sitespeed.io -will make on the given URL: +You can also customize options by setting the `SITESPEED_OPTIONS` variable. +For example, you can override the number of runs sitespeed.io +makes on the given URL: ```yaml include: @@ -122,7 +116,8 @@ performance: ``` For further customization options for sitespeed.io, including the ability to provide a -list of URLs to test, please see the [Sitespeed.io Configuration](https://www.sitespeed.io/documentation/sitespeed.io/configuration/) +list of URLs to test, please see the +[Sitespeed.io Configuration](https://www.sitespeed.io/documentation/sitespeed.io/configuration/) documentation. TIP: **Tip:** @@ -130,19 +125,18 @@ Key metrics are automatically extracted and shown in the merge request widget. ### Performance testing on Review Apps -The above CI YML is great for testing against static environments, and it can -be extended for dynamic environments. There are a few extra steps to take to -set this up: +The above CI YAML configuration is great for testing against static environments, and it can +be extended for dynamic environments, but a few extra steps are required: 1. The `performance` job should run after the dynamic environment has started. 1. In the `review` job, persist the hostname and upload it as an artifact so - it's available to the `performance` job (the same can be done for static - environments like staging and production to unify the code path). Saving it - as an artifact is as simple as `echo $CI_ENVIRONMENT_URL > environment_url.txt` + it's available to the `performance` job. The same can be done for static + environments like staging and production to unify the code path. You can save it + as an artifact with `echo $CI_ENVIRONMENT_URL > environment_url.txt` in your job's `script`. 1. In the `performance` job, read the previous artifact into an environment - variable, in this case `$URL` because this is what our sitespeed.io command - uses for the URL parameter. Because Review App URLs are dynamic, we define + variable. In this case, use `$URL` because the sitespeed.io command + uses it for the URL parameter. Because Review App URLs are dynamic, define the `URL` variable through `before_script` instead of `variables`. 1. You can now run the sitespeed.io container against the desired hostname and paths. @@ -183,11 +177,11 @@ performance: ### Previous job definitions CAUTION: **Caution:** -Before GitLab 11.5, Performance job and artifact had to be named specifically +Before GitLab 11.5, the Performance job and artifact had to be named specifically to automatically extract report data and show it in the merge request widget. -While these old job definitions are still maintained they have been deprecated +While these old job definitions are still maintained, they have been deprecated and may be removed in next major release, GitLab 12.0. -You are advised to update your current `.gitlab-ci.yml` configuration to reflect that change. +GitLab recommends you update your current `.gitlab-ci.yml` configuration to reflect that change. For GitLab 11.4 and earlier, the job should look like: diff --git a/lib/gitlab/ci/templates/Jobs/Test.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Test.gitlab-ci.yml index a0ddd273552..c6b0de44207 100644 --- a/lib/gitlab/ci/templates/Jobs/Test.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Test.gitlab-ci.yml @@ -5,6 +5,7 @@ test: POSTGRES_DB: test stage: test image: gliderlabs/herokuish:latest + needs: [] script: - | if [ -z ${KUBERNETES_PORT+x} ]; then diff --git a/spec/controllers/concerns/issuable_actions_spec.rb b/spec/controllers/concerns/issuable_actions_spec.rb index 7b0b4497f3f..2ab46992b99 100644 --- a/spec/controllers/concerns/issuable_actions_spec.rb +++ b/spec/controllers/concerns/issuable_actions_spec.rb @@ -14,7 +14,7 @@ describe IssuableActions do klass = Class.new do attr_reader :current_user, :project, :issuable - def self.before_action(action, params = nil) + def self.before_action(action = nil, params = nil) end include IssuableActions diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index 5e2b5921e06..3ee5840e1b9 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -81,6 +81,26 @@ describe 'Filter issues', :js do expect_filtered_search_input(search_term) end + context 'with the NOT queries feature flag disabled' do + before do + stub_feature_flags(not_issuable_queries: false) + visit project_issues_path(project) + end + + it 'does not have the != option' do + input_filtered_search("label:", submit: false) + + wait_for_requests + within('#js-dropdown-operator') do + tokens = all(:css, 'li.filter-dropdown-item') + expect(tokens.count).to eq(1) + button = tokens[0].find('button') + expect(button).to have_content('=') + expect(button).not_to have_content('!=') + end + end + end + describe 'filter issues by author' do context 'only author' do it 'filters issues by searched author' do @@ -153,16 +173,16 @@ describe 'Filter issues', :js do expect_filtered_search_input_empty end - it 'filters issues by no label' do - input_filtered_search('label:=none') + it 'filters issues by any label' do + input_filtered_search('label:=any') - expect_tokens([label_token('None', false)]) + expect_tokens([label_token('Any', false)]) expect_issues_list_count(4) expect_filtered_search_input_empty end it 'filters issues by no label' do - input_filtered_search('label:!=none') + input_filtered_search('label:=none') expect_tokens([label_token('None', false)]) expect_issues_list_count(4) @@ -351,14 +371,6 @@ describe 'Filter issues', :js do expect_filtered_search_input_empty end - it 'filters issues by negation of no milestone' do - input_filtered_search("milestone:!=none ") - - expect_tokens([milestone_token('None', false, '!=')]) - expect_issues_list_count(5) - expect_filtered_search_input_empty - end - it 'filters issues by upcoming milestones' do create(:milestone, project: project, due_date: 1.month.from_now) do |future_milestone| create(:issue, project: project, milestone: future_milestone, author: user) @@ -376,10 +388,14 @@ describe 'Filter issues', :js do create(:issue, project: project, milestone: future_milestone, author: user) end + create(:milestone, project: project, due_date: 3.days.ago) do |past_milestone| + create(:issue, project: project, milestone: past_milestone, author: user) + end + input_filtered_search("milestone:!=upcoming") expect_tokens([milestone_token('Upcoming', false, '!=')]) - expect_issues_list_count(8) + expect_issues_list_count(1) expect_filtered_search_input_empty end @@ -392,10 +408,13 @@ describe 'Filter issues', :js do end it 'filters issues by negation of started milestones' do + milestone2 = create(:milestone, title: "9", project: project, start_date: 2.weeks.from_now) + create(:issue, project: project, author: user, title: "something else", milestone: milestone2) + input_filtered_search("milestone:!=started") expect_tokens([milestone_token('Started', false, '!=')]) - expect_issues_list_count(3) + expect_issues_list_count(1) expect_filtered_search_input_empty end diff --git a/spec/features/issues/filtered_search/visual_tokens_spec.rb b/spec/features/issues/filtered_search/visual_tokens_spec.rb index 2b164e2fd97..d34253b3c5e 100644 --- a/spec/features/issues/filtered_search/visual_tokens_spec.rb +++ b/spec/features/issues/filtered_search/visual_tokens_spec.rb @@ -175,4 +175,20 @@ describe 'Visual tokens', :js do expect(token.find('.name').text).to eq('Label') expect(token.find('.operator').text).to eq('=') end + + describe 'Any/None option' do + it 'hidden when NOT operator is selected' do + input_filtered_search('milestone:!=', extra_space: false, submit: false) + + expect(page).not_to have_selector("#js-dropdown-milestone", text: 'Any') + expect(page).not_to have_selector("#js-dropdown-milestone", text: 'None') + end + + it 'shown when EQUAL operator is selected' do + input_filtered_search('milestone:=', extra_space: false, submit: false) + + expect(page).to have_selector("#js-dropdown-milestone", text: 'Any') + expect(page).to have_selector("#js-dropdown-milestone", text: 'None') + end + end end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index baf40861a6e..7493fafb5cc 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -132,26 +132,6 @@ describe IssuesFinder do end end - context 'filtering by NOT group_id' do - let(:params) { { not: { group_id: group.id } } } - - context 'when include_subgroup param not set' do - it 'returns all other group issues' do - expect(issues).to contain_exactly(issue2, issue3, issue4) - end - end - - context 'when include_subgroup param is true', :nested_groups do - before do - params[:include_subgroups] = true - end - - it 'returns all other group and subgroup issues' do - expect(issues).to contain_exactly(issue2, issue3) - end - end - end - context 'filtering by author ID' do let(:params) { { author_id: user2.id } } @@ -292,12 +272,12 @@ describe IssuesFinder do context 'using NOT' do let(:params) { { not: { milestone_title: Milestone::Upcoming.name } } } - it 'returns issues not in upcoming milestones for each project or group' do - target_issues = @created_issues.reject do |issue| - issue.milestone&.due_date && issue.milestone.due_date > Date.current - end + @created_issues.select { |issue| issue.milestone&.title == '8.9' } + it 'returns issues not in upcoming milestones for each project or group, but must have a due date' do + target_issues = @created_issues.select do |issue| + issue.milestone&.due_date && issue.milestone.due_date <= Date.current + end - expect(issues).to contain_exactly(issue1, issue2, issue3, issue4, *target_issues) + expect(issues).to contain_exactly(*target_issues) end end end @@ -343,9 +323,9 @@ describe IssuesFinder do let(:params) { { not: { milestone_title: Milestone::Started.name } } } it 'returns issues not in the started milestones for each project' do - target_issues = Issue.where.not(milestone: Milestone.started) + target_issues = Issue.where(milestone: Milestone.not_started) - expect(issues).to contain_exactly(issue2, issue3, issue4, *target_issues) + expect(issues).to contain_exactly(*target_issues) end end end @@ -452,14 +432,6 @@ describe IssuesFinder do it 'returns issues with title and description match for search term' do expect(issues).to contain_exactly(issue1, issue2) end - - context 'using NOT' do - let(:params) { { not: { search: 'git' } } } - - it 'returns issues with no title and description match for search term' do - expect(issues).to contain_exactly(issue3, issue4) - end - end end context 'filtering by issue term in title' do @@ -468,14 +440,6 @@ describe IssuesFinder do it 'returns issues with title match for search term' do expect(issues).to contain_exactly(issue1) end - - context 'using NOT' do - let(:params) { { not: { search: 'git', in: 'title' } } } - - it 'returns issues with no title match for search term' do - expect(issues).to contain_exactly(issue2, issue3, issue4) - end - end end context 'filtering by issues iids' do diff --git a/spec/graphql/mutations/issues/set_confidential_spec.rb b/spec/graphql/mutations/issues/set_confidential_spec.rb index 6031953c869..c90ce2658d6 100644 --- a/spec/graphql/mutations/issues/set_confidential_spec.rb +++ b/spec/graphql/mutations/issues/set_confidential_spec.rb @@ -8,6 +8,8 @@ describe Mutations::Issues::SetConfidential do subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_issue) } + describe '#resolve' do let(:confidential) { true } let(:mutated_issue) { subject[:issue] } diff --git a/spec/graphql/mutations/issues/set_due_date_spec.rb b/spec/graphql/mutations/issues/set_due_date_spec.rb index 73ba11fc551..84df6fce7c7 100644 --- a/spec/graphql/mutations/issues/set_due_date_spec.rb +++ b/spec/graphql/mutations/issues/set_due_date_spec.rb @@ -8,6 +8,8 @@ describe Mutations::Issues::SetDueDate do subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_issue) } + describe '#resolve' do let(:due_date) { 2.days.since } let(:mutated_issue) { subject[:issue] } diff --git a/spec/graphql/mutations/merge_requests/set_labels_spec.rb b/spec/graphql/mutations/merge_requests/set_labels_spec.rb index f58f35eb6f3..0fd2c20a5c8 100644 --- a/spec/graphql/mutations/merge_requests/set_labels_spec.rb +++ b/spec/graphql/mutations/merge_requests/set_labels_spec.rb @@ -8,6 +8,8 @@ describe Mutations::MergeRequests::SetLabels do subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) } + describe '#resolve' do let(:label) { create(:label, project: merge_request.project) } let(:label2) { create(:label, project: merge_request.project) } diff --git a/spec/graphql/mutations/merge_requests/set_locked_spec.rb b/spec/graphql/mutations/merge_requests/set_locked_spec.rb index 12ae1314f22..d5219c781fd 100644 --- a/spec/graphql/mutations/merge_requests/set_locked_spec.rb +++ b/spec/graphql/mutations/merge_requests/set_locked_spec.rb @@ -8,6 +8,8 @@ describe Mutations::MergeRequests::SetLocked do subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) } + describe '#resolve' do let(:locked) { true } let(:mutated_merge_request) { subject[:merge_request] } diff --git a/spec/graphql/mutations/merge_requests/set_milestone_spec.rb b/spec/graphql/mutations/merge_requests/set_milestone_spec.rb index ad7f2df0842..d77ec4de4d0 100644 --- a/spec/graphql/mutations/merge_requests/set_milestone_spec.rb +++ b/spec/graphql/mutations/merge_requests/set_milestone_spec.rb @@ -8,6 +8,8 @@ describe Mutations::MergeRequests::SetMilestone do subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) } + describe '#resolve' do let(:milestone) { create(:milestone, project: merge_request.project) } let(:mutated_merge_request) { subject[:merge_request] } diff --git a/spec/graphql/mutations/merge_requests/set_subscription_spec.rb b/spec/graphql/mutations/merge_requests/set_subscription_spec.rb index a28bab363f3..cf569a74aa9 100644 --- a/spec/graphql/mutations/merge_requests/set_subscription_spec.rb +++ b/spec/graphql/mutations/merge_requests/set_subscription_spec.rb @@ -9,6 +9,8 @@ describe Mutations::MergeRequests::SetSubscription do subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) } + describe '#resolve' do let(:subscribe) { true } let(:mutated_merge_request) { subject[:merge_request] } diff --git a/spec/graphql/mutations/merge_requests/set_wip_spec.rb b/spec/graphql/mutations/merge_requests/set_wip_spec.rb index 9f0adcf117a..7255d0fe7d7 100644 --- a/spec/graphql/mutations/merge_requests/set_wip_spec.rb +++ b/spec/graphql/mutations/merge_requests/set_wip_spec.rb @@ -8,6 +8,8 @@ describe Mutations::MergeRequests::SetWip do subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) } + describe '#resolve' do let(:wip) { true } let(:mutated_merge_request) { subject[:merge_request] } diff --git a/spec/graphql/mutations/todos/mark_all_done_spec.rb b/spec/graphql/mutations/todos/mark_all_done_spec.rb index 98b22a3e761..4af00307969 100644 --- a/spec/graphql/mutations/todos/mark_all_done_spec.rb +++ b/spec/graphql/mutations/todos/mark_all_done_spec.rb @@ -17,6 +17,8 @@ describe Mutations::Todos::MarkAllDone do let_it_be(:user3) { create(:user) } + specify { expect(described_class).to require_graphql_authorizations(:update_user) } + describe '#resolve' do it 'marks all pending todos as done' do updated_todo_ids = mutation_for(current_user).resolve.dig(:updated_ids) diff --git a/spec/graphql/mutations/todos/mark_done_spec.rb b/spec/graphql/mutations/todos/mark_done_spec.rb index 059ef3c8eee..44065f83f74 100644 --- a/spec/graphql/mutations/todos/mark_done_spec.rb +++ b/spec/graphql/mutations/todos/mark_done_spec.rb @@ -16,6 +16,8 @@ describe Mutations::Todos::MarkDone do let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_todo) } + describe '#resolve' do it 'marks a single todo as done' do result = mark_done_mutation(todo1) diff --git a/spec/graphql/mutations/todos/restore_spec.rb b/spec/graphql/mutations/todos/restore_spec.rb index 1637acc2fb5..949ab6a164b 100644 --- a/spec/graphql/mutations/todos/restore_spec.rb +++ b/spec/graphql/mutations/todos/restore_spec.rb @@ -14,6 +14,8 @@ describe Mutations::Todos::Restore do let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) } + specify { expect(described_class).to require_graphql_authorizations(:update_todo) } + describe '#resolve' do it 'restores a single todo' do result = restore_mutation(todo1) |