Commit 2dca2bce authored by Mark Florian's avatar Mark Florian Committed by Illya Klymov

Use GraphQL mutation for SAST Configuration

This replaces the existing form submission POST request with a GraphQL
mutation.

This change also updates some stale TODO comments to point to a more
relevant issue.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/227575.
parent 0a020f7a
......@@ -67,7 +67,7 @@ export default {
required: false,
default: false,
},
// TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575
// TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/241377
createSastMergeRequestPath: {
type: String,
required: true,
......
......@@ -29,7 +29,7 @@ export default {
canConfigureFeature() {
return Boolean(this.glFeatures.sastConfigurationUi && this.feature.configuration_path);
},
// TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575
// TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/241377
canCreateSASTMergeRequest() {
return Boolean(this.feature.type === 'sast' && this.createSastMergeRequestPath);
},
......@@ -71,7 +71,7 @@ export default {
>{{ s__('SecurityConfiguration|Enable') }}</gl-button
>
<!-- TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575 -->
<!-- TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/241377 -->
<create-merge-request-button
v-else-if="canCreateSASTMergeRequest"
:auto-devops-enabled="autoDevopsEnabled"
......
......@@ -3,7 +3,6 @@ import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
import { s__ } from '~/locale';
import sastCiConfigurationQuery from '../graphql/sast_ci_configuration.query.graphql';
import ConfigurationForm from './configuration_form.vue';
import { extractSastConfigurationEntities } from './utils';
export default {
components: {
......@@ -24,16 +23,18 @@ export default {
},
},
apollo: {
sastConfigurationEntities: {
sastCiConfiguration: {
query: sastCiConfigurationQuery,
variables() {
return {
fullPath: this.projectPath,
};
},
update: extractSastConfigurationEntities,
update({ project }) {
return project?.sastCiConfiguration;
},
result({ loading }) {
if (!loading && this.sastConfigurationEntities.length === 0) {
if (!loading && !this.sastCiConfiguration) {
this.onError();
}
},
......@@ -44,7 +45,7 @@ export default {
},
data() {
return {
sastConfigurationEntities: [],
sastCiConfiguration: null,
hasLoadingError: false,
showFeedbackAlert: true,
};
......@@ -114,6 +115,6 @@ export default {
>{{ $options.i18n.loadingErrorText }}</gl-alert
>
<configuration-form v-else :entities="sastConfigurationEntities" />
<configuration-form v-else :sast-ci-configuration="sastCiConfiguration" />
</article>
</template>
......@@ -3,10 +3,10 @@ import { GlAlert, GlButton } from '@gitlab/ui';
import * as Sentry from '@sentry/browser';
import { cloneDeep } from 'lodash';
import { __, s__ } from '~/locale';
import axios from '~/lib/utils/axios_utils';
import { redirectTo } from '~/lib/utils/url_utility';
import DynamicFields from './dynamic_fields.vue';
import { isValidConfigurationEntity } from './utils';
import configureSastMutation from '../graphql/configure_sast.mutation.graphql';
import { toSastCiConfigurationEntityInput } from './utils';
export default {
components: {
......@@ -23,17 +23,22 @@ export default {
from: 'securityConfigurationPath',
default: '',
},
projectPath: {
from: 'projectPath',
default: '',
},
},
props: {
entities: {
type: Array,
// A SastCiConfiguration GraphQL object
sastCiConfiguration: {
type: Object,
required: true,
validator: value => value.every(isValidConfigurationEntity),
},
},
data() {
return {
formEntities: cloneDeep(this.entities),
globalConfiguration: cloneDeep(this.sastCiConfiguration.global.nodes),
pipelineConfiguration: cloneDeep(this.sastCiConfiguration.pipeline.nodes),
hasSubmissionError: false,
isSubmitting: false,
};
......@@ -43,16 +48,25 @@ export default {
this.isSubmitting = true;
this.hasSubmissionError = false;
return axios
.post(this.createSastMergeRequestPath, this.getFormData())
return this.$apollo
.mutate({
mutation: configureSastMutation,
variables: {
input: {
projectPath: this.projectPath,
configuration: this.getMutationConfiguration(),
},
},
})
.then(({ data }) => {
const { filePath } = data;
if (!filePath) {
const { errors, successPath } = data.configureSast;
if (errors.length > 0 || !successPath) {
// eslint-disable-next-line @gitlab/require-i18n-strings
throw new Error('SAST merge request creation failed');
throw new Error('SAST merge request creation mutation failed');
}
redirectTo(filePath);
redirectTo(successPath);
})
.catch(error => {
this.isSubmitting = false;
......@@ -60,11 +74,11 @@ export default {
Sentry.captureException(error);
});
},
getFormData() {
return this.formEntities.reduce((acc, { field, value }) => {
acc[field] = value;
return acc;
}, {});
getMutationConfiguration() {
return {
global: this.globalConfiguration.map(toSastCiConfigurationEntityInput),
pipeline: this.pipelineConfiguration.map(toSastCiConfigurationEntityInput),
};
},
},
i18n: {
......@@ -79,7 +93,8 @@ export default {
<template>
<form @submit.prevent="onSubmit">
<dynamic-fields v-model="formEntities" />
<dynamic-fields v-model="globalConfiguration" class="gl-m-0" />
<dynamic-fields v-model="pipelineConfiguration" class="gl-m-0" />
<hr />
......
......@@ -28,11 +28,14 @@ export const isValidAnalyzerEntity = object => {
return isString(name) && isString(label) && isString(description) && isBoolean(enabled);
};
export const extractSastConfigurationEntities = ({ project }) => {
if (!project?.sastCiConfiguration) {
return [];
}
const { global, pipeline } = project.sastCiConfiguration;
return [...global.nodes, ...pipeline.nodes];
};
/**
* Given a SastCiConfigurationEntity, returns a SastCiConfigurationEntityInput
* suitable for use in the configureSast GraphQL mutation.
* @param {SastCiConfigurationEntity}
* @returns {SastCiConfigurationEntityInput}
*/
export const toSastCiConfigurationEntityInput = ({ field, defaultValue, value }) => ({
field,
defaultValue,
value,
});
mutation configureSast($input: ConfigureSastInput!) {
configureSast(input: $input) {
successPath
errors
}
}
......@@ -26,7 +26,6 @@ module Mutations
def resolve(project_path:, configuration:)
project = authorized_find!(full_path: project_path)
validate_flag!(project)
sast_create_service_params = format_for_service(configuration)
result = ::Security::CiConfiguration::SastCreateService.new(project, current_user, sast_create_service_params).execute
......@@ -35,12 +34,6 @@ module Mutations
private
def validate_flag!(project)
return if ::Feature.enabled?(:security_sast_configuration, project)
raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'security_sast_configuration flag is not enabled on this project'
end
def find_object(full_path:)
resolve_project(full_path: full_path)
end
......
---
name: security_sast_configuration
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40637
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/235929
group: group::static analysis
type: development
default_enabled: false
......@@ -2,7 +2,7 @@ import { shallowMount } from '@vue/test-utils';
import { GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
import SASTConfigurationApp from 'ee/security_configuration/sast/components/app.vue';
import ConfigurationForm from 'ee/security_configuration/sast/components/configuration_form.vue';
import { makeEntities } from './helpers';
import { makeSastCiConfiguration } from './helpers';
const sastDocumentationPath = '/help/sast';
const projectPath = 'namespace/project';
......@@ -11,11 +11,10 @@ describe('SAST Configuration App', () => {
let wrapper;
const createComponent = ({
provide = {},
stubs = {},
loading = false,
loading = true,
hasLoadingError = false,
sastConfigurationEntities = [],
sastCiConfiguration = null,
} = {}) => {
wrapper = shallowMount(SASTConfigurationApp, {
mocks: { $apollo: { loading } },
......@@ -23,16 +22,16 @@ describe('SAST Configuration App', () => {
provide: {
sastDocumentationPath,
projectPath,
...provide,
},
});
// While setData is usually frowned upon, it is the documented way of
// mocking GraphQL response data:
// https://docs.gitlab.com/ee/development/fe_guide/graphql.html#testing
wrapper.setData({
hasLoadingError,
sastConfigurationEntities,
// While setting data is usually frowned upon, it is the documented way
// of mocking GraphQL response data:
// https://docs.gitlab.com/ee/development/fe_guide/graphql.html#testing
data() {
return {
hasLoadingError,
sastCiConfiguration,
};
},
});
};
......@@ -119,6 +118,7 @@ describe('SAST Configuration App', () => {
describe('when loading failed', () => {
beforeEach(() => {
createComponent({
loading: false,
hasLoadingError: true,
});
});
......@@ -137,12 +137,13 @@ describe('SAST Configuration App', () => {
});
describe('when loaded', () => {
let entities;
let sastCiConfiguration;
beforeEach(() => {
entities = makeEntities(3);
sastCiConfiguration = makeSastCiConfiguration();
createComponent({
sastConfigurationEntities: entities,
loading: false,
sastCiConfiguration,
});
});
......@@ -154,8 +155,8 @@ describe('SAST Configuration App', () => {
expect(findConfigurationForm().exists()).toBe(true);
});
it('passes the sastConfigurationEntities to the entities prop', () => {
expect(findConfigurationForm().props('entities')).toBe(entities);
it('passes the sastCiConfiguration to the sastCiConfiguration prop', () => {
expect(findConfigurationForm().props('sastCiConfiguration')).toBe(sastCiConfiguration);
});
it('does not display an alert message', () => {
......
import * as Sentry from '@sentry/browser';
import AxiosMockAdapter from 'axios-mock-adapter';
import { GlAlert } from '@gitlab/ui';
import waitForPromises from 'helpers/wait_for_promises';
import { shallowMount } from '@vue/test-utils';
import ConfigurationForm from 'ee/security_configuration/sast/components/configuration_form.vue';
import DynamicFields from 'ee/security_configuration/sast/components/dynamic_fields.vue';
import configureSastMutation from 'ee/security_configuration/sast/graphql/configure_sast.mutation.graphql';
import { redirectTo } from '~/lib/utils/url_utility';
import axios from '~/lib/utils/axios_utils';
import { makeEntities } from './helpers';
import { makeEntities, makeSastCiConfiguration } from './helpers';
jest.mock('~/lib/utils/url_utility', () => ({
redirectTo: jest.fn(),
}));
const createSastMergeRequestPath = '/merge_request/create';
const projectPath = 'group/project';
const securityConfigurationPath = '/security/configuration';
const newMergeRequestPath = '/merge_request/new';
describe('ConfigurationForm component', () => {
let wrapper;
let entities;
let axiosMock;
let sastCiConfiguration;
const createComponent = ({ props = {} } = {}) => {
entities = makeEntities(3, { value: 'foo' });
let pendingPromiseResolvers;
const fulfillPendingPromises = () => {
pendingPromiseResolvers.forEach(resolve => resolve());
};
const createComponent = ({ mutationResult } = {}) => {
sastCiConfiguration = makeSastCiConfiguration();
wrapper = shallowMount(ConfigurationForm, {
provide: {
createSastMergeRequestPath,
projectPath,
securityConfigurationPath,
},
propsData: {
entities,
...props,
sastCiConfiguration,
},
mocks: {
$apollo: {
mutate: jest.fn(
() =>
new Promise(resolve => {
pendingPromiseResolvers.push(() =>
resolve({
data: { configureSast: mutationResult },
}),
);
}),
),
},
},
});
};
......@@ -41,52 +56,73 @@ describe('ConfigurationForm component', () => {
const findSubmitButton = () => wrapper.find({ ref: 'submitButton' });
const findErrorAlert = () => wrapper.find(GlAlert);
const findCancelButton = () => wrapper.find({ ref: 'cancelButton' });
const findDynamicFieldsComponent = () => wrapper.find(DynamicFields);
const findDynamicFieldsComponents = () => wrapper.findAll(DynamicFields);
const expectPayloadForEntities = () => {
const { post } = axiosMock.history;
expect(post).toHaveLength(1);
const expectedPayload = {
mutation: configureSastMutation,
variables: {
input: {
projectPath,
configuration: {
global: [
{
field: 'field0',
defaultValue: 'defaultValue0',
value: 'value0',
},
],
pipeline: [
{
field: 'field1',
defaultValue: 'defaultValue1',
value: 'value1',
},
],
},
},
},
};
const postedData = JSON.parse(post[0].data);
entities.forEach(entity => {
expect(postedData[entity.field]).toBe(entity.value);
});
expect(wrapper.vm.$apollo.mutate.mock.calls).toEqual([[expectedPayload]]);
};
beforeEach(() => {
axiosMock = new AxiosMockAdapter(axios);
pendingPromiseResolvers = [];
});
afterEach(() => {
wrapper.destroy();
wrapper = null;
axiosMock.restore();
});
describe('the DynamicFields component', () => {
describe.each`
type | expectedPosition
${'global'} | ${0}
${'pipeline'} | ${1}
`('the $type DynamicFields component', ({ type, expectedPosition }) => {
let dynamicFields;
beforeEach(() => {
createComponent();
dynamicFields = findDynamicFieldsComponents().at(expectedPosition);
});
it('renders', () => {
expect(findDynamicFieldsComponent().exists()).toBe(true);
expect(dynamicFields.exists()).toBe(true);
});
it('recieves a copy of the entities prop', () => {
const entitiesProp = findDynamicFieldsComponent().props('entities');
it(`receives a copy of the ${type} entities`, () => {
const entitiesProp = dynamicFields.props('entities');
expect(entitiesProp).not.toBe(entities);
expect(entitiesProp).toEqual(entities);
expect(entitiesProp).not.toBe(sastCiConfiguration[type].nodes);
expect(entitiesProp).toEqual(sastCiConfiguration[type].nodes);
});
describe('when the dynamic fields component emits an input event', () => {
let dynamicFields;
describe('when it emits an input event', () => {
let newEntities;
beforeEach(() => {
dynamicFields = findDynamicFieldsComponent();
newEntities = makeEntities(3, { value: 'foo' });
newEntities = makeEntities(1);
dynamicFields.vm.$emit(DynamicFields.model.event, newEntities);
});
......@@ -102,65 +138,68 @@ describe('ConfigurationForm component', () => {
});
describe.each`
context | filePath | statusCode | partialErrorMessage
${'a response error code'} | ${newMergeRequestPath} | ${500} | ${'500'}
${'no filePath'} | ${''} | ${200} | ${/merge request.*fail/}
`(
'given an unsuccessful endpoint response due to $context',
({ filePath, statusCode, partialErrorMessage }) => {
beforeEach(() => {
axiosMock.onPost(createSastMergeRequestPath).replyOnce(statusCode, { filePath });
createComponent();
findForm().trigger('submit');
context | successPath | errors
${'no successPath'} | ${''} | ${[]}
${'any errors'} | ${''} | ${['an error']}
`('given an unsuccessful endpoint response due to $context', ({ successPath, errors }) => {
beforeEach(() => {
createComponent({
mutationResult: {
successPath,
errors,
},
});
it('includes the value of each entity in the payload', expectPayloadForEntities);
findForm().trigger('submit');
});
it(`sets the submit button's loading prop to true`, () => {
expect(findSubmitButton().props('loading')).toBe(true);
});
it('includes the value of each entity in the payload', expectPayloadForEntities);
describe('after async tasks', () => {
beforeEach(() => waitForPromises());
it(`sets the submit button's loading prop to true`, () => {
expect(findSubmitButton().props('loading')).toBe(true);
});
it('does not call redirectTo', () => {
expect(redirectTo).not.toHaveBeenCalled();
});
describe('after async tasks', () => {
beforeEach(fulfillPendingPromises);
it('displays an alert message', () => {
expect(findErrorAlert().exists()).toBe(true);
});
it('does not call redirectTo', () => {
expect(redirectTo).not.toHaveBeenCalled();
});
it('sends the error to Sentry', () => {
expect(Sentry.captureException.mock.calls).toMatchObject([
[{ message: expect.stringMatching(partialErrorMessage) }],
]);
});
it('displays an alert message', () => {
expect(findErrorAlert().exists()).toBe(true);
});
it(`sets the submit button's loading prop to false`, () => {
expect(findSubmitButton().props('loading')).toBe(false);
});
it('sends the error to Sentry', () => {
expect(Sentry.captureException.mock.calls).toMatchObject([
[{ message: expect.stringMatching(/merge request.*fail/) }],
]);
});
describe('submitting again after a previous error', () => {
beforeEach(() => {
findForm().trigger('submit');
});
it(`sets the submit button's loading prop to false`, () => {
expect(findSubmitButton().props('loading')).toBe(false);
});
describe('submitting again after a previous error', () => {
beforeEach(() => {
findForm().trigger('submit');
});
it('hides the alert message', () => {
expect(findErrorAlert().exists()).toBe(false);
});
it('hides the alert message', () => {
expect(findErrorAlert().exists()).toBe(false);
});
});
},
);
});
});
describe('given a successful endpoint response', () => {
beforeEach(() => {
axiosMock
.onPost(createSastMergeRequestPath)
.replyOnce(200, { filePath: newMergeRequestPath });
createComponent();
createComponent({
mutationResult: {
successPath: newMergeRequestPath,
errors: [],
},
});
findForm().trigger('submit');
});
......@@ -172,7 +211,7 @@ describe('ConfigurationForm component', () => {
});
describe('after async tasks', () => {
beforeEach(() => waitForPromises());
beforeEach(fulfillPendingPromises);
it('calls redirectTo', () => {
expect(redirectTo).toHaveBeenCalledWith(newMergeRequestPath);
......
......@@ -14,10 +14,30 @@ export const makeEntities = (count, changes) =>
field: `field${i}`,
label: `label${i}`,
type: 'string',
value: `defaultValue${i}`,
value: `value${i}`,
...changes,
}));
/**
* Creates a mock SastCiConfiguration GraphQL object instance.
*
* @param {number} totalEntities - The total number of entities to create.
* @returns {SastCiConfiguration}
*/
export const makeSastCiConfiguration = (totalEntities = 2) => {
// Call makeEntities just once to ensure unique fields
const entities = makeEntities(totalEntities);
return {
global: {
nodes: entities.slice(0, totalEntities - 1),
},
pipeline: {
nodes: entities.slice(totalEntities - 1),
},
};
};
/**
* Creates an array of objects matching the shape of a GraphQl
* SastCiConfigurationAnalyzersEntity.
......
import {
isValidConfigurationEntity,
isValidAnalyzerEntity,
extractSastConfigurationEntities,
toSastCiConfigurationEntityInput,
} from 'ee/security_configuration/sast/components/utils';
import { makeEntities, makeAnalyzerEntities } from './helpers';
......@@ -54,36 +54,20 @@ describe('isValidAnalyzerEntity', () => {
});
});
describe('extractSastConfigurationEntities', () => {
describe.each`
context | response
${'which is empty'} | ${{}}
${'with no project'} | ${{ project: null }}
${'with no configuration'} | ${{ project: {} }}
`('given a response $context', ({ response }) => {
it('returns an empty array', () => {
expect(extractSastConfigurationEntities(response)).toEqual([]);
});
});
describe('toSastCiConfigurationEntityInput', () => {
let entity = makeEntities(1);
describe('given a valid response', () => {
it('returns an array of entities from the global and pipeline sections', () => {
const globalEntities = makeEntities(3, { description: 'global' });
const pipelineEntities = makeEntities(3, { description: 'pipeline' });
const response = {
project: {
sastCiConfiguration: {
global: { nodes: globalEntities },
pipeline: { nodes: pipelineEntities },
},
},
};
describe('given a SastCiConfigurationEntity', () => {
beforeEach(() => {
[entity] = makeEntities(1);
});
expect(extractSastConfigurationEntities(response)).toEqual([
...globalEntities,
...pipelineEntities,
]);
it('returns a SastCiConfigurationEntityInput object', () => {
expect(toSastCiConfigurationEntityInput(entity)).toEqual({
field: entity.field,
defaultValue: entity.defaultValue,
value: entity.value,
});
});
});
});
......@@ -32,10 +32,6 @@ RSpec.describe Mutations::Security::CiConfiguration::ConfigureSast do
)
end
before do
stub_feature_flags(security_sast_configuration: true)
end
specify { expect(described_class).to require_graphql_authorizations(:push_code) }
describe '#resolve' do
......@@ -86,14 +82,6 @@ RSpec.describe Mutations::Security::CiConfiguration::ConfigureSast do
end
end
context 'when sast configuration feature is not enabled' do
it 'raises an exception' do
stub_feature_flags(security_sast_configuration: false)
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
context 'when service can not generate any path to create a new merge request' do
it 'returns an array of errors' do
allow_next_instance_of(::Security::CiConfiguration::SastCreateService) do |service|
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment