Commit 8a1e5104 authored by Shinya Maeda's avatar Shinya Maeda

Clean up deploy_keys_on_protected_branches feature flag

This commit cleans up the deploy_keys_on_protected_branches
feature flag to enable it by default.
parent 29f7d075
...@@ -11,7 +11,6 @@ export default class AccessDropdown { ...@@ -11,7 +11,6 @@ export default class AccessDropdown {
const { $dropdown, accessLevel, accessLevelsData, hasLicense = true } = options; const { $dropdown, accessLevel, accessLevelsData, hasLicense = true } = options;
this.options = options; this.options = options;
this.hasLicense = hasLicense; this.hasLicense = hasLicense;
this.deployKeysOnProtectedBranchesEnabled = gon.features.deployKeysOnProtectedBranches;
this.groups = []; this.groups = [];
this.accessLevel = accessLevel; this.accessLevel = accessLevel;
this.accessLevelsData = accessLevelsData.roles; this.accessLevelsData = accessLevelsData.roles;
...@@ -330,11 +329,7 @@ export default class AccessDropdown { ...@@ -330,11 +329,7 @@ export default class AccessDropdown {
); );
}) })
.catch(() => { .catch(() => {
if (this.deployKeysOnProtectedBranchesEnabled) { createFlash({ message: __('Failed to load groups, users and deploy keys.') });
createFlash({ message: __('Failed to load groups, users and deploy keys.') });
} else {
createFlash({ message: __('Failed to load groups & users.') });
}
}); });
} else { } else {
this.getDeployKeys(query) this.getDeployKeys(query)
...@@ -445,35 +440,33 @@ export default class AccessDropdown { ...@@ -445,35 +440,33 @@ export default class AccessDropdown {
} }
} }
if (this.deployKeysOnProtectedBranchesEnabled) { const deployKeys = deployKeysResponse.map((response) => {
const deployKeys = deployKeysResponse.map((response) => { const {
const { id,
id, fingerprint,
fingerprint, title,
title, owner: { avatar_url, name, username },
owner: { avatar_url, name, username }, } = response;
} = response;
const shortFingerprint = `(${fingerprint.substring(0, 14)}...)`;
const shortFingerprint = `(${fingerprint.substring(0, 14)}...)`;
return {
return { id,
id, title: title.concat(' ', shortFingerprint),
title: title.concat(' ', shortFingerprint), avatar_url,
avatar_url, fullname: name,
fullname: name, username,
username, type: LEVEL_TYPES.DEPLOY_KEY,
type: LEVEL_TYPES.DEPLOY_KEY, };
}; });
});
if (this.accessLevel === ACCESS_LEVELS.PUSH) { if (this.accessLevel === ACCESS_LEVELS.PUSH) {
if (deployKeys.length) { if (deployKeys.length) {
consolidatedData = consolidatedData.concat( consolidatedData = consolidatedData.concat(
[{ type: 'divider' }], [{ type: 'divider' }],
[{ type: 'header', content: s__('AccessDropdown|Deploy Keys') }], [{ type: 'header', content: s__('AccessDropdown|Deploy Keys') }],
deployKeys, deployKeys,
); );
}
} }
} }
...@@ -501,19 +494,15 @@ export default class AccessDropdown { ...@@ -501,19 +494,15 @@ export default class AccessDropdown {
} }
getDeployKeys(query) { getDeployKeys(query) {
if (this.deployKeysOnProtectedBranchesEnabled) { return axios.get(this.buildUrl(gon.relative_url_root, this.deployKeysPath), {
return axios.get(this.buildUrl(gon.relative_url_root, this.deployKeysPath), { params: {
params: { search: query,
search: query, per_page: 20,
per_page: 20, active: true,
active: true, project_id: gon.current_project_id,
project_id: gon.current_project_id, push_code: true,
push_code: true, },
}, });
});
}
return Promise.resolve({ data: [] });
} }
buildUrl(urlRoot, url) { buildUrl(urlRoot, url) {
......
...@@ -7,7 +7,6 @@ module Projects ...@@ -7,7 +7,6 @@ module Projects
before_action :define_variables, only: [:create_deploy_token] before_action :define_variables, only: [:create_deploy_token]
before_action do before_action do
push_frontend_feature_flag(:ajax_new_deploy_token, @project) push_frontend_feature_flag(:ajax_new_deploy_token, @project)
push_frontend_feature_flag(:deploy_keys_on_protected_branches, @project)
end end
feature_category :source_code_management, [:show, :cleanup] feature_category :source_code_management, [:show, :cleanup]
......
...@@ -19,7 +19,7 @@ class ProtectedBranch::PushAccessLevel < ApplicationRecord ...@@ -19,7 +19,7 @@ class ProtectedBranch::PushAccessLevel < ApplicationRecord
end end
def check_access(user) def check_access(user)
if Feature.enabled?(:deploy_keys_on_protected_branches, project) && user && deploy_key.present? if user && deploy_key.present?
return true if user.can?(:read_project, project) && enabled_deploy_key_for_user?(deploy_key, user) return true if user.can?(:read_project, project) && enabled_deploy_key_for_user?(deploy_key, user)
end end
......
- select_mode_for_dropdown = Feature.enabled?(:deploy_keys_on_protected_branches, @project) ? 'js-multiselect' : ''
- content_for :merge_access_levels do - content_for :merge_access_levels do
.merge_access_levels-container .merge_access_levels-container
= dropdown_tag('Select', = dropdown_tag('Select',
...@@ -9,7 +7,7 @@ ...@@ -9,7 +7,7 @@
- content_for :push_access_levels do - content_for :push_access_levels do
.push_access_levels-container .push_access_levels-container
= dropdown_tag('Select', = dropdown_tag('Select',
options: { toggle_class: "js-allowed-to-push qa-allowed-to-push-select #{select_mode_for_dropdown} wide", options: { toggle_class: "js-allowed-to-push qa-allowed-to-push-select js-multiselect wide",
dropdown_class: 'dropdown-menu-selectable qa-allowed-to-push-dropdown rspec-allowed-to-push-dropdown capitalize-header', dropdown_class: 'dropdown-menu-selectable qa-allowed-to-push-dropdown rspec-allowed-to-push-dropdown capitalize-header',
data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }}) data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }})
......
- select_mode_for_dropdown = Feature.enabled?(:deploy_keys_on_protected_branches, protected_branch.project) ? 'js-multiselect' : ''
- merge_access_levels = protected_branch.merge_access_levels.for_role - merge_access_levels = protected_branch.merge_access_levels.for_role
- push_access_levels = protected_branch.push_access_levels.for_role - push_access_levels = protected_branch.push_access_levels.for_role
...@@ -25,7 +23,7 @@ ...@@ -25,7 +23,7 @@
%td.push_access_levels-container %td.push_access_levels-container
= hidden_field_tag "allowed_to_push_#{protected_branch.id}", push_access_levels.first&.access_level = hidden_field_tag "allowed_to_push_#{protected_branch.id}", push_access_levels.first&.access_level
= dropdown_tag( (push_access_levels.first&.humanize || 'Select') , = dropdown_tag( (push_access_levels.first&.humanize || 'Select') ,
options: { toggle_class: "js-allowed-to-push #{select_mode_for_dropdown}", dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container capitalize-header', options: { toggle_class: "js-allowed-to-push js-multiselect", dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container capitalize-header',
data: { field_name: "allowed_to_push_#{protected_branch.id}", preselected_items: access_levels_data(push_access_levels) }}) data: { field_name: "allowed_to_push_#{protected_branch.id}", preselected_items: access_levels_data(push_access_levels) }})
- if user_push_access_levels.any? - if user_push_access_levels.any?
%p.small %p.small
......
---
title: Allow deploy keys to push to a protected branch
merge_request: 53812
author:
type: added
---
name: deploy_keys_on_protected_branches
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/35638
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/247866
milestone: '13.5'
type: development
group: group::release
default_enabled: false
...@@ -80,6 +80,7 @@ they are set to Maintainers by default. ...@@ -80,6 +80,7 @@ they are set to Maintainers by default.
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/30769) in GitLab 13.7. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/30769) in GitLab 13.7.
> - This feature is being selectively deployed in GitLab.com 13.7, and may not be available for all users. > - This feature is being selectively deployed in GitLab.com 13.7, and may not be available for all users.
> - This feature is available for all users in GitLab 13.9.
You can allow specific machines to access protected branches in your repository with You can allow specific machines to access protected branches in your repository with
[deploy keys](deploy_keys/index.md). This can be useful for your CI/CD workflow, [deploy keys](deploy_keys/index.md). This can be useful for your CI/CD workflow,
......
...@@ -9,7 +9,6 @@ RSpec.describe 'Protected Branches', :js do ...@@ -9,7 +9,6 @@ RSpec.describe 'Protected Branches', :js do
let(:user) { project.owner } let(:user) { project.owner }
before do before do
stub_feature_flags(deploy_keys_on_protected_branches: false)
sign_in(user) sign_in(user)
end end
...@@ -194,7 +193,7 @@ RSpec.describe 'Protected Branches', :js do ...@@ -194,7 +193,7 @@ RSpec.describe 'Protected Branches', :js do
stub_licensed_features(protected_refs_for_users: true) stub_licensed_features(protected_refs_for_users: true)
end end
include_examples 'when the deploy_keys_on_protected_branches FF is turned on' do include_examples 'Deploy keys with protected branches' do
let(:all_dropdown_sections) { %w(Roles Users Deploy\ Keys) } let(:all_dropdown_sections) { %w(Roles Users Deploy\ Keys) }
end end
end end
......
...@@ -19,7 +19,6 @@ describe('EE ProtectedBranchEdit', () => { ...@@ -19,7 +19,6 @@ describe('EE ProtectedBranchEdit', () => {
</div>`); </div>`);
jest.spyOn(ProtectedBranchEdit.prototype, 'buildDropdowns').mockImplementation(); jest.spyOn(ProtectedBranchEdit.prototype, 'buildDropdowns').mockImplementation();
gon.features = { deployKeysOnProtectedBranches: false };
mock = new MockAdapter(axios); mock = new MockAdapter(axios);
}); });
......
...@@ -319,10 +319,8 @@ module Gitlab ...@@ -319,10 +319,8 @@ module Gitlab
end end
def check_change_access! def check_change_access!
return if deploy_key? && !deploy_keys_on_protected_branches_enabled?
if changes == ANY if changes == ANY
can_push = (deploy_key? && deploy_keys_on_protected_branches_enabled?) || can_push = deploy_key? ||
user_can_push? || user_can_push? ||
project&.any_branch_allows_collaboration?(user_access.user) project&.any_branch_allows_collaboration?(user_access.user)
...@@ -453,7 +451,7 @@ module Gitlab ...@@ -453,7 +451,7 @@ module Gitlab
CiAccess.new CiAccess.new
elsif user && request_from_ci_build? elsif user && request_from_ci_build?
BuildAccess.new(user, container: container) BuildAccess.new(user, container: container)
elsif deploy_key? && deploy_keys_on_protected_branches_enabled? elsif deploy_key?
DeployKeyAccess.new(deploy_key, container: container) DeployKeyAccess.new(deploy_key, container: container)
else else
UserAccess.new(user, container: container) UserAccess.new(user, container: container)
...@@ -532,10 +530,6 @@ module Gitlab ...@@ -532,10 +530,6 @@ module Gitlab
def size_checker def size_checker
container.repository_size_checker container.repository_size_checker
end end
def deploy_keys_on_protected_branches_enabled?
Feature.enabled?(:deploy_keys_on_protected_branches, project)
end
end end
end end
......
...@@ -12309,9 +12309,6 @@ msgstr "" ...@@ -12309,9 +12309,6 @@ msgstr ""
msgid "Failed to load group activity metrics. Please try again." msgid "Failed to load group activity metrics. Please try again."
msgstr "" msgstr ""
msgid "Failed to load groups & users."
msgstr ""
msgid "Failed to load groups, users and deploy keys." msgid "Failed to load groups, users and deploy keys."
msgstr "" msgstr ""
......
...@@ -9,10 +9,6 @@ RSpec.describe 'Protected Branches', :js do ...@@ -9,10 +9,6 @@ RSpec.describe 'Protected Branches', :js do
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
before do
stub_feature_flags(deploy_keys_on_protected_branches: false)
end
context 'logged in as developer' do context 'logged in as developer' do
before do before do
project.add_developer(user) project.add_developer(user)
...@@ -174,7 +170,7 @@ RSpec.describe 'Protected Branches', :js do ...@@ -174,7 +170,7 @@ RSpec.describe 'Protected Branches', :js do
stub_licensed_features(protected_refs_for_users: false) stub_licensed_features(protected_refs_for_users: false)
end end
include_examples 'when the deploy_keys_on_protected_branches FF is turned on' do include_examples 'Deploy keys with protected branches' do
let(:all_dropdown_sections) { %w(Roles Deploy\ Keys) } let(:all_dropdown_sections) { %w(Roles Deploy\ Keys) }
end end
end end
......
...@@ -14,7 +14,6 @@ describe('AccessDropdown', () => { ...@@ -14,7 +14,6 @@ describe('AccessDropdown', () => {
`); `);
const $dropdown = $('#dummy-dropdown'); const $dropdown = $('#dummy-dropdown');
$dropdown.data('defaultLabel', defaultLabel); $dropdown.data('defaultLabel', defaultLabel);
gon.features = { deployKeysOnProtectedBranches: true };
const options = { const options = {
$dropdown, $dropdown,
accessLevelsData: { accessLevelsData: {
......
...@@ -54,16 +54,6 @@ RSpec.describe ProtectedBranch::PushAccessLevel do ...@@ -54,16 +54,6 @@ RSpec.describe ProtectedBranch::PushAccessLevel do
specify do specify do
expect(push_access_level.check_access(user)).to be_truthy expect(push_access_level.check_access(user)).to be_truthy
end end
context 'when the deploy_keys_on_protected_branches FF is false' do
before do
stub_feature_flags(deploy_keys_on_protected_branches: false)
end
it 'is false' do
expect(push_access_level.check_access(user)).to be_falsey
end
end
end end
context 'when the deploy key is not among the active keys of this project' do context 'when the deploy key is not among the active keys of this project' do
......
...@@ -56,6 +56,8 @@ RSpec.shared_examples "protected branches > access control > CE" do ...@@ -56,6 +56,8 @@ RSpec.shared_examples "protected branches > access control > CE" do
expect(first("li")).to have_content("Roles") expect(first("li")).to have_content("Roles")
find(:link, access_type_name).click find(:link, access_type_name).click
end end
find(".js-allowed-to-push").click
end end
wait_for_requests wait_for_requests
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'when the deploy_keys_on_protected_branches FF is turned on' do RSpec.shared_examples 'Deploy keys with protected branches' do
before do before do
stub_feature_flags(deploy_keys_on_protected_branches: true)
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
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