Commit ff805272 authored by Mark Florian's avatar Mark Florian Committed by Natalia Tepluhina

Allow SAST configuration with existing CI file

Previously, the Security Configuration page only linked to the SAST
Configuration page for projects _without_ an existing CI file. This was
because the initial implementation did not support updating an existing
CI file.

Now that the main technical limitation has been [lifted][1] (with
[caveats][2]), the UI can direct users to the SAST Configuration page
regardless of whether their project has an existing CI file.

This also changes the configuration button to say "Configure" if the
given feature is already configured (by any means), and "Enable" if not.
Previously this was based on whether Auto DevOps was enabled, but
checking the configured status is more direct.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/240941.

[1]: https://gitlab.com/gitlab-org/gitlab/-/issues/232862
[2]: https://gitlab.com/gitlab-org/gitlab/-/issues/228959
parent 33535063
...@@ -147,17 +147,18 @@ always take the latest SAST artifact available. ...@@ -147,17 +147,18 @@ always take the latest SAST artifact available.
### Configure SAST in the UI ### Configure SAST in the UI
> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3659) in GitLab Ultimate 13.3. > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3659) in GitLab Ultimate 13.3.
> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/232862) in GitLab Ultimate 13.4.
For a project that does not have a `.gitlab-ci.yml` file, you can enable SAST with a basic You can enable and configure SAST with a basic configuration using the **SAST Configuration**
configuration using the **SAST Configuration** page: page:
1. From the project's home page, go to **Security & Compliance** > **Configuration** in the 1. From the project's home page, go to **Security & Compliance** > **Configuration** in the
left sidebar. left sidebar.
1. Click **Enable via Merge Request** on the Static Application Security Testing (SAST) row. 1. If the project does not have a `gitlab-ci.yml` file, click **Enable** in the Static Application Security Testing (SAST) row, otherwise click **Configure**.
1. Enter the appropriate SAST details into the fields on the page. See [Available variables](#available-variables) 1. Enter the custom SAST values, then click **Create Merge Request**.
for a description of these variables.
1. Click **Create Merge Request**. Custom values are stored in the `.gitlab-ci.yml` file. For variables not in the SAST Configuration page, their values are left unchanged. Default values are inherited from the GitLab SAST template.
1. Review and merge the merge request. 1. Review and merge the merge request.
### Customizing the SAST settings ### Customizing the SAST settings
......
...@@ -180,7 +180,6 @@ export default { ...@@ -180,7 +180,6 @@ export default {
<template #cell(manage)="{ item }"> <template #cell(manage)="{ item }">
<manage-feature <manage-feature
:feature="item" :feature="item"
:gitlab-ci-present="gitlabCiPresent"
:auto-devops-enabled="autoDevopsEnabled" :auto-devops-enabled="autoDevopsEnabled"
:create-sast-merge-request-path="createSastMergeRequestPath" :create-sast-merge-request-path="createSastMergeRequestPath"
/> />
......
...@@ -20,10 +20,6 @@ export default { ...@@ -20,10 +20,6 @@ export default {
type: Boolean, type: Boolean,
required: true, required: true,
}, },
gitlabCiPresent: {
type: Boolean,
required: true,
},
createSastMergeRequestPath: { createSastMergeRequestPath: {
type: String, type: String,
required: true, required: true,
...@@ -31,17 +27,11 @@ export default { ...@@ -31,17 +27,11 @@ export default {
}, },
computed: { computed: {
canConfigureFeature() { canConfigureFeature() {
return Boolean( return Boolean(this.glFeatures.sastConfigurationUi && this.feature.configuration_path);
this.glFeatures.sastConfigurationUi &&
this.feature.configuration_path &&
!this.gitlabCiPresent,
);
}, },
// 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/227575
canCreateSASTMergeRequest() { canCreateSASTMergeRequest() {
return Boolean( return Boolean(this.feature.type === 'sast' && this.createSastMergeRequestPath);
this.feature.type === 'sast' && this.createSastMergeRequestPath && !this.gitlabCiPresent,
);
}, },
getFeatureDocumentationLinkLabel() { getFeatureDocumentationLinkLabel() {
return sprintf(s__('SecurityConfiguration|Feature documentation for %{featureName}'), { return sprintf(s__('SecurityConfiguration|Feature documentation for %{featureName}'), {
...@@ -54,7 +44,7 @@ export default { ...@@ -54,7 +44,7 @@ export default {
<template> <template>
<gl-button <gl-button
v-if="canConfigureFeature && autoDevopsEnabled" v-if="canConfigureFeature && feature.configured"
:href="feature.configuration_path" :href="feature.configuration_path"
data-testid="configureButton" data-testid="configureButton"
>{{ s__('SecurityConfiguration|Configure') }}</gl-button >{{ s__('SecurityConfiguration|Configure') }}</gl-button
......
---
title: Allow SAST Configuration UI to be used when there's an existing CI file
merge_request: 40528
author:
type: changed
...@@ -177,7 +177,6 @@ describe('Security Configuration App', () => { ...@@ -177,7 +177,6 @@ describe('Security Configuration App', () => {
expect(manage.find(ManageFeature).props()).toMatchObject({ expect(manage.find(ManageFeature).props()).toMatchObject({
feature: features[i], feature: features[i],
autoDevopsEnabled: propsData.autoDevopsEnabled, autoDevopsEnabled: propsData.autoDevopsEnabled,
gitlabCiPresent: propsData.gitlabCiPresent,
createSastMergeRequestPath: propsData.createSastMergeRequestPath, createSastMergeRequestPath: propsData.createSastMergeRequestPath,
}); });
} }
......
...@@ -17,7 +17,6 @@ describe('ManageFeature component', () => { ...@@ -17,7 +17,6 @@ describe('ManageFeature component', () => {
{ {
propsData: { propsData: {
createSastMergeRequestPath, createSastMergeRequestPath,
gitlabCiPresent: false,
autoDevopsEnabled: false, autoDevopsEnabled: false,
}, },
}, },
...@@ -28,7 +27,6 @@ describe('ManageFeature component', () => { ...@@ -28,7 +27,6 @@ describe('ManageFeature component', () => {
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
wrapper = null;
}); });
const findCreateMergeRequestButton = () => wrapper.find(CreateMergeRequestButton); const findCreateMergeRequestButton = () => wrapper.find(CreateMergeRequestButton);
...@@ -44,17 +42,17 @@ describe('ManageFeature component', () => { ...@@ -44,17 +42,17 @@ describe('ManageFeature component', () => {
}; };
describe.each` describe.each`
autoDevopsEnabled | expectedTestId configured | expectedTestId
${true} | ${'configureButton'} ${true} | ${'configureButton'}
${false} | ${'enableButton'} ${false} | ${'enableButton'}
`('given autoDevopsEnabled is $autoDevopsEnabled', ({ autoDevopsEnabled, expectedTestId }) => { `('given feature.configured is $configured', ({ configured, expectedTestId }) => {
describe('given no CI file and feature with a configuration path', () => { describe('given a configuration path', () => {
beforeEach(() => { beforeEach(() => {
[feature] = generateFeatures(1, { configuration_path: 'foo' }); [feature] = generateFeatures(1, { configured, configuration_path: 'foo' });
createComponent({ createComponent({
...featureFlagEnabled, ...featureFlagEnabled,
propsData: { feature, gitlabCiPresent: false, autoDevopsEnabled }, propsData: { feature },
}); });
}); });
...@@ -67,14 +65,14 @@ describe('ManageFeature component', () => { ...@@ -67,14 +65,14 @@ describe('ManageFeature component', () => {
}); });
}); });
describe('given a feature with type "sast" and no CI file', () => { describe('given a feature with type "sast"', () => {
const autoDevopsEnabled = true; const autoDevopsEnabled = true;
beforeEach(() => { beforeEach(() => {
[feature] = generateFeatures(1, { type: 'sast' }); [feature] = generateFeatures(1, { type: 'sast' });
createComponent({ createComponent({
propsData: { feature, gitlabCiPresent: false, autoDevopsEnabled }, propsData: { feature, autoDevopsEnabled },
}); });
}); });
...@@ -88,27 +86,20 @@ describe('ManageFeature component', () => { ...@@ -88,27 +86,20 @@ describe('ManageFeature component', () => {
}); });
}); });
describe.each` describe('given a feature type that is not "sast"', () => {
featureProps | gitlabCiPresent beforeEach(() => {
${{ type: 'sast' }} | ${true} [feature] = generateFeatures(1, { type: 'something_that_is_not_sast' });
${{}} | ${false}
`(
'given a featureProps with $featureProps and gitlabCiPresent is $gitlabCiPresent',
({ featureProps, gitlabCiPresent }) => {
beforeEach(() => {
[feature] = generateFeatures(1, featureProps);
createComponent({
propsData: { feature, gitlabCiPresent },
});
});
it('shows docs link for feature', () => { createComponent({
const link = findTestId('docsLink'); propsData: { feature },
expect(link.exists()).toBe(true);
expect(link.attributes('aria-label')).toContain(feature.name);
expect(link.attributes('href')).toBe(feature.link);
}); });
}, });
);
it('shows docs link for feature', () => {
const link = findTestId('docsLink');
expect(link.exists()).toBe(true);
expect(link.attributes('aria-label')).toContain(feature.name);
expect(link.attributes('href')).toBe(feature.link);
});
});
}); });
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