diff options
24 files changed, 472 insertions, 337 deletions
diff --git a/app/assets/images/aws_logo.svg b/app/assets/images/aws_logo.svg new file mode 100644 index 00000000000..e028fd1b1c0 --- /dev/null +++ b/app/assets/images/aws_logo.svg @@ -0,0 +1,38 @@ +<?xml version="1.0" encoding="utf-8"?>
+<!-- Generator: Adobe Illustrator 19.0.1, SVG Export Plug-In . SVG Version: 6.00 Build 0) -->
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
+ viewBox="0 0 304 182" style="enable-background:new 0 0 304 182;" xml:space="preserve">
+<style type="text/css">
+ .st0{fill:#252F3E;}
+ .st1{fill-rule:evenodd;clip-rule:evenodd;fill:#FF9900;}
+</style>
+<g>
+ <path class="st0" d="M86.4,66.4c0,3.7,0.4,6.7,1.1,8.9c0.8,2.2,1.8,4.6,3.2,7.2c0.5,0.8,0.7,1.6,0.7,2.3c0,1-0.6,2-1.9,3l-6.3,4.2
+ c-0.9,0.6-1.8,0.9-2.6,0.9c-1,0-2-0.5-3-1.4C76.2,90,75,88.4,74,86.8c-1-1.7-2-3.6-3.1-5.9c-7.8,9.2-17.6,13.8-29.4,13.8
+ c-8.4,0-15.1-2.4-20-7.2c-4.9-4.8-7.4-11.2-7.4-19.2c0-8.5,3-15.4,9.1-20.6c6.1-5.2,14.2-7.8,24.5-7.8c3.4,0,6.9,0.3,10.6,0.8
+ c3.7,0.5,7.5,1.3,11.5,2.2v-7.3c0-7.6-1.6-12.9-4.7-16c-3.2-3.1-8.6-4.6-16.3-4.6c-3.5,0-7.1,0.4-10.8,1.3c-3.7,0.9-7.3,2-10.8,3.4
+ c-1.6,0.7-2.8,1.1-3.5,1.3c-0.7,0.2-1.2,0.3-1.6,0.3c-1.4,0-2.1-1-2.1-3.1v-4.9c0-1.6,0.2-2.8,0.7-3.5c0.5-0.7,1.4-1.4,2.8-2.1
+ c3.5-1.8,7.7-3.3,12.6-4.5c4.9-1.3,10.1-1.9,15.6-1.9c11.9,0,20.6,2.7,26.2,8.1c5.5,5.4,8.3,13.6,8.3,24.6V66.4z M45.8,81.6
+ c3.3,0,6.7-0.6,10.3-1.8c3.6-1.2,6.8-3.4,9.5-6.4c1.6-1.9,2.8-4,3.4-6.4c0.6-2.4,1-5.3,1-8.7v-4.2c-2.9-0.7-6-1.3-9.2-1.7
+ c-3.2-0.4-6.3-0.6-9.4-0.6c-6.7,0-11.6,1.3-14.9,4c-3.3,2.7-4.9,6.5-4.9,11.5c0,4.7,1.2,8.2,3.7,10.6
+ C37.7,80.4,41.2,81.6,45.8,81.6z M126.1,92.4c-1.8,0-3-0.3-3.8-1c-0.8-0.6-1.5-2-2.1-3.9L96.7,10.2c-0.6-2-0.9-3.3-0.9-4
+ c0-1.6,0.8-2.5,2.4-2.5h9.8c1.9,0,3.2,0.3,3.9,1c0.8,0.6,1.4,2,2,3.9l16.8,66.2l15.6-66.2c0.5-2,1.1-3.3,1.9-3.9c0.8-0.6,2.2-1,4-1
+ h8c1.9,0,3.2,0.3,4,1c0.8,0.6,1.5,2,1.9,3.9l15.8,67l17.3-67c0.6-2,1.3-3.3,2-3.9c0.8-0.6,2.1-1,3.9-1h9.3c1.6,0,2.5,0.8,2.5,2.5
+ c0,0.5-0.1,1-0.2,1.6c-0.1,0.6-0.3,1.4-0.7,2.5l-24.1,77.3c-0.6,2-1.3,3.3-2.1,3.9c-0.8,0.6-2.1,1-3.8,1h-8.6c-1.9,0-3.2-0.3-4-1
+ c-0.8-0.7-1.5-2-1.9-4L156,23l-15.4,64.4c-0.5,2-1.1,3.3-1.9,4c-0.8,0.7-2.2,1-4,1H126.1z M254.6,95.1c-5.2,0-10.4-0.6-15.4-1.8
+ c-5-1.2-8.9-2.5-11.5-4c-1.6-0.9-2.7-1.9-3.1-2.8c-0.4-0.9-0.6-1.9-0.6-2.8v-5.1c0-2.1,0.8-3.1,2.3-3.1c0.6,0,1.2,0.1,1.8,0.3
+ c0.6,0.2,1.5,0.6,2.5,1c3.4,1.5,7.1,2.7,11,3.5c4,0.8,7.9,1.2,11.9,1.2c6.3,0,11.2-1.1,14.6-3.3c3.4-2.2,5.2-5.4,5.2-9.5
+ c0-2.8-0.9-5.1-2.7-7c-1.8-1.9-5.2-3.6-10.1-5.2L246,52c-7.3-2.3-12.7-5.7-16-10.2c-3.3-4.4-5-9.3-5-14.5c0-4.2,0.9-7.9,2.7-11.1
+ c1.8-3.2,4.2-6,7.2-8.2c3-2.3,6.4-4,10.4-5.2c4-1.2,8.2-1.7,12.6-1.7c2.2,0,4.5,0.1,6.7,0.4c2.3,0.3,4.4,0.7,6.5,1.1
+ c2,0.5,3.9,1,5.7,1.6c1.8,0.6,3.2,1.2,4.2,1.8c1.4,0.8,2.4,1.6,3,2.5c0.6,0.8,0.9,1.9,0.9,3.3v4.7c0,2.1-0.8,3.2-2.3,3.2
+ c-0.8,0-2.1-0.4-3.8-1.2c-5.7-2.6-12.1-3.9-19.2-3.9c-5.7,0-10.2,0.9-13.3,2.8c-3.1,1.9-4.7,4.8-4.7,8.9c0,2.8,1,5.2,3,7.1
+ c2,1.9,5.7,3.8,11,5.5l14.2,4.5c7.2,2.3,12.4,5.5,15.5,9.6c3.1,4.1,4.6,8.8,4.6,14c0,4.3-0.9,8.2-2.6,11.6
+ c-1.8,3.4-4.2,6.4-7.3,8.8c-3.1,2.5-6.8,4.3-11.1,5.6C264.4,94.4,259.7,95.1,254.6,95.1z"/>
+ <g>
+ <path class="st1" d="M273.5,143.7c-32.9,24.3-80.7,37.2-121.8,37.2c-57.6,0-109.5-21.3-148.7-56.7c-3.1-2.8-0.3-6.6,3.4-4.4
+ c42.4,24.6,94.7,39.5,148.8,39.5c36.5,0,76.6-7.6,113.5-23.2C274.2,133.6,278.9,139.7,273.5,143.7z"/>
+ <path class="st1" d="M287.2,128.1c-4.2-5.4-27.8-2.6-38.5-1.3c-3.2,0.4-3.7-2.4-0.8-4.5c18.8-13.2,49.7-9.4,53.3-5
+ c3.6,4.5-1,35.4-18.6,50.2c-2.7,2.3-5.3,1.1-4.1-1.9C282.5,155.7,291.4,133.4,287.2,128.1z"/>
+ </g>
+</g>
+</svg>
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue index e57da6c6d65..6531b945212 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue @@ -1,19 +1,29 @@ <script> import { + GlAlert, + GlButton, + GlCollapse, GlDeprecatedButton, - GlModal, - GlFormSelect, + GlFormCheckbox, GlFormGroup, GlFormInput, + GlFormSelect, GlFormTextarea, - GlFormCheckbox, - GlLink, GlIcon, + GlLink, + GlModal, + GlSprintf, } from '@gitlab/ui'; +import Cookies from 'js-cookie'; import { mapActions, mapState } from 'vuex'; import { __ } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import { ADD_CI_VARIABLE_MODAL_ID } from '../constants'; +import { + AWS_TOKEN_CONSTANTS, + ADD_CI_VARIABLE_MODAL_ID, + AWS_TIP_DISMISSED_COOKIE_NAME, + AWS_TIP_MESSAGE, +} from '../constants'; import { awsTokens, awsTokenList } from './ci_variable_autocomplete_tokens'; import CiKeyField from './ci_key_field.vue'; import CiEnvironmentsDropdown from './ci_environments_dropdown.vue'; @@ -23,19 +33,29 @@ export default { components: { CiEnvironmentsDropdown, CiKeyField, + GlAlert, + GlButton, + GlCollapse, GlDeprecatedButton, - GlModal, - GlFormSelect, + GlFormCheckbox, GlFormGroup, GlFormInput, + GlFormSelect, GlFormTextarea, - GlFormCheckbox, - GlLink, GlIcon, + GlLink, + GlModal, + GlSprintf, }, mixins: [glFeatureFlagsMixin()], tokens: awsTokens, tokenList: awsTokenList, + awsTipMessage: AWS_TIP_MESSAGE, + data() { + return { + isTipDismissed: Cookies.get(AWS_TIP_DISMISSED_COOKIE_NAME) === 'true', + }; + }, computed: { ...mapState([ 'projectId', @@ -47,7 +67,16 @@ export default { 'maskableRegex', 'selectedEnvironment', 'isProtectedByDefault', + 'awsLogoSvgPath', + 'awsTipDeployLink', + 'awsTipCommandsLink', + 'awsTipLearnLink', + 'protectedEnvironmentVariablesLink', + 'maskedEnvironmentVariablesLink', ]), + isTipVisible() { + return !this.isTipDismissed && AWS_TOKEN_CONSTANTS.includes(this.variableData.key); + }, canSubmit() { return ( this.variableValidationState && @@ -126,6 +155,10 @@ export default { 'setSelectedEnvironment', 'setVariableProtected', ]), + dismissTip() { + Cookies.set(AWS_TIP_DISMISSED_COOKIE_NAME, 'true', { expires: 90 }); + this.isTipDismissed = true; + }, deleteVarAndClose() { this.deleteVariable(this.variableBeingEdited); this.hideModal(); @@ -232,7 +265,7 @@ export default { <gl-form-group :label="__('Flags')" label-for="ci-variable-flags"> <gl-form-checkbox v-model="variableData.protected" class="mb-0"> {{ __('Protect variable') }} - <gl-link href="/help/ci/variables/README#protected-environment-variables"> + <gl-link target="_blank" :href="protectedEnvironmentVariablesLink"> <gl-icon name="question" :size="12" /> </gl-link> <p class="gl-mt-2 text-secondary"> @@ -246,7 +279,7 @@ export default { data-qa-selector="ci_variable_masked_checkbox" > {{ __('Mask variable') }} - <gl-link href="/help/ci/variables/README#masked-variables"> + <gl-link target="_blank" :href="maskedEnvironmentVariablesLink"> <gl-icon name="question" :size="12" /> </gl-link> <p class="gl-mt-2 gl-mb-0 text-secondary"> @@ -258,13 +291,52 @@ export default { > {{ __('Requires values to meet regular expression requirements.') }}</span > - <gl-link href="/help/ci/variables/README#masked-variables">{{ + <gl-link target="_blank" :href="maskedEnvironmentVariablesLink">{{ __('More information') }}</gl-link> </p> </gl-form-checkbox> </gl-form-group> </form> + <gl-collapse :visible="isTipVisible"> + <gl-alert + :title="__('Deploying to AWS is easy with GitLab')" + variant="tip" + data-testid="aws-guidance-tip" + @dismiss="dismissTip" + > + <div class="gl-display-flex gl-flex-direction-row"> + <div> + <p> + <gl-sprintf :message="$options.awsTipMessage"> + <template #deployLink="{ content }"> + <gl-link :href="awsTipDeployLink" target="_blank">{{ content }}</gl-link> + </template> + <template #commandsLink="{ content }"> + <gl-link :href="awsTipCommandsLink" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> + </p> + <p> + <gl-button + :href="awsTipLearnLink" + target="_blank" + category="secondary" + variant="info" + class="gl-overflow-wrap-break" + >{{ __('Learn more about deploying to AWS') }}</gl-button + > + </p> + </div> + <img + class="gl-mt-3" + :alt="__('Amazon Web Services Logo')" + :src="awsLogoSvgPath" + height="32" + /> + </div> + </gl-alert> + </gl-collapse> <template #modal-footer> <gl-deprecated-button @click="hideModal">{{ __('Cancel') }}</gl-deprecated-button> <gl-deprecated-button diff --git a/app/assets/javascripts/ci_variable_list/constants.js b/app/assets/javascripts/ci_variable_list/constants.js index a4db6481720..ef304c7ccee 100644 --- a/app/assets/javascripts/ci_variable_list/constants.js +++ b/app/assets/javascripts/ci_variable_list/constants.js @@ -15,7 +15,13 @@ export const types = { allEnvironmentsType: '*', }; +export const AWS_TIP_DISMISSED_COOKIE_NAME = 'ci_variable_list_constants_aws_tip_dismissed'; +export const AWS_TIP_MESSAGE = __( + '%{deployLinkStart}Use a template to deploy to ECS%{deployLinkEnd}, or use a docker image to %{commandsLinkStart}run AWS commands in GitLab CI/CD%{commandsLinkEnd}.', +); + // AWS TOKEN CONSTANTS export const AWS_ACCESS_KEY_ID = 'AWS_ACCESS_KEY_ID'; export const AWS_DEFAULT_REGION = 'AWS_DEFAULT_REGION'; export const AWS_SECRET_ACCESS_KEY = 'AWS_SECRET_ACCESS_KEY'; +export const AWS_TOKEN_CONSTANTS = [AWS_ACCESS_KEY_ID, AWS_DEFAULT_REGION, AWS_SECRET_ACCESS_KEY]; diff --git a/app/assets/javascripts/ci_variable_list/index.js b/app/assets/javascripts/ci_variable_list/index.js index 095c39d54b2..a28b52d6b57 100644 --- a/app/assets/javascripts/ci_variable_list/index.js +++ b/app/assets/javascripts/ci_variable_list/index.js @@ -5,7 +5,19 @@ import { parseBoolean } from '~/lib/utils/common_utils'; export default (containerId = 'js-ci-project-variables') => { const containerEl = document.getElementById(containerId); - const { endpoint, projectId, group, maskableRegex, protectedByDefault } = containerEl.dataset; + const { + endpoint, + projectId, + group, + maskableRegex, + protectedByDefault, + awsLogoSvgPath, + awsTipDeployLink, + awsTipCommandsLink, + awsTipLearnLink, + protectedEnvironmentVariablesLink, + maskedEnvironmentVariablesLink, + } = containerEl.dataset; const isGroup = parseBoolean(group); const isProtectedByDefault = parseBoolean(protectedByDefault); @@ -15,6 +27,12 @@ export default (containerId = 'js-ci-project-variables') => { isGroup, maskableRegex, isProtectedByDefault, + awsLogoSvgPath, + awsTipDeployLink, + awsTipCommandsLink, + awsTipLearnLink, + protectedEnvironmentVariablesLink, + maskedEnvironmentVariablesLink, }); return new Vue({ diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index 26051261715..fa5f2c514ae 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -8,7 +8,18 @@ - if Feature.enabled?(:new_variables_ui, @project || @group, default_enabled: true) - is_group = !@group.nil? - #js-ci-project-variables{ data: { endpoint: save_endpoint, project_id: @project&.id || '', group: is_group.to_s, maskable_regex: ci_variable_maskable_regex, protected_by_default: ci_variable_protected_by_default?.to_s} } + #js-ci-project-variables{ data: { endpoint: save_endpoint, + project_id: @project&.id || '', + group: is_group.to_s, + maskable_regex: ci_variable_maskable_regex, + protected_by_default: ci_variable_protected_by_default?.to_s, + aws_logo_svg_path: image_path('aws_logo.svg'), + aws_tip_deploy_link: help_page_path('ci/cloud_deployment/index.md', anchor: 'deploy-your-application-to-the-aws-elastic-container-service-ecs'), + aws_tip_commands_link: help_page_path('ci/cloud_deployment/index.md', anchor: 'run-aws-commands-from-gitlab-cicd'), + aws_tip_learn_link: help_page_path('ci/cloud_deployment/index.md', anchor: 'aws'), + protected_environment_variables_link: help_page_path('ci/variables/README', anchor: 'protect-a-custom-variable'), + masked_environment_variables_link: help_page_path('ci/variables/README', anchor: 'mask-a-custom-variable'), + } } - else .row diff --git a/app/workers/authorized_project_update/user_refresh_with_low_urgency_worker.rb b/app/workers/authorized_project_update/user_refresh_with_low_urgency_worker.rb index 19038cb8900..7ca59a72adf 100644 --- a/app/workers/authorized_project_update/user_refresh_with_low_urgency_worker.rb +++ b/app/workers/authorized_project_update/user_refresh_with_low_urgency_worker.rb @@ -5,6 +5,7 @@ module AuthorizedProjectUpdate feature_category :authentication_and_authorization urgency :low queue_namespace :authorized_project_update + deduplicate :until_executing, including_scheduled: true idempotent! end diff --git a/app/workers/concerns/worker_attributes.rb b/app/workers/concerns/worker_attributes.rb index 7fa56c25210..b19217b15de 100644 --- a/app/workers/concerns/worker_attributes.rb +++ b/app/workers/concerns/worker_attributes.rb @@ -119,6 +119,20 @@ module WorkerAttributes Array(worker_attributes[:tags]) end + def deduplicate(strategy, options = {}) + worker_attributes[:deduplication_strategy] = strategy + worker_attributes[:deduplication_options] = options + end + + def get_deduplicate_strategy + worker_attributes[:deduplication_strategy] || + Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DEFAULT_STRATEGY + end + + def get_deduplication_options + worker_attributes[:deduplication_options] || {} + end + protected # Returns a worker attribute declared on this class or its parent class. diff --git a/changelogs/unreleased/215194-add-section-to-index_approval_rule_name_for_code_owners_rule_type.yml b/changelogs/unreleased/215194-add-section-to-index_approval_rule_name_for_code_owners_rule_type.yml deleted file mode 100644 index 4f5103d2305..00000000000 --- a/changelogs/unreleased/215194-add-section-to-index_approval_rule_name_for_code_owners_rule_type.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Add :section to approval_merge_request_rule unique index -merge_request: 33121 -author: -type: other diff --git a/changelogs/unreleased/aws-guidance.yml b/changelogs/unreleased/aws-guidance.yml new file mode 100644 index 00000000000..ab3725c1397 --- /dev/null +++ b/changelogs/unreleased/aws-guidance.yml @@ -0,0 +1,5 @@ +--- +title: Adds AWS guidance to CI/CD > Add Variable modal +merge_request: 34009 +author: +type: added diff --git a/db/migrate/20200526231421_update_index_approval_rule_name_for_code_owners_rule_type.rb b/db/migrate/20200526231421_update_index_approval_rule_name_for_code_owners_rule_type.rb deleted file mode 100644 index 7e31c9880cd..00000000000 --- a/db/migrate/20200526231421_update_index_approval_rule_name_for_code_owners_rule_type.rb +++ /dev/null @@ -1,112 +0,0 @@ -# frozen_string_literal: true - -class UpdateIndexApprovalRuleNameForCodeOwnersRuleType < ActiveRecord::Migration[6.0] - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - LEGACY_INDEX_NAME_RULE_TYPE = "index_approval_rule_name_for_code_owners_rule_type" - LEGACY_INDEX_NAME_CODE_OWNERS = "approval_rule_name_index_for_code_owners" - - SECTIONAL_INDEX_NAME = "index_approval_rule_name_for_sectional_code_owners_rule_type" - - CODE_OWNER_RULE_TYPE = 2 - - def up - unless index_exists_by_name?(:approval_merge_request_rules, SECTIONAL_INDEX_NAME) - # Ensure only 1 code_owner rule with the same name and section per merge_request - # - add_concurrent_index( - :approval_merge_request_rules, - [:merge_request_id, :name, :section], - unique: true, - where: "rule_type = #{CODE_OWNER_RULE_TYPE}", - name: SECTIONAL_INDEX_NAME - ) - end - - remove_concurrent_index_by_name :approval_merge_request_rules, LEGACY_INDEX_NAME_RULE_TYPE - remove_concurrent_index_by_name :approval_merge_request_rules, LEGACY_INDEX_NAME_CODE_OWNERS - - add_concurrent_index( - :approval_merge_request_rules, - [:merge_request_id, :name], - unique: true, - where: "rule_type = #{CODE_OWNER_RULE_TYPE} AND section IS NULL", - name: LEGACY_INDEX_NAME_RULE_TYPE - ) - - add_concurrent_index( - :approval_merge_request_rules, - [:merge_request_id, :code_owner, :name], - unique: true, - where: "code_owner = true AND section IS NULL", - name: LEGACY_INDEX_NAME_CODE_OWNERS - ) - end - - def down - # In a rollback situation, we can't guarantee that there will not be - # records that were allowed under the more specific SECTIONAL_INDEX_NAME - # index but would cause uniqueness violations under both the - # LEGACY_INDEX_NAME_RULE_TYPE and LEGACY_INDEX_NAME_CODE_OWNERS indices. - # Therefore, we need to first find all the MergeRequests with - # ApprovalMergeRequestRules that would violate these "new" indices and - # delete those approval rules, then create the new index, then finally - # recreate the approval rules for those merge requests. - # - - # First, find all MergeRequests with ApprovalMergeRequestRules that will - # violate the new index. - # - merge_request_ids = ApprovalMergeRequestRule - .select(:merge_request_id) - .where(rule_type: CODE_OWNER_RULE_TYPE) - .group(:merge_request_id, :rule_type, :name) - .includes(:merge_request) - .having("count(*) > 1") - .collect(&:merge_request_id) - - # Delete ALL their code_owner approval rules - # - merge_request_ids.each_slice(10) do |ids| - ApprovalMergeRequestRule.where(merge_request_id: ids).code_owner.delete_all - end - - # Remove legacy partial indices that only apply to `section IS NULL` records - # - remove_concurrent_index_by_name :approval_merge_request_rules, LEGACY_INDEX_NAME_RULE_TYPE - remove_concurrent_index_by_name :approval_merge_request_rules, LEGACY_INDEX_NAME_CODE_OWNERS - - # Reconstruct original "legacy" indices - # - add_concurrent_index( - :approval_merge_request_rules, - [:merge_request_id, :name], - unique: true, - where: "rule_type = #{CODE_OWNER_RULE_TYPE}", - name: LEGACY_INDEX_NAME_RULE_TYPE - ) - - add_concurrent_index( - :approval_merge_request_rules, - [:merge_request_id, :code_owner, :name], - unique: true, - where: "code_owner = true", - name: LEGACY_INDEX_NAME_CODE_OWNERS - ) - - # MergeRequest::SyncCodeOwnerApprovalRules recreates the code_owner rules - # from scratch, adding them to the index. Duplicates will be rejected. - # - merge_request_ids.each_slice(10) do |ids| - MergeRequest.where(id: ids).each do |merge_request| - MergeRequests::SyncCodeOwnerApprovalRules.new(merge_request).execute - end - end - - remove_concurrent_index_by_name :approval_merge_request_rules, SECTIONAL_INDEX_NAME - end -end diff --git a/db/structure.sql b/db/structure.sql index 990ed37bc8e..7cd30c15c33 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9156,7 +9156,7 @@ CREATE UNIQUE INDEX any_approver_merge_request_rule_type_unique_index ON public. CREATE UNIQUE INDEX any_approver_project_rule_type_unique_index ON public.approval_project_rules USING btree (project_id) WHERE (rule_type = 3); -CREATE UNIQUE INDEX approval_rule_name_index_for_code_owners ON public.approval_merge_request_rules USING btree (merge_request_id, code_owner, name) WHERE ((code_owner = true) AND (section IS NULL)); +CREATE UNIQUE INDEX approval_rule_name_index_for_code_owners ON public.approval_merge_request_rules USING btree (merge_request_id, code_owner, name) WHERE (code_owner = true); CREATE INDEX ci_builds_gitlab_monitor_metrics ON public.ci_builds USING btree (status, created_at, project_id) WHERE ((type)::text = 'Ci::Build'::text); @@ -9334,9 +9334,7 @@ CREATE UNIQUE INDEX index_approval_project_rules_users_1 ON public.approval_proj CREATE INDEX index_approval_project_rules_users_2 ON public.approval_project_rules_users USING btree (user_id); -CREATE UNIQUE INDEX index_approval_rule_name_for_code_owners_rule_type ON public.approval_merge_request_rules USING btree (merge_request_id, name) WHERE ((rule_type = 2) AND (section IS NULL)); - -CREATE UNIQUE INDEX index_approval_rule_name_for_sectional_code_owners_rule_type ON public.approval_merge_request_rules USING btree (merge_request_id, name, section) WHERE (rule_type = 2); +CREATE UNIQUE INDEX index_approval_rule_name_for_code_owners_rule_type ON public.approval_merge_request_rules USING btree (merge_request_id, name) WHERE (rule_type = 2); CREATE INDEX index_approval_rules_code_owners_rule_type ON public.approval_merge_request_rules USING btree (merge_request_id) WHERE (rule_type = 2); @@ -13963,7 +13961,6 @@ COPY "schema_migrations" (version) FROM STDIN; 20200526153844 20200526164946 20200526164947 -20200526231421 20200527092027 20200527094322 20200527095401 diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index 4b1f7a68f7e..8c1f62330f4 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -42,7 +42,7 @@ are very appreciative of the work done by translators and proofreaders! - Estonian - Proofreaders needed. - Filipino - - Proofreaders needed. + - Andrei Jiroh Halili - [GitLab](https://gitlab.com/AJHalili2006DevPH), [Crowdin](https://crowdin.com/profile/AndreiJirohHaliliDev2006) - French - Davy Defaud - [GitLab](https://gitlab.com/DevDef), [CrowdIn](https://crowdin.com/profile/DevDef) - Galician diff --git a/doc/development/telemetry/usage_ping.md b/doc/development/telemetry/usage_ping.md index 9ead605548b..0f438e02772 100644 --- a/doc/development/telemetry/usage_ping.md +++ b/doc/development/telemetry/usage_ping.md @@ -322,7 +322,11 @@ Examples of query optimization work: When adding, changing, or updating metrics, please update the [Usage Statistics definition table](#usage-statistics-definitions). -### 5. Ask for a Telemetry Review +### 5. Add new metric to Versions Application + +Check if new metrics need to be added to the Versions Application. See `usage_data` [schema](https://gitlab.com/gitlab-services/version-gitlab-com/-/blob/master/db/schema.rb#L147) and usage data [parameters accepted](https://gitlab.com/gitlab-services/version-gitlab-com/-/blob/master/app/services/usage_ping.rb). Any metrics added under the `counts` key are saved in the `counts` column. + +### 6. Ask for a Telemetry Review On GitLab.com, we have DangerBot setup to monitor Telemetry related files and DangerBot will recommend a Telemetry review. Mention `@gitlab-org/growth/telemetry/engineers` in your MR for a review. diff --git a/doc/user/application_security/dast/index.md b/doc/user/application_security/dast/index.md index 65c4f9cfe35..256daae46d7 100644 --- a/doc/user/application_security/dast/index.md +++ b/doc/user/application_security/dast/index.md @@ -471,9 +471,6 @@ DAST can be [configured](#customizing-the-dast-settings) using environment varia | `DAST_INCLUDE_ALPHA_VULNERABILITIES` | no | Include alpha passive and active scan rules. Boolean. `true`, `True`, or `1` are considered as true value, otherwise false. Defaults to `false`. | | `DAST_USE_AJAX_SPIDER` | no | Use the AJAX spider in addition to the traditional spider, useful for crawling sites that require JavaScript. Boolean. `true`, `True`, or `1` are considered as true value, otherwise false. Defaults to `false`. | | `DAST_ZAP_CLI_OPTIONS` | no | ZAP server command-line options. For example, `-Xmx3072m` would set the Java maximum memory allocation pool size. | -| `DAST_ZAP_GENERATE_CONFIG` | no | The file name of the generated sample ZAP configuration file for use with `DAST_ZAP_CONFIG_FILE`. | -| `DAST_ZAP_CONFIG_FILE` | no | Name of configuration file used to determine thresholds of vulnerability rules. | -| `DAST_ZAP_CONFIG_URL` | no | URL of configuration file used to determine thresholds of vulnerability rules. | | `DAST_ZAP_LOG_CONFIGURATION` | no | Set to a semicolon-separated list of additional log4j properties for the ZAP Server. For example, `log4j.logger.org.parosproxy.paros.network.HttpSender=DEBUG` | ### DAST command-line options diff --git a/doc/user/project/operations/alert_management.md b/doc/user/project/operations/alert_management.md index 2c5593a8df2..411b36411af 100644 --- a/doc/user/project/operations/alert_management.md +++ b/doc/user/project/operations/alert_management.md @@ -38,12 +38,14 @@ immediately identify which alerts you should prioritize investigating: Alerts contain one of the following icons: -- **Critical**: **{severity-critical}** and hexadecimal color `#8b2615` -- **High**: **{severity-high}** and hexadecimal color `#c0341d` -- **Medium**: **{severity-medium}** and hexadecimal color `#fca429` -- **Low**: **{severity-low}** and hexadecimal color `#fdbc60` -- **Info**: **{severity-info}** and hexadecimal color `#418cd8` -- **Unknown**: **{severity-unknown}** and hexadecimal color `#bababa` +| Severity | Icon | Color (hexadecimal) | +|---|---|---| +| Critical | **{severity-critical}** | `#8b2615` | +| High | **{severity-high}** | `#c0341d` | +| Medium | **{severity-medium}** | `#fca429` | +| Low | **{severity-low}** | `#fdbc60` | +| Info | **{severity-info}** | `#418cd8` | +| Unknown | **{severity-unknown}** | `#bababa` | ## Alert Management list diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/client.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/client.rb index ddd1b91410b..bb0c18735bb 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/client.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/client.rb @@ -5,9 +5,6 @@ module Gitlab module DuplicateJobs class Client def call(worker_class, job, queue, _redis_pool, &block) - # We don't try to deduplicate jobs that are scheduled in the future - return yield if job['at'] - DuplicateJob.new(job, queue).schedule(&block) end end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb index fa742d07af2..0dc53c61e84 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb @@ -18,13 +18,13 @@ module Gitlab # When new jobs can be scheduled again, the strategy calls `#delete`. class DuplicateJob DUPLICATE_KEY_TTL = 6.hours + DEFAULT_STRATEGY = :until_executing attr_reader :existing_jid - def initialize(job, queue_name, strategy: :until_executing) + def initialize(job, queue_name) @job = job @queue_name = queue_name - @strategy = strategy end # This will continue the middleware chain if the job should be scheduled @@ -41,12 +41,12 @@ module Gitlab end # This method will return the jid that was set in redis - def check! + def check!(expiry = DUPLICATE_KEY_TTL) read_jid = nil Sidekiq.redis do |redis| redis.multi do |multi| - redis.set(idempotency_key, jid, ex: DUPLICATE_KEY_TTL, nx: true) + redis.set(idempotency_key, jid, ex: expiry, nx: true) read_jid = redis.get(idempotency_key) end end @@ -60,6 +60,10 @@ module Gitlab end end + def scheduled? + scheduled_at.present? + end + def duplicate? raise "Call `#check!` first to check for existing duplicates" unless existing_jid @@ -67,14 +71,36 @@ module Gitlab end def droppable? - idempotent? && duplicate? && ::Feature.disabled?("disable_#{queue_name}_deduplication") + idempotent? && ::Feature.disabled?("disable_#{queue_name}_deduplication") + end + + def scheduled_at + job['at'] + end + + def options + return {} unless worker_klass + return {} unless worker_klass.respond_to?(:get_deduplication_options) + + worker_klass.get_deduplication_options end private - attr_reader :queue_name, :strategy, :job + attr_reader :queue_name, :job attr_writer :existing_jid + def worker_klass + @worker_klass ||= worker_class_name.to_s.safe_constantize + end + + def strategy + return DEFAULT_STRATEGY unless worker_klass + return DEFAULT_STRATEGY unless worker_klass.respond_to?(:idempotent?) + + worker_klass.get_deduplicate_strategy + end + def worker_class_name job['class'] end @@ -104,11 +130,10 @@ module Gitlab end def idempotent? - worker_class = worker_class_name.to_s.safe_constantize - return false unless worker_class - return false unless worker_class.respond_to?(:idempotent?) + return false unless worker_klass + return false unless worker_klass.respond_to?(:idempotent?) - worker_class.idempotent? + worker_klass.idempotent? end end end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb index 674e436b714..0ed4912c4cc 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb @@ -13,13 +13,13 @@ module Gitlab end def schedule(job) - if duplicate_job.check! && duplicate_job.duplicate? + if deduplicatable_job? && check! && duplicate_job.duplicate? job['duplicate-of'] = duplicate_job.existing_jid - end - if duplicate_job.droppable? - Gitlab::SidekiqLogging::DeduplicationLogger.instance.log(job, "dropped until executing") - return false + if duplicate_job.droppable? + Gitlab::SidekiqLogging::DeduplicationLogger.instance.log(job, "dropped until executing") + return false + end end yield @@ -34,6 +34,22 @@ module Gitlab private attr_reader :duplicate_job + + def deduplicatable_job? + !duplicate_job.scheduled? || duplicate_job.options[:including_scheduled] + end + + def check! + duplicate_job.check!(expiry) + end + + def expiry + return DuplicateJob::DUPLICATE_KEY_TTL unless duplicate_job.scheduled? + + time_diff = duplicate_job.scheduled_at.to_i - Time.now.to_i + + time_diff > 0 ? time_diff : DuplicateJob::DUPLICATE_KEY_TTL + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index acf8bf59211..283e1a41bc8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -329,6 +329,9 @@ msgstr "" msgid "%{days} days until tags are automatically removed" msgstr "" +msgid "%{deployLinkStart}Use a template to deploy to ECS%{deployLinkEnd}, or use a docker image to %{commandsLinkStart}run AWS commands in GitLab CI/CD%{commandsLinkEnd}." +msgstr "" + msgid "%{description}- Sentry event: %{errorUrl}- First seen: %{firstSeen}- Last seen: %{lastSeen} %{countLabel}: %{count}%{userCountLabel}: %{userCount}" msgstr "" @@ -2176,6 +2179,9 @@ msgstr "" msgid "Amazon Web Services" msgstr "" +msgid "Amazon Web Services Logo" +msgstr "" + msgid "Amazon authentication is not %{link_start}correctly configured%{link_end}. Ask your GitLab administrator if you want to use this service." msgstr "" @@ -7572,6 +7578,9 @@ msgstr "" msgid "Deploying to" msgstr "" +msgid "Deploying to AWS is easy with GitLab" +msgstr "" + msgid "Deployment Frequency" msgstr "" @@ -13039,6 +13048,9 @@ msgstr "" msgid "Learn more about custom project templates" msgstr "" +msgid "Learn more about deploying to AWS" +msgstr "" + msgid "Learn more about deploying to a cluster" msgstr "" diff --git a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js index 9179302f786..094fdcdc185 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -105,6 +105,46 @@ describe('Ci variable modal', () => { }); }); + describe('Adding a new non-AWS variable', () => { + beforeEach(() => { + const [variable] = mockData.mockVariables; + const invalidKeyVariable = { + ...variable, + key: 'key', + value: 'value', + secret_value: 'secret_value', + }; + createComponent(mount); + store.state.variable = invalidKeyVariable; + }); + + it('does not show AWS guidance tip', () => { + const tip = wrapper.find(`div[data-testid='aws-guidance-tip']`); + expect(tip.exists()).toBe(true); + expect(tip.isVisible()).toBe(false); + }); + }); + + describe('Adding a new AWS variable', () => { + beforeEach(() => { + const [variable] = mockData.mockVariables; + const invalidKeyVariable = { + ...variable, + key: AWS_ACCESS_KEY_ID, + value: 'AKIAIOSFODNN7EXAMPLEjdhy', + secret_value: 'AKIAIOSFODNN7EXAMPLEjdhy', + }; + createComponent(mount); + store.state.variable = invalidKeyVariable; + }); + + it('shows AWS guidance tip', () => { + const tip = wrapper.find(`[data-testid='aws-guidance-tip']`); + expect(tip.exists()).toBe(true); + expect(tip.isVisible()).toBe(true); + }); + }); + describe('Editing a variable', () => { beforeEach(() => { const [variable] = mockData.mockVariables; diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb index 9c7f6638913..a1e4cbb1e31 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb @@ -31,14 +31,51 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::Client, :clean_gitlab_redis_q expect(job3['duplicate-of']).to eq(job1['jid']) end - it "does not mark a job that's scheduled in the future as a duplicate" do - TestDeduplicationWorker.perform_async('args1') - TestDeduplicationWorker.perform_at(1.day.from_now, 'args1') - TestDeduplicationWorker.perform_in(3.hours, 'args1') + context 'without scheduled deduplication' do + it "does not mark a job that's scheduled in the future as a duplicate" do + TestDeduplicationWorker.perform_async('args1') + TestDeduplicationWorker.perform_at(1.day.from_now, 'args1') + TestDeduplicationWorker.perform_in(3.hours, 'args1') - duplicates = TestDeduplicationWorker.jobs.map { |job| job['duplicate-of'] } + duplicates = TestDeduplicationWorker.jobs.map { |job| job['duplicate-of'] } - expect(duplicates).to all(be_nil) + expect(duplicates).to all(be_nil) + end + end + + context 'with scheduled deduplication' do + let(:scheduled_worker_class) do + Class.new do + def self.name + 'TestDeduplicationWorker' + end + + include ApplicationWorker + + deduplicate :until_executing, including_scheduled: true + + def perform(*args) + end + end + end + + before do + stub_const('TestDeduplicationWorker', scheduled_worker_class) + end + + it 'adds a correct duplicate tag to the jobs', :aggregate_failures do + TestDeduplicationWorker.perform_async('args1') + TestDeduplicationWorker.perform_at(1.day.from_now, 'args1') + TestDeduplicationWorker.perform_in(3.hours, 'args1') + TestDeduplicationWorker.perform_in(3.hours, 'args2') + + job1, job2, job3, job4 = TestDeduplicationWorker.jobs + + expect(job1['duplicate-of']).to be_nil + expect(job2['duplicate-of']).to eq(job1['jid']) + expect(job3['duplicate-of']).to eq(job1['jid']) + expect(job4['duplicate-of']).to be_nil + end end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb index 929df0a7ffb..13c86563be7 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb @@ -93,6 +93,25 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r end end + describe '#scheduled?' do + it 'returns false for non-scheduled jobs' do + expect(duplicate_job.scheduled?).to be(false) + end + + context 'scheduled jobs' do + let(:job) do + { 'class' => 'AuthorizedProjectsWorker', + 'args' => [1], + 'jid' => '123', + 'at' => 42 } + end + + it 'returns true' do + expect(duplicate_job.scheduled?).to be(true) + end + end + end + describe '#duplicate?' do it "raises an error if the check wasn't performed" do expect { duplicate_job.duplicate? }.to raise_error /Call `#check!` first/ @@ -112,28 +131,23 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r end end - describe 'droppable?' do - where(:idempotent, :duplicate, :prevent_deduplication) do - # [true, false].repeated_permutation(3) - [[true, true, true], - [true, true, false], - [true, false, true], - [true, false, false], - [false, true, true], - [false, true, false], - [false, false, true], - [false, false, false]] + describe '#droppable?' do + where(:idempotent, :prevent_deduplication) do + # [true, false].repeated_permutation(2) + [[true, true], + [true, false], + [false, true], + [false, false]] end with_them do before do allow(AuthorizedProjectsWorker).to receive(:idempotent?).and_return(idempotent) - allow(duplicate_job).to receive(:duplicate?).and_return(duplicate) stub_feature_flags("disable_#{queue}_deduplication" => prevent_deduplication) end it 'is droppable when all conditions are met' do - if idempotent && duplicate && !prevent_deduplication + if idempotent && !prevent_deduplication expect(duplicate_job).to be_droppable else expect(duplicate_job).not_to be_droppable @@ -142,6 +156,31 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gitlab_r end end + describe '#scheduled_at' do + let(:scheduled_at) { 42 } + let(:job) do + { 'class' => 'AuthorizedProjectsWorker', + 'args' => [1], + 'jid' => '123', + 'at' => scheduled_at } + end + + it 'returns when the job is scheduled at' do + expect(duplicate_job.scheduled_at).to eq(scheduled_at) + end + end + + describe '#options' do + let(:worker_options) { { foo: true } } + + it 'returns worker options' do + allow(AuthorizedProjectsWorker).to( + receive(:get_deduplication_options).and_return(worker_options)) + + expect(duplicate_job.options).to eq(worker_options) + end + end + def set_idempotency_key(key, value = '1') Sidekiq.redis { |r| r.set(key, value) } end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb index 31b51260ebd..eb8b0a951a8 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'fast_spec_helper' +require 'timecop' describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting do let(:fake_duplicate_job) do @@ -15,28 +16,90 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting do end it 'checks for duplicates before yielding' do - expect(fake_duplicate_job).to receive(:check!).ordered.and_return('a jid') + expect(fake_duplicate_job).to receive(:scheduled?).twice.ordered.and_return(false) + expect(fake_duplicate_job).to( + receive(:check!) + .with(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DUPLICATE_KEY_TTL) + .ordered + .and_return('a jid')) expect(fake_duplicate_job).to receive(:duplicate?).ordered.and_return(false) - expect(fake_duplicate_job).to receive(:droppable?).ordered.and_return(false) expect { |b| strategy.schedule({}, &b) }.to yield_control end - it 'adds the jid of the existing job to the job hash' do - allow(fake_duplicate_job).to receive(:check!).and_return('the jid') - allow(fake_duplicate_job).to receive(:droppable?).and_return(true) - job_hash = {} + it 'checks worker options for scheduled jobs' do + expect(fake_duplicate_job).to receive(:scheduled?).ordered.and_return(true) + expect(fake_duplicate_job).to receive(:options).ordered.and_return({}) + expect(fake_duplicate_job).not_to receive(:check!) - expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) - expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') + expect { |b| strategy.schedule({}, &b) }.to yield_control + end + + context 'job marking' do + it 'adds the jid of the existing job to the job hash' do + allow(fake_duplicate_job).to receive(:scheduled?).and_return(false) + allow(fake_duplicate_job).to receive(:check!).and_return('the jid') + allow(fake_duplicate_job).to receive(:droppable?).and_return(true) + job_hash = {} - strategy.schedule(job_hash) {} + expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) + expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') - expect(job_hash).to include('duplicate-of' => 'the jid') + strategy.schedule(job_hash) {} + + expect(job_hash).to include('duplicate-of' => 'the jid') + end + + context 'scheduled jobs' do + let(:time_diff) { 1.minute } + + context 'scheduled in the past' do + it 'adds the jid of the existing job to the job hash' do + allow(fake_duplicate_job).to receive(:scheduled?).twice.and_return(true) + allow(fake_duplicate_job).to receive(:scheduled_at).and_return(Time.now - time_diff) + allow(fake_duplicate_job).to receive(:options).and_return({ including_scheduled: true }) + allow(fake_duplicate_job).to( + receive(:check!) + .with(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DUPLICATE_KEY_TTL) + .and_return('the jid')) + allow(fake_duplicate_job).to receive(:droppable?).and_return(true) + job_hash = {} + + expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) + expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') + + strategy.schedule(job_hash) {} + + expect(job_hash).to include('duplicate-of' => 'the jid') + end + end + + context 'scheduled in the future' do + it 'adds the jid of the existing job to the job hash' do + Timecop.freeze do + allow(fake_duplicate_job).to receive(:scheduled?).twice.and_return(true) + allow(fake_duplicate_job).to receive(:scheduled_at).and_return(Time.now + time_diff) + allow(fake_duplicate_job).to receive(:options).and_return({ including_scheduled: true }) + allow(fake_duplicate_job).to( + receive(:check!).with(time_diff.to_i).and_return('the jid')) + allow(fake_duplicate_job).to receive(:droppable?).and_return(true) + job_hash = {} + + expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) + expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') + + strategy.schedule(job_hash) {} + + expect(job_hash).to include('duplicate-of' => 'the jid') + end + end + end + end end context "when the job is droppable" do before do + allow(fake_duplicate_job).to receive(:scheduled?).and_return(false) allow(fake_duplicate_job).to receive(:check!).and_return('the jid') allow(fake_duplicate_job).to receive(:duplicate?).and_return(true) allow(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') @@ -52,7 +115,7 @@ describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting do expect(schedule_result).to be(false) end - it 'logs that the job wass dropped' do + it 'logs that the job was dropped' do fake_logger = instance_double(Gitlab::SidekiqLogging::DeduplicationLogger) expect(Gitlab::SidekiqLogging::DeduplicationLogger).to receive(:instance).and_return(fake_logger) diff --git a/spec/migrations/20200526231421_update_index_approval_rule_name_for_code_owners_rule_type_spec.rb b/spec/migrations/20200526231421_update_index_approval_rule_name_for_code_owners_rule_type_spec.rb deleted file mode 100644 index 4aa912ef873..00000000000 --- a/spec/migrations/20200526231421_update_index_approval_rule_name_for_code_owners_rule_type_spec.rb +++ /dev/null @@ -1,142 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require Rails.root.join('db', 'migrate', '20200526231421_update_index_approval_rule_name_for_code_owners_rule_type.rb') - -describe UpdateIndexApprovalRuleNameForCodeOwnersRuleType do - let(:migration) { described_class.new } - - let(:approval_rules) { table(:approval_merge_request_rules) } - let(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab') } - - let(:project) do - table(:projects).create!( - namespace_id: namespace.id, - name: 'gitlab', - path: 'gitlab' - ) - end - - let(:merge_request) do - table(:merge_requests).create!( - target_project_id: project.id, - source_project_id: project.id, - target_branch: 'feature', - source_branch: 'master' - ) - end - - let(:index_names) do - ActiveRecord::Base.connection - .indexes(:approval_merge_request_rules) - .collect(&:name) - end - - def create_sectional_approval_rules - approval_rules.create!( - merge_request_id: merge_request.id, - name: "*.rb", - code_owner: true, - rule_type: 2, - section: "First Section" - ) - - approval_rules.create!( - merge_request_id: merge_request.id, - name: "*.rb", - code_owner: true, - rule_type: 2, - section: "Second Section" - ) - end - - def create_two_matching_nil_section_approval_rules - 2.times do - approval_rules.create!( - merge_request_id: merge_request.id, - name: "nil_section", - code_owner: true, - rule_type: 2 - ) - end - end - - before do - approval_rules.delete_all - end - - describe "#up" do - it "creates the new index and removes the 'legacy' indices" do - # Confirm that existing legacy indices prevent duplicate entries - # - expect { create_sectional_approval_rules } - .to raise_exception(ActiveRecord::RecordNotUnique) - expect { create_two_matching_nil_section_approval_rules } - .to raise_exception(ActiveRecord::RecordNotUnique) - - approval_rules.delete_all - - disable_migrations_output { migrate! } - - # After running the migration, expect `section == nil` rules to still - # be blocked by the legacy indices, but sectional rules are allowed. - # - expect { create_sectional_approval_rules } - .to change { approval_rules.count }.by(2) - expect { create_two_matching_nil_section_approval_rules } - .to raise_exception(ActiveRecord::RecordNotUnique) - - # Attempt to rerun the creation of sectional rules, and see that sectional - # rules are unique by section - # - expect { create_sectional_approval_rules } - .to raise_exception(ActiveRecord::RecordNotUnique) - - expect(index_names).to include( - described_class::SECTIONAL_INDEX_NAME, - described_class::LEGACY_INDEX_NAME_RULE_TYPE, - described_class::LEGACY_INDEX_NAME_CODE_OWNERS - ) - end - end - - describe "#down" do - it "recreates 'legacy' indices and removes duplicate code owner approval rules" do - disable_migrations_output { migrate! } - - expect { create_sectional_approval_rules } - .to change { approval_rules.count }.by(2) - expect { create_two_matching_nil_section_approval_rules } - .to raise_exception(ActiveRecord::RecordNotUnique) - - # Run the down migration. This will remove the 2 approval rules we create - # above, and call MergeRequests::SyncCodeOwnerApprovalRules to recreate - # new ones. - # - expect(MergeRequests::SyncCodeOwnerApprovalRules) - .to receive(:new).with(MergeRequest.find(merge_request.id)).once.and_call_original - - # We expect approval_rules.count to be changed by -2 as we're deleting the - # 3 rules created above, and MergeRequests::SyncCodeOwnerApprovalRules - # will not be able to create new one with an empty (or missing) - # CODEOWNERS file. - # - expect { disable_migrations_output { migration.down } } - .to change { approval_rules.count }.by(-3) - - # Test that the index does not allow us to create the same rules as the - # previous sectional index. - # - expect { create_sectional_approval_rules } - .to raise_exception(ActiveRecord::RecordNotUnique) - expect { create_two_matching_nil_section_approval_rules } - .to raise_exception(ActiveRecord::RecordNotUnique) - - expect(index_names).not_to include(described_class::SECTIONAL_INDEX_NAME) - expect(index_names).to include( - described_class::LEGACY_INDEX_NAME_RULE_TYPE, - described_class::LEGACY_INDEX_NAME_CODE_OWNERS - ) - end - end -end |