Merge branch 'remove-feature-flag-deploy_keys_on_protected_branches' into 'master'

Clean up deploy_keys_on_protected_branches feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!53812

app/assets/javascripts/projects/settings/access_dropdown.js

@@ -11,7 +11,6 @@ export default class AccessDropdown {
     const { $dropdown, accessLevel, accessLevelsData, hasLicense = true } = options;
     this.options = options;
     this.hasLicense = hasLicense;
-    this.deployKeysOnProtectedBranchesEnabled = gon.features.deployKeysOnProtectedBranches;
     this.groups = [];
     this.accessLevel = accessLevel;
     this.accessLevelsData = accessLevelsData.roles;
@@ -330,11 +329,7 @@ export default class AccessDropdown {
           );
         })
         .catch(() => {
-          if (this.deployKeysOnProtectedBranchesEnabled) {
-            createFlash({ message: __('Failed to load groups, users and deploy keys.') });
-          } else {
-            createFlash({ message: __('Failed to load groups & users.') });
-          }
+          createFlash({ message: __('Failed to load groups, users and deploy keys.') });
         });
     } else {
       this.getDeployKeys(query)
@@ -445,35 +440,33 @@ export default class AccessDropdown {
       }
     }
 
-    if (this.deployKeysOnProtectedBranchesEnabled) {
-      const deployKeys = deployKeysResponse.map((response) => {
-        const {
-          id,
-          fingerprint,
-          title,
-          owner: { avatar_url, name, username },
-        } = response;
-
-        const shortFingerprint = `(${fingerprint.substring(0, 14)}...)`;
-
-        return {
-          id,
-          title: title.concat(' ', shortFingerprint),
-          avatar_url,
-          fullname: name,
-          username,
-          type: LEVEL_TYPES.DEPLOY_KEY,
-        };
-      });
+    const deployKeys = deployKeysResponse.map((response) => {
+      const {
+        id,
+        fingerprint,
+        title,
+        owner: { avatar_url, name, username },
+      } = response;
+
+      const shortFingerprint = `(${fingerprint.substring(0, 14)}...)`;
+
+      return {
+        id,
+        title: title.concat(' ', shortFingerprint),
+        avatar_url,
+        fullname: name,
+        username,
+        type: LEVEL_TYPES.DEPLOY_KEY,
+      };
+    });
 
-      if (this.accessLevel === ACCESS_LEVELS.PUSH) {
-        if (deployKeys.length) {
-          consolidatedData = consolidatedData.concat(
-            [{ type: 'divider' }],
-            [{ type: 'header', content: s__('AccessDropdown|Deploy Keys') }],
-            deployKeys,
-          );
-        }
+    if (this.accessLevel === ACCESS_LEVELS.PUSH) {
+      if (deployKeys.length) {
+        consolidatedData = consolidatedData.concat(
+          [{ type: 'divider' }],
+          [{ type: 'header', content: s__('AccessDropdown|Deploy Keys') }],
+          deployKeys,
+        );
       }
     }
 
@@ -501,19 +494,15 @@ export default class AccessDropdown {
   }
 
   getDeployKeys(query) {
-    if (this.deployKeysOnProtectedBranchesEnabled) {
-      return axios.get(this.buildUrl(gon.relative_url_root, this.deployKeysPath), {
-        params: {
-          search: query,
-          per_page: 20,
-          active: true,
-          project_id: gon.current_project_id,
-          push_code: true,
-        },
-      });
-    }
-
-    return Promise.resolve({ data: [] });
+    return axios.get(this.buildUrl(gon.relative_url_root, this.deployKeysPath), {
+      params: {
+        search: query,
+        per_page: 20,
+        active: true,
+        project_id: gon.current_project_id,
+        push_code: true,
+      },
+    });
   }
 
   buildUrl(urlRoot, url) {

app/controllers/projects/settings/repository_controller.rb

@@ -7,7 +7,6 @@ module Projects
       before_action :define_variables, only: [:create_deploy_token]
       before_action do
         push_frontend_feature_flag(:ajax_new_deploy_token, @project)
-        push_frontend_feature_flag(:deploy_keys_on_protected_branches, @project)
       end
 
       feature_category :source_code_management, [:show, :cleanup]

app/models/protected_branch/push_access_level.rb

@@ -19,7 +19,7 @@ class ProtectedBranch::PushAccessLevel < ApplicationRecord
   end
 
   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)
     end
 

app/views/projects/protected_branches/_create_protected_branch.html.haml

-- select_mode_for_dropdown = Feature.enabled?(:deploy_keys_on_protected_branches, @project) ? 'js-multiselect' : ''
-
 - content_for :merge_access_levels do
   .merge_access_levels-container
     = dropdown_tag('Select',
@@ -9,7 +7,7 @@
 - content_for :push_access_levels do
   .push_access_levels-container
     = 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',
                     data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }})
 

app/views/shared/projects/protected_branches/_update_protected_branch.html.haml

-- 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
 - push_access_levels = protected_branch.push_access_levels.for_role
 
@@ -25,7 +23,7 @@
 %td.push_access_levels-container
   = hidden_field_tag "allowed_to_push_#{protected_branch.id}", push_access_levels.first&.access_level
   = 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) }})
   - if user_push_access_levels.any?
     %p.small

changelogs/unreleased/remove-feature-flag-deploy_keys_on_protected_branches.yml

+---
+title: Allow deploy keys to push to a protected branch
+merge_request: 53812
+author:
+type: added

config/feature_flags/development/deploy_keys_on_protected_branches.yml

----
-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

doc/user/project/protected_branches.md

@@ -80,6 +80,7 @@ they are set to Maintainers by default.
 
 > - [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 available for all users in GitLab 13.9.
 
 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,

ee/spec/features/protected_branches_spec.rb

@@ -9,7 +9,6 @@ RSpec.describe 'Protected Branches', :js do
   let(:user) { project.owner }
 
   before do
-    stub_feature_flags(deploy_keys_on_protected_branches: false)
     sign_in(user)
   end
 
@@ -194,7 +193,7 @@ RSpec.describe 'Protected Branches', :js do
       stub_licensed_features(protected_refs_for_users: true)
     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) }
     end
   end

ee/spec/frontend/protected_branches/protected_branch_edit_spec.js

@@ -19,7 +19,6 @@ describe('EE ProtectedBranchEdit', () => {
     </div>`);
 
     jest.spyOn(ProtectedBranchEdit.prototype, 'buildDropdowns').mockImplementation();
-    gon.features = { deployKeysOnProtectedBranches: false };
 
     mock = new MockAdapter(axios);
   });

lib/gitlab/git_access.rb

@@ -319,10 +319,8 @@ module Gitlab
     end
 
     def check_change_access!
-      return if deploy_key? && !deploy_keys_on_protected_branches_enabled?
-
       if changes == ANY
-        can_push = (deploy_key? && deploy_keys_on_protected_branches_enabled?) ||
+        can_push = deploy_key? ||
                    user_can_push? ||
           project&.any_branch_allows_collaboration?(user_access.user)
 
@@ -453,7 +451,7 @@ module Gitlab
                          CiAccess.new
                        elsif user && request_from_ci_build?
                          BuildAccess.new(user, container: container)
-                       elsif deploy_key? && deploy_keys_on_protected_branches_enabled?
+                       elsif deploy_key?
                          DeployKeyAccess.new(deploy_key, container: container)
                        else
                          UserAccess.new(user, container: container)
@@ -532,10 +530,6 @@ module Gitlab
     def size_checker
       container.repository_size_checker
     end
-
-    def deploy_keys_on_protected_branches_enabled?
-      Feature.enabled?(:deploy_keys_on_protected_branches, project)
-    end
   end
 end
 

locale/gitlab.pot

@@ -12320,9 +12320,6 @@ msgstr ""
 msgid "Failed to load group activity metrics. Please try again."
 msgstr ""
 
-msgid "Failed to load groups & users."
-msgstr ""
-
 msgid "Failed to load groups, users and deploy keys."
 msgstr ""
 

spec/features/protected_branches_spec.rb

@@ -9,10 +9,6 @@ RSpec.describe 'Protected Branches', :js do
   let(:admin) { create(:admin) }
   let(:project) { create(:project, :repository) }
 
-  before do
-    stub_feature_flags(deploy_keys_on_protected_branches: false)
-  end
-
   context 'logged in as developer' do
     before do
       project.add_developer(user)
@@ -174,7 +170,7 @@ RSpec.describe 'Protected Branches', :js do
       stub_licensed_features(protected_refs_for_users: false)
     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) }
     end
   end

spec/frontend/projects/settings/access_dropdown_spec.js

@@ -14,7 +14,6 @@ describe('AccessDropdown', () => {
     `);
     const $dropdown = $('#dummy-dropdown');
     $dropdown.data('defaultLabel', defaultLabel);
-    gon.features = { deployKeysOnProtectedBranches: true };
     const options = {
       $dropdown,
       accessLevelsData: {

spec/models/protected_branch/push_access_level_spec.rb

@@ -54,16 +54,6 @@ RSpec.describe ProtectedBranch::PushAccessLevel do
         specify do
           expect(push_access_level.check_access(user)).to be_truthy
         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
 
       context 'when the deploy key is not among the active keys of this project' do

spec/support/shared_examples/features/protected_branches_access_control_ce_shared_examples.rb

@@ -56,6 +56,8 @@ RSpec.shared_examples "protected branches > access control > CE" do
           expect(first("li")).to have_content("Roles")
           find(:link, access_type_name).click
         end
+
+        find(".js-allowed-to-push").click
       end
 
       wait_for_requests

spec/support/shared_examples/features/protected_branches_with_deploy_keys_examples.rb

 # 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
-    stub_feature_flags(deploy_keys_on_protected_branches: true)
     project.add_maintainer(user)
     sign_in(user)
   end