diff options
103 files changed, 1604 insertions, 427 deletions
@@ -423,7 +423,7 @@ group :development, :test do end group :development, :test, :danger do - gem 'gitlab-dangerfiles', '~> 3.6.2', require: false + gem 'gitlab-dangerfiles', '~> 3.6.3', require: false end group :development, :test, :coverage do diff --git a/Gemfile.checksum b/Gemfile.checksum index 5f15974fe93..7d259f352bf 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -200,9 +200,9 @@ {"name":"gettext_i18n_rails_js","version":"1.3.0","platform":"ruby","checksum":"5d10afe4be3639bff78c50a56768c20f39aecdabc580c08aa45573911c2bd687"}, {"name":"git","version":"1.11.0","platform":"ruby","checksum":"7e95ba4da8298a0373ef1a6862aa22007d761f3c8274b675aa787966fecea0f1"}, {"name":"gitaly","version":"15.5.0","platform":"ruby","checksum":"d85dd4890a1f0fd95f935c848bcedf03f19b78872f20f04b9811e602bea4ef42"}, -{"name":"gitlab","version":"4.16.1","platform":"ruby","checksum":"13fd7059cbdad5a1a21b15fa2cf9070b97d92e27f8c688581fe3d84dc038074f"}, +{"name":"gitlab","version":"4.19.0","platform":"ruby","checksum":"3f645e3e195dbc24f0834fbf83e8ccfb2056d8e9712b01a640aad418a6949679"}, {"name":"gitlab-chronic","version":"0.10.5","platform":"ruby","checksum":"f80f18dc699b708870a80685243331290bc10cfeedb6b99c92219722f729c875"}, -{"name":"gitlab-dangerfiles","version":"3.6.2","platform":"ruby","checksum":"88585532bbb5c0e862ad0776b3804a32129eab06c6a8a7bc96b577baa7aac6c5"}, +{"name":"gitlab-dangerfiles","version":"3.6.3","platform":"ruby","checksum":"c696893dcadd99cd1f2074a335085f64bd601ff5778f3c704c98fd6c5ba23e88"}, {"name":"gitlab-experiment","version":"0.7.1","platform":"ruby","checksum":"166dddb3aa83428bcaa93c35684ed01dc4d61f321fd2ae40b020806dc54a7824"}, {"name":"gitlab-fog-azure-rm","version":"1.4.0","platform":"ruby","checksum":"af4163c32b028aa5208814a3f4765a5817d50527e6c61931f766bf18a2e0eb7e"}, {"name":"gitlab-labkit","version":"0.28.0","platform":"ruby","checksum":"a7ebf52336566f7607d280056acd64f390c9991f152fc3d6b1dd966a372d5654"}, @@ -586,7 +586,7 @@ {"name":"telesignenterprise","version":"2.2.2","platform":"ruby","checksum":"f147a03263a8c2fe0a0db1a7a9454a6ee37d9e8abd58eaca305bdd8081f9f1b3"}, {"name":"temple","version":"0.8.2","platform":"ruby","checksum":"c12071214346c606dbd219b4117276d04a9f2c20d65e66a66b2c4ec18efc1f18"}, {"name":"term-ansicolor","version":"1.7.1","platform":"ruby","checksum":"92339ffec77c4bddc786a29385c91601dd52fc68feda23609bba0491229b05f7"}, -{"name":"terminal-table","version":"1.8.0","platform":"ruby","checksum":"13371f069af18e9baa4e44d404a4ada9301899ce0530c237ac1a96c19f652294"}, +{"name":"terminal-table","version":"3.0.2","platform":"ruby","checksum":"f951b6af5f3e00203fb290a669e0a85c5dd5b051b3b023392ccfd67ba5abae91"}, {"name":"terser","version":"1.0.2","platform":"ruby","checksum":"80c2e0bc7e2db4e12e8529658f9e0820e13d685ae67d745bf981f269743bb28e"}, {"name":"test-prof","version":"1.0.7","platform":"ruby","checksum":"7df2ece7acf4f14c52788abdfdefae689b4d1cd84530b2fe7cacadf89cfce0ed"}, {"name":"test_file_finder","version":"0.1.4","platform":"ruby","checksum":"bc36d8339eac4fb9dc36514a7c5f4d389ac2fb6d010716fc715c5c8fbb98eacd"}, diff --git a/Gemfile.lock b/Gemfile.lock index 97b8c995cb1..04df532ab89 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -556,12 +556,12 @@ GEM rchardet (~> 1.8) gitaly (15.5.0) grpc (~> 1.0) - gitlab (4.16.1) - httparty (~> 0.14, >= 0.14.0) - terminal-table (~> 1.5, >= 1.5.1) + gitlab (4.19.0) + httparty (~> 0.20) + terminal-table (>= 1.5.1) gitlab-chronic (0.10.5) numerizer (~> 0.2) - gitlab-dangerfiles (3.6.2) + gitlab-dangerfiles (3.6.3) danger (>= 8.4.5) danger-gitlab (>= 8.0.0) rake @@ -1442,8 +1442,8 @@ GEM temple (0.8.2) term-ansicolor (1.7.1) tins (~> 1.0) - terminal-table (1.8.0) - unicode-display_width (~> 1.1, >= 1.1.1) + terminal-table (3.0.2) + unicode-display_width (>= 1.1.1, < 3) terser (1.0.2) execjs (>= 0.3.0, < 3) test-prof (1.0.7) @@ -1664,7 +1664,7 @@ DEPENDENCIES gettext_i18n_rails_js (~> 1.3) gitaly (~> 15.5.0) gitlab-chronic (~> 0.10.5) - gitlab-dangerfiles (~> 3.6.2) + gitlab-dangerfiles (~> 3.6.3) gitlab-experiment (~> 0.7.1) gitlab-fog-azure-rm (~> 1.4.0) gitlab-labkit (~> 0.28.0) diff --git a/app/assets/javascripts/alert_management/components/alert_management_table.vue b/app/assets/javascripts/alert_management/components/alert_management_table.vue index c0cac958a42..fe18b9da560 100644 --- a/app/assets/javascripts/alert_management/components/alert_management_table.vue +++ b/app/assets/javascripts/alert_management/components/alert_management_table.vue @@ -312,6 +312,7 @@ export default { <template #table> <gl-table class="alert-management-table" + data-qa-selector="alert_table_container" :items=" alerts ? alerts.list diff --git a/app/assets/javascripts/alerts_settings/components/alerts_form.vue b/app/assets/javascripts/alerts_settings/components/alerts_form.vue index 388d925196b..a0d5cb7f4c3 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_form.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_form.vue @@ -83,7 +83,7 @@ export default { </p> <form ref="settingsForm" @submit.prevent="updateAlertsIntegrationSettings"> <gl-form-group class="gl-pl-0"> - <gl-form-checkbox v-model="createIssueEnabled" data-qa-selector="create_issue_checkbox"> + <gl-form-checkbox v-model="createIssueEnabled" data-qa-selector="create_incident_checkbox"> <span>{{ $options.i18n.createIncident.label }}</span> </gl-form-checkbox> </gl-form-group> diff --git a/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue b/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue index 03bc4b825ae..65c3bc732ed 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue @@ -430,6 +430,7 @@ export default { v-model="integrationForm.type" :disabled="isSelectDisabled" class="gl-max-w-full" + data-qa-selector="integration_type_dropdown" :options="integrationTypesOptions" /> @@ -461,6 +462,7 @@ export default { v-model="integrationForm.name" type="text" :placeholder="$options.i18n.integrationFormSteps.nameIntegration.placeholder" + data-qa-selector="integration_name_field" @input="validateName" /> </gl-form-group> @@ -483,6 +485,7 @@ export default { v-model="integrationForm.active" :is-loading="loading" :label="$options.i18n.integrationFormSteps.nameIntegration.activeToggle" + data-qa-selector="active_toggle_container" class="gl-mt-4 gl-font-weight-normal" /> </gl-form-group> @@ -594,6 +597,7 @@ export default { category="secondary" class="gl-ml-3 js-no-auto-disable" data-testid="integration-form-test-and-submit" + data-qa-selector="save_and_create_alert_button" @click="submit(true)" > {{ $options.i18n.saveAndTestIntegration }} @@ -695,6 +699,7 @@ export default { :debounce="$options.JSON_VALIDATE_DELAY" rows="6" max-rows="10" + data-qa-selector="test_payload_field" @input="validateJson(false)" /> </gl-form-group> @@ -706,6 +711,7 @@ export default { data-testid="send-test-alert" variant="confirm" class="js-no-auto-disable" + data-qa-selector="send_test_alert_button" @click="isFormDirty ? null : sendTestAlert()" > {{ $options.i18n.send }} diff --git a/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue b/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue index bf456b6adaa..010cb5721a1 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue @@ -375,6 +375,7 @@ export default { category="secondary" variant="confirm" data-testid="add-integration-btn" + data-qa-selector="add_integration_button" class="gl-mt-3" @click="setFormVisibility(true)" > diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index 5f35dbdc5e7..3c9c0b1ade1 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -7,6 +7,7 @@ import { EDITOR_TYPE_CODE, EDITOR_CODE_INSTANCE_FN, EDITOR_DIFF_INSTANCE_FN, + EXTENSION_CI_SCHEMA_FILE_NAME_MATCH, } from '~/editor/constants'; import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; import { EditorWebIdeExtension } from '~/editor/extensions/source_editor_webide_ext'; @@ -26,6 +27,7 @@ import { performanceMarkAndMeasure } from '~/performance/utils'; import ContentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue'; import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { leftSidebarViews, viewerTypes, @@ -53,6 +55,7 @@ export default { DiffViewer, FileTemplatesBar, }, + mixins: [glFeatureFlagMixin()], props: { file: { type: Object, @@ -145,6 +148,12 @@ export default { showTabs() { return !this.shouldHideEditor && this.isEditModeActive && this.previewMode; }, + isCiConfigFile() { + return ( + this.file.path === EXTENSION_CI_SCHEMA_FILE_NAME_MATCH && + this.editor?.getEditorType() === EDITOR_TYPE_CODE + ); + }, }, watch: { 'file.name': { @@ -232,8 +241,6 @@ export default { return; } - this.registerSchemaForFile(); - Promise.all([this.fetchFileData(), this.fetchEditorconfigRules()]) .then(() => { this.createEditorInstance(); @@ -357,6 +364,8 @@ export default { this.model.updateOptions(this.rules); + this.registerSchemaForFile(); + this.model.onChange((model) => { const { file } = model; if (!file.active) return; @@ -446,8 +455,33 @@ export default { return Promise.resolve(); }, registerSchemaForFile() { - const schema = this.getJsonSchemaForPath(this.file.path); - registerSchema(schema); + const registerExternalSchema = () => { + const schema = this.getJsonSchemaForPath(this.file.path); + return registerSchema(schema); + }; + const registerLocalSchema = async () => { + if (!this.CiSchemaExtension) { + const { CiSchemaExtension } = await import( + '~/editor/extensions/source_editor_ci_schema_ext' + ).catch((e) => + createAlert({ + message: e, + }), + ); + this.CiSchemaExtension = CiSchemaExtension; + } + this.editor.use({ definition: this.CiSchemaExtension }); + this.editor.registerCiSchema(); + }; + + if (this.isCiConfigFile && this.glFeatures.schemaLinting) { + registerLocalSchema(); + } else { + if (this.CiSchemaExtension) { + this.editor.unuse(this.CiSchemaExtension); + } + registerExternalSchema(); + } }, updateEditor(data) { // Looks like our model wrapper `.dispose` causes the monaco editor to emit some position changes after diff --git a/app/assets/javascripts/jobs/components/job/empty_state.vue b/app/assets/javascripts/jobs/components/job/empty_state.vue index 65b9600e664..053d5a4e740 100644 --- a/app/assets/javascripts/jobs/components/job/empty_state.vue +++ b/app/assets/javascripts/jobs/components/job/empty_state.vue @@ -20,6 +20,14 @@ export default { type: String, required: true, }, + isRetryable: { + type: Boolean, + required: false, + }, + jobId: { + type: Number, + required: true, + }, title: { type: String, required: true, @@ -54,8 +62,8 @@ export default { }, }, computed: { - isGraphQL() { - return this.glFeatures?.graphqlJobApp; + showGraphQLManualVariablesForm() { + return this.glFeatures?.graphqlJobApp && this.isRetryable; }, shouldRenderManualVariables() { return this.playable && !this.scheduled; @@ -77,14 +85,18 @@ export default { <p v-if="content" data-testid="job-empty-state-content">{{ content }}</p> </div> - <template v-if="isGraphQL"> - <manual-variables-form v-if="shouldRenderManualVariables" :action="action" /> + <template v-if="showGraphQLManualVariablesForm"> + <manual-variables-form + v-if="shouldRenderManualVariables" + :job-id="jobId" + @hideManualVariablesForm="$emit('hideManualVariablesForm')" + /> </template> <template v-else> <legacy-manual-variables-form v-if="shouldRenderManualVariables" :action="action" /> </template> - <div class="text-content"> - <div v-if="action && !shouldRenderManualVariables" class="text-center"> + <div v-if="action && !shouldRenderManualVariables" class="text-content"> + <div class="text-center"> <gl-link :href="action.path" :data-method="action.method" diff --git a/app/assets/javascripts/jobs/components/job/graphql/mutations/job_retry_with_variables.mutation.graphql b/app/assets/javascripts/jobs/components/job/graphql/mutations/job_retry_with_variables.mutation.graphql new file mode 100644 index 00000000000..2b79892a072 --- /dev/null +++ b/app/assets/javascripts/jobs/components/job/graphql/mutations/job_retry_with_variables.mutation.graphql @@ -0,0 +1,16 @@ +mutation retryJobWithVariables($id: CiBuildID!, $variables: [CiVariableInput!]) { + jobRetry(input: { id: $id, variables: $variables }) { + job { + id + manualVariables { + nodes { + id + key + value + } + } + webPath + } + errors + } +} diff --git a/app/assets/javascripts/jobs/components/job/graphql/queries/get_job.query.graphql b/app/assets/javascripts/jobs/components/job/graphql/queries/get_job.query.graphql new file mode 100644 index 00000000000..aaf1dec8e0f --- /dev/null +++ b/app/assets/javascripts/jobs/components/job/graphql/queries/get_job.query.graphql @@ -0,0 +1,17 @@ +query getJob($fullPath: ID!, $id: JobID!) { + project(fullPath: $fullPath) { + id + job(id: $id) { + id + manualJob + manualVariables { + nodes { + id + key + value + } + } + name + } + } +} diff --git a/app/assets/javascripts/jobs/components/job/job_app.vue b/app/assets/javascripts/jobs/components/job/job_app.vue index e5fbf77be1e..c6d900ef13e 100644 --- a/app/assets/javascripts/jobs/components/job/job_app.vue +++ b/app/assets/javascripts/jobs/components/job/job_app.vue @@ -72,6 +72,7 @@ export default { data() { return { searchResults: [], + showUpdateVariablesState: false, }; }, computed: { @@ -122,6 +123,10 @@ export default { return this.shouldRenderCalloutMessage && !this.hasUnmetPrerequisitesFailure; }, + isJobRetryable() { + return Boolean(this.job.retry_path); + }, + itemName() { return sprintf(__('Job %{jobName}'), { jobName: this.job.name }); }, @@ -169,10 +174,16 @@ export default { 'toggleScrollButtons', 'toggleScrollAnimation', ]), + onHideManualVariablesForm() { + this.showUpdateVariablesState = false; + }, onResize() { this.updateSidebar(); this.updateScroll(); }, + onUpdateVariables() { + this.showUpdateVariablesState = true; + }, updateSidebar() { const breakpoint = bp.getBreakpointSize(); if (breakpoint === 'xs' || breakpoint === 'sm') { @@ -272,14 +283,12 @@ export default { </div> <!-- job log --> <div - v-if="hasJobLog" + v-if="hasJobLog && !showUpdateVariablesState" class="build-log-container gl-relative" :class="{ 'gl-mt-3': !job.archived }" > <log-top-bar :class="{ - 'sidebar-expanded': isSidebarOpen, - 'sidebar-collapsed': !isSidebarOpen, 'has-archived-block': job.archived, }" :size="jobLogSize" @@ -300,14 +309,17 @@ export default { <!-- empty state --> <empty-state - v-if="!hasJobLog" + v-if="!hasJobLog || showUpdateVariablesState" :illustration-path="emptyStateIllustration.image" :illustration-size-class="emptyStateIllustration.size" + :is-retryable="isJobRetryable" + :job-id="job.id" :title="emptyStateTitle" :content="emptyStateIllustration.content" :action="emptyStateAction" :playable="job.playable" :scheduled="job.scheduled" + @hideManualVariablesForm="onHideManualVariablesForm()" /> <!-- EO empty state --> @@ -321,9 +333,9 @@ export default { 'right-sidebar-expanded': isSidebarOpen, 'right-sidebar-collapsed': !isSidebarOpen, }" - :erase-path="job.erase_path" :artifact-help-url="artifactHelpUrl" data-testid="job-sidebar" + @updateVariables="onUpdateVariables()" /> </div> </template> diff --git a/app/assets/javascripts/jobs/components/job/legacy_manual_variables_form.vue b/app/assets/javascripts/jobs/components/job/legacy_manual_variables_form.vue index 1898e02c94e..2b6b6f8e59e 100644 --- a/app/assets/javascripts/jobs/components/job/legacy_manual_variables_form.vue +++ b/app/assets/javascripts/jobs/components/job/legacy_manual_variables_form.vue @@ -6,6 +6,7 @@ import { GlButton, GlLink, GlSprintf, + GlTooltipDirective, } from '@gitlab/ui'; import { uniqueId } from 'lodash'; import { mapActions } from 'vuex'; @@ -13,7 +14,7 @@ import { helpPagePath } from '~/helpers/help_page_helper'; import { s__ } from '~/locale'; export default { - name: 'ManualVariablesForm', + name: 'LegacyManualVariablesForm', components: { GlFormInputGroup, GlInputGroupText, @@ -22,6 +23,9 @@ export default { GlLink, GlSprintf, }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { action: { type: Object, @@ -42,6 +46,7 @@ export default { value: 'value', }, i18n: { + clearInputs: s__('CiVariables|Clear inputs'), header: s__('CiVariables|Variables'), keyLabel: s__('CiVariables|Key'), valueLabel: s__('CiVariables|Value'), @@ -152,11 +157,13 @@ export default { <gl-button v-if="canRemove(index)" + v-gl-tooltip + :aria-label="$options.i18n.clearInputs" + :title="$options.i18n.clearInputs" class="gl-flex-grow-0 gl-flex-basis-0" category="tertiary" variant="danger" icon="clear" - :aria-label="__('Delete variable')" data-testid="delete-variable-btn" @click="deleteVariable(variable.id)" /> diff --git a/app/assets/javascripts/jobs/components/job/manual_variables_form.vue b/app/assets/javascripts/jobs/components/job/manual_variables_form.vue index 2f97301979c..e8edc7fc56f 100644 --- a/app/assets/javascripts/jobs/components/job/manual_variables_form.vue +++ b/app/assets/javascripts/jobs/components/job/manual_variables_form.vue @@ -5,15 +5,23 @@ import { GlFormInput, GlButton, GlLink, + GlLoadingIcon, GlSprintf, + GlTooltipDirective, } from '@gitlab/ui'; -import { uniqueId } from 'lodash'; -import { mapActions } from 'vuex'; +import { cloneDeep, uniqueId } from 'lodash'; +import { fetchPolicies } from '~/lib/graphql'; +import { createAlert } from '~/flash'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { JOB_GRAPHQL_ERRORS, GRAPHQL_ID_TYPES } from '~/jobs/constants'; import { helpPagePath } from '~/helpers/help_page_helper'; +import { redirectTo } from '~/lib/utils/url_utility'; import { s__ } from '~/locale'; +import GetJob from './graphql/queries/get_job.query.graphql'; +import retryJobWithVariablesMutation from './graphql/mutations/job_retry_with_variables.mutation.graphql'; // This component is a port of ~/jobs/components/job/legacy_manual_variables_form.vue -// It is meant to fetch the job information via GraphQL instead of REST API. +// It is meant to fetch/update the job information via GraphQL instead of REST API. export default { name: 'ManualVariablesForm', @@ -23,63 +31,73 @@ export default { GlFormInput, GlButton, GlLink, + GlLoadingIcon, GlSprintf, }, - props: { - action: { - type: Object, - required: false, - default: null, - validator(value) { - return ( - value === null || - (Object.prototype.hasOwnProperty.call(value, 'path') && - Object.prototype.hasOwnProperty.call(value, 'method') && - Object.prototype.hasOwnProperty.call(value, 'button_title')) - ); + directives: { + GlTooltip: GlTooltipDirective, + }, + inject: ['projectPath'], + apollo: { + variables: { + query: GetJob, + variables() { + return { + fullPath: this.projectPath, + id: convertToGraphQLId(GRAPHQL_ID_TYPES.commitStatus, this.jobId), + }; + }, + fetchPolicy: fetchPolicies.CACHE_AND_NETWORK, + update(data) { + const jobVariables = cloneDeep(data?.project?.job?.manualVariables?.nodes); + return [...jobVariables.reverse(), ...this.variables]; + }, + error() { + createAlert({ message: JOB_GRAPHQL_ERRORS.jobQueryErrorText }); }, }, }, + props: { + jobId: { + type: Number, + required: true, + }, + }, inputTypes: { key: 'key', value: 'value', }, i18n: { + clearInputs: s__('CiVariables|Clear inputs'), + formHelpText: s__( + 'CiVariables|Specify variable values to be used in this run. The values specified in %{linkStart}CI/CD settings%{linkEnd} will be used as default', + ), header: s__('CiVariables|Variables'), keyLabel: s__('CiVariables|Key'), - valueLabel: s__('CiVariables|Value'), keyPlaceholder: s__('CiVariables|Input variable key'), + runAgainButtonText: s__('CiVariables|Run job again'), + valueLabel: s__('CiVariables|Value'), valuePlaceholder: s__('CiVariables|Input variable value'), - formHelpText: s__( - 'CiVariables|Specify variable values to be used in this run. The values specified in %{linkStart}CI/CD settings%{linkEnd} will be used as default', - ), }, data() { return { + job: {}, variables: [ { - key: '', - secretValue: '', id: uniqueId(), + key: '', + value: '', }, ], - triggerBtnDisabled: false, + runAgainBtnDisabled: false, }; }, computed: { variableSettings() { return helpPagePath('ci/variables/index', { anchor: 'add-a-cicd-variable-to-a-project' }); }, - preparedVariables() { - // we need to ensure no empty variables are passed to the API - // and secretValue should be snake_case when passed to the API - return this.variables - .filter((variable) => variable.key !== '') - .map(({ key, secretValue }) => ({ key, secret_value: secretValue })); - }, }, methods: { - ...mapActions(['triggerManualJob']), addEmptyVariable() { const lastVar = this.variables[this.variables.length - 1]; @@ -88,9 +106,9 @@ export default { } this.variables.push({ - key: '', - secret_value: '', id: uniqueId(), + key: '', + value: '', }); }, canRemove(index) { @@ -105,16 +123,45 @@ export default { inputRef(type, id) { return `${this.$options.inputTypes[type]}-${id}`; }, - trigger() { - this.triggerBtnDisabled = true; + navigateToRetriedJob(retryPath) { + redirectTo(retryPath); + }, + async retryJob() { + try { + // filtering out 'id' along with empty variables to send only key, value in the mutation. + // This will be removed in: https://gitlab.com/gitlab-org/gitlab/-/issues/377268 + const preparedVariables = this.variables + .filter((variable) => variable.key !== '') + .map(({ key, value }) => ({ key, value })); - this.triggerManualJob(this.preparedVariables); + const { data } = await this.$apollo.mutate({ + mutation: retryJobWithVariablesMutation, + variables: { + id: convertToGraphQLId(GRAPHQL_ID_TYPES.ciBuild, this.jobId), + // we need to ensure no empty variables are passed to the API + variables: preparedVariables, + }, + }); + if (data.jobRetry?.errors?.length) { + createAlert({ message: data.jobRetry.errors[0] }); + } else { + this.navigateToRetriedJob(data.jobRetry?.job?.webPath); + } + } catch (error) { + createAlert({ message: JOB_GRAPHQL_ERRORS.retryMutationErrorText }); + } + }, + runAgain() { + this.runAgainBtnDisabled = true; + + this.retryJob(); }, }, }; </script> <template> - <div class="row gl-justify-content-center"> + <gl-loading-icon v-if="$apollo.queries.variables.loading" class="gl-mt-9" size="lg" /> + <div v-else class="row gl-justify-content-center"> <div class="col-10" data-testid="manual-vars-form"> <label>{{ $options.i18n.header }}</label> @@ -147,7 +194,7 @@ export default { </template> <gl-form-input :ref="inputRef('value', variable.id)" - v-model="variable.secretValue" + v-model="variable.value" :placeholder="$options.i18n.valuePlaceholder" data-testid="ci-variable-value" /> @@ -155,11 +202,13 @@ export default { <gl-button v-if="canRemove(index)" + v-gl-tooltip + :aria-label="$options.i18n.clearInputs" + :title="$options.i18n.clearInputs" class="gl-flex-grow-0 gl-flex-basis-0" category="tertiary" variant="danger" icon="clear" - :aria-label="__('Delete variable')" data-testid="delete-variable-btn" @click="deleteVariable(variable.id)" /> @@ -180,14 +229,21 @@ export default { <div class="gl-display-flex gl-justify-content-center gl-mt-5"> <gl-button class="gl-mt-5" + :aria-label="__('Cancel')" + data-testid="cancel-btn" + @click="$emit('hideManualVariablesForm')" + >{{ __('Cancel') }}</gl-button + > + <gl-button + class="gl-mt-5" variant="confirm" category="primary" - :aria-label="__('Trigger manual job')" - :disabled="triggerBtnDisabled" - data-testid="trigger-manual-job-btn" - @click="trigger" + :aria-label="__('Run manual job again')" + :disabled="runAgainBtnDisabled" + data-testid="run-manual-job-btn" + @click="runAgain" > - {{ action.button_title }} + {{ $options.i18n.runAgainButtonText }} </gl-button> </div> </div> diff --git a/app/assets/javascripts/jobs/components/job/sidebar/job_sidebar_retry_button.vue b/app/assets/javascripts/jobs/components/job/sidebar/job_sidebar_retry_button.vue index dd620977f0c..65175df555a 100644 --- a/app/assets/javascripts/jobs/components/job/sidebar/job_sidebar_retry_button.vue +++ b/app/assets/javascripts/jobs/components/job/sidebar/job_sidebar_retry_button.vue @@ -1,19 +1,23 @@ <script> -import { GlButton, GlModalDirective } from '@gitlab/ui'; +import { GlButton, GlDropdown, GlDropdownItem, GlModalDirective } from '@gitlab/ui'; import { mapGetters } from 'vuex'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { JOB_SIDEBAR_COPY } from '~/jobs/constants'; export default { name: 'JobSidebarRetryButton', i18n: { - retryLabel: JOB_SIDEBAR_COPY.retry, + ...JOB_SIDEBAR_COPY, }, components: { GlButton, + GlDropdown, + GlDropdownItem, }, directives: { GlModal: GlModalDirective, }, + mixins: [glFeatureFlagsMixin()], props: { modalId: { type: String, @@ -23,9 +27,16 @@ export default { type: String, required: true, }, + isManualJob: { + type: Boolean, + required: true, + }, }, computed: { ...mapGetters(['hasForwardDeploymentFailure']), + showRetryDropdown() { + return this.glFeatures?.graphqlJobApp && this.isManualJob; + }, }, }; </script> @@ -33,17 +44,30 @@ export default { <gl-button v-if="hasForwardDeploymentFailure" v-gl-modal="modalId" - :aria-label="$options.i18n.retryLabel" + :aria-label="$options.i18n.retryJobLabel" category="primary" variant="confirm" icon="retry" data-testid="retry-job-button" /> - + <gl-dropdown + v-else-if="showRetryDropdown" + icon="retry" + category="primary" + :right="true" + variant="confirm" + > + <gl-dropdown-item :href="href" data-method="post"> + {{ $options.i18n.runAgainJobButtonLabel }} + </gl-dropdown-item> + <gl-dropdown-item @click="$emit('updateVariablesClicked')"> + {{ $options.i18n.updateVariables }} + </gl-dropdown-item> + </gl-dropdown> <gl-button v-else :href="href" - :aria-label="$options.i18n.retryLabel" + :aria-label="$options.i18n.retryJobLabel" category="primary" variant="confirm" icon="retry" diff --git a/app/assets/javascripts/jobs/components/job/sidebar/legacy_job_sidebar_retry_button.vue b/app/assets/javascripts/jobs/components/job/sidebar/legacy_job_sidebar_retry_button.vue new file mode 100644 index 00000000000..8a821b69f8c --- /dev/null +++ b/app/assets/javascripts/jobs/components/job/sidebar/legacy_job_sidebar_retry_button.vue @@ -0,0 +1,53 @@ +<script> +import { GlButton, GlModalDirective } from '@gitlab/ui'; +import { mapGetters } from 'vuex'; +import { JOB_SIDEBAR_COPY } from '~/jobs/constants'; + +export default { + name: 'LegacyJobSidebarRetryButton', + i18n: { + retryLabel: JOB_SIDEBAR_COPY.retryJobLabel, + }, + components: { + GlButton, + }, + directives: { + GlModal: GlModalDirective, + }, + props: { + modalId: { + type: String, + required: true, + }, + href: { + type: String, + required: true, + }, + }, + computed: { + ...mapGetters(['hasForwardDeploymentFailure']), + }, +}; +</script> +<template> + <gl-button + v-if="hasForwardDeploymentFailure" + v-gl-modal="modalId" + :aria-label="$options.i18n.retryLabel" + category="primary" + variant="confirm" + icon="retry" + data-testid="retry-job-button" + /> + + <gl-button + v-else + :href="href" + :aria-label="$options.i18n.retryLabel" + category="primary" + variant="confirm" + icon="retry" + data-method="post" + data-testid="retry-job-link" + /> +</template> diff --git a/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue b/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue index 64b497c3550..5bbb831a293 100644 --- a/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue +++ b/app/assets/javascripts/jobs/components/job/sidebar/legacy_sidebar_header.vue @@ -2,7 +2,7 @@ import { GlButton, GlTooltipDirective } from '@gitlab/ui'; import { mapActions } from 'vuex'; import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue'; -import { JOB_SIDEBAR_COPY, forwardDeploymentFailureModalId } from '~/jobs/constants'; +import { JOB_SIDEBAR_COPY, forwardDeploymentFailureModalId, PASSED_STATUS } from '~/jobs/constants'; import JobSidebarRetryButton from './job_sidebar_retry_button.vue'; export default { @@ -25,20 +25,15 @@ export default { required: true, default: () => ({}), }, - erasePath: { - type: String, - required: false, - default: null, - }, }, computed: { retryButtonCategory() { return this.job.status && this.job.recoverable ? 'primary' : 'secondary'; }, buttonTitle() { - return this.job.status && this.job.status.text === 'passed' + return this.job.status && this.job.status.text === PASSED_STATUS ? this.$options.i18n.runAgainJobButtonLabel - : this.$options.i18n.retryJobButtonLabel; + : this.$options.i18n.retryJobLabel; }, }, methods: { @@ -50,17 +45,15 @@ export default { <template> <div class="gl-py-5 gl-display-flex gl-align-items-center"> <tooltip-on-truncate :title="job.name" truncate-target="child" - ><h4 class="gl-my-0 gl-mr-3 gl-text-truncate"> - {{ job.name }} - </h4> + ><h4 class="gl-my-0 gl-mr-3 gl-text-truncate">{{ job.name }}</h4> </tooltip-on-truncate> <div class="gl-flex-grow-1 gl-flex-shrink-0 gl-text-right"> <gl-button - v-if="erasePath" + v-if="job.erase_path" v-gl-tooltip.left :title="$options.i18n.eraseLogButtonLabel" :aria-label="$options.i18n.eraseLogButtonLabel" - :href="erasePath" + :href="job.erase_path" :data-confirm="$options.i18n.eraseLogConfirmText" class="gl-mr-2" data-testid="job-log-erase-link" @@ -76,6 +69,7 @@ export default { :category="retryButtonCategory" :href="job.retry_path" :modal-id="$options.forwardDeploymentFailureModalId" + :is-manual-job="false" variant="confirm" data-qa-selector="retry_button" data-testid="retry-button" diff --git a/app/assets/javascripts/jobs/components/job/sidebar/sidebar.vue b/app/assets/javascripts/jobs/components/job/sidebar/sidebar.vue index aac6a0ad6d3..02c3d60557b 100644 --- a/app/assets/javascripts/jobs/components/job/sidebar/sidebar.vue +++ b/app/assets/javascripts/jobs/components/job/sidebar/sidebar.vue @@ -2,13 +2,13 @@ import { GlButton, GlIcon } from '@gitlab/ui'; import { isEmpty } from 'lodash'; import { mapActions, mapGetters, mapState } from 'vuex'; -import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { JOB_SIDEBAR_COPY, forwardDeploymentFailureModalId } from '~/jobs/constants'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import ArtifactsBlock from './artifacts_block.vue'; import CommitBlock from './commit_block.vue'; import JobsContainer from './jobs_container.vue'; import JobRetryForwardDeploymentModal from './job_retry_forward_deployment_modal.vue'; import JobSidebarDetailsContainer from './sidebar_job_details_container.vue'; -import ArtifactsBlock from './artifacts_block.vue'; import LegacySidebarHeader from './legacy_sidebar_header.vue'; import SidebarHeader from './sidebar_header.vue'; import StagesDropdown from './stages_dropdown.vue'; @@ -41,11 +41,6 @@ export default { required: false, default: '', }, - erasePath: { - type: String, - required: false, - default: null, - }, }, computed: { ...mapGetters(['hasForwardDeploymentFailure']), @@ -89,8 +84,13 @@ export default { <aside class="right-sidebar build-sidebar" data-offset-top="101" data-spy="affix"> <div class="sidebar-container"> <div class="blocks-container"> - <sidebar-header v-if="isGraphQL" :erase-path="erasePath" :job="job" /> - <legacy-sidebar-header v-else :erase-path="erasePath" :job="job" /> + <sidebar-header + v-if="isGraphQL" + :rest-job="job" + :job-id="job.id" + @updateVariables="$emit('updateVariables')" + /> + <legacy-sidebar-header v-else :job="job" /> <div v-if="job.terminal_path || job.new_issue_path" class="gl-py-5" diff --git a/app/assets/javascripts/jobs/components/job/sidebar/sidebar_header.vue b/app/assets/javascripts/jobs/components/job/sidebar/sidebar_header.vue index 523710598bf..c124f52ae79 100644 --- a/app/assets/javascripts/jobs/components/job/sidebar/sidebar_header.vue +++ b/app/assets/javascripts/jobs/components/job/sidebar/sidebar_header.vue @@ -1,8 +1,17 @@ <script> import { GlButton, GlTooltipDirective } from '@gitlab/ui'; import { mapActions } from 'vuex'; +import { createAlert } from '~/flash'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue'; -import { JOB_SIDEBAR_COPY, forwardDeploymentFailureModalId } from '~/jobs/constants'; +import { + JOB_GRAPHQL_ERRORS, + GRAPHQL_ID_TYPES, + JOB_SIDEBAR_COPY, + forwardDeploymentFailureModalId, + PASSED_STATUS, +} from '~/jobs/constants'; +import GetJob from '../graphql/queries/get_job.query.graphql'; import JobSidebarRetryButton from './job_sidebar_retry_button.vue'; // This component is a port of ~/jobs/components/job/sidebar/legacy_sidebar_header.vue @@ -22,21 +31,58 @@ export default { JobSidebarRetryButton, TooltipOnTruncate, }, - props: { + inject: ['projectPath'], + apollo: { job: { + query: GetJob, + variables() { + return { + fullPath: this.projectPath, + id: convertToGraphQLId(GRAPHQL_ID_TYPES.commitStatus, this.jobId), + }; + }, + update(data) { + const { name, manualJob } = data?.project?.job || {}; + return { + name, + manualJob, + }; + }, + error() { + createAlert({ message: JOB_GRAPHQL_ERRORS.jobQueryErrorText }); + }, + }, + }, + props: { + jobId: { + type: Number, + required: true, + }, + restJob: { type: Object, required: true, default: () => ({}), }, - erasePath: { - type: String, - required: false, - default: null, - }, + }, + data() { + return { + job: {}, + }; }, computed: { + buttonTitle() { + return this.restJob.status?.text === PASSED_STATUS + ? this.$options.i18n.runAgainJobButtonLabel + : this.$options.i18n.retryJobLabel; + }, + canShowJobRetryButton() { + return this.restJob.retry_path && !this.$apollo.queries.job.loading; + }, + isManualJob() { + return this.job?.manualJob; + }, retryButtonCategory() { - return this.job.status && this.job.recoverable ? 'primary' : 'secondary'; + return this.restJob.status && this.restJob.recoverable ? 'primary' : 'secondary'; }, }, methods: { @@ -48,17 +94,15 @@ export default { <template> <div class="gl-py-5 gl-display-flex gl-align-items-center"> <tooltip-on-truncate :title="job.name" truncate-target="child" - ><h4 class="gl-my-0 gl-mr-3 gl-text-truncate"> - {{ job.name }} - </h4> + ><h4 class="gl-my-0 gl-mr-3 gl-text-truncate" data-testid="job-name">{{ job.name }}</h4> </tooltip-on-truncate> <div class="gl-flex-grow-1 gl-flex-shrink-0 gl-text-right"> <gl-button - v-if="erasePath" + v-if="restJob.erase_path" v-gl-tooltip.left :title="$options.i18n.eraseLogButtonLabel" :aria-label="$options.i18n.eraseLogButtonLabel" - :href="erasePath" + :href="restJob.erase_path" :data-confirm="$options.i18n.eraseLogConfirmText" class="gl-mr-2" data-testid="job-log-erase-link" @@ -67,23 +111,25 @@ export default { icon="remove" /> <job-sidebar-retry-button - v-if="job.retry_path" + v-if="canShowJobRetryButton" v-gl-tooltip.left - :title="$options.i18n.retryJobButtonLabel" - :aria-label="$options.i18n.retryJobButtonLabel" + :title="buttonTitle" + :aria-label="buttonTitle" + :is-manual-job="isManualJob" :category="retryButtonCategory" - :href="job.retry_path" + :href="restJob.retry_path" :modal-id="$options.forwardDeploymentFailureModalId" variant="confirm" data-qa-selector="retry_button" data-testid="retry-button" + @updateVariablesClicked="$emit('updateVariables')" /> <gl-button - v-if="job.cancel_path" + v-if="restJob.cancel_path" v-gl-tooltip.left :title="$options.i18n.cancelJobButtonLabel" :aria-label="$options.i18n.cancelJobButtonLabel" - :href="job.cancel_path" + :href="restJob.cancel_path" variant="danger" icon="cancel" data-method="post" diff --git a/app/assets/javascripts/jobs/constants.js b/app/assets/javascripts/jobs/constants.js index e9475994e8b..405aea11181 100644 --- a/app/assets/javascripts/jobs/constants.js +++ b/app/assets/javascripts/jobs/constants.js @@ -5,6 +5,11 @@ const moreInfo = __('More information'); export const forwardDeploymentFailureModalId = 'forward-deployment-failure'; +export const GRAPHQL_ID_TYPES = { + commitStatus: 'CommitStatus', + ciBuild: 'Ci::Build', +}; + export const JOB_SIDEBAR_COPY = { cancel, cancelJobButtonLabel: s__('Job|Cancel'), @@ -12,10 +17,15 @@ export const JOB_SIDEBAR_COPY = { eraseLogButtonLabel: s__('Job|Erase job log and artifacts'), eraseLogConfirmText: s__('Job|Are you sure you want to erase this job log and artifacts?'), newIssue: __('New issue'), - retry: __('Retry'), - retryJobButtonLabel: s__('Job|Retry'), + retryJobLabel: s__('Job|Retry'), toggleSidebar: __('Toggle Sidebar'), runAgainJobButtonLabel: s__('Job|Run again'), + updateVariables: s__('Job|Update CI/CD variables'), +}; + +export const JOB_GRAPHQL_ERRORS = { + retryMutationErrorText: __('There was an error running the job. Please try again.'), + jobQueryErrorText: __('There was an error fetching the job.'), }; export const JOB_RETRY_FORWARD_DEPLOYMENT_MODAL = { @@ -31,3 +41,4 @@ export const JOB_RETRY_FORWARD_DEPLOYMENT_MODAL = { }; export const SUCCESS_STATUS = 'SUCCESS'; +export const PASSED_STATUS = 'passed'; diff --git a/app/assets/javascripts/jobs/index.js b/app/assets/javascripts/jobs/index.js index 9dd47f4046c..44bb1ffb1bc 100644 --- a/app/assets/javascripts/jobs/index.js +++ b/app/assets/javascripts/jobs/index.js @@ -1,10 +1,17 @@ import { GlToast } from '@gitlab/ui'; import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; import JobApp from './components/job/job_app.vue'; import createStore from './store'; +Vue.use(VueApollo); Vue.use(GlToast); +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), +}); + const initializeJobPage = (element) => { const store = createStore(); @@ -26,11 +33,13 @@ const initializeJobPage = (element) => { return new Vue({ el: element, + apolloProvider, store, components: { JobApp, }, provide: { + projectPath, retryOutdatedJobDocsUrl, }, render(createElement) { diff --git a/app/assets/javascripts/lib/dompurify.js b/app/assets/javascripts/lib/dompurify.js index 27760e483aa..5372f6555d2 100644 --- a/app/assets/javascripts/lib/dompurify.js +++ b/app/assets/javascripts/lib/dompurify.js @@ -18,7 +18,7 @@ export const defaultConfig = { 'data-disable', 'data-turbo', ], - FORBID_TAGS: ['style', 'mstyle'], + FORBID_TAGS: ['style', 'mstyle', 'form'], ALLOW_UNKNOWN_PROTOCOLS: true, }; diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 19318d87731..dd24e3fcb5d 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -338,11 +338,6 @@ color: $gl-text-color; } -.commit .gpg-popover-help-link { - display: block; - color: $link-color; -} - .add-review-item { .gl-tab-nav-item { height: 100%; diff --git a/app/finders/git_refs_finder.rb b/app/finders/git_refs_finder.rb index dbe0060d8ae..0492dd9934f 100644 --- a/app/finders/git_refs_finder.rb +++ b/app/finders/git_refs_finder.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class GitRefsFinder + include Gitlab::Utils::StrongMemoize + def initialize(repository, params = {}) @repository = repository @params = params @@ -10,44 +12,28 @@ class GitRefsFinder attr_reader :repository, :params - def search - @params[:search].to_s.presence - end - - def sort - @params[:sort].to_s.presence || 'name' - end - def by_search(refs) return refs unless search - case search - when ->(v) { v.starts_with?('^') } - filter_refs_with_prefix(refs, search.slice(1..-1)) - when ->(v) { v.ends_with?('$') } - filter_refs_with_suffix(refs, search.chop) - else - matches = filter_refs_by_name(refs, search) - set_exact_match_as_first_result(matches, search) - end - end - - def filter_refs_with_prefix(refs, prefix) - prefix = prefix.downcase + matches = filter_refs(refs, search) + return matches if regex_search? - refs.select { |ref| ref.name.downcase.starts_with?(prefix) } + set_exact_match_as_first_result(matches, search) end - def filter_refs_with_suffix(refs, suffix) - suffix = suffix.downcase - - refs.select { |ref| ref.name.downcase.ends_with?(suffix) } + def search + @params[:search].to_s.presence end + strong_memoize_attr :search - def filter_refs_by_name(refs, term) - term = term.downcase + def sort + @params[:sort].to_s.presence || 'name' + end - refs.select { |ref| ref.name.downcase.include?(term) } + def filter_refs(refs, term) + regex_string = Regexp.quote(term.downcase) + regex_string = unescape_regex_operators(regex_string) if regex_search? + refs.select { |ref| /#{regex_string}/ === ref.name.downcase } end def set_exact_match_as_first_result(matches, term) @@ -59,4 +45,13 @@ class GitRefsFinder def find_exact_match_index(matches, term) matches.index { |ref| ref.name.casecmp(term) == 0 } end + + def regex_search? + Regexp.union('^', '$', '*') === search + end + strong_memoize_attr :regex_search?, :regex_search + + def unescape_regex_operators(regex_string) + regex_string.sub('\^', '^').gsub('\*', '.*?').sub('\$', '$') + end end diff --git a/app/graphql/types/commit_signatures/gpg_signature_type.rb b/app/graphql/types/commit_signatures/gpg_signature_type.rb index 2a845fff3e2..3baf2d9d21d 100644 --- a/app/graphql/types/commit_signatures/gpg_signature_type.rb +++ b/app/graphql/types/commit_signatures/gpg_signature_type.rb @@ -11,6 +11,7 @@ module Types authorize :download_code field :user, Types::UserType, null: true, + method: :signed_by_user, description: 'User associated with the key.' field :gpg_key_user_name, GraphQL::Types::String, diff --git a/app/graphql/types/commit_signatures/x509_signature_type.rb b/app/graphql/types/commit_signatures/x509_signature_type.rb index 9ac96dbc015..2d58c3d5b5d 100644 --- a/app/graphql/types/commit_signatures/x509_signature_type.rb +++ b/app/graphql/types/commit_signatures/x509_signature_type.rb @@ -11,6 +11,7 @@ module Types authorize :download_code field :user, Types::UserType, null: true, + method: :signed_by_user, calls_gitaly: true, description: 'User associated with the key.' diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index e05adc5cd0e..6868fed711e 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -227,7 +227,7 @@ module DiffHelper end def conflicts(allow_tree_conflicts: false) - return unless merge_request.cannot_be_merged? + return unless merge_request.cannot_be_merged? && merge_request.source_branch_exists? && merge_request.target_branch_exists? conflicts_service = MergeRequests::Conflicts::ListService.new(merge_request, allow_tree_conflicts: allow_tree_conflicts) # rubocop:disable CodeReuse/ServiceClass diff --git a/app/helpers/x509_helper.rb b/app/helpers/x509_helper.rb index 1a9dbefceef..599be0b91f7 100644 --- a/app/helpers/x509_helper.rb +++ b/app/helpers/x509_helper.rb @@ -16,8 +16,4 @@ module X509Helper rescue StandardError {} end - - def x509_signature?(sig) - sig.is_a?(CommitSignatures::X509CommitSignature) || sig.is_a?(Gitlab::X509::Signature) - end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 54de45ebba7..5175842e5de 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -359,6 +359,10 @@ class Commit end def has_signature? + if signature_type == :SSH && !ssh_signatures_enabled? + return false + end + signature_type && signature_type != :NONE end @@ -378,6 +382,10 @@ class Commit @signature_type ||= raw_signature_type || :NONE end + def ssh_signatures_enabled? + Feature.enabled?(:ssh_commit_signatures, project) + end + def signature strong_memoize(:signature) do case signature_type @@ -385,6 +393,8 @@ class Commit gpg_commit.signature when :X509 Gitlab::X509::Commit.new(self).signature + when :SSH + Gitlab::Ssh::Commit.new(self).signature if ssh_signatures_enabled? else nil end diff --git a/app/models/commit_signatures/gpg_signature.rb b/app/models/commit_signatures/gpg_signature.rb index 2ae59853520..a9e8ca2dd33 100644 --- a/app/models/commit_signatures/gpg_signature.rb +++ b/app/models/commit_signatures/gpg_signature.rb @@ -2,6 +2,7 @@ module CommitSignatures class GpgSignature < ApplicationRecord include CommitSignature + include SignatureType sha_attribute :gpg_key_primary_keyid @@ -10,6 +11,14 @@ module CommitSignatures validates :gpg_key_primary_keyid, presence: true + def signed_by_user + gpg_key&.user + end + + def type + :gpg + end + def self.with_key_and_subkeys(gpg_key) subkey_ids = gpg_key.subkeys.pluck(:id) diff --git a/app/models/commit_signatures/ssh_signature.rb b/app/models/commit_signatures/ssh_signature.rb index 7a8d0653fcd..1e64e2b2978 100644 --- a/app/models/commit_signatures/ssh_signature.rb +++ b/app/models/commit_signatures/ssh_signature.rb @@ -3,7 +3,16 @@ module CommitSignatures class SshSignature < ApplicationRecord include CommitSignature + include SignatureType belongs_to :key, optional: true + + def type + :ssh + end + + def signed_by_user + key&.user + end end end diff --git a/app/models/commit_signatures/x509_commit_signature.rb b/app/models/commit_signatures/x509_commit_signature.rb index 2cbb331dd7e..4edbc147502 100644 --- a/app/models/commit_signatures/x509_commit_signature.rb +++ b/app/models/commit_signatures/x509_commit_signature.rb @@ -2,15 +2,24 @@ module CommitSignatures class X509CommitSignature < ApplicationRecord include CommitSignature + include SignatureType belongs_to :x509_certificate, class_name: 'X509Certificate', foreign_key: 'x509_certificate_id', optional: false validates :x509_certificate_id, presence: true + def type + :x509 + end + def x509_commit return unless commit Gitlab::X509::Commit.new(commit) end + + def signed_by_user + commit&.committer + end end end diff --git a/app/models/concerns/commit_signature.rb b/app/models/concerns/commit_signature.rb index 5bdfa9a2966..7f1fbbefd94 100644 --- a/app/models/concerns/commit_signature.rb +++ b/app/models/concerns/commit_signature.rb @@ -44,7 +44,7 @@ module CommitSignature project.commit(commit_sha) end - def user - commit.committer + def signed_by_user + raise NoMethodError, 'must implement `signed_by_user` method' end end diff --git a/app/models/concerns/signature_type.rb b/app/models/concerns/signature_type.rb new file mode 100644 index 00000000000..804f42b6f72 --- /dev/null +++ b/app/models/concerns/signature_type.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module SignatureType + TYPES = %i[gpg ssh x509].freeze + + def type + raise NoMethodError, 'must implement `type` method' + end + + TYPES.each do |type| + define_method("#{type}?") { self.type == type } + end +end diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index 7de56c037ed..71dd9501648 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/app/services/git/branch_hooks_service.rb @@ -164,16 +164,27 @@ module Git end end - def unsigned_x509_shas(commits) - CommitSignatures::X509CommitSignature.unsigned_commit_shas(commits.map(&:sha)) + def signature_types + types = [ + ::CommitSignatures::GpgSignature, + ::CommitSignatures::X509CommitSignature + ] + + types.push(::CommitSignatures::SshSignature) if Feature.enabled?(:ssh_commit_signatures, project) + + types end - def unsigned_gpg_shas(commits) - CommitSignatures::GpgSignature.unsigned_commit_shas(commits.map(&:sha)) + def unsigned_commit_shas(commits) + commit_shas = commits.map(&:sha) + + signature_types + .map { |signature| signature.unsigned_commit_shas(commit_shas) } + .reduce(&:&) end def enqueue_update_signatures - unsigned = unsigned_x509_shas(limited_commits) & unsigned_gpg_shas(limited_commits) + unsigned = unsigned_commit_shas(limited_commits) return if unsigned.empty? signable = Gitlab::Git::Commit.shas_with_signatures(project.repository, unsigned) diff --git a/app/views/projects/commit/_signature.html.haml b/app/views/projects/commit/_signature.html.haml index 978d83bf2b4..c6f1e51049e 100644 --- a/app/views/projects/commit/_signature.html.haml +++ b/app/views/projects/commit/_signature.html.haml @@ -1,3 +1,3 @@ - if signature - - uri = "projects/commit/#{'x509/' if x509_signature?(signature)}" + - uri = "projects/commit/#{'x509/' if signature.x509?}" = render partial: "#{uri}#{signature.verification_status}_signature_badge", locals: { signature: signature } diff --git a/app/views/projects/commit/_signature_badge.html.haml b/app/views/projects/commit/_signature_badge.html.haml index fb30bfc2953..ad6b524c01b 100644 --- a/app/views/projects/commit/_signature_badge.html.haml +++ b/app/views/projects/commit/_signature_badge.html.haml @@ -17,18 +17,23 @@ - content = capture do - if show_user .clearfix - - uri_signature_badge_user = "projects/commit/#{'x509/' if x509_signature?(signature)}signature_badge_user" + - uri_signature_badge_user = "projects/commit/#{'x509/' if signature.x509?}signature_badge_user" = render partial: "#{uri_signature_badge_user}", locals: { signature: signature } - - if x509_signature?(signature) + - if signature.x509? = render partial: "projects/commit/x509/certificate_details", locals: { signature: signature } - = link_to(_('Learn more about X.509 signed commits'), help_page_path('user/project/repository/x509_signed_commits/index.md'), class: 'gpg-popover-help-link') + = link_to(_('Learn more about X.509 signed commits'), help_page_path('user/project/repository/x509_signed_commits/index.md'), class: 'gl-link gl-display-block') + - elsif ::Feature.enabled?(:ssh_commit_signatures, signature.project) && signature.ssh? + = _('SSH key fingerprint:') + %span.gl-font-monospace= signature.key&.fingerprint_sha256 || _('Unknown') + + = link_to(_('Learn about signing commits with SSH keys.'), help_page_path('user/project/repository/ssh_signed_commits/index.md'), class: 'gl-link gl-display-block') - else = _('GPG Key ID:') - %span.monospace= signature.gpg_key_primary_keyid + %span.gl-font-monospace= signature.gpg_key_primary_keyid - = link_to(_('Learn more about signing commits'), help_page_path('user/project/repository/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link gl-display-block') + = link_to(_('Learn more about signing commits'), help_page_path('user/project/repository/gpg_signed_commits/index.md'), class: 'gl-link gl-display-block') %a{ role: 'button', tabindex: 0, class: css_classes, data: { toggle: 'popover', html: 'true', placement: 'top', title: title, content: content } } = label diff --git a/app/views/projects/commit/_signature_badge_user.html.haml b/app/views/projects/commit/_signature_badge_user.html.haml index b20198e76db..656adef6a72 100644 --- a/app/views/projects/commit/_signature_badge_user.html.haml +++ b/app/views/projects/commit/_signature_badge_user.html.haml @@ -1,7 +1,4 @@ -- gpg_key = signature.gpg_key -- user = gpg_key&.user -- user_name = signature.gpg_key_user_name -- user_email = signature.gpg_key_user_email +- user = signature.signed_by_user - if user = link_to user_path(user), class: 'gpg-popover-user-link' do @@ -11,11 +8,14 @@ %div %strong= user.name %div= user.to_reference -- else - = mail_to user_email do - %div - = user_avatar_without_link(user_name: user_name, user_email: user_email, size: 32) +- elsif signature.gpg? # SSH signatures do not have an email embedded in them + - user_name = signature.gpg_key_user_name + - user_email = signature.gpg_key_user_email + - if user_name && user_email + = mail_to user_email do + %div + = user_avatar_without_link(user_name: user_name, user_email: user_email, size: 32) - %div - %strong= user_name - %div= user_email + %div + %strong= user_name + %div= user_email diff --git a/app/views/projects/settings/operations/_alert_management.html.haml b/app/views/projects/settings/operations/_alert_management.html.haml index d80f1e4597c..7433e81c11c 100644 --- a/app/views/projects/settings/operations/_alert_management.html.haml +++ b/app/views/projects/settings/operations/_alert_management.html.haml @@ -3,7 +3,7 @@ - add_page_specific_style 'page_bundles/alert_management_settings' - add_page_specific_style 'page_bundles/incident_management_list' -%section.settings.no-animate#js-alert-management-settings{ class: ('expanded' if expanded) } +%section.settings.no-animate#js-alert-management-settings{ class: ('expanded' if expanded), data: { qa_selector: 'alerts_settings_content' } } .settings-header %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only = _('Alerts') diff --git a/config/feature_flags/development/ssh_commit_signatures.yml b/config/feature_flags/development/ssh_commit_signatures.yml new file mode 100644 index 00000000000..3941631996e --- /dev/null +++ b/config/feature_flags/development/ssh_commit_signatures.yml @@ -0,0 +1,8 @@ +--- +name: ssh_commit_signatures +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97248 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350301 +milestone: '15.7' +type: development +group: group::source code +default_enabled: false diff --git a/db/post_migrate/20221121181627_drop_index_on_vulnerabilities_state_case_id_desc.rb b/db/post_migrate/20221121181627_drop_index_on_vulnerabilities_state_case_id_desc.rb new file mode 100644 index 00000000000..712343bc7b0 --- /dev/null +++ b/db/post_migrate/20221121181627_drop_index_on_vulnerabilities_state_case_id_desc.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class DropIndexOnVulnerabilitiesStateCaseIdDesc < Gitlab::Database::Migration[2.0] + INDEX_NAME = "index_vulnerabilities_on_state_case_id_desc" + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name( + :vulnerabilities, + INDEX_NAME + ) + end + + def down + execute <<~SQL + CREATE INDEX CONCURRENTLY index_vulnerabilities_on_state_case_id_desc ON vulnerabilities + USING btree (array_position(ARRAY[(1)::smallint, (4)::smallint, (3)::smallint, (2)::smallint], state) DESC, id DESC); + SQL + end +end diff --git a/db/schema_migrations/20221121181627 b/db/schema_migrations/20221121181627 new file mode 100644 index 00000000000..2be29d4fcec --- /dev/null +++ b/db/schema_migrations/20221121181627 @@ -0,0 +1 @@ +91c8b8327b502611b47400f229204f3093b87f6dd555c7471a2a827c0ee2d7fe
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0e8904c919e..3331998bdbf 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31149,8 +31149,6 @@ CREATE INDEX index_vulnerabilities_on_resolved_by_id ON vulnerabilities USING bt CREATE INDEX index_vulnerabilities_on_start_date_sourcing_milestone_id ON vulnerabilities USING btree (start_date_sourcing_milestone_id); -CREATE INDEX index_vulnerabilities_on_state_case_id_desc ON vulnerabilities USING btree (array_position(ARRAY[(1)::smallint, (4)::smallint, (3)::smallint, (2)::smallint], state) DESC, id DESC); - CREATE INDEX index_vulnerabilities_on_updated_by_id ON vulnerabilities USING btree (updated_by_id); CREATE INDEX index_vulnerabilities_project_id_and_id_on_default_branch ON vulnerabilities USING btree (project_id, id) WHERE (present_on_default_branch IS TRUE); diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index e8d19363242..bd8699c2c1f 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -64,8 +64,9 @@ To view instance audit events: The time zone used for audit events depends on where you view them: -- In GitLab UI, your local time zone (GitLab 15.6 and later) or UTC (GitLab 15.5 and earlier) is used. -- The [Audit Events API](../api/audit_events.md) returns dates and times in UTC by default, or the [configured time zone](timezone.md) on a self-managed GitLab instance. +- In GitLab UI, your local time zone (GitLab 15.7 and later) or UTC (GitLab 15.6 and earlier) is used. +- The [Audit Events API](../api/audit_events.md) returns dates and times in UTC by default, or the + [configured time zone](timezone.md) on a self-managed GitLab instance. - In `audit_json.log`, UTC is used. - In CSV exports, UTC is used. diff --git a/doc/user/application_security/policies/index.md b/doc/user/application_security/policies/index.md index f6d22ab28cd..453b815509e 100644 --- a/doc/user/application_security/policies/index.md +++ b/doc/user/application_security/policies/index.md @@ -140,10 +140,10 @@ for more information on the product direction of security policies within GitLab ## Troubleshooting -### `Branch name does not follow the pattern 'update-policy-<timestamp>'` +### `Branch name 'update-policy-<timestamp>' does not follow the pattern '<branch_name_regex>'` When you create a new security policy or change an existing policy, a new branch is automatically created with the branch name following the pattern `update-policy-<timestamp>`. For example: `update-policy-1659094451`. -If you have group or instance push rules that do not allow branch name patterns that contain the text `update-policy-<timestamp>`, you will get an error that states `Branch name does not follow the pattern 'update-policy-<timestamp>'`. +If you have group or instance [push rules that do not allow branch name patterns](../../project/repository/push_rules.md#validate-branch-names) that contain the text `update-policy-<timestamp>`, you will get an error that states `Branch name 'update-policy-<timestamp>' does not follow the pattern '<branch_name_regex>'`. The workaround is to amend your group or instance push rules to allow branches following the pattern `update-policy-` followed by an integer timestamp. diff --git a/doc/user/project/repository/branches/index.md b/doc/user/project/repository/branches/index.md index 6cc7394e7b3..645144522e4 100644 --- a/doc/user/project/repository/branches/index.md +++ b/doc/user/project/repository/branches/index.md @@ -101,10 +101,13 @@ This feature allows you to search and select branches quickly. Search results ap - Branches with names that matched search terms exactly. - Other branches with names that include search terms, sorted alphabetically. -Sometimes when you have hundreds of branches you may want a more flexible matching pattern. In such cases you can use the following: +Sometimes when you have hundreds of branches you may want a more flexible matching pattern. In such cases you can use the following operators: -- `^feature` matches only branch names that begin with 'feature'. -- `feature$` matches only branch names that end with 'feature'. +- `^` matches beginning of branch name, for example `^feat` would match `feat/user-authentication` +- `$` matches end of branch name, for example `widget$` would match `feat/search-box-widget` +- `*` wildcard matcher, for example `branch*cache*` would match `fix/branch-search-cache-expiration` + +These operators can be mixed, for example `^chore/*migration$` would match `chore/user-data-migration` ## Swap revisions diff --git a/doc/user/project/repository/gpg_signed_commits/index.md b/doc/user/project/repository/gpg_signed_commits/index.md index 61fc0b88d05..6b67ffd0e59 100644 --- a/doc/user/project/repository/gpg_signed_commits/index.md +++ b/doc/user/project/repository/gpg_signed_commits/index.md @@ -4,7 +4,7 @@ group: Source Code info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Signing commits with GPG **(FREE)** +# Sign commits with GPG **(FREE)** You can sign the commits you make in a GitLab repository with a GPG ([GNU Privacy Guard](https://gnupg.org/)) key. When you add a cryptographic @@ -238,6 +238,7 @@ If you must unverify both future and past commits, ## Related topics - [Sign commits and tags with X.509 certificates](../x509_signed_commits/index.md) +- [Sign commits with SSH keys](../ssh_signed_commits/index.md) - [Commits API](../../../../api/commits.md) - GPG resources: - [Git Tools - Signing Your Work](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work) diff --git a/doc/user/project/repository/ssh_signed_commits/index.md b/doc/user/project/repository/ssh_signed_commits/index.md new file mode 100644 index 00000000000..4c7e07da7f1 --- /dev/null +++ b/doc/user/project/repository/ssh_signed_commits/index.md @@ -0,0 +1,112 @@ +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Sign commits with SSH keys **(FREE)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/343879) in GitLab 15.7 [with a flag](../../../../administration/feature_flags.md) named `ssh_commit_signatures`. Disabled by default. + +Use SSH keys to sign Git commits in the same manner as +[GPG signed commits](../gpg_signed_commits/index.md). When you sign commits +with SSH keys, GitLab uses the SSH public keys associated with your +GitLab account to cryptographically verify the commit signature. +If successful, GitLab displays a **Verified** label on the commit. + +You may use the same SSH keys for `git+ssh` authentication to GitLab +and signing commit signatures. + +To learn more about managing the SSH keys associated with your GitLab account, read +[use SSH keys to communicate with GitLab](../../../ssh.md). + +## Configure Git to sign commits with your SSH key + +After you have [created an SSH key](../../../ssh.md#generate-an-ssh-key-pair) and +[added it to your GitLab account](../../../ssh.md#add-an-ssh-key-to-your-gitlab-account), +you need to configure Git to begin using it. + +Prerequisites: + +- Git 2.34.0 or newer. +- OpenSSH 8.0 or newer. + + NOTE: + OpenSSH 8.7 has broken signing functionality. If you are on OpenSSH 8.7, upgrade to OpenSSH 8.8. + +- A SSH key of one of these types: + - [ED25519](../../../ssh.md#ed25519-ssh-keys) (recommended) + - [RSA](../../../ssh.md#rsa-ssh-keys) + +To configure Git: + +1. Configure Git to use SSH for commit signing: + + ```shell + git config --global gpg.format ssh + ``` + +1. Specify which SSH key should be used as the signing key, changing the filename + (here, `~/.ssh/examplekey`) to the location of your key. The filename may + differ, depending on how you generated your key: + + ```shell + git config --global user.signingkey ~/.ssh/examplekey + ``` + +## Sign commits with your SSH key + +Prerequisites: + +- You've [created an SSH key](../../../ssh.md#generate-an-ssh-key-pair). +- You've [added the key](../../../ssh.md#add-an-ssh-key-to-your-gitlab-account) to your GitLab account. +- You've [configured Git to sign commits](#configure-git-to-sign-commits-with-your-ssh-key) with your SSH key. + +To sign a commit: + +1. Use the `-S` flag when signing your commits: + + ```shell + git commit -S -m "My commit msg" + ``` + +1. Optional. If you don't want to type the `-S` flag every time you commit, tell + Git to sign your commits automatically: + + ```shell + git config --global commit.gpgsign true + ``` + +1. If your SSH key is protected, Git prompts you to enter your passphrase. +1. Push to GitLab. +1. Check that your commits [are verified](../gpg_signed_commits/index.md#verify-commits). + +## Verify commits + +You can review commits for a merge request, or for an entire project, to confirm +they are signed: + +1. To review commits for a project: + 1. On the top bar, select **Main menu > Projects** and find your project. + 1. On the left sidebar, select **Repository > Commits**. +1. To review commits for a merge request: + 1. On the top bar, select **Main menu > Projects** and find your project. + 1. On the left sidebar, select **Merge requests**, then select your merge request. + 1. Select **Commits**. +1. Identify the commit you want to review. Signed commits show either a **Verified** + or **Unverified** badge, depending on the verification status of the signature. + Unsigned commits do not display a badge. + +1. To display the signature details for a commit, select **Verified**. GitLab shows + the SSH key's fingerprint. + +## Revoke an SSH key for signing commits + +You can't revoke an SSH key used for signing commits. To learn more, read +[Add revocation for SSH keys](https://gitlab.com/gitlab-org/gitlab/-/issues/382984). + +## Related topics + +- [Sign commits and tags with X.509 certificates](../x509_signed_commits/index.md) +- [Sign commits with GPG](../gpg_signed_commits/index.md) +- [Commits API](../../../../api/commits.md) diff --git a/doc/user/project/repository/x509_signed_commits/index.md b/doc/user/project/repository/x509_signed_commits/index.md index e16f5e4defe..42f7be30822 100644 --- a/doc/user/project/repository/x509_signed_commits/index.md +++ b/doc/user/project/repository/x509_signed_commits/index.md @@ -160,6 +160,8 @@ can start signing your tags: ## Related topics - [Rake task for X.509 signatures](../../../../raketasks/x509_signatures.md) +- [Sign commits with GPG](../gpg_signed_commits/index.md) +- [Sign commits with SSH keys](../ssh_signed_commits/index.md) ## Troubleshooting diff --git a/lib/api/api.rb b/lib/api/api.rb index 318c3d4133b..00fac054547 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -267,6 +267,7 @@ module API mount ::API::Terraform::StateVersion mount ::API::Topics mount ::API::Unleash + mount ::API::UsageDataNonSqlMetrics mount ::API::UserCounts mount ::API::Wikis @@ -329,7 +330,6 @@ module API mount ::API::Templates mount ::API::Todos mount ::API::UsageData - mount ::API::UsageDataNonSqlMetrics mount ::API::UsageDataQueries mount ::API::Users mount ::API::Ml::Mlflow diff --git a/lib/api/entities/commit_signature.rb b/lib/api/entities/commit_signature.rb index 9430dd5e2a2..9c30c3c59ea 100644 --- a/lib/api/entities/commit_signature.rb +++ b/lib/api/entities/commit_signature.rb @@ -10,6 +10,8 @@ module API ::API::Entities::GpgCommitSignature.represent commit_signature(commit), options elsif commit.signature.is_a?(::CommitSignatures::X509CommitSignature) ::API::Entities::X509Signature.represent commit.signature, options + elsif commit.signature.is_a?(::CommitSignatures::SshSignature) + ::API::Entities::SshSignature.represent(commit.signature, options) end end diff --git a/lib/api/entities/ssh_signature.rb b/lib/api/entities/ssh_signature.rb new file mode 100644 index 00000000000..dc3800c87c5 --- /dev/null +++ b/lib/api/entities/ssh_signature.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module API + module Entities + class SshSignature < Grape::Entity + expose :verification_status, documentation: { type: 'string', example: 'unverified' } + expose :key, using: 'API::Entities::SSHKey' + end + end +end diff --git a/lib/api/usage_data_non_sql_metrics.rb b/lib/api/usage_data_non_sql_metrics.rb index 41f369a43b8..81f96a7958b 100644 --- a/lib/api/usage_data_non_sql_metrics.rb +++ b/lib/api/usage_data_non_sql_metrics.rb @@ -14,6 +14,12 @@ module API desc 'Get Non SQL usage ping metrics' do detail 'This feature was introduced in GitLab 13.11.' + success code: 200 + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' } + ] end get 'non_sql_metrics' do diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb index 6b5f10ed78e..05d680c139c 100644 --- a/lib/gitlab/gfm/uploads_rewriter.rb +++ b/lib/gitlab/gfm/uploads_rewriter.rb @@ -54,7 +54,6 @@ module Gitlab file = find_file(match[:secret], match[:file]) # No file will be returned for a path traversal - return markdown if file.nil? return markdown unless file.try(:exists?) klass = @target_parent.is_a?(Namespace) ? NamespaceFileUploader : FileUploader diff --git a/lib/gitlab/x509/signature.rb b/lib/gitlab/x509/signature.rb index f8a6980f208..3b941853ebd 100644 --- a/lib/gitlab/x509/signature.rb +++ b/lib/gitlab/x509/signature.rb @@ -6,6 +6,7 @@ module Gitlab module X509 class Signature include Gitlab::Utils::StrongMemoize + include SignatureType attr_reader :signature_text, :signed_text, :created_at @@ -16,6 +17,10 @@ module Gitlab @created_at = created_at end + def type + :x509 + end + def x509_certificate return if certificate_attributes.nil? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 82fd192bb7e..5d9bb3d40aa 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8499,6 +8499,9 @@ msgstr "" msgid "CiVariables|Cannot use Masked Variable with current value" msgstr "" +msgid "CiVariables|Clear inputs" +msgstr "" + msgid "CiVariables|Environments" msgstr "" @@ -8526,6 +8529,9 @@ msgstr "" msgid "CiVariables|Remove variable row" msgstr "" +msgid "CiVariables|Run job again" +msgstr "" + msgid "CiVariables|Scope" msgstr "" @@ -23615,6 +23621,9 @@ msgstr "" msgid "Job|This job is stuck because you don't have any active runners that can run this job." msgstr "" +msgid "Job|Update CI/CD variables" +msgstr "" + msgid "Job|Waiting for resource" msgstr "" @@ -24048,6 +24057,9 @@ msgstr "" msgid "Learn More." msgstr "" +msgid "Learn about signing commits with SSH keys." +msgstr "" + msgid "Learn how to %{link_start}contribute to the built-in templates%{link_end}" msgstr "" @@ -35095,6 +35107,9 @@ msgstr "" msgid "Run housekeeping tasks to automatically optimize Git repositories. Disabling this option will cause performance to degenerate over time." msgstr "" +msgid "Run manual job again" +msgstr "" + msgid "Run manual or delayed jobs" msgstr "" @@ -35755,6 +35770,9 @@ msgstr "" msgid "SSH key" msgstr "" +msgid "SSH key fingerprint:" +msgstr "" + msgid "SSH keys" msgstr "" @@ -41452,6 +41470,9 @@ msgstr "" msgid "There was an error fetching the environments information." msgstr "" +msgid "There was an error fetching the job." +msgstr "" + msgid "There was an error fetching the jobs for your project." msgstr "" @@ -41491,6 +41512,9 @@ msgstr "" msgid "There was an error retrieving the Jira users." msgstr "" +msgid "There was an error running the job. Please try again." +msgstr "" + msgid "There was an error saving your changes." msgstr "" diff --git a/qa/qa/flow/alert_settings.rb b/qa/qa/flow/alert_settings.rb new file mode 100644 index 00000000000..0e884f58773 --- /dev/null +++ b/qa/qa/flow/alert_settings.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module QA + module Flow + module AlertSettings + extend self + + def setup_http_endpoint_and_send_alert(integration_name: nil, payload: nil) + integration_name ||= random_word + payload ||= { title: random_word, description: random_word } + Page::Project::Menu.perform(&:go_to_monitor_settings) + Page::Project::Settings::Monitor.perform do |setting| + setting.expand_alerts do |alert| + alert.add_new_integration + alert.select_http_endpoint + alert.enter_integration_name(integration_name) + alert.activate_integration + alert.save_and_create_alert + alert.fill_in_test_payload(payload.to_json) + alert.send_test_alert + end + end + end + + private + + def random_word + Faker::Lorem.word + end + end + end +end diff --git a/qa/qa/page/project/monitor/alerts/index.rb b/qa/qa/page/project/monitor/alerts/index.rb new file mode 100644 index 00000000000..50b69d59db7 --- /dev/null +++ b/qa/qa/page/project/monitor/alerts/index.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module QA + module Page + module Project + module Monitor + module Alerts + class Index < Page::Base + view 'app/assets/javascripts/alert_management/components/alert_management_table.vue' do + element :alert_table_container, required: true + end + + def has_alert_with_title?(title) + has_link?(title) + end + end + end + end + end + end +end diff --git a/qa/qa/page/project/settings/alerts.rb b/qa/qa/page/project/settings/alerts.rb index be9b61ded80..a74a227d697 100644 --- a/qa/qa/page/project/settings/alerts.rb +++ b/qa/qa/page/project/settings/alerts.rb @@ -6,14 +6,27 @@ module QA module Settings class Alerts < Page::Base view 'app/assets/javascripts/alerts_settings/components/alerts_form.vue' do - element :create_issue_checkbox + element :create_incident_checkbox element :incident_templates_dropdown element :save_changes_button element :incident_templates_item end - def enable_issues_for_incidents - check_element(:create_issue_checkbox) + view 'app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue' do + element :add_integration_button + end + + view 'app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue' do + element :integration_type_dropdown + element :integration_name_field + element :active_toggle_container + element :save_and_create_alert_button + element :test_payload_field + element :send_test_alert_button + end + + def enable_incident_for_alert + check_element(:create_incident_checkbox) end def select_issue_template(template) @@ -32,6 +45,43 @@ module QA has_text?(template) end end + + def add_new_integration + wait_for_requests + click_element(:add_integration_button) + end + + def select_http_endpoint + click_element(:integration_type_dropdown) + find("option[value='HTTP']").click + + # Click outside of the list to close it + click_element(:integration_name_field) + end + + def enter_integration_name(name) + fill_element(:integration_name_field, name) + end + + def activate_integration + within_element(:active_toggle_container) do + find('.gl-toggle').click + end + + wait_for_requests + end + + def save_and_create_alert + click_element(:save_and_create_alert_button) + end + + def fill_in_test_payload(payload) + fill_element(:test_payload_field, payload) + end + + def send_test_alert + click_element(:send_test_alert_button) + end end end end diff --git a/qa/qa/page/project/settings/monitor.rb b/qa/qa/page/project/settings/monitor.rb index 87fb0698897..8170ae31a13 100644 --- a/qa/qa/page/project/settings/monitor.rb +++ b/qa/qa/page/project/settings/monitor.rb @@ -11,8 +11,18 @@ module QA element :incidents_settings_content end + view 'app/views/projects/settings/operations/_alert_management.html.haml' do + element :alerts_settings_content + end + def expand_incidents(&block) expand_content(:incidents_settings_content) do + # Fill in with incidents settings + end + end + + def expand_alerts(&block) + expand_content(:alerts_settings_content) do Settings::Alerts.perform(&block) end end diff --git a/qa/qa/page/project/sub_menus/monitor.rb b/qa/qa/page/project/sub_menus/monitor.rb index 927b4b6970c..27fb58fb146 100644 --- a/qa/qa/page/project/sub_menus/monitor.rb +++ b/qa/qa/page/project/sub_menus/monitor.rb @@ -23,6 +23,14 @@ module QA end end + def go_to_monitor_alerts + hover_monitor do + within_submenu do + click_element(:sidebar_menu_item_link, menu_item: 'Alerts') + end + end + end + private def hover_monitor diff --git a/qa/qa/resource/group_base.rb b/qa/qa/resource/group_base.rb index f6d1aacca0a..c5b1a4ecea0 100644 --- a/qa/qa/resource/group_base.rb +++ b/qa/qa/resource/group_base.rb @@ -24,6 +24,7 @@ module QA def projects parse_body(api_get_from("#{api_get_path}/projects")).map do |project| Project.init do |resource| + resource.add_name_uuid = false resource.api_client = api_client resource.group = self resource.id = project[:id] diff --git a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb index a0e3e4f4d43..93a47a40a7e 100644 --- a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb +++ b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_large_project_spec.rb @@ -50,7 +50,7 @@ module QA # Source objects # - let(:source_project) { source_group.projects.find { |project| project.name.include?(gitlab_source_project) }.reload! } + let(:source_project) { source_group.projects.find { |project| project.name == gitlab_source_project }.reload! } let(:source_branches) { source_project.repository_branches(auto_paginate: true).map { |b| b[:name] } } let(:source_commits) { source_project.commits(auto_paginate: true).map { |c| c[:id] } } let(:source_labels) { source_project.labels(auto_paginate: true).map { |l| l.except(:id) } } @@ -61,7 +61,7 @@ module QA # Imported objects # - let(:imported_project) { imported_group.projects.find { |project| project.name.include?(gitlab_source_project) }.reload! } + let(:imported_project) { imported_group.projects.find { |project| project.name == gitlab_source_project }.reload! } let(:branches) { imported_project.repository_branches(auto_paginate: true).map { |b| b[:name] } } let(:commits) { imported_project.commits(auto_paginate: true).map { |c| c[:id] } } let(:labels) { imported_project.labels(auto_paginate: true).map { |l| l.except(:id) } } diff --git a/qa/qa/specs/features/browser_ui/8_monitor/.gitkeep b/qa/qa/specs/features/browser_ui/8_monitor/.gitkeep deleted file mode 100644 index e69de29bb2d..00000000000 --- a/qa/qa/specs/features/browser_ui/8_monitor/.gitkeep +++ /dev/null diff --git a/qa/qa/specs/features/browser_ui/8_monitor/incident_management/http_endpoint_integration_creates_alert_spec.rb b/qa/qa/specs/features/browser_ui/8_monitor/incident_management/http_endpoint_integration_creates_alert_spec.rb new file mode 100644 index 00000000000..8ea728ca94c --- /dev/null +++ b/qa/qa/specs/features/browser_ui/8_monitor/incident_management/http_endpoint_integration_creates_alert_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module QA + RSpec.describe 'Monitor', product_group: :respond do + describe 'Http endpoint integration' do + let(:project) do + Resource::Project.fabricate_via_api! do |project| + project.name = 'project-for-alerts' + project.description = 'Project for alerts' + end + end + + let(:random_word) { Faker::Lorem.word } + + let(:payload) do + { title: random_word, description: random_word } + end + + before do + Flow::Login.sign_in + project.visit! + Flow::AlertSettings.setup_http_endpoint_and_send_alert(payload: payload) + end + + it( + 'can send test alert that creates new alert', + testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/382803' + ) do + Page::Project::Menu.perform(&:go_to_monitor_alerts) + Page::Project::Monitor::Alerts::Index.perform do |alerts| + expect(alerts).to have_alert_with_title(random_word) + end + end + end + end +end diff --git a/spec/features/callouts/registration_enabled_spec.rb b/spec/features/callouts/registration_enabled_spec.rb index 79e99712183..4fc73863de6 100644 --- a/spec/features/callouts/registration_enabled_spec.rb +++ b/spec/features/callouts/registration_enabled_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Registration enabled callout' do +RSpec.describe 'Registration enabled callout', feature_category: :authentication_and_authorization do let_it_be(:admin) { create(:admin) } let_it_be(:non_admin) { create(:user) } let_it_be(:project) { create(:project) } diff --git a/spec/features/clusters/cluster_detail_page_spec.rb b/spec/features/clusters/cluster_detail_page_spec.rb index 06e3e00db7d..e8fb5f4105d 100644 --- a/spec/features/clusters/cluster_detail_page_spec.rb +++ b/spec/features/clusters/cluster_detail_page_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Clusterable > Show page' do +RSpec.describe 'Clusterable > Show page', feature_category: :kubernetes_management do include KubernetesHelpers let(:current_user) { create(:user) } diff --git a/spec/features/clusters/cluster_health_dashboard_spec.rb b/spec/features/clusters/cluster_health_dashboard_spec.rb index 88d6976c2be..b557f803a99 100644 --- a/spec/features/clusters/cluster_health_dashboard_spec.rb +++ b/spec/features/clusters/cluster_health_dashboard_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe 'Cluster Health board', :js, :kubeclient, :use_clean_rails_memory_store_caching, :sidekiq_inline do +RSpec.describe 'Cluster Health board', :js, :kubeclient, :use_clean_rails_memory_store_caching, :sidekiq_inline, +feature_category: :kubernetes_management do include KubernetesHelpers include PrometheusHelpers diff --git a/spec/features/clusters/create_agent_spec.rb b/spec/features/clusters/create_agent_spec.rb index b19e57c550c..d01fa520cb0 100644 --- a/spec/features/clusters/create_agent_spec.rb +++ b/spec/features/clusters/create_agent_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Cluster agent registration', :js do +RSpec.describe 'Cluster agent registration', :js, feature_category: :kubernetes_management do let_it_be(:project) { create(:project, :custom_repo, files: { '.gitlab/agents/example-agent-1/config.yaml' => '' }) } let_it_be(:current_user) { create(:user, maintainer_projects: [project]) } let_it_be(:token) { Devise.friendly_token } diff --git a/spec/features/commits/user_uses_quick_actions_spec.rb b/spec/features/commits/user_uses_quick_actions_spec.rb index 12e7865e490..6d043a0bb2f 100644 --- a/spec/features/commits/user_uses_quick_actions_spec.rb +++ b/spec/features/commits/user_uses_quick_actions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Commit > User uses quick actions', :js do +RSpec.describe 'Commit > User uses quick actions', :js, feature_category: :source_code_management do include Spec::Support::Helpers::Features::NotesHelpers include RepoHelpers diff --git a/spec/features/commits/user_view_commits_spec.rb b/spec/features/commits/user_view_commits_spec.rb index f7fd3a6e209..b58d7cf3741 100644 --- a/spec/features/commits/user_view_commits_spec.rb +++ b/spec/features/commits/user_view_commits_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Commit > User view commits' do +RSpec.describe 'Commit > User view commits', feature_category: :source_code_management do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group, :public) } diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 96a8168e708..4f7b7b5b98f 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -215,10 +215,6 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do visit project_job_path(project, job) end - it 'shows retry button' do - expect(page).to have_link('Retry') - end - context 'if job passed' do it 'does not show New issue button' do expect(page).not_to have_link('New issue') diff --git a/spec/finders/branches_finder_spec.rb b/spec/finders/branches_finder_spec.rb index f14c60c4b8f..18f8d1adecc 100644 --- a/spec/finders/branches_finder_spec.rb +++ b/spec/finders/branches_finder_spec.rb @@ -72,16 +72,6 @@ RSpec.describe BranchesFinder do end end - context 'with an unknown name' do - let(:params) { { search: 'random' } } - - it 'does not find any branch' do - result = subject - - expect(result.count).to eq(0) - end - end - context 'by provided names' do let(:params) { { names: %w[fix csv lfs does-not-exist] } } @@ -115,6 +105,49 @@ RSpec.describe BranchesFinder do end end + context 'by name with wildcard' do + let(:params) { { search: 'f*e' } } + + it 'filters branches' do + result = subject + + expect(result.first.name).to eq('2-mb-file') + expect(result.count).to eq(30) + end + end + + context 'by mixed regex operators' do + let(:params) { { search: '^f*e$' } } + + it 'filters branches' do + result = subject + + expect(result.first.name).to eq('feature') + expect(result.count).to eq(1) + end + end + + context 'by name with multiple wildcards' do + let(:params) { { search: 'f*a*e' } } + + it 'filters branches' do + result = subject + + expect(result.first.name).to eq('after-create-delete-modify-move') + expect(result.count).to eq(11) + end + end + + context 'with an unknown name' do + let(:params) { { search: 'random' } } + + it 'does not find any branch' do + result = subject + + expect(result.count).to eq(0) + end + end + context 'by nonexistent name that begins with' do let(:params) { { search: '^nope' } } @@ -134,6 +167,16 @@ RSpec.describe BranchesFinder do expect(result.count).to eq(0) end end + + context 'by nonexistent name with wildcard' do + let(:params) { { search: 'zz*asdf' } } + + it 'filters branches' do + result = subject + + expect(result.count).to eq(0) + end + end end context 'filter and sort' do diff --git a/spec/finders/tags_finder_spec.rb b/spec/finders/tags_finder_spec.rb index 0bf9b228c8a..2af23c466fb 100644 --- a/spec/finders/tags_finder_spec.rb +++ b/spec/finders/tags_finder_spec.rb @@ -68,6 +68,14 @@ RSpec.describe TagsFinder do expect(result.count).to eq(1) end + it 'filters tags by name with wildcard' do + result = load_tags({ search: 'v1.*.0' }) + + expect(result.first.name).to eq('v1.0.0') + expect(result.second.name).to eq('v1.1.0') + expect(result.count).to eq(2) + end + it 'filters tags by nonexistent name that begins with' do result = load_tags({ search: '^nope' }) @@ -79,6 +87,11 @@ RSpec.describe TagsFinder do expect(result.count).to eq(0) end + it 'filters tags by nonexistent name with wildcard' do + result = load_tags({ search: 'n*e' }) + expect(result.count).to eq(0) + end + context 'when search is not a string' do it 'returns no matches' do result = load_tags({ search: { 'a' => 'b' } }) diff --git a/spec/frontend/alerts_settings/components/__snapshots__/alerts_form_spec.js.snap b/spec/frontend/alerts_settings/components/__snapshots__/alerts_form_spec.js.snap index 4693d5a47e4..bff4905a12c 100644 --- a/spec/frontend/alerts_settings/components/__snapshots__/alerts_form_spec.js.snap +++ b/spec/frontend/alerts_settings/components/__snapshots__/alerts_form_spec.js.snap @@ -16,7 +16,7 @@ exports[`Alert integration settings form default state should match the default > <gl-form-checkbox-stub checked="true" - data-qa-selector="create_issue_checkbox" + data-qa-selector="create_incident_checkbox" id="2" > <span> diff --git a/spec/frontend/ide/components/repo_editor_spec.js b/spec/frontend/ide/components/repo_editor_spec.js index 9921d8cba18..2f9fd957c6b 100644 --- a/spec/frontend/ide/components/repo_editor_spec.js +++ b/spec/frontend/ide/components/repo_editor_spec.js @@ -8,9 +8,14 @@ import '~/behaviors/markdown/render_gfm'; import waitForPromises from 'helpers/wait_for_promises'; import { stubPerformanceWebAPI } from 'helpers/performance'; import { exampleConfigs, exampleFiles } from 'jest/ide/lib/editorconfig/mock_data'; -import { EDITOR_CODE_INSTANCE_FN, EDITOR_DIFF_INSTANCE_FN } from '~/editor/constants'; +import { + EDITOR_CODE_INSTANCE_FN, + EDITOR_DIFF_INSTANCE_FN, + EXTENSION_CI_SCHEMA_FILE_NAME_MATCH, +} from '~/editor/constants'; import { EditorMarkdownExtension } from '~/editor/extensions/source_editor_markdown_ext'; import { EditorMarkdownPreviewExtension } from '~/editor/extensions/source_editor_markdown_livepreview_ext'; +import { CiSchemaExtension } from '~/editor/extensions/source_editor_ci_schema_ext'; import SourceEditor from '~/editor/source_editor'; import RepoEditor from '~/ide/components/repo_editor.vue'; import { leftSidebarViews, FILE_VIEW_MODE_PREVIEW, viewerTypes } from '~/ide/constants'; @@ -22,6 +27,8 @@ import ContentViewer from '~/vue_shared/components/content_viewer/content_viewer import SourceEditorInstance from '~/editor/source_editor_instance'; import { file } from '../helpers'; +jest.mock('~/editor/extensions/source_editor_ci_schema_ext'); + const PREVIEW_MARKDOWN_PATH = '/foo/bar/preview_markdown'; const CURRENT_PROJECT_ID = 'gitlab-org/gitlab'; @@ -46,6 +53,12 @@ const dummyFile = { tempFile: true, active: true, }, + ciConfig: { + ...file(EXTENSION_CI_SCHEMA_FILE_NAME_MATCH), + content: '', + tempFile: true, + active: true, + }, empty: { ...file('empty'), tempFile: false, @@ -101,6 +114,7 @@ describe('RepoEditor', () => { let createDiffInstanceSpy; let createModelSpy; let applyExtensionSpy; + let removeExtensionSpy; let extensionsStore; const waitForEditorSetup = () => @@ -108,7 +122,7 @@ describe('RepoEditor', () => { vm.$once('editorSetup', resolve); }); - const createComponent = async ({ state = {}, activeFile = dummyFile.text } = {}) => { + const createComponent = async ({ state = {}, activeFile = dummyFile.text, flags = {} } = {}) => { const store = prepareStore(state, activeFile); wrapper = shallowMount(RepoEditor, { store, @@ -118,6 +132,9 @@ describe('RepoEditor', () => { mocks: { ContentViewer, }, + provide: { + glFeatures: flags, + }, }); await waitForPromises(); vm = wrapper.vm; @@ -137,6 +154,7 @@ describe('RepoEditor', () => { createDiffInstanceSpy = jest.spyOn(SourceEditor.prototype, EDITOR_DIFF_INSTANCE_FN); createModelSpy = jest.spyOn(monacoEditor, 'createModel'); applyExtensionSpy = jest.spyOn(SourceEditorInstance.prototype, 'use'); + removeExtensionSpy = jest.spyOn(SourceEditorInstance.prototype, 'unuse'); jest.spyOn(service, 'getFileData').mockResolvedValue(); jest.spyOn(service, 'getRawFileData').mockResolvedValue(); }); @@ -177,6 +195,76 @@ describe('RepoEditor', () => { }); }); + describe('schema registration for .gitlab-ci.yml', () => { + const setup = async (activeFile, flagIsOn = true) => { + await createComponent({ + flags: { + schemaLinting: flagIsOn, + }, + }); + vm.editor.registerCiSchema = jest.fn(); + if (activeFile) { + wrapper.setProps({ file: activeFile }); + } + await waitForPromises(); + await nextTick(); + }; + it.each` + flagIsOn | activeFile | shouldUseExtension | desc + ${false} | ${dummyFile.markdown} | ${false} | ${`file is not CI config; should NOT`} + ${true} | ${dummyFile.markdown} | ${false} | ${`file is not CI config; should NOT`} + ${false} | ${dummyFile.ciConfig} | ${false} | ${`file is CI config; should NOT`} + ${true} | ${dummyFile.ciConfig} | ${true} | ${`file is CI config; should`} + `( + 'when the flag is "$flagIsOn", $desc use extension', + async ({ flagIsOn, activeFile, shouldUseExtension }) => { + await setup(activeFile, flagIsOn); + + if (shouldUseExtension) { + expect(applyExtensionSpy).toHaveBeenCalledWith({ + definition: CiSchemaExtension, + }); + } else { + expect(applyExtensionSpy).not.toHaveBeenCalledWith({ + definition: CiSchemaExtension, + }); + } + }, + ); + it('stores the fetched extension and does not double-fetch the schema', async () => { + await setup(); + expect(CiSchemaExtension).toHaveBeenCalledTimes(0); + + wrapper.setProps({ file: dummyFile.ciConfig }); + await waitForPromises(); + await nextTick(); + expect(CiSchemaExtension).toHaveBeenCalledTimes(1); + expect(vm.CiSchemaExtension).toEqual(CiSchemaExtension); + expect(vm.editor.registerCiSchema).toHaveBeenCalledTimes(1); + + wrapper.setProps({ file: dummyFile.markdown }); + await waitForPromises(); + await nextTick(); + expect(CiSchemaExtension).toHaveBeenCalledTimes(1); + expect(vm.editor.registerCiSchema).toHaveBeenCalledTimes(1); + + wrapper.setProps({ file: dummyFile.ciConfig }); + await waitForPromises(); + await nextTick(); + expect(CiSchemaExtension).toHaveBeenCalledTimes(1); + expect(vm.editor.registerCiSchema).toHaveBeenCalledTimes(2); + }); + it('unuses the existing CI extension if the new model is not CI config', async () => { + await setup(dummyFile.ciConfig); + + expect(removeExtensionSpy).not.toHaveBeenCalled(); + wrapper.setProps({ file: dummyFile.markdown }); + await waitForPromises(); + await nextTick(); + expect(removeExtensionSpy).toHaveBeenCalledWith(CiSchemaExtension); + }); + }); + describe('when file is markdown', () => { let mock; let activeFile; diff --git a/spec/frontend/jobs/components/job/empty_state_spec.js b/spec/frontend/jobs/components/job/empty_state_spec.js index 299b607ad78..e1b9aa743e0 100644 --- a/spec/frontend/jobs/components/job/empty_state_spec.js +++ b/spec/frontend/jobs/components/job/empty_state_spec.js @@ -1,5 +1,6 @@ import { mount } from '@vue/test-utils'; import EmptyState from '~/jobs/components/job/empty_state.vue'; +import { mockId } from './mock_data'; describe('Empty State', () => { let wrapper; @@ -7,6 +8,7 @@ describe('Empty State', () => { const defaultProps = { illustrationPath: 'illustrations/pending_job_empty.svg', illustrationSizeClass: 'svg-430', + jobId: mockId, title: 'This job has not started yet', playable: false, }; diff --git a/spec/frontend/jobs/components/job/job_sidebar_retry_button_spec.js b/spec/frontend/jobs/components/job/job_sidebar_retry_button_spec.js index 18d5f35bde4..b04a5e07ea5 100644 --- a/spec/frontend/jobs/components/job/job_sidebar_retry_button_spec.js +++ b/spec/frontend/jobs/components/job/job_sidebar_retry_button_spec.js @@ -16,6 +16,7 @@ describe('Job Sidebar Retry Button', () => { wrapper = shallowMountExtended(JobsSidebarRetryButton, { propsData: { href: job.retry_path, + isManualJob: true, modalId: 'modal-id', ...props, }, diff --git a/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js b/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js index 95eb10118ee..8fbb418232b 100644 --- a/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js +++ b/spec/frontend/jobs/components/job/legacy_sidebar_header_spec.js @@ -32,12 +32,8 @@ describe('Legacy Sidebar Header', () => { }); describe('when job log is erasable', () => { - const path = '/root/ci-project/-/jobs/1447/erase'; - beforeEach(() => { - createWrapper({ - erasePath: path, - }); + createWrapper(); }); it('renders erase job link', () => { @@ -45,13 +41,13 @@ describe('Legacy Sidebar Header', () => { }); it('erase job link has correct path', () => { - expect(findEraseLink().attributes('href')).toBe(path); + expect(findEraseLink().attributes('href')).toBe(job.erase_path); }); }); describe('when job log is not erasable', () => { beforeEach(() => { - createWrapper(); + createWrapper({ job: { ...job, erase_path: null } }); }); it('does not render erase button', () => { @@ -77,8 +73,7 @@ describe('Legacy Sidebar Header', () => { describe('when there is no retry path', () => { it('should not render a retry button', async () => { - const copy = { ...job, retry_path: null }; - createWrapper({ job: copy }); + createWrapper({ job: { ...job, retry_path: null } }); expect(findRetryButton().exists()).toBe(false); }); @@ -100,9 +95,7 @@ describe('Legacy Sidebar Header', () => { it('should have a different label when the job status is failed', () => { createWrapper({ job: { ...job, status: failedJobStatus } }); - expect(findRetryButton().attributes('title')).toBe( - LegacySidebarHeader.i18n.retryJobButtonLabel, - ); + expect(findRetryButton().attributes('title')).toBe(LegacySidebarHeader.i18n.retryJobLabel); }); }); }); diff --git a/spec/frontend/jobs/components/job/manual_variables_form_spec.js b/spec/frontend/jobs/components/job/manual_variables_form_spec.js index 5806f9f75f9..4384b2f4d7f 100644 --- a/spec/frontend/jobs/components/job/manual_variables_form_spec.js +++ b/spec/frontend/jobs/components/job/manual_variables_form_spec.js @@ -1,46 +1,70 @@ import { GlSprintf, GlLink } from '@gitlab/ui'; -import { mount } from '@vue/test-utils'; -import Vue, { nextTick } from 'vue'; -import Vuex from 'vuex'; -import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import { createLocalVue } from '@vue/test-utils'; +import VueApollo from 'vue-apollo'; +import { nextTick } from 'vue'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { GRAPHQL_ID_TYPES } from '~/jobs/constants'; +import waitForPromises from 'helpers/wait_for_promises'; import ManualVariablesForm from '~/jobs/components/job/manual_variables_form.vue'; - -Vue.use(Vuex); +import getJobQuery from '~/jobs/components/job/graphql/queries/get_job.query.graphql'; +import retryJobMutation from '~/jobs/components/job/graphql/mutations/job_retry_with_variables.mutation.graphql'; +import { + mockFullPath, + mockId, + mockJobResponse, + mockJobWithVariablesResponse, + mockJobMutationData, +} from './mock_data'; + +const localVue = createLocalVue(); +localVue.use(VueApollo); + +const defaultProvide = { + projectPath: mockFullPath, +}; describe('Manual Variables Form', () => { let wrapper; - let store; - - const requiredProps = { - action: { - path: '/play', - method: 'post', - button_title: 'Trigger this manual action', - }, + let mockApollo; + let getJobQueryResponse; + + const createComponent = ({ options = {}, props = {} } = {}) => { + wrapper = mountExtended(ManualVariablesForm, { + propsData: { + ...props, + jobId: mockId, + }, + provide: { + ...defaultProvide, + }, + ...options, + }); }; - const createComponent = (props = {}) => { - store = new Vuex.Store({ - actions: { - triggerManualJob: jest.fn(), - }, + const createComponentWithApollo = async ({ props = {} } = {}) => { + const requestHandlers = [[getJobQuery, getJobQueryResponse]]; + + mockApollo = createMockApollo(requestHandlers); + + const options = { + localVue, + apolloProvider: mockApollo, + }; + + createComponent({ + props, + options, }); - wrapper = extendedWrapper( - mount(ManualVariablesForm, { - propsData: { ...requiredProps, ...props }, - store, - stubs: { - GlSprintf, - }, - }), - ); + return waitForPromises(); }; const findHelpText = () => wrapper.findComponent(GlSprintf); const findHelpLink = () => wrapper.findComponent(GlLink); - - const findTriggerBtn = () => wrapper.findByTestId('trigger-manual-job-btn'); + const findCancelBtn = () => wrapper.findByTestId('cancel-btn'); + const findRerunBtn = () => wrapper.findByTestId('run-manual-job-btn'); const findDeleteVarBtn = () => wrapper.findByTestId('delete-variable-btn'); const findAllDeleteVarBtns = () => wrapper.findAllByTestId('delete-variable-btn'); const findDeleteVarBtnPlaceholder = () => wrapper.findByTestId('delete-variable-btn-placeholder'); @@ -62,95 +86,134 @@ describe('Manual Variables Form', () => { }; beforeEach(() => { - createComponent(); + getJobQueryResponse = jest.fn(); }); afterEach(() => { wrapper.destroy(); }); - it('creates a new variable when user enters a new key value', async () => { - expect(findAllVariables()).toHaveLength(1); + describe('when page renders', () => { + beforeEach(async () => { + getJobQueryResponse.mockResolvedValue(mockJobResponse); + await createComponentWithApollo(); + }); + + it('renders help text with provided link', () => { + expect(findHelpText().exists()).toBe(true); + expect(findHelpLink().attributes('href')).toBe( + '/help/ci/variables/index#add-a-cicd-variable-to-a-project', + ); + }); + + it('renders buttons', () => { + expect(findCancelBtn().exists()).toBe(true); + expect(findRerunBtn().exists()).toBe(true); + }); + }); + + describe('when job has variables', () => { + beforeEach(async () => { + getJobQueryResponse.mockResolvedValue(mockJobWithVariablesResponse); + await createComponentWithApollo(); + }); - await setCiVariableKey(); + it('sets manual job variables', () => { + const queryKey = mockJobWithVariablesResponse.data.project.job.manualVariables.nodes[0].key; + const queryValue = + mockJobWithVariablesResponse.data.project.job.manualVariables.nodes[0].value; - expect(findAllVariables()).toHaveLength(2); + expect(findCiVariableKey().element.value).toBe(queryKey); + expect(findCiVariableValue().element.value).toBe(queryValue); + }); }); - it('does not create extra empty variables', async () => { - expect(findAllVariables()).toHaveLength(1); + describe('when mutation fires', () => { + beforeEach(async () => { + await createComponentWithApollo(); + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue(mockJobMutationData); + }); - await setCiVariableKey(); + it('passes variables in correct format', async () => { + await setCiVariableKey(); - expect(findAllVariables()).toHaveLength(2); + await findCiVariableValue().setValue('new value'); - await setCiVariableKey(); + await findRerunBtn().vm.$emit('click'); - expect(findAllVariables()).toHaveLength(2); + expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledTimes(1); + expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({ + mutation: retryJobMutation, + variables: { + id: convertToGraphQLId(GRAPHQL_ID_TYPES.ciBuild, mockId), + variables: [ + { + key: 'new key', + value: 'new value', + }, + ], + }, + }); + }); }); - it('removes the correct variable row', async () => { - const variableKeyNameOne = 'key-one'; - const variableKeyNameThree = 'key-three'; + describe('updating variables in UI', () => { + beforeEach(async () => { + getJobQueryResponse.mockResolvedValue(mockJobResponse); + await createComponentWithApollo(); + }); - await setCiVariableKeyByPosition(0, variableKeyNameOne); + it('creates a new variable when user enters a new key value', async () => { + expect(findAllVariables()).toHaveLength(1); - await setCiVariableKeyByPosition(1, 'key-two'); + await setCiVariableKey(); - await setCiVariableKeyByPosition(2, variableKeyNameThree); + expect(findAllVariables()).toHaveLength(2); + }); - expect(findAllVariables()).toHaveLength(4); + it('does not create extra empty variables', async () => { + expect(findAllVariables()).toHaveLength(1); - await findAllDeleteVarBtns().at(1).trigger('click'); + await setCiVariableKey(); - expect(findAllVariables()).toHaveLength(3); + expect(findAllVariables()).toHaveLength(2); - expect(findAllCiVariableKeys().at(0).element.value).toBe(variableKeyNameOne); - expect(findAllCiVariableKeys().at(1).element.value).toBe(variableKeyNameThree); - expect(findAllCiVariableKeys().at(2).element.value).toBe(''); - }); + await setCiVariableKey(); - it('trigger button is disabled after trigger action', async () => { - expect(findTriggerBtn().props('disabled')).toBe(false); + expect(findAllVariables()).toHaveLength(2); + }); - await findTriggerBtn().trigger('click'); + it('removes the correct variable row', async () => { + const variableKeyNameOne = 'key-one'; + const variableKeyNameThree = 'key-three'; - expect(findTriggerBtn().props('disabled')).toBe(true); - }); + await setCiVariableKeyByPosition(0, variableKeyNameOne); - it('delete variable button should only show when there is more than one variable', async () => { - expect(findDeleteVarBtn().exists()).toBe(false); + await setCiVariableKeyByPosition(1, 'key-two'); - await setCiVariableKey(); + await setCiVariableKeyByPosition(2, variableKeyNameThree); - expect(findDeleteVarBtn().exists()).toBe(true); - }); + expect(findAllVariables()).toHaveLength(4); - it('delete variable button placeholder should only exist when a user cannot remove', async () => { - expect(findDeleteVarBtnPlaceholder().exists()).toBe(true); - }); + await findAllDeleteVarBtns().at(1).trigger('click'); - it('renders help text with provided link', () => { - expect(findHelpText().exists()).toBe(true); - expect(findHelpLink().attributes('href')).toBe( - '/help/ci/variables/index#add-a-cicd-variable-to-a-project', - ); - }); + expect(findAllVariables()).toHaveLength(3); - it('passes variables in correct format', async () => { - jest.spyOn(store, 'dispatch'); + expect(findAllCiVariableKeys().at(0).element.value).toBe(variableKeyNameOne); + expect(findAllCiVariableKeys().at(1).element.value).toBe(variableKeyNameThree); + expect(findAllCiVariableKeys().at(2).element.value).toBe(''); + }); - await setCiVariableKey(); + it('delete variable button should only show when there is more than one variable', async () => { + expect(findDeleteVarBtn().exists()).toBe(false); - await findCiVariableValue().setValue('new value'); + await setCiVariableKey(); - await findTriggerBtn().trigger('click'); + expect(findDeleteVarBtn().exists()).toBe(true); + }); - expect(store.dispatch).toHaveBeenCalledWith('triggerManualJob', [ - { - key: 'new key', - secret_value: 'new value', - }, - ]); + it('delete variable button placeholder should only exist when a user cannot remove', async () => { + expect(findDeleteVarBtnPlaceholder().exists()).toBe(true); + }); }); }); diff --git a/spec/frontend/jobs/components/job/mock_data.js b/spec/frontend/jobs/components/job/mock_data.js new file mode 100644 index 00000000000..9596e859475 --- /dev/null +++ b/spec/frontend/jobs/components/job/mock_data.js @@ -0,0 +1,76 @@ +export const mockFullPath = 'Commit451/lab-coat'; +export const mockId = 401; + +export const mockJobResponse = { + data: { + project: { + id: 'gid://gitlab/Project/4', + job: { + id: 'gid://gitlab/Ci::Build/401', + manualJob: true, + manualVariables: { + nodes: [], + __typename: 'CiManualVariableConnection', + }, + name: 'manual_job', + retryable: true, + status: 'SUCCESS', + __typename: 'CiJob', + }, + __typename: 'Project', + }, + }, +}; + +export const mockJobWithVariablesResponse = { + data: { + project: { + id: 'gid://gitlab/Project/4', + job: { + id: 'gid://gitlab/Ci::Build/401', + manualJob: true, + manualVariables: { + nodes: [ + { + id: 'gid://gitlab/Ci::JobVariable/150', + key: 'new key', + value: 'new value', + __typename: 'CiManualVariable', + }, + ], + __typename: 'CiManualVariableConnection', + }, + name: 'manual_job', + retryable: true, + status: 'SUCCESS', + __typename: 'CiJob', + }, + __typename: 'Project', + }, + }, +}; + +export const mockJobMutationData = { + data: { + jobRetry: { + job: { + id: 'gid://gitlab/Ci::Build/401', + manualVariables: { + nodes: [ + { + id: 'gid://gitlab/Ci::JobVariable/151', + key: 'new key', + value: 'new value', + __typename: 'CiManualVariable', + }, + ], + __typename: 'CiManualVariableConnection', + }, + webPath: '/Commit451/lab-coat/-/jobs/401', + __typename: 'CiJob', + }, + errors: [], + __typename: 'JobRetryPayload', + }, + }, +}; diff --git a/spec/frontend/jobs/components/job/sidebar_header_spec.js b/spec/frontend/jobs/components/job/sidebar_header_spec.js index cb32ca9d3dc..422e2f6207c 100644 --- a/spec/frontend/jobs/components/job/sidebar_header_spec.js +++ b/spec/frontend/jobs/components/job/sidebar_header_spec.js @@ -1,91 +1,101 @@ -import { shallowMount } from '@vue/test-utils'; -import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import { createLocalVue } from '@vue/test-utils'; +import VueApollo from 'vue-apollo'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import SidebarHeader from '~/jobs/components/job/sidebar/sidebar_header.vue'; import JobRetryButton from '~/jobs/components/job/sidebar/job_sidebar_retry_button.vue'; -import LegacySidebarHeader from '~/jobs/components/job/sidebar/legacy_sidebar_header.vue'; -import createStore from '~/jobs/store'; -import job from '../../mock_data'; +import getJobQuery from '~/jobs/components/job/graphql/queries/get_job.query.graphql'; +import { mockFullPath, mockId, mockJobResponse } from './mock_data'; -describe('Legacy Sidebar Header', () => { - let store; - let wrapper; +const localVue = createLocalVue(); +localVue.use(VueApollo); - const findCancelButton = () => wrapper.findByTestId('cancel-button'); - const findRetryButton = () => wrapper.findComponent(JobRetryButton); - const findEraseLink = () => wrapper.findByTestId('job-log-erase-link'); - - const createWrapper = (props) => { - store = createStore(); - - wrapper = extendedWrapper( - shallowMount(LegacySidebarHeader, { - propsData: { - job, - ...props, - }, - store, - }), - ); +const defaultProvide = { + projectPath: mockFullPath, +}; + +describe('Sidebar Header', () => { + let wrapper; + let mockApollo; + let getJobQueryResponse; + + const createComponent = ({ options = {}, props = {}, restJob = {} } = {}) => { + wrapper = shallowMountExtended(SidebarHeader, { + propsData: { + ...props, + jobId: mockId, + restJob, + }, + provide: { + ...defaultProvide, + }, + ...options, + }); }; - afterEach(() => { - wrapper.destroy(); - }); + const createComponentWithApollo = async ({ props = {}, restJob = {} } = {}) => { + const requestHandlers = [[getJobQuery, getJobQueryResponse]]; - describe('when job log is erasable', () => { - const path = '/root/ci-project/-/jobs/1447/erase'; + mockApollo = createMockApollo(requestHandlers); - beforeEach(() => { - createWrapper({ - erasePath: path, - }); - }); + const options = { + localVue, + apolloProvider: mockApollo, + }; - it('renders erase job link', () => { - expect(findEraseLink().exists()).toBe(true); + createComponent({ + props, + restJob, + options, }); - it('erase job link has correct path', () => { - expect(findEraseLink().attributes('href')).toBe(path); - }); - }); + return waitForPromises(); + }; - describe('when job log is not erasable', () => { - beforeEach(() => { - createWrapper(); - }); + const findCancelButton = () => wrapper.findByTestId('cancel-button'); + const findEraseButton = () => wrapper.findByTestId('job-log-erase-link'); + const findJobName = () => wrapper.findByTestId('job-name'); + const findRetryButton = () => wrapper.findComponent(JobRetryButton); - it('does not render erase button', () => { - expect(findEraseLink().exists()).toBe(false); - }); + beforeEach(async () => { + getJobQueryResponse = jest.fn(); }); - describe('when the job is retryable', () => { - beforeEach(() => { - createWrapper(); - }); + afterEach(() => { + wrapper.destroy(); + }); - it('should render the retry button', () => { - expect(findRetryButton().props('href')).toBe(job.retry_path); + describe('when rendering contents', () => { + beforeEach(async () => { + getJobQueryResponse.mockResolvedValue(mockJobResponse); }); - }); - describe('when there is no retry path', () => { - it('should not render a retry button', async () => { - const copy = { ...job, retry_path: null }; - createWrapper({ job: copy }); + it('renders the correct job name', async () => { + await createComponentWithApollo(); + expect(findJobName().text()).toBe(mockJobResponse.data.project.job.name); + }); + it('does not render buttons with no paths', async () => { + await createComponentWithApollo(); + expect(findCancelButton().exists()).toBe(false); + expect(findEraseButton().exists()).toBe(false); expect(findRetryButton().exists()).toBe(false); }); - }); - describe('when the job is cancelable', () => { - beforeEach(() => { - createWrapper(); + it('renders a retry button with a path', async () => { + await createComponentWithApollo({ restJob: { retry_path: 'retry/path' } }); + expect(findRetryButton().exists()).toBe(true); + }); + + it('renders a cancel button with a path', async () => { + await createComponentWithApollo({ restJob: { cancel_path: 'cancel/path' } }); + expect(findCancelButton().exists()).toBe(true); }); - it('should render link to cancel job', () => { - expect(findCancelButton().props('icon')).toBe('cancel'); - expect(findCancelButton().attributes('href')).toBe(job.cancel_path); + it('renders an erase button with a path', async () => { + await createComponentWithApollo({ restJob: { erase_path: 'erase/path' } }); + expect(findEraseButton().exists()).toBe(true); }); }); }); diff --git a/spec/frontend/lib/dompurify_spec.js b/spec/frontend/lib/dompurify_spec.js index 412408ce377..f767a673553 100644 --- a/spec/frontend/lib/dompurify_spec.js +++ b/spec/frontend/lib/dompurify_spec.js @@ -94,6 +94,11 @@ describe('~/lib/dompurify', () => { expect(sanitize('<link rel="stylesheet" href="styles.css">')).toBe(''); }); + it("doesn't allow form tags", () => { + expect(sanitize('<form>')).toBe(''); + expect(sanitize('<form method="post" action="path"></form>')).toBe(''); + }); + describe.each` type | gon ${'root'} | ${rootGon} diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 78c0d0a2b11..a46f8c13f00 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -483,7 +483,18 @@ RSpec.describe DiffHelper do end describe '#conflicts' do - let(:merge_request) { instance_double(MergeRequest, cannot_be_merged?: true) } + let(:merge_request) do + instance_double( + MergeRequest, + cannot_be_merged?: cannot_be_merged?, + source_branch_exists?: source_branch_exists?, + target_branch_exists?: target_branch_exists? + ) + end + + let(:cannot_be_merged?) { true } + let(:source_branch_exists?) { true } + let(:target_branch_exists?) { true } let(:can_be_resolved_in_ui?) { true } let(:allow_tree_conflicts) { false } let(:files) { [instance_double(Gitlab::Conflict::File, path: 'a')] } @@ -508,7 +519,23 @@ RSpec.describe DiffHelper do end context 'when merge request can be merged' do - let(:merge_request) { instance_double(MergeRequest, cannot_be_merged?: false) } + let(:cannot_be_merged?) { false } + + it 'returns nil' do + expect(helper.conflicts).to be_nil + end + end + + context 'when source branch does not exist' do + let(:source_branch_exists?) { false } + + it 'returns nil' do + expect(helper.conflicts).to be_nil + end + end + + context 'when target branch does not exist' do + let(:target_branch_exists?) { false } it 'returns nil' do expect(helper.conflicts).to be_nil diff --git a/spec/helpers/x509_helper_spec.rb b/spec/helpers/x509_helper_spec.rb index 4e3e8c8d3f6..dfe9259bd0f 100644 --- a/spec/helpers/x509_helper_spec.rb +++ b/spec/helpers/x509_helper_spec.rb @@ -57,22 +57,4 @@ RSpec.describe X509Helper do end end end - - describe '#x509_signature?' do - let(:x509_signature) { create(:x509_commit_signature) } - let(:gpg_signature) { create(:gpg_signature) } - - it 'detects a x509 signed commit' do - signature = Gitlab::X509::Signature.new( - X509Helpers::User1.signed_commit_signature, - X509Helpers::User1.signed_commit_base_data, - X509Helpers::User1.certificate_email, - X509Helpers::User1.signed_commit_time - ) - - expect(x509_signature?(x509_signature)).to be_truthy - expect(x509_signature?(signature)).to be_truthy - expect(x509_signature?(gpg_signature)).to be_falsey - end - end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index a00df3a7dda..93b4d1bf105 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1352,7 +1352,7 @@ RSpec.describe Gitlab::Git::Repository do it "returns the number of commits in the whole repository" do options = { all: true } - expect(repository.count_commits(options)).to eq(314) + expect(repository.count_commits(options)).to eq(315) end end diff --git a/spec/lib/gitlab/x509/signature_spec.rb b/spec/lib/gitlab/x509/signature_spec.rb index 31f66232f38..32b22c0accd 100644 --- a/spec/lib/gitlab/x509/signature_spec.rb +++ b/spec/lib/gitlab/x509/signature_spec.rb @@ -11,6 +11,17 @@ RSpec.describe Gitlab::X509::Signature do } end + it_behaves_like 'signature with type checking', :x509 do + subject(:signature) do + described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + end + end + shared_examples "a verified signature" do let!(:user) { create(:user, email: X509Helpers::User1.certificate_email) } diff --git a/spec/models/commit_signatures/gpg_signature_spec.rb b/spec/models/commit_signatures/gpg_signature_spec.rb index 1ffaaeba396..75cc5d448df 100644 --- a/spec/models/commit_signatures/gpg_signature_spec.rb +++ b/spec/models/commit_signatures/gpg_signature_spec.rb @@ -23,6 +23,7 @@ RSpec.describe CommitSignatures::GpgSignature do it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' + it_behaves_like 'signature with type checking', :gpg describe 'associations' do it { is_expected.to belong_to(:gpg_key) } @@ -86,9 +87,9 @@ RSpec.describe CommitSignatures::GpgSignature do end end - describe '#user' do + describe '#signed_by_user' do it 'retrieves the gpg_key user' do - expect(signature.user).to eq(gpg_key.user) + expect(signature.signed_by_user).to eq(gpg_key.user) end end end diff --git a/spec/models/commit_signatures/ssh_signature_spec.rb b/spec/models/commit_signatures/ssh_signature_spec.rb index 08530bf6964..629d9c5ec53 100644 --- a/spec/models/commit_signatures/ssh_signature_spec.rb +++ b/spec/models/commit_signatures/ssh_signature_spec.rb @@ -22,6 +22,7 @@ RSpec.describe CommitSignatures::SshSignature do it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' + it_behaves_like 'signature with type checking', :ssh describe 'associations' do it { is_expected.to belong_to(:key).optional } @@ -37,4 +38,10 @@ RSpec.describe CommitSignatures::SshSignature do ).to contain_exactly(signature, another_signature) end end + + describe '#signed_by_user' do + it 'returns the user associated with the SSH key' do + expect(signature.signed_by_user).to eq(ssh_key.user) + end + end end diff --git a/spec/models/commit_signatures/x509_commit_signature_spec.rb b/spec/models/commit_signatures/x509_commit_signature_spec.rb index b971fd078e2..cceb96ec70d 100644 --- a/spec/models/commit_signatures/x509_commit_signature_spec.rb +++ b/spec/models/commit_signatures/x509_commit_signature_spec.rb @@ -23,6 +23,7 @@ RSpec.describe CommitSignatures::X509CommitSignature do it_behaves_like 'having unique enum values' it_behaves_like 'commit signature' + it_behaves_like 'signature with type checking', :x509 describe 'validation' do it { is_expected.to validate_presence_of(:x509_certificate_id) } @@ -37,12 +38,12 @@ RSpec.describe CommitSignatures::X509CommitSignature do let!(:user) { create(:user, email: X509Helpers::User1.certificate_email) } it 'returns user' do - expect(described_class.safe_create!(attributes).user).to eq(user) + expect(described_class.safe_create!(attributes).signed_by_user).to eq(user) end end it 'if email is not assigned to a user, return nil' do - expect(described_class.safe_create!(attributes).user).to be_nil + expect(described_class.safe_create!(attributes).signed_by_user).to be_nil end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index bab6247d4f9..4b5aabe745b 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -828,12 +828,14 @@ eos describe 'signed commits' do let(:gpg_signed_commit) { project.commit_by(oid: '0b4bc9a49b562e85de7cc9e834518ea6828729b9') } let(:x509_signed_commit) { project.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') } + let(:ssh_signed_commit) { project.commit_by(oid: '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9') } let(:unsigned_commit) { project.commit_by(oid: '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51') } let!(:commit) { create(:commit, project: project) } it 'returns signature_type properly' do expect(gpg_signed_commit.signature_type).to eq(:PGP) expect(x509_signed_commit.signature_type).to eq(:X509) + expect(ssh_signed_commit.signature_type).to eq(:SSH) expect(unsigned_commit.signature_type).to eq(:NONE) expect(commit.signature_type).to eq(:NONE) end @@ -841,9 +843,24 @@ eos it 'returns has_signature? properly' do expect(gpg_signed_commit.has_signature?).to be_truthy expect(x509_signed_commit.has_signature?).to be_truthy + expect(ssh_signed_commit.has_signature?).to be_truthy expect(unsigned_commit.has_signature?).to be_falsey expect(commit.has_signature?).to be_falsey end + + context 'when feature flag "ssh_commit_signatures" is disabled' do + before do + stub_feature_flags(ssh_commit_signatures: false) + end + + it 'reports no signature' do + expect(ssh_signed_commit).not_to have_signature + end + + it 'does not return signature data' do + expect(ssh_signed_commit.signature).to be_nil + end + end end describe '#has_been_reverted?' do diff --git a/spec/models/concerns/commit_signature_spec.rb b/spec/models/concerns/commit_signature_spec.rb new file mode 100644 index 00000000000..4bba5a6ee41 --- /dev/null +++ b/spec/models/concerns/commit_signature_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CommitSignature do + describe '#signed_by_user' do + context 'when class does not define the signed_by_user method' do + subject(:implementation) do + Class.new(ActiveRecord::Base) do + self.table_name = 'ssh_signatures' + end.include(described_class).new + end + + it 'raises a NoMethodError with custom message' do + expect do + implementation.signed_by_user + end.to raise_error(NoMethodError, 'must implement `signed_by_user` method') + end + end + end +end diff --git a/spec/models/concerns/signature_type_spec.rb b/spec/models/concerns/signature_type_spec.rb new file mode 100644 index 00000000000..d8e2b617e0e --- /dev/null +++ b/spec/models/concerns/signature_type_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe SignatureType do + describe '#type' do + context 'when class does not define a type method' do + subject(:implementation) { Class.new.include(described_class).new } + + it 'raises a NoMethodError with custom message' do + expect { implementation.type }.to raise_error(NoMethodError, 'must implement `type` method') + end + end + end +end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 8a08d5203fd..acb6c323e13 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -2206,7 +2206,7 @@ RSpec.describe API::Commits do end describe 'GET /projects/:id/repository/commits/:sha/signature' do - let!(:project) { create(:project, :repository, :public) } + let_it_be(:project) { create(:project, :repository, :public) } let(:project_id) { project.id } let(:commit_id) { project.repository.commit.id } let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}/signature" } @@ -2228,7 +2228,7 @@ RSpec.describe API::Commits do end context 'gpg signed commit' do - let(:commit) { project.repository.commit(GpgHelpers::SIGNED_COMMIT_SHA) } + let!(:commit) { project.commit(GpgHelpers::SIGNED_COMMIT_SHA) } let(:commit_id) { commit.id } it 'returns correct JSON' do @@ -2244,8 +2244,8 @@ RSpec.describe API::Commits do end context 'x509 signed commit' do - let(:commit) { project.repository.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') } - let(:commit_id) { commit.id } + let(:commit_id) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' } + let!(:commit) { project.commit(commit_id) } it 'returns correct JSON' do get api(route, current_user) @@ -2276,5 +2276,59 @@ RSpec.describe API::Commits do end end end + + context 'with ssh signed commit' do + let(:commit_id) { '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9' } + let!(:commit) { project.commit(commit_id) } + + context 'when key belonging to author does not exist' do + it 'returns data without key' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['signature_type']).to eq('SSH') + expect(json_response['verification_status']).to eq(commit.signature.verification_status) + expect(json_response['key']).to be_nil + expect(json_response['commit_source']).to eq('gitaly') + end + end + + context 'when key belonging to author exists' do + let(:user) { create(:user, email: commit.committer_email) } + let!(:key) { create(:key, user: user, key: extract_public_key_from_commit(commit), expires_at: 2.days.from_now) } + + def extract_public_key_from_commit(commit) + ssh_commit = Gitlab::Ssh::Commit.new(commit) + signature_data = ::SSHData::Signature.parse_pem(ssh_commit.signature_text) + signature_data.public_key.openssh + end + + it 'returns data including key' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['signature_type']).to eq('SSH') + expect(json_response['verification_status']).to eq(commit.signature.verification_status) + expect(json_response['key']['id']).to eq(key.id) + expect(json_response['key']['title']).to eq(key.title) + expect(json_response['key']['key']).to eq(key.publishable_key) + expect(Time.parse(json_response['key']['created_at'])).to be_like_time(key.created_at) + expect(Time.parse(json_response['key']['expires_at'])).to be_like_time(key.expires_at) + expect(json_response['commit_source']).to eq('gitaly') + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(ssh_commit_signatures: false) + end + + it 'returns 404' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end end diff --git a/spec/rubocop/cop/filename_length_spec.rb b/spec/rubocop/cop/filename_length_spec.rb index 1ea368d282f..a5bdce9a339 100644 --- a/spec/rubocop/cop/filename_length_spec.rb +++ b/spec/rubocop/cop/filename_length_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'rubocop_spec_helper' -require 'rubocop/rspec/support' require_relative '../../../rubocop/cop/filename_length' RSpec.describe RuboCop::Cop::FilenameLength do diff --git a/spec/rubocop/cop/gitlab/feature_available_usage_spec.rb b/spec/rubocop/cop/gitlab/feature_available_usage_spec.rb index 30edd33a318..b15c298099d 100644 --- a/spec/rubocop/cop/gitlab/feature_available_usage_spec.rb +++ b/spec/rubocop/cop/gitlab/feature_available_usage_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true require 'rubocop_spec_helper' -require 'rubocop' -require 'rubocop/rspec/support' require_relative '../../../../rubocop/cop/gitlab/feature_available_usage' RSpec.describe RuboCop::Cop::Gitlab::FeatureAvailableUsage do diff --git a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb index 6e60889f737..bfc0cebe203 100644 --- a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb +++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true require 'rubocop_spec_helper' -require 'rubocop' -require 'rubocop/rspec/support' require_relative '../../../../rubocop/cop/gitlab/mark_used_feature_flags' RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do diff --git a/spec/rubocop/cop/user_admin_spec.rb b/spec/rubocop/cop/user_admin_spec.rb index 99e87d619c0..21bf027324b 100644 --- a/spec/rubocop/cop/user_admin_spec.rb +++ b/spec/rubocop/cop/user_admin_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true require 'rubocop_spec_helper' - -require 'rubocop' require_relative '../../../rubocop/cop/user_admin' RSpec.describe RuboCop::Cop::UserAdmin do diff --git a/spec/rubocop/formatter/graceful_formatter_spec.rb b/spec/rubocop/formatter/graceful_formatter_spec.rb index 1ed8533ac16..d76e566e2b4 100644 --- a/spec/rubocop/formatter/graceful_formatter_spec.rb +++ b/spec/rubocop/formatter/graceful_formatter_spec.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'rubocop_spec_helper' require 'rspec-parameterized' -require 'rubocop' -require 'rubocop/rspec/support' require 'stringio' require_relative '../../../rubocop/formatter/graceful_formatter' diff --git a/spec/rubocop/support_workaround.rb b/spec/rubocop/support_workaround.rb new file mode 100644 index 00000000000..d83aa8a7232 --- /dev/null +++ b/spec/rubocop/support_workaround.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +# This replicates `require 'rubocop/rspec/support'` to workaround the issue +# in https://gitlab.com/gitlab-org/gitlab/-/issues/382452. +# +# All helpers are only included in rubocop specs (type: :rubocop/:rubocop_rspec). + +require 'rubocop/rspec/cop_helper' +require 'rubocop/rspec/host_environment_simulation_helper' +require 'rubocop/rspec/shared_contexts' +require 'rubocop/rspec/expect_offense' +require 'rubocop/rspec/parallel_formatter' + +RSpec.configure do |config| + config.include CopHelper, type: :rubocop + config.include CopHelper, type: :rubocop_rspec + config.include HostEnvironmentSimulatorHelper, type: :rubocop + config.include HostEnvironmentSimulatorHelper, type: :rubocop_rspec + config.include_context 'config', :config + config.include_context 'isolated environment', :isolated_environment + config.include_context 'maintain registry', :restore_registry + config.include_context 'ruby 2.0', :ruby20 + config.include_context 'ruby 2.1', :ruby21 + config.include_context 'ruby 2.2', :ruby22 + config.include_context 'ruby 2.3', :ruby23 + config.include_context 'ruby 2.4', :ruby24 + config.include_context 'ruby 2.5', :ruby25 + config.include_context 'ruby 2.6', :ruby26 + config.include_context 'ruby 2.7', :ruby27 + config.include_context 'ruby 3.0', :ruby30 + config.include_context 'ruby 3.1', :ruby31 + config.include_context 'ruby 3.2', :ruby32 +end diff --git a/spec/rubocop_spec_helper.rb b/spec/rubocop_spec_helper.rb index 6c6e588d42f..9884cdd0272 100644 --- a/spec/rubocop_spec_helper.rb +++ b/spec/rubocop_spec_helper.rb @@ -6,9 +6,10 @@ require 'fast_spec_helper' # To prevent load order issues we need to require `rubocop` first. # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47008 require 'rubocop' -require 'rubocop/rspec/support' require 'rubocop/rspec/shared_contexts/default_rspec_language_config_context' +require_relative 'rubocop/support_workaround' + RSpec.configure do |config| config.define_derived_metadata(file_path: %r{spec/rubocop}) do |metadata| metadata[:type] = :rubocop diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index e1b461cf37e..6292cf83297 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -91,7 +91,8 @@ module TestEnv 'utf-16' => 'f05a987', 'gitaly-rename-test' => '94bb47c', 'smime-signed-commits' => 'ed775cc', - 'Ääh-test-utf-8' => '7975be0' + 'Ääh-test-utf-8' => '7975be0', + 'ssh-signed-commit' => '7b5160f' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily diff --git a/spec/support/shared_examples/models/concerns/signature_type_shared_examples.rb b/spec/support/shared_examples/models/concerns/signature_type_shared_examples.rb new file mode 100644 index 00000000000..728855b74f8 --- /dev/null +++ b/spec/support/shared_examples/models/concerns/signature_type_shared_examples.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +METHODS = %i[ + gpg? + ssh? + x509? +].freeze + +RSpec.shared_examples 'signature with type checking' do |type| + describe 'signature type checkers' do + where(:method, :expected) do + METHODS.map do |method| + [method, method == "#{type}?".to_sym] + end + end + + with_them do + specify { expect(subject.public_send(method)).to eq(expected) } + end + end +end |