diff options
33 files changed, 302 insertions, 193 deletions
diff --git a/app/assets/javascripts/issues/create_merge_request_dropdown.js b/app/assets/javascripts/issues/create_merge_request_dropdown.js index 27ba94c8381..caf82e482ea 100644 --- a/app/assets/javascripts/issues/create_merge_request_dropdown.js +++ b/app/assets/javascripts/issues/create_merge_request_dropdown.js @@ -11,6 +11,10 @@ import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { __, sprintf } from '~/locale'; import { mergeUrlParams } from '~/lib/utils/url_utility'; +import { + findInvalidBranchNameCharacters, + humanizeBranchValidationErrors, +} from '~/lib/utils/text_utility'; import api from '~/api'; // Todo: Remove this when fixing issue in input_setter plugin @@ -19,6 +23,12 @@ const InputSetter = { ...ISetter }; const CREATE_MERGE_REQUEST = 'create-mr'; const CREATE_BRANCH = 'create-branch'; +const VALIDATION_TYPE_BRANCH_UNAVAILABLE = 'branch_unavailable'; +const VALIDATION_TYPE_INVALID_CHARS = 'invalid_chars'; + +const INPUT_TARGET_BRANCH = 'branch'; +const INPUT_TARGET_REF = 'ref'; + function createEndpoint(projectPath, endpoint) { if (canCreateConfidentialMergeRequest()) { return endpoint.replace( @@ -30,6 +40,23 @@ function createEndpoint(projectPath, endpoint) { return endpoint; } +function getValidationError(target, inputValue, validationType) { + const invalidChars = findInvalidBranchNameCharacters(inputValue.value); + let text; + + if (invalidChars && validationType === VALIDATION_TYPE_INVALID_CHARS) { + text = humanizeBranchValidationErrors(invalidChars); + } + + if (validationType === VALIDATION_TYPE_BRANCH_UNAVAILABLE) { + text = + target === INPUT_TARGET_BRANCH + ? __('Branch is already taken') + : __('Source is not available'); + } + + return text; +} export default class CreateMergeRequestDropdown { constructor(wrapperEl) { this.wrapperEl = wrapperEl; @@ -124,18 +151,19 @@ export default class CreateMergeRequestDropdown { .then(({ data }) => { this.setUnavailableButtonState(false); - if (data.can_create_branch) { - this.available(); - this.enable(); - this.updateBranchName(data.suggested_branch_name); - - if (!this.droplabInitialized) { - this.droplabInitialized = true; - this.initDroplab(); - this.bindEvents(); - } - } else { + if (!data.can_create_branch) { this.hide(); + return; + } + + this.available(); + this.enable(); + this.updateBranchName(data.suggested_branch_name); + + if (!this.droplabInitialized) { + this.droplabInitialized = true; + this.initDroplab(); + this.bindEvents(); } }) .catch(() => { @@ -274,7 +302,7 @@ export default class CreateMergeRequestDropdown { const tags = data[Object.keys(data)[1]]; let result; - if (target === 'branch') { + if (target === INPUT_TARGET_BRANCH) { result = CreateMergeRequestDropdown.findByValue(branches, ref); } else { result = @@ -354,10 +382,10 @@ export default class CreateMergeRequestDropdown { } if (event.target === this.branchInput) { - target = 'branch'; + target = INPUT_TARGET_BRANCH; ({ value } = this.branchInput); } else if (event.target === this.refInput) { - target = 'ref'; + target = INPUT_TARGET_REF; if (event.target === document.activeElement) { value = event.target.value.slice(0, event.target.selectionStart) + @@ -382,7 +410,7 @@ export default class CreateMergeRequestDropdown { this.createBranchPath = this.wrapperEl.dataset.createBranchPath; this.createMrPath = this.wrapperEl.dataset.createMrPath; - if (target === 'branch') { + if (target === INPUT_TARGET_BRANCH) { this.branchIsValid = true; } else { this.refIsValid = true; @@ -473,7 +501,7 @@ export default class CreateMergeRequestDropdown { showAvailableMessage(target) { const { input, message } = this.getTargetData(target); - const text = target === 'branch' ? __('Branch name') : __('Source'); + const text = target === INPUT_TARGET_BRANCH ? __('Branch name') : __('Source'); this.removeMessage(target); input.classList.add('gl-field-success-outline'); @@ -484,7 +512,7 @@ export default class CreateMergeRequestDropdown { showCheckingMessage(target) { const { message } = this.getTargetData(target); - const text = target === 'branch' ? __('branch name') : __('source'); + const text = target === INPUT_TARGET_BRANCH ? __('branch name') : __('source'); this.removeMessage(target); message.classList.add('gl-text-gray-600'); @@ -492,10 +520,9 @@ export default class CreateMergeRequestDropdown { message.style.display = 'inline-block'; } - showNotAvailableMessage(target) { + showNotAvailableMessage(target, validationType = VALIDATION_TYPE_BRANCH_UNAVAILABLE) { const { input, message } = this.getTargetData(target); - const text = - target === 'branch' ? __('Branch is already taken') : __('Source is not available'); + const text = getValidationError(target, input, validationType); this.removeMessage(target); input.classList.add('gl-field-error-outline'); @@ -511,35 +538,35 @@ export default class CreateMergeRequestDropdown { updateBranchName(suggestedBranchName) { this.branchInput.value = suggestedBranchName; - this.updateCreatePaths('branch', suggestedBranchName); + this.updateInputState(INPUT_TARGET_BRANCH, suggestedBranchName, ''); + this.updateCreatePaths(INPUT_TARGET_BRANCH, suggestedBranchName); } updateInputState(target, ref, result) { // target - 'branch' or 'ref' - which the input field we are searching a ref for. // ref - string - what a user typed. // result - string - what has been found on backend. + if (target === INPUT_TARGET_BRANCH) this.updateTargetBranchInput(ref, result); + if (target === INPUT_TARGET_REF) this.updateRefInput(ref, result); + + if (this.inputsAreValid()) { + this.enable(); + } else { + this.disableCreateAction(); + } + } - // If a found branch equals exact the same text a user typed, - // that means a new branch cannot be created as it already exists. + updateRefInput(ref, result) { + this.refInput.dataset.value = ref; if (ref === result) { - if (target === 'branch') { - this.branchIsValid = false; - this.showNotAvailableMessage('branch'); - } else { - this.refIsValid = true; - this.refInput.dataset.value = ref; - this.showAvailableMessage('ref'); - this.updateCreatePaths(target, ref); - } - } else if (target === 'branch') { - this.branchIsValid = true; - this.showAvailableMessage('branch'); - this.updateCreatePaths(target, ref); + this.refIsValid = true; + this.showAvailableMessage(INPUT_TARGET_REF); + this.updateCreatePaths(INPUT_TARGET_REF, ref); } else { this.refIsValid = false; this.refInput.dataset.value = ref; this.disableCreateAction(); - this.showNotAvailableMessage('ref'); + this.showNotAvailableMessage(INPUT_TARGET_REF); // Show ref hint. if (result) { @@ -547,11 +574,24 @@ export default class CreateMergeRequestDropdown { this.refInput.setSelectionRange(ref.length, result.length); } } + } - if (this.inputsAreValid()) { - this.enable(); + updateTargetBranchInput(ref, result) { + const branchNameErrors = findInvalidBranchNameCharacters(ref); + const isInvalidString = branchNameErrors.length; + if (ref !== result && !isInvalidString) { + this.branchIsValid = true; + // If a found branch equals exact the same text a user typed, + // Or user typed input contains invalid chars, + // that means a new branch cannot be created as it already exists. + this.showAvailableMessage(INPUT_TARGET_BRANCH, VALIDATION_TYPE_BRANCH_UNAVAILABLE); + this.updateCreatePaths(INPUT_TARGET_BRANCH, ref); + } else if (isInvalidString) { + this.branchIsValid = false; + this.showNotAvailableMessage(INPUT_TARGET_BRANCH, VALIDATION_TYPE_INVALID_CHARS); } else { - this.disableCreateAction(); + this.branchIsValid = false; + this.showNotAvailableMessage(INPUT_TARGET_BRANCH); } } diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index 367180714df..1bed38b7dbe 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -1,4 +1,5 @@ import { isString, memoize } from 'lodash'; +import { sprintf, __ } from '~/locale'; import { base64ToBuffer, bufferToBase64 } from '~/authentication/webauthn/util'; import { TRUNCATE_WIDTH_DEFAULT_WIDTH, @@ -525,3 +526,45 @@ export function base64DecodeUnicode(str) { const decoder = new TextDecoder('utf8'); return decoder.decode(base64ToBuffer(str)); } + +// returns an array of errors (if there are any) +const INVALID_BRANCH_NAME_CHARS = [' ', '~', '^', ':', '?', '*', '[', '..', '@{', '\\', '//']; + +/** + * Returns an array of invalid characters found in a branch name + * + * @param {String} name branch name to check + * @return {Array} Array of invalid characters found + */ +export const findInvalidBranchNameCharacters = (name) => { + const invalidChars = []; + + INVALID_BRANCH_NAME_CHARS.forEach((pattern) => { + if (name.indexOf(pattern) > -1) { + invalidChars.push(pattern); + } + }); + + return invalidChars; +}; + +/** + * Returns a string describing validation errors for a branch name + * + * @param {Array} invalidChars Array of invalid characters that were found + * @return {String} Error message describing on the invalid characters found + */ +export const humanizeBranchValidationErrors = (invalidChars = []) => { + const chars = invalidChars.filter((c) => INVALID_BRANCH_NAME_CHARS.includes(c)); + + if (chars.length && !chars.includes(' ')) { + return sprintf(__("Can't contain %{chars}"), { chars: chars.join(', ') }); + } else if (chars.includes(' ') && chars.length <= 1) { + return __("Can't contain spaces"); + } else if (chars.includes(' ') && chars.length > 1) { + return sprintf(__("Can't contain spaces, %{chars}"), { + chars: chars.filter((c) => c !== ' ').join(', '), + }); + } + return ''; +}; diff --git a/app/assets/javascripts/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue b/app/assets/javascripts/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue index 09414e679bb..bda88a48e48 100644 --- a/app/assets/javascripts/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue +++ b/app/assets/javascripts/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue @@ -21,6 +21,11 @@ export default { required: false, default: 'top', }, + boundary: { + type: String, + required: false, + default: '', + }, truncateTarget: { type: [String, Function], required: false, @@ -44,6 +49,8 @@ export default { title: this.title, placement: this.placement, disabled: this.tooltipDisabled, + // Only set the tooltip boundary if it's truthy + ...(this.boundary && { boundary: this.boundary }), }; }, }, diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 4ba0eead599..c1c1691e424 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -640,10 +640,6 @@ module Issuable false end - def ensure_metrics - self.metrics || create_metrics - end - ## # Overridden in MergeRequest # diff --git a/app/models/integrations/mattermost_slash_commands.rb b/app/models/integrations/mattermost_slash_commands.rb index 30a8ba973c1..62fe4820e55 100644 --- a/app/models/integrations/mattermost_slash_commands.rb +++ b/app/models/integrations/mattermost_slash_commands.rb @@ -37,10 +37,6 @@ module Integrations [[], e.message] end - def chat_responder - ::Gitlab::Chat::Responder::Mattermost - end - private def command(params) diff --git a/app/models/integrations/slack_slash_commands.rb b/app/models/integrations/slack_slash_commands.rb index 72e3c4a8cbc..01a87e8e25e 100644 --- a/app/models/integrations/slack_slash_commands.rb +++ b/app/models/integrations/slack_slash_commands.rb @@ -23,10 +23,6 @@ module Integrations end end - def chat_responder - ::Gitlab::Chat::Responder::Slack - end - private def format(text) diff --git a/app/models/issue.rb b/app/models/issue.rb index adebb91f905..7fdededb8aa 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -215,7 +215,7 @@ class Issue < ApplicationRecord before_validation :ensure_namespace_id, :ensure_work_item_type - after_save :ensure_metrics, unless: :importing? + after_save :ensure_metrics!, unless: :importing? after_commit :expire_etag_cache, unless: :importing? after_create_commit :record_create_action, unless: :importing? @@ -723,8 +723,7 @@ class Issue < ApplicationRecord confidential_changed?(from: true, to: false) end - override :ensure_metrics - def ensure_metrics + def ensure_metrics! Issue::Metrics.record!(self) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 700ee6a346e..e59393f7533 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -141,7 +141,7 @@ class MergeRequest < ApplicationRecord after_update :clear_memoized_shas after_update :reload_diff_if_branch_changed after_save :keep_around_commit, unless: :importing? - after_commit :ensure_metrics, on: [:create, :update], unless: :importing? + after_commit :ensure_metrics!, on: [:create, :update], unless: :importing? after_commit :expire_etag_cache, unless: :importing? # When this attribute is true some MR validation is ignored @@ -1948,8 +1948,7 @@ class MergeRequest < ApplicationRecord super.merge(label_url_method: :project_merge_requests_url) end - override :ensure_metrics - def ensure_metrics + def ensure_metrics! MergeRequest::Metrics.record!(self) end diff --git a/app/models/users/banned_user.rb b/app/models/users/banned_user.rb index 615668e2b55..466fc71f83a 100644 --- a/app/models/users/banned_user.rb +++ b/app/models/users/banned_user.rb @@ -10,3 +10,5 @@ module Users validates :user_id, uniqueness: { message: N_("banned user already exists") } end end + +Users::BannedUser.prepend_mod_with('Users::BannedUser') diff --git a/config/feature_flags/development/use_response_url_for_chat_responder.yml b/config/feature_flags/development/use_response_url_for_chat_responder.yml deleted file mode 100644 index 84ac2a27fab..00000000000 --- a/config/feature_flags/development/use_response_url_for_chat_responder.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: use_response_url_for_chat_responder -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110625 -rollout_issue_url: -milestone: '15.9' -type: development -group: group::integrations -default_enabled: false diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index fa890531861..223cc7ba867 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -18,7 +18,8 @@ require 'active_support/testing/time_helpers' # # VSA_SEED_PROJECT_ID=10 FILTER=cycle_analytics SEED_VSA=1 bundle exec rake db:seed_fu -class Gitlab::Seeder::CycleAnalytics +# rubocop:disable Rails/Output +class Gitlab::Seeder::CycleAnalytics # rubocop:disable Style/ClassAndModuleChildren include ActiveSupport::Testing::TimeHelpers attr_reader :project, :issues, :merge_requests, :developers @@ -26,18 +27,20 @@ class Gitlab::Seeder::CycleAnalytics FLAG = 'SEED_VSA' PERF_TEST = 'VSA_PERF_TEST' - ISSUE_STAGE_MAX_DURATION_IN_HOURS = 72 - PLAN_STAGE_MAX_DURATION_IN_HOURS = 48 - CODE_STAGE_MAX_DURATION_IN_HOURS = 72 - TEST_STAGE_MAX_DURATION_IN_HOURS = 5 - REVIEW_STAGE_MAX_DURATION_IN_HOURS = 72 - DEPLOYMENT_MAX_DURATION_IN_HOURS = 48 + MAX_DURATIONS = { # in hours + issue: 72, + plan: 48, + code: 72, + test: 5, + review: 72, + deployment: 48 + }.freeze def self.seeder_based_on_env(project) if ENV[FLAG] - self.new(project: project) + new(project: project) elsif ENV[PERF_TEST] - self.new(project: project, perf: true) + new(project: project, perf: true) end end @@ -54,8 +57,10 @@ class Gitlab::Seeder::CycleAnalytics puts puts 'WARNING' puts '=======' - puts "Seeding #{self.class} is not possible because the given project (#{project.full_path}) doesn't have a repository." - puts 'Try specifying a project with working repository or omit the VSA_SEED_PROJECT_ID parameter so the seed script will automatically create one.' + puts "Seeding #{self.class} is not possible because the given project " \ + "(#{project.full_path}) doesn't have a repository." + puts 'Try specifying a project with working repository or omit the VSA_SEED_PROJECT_ID parameter ' \ + 'so the seed script will automatically create one.' puts return @@ -79,7 +84,7 @@ class Gitlab::Seeder::CycleAnalytics def seed_issue_stage! issues.each do |issue| - time = within_end_time(issue.created_at + rand(ISSUE_STAGE_MAX_DURATION_IN_HOURS).hours) + time = within_end_time(issue.created_at + rand(MAX_DURATIONS[:issue]).hours) if issue.id.even? issue.metrics.update!(first_associated_with_milestone_at: time) @@ -93,7 +98,7 @@ class Gitlab::Seeder::CycleAnalytics issues.each do |issue| plan_stage_start = issue.metrics.first_associated_with_milestone_at || issue.metrics.first_added_to_board_at - first_mentioned_in_commit_at = within_end_time(plan_stage_start + rand(PLAN_STAGE_MAX_DURATION_IN_HOURS).hours) + first_mentioned_in_commit_at = within_end_time(plan_stage_start + rand(MAX_DURATIONS[:plan]).hours) issue.metrics.update!(first_mentioned_in_commit_at: first_mentioned_in_commit_at) end end @@ -107,7 +112,7 @@ class Gitlab::Seeder::CycleAnalytics source_branch: "#{issue.iid}-feature-branch", target_branch: 'master', author: developers.sample, - created_at: within_end_time(issue.metrics.first_mentioned_in_commit_at + rand(CODE_STAGE_MAX_DURATION_IN_HOURS).hours) + created_at: within_end_time(issue.metrics.first_mentioned_in_commit_at + rand(MAX_DURATIONS[:code]).hours) ) @merge_requests << merge_request @@ -118,16 +123,17 @@ class Gitlab::Seeder::CycleAnalytics def seed_test_stage! merge_requests.each do |merge_request| - pipeline = FactoryBot.create(:ci_pipeline, :success, project: project, partition_id: Ci::Pipeline.current_partition_value) + pipeline = FactoryBot.create(:ci_pipeline, :success, project: project, + partition_id: Ci::Pipeline.current_partition_value) build = FactoryBot.create(:ci_build, pipeline: pipeline, project: project, user: developers.sample) # Required because seeds run in a transaction and these are now # created in an `after_commit` hook. - merge_request.ensure_metrics + merge_request.ensure_metrics! merge_request.metrics.update!( latest_build_started_at: merge_request.created_at, - latest_build_finished_at: within_end_time(merge_request.created_at + TEST_STAGE_MAX_DURATION_IN_HOURS.hours), + latest_build_finished_at: within_end_time(merge_request.created_at + MAX_DURATIONS[:test].hours), pipeline_id: build.commit_id ) end @@ -135,13 +141,18 @@ class Gitlab::Seeder::CycleAnalytics def seed_review_stage! merge_requests.each do |merge_request| - merge_request.metrics.update!(merged_at: within_end_time(merge_request.created_at + REVIEW_STAGE_MAX_DURATION_IN_HOURS.hours)) + merge_request.metrics.update!( + merged_at: within_end_time(merge_request.created_at + MAX_DURATIONS[:review].hours) + ) end end def seed_staging_stage! merge_requests.each do |merge_request| - merge_request.metrics.update!(first_deployed_to_production_at: within_end_time(merge_request.metrics.merged_at + DEPLOYMENT_MAX_DURATION_IN_HOURS.hours)) + first_deployed_to_production_at = merge_request.metrics.merged_at + MAX_DURATIONS[:deployment].hours + merge_request.metrics.update!( + first_deployed_to_production_at: within_end_time(first_deployed_to_production_at) + ) end end @@ -224,3 +235,4 @@ Gitlab::Seeder.quiet do puts "Skipped. Use the `#{Gitlab::Seeder::CycleAnalytics::FLAG}` environment variable to enable." end end +# rubocop:enable Rails/Output diff --git a/doc/ci/pipelines/merge_request_pipelines.md b/doc/ci/pipelines/merge_request_pipelines.md index 7e18c11f234..881e94a6559 100644 --- a/doc/ci/pipelines/merge_request_pipelines.md +++ b/doc/ci/pipelines/merge_request_pipelines.md @@ -140,14 +140,7 @@ the parent project. Additionally, if you do not trust the fork project's runner, running the pipeline in the parent project uses the parent project's trusted runners. WARNING: -Fork merge requests can contain malicious code that tries to steal secrets in the -parent project when the pipeline runs, even before merge. As a reviewer, carefully -check the changes in the merge request before triggering the pipeline. If you trigger -the pipeline by selecting **Run pipeline** or applying a suggestion, GitLab shows -a warning that you must accept before the pipeline runs. If you trigger the pipeline -by using any other method, including the API, [`/rebase` quick action](../../user/project/quick_actions.md#issues-merge-requests-and-epics), -or [**Rebase** option](../../user/project/merge_requests/methods/index.md#rebasing-in-semi-linear-merge-methods), -**no warning displays**. +Fork merge requests can contain malicious code that tries to steal secrets in the parent project when the pipeline runs, even before merge. As a reviewer, carefully check the changes in the merge request before triggering the pipeline. Unless you trigger the pipeline through the API or the [`/rebase` quick action](../../user/project/quick_actions.md#issues-merge-requests-and-epics), GitLab shows a warning that you must accept before the pipeline runs. Otherwise, **no warning displays**. Prerequisites: diff --git a/doc/user/search/advanced_search.md b/doc/user/search/advanced_search.md index 4f595af7ba9..d1744e3bb4e 100644 --- a/doc/user/search/advanced_search.md +++ b/doc/user/search/advanced_search.md @@ -59,7 +59,7 @@ Advanced Search uses [Elasticsearch syntax](https://www.elastic.co/guide/en/elas FLAG: On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `user_search_simple_query_string`. -On GitLab.com, this feature is not available. +On GitLab.com, this feature is available. In user search, a [fuzzy query](https://www.elastic.co/guide/en/elasticsearch/reference/7.2/query-dsl-fuzzy-query.html) is used by default. You can refine your search with [Elasticsearch syntax](#syntax). diff --git a/lib/api/helpers/packages/dependency_proxy_helpers.rb b/lib/api/helpers/packages/dependency_proxy_helpers.rb index 4b0e63c8f3b..a8e5560e106 100644 --- a/lib/api/helpers/packages/dependency_proxy_helpers.rb +++ b/lib/api/helpers/packages/dependency_proxy_helpers.rb @@ -28,7 +28,7 @@ module API end def registry_url(package_type, options) - base_url = REGISTRY_BASE_URLS[package_type] + base_url = registry_base_url(package_type) raise ArgumentError, "Can't build registry_url for package_type #{package_type}" unless base_url @@ -66,7 +66,14 @@ module API Feature.enabled?(:maven_central_request_forwarding, target.root_ancestor) end + + # Override in JiHu repo + def registry_base_url(package_type) + REGISTRY_BASE_URLS[package_type] + end end end end end + +API::Helpers::Packages::DependencyProxyHelpers.prepend_mod diff --git a/lib/gitlab/chat/responder.rb b/lib/gitlab/chat/responder.rb index 478be5bd350..7ec64b7cfbf 100644 --- a/lib/gitlab/chat/responder.rb +++ b/lib/gitlab/chat/responder.rb @@ -11,21 +11,13 @@ module Gitlab # # build - A `Ci::Build` that executed a chat command. def self.responder_for(build) - if Feature.enabled?(:use_response_url_for_chat_responder) - response_url = build.pipeline.chat_data&.response_url - return unless response_url + response_url = build.pipeline.chat_data&.response_url + return unless response_url - if response_url.start_with?('https://hooks.slack.com/') - Gitlab::Chat::Responder::Slack.new(build) - else - Gitlab::Chat::Responder::Mattermost.new(build) - end + if response_url.start_with?('https://hooks.slack.com/') + Gitlab::Chat::Responder::Slack.new(build) else - integration = build.pipeline.chat_data&.chat_name&.integration - - if (responder = integration.try(:chat_responder)) - responder.new(build) - end + Gitlab::Chat::Responder::Mattermost.new(build) end end end diff --git a/lib/tasks/gitlab/assets.rake b/lib/tasks/gitlab/assets.rake index 52a6ea6ae41..8eae36008fd 100644 --- a/lib/tasks/gitlab/assets.rake +++ b/lib/tasks/gitlab/assets.rake @@ -76,6 +76,8 @@ namespace :gitlab do desc 'GitLab | Assets | Compile all frontend assets' task :compile do + require 'fileutils' + require_dependency 'gitlab/task_helpers' puts "Assets SHA256 for `master`: #{Tasks::Gitlab::Assets.master_assets_sha256.inspect}" diff --git a/lib/tasks/gitlab/terraform/migrate.rake b/lib/tasks/gitlab/terraform/migrate.rake index 1a85785c88e..3347b823376 100644 --- a/lib/tasks/gitlab/terraform/migrate.rake +++ b/lib/tasks/gitlab/terraform/migrate.rake @@ -4,6 +4,8 @@ desc "GitLab | Terraform | Migrate Terraform states to remote storage" namespace :gitlab do namespace :terraform_states do task migrate: :environment do + require 'logger' + logger = Logger.new($stdout) logger.info('Starting transfer of Terraform states to object storage') diff --git a/lib/tasks/gitlab/tw/codeowners.rake b/lib/tasks/gitlab/tw/codeowners.rake index 742f13063c9..25d6277a2be 100644 --- a/lib/tasks/gitlab/tw/codeowners.rake +++ b/lib/tasks/gitlab/tw/codeowners.rake @@ -3,6 +3,8 @@ namespace :tw do desc 'Generates a list of codeowners for documentation pages.' task :codeowners do + require 'yaml' + CodeOwnerRule = Struct.new(:category, :writer) DocumentOwnerMapping = Struct.new(:path, :writer) do def writer_owns_directory?(mappings) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9c86b87a01a..f89b1fbd69a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8015,6 +8015,15 @@ msgstr "" msgid "Can't be empty" msgstr "" +msgid "Can't contain %{chars}" +msgstr "" + +msgid "Can't contain spaces" +msgstr "" + +msgid "Can't contain spaces, %{chars}" +msgstr "" + msgid "Can't create snippet: %{err}" msgstr "" diff --git a/scripts/pipeline_test_report_builder.rb b/scripts/pipeline_test_report_builder.rb index 6f69a5c692f..15a78969266 100755 --- a/scripts/pipeline_test_report_builder.rb +++ b/scripts/pipeline_test_report_builder.rb @@ -20,6 +20,7 @@ require_relative 'api/default_options' class PipelineTestReportBuilder DEFAULT_OPTIONS = { target_project: Host::DEFAULT_OPTIONS[:target_project], + current_pipeline_id: API::DEFAULT_OPTIONS[:pipeline_id], mr_iid: Host::DEFAULT_OPTIONS[:mr_iid], api_endpoint: API::DEFAULT_OPTIONS[:endpoint], output_file_path: 'test_results/test_reports.json', @@ -28,6 +29,7 @@ class PipelineTestReportBuilder def initialize(options) @target_project = options.delete(:target_project) + @current_pipeline_id = options.delete(:current_pipeline_id) @mr_iid = options.delete(:mr_iid) @api_endpoint = options.delete(:api_endpoint).to_s @output_file_path = options.delete(:output_file_path).to_s @@ -47,7 +49,7 @@ class PipelineTestReportBuilder end def latest_pipeline - pipelines_sorted_descending[0] + fetch("#{target_project_api_base_url}/pipelines/#{current_pipeline_id}") end def previous_pipeline @@ -58,6 +60,8 @@ class PipelineTestReportBuilder private + attr_reader :target_project, :current_pipeline_id, :mr_iid, :api_endpoint, :output_file_path, :pipeline_index + def pipeline @pipeline ||= case pipeline_index @@ -76,8 +80,6 @@ class PipelineTestReportBuilder pipelines_for_mr.sort_by { |a| -a['id'] } end - attr_reader :target_project, :mr_iid, :api_endpoint, :output_file_path, :pipeline_index - def pipeline_project_api_base_url(pipeline) "#{api_endpoint}/projects/#{pipeline['project_id']}" end diff --git a/spec/factories/users/banned_users.rb b/spec/factories/users/banned_users.rb new file mode 100644 index 00000000000..f2b6eb5893a --- /dev/null +++ b/spec/factories/users/banned_users.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :banned_user, class: 'Users::BannedUser' do + user { association(:user) } + end +end diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb index bbc14368d82..6325f226ccf 100644 --- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb +++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb @@ -113,6 +113,26 @@ RSpec.describe 'User creates branch and merge request on issue page', :js, featu expect(page).to have_current_path project_tree_path(project, branch_name), ignore_query: true end end + + context 'when branch name is invalid' do + shared_examples 'has error message' do |dropdown| + it 'has error message' do + select_dropdown_option(dropdown, 'custom-branch-name w~th ^bad chars?') + + wait_for_requests + + expect(page).to have_text("Can't contain spaces, ~, ^, ?") + end + end + + context 'when creating a merge request', :sidekiq_might_not_need_inline do + it_behaves_like 'has error message', 'create-mr' + end + + context 'when creating a branch', :sidekiq_might_not_need_inline do + it_behaves_like 'has error message', 'create-branch' + end + end end context "when there is a referenced merge request" do diff --git a/spec/frontend/lib/utils/text_utility_spec.js b/spec/frontend/lib/utils/text_utility_spec.js index f2572ca0ad2..71a84d56791 100644 --- a/spec/frontend/lib/utils/text_utility_spec.js +++ b/spec/frontend/lib/utils/text_utility_spec.js @@ -398,4 +398,36 @@ describe('text_utility', () => { expect(textUtils.base64DecodeUnicode('8J+YgA==')).toBe('😀'); }); }); + + describe('findInvalidBranchNameCharacters', () => { + const invalidChars = [' ', '~', '^', ':', '?', '*', '[', '..', '@{', '\\', '//']; + const badBranchName = 'branch-with all these ~ ^ : ? * [ ] \\ // .. @{ } //'; + const goodBranch = 'branch-with-no-errrors'; + + it('returns an array of invalid characters in a branch name', () => { + const chars = textUtils.findInvalidBranchNameCharacters(badBranchName); + chars.forEach((char) => { + expect(invalidChars).toContain(char); + }); + }); + + it('returns an empty array with no invalid characters', () => { + expect(textUtils.findInvalidBranchNameCharacters(goodBranch)).toEqual([]); + }); + }); + + describe('humanizeBranchValidationErrors', () => { + it.each` + errors | message + ${[' ']} | ${"Can't contain spaces"} + ${['?', '//', ' ']} | ${"Can't contain spaces, ?, //"} + ${['\\', '[', '..']} | ${"Can't contain \\, [, .."} + `('returns an $message with $errors', ({ errors, message }) => { + expect(textUtils.humanizeBranchValidationErrors(errors)).toEqual(message); + }); + + it('returns an empty string with no invalid characters', () => { + expect(textUtils.humanizeBranchValidationErrors([])).toEqual(''); + }); + }); }); diff --git a/spec/frontend/vue_shared/components/tooltip_on_truncate_spec.js b/spec/frontend/vue_shared/components/tooltip_on_truncate_spec.js index 5e7b2a89235..01fc2dc4661 100644 --- a/spec/frontend/vue_shared/components/tooltip_on_truncate_spec.js +++ b/spec/frontend/vue_shared/components/tooltip_on_truncate_spec.js @@ -90,7 +90,7 @@ describe('TooltipOnTruncate component', () => { it('renders tooltip', async () => { expect(hasHorizontalOverflow).toHaveBeenLastCalledWith(wrapper.element); - expect(getTooltipValue()).toMatchObject({ + expect(getTooltipValue()).toStrictEqual({ title: MOCK_TITLE, placement: 'top', disabled: false, @@ -144,7 +144,7 @@ describe('TooltipOnTruncate component', () => { await nextTick(); - expect(getTooltipValue()).toMatchObject({ + expect(getTooltipValue()).toStrictEqual({ title: MOCK_TITLE, placement: 'top', disabled: false, @@ -194,20 +194,22 @@ describe('TooltipOnTruncate component', () => { }); }); - describe('placement', () => { - it('sets placement when tooltip is rendered', () => { - const mockPlacement = 'bottom'; - + describe('tooltip customization', () => { + it.each` + property | mockValue + ${'placement'} | ${'bottom'} + ${'boundary'} | ${'viewport'} + `('sets $property when the tooltip is rendered', ({ property, mockValue }) => { hasHorizontalOverflow.mockReturnValueOnce(true); createComponent({ propsData: { - placement: mockPlacement, + [property]: mockValue, }, }); expect(hasHorizontalOverflow).toHaveBeenLastCalledWith(wrapper.element); expect(getTooltipValue()).toMatchObject({ - placement: mockPlacement, + [property]: mockValue, }); }); }); diff --git a/spec/lib/gitlab/chat/responder_spec.rb b/spec/lib/gitlab/chat/responder_spec.rb index a9d290cb87c..15ca3427ae8 100644 --- a/spec/lib/gitlab/chat/responder_spec.rb +++ b/spec/lib/gitlab/chat/responder_spec.rb @@ -4,68 +4,32 @@ require 'spec_helper' RSpec.describe Gitlab::Chat::Responder, feature_category: :integrations do describe '.responder_for' do - context 'when the feature flag is disabled' do - before do - stub_feature_flags(use_response_url_for_chat_responder: false) - end - - context 'using a regular build' do - it 'returns nil' do - build = create(:ci_build) + context 'using a regular build' do + it 'returns nil' do + build = create(:ci_build) - expect(described_class.responder_for(build)).to be_nil - end - end - - context 'using a chat build' do - it 'returns the responder for the build' do - pipeline = create(:ci_pipeline) - build = create(:ci_build, pipeline: pipeline) - integration = double(:integration, chat_responder: Gitlab::Chat::Responder::Slack) - chat_name = double(:chat_name, integration: integration) - chat_data = double(:chat_data, chat_name: chat_name) - - allow(pipeline) - .to receive(:chat_data) - .and_return(chat_data) - - expect(described_class.responder_for(build)) - .to be_an_instance_of(Gitlab::Chat::Responder::Slack) - end + expect(described_class.responder_for(build)).to be_nil end end - context 'when the feature flag is enabled' do - before do - stub_feature_flags(use_response_url_for_chat_responder: true) - end - - context 'using a regular build' do - it 'returns nil' do - build = create(:ci_build) + context 'using a chat build' do + let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:build) { create(:ci_build, pipeline: pipeline) } - expect(described_class.responder_for(build)).to be_nil + context "when response_url starts with 'https://hooks.slack.com/'" do + before do + pipeline.build_chat_data(response_url: 'https://hooks.slack.com/services/12345', chat_name_id: 'U123') end + + it { expect(described_class.responder_for(build)).to be_an_instance_of(Gitlab::Chat::Responder::Slack) } end - context 'using a chat build' do - let(:chat_name) { create(:chat_name, chat_id: 'U123') } - let(:pipeline) do - pipeline = create(:ci_pipeline) - pipeline.create_chat_data!( - response_url: 'https://hooks.slack.com/services/12345', - chat_name_id: chat_name.id - ) - pipeline + context "when response_url does not start with 'https://hooks.slack.com/'" do + before do + pipeline.build_chat_data(response_url: 'https://mattermost.example.com/services/12345', chat_name_id: 'U123') end - let(:build) { create(:ci_build, pipeline: pipeline) } - let(:responder) { described_class.new(build) } - - it 'returns the responder for the build' do - expect(described_class.responder_for(build)) - .to be_an_instance_of(Gitlab::Chat::Responder::Slack) - end + it { expect(described_class.responder_for(build)).to be_an_instance_of(Gitlab::Chat::Responder::Mattermost) } end end end diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb index 8ff8de2379a..369d7e994d2 100644 --- a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -116,7 +116,7 @@ RSpec.describe Gitlab::Email::Handler::CreateIssueHandler do context "when the issue could not be saved" do before do allow_any_instance_of(Issue).to receive(:persisted?).and_return(false) - allow_any_instance_of(Issue).to receive(:ensure_metrics).and_return(nil) + allow_any_instance_of(Issue).to receive(:ensure_metrics!).and_return(nil) end it "raises an InvalidIssueError" do diff --git a/spec/models/integrations/mattermost_slash_commands_spec.rb b/spec/models/integrations/mattermost_slash_commands_spec.rb index 070adb9ba93..6effc035161 100644 --- a/spec/models/integrations/mattermost_slash_commands_spec.rb +++ b/spec/models/integrations/mattermost_slash_commands_spec.rb @@ -123,12 +123,5 @@ RSpec.describe Integrations::MattermostSlashCommands do end end end - - describe '#chat_responder' do - it 'returns the responder to use for Mattermost' do - expect(described_class.new.chat_responder) - .to eq(Gitlab::Chat::Responder::Mattermost) - end - end end end diff --git a/spec/models/integrations/slack_slash_commands_spec.rb b/spec/models/integrations/slack_slash_commands_spec.rb index 22cbaa777cd..7336424623d 100644 --- a/spec/models/integrations/slack_slash_commands_spec.rb +++ b/spec/models/integrations/slack_slash_commands_spec.rb @@ -40,11 +40,4 @@ RSpec.describe Integrations::SlackSlashCommands do end end end - - describe '#chat_responder' do - it 'returns the responder to use for Slack' do - expect(described_class.new.chat_responder) - .to eq(Gitlab::Chat::Responder::Slack) - end - end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index d00f2f29fed..6e54ac3bd3f 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -220,7 +220,7 @@ RSpec.describe Issue, feature_category: :team_planning do subject { create(:issue, project: reusable_project) } describe 'callbacks' do - describe '#ensure_metrics' do + describe '#ensure_metrics!' do it 'creates metrics after saving' do expect(subject.metrics).to be_persisted expect(Issue::Metrics.count).to eq(1) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d31fe8d2382..3932a925086 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -393,8 +393,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev instance1 = MergeRequest.find(merge_request.id) instance2 = MergeRequest.find(merge_request.id) - instance1.ensure_metrics - instance2.ensure_metrics + instance1.ensure_metrics! + instance2.ensure_metrics! metrics_records = MergeRequest::Metrics.where(merge_request_id: merge_request.id) expect(metrics_records.size).to eq(1) diff --git a/spec/scripts/pipeline_test_report_builder_spec.rb b/spec/scripts/pipeline_test_report_builder_spec.rb index e7529eb0d41..bee2a4a5835 100644 --- a/spec/scripts/pipeline_test_report_builder_spec.rb +++ b/spec/scripts/pipeline_test_report_builder_spec.rb @@ -9,6 +9,7 @@ RSpec.describe PipelineTestReportBuilder, feature_category: :tooling do let(:options) do described_class::DEFAULT_OPTIONS.merge( target_project: 'gitlab-org/gitlab', + current_pipeline_id: '42', mr_id: '999', instance_base_url: 'https://gitlab.com', output_file_path: output_file_path @@ -191,10 +192,14 @@ RSpec.describe PipelineTestReportBuilder, feature_category: :tooling do context 'for latest pipeline' do let(:failed_build_uri) { "#{latest_pipeline_url}/tests/suite.json?build_ids[]=#{failed_build_id}" } + let(:current_pipeline_uri) do + "#{options[:api_endpoint]}/projects/#{options[:target_project]}/pipelines/#{options[:current_pipeline_id]}" + end subject { described_class.new(options.merge(pipeline_index: :latest)) } it 'fetches builds from pipeline related to MR' do + expect(subject).to receive(:fetch).with(current_pipeline_uri).and_return(mr_pipelines[0]) expect(subject).to receive(:fetch).with(failed_build_uri).and_return(test_report_for_build) subject.test_report_for_pipeline diff --git a/workhorse/go.mod b/workhorse/go.mod index df4ea11aa9c..d847bfa673b 100644 --- a/workhorse/go.mod +++ b/workhorse/go.mod @@ -34,7 +34,7 @@ require ( golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 golang.org/x/net v0.7.0 golang.org/x/oauth2 v0.5.0 - golang.org/x/tools v0.2.0 + golang.org/x/tools v0.6.0 google.golang.org/grpc v1.53.0 google.golang.org/protobuf v1.28.1 honnef.co/go/tools v0.3.3 @@ -109,7 +109,7 @@ require ( go.uber.org/atomic v1.10.0 // indirect golang.org/x/crypto v0.5.0 // indirect golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e // indirect - golang.org/x/mod v0.6.0 // indirect + golang.org/x/mod v0.8.0 // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/sys v0.5.0 // indirect golang.org/x/text v0.7.0 // indirect diff --git a/workhorse/go.sum b/workhorse/go.sum index 6196b2f0e90..55c54c2764e 100644 --- a/workhorse/go.sum +++ b/workhorse/go.sum @@ -2042,8 +2042,9 @@ golang.org/x/mod v0.5.0/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro= golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro= golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.6.0 h1:b9gGHsz9/HhJ3HF5DHQytPpuwocVTChQJK3AvoLRD5I= golang.org/x/mod v0.6.0/go.mod h1:4mET923SAdbXp2ki8ey+zGs1SLqsuM2Y0uvdZR/fUNI= +golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= +golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -2456,8 +2457,9 @@ golang.org/x/tools v0.1.6-0.20210726203631-07bc1bf47fb2/go.mod h1:o0xws9oXOQQZyj golang.org/x/tools v0.1.9/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU= golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE= golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA= +golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190410155217-1f06c39b4373/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= |