Commit 2ccf3ebd authored by Mark Florian's avatar Mark Florian

Merge branch 'philipcunningham-remove-references-to-feature-flag-322672' into 'master'

Remove dast_branch_selection feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!59349
parents b270cb6a 41568c88
...@@ -1960,7 +1960,7 @@ Represents a DAST Profile. ...@@ -1960,7 +1960,7 @@ Represents a DAST Profile.
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `branch` | [`DastProfileBranch`](#dastprofilebranch) | The associated branch. Will always return `null` if `dast_branch_selection` feature flag is disabled. | | `branch` | [`DastProfileBranch`](#dastprofilebranch) | The associated branch. |
| `dastScannerProfile` | [`DastScannerProfile`](#dastscannerprofile) | The associated scanner profile. | | `dastScannerProfile` | [`DastScannerProfile`](#dastscannerprofile) | The associated scanner profile. |
| `dastSiteProfile` | [`DastSiteProfile`](#dastsiteprofile) | The associated site profile. | | `dastSiteProfile` | [`DastSiteProfile`](#dastsiteprofile) | The associated site profile. |
| `description` | [`String`](#string) | The description of the scan. | | `description` | [`String`](#string) | The description of the scan. |
......
...@@ -820,6 +820,7 @@ Alternatively, you can use the CI/CD variable `SECURE_ANALYZERS_PREFIX` to overr ...@@ -820,6 +820,7 @@ Alternatively, you can use the CI/CD variable `SECURE_ANALYZERS_PREFIX` to overr
> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/218465) in GitLab 13.3. > - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/218465) in GitLab 13.3.
> - The saved scans feature was [introduced](https://gitlab.com/groups/gitlab-org/-/epics/5100) in GitLab 13.9. > - The saved scans feature was [introduced](https://gitlab.com/groups/gitlab-org/-/epics/5100) in GitLab 13.9.
> - The option to select a branch was [introduced](https://gitlab.com/groups/gitlab-org/-/epics/4847) in GitLab 13.10. > - The option to select a branch was [introduced](https://gitlab.com/groups/gitlab-org/-/epics/4847) in GitLab 13.10.
> - DAST branch selection [feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/322672) in GitLab 13.11.
An on-demand DAST scan runs outside the DevOps life cycle. Changes in your repository don't trigger An on-demand DAST scan runs outside the DevOps life cycle. Changes in your repository don't trigger
the scan. You must start it manually. the scan. You must start it manually.
...@@ -831,10 +832,7 @@ An on-demand DAST scan: ...@@ -831,10 +832,7 @@ An on-demand DAST scan:
- Is associated with your project's default branch. - Is associated with your project's default branch.
- Is saved on creation so it can be run later. - Is saved on creation so it can be run later.
In GitLab 13.10 and later, you can select to run an on-demand scan against a specific branch. This In GitLab 13.10 and later, you can select to run an on-demand scan against a specific branch.
feature is [deployed behind a feature flag](../../feature_flags.md), enabled by default. It's
enabled on GitLab.com and recommended for production use. [GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
can opt to disable it with `Feature.disable(:dast_branch_selection)`.
### On-demand scan modes ### On-demand scan modes
......
...@@ -263,13 +263,11 @@ export default { ...@@ -263,13 +263,11 @@ export default {
fullPath: this.projectPath, fullPath: this.projectPath,
dastScannerProfileId: this.selectedScannerProfile.id, dastScannerProfileId: this.selectedScannerProfile.id,
dastSiteProfileId: this.selectedSiteProfile.id, dastSiteProfileId: this.selectedSiteProfile.id,
branchName: this.selectedBranch,
...(this.isEdit ? { id: this.dastScan.id } : {}), ...(this.isEdit ? { id: this.dastScan.id } : {}),
...serializeFormObject(this.form.fields), ...serializeFormObject(this.form.fields),
[this.isEdit ? 'runAfterUpdate' : 'runAfterCreate']: runAfter, [this.isEdit ? 'runAfterUpdate' : 'runAfterCreate']: runAfter,
}; };
if (this.glFeatures.dastBranchSelection) {
input.branchName = this.selectedBranch;
}
this.$apollo this.$apollo
.mutate({ .mutate({
...@@ -429,7 +427,7 @@ export default { ...@@ -429,7 +427,7 @@ export default {
/> />
</gl-form-group> </gl-form-group>
<gl-form-group v-if="glFeatures.dastBranchSelection" :label="__('Branch')"> <gl-form-group :label="__('Branch')">
<ref-selector <ref-selector
v-model="selectedBranch" v-model="selectedBranch"
data-testid="dast-scan-branch-input" data-testid="dast-scan-branch-input"
......
...@@ -107,11 +107,7 @@ export default { ...@@ -107,11 +107,7 @@ export default {
> >
<template #cell(name)="{ item: { name, branch, editPath } }"> <template #cell(name)="{ item: { name, branch, editPath } }">
{{ name }} {{ name }}
<dast-scan-branch <dast-scan-branch :branch="branch" :edit-path="editPath" />
v-if="glFeatures.dastBranchSelection"
:branch="branch"
:edit-path="editPath"
/>
</template> </template>
<!-- eslint-disable-next-line vue/valid-v-slot --> <!-- eslint-disable-next-line vue/valid-v-slot -->
......
...@@ -6,7 +6,6 @@ module Projects ...@@ -6,7 +6,6 @@ module Projects
before_action do before_action do
push_frontend_feature_flag(:security_dast_site_profiles_additional_fields, @project, default_enabled: :yaml) push_frontend_feature_flag(:security_dast_site_profiles_additional_fields, @project, default_enabled: :yaml)
push_frontend_feature_flag(:dast_branch_selection, @project, default_enabled: :yaml)
end end
before_action :authorize_read_on_demand_scans!, only: :index before_action :authorize_read_on_demand_scans!, only: :index
......
...@@ -8,7 +8,6 @@ module Projects ...@@ -8,7 +8,6 @@ module Projects
before_action do before_action do
authorize_read_on_demand_scans! authorize_read_on_demand_scans!
push_frontend_feature_flag(:dast_failed_site_validations, @project, default_enabled: :yaml) push_frontend_feature_flag(:dast_failed_site_validations, @project, default_enabled: :yaml)
push_frontend_feature_flag(:dast_branch_selection, @project, default_enabled: :yaml)
end end
feature_category :dynamic_application_security_testing feature_category :dynamic_application_security_testing
......
...@@ -31,8 +31,7 @@ module Mutations ...@@ -31,8 +31,7 @@ module Mutations
argument :branch_name, GraphQL::STRING_TYPE, argument :branch_name, GraphQL::STRING_TYPE,
required: false, required: false,
description: 'The associated branch. Will be ignored ' \ description: 'The associated branch.'
'if `dast_branch_selection` feature flag is disabled.'
argument :dast_site_profile_id, ::Types::GlobalIDType[::DastSiteProfile], argument :dast_site_profile_id, ::Types::GlobalIDType[::DastSiteProfile],
required: true, required: true,
...@@ -68,7 +67,7 @@ module Mutations ...@@ -68,7 +67,7 @@ module Mutations
project: project, project: project,
name: name, name: name,
description: description, description: description,
branch_name: feature_flagged_branch_name(project, branch_name), branch_name: branch_name,
dast_site_profile: dast_site_profile, dast_site_profile: dast_site_profile,
dast_scanner_profile: dast_scanner_profile, dast_scanner_profile: dast_scanner_profile,
run_after_create: run_after_create run_after_create: run_after_create
...@@ -85,12 +84,6 @@ module Mutations ...@@ -85,12 +84,6 @@ module Mutations
def allowed?(project) def allowed?(project)
project.feature_available?(:security_on_demand_scans) project.feature_available?(:security_on_demand_scans)
end end
def feature_flagged_branch_name(project, branch_name)
return unless Feature.enabled?(:dast_branch_selection, project, default_enabled: :yaml)
branch_name
end
end end
end end
end end
......
...@@ -41,8 +41,7 @@ module Mutations ...@@ -41,8 +41,7 @@ module Mutations
argument :branch_name, GraphQL::STRING_TYPE, argument :branch_name, GraphQL::STRING_TYPE,
required: false, required: false,
description: 'The associated branch. Will be ignored ' \ description: 'The associated branch.'
'if `dast_branch_selection` feature flag is disabled.'
argument :dast_site_profile_id, SiteProfileID, argument :dast_site_profile_id, SiteProfileID,
required: false, required: false,
...@@ -70,7 +69,7 @@ module Mutations ...@@ -70,7 +69,7 @@ module Mutations
dast_profile: dast_profile, dast_profile: dast_profile,
name: name, name: name,
description: description, description: description,
branch_name: feature_flagged_branch_name(project, branch_name) || dast_profile.branch_name, branch_name: branch_name,
dast_site_profile_id: as_model_id(SiteProfileID, dast_site_profile_id), dast_site_profile_id: as_model_id(SiteProfileID, dast_site_profile_id),
dast_scanner_profile_id: as_model_id(ScannerProfileID, dast_scanner_profile_id), dast_scanner_profile_id: as_model_id(ScannerProfileID, dast_scanner_profile_id),
run_after_update: run_after_update run_after_update: run_after_update
...@@ -108,12 +107,6 @@ module Mutations ...@@ -108,12 +107,6 @@ module Mutations
.execute .execute
.first .first
end end
def feature_flagged_branch_name(project, branch_name)
return unless Feature.enabled?(:dast_branch_selection, project, default_enabled: :yaml)
branch_name
end
end end
end end
end end
......
...@@ -24,19 +24,12 @@ module Types ...@@ -24,19 +24,12 @@ module Types
description: 'The associated scanner profile.' description: 'The associated scanner profile.'
field :branch, Dast::ProfileBranchType, null: true, field :branch, Dast::ProfileBranchType, null: true,
description: 'The associated branch. Will always return `null` ' \ description: 'The associated branch.',
'if `dast_branch_selection` feature flag is disabled.',
calls_gitaly: true calls_gitaly: true
field :edit_path, GraphQL::STRING_TYPE, null: true, field :edit_path, GraphQL::STRING_TYPE, null: true,
description: 'Relative web path to the edit page of a profile.' description: 'Relative web path to the edit page of a profile.'
def branch
return unless Feature.enabled?(:dast_branch_selection, object.project, default_enabled: :yaml)
object.branch
end
def edit_path def edit_path
Gitlab::Routing.url_helpers.edit_project_on_demand_scan_path(object.project, object) Gitlab::Routing.url_helpers.edit_project_on_demand_scan_path(object.project, object)
end end
......
---
title: Remove dast_branch_selection feature flag
merge_request: 59349
author:
type: other
---
name: dast_branch_selection
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55015
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/322672
milestone: '13.10'
type: development
group: group::dynamic analysis
default_enabled: true
...@@ -157,9 +157,6 @@ describe('OnDemandScansForm', () => { ...@@ -157,9 +157,6 @@ describe('OnDemandScansForm', () => {
siteProfilesLibraryPath, siteProfilesLibraryPath,
newScannerProfilePath, newScannerProfilePath,
newSiteProfilePath, newSiteProfilePath,
glFeatures: {
dastBranchSelection: true,
},
}, },
stubs: { stubs: {
GlFormInput: GlFormInputStub, GlFormInput: GlFormInputStub,
...@@ -662,99 +659,4 @@ describe('OnDemandScansForm', () => { ...@@ -662,99 +659,4 @@ describe('OnDemandScansForm', () => {
); );
}); });
}); });
describe('dastBranchSelection feature flag disabled', () => {
describe.each`
action | actionFunction | runAfter
${'submit'} | ${submitForm} | ${true}
${'save'} | ${saveScan} | ${false}
`('on $action', ({ actionFunction, runAfter }) => {
describe('when creating a new scan', () => {
beforeEach(async () => {
createShallowComponent({
provide: {
glFeatures: {
dastBranchSelection: false,
},
},
});
wrapper.vm.$apollo.mutate.mockResolvedValue({
data: {
dastProfileCreate: {
dastProfile: { editPath },
pipelineUrl,
errors: [],
},
},
});
findNameInput().vm.$emit('input', 'My daily scan');
findScannerProfilesSelector().vm.$emit('input', passiveScannerProfile.id);
findSiteProfilesSelector().vm.$emit('input', nonValidatedSiteProfile.id);
await wrapper.vm.$nextTick();
actionFunction();
});
it(`triggers dastProfileCreateMutation mutation without the branch name and runAfterCreate set to ${runAfter}`, async () => {
expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({
mutation: dastProfileCreateMutation,
variables: {
input: {
name: 'My daily scan',
dastScannerProfileId: passiveScannerProfile.id,
dastSiteProfileId: nonValidatedSiteProfile.id,
fullPath: projectPath,
runAfterCreate: runAfter,
},
},
});
});
});
describe('when editing an existing scan', () => {
beforeEach(async () => {
createShallowComponent({
propsData: {
dastScan,
},
provide: {
glFeatures: {
dastBranchSelection: false,
},
},
});
wrapper.vm.$apollo.mutate.mockResolvedValue({
data: {
dastProfileUpdate: {
dastProfile: { editPath },
pipelineUrl,
errors: [],
},
},
});
findNameInput().vm.$emit('input', 'My daily scan');
findScannerProfilesSelector().vm.$emit('input', passiveScannerProfile.id);
findSiteProfilesSelector().vm.$emit('input', nonValidatedSiteProfile.id);
await wrapper.vm.$nextTick();
actionFunction();
});
it(`triggers dastProfileUpdateMutation mutation without the branch name and runAfterUpdate set to ${runAfter}`, async () => {
expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({
mutation: dastProfileUpdateMutation,
variables: {
input: {
id: 1,
name: 'My daily scan',
description: 'Tests for SQL injections',
dastScannerProfileId: passiveScannerProfile.id,
dastSiteProfileId: nonValidatedSiteProfile.id,
fullPath: projectPath,
runAfterUpdate: runAfter,
},
},
});
});
});
});
});
}); });
...@@ -39,11 +39,6 @@ describe('EE - DastSavedScansList', () => { ...@@ -39,11 +39,6 @@ describe('EE - DastSavedScansList', () => {
Component, Component,
merge( merge(
{ {
provide: {
glFeatures: {
dastBranchSelection: true,
},
},
propsData: defaultProps, propsData: defaultProps,
}, },
options, options,
...@@ -195,19 +190,4 @@ describe('EE - DastSavedScansList', () => { ...@@ -195,19 +190,4 @@ describe('EE - DastSavedScansList', () => {
expect(redirectTo).not.toHaveBeenCalled(); expect(redirectTo).not.toHaveBeenCalled();
}); });
}); });
describe('dastBranchSelection feature flag disabled', () => {
it('does not render branch information', () => {
createFullComponent({
provide: {
glFeatures: {
dastBranchSelection: false,
},
},
propsData: { profiles: savedScans },
});
expect(wrapper.findAll(DastScanBranch)).toHaveLength(0);
});
});
}); });
...@@ -52,18 +52,8 @@ RSpec.describe Mutations::Dast::Profiles::Create do ...@@ -52,18 +52,8 @@ RSpec.describe Mutations::Dast::Profiles::Create do
end end
context "when branch_name='orphaned_branch'" do context "when branch_name='orphaned_branch'" do
context 'when the feature flag dast_branch_selection is disabled' do it 'sets the branch_name' do
it 'does not set the branch_name' do expect(subject[:dast_profile].branch_name).to eq(project.default_branch)
stub_feature_flags(dast_branch_selection: false)
expect(subject[:dast_profile].branch_name).to be_nil
end
end
context 'when the feature flag dast_branch_selection is enabled' do
it 'sets the branch_name' do
expect(subject[:dast_profile].branch_name).to eq(project.default_branch)
end
end end
end end
end end
......
...@@ -89,14 +89,6 @@ RSpec.describe Mutations::Dast::Profiles::Update do ...@@ -89,14 +89,6 @@ RSpec.describe Mutations::Dast::Profiles::Update do
end end
end end
context 'when the feature flag dast_branch_selection is disabled' do
it 'does not set the branch_name' do
stub_feature_flags(dast_branch_selection: false)
expect(subject[:dast_profile].branch_name).to eq(dast_profile.branch_name)
end
end
context 'when the dast_profile does not exist' do context 'when the dast_profile does not exist' do
let(:dast_profile_gid) { Gitlab::GlobalId.build(nil, model_name: 'Dast::Profile', id: 'does_not_exist') } let(:dast_profile_gid) { Gitlab::GlobalId.build(nil, model_name: 'Dast::Profile', id: 'does_not_exist') }
......
...@@ -21,20 +21,10 @@ RSpec.describe GitlabSchema.types['DastProfile'] do ...@@ -21,20 +21,10 @@ RSpec.describe GitlabSchema.types['DastProfile'] do
it { expect(described_class).to have_graphql_field(:branch, calls_gitaly?: true) } it { expect(described_class).to have_graphql_field(:branch, calls_gitaly?: true) }
describe 'branch field' do describe 'branch field' do
context 'when the feature flag is disabled' do it 'correctly resolves the field' do
it 'resolves nil' do expected_result = Dast::Branch.new(object)
stub_feature_flags(dast_branch_selection: false)
expect(resolve_field(:branch, object, current_user: user)).to eq(nil)
end
end
context 'when the feature flag is enabled' do
it 'correctly resolves the field' do
expected_result = Dast::Branch.new(object)
expect(resolve_field(:branch, object, current_user: user)).to eq(expected_result) expect(resolve_field(:branch, object, current_user: user)).to eq(expected_result)
end
end end
end end
......
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