Merge branch 'pedropombeiro/326102-remove-group-project-params' into 'master'

Remove group and project params from runner install

See merge request gitlab-org/gitlab!57720

app/controllers/concerns/runner_setup_scripts.rb

@@ -5,8 +5,8 @@ module RunnerSetupScripts
 
   private
 
-  def private_runner_setup_scripts(**kwargs)
-    instructions = Gitlab::Ci::RunnerInstructions.new(current_user: current_user, os: script_params[:os], arch: script_params[:arch], **kwargs)
+  def private_runner_setup_scripts
+    instructions = Gitlab::Ci::RunnerInstructions.new(os: script_params[:os], arch: script_params[:arch])
     output = {
       install: instructions.install_script,
       register: instructions.register_command

app/controllers/groups/settings/ci_cd_controller.rb

@@ -52,7 +52,7 @@ module Groups
       end
 
       def runner_setup_scripts
-        private_runner_setup_scripts(group: group)
+        private_runner_setup_scripts
       end
 
       private

app/controllers/projects/settings/ci_cd_controller.rb

@@ -62,7 +62,7 @@ module Projects
       end
 
       def runner_setup_scripts
-        private_runner_setup_scripts(project: @project)
+        private_runner_setup_scripts
       end
 
       private

app/graphql/resolvers/ci/runner_setup_resolver.rb

@@ -21,19 +21,19 @@ module Resolvers
       argument :project_id,
                type: ::Types::GlobalIDType[::Project],
                required: false,
+               deprecated: { reason: 'No longer used', milestone: '13.11' },
                description: 'Project to register the runner for.'
 
       argument :group_id,
                type: ::Types::GlobalIDType[::Group],
                required: false,
+               deprecated: { reason: 'No longer used', milestone: '13.11' },
                description: 'Group to register the runner for.'
 
       def resolve(platform:, architecture:, **args)
         instructions = Gitlab::Ci::RunnerInstructions.new(
-          current_user: current_user,
           os: platform,
-          arch: architecture,
-          **target_param(args)
+          arch: architecture
         )
 
         {

changelogs/unreleased/326102-deprecate-runner-install-arguments.yml

+---
+title: Remove groupId and projectId arguments to Runner install instructions
+merge_request: 57720
+author:
+type: changed

doc/api/graphql/reference/index.md

@@ -281,9 +281,9 @@ Returns [`RunnerSetup`](#runnersetup).
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | `architecture` | [`String!`](#string) | Architecture to generate the instructions for. |
-| `groupId` | [`GroupID`](#groupid) | Group to register the runner for. |
+| `groupId` | [`GroupID`](#groupid) | Group to register the runner for. Deprecated in 13.11: No longer used. |
 | `platform` | [`String!`](#string) | Platform to generate the instructions for. |
-| `projectId` | [`ProjectID`](#projectid) | Project to register the runner for. |
+| `projectId` | [`ProjectID`](#projectid) | Project to register the runner for. Deprecated in 13.11: No longer used. |
 
 ### `snippets`
 

lib/gitlab/ci/runner_instructions.rb

@@ -51,10 +51,7 @@ module Gitlab
 
       attr_reader :errors
 
-      def initialize(current_user:, group: nil, project: nil, os:, arch:)
-        @current_user = current_user
-        @group = group
-        @project = project
+      def initialize(os:, arch:)
         @os = os
         @arch = arch
         @errors = []
@@ -77,7 +74,7 @@ module Gitlab
           server_url = Gitlab::Routing.url_helpers.root_url(only_path: false)
           runner_executable = environment[:runner_executable]
 
-          "#{runner_executable} register --url #{server_url} --registration-token #{registration_token}"
+          "#{runner_executable} register --url #{server_url} --registration-token $REGISTRATION_TOKEN"
         end
       end
 
@@ -108,30 +105,6 @@ module Gitlab
       def get_file(path)
         File.read(Rails.root.join(path).to_s)
       end
-
-      def registration_token
-        project_token || group_token || instance_token
-      end
-
-      def project_token
-        return unless @project
-        raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_pipeline, @project)
-
-        '$REGISTRATION_TOKEN'
-      end
-
-      def group_token
-        return unless @group
-        raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, @group)
-
-        '$REGISTRATION_TOKEN'
-      end
-
-      def instance_token
-        raise Gitlab::Access::AccessDeniedError unless @current_user&.admin?
-
-        '$REGISTRATION_TOKEN'
-      end
     end
   end
 end

spec/graphql/resolvers/ci/runner_setup_resolver_spec.rb

@@ -8,12 +8,11 @@ RSpec.describe Resolvers::Ci::RunnerSetupResolver do
   describe '#resolve' do
     let(:user) { create(:user) }
 
-    subject(:resolve_subject) { resolve(described_class, ctx: { current_user: user }, args: { platform: platform, architecture: 'amd64' }.merge(target_param)) }
+    subject(:resolve_subject) { resolve(described_class, ctx: { current_user: user }, args: { platform: platform, architecture: 'amd64' }) }
 
     context 'with container platforms' do
       let(:platform) { 'docker' }
       let(:project) { create(:project) }
-      let(:target_param) { { project_id: project.to_global_id } }
 
       it 'returns install instructions' do
         expect(resolve_subject[:install_instructions]).not_to eq(nil)
@@ -27,77 +26,9 @@ RSpec.describe Resolvers::Ci::RunnerSetupResolver do
     context 'with regular platforms' do
       let(:platform) { 'linux' }
 
-      context 'without target parameter' do
-        let(:target_param) { {} }
-
-        context 'when user is not admin' do
-          it 'returns access error' do
-            expect { resolve_subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
-          end
-        end
-
-        context 'when user is admin' do
-          before do
-            user.update!(admin: true)
-          end
-
-          it 'returns install and register instructions' do
-            expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
-            expect(resolve_subject.values).not_to include(nil)
-          end
-        end
-      end
-
-      context 'with project target parameter' do
-        let(:project) { create(:project) }
-        let(:target_param) { { project_id: project.to_global_id } }
-
-        context 'when user has access to admin builds on project' do
-          before do
-            project.add_maintainer(user)
-          end
-
-          it 'returns install and register instructions' do
-            expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
-            expect(resolve_subject.values).not_to include(nil)
-          end
-        end
-
-        context 'when user does not have access to admin builds on project' do
-          before do
-            project.add_developer(user)
-          end
-
-          it 'returns access error' do
-            expect { resolve_subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
-          end
-        end
-      end
-
-      context 'with group target parameter' do
-        let(:group) { create(:group) }
-        let(:target_param) { { group_id: group.to_global_id } }
-
-        context 'when user has access to admin builds on group' do
-          before do
-            group.add_owner(user)
-          end
-
-          it 'returns install and register instructions' do
-            expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
-            expect(resolve_subject.values).not_to include(nil)
-          end
-        end
-
-        context 'when user does not have access to admin builds on group' do
-          before do
-            group.add_developer(user)
-          end
-
-          it 'returns access error' do
-            expect { resolve_subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
-          end
-        end
+      it 'returns install and register instructions' do
+        expect(resolve_subject.keys).to contain_exactly(:install_instructions, :register_instructions)
+        expect(resolve_subject.values).not_to include(nil)
       end
     end
   end

spec/lib/gitlab/ci/runner_instructions_spec.rb

@@ -6,7 +6,6 @@ RSpec.describe Gitlab::Ci::RunnerInstructions do
   using RSpec::Parameterized::TableSyntax
 
   let(:params) { {} }
-  let(:user) { create(:user) }
 
   describe 'OS' do
     Gitlab::Ci::RunnerInstructions::OS.each do |name, subject|
@@ -37,7 +36,7 @@ RSpec.describe Gitlab::Ci::RunnerInstructions do
   end
 
   describe '#install_script' do
-    subject { described_class.new(current_user: user, **params) }
+    subject { described_class.new(**params) }
 
     context 'invalid params' do
       where(:current_params, :expected_error_message) do
@@ -106,117 +105,18 @@ RSpec.describe Gitlab::Ci::RunnerInstructions do
       end
     end
 
-    context 'group' do
-      let(:group) { create(:group) }
-
-      subject { described_class.new(current_user: user, group: group, **params) }
-
-      context 'user is owner' do
-        before do
-          group.add_owner(user)
-        end
-
-        with_them do
-          let(:params) { { os: commands.each_key.first, arch: 'foo' } }
-
-          it 'have correct configurations' do
-            result = subject.register_command
-
-            expect(result).to include("#{commands[commands.each_key.first]} register")
-            expect(result).to include("--registration-token $REGISTRATION_TOKEN")
-            expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
-          end
-        end
-      end
-
-      context 'user is not owner' do
-        where(:user_permission) do
-          [:maintainer, :developer, :reporter, :guest]
-        end
-
-        with_them do
-          before do
-            create(:group_member, user_permission, group: group, user: user)
-          end
-
-          it 'raises error' do
-            result = subject.register_command
-
-            expect(result).to be_nil
-            expect(subject.errors).to include("Gitlab::Access::AccessDeniedError")
-          end
-        end
-      end
-    end
-
-    context 'project' do
-      let(:project) { create(:project) }
-
-      subject { described_class.new(current_user: user, project: project, **params) }
-
-      context 'user is maintainer' do
-        before do
-          project.add_maintainer(user)
-        end
-
-        with_them do
-          let(:params) { { os: commands.each_key.first, arch: 'foo' } }
-
-          it 'have correct configurations' do
-            result = subject.register_command
-
-            expect(result).to include("#{commands[commands.each_key.first]} register")
-            expect(result).to include("--registration-token $REGISTRATION_TOKEN")
-            expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
-          end
-        end
-      end
-
-      context 'user is not maintainer' do
-        where(:user_permission) do
-          [:developer, :reporter, :guest]
-        end
-
-        with_them do
-          before do
-            create(:project_member, user_permission, project: project, user: user)
-          end
-
-          it 'raises error' do
-            result = subject.register_command
-
-            expect(result).to be_nil
-            expect(subject.errors).to include("Gitlab::Access::AccessDeniedError")
-          end
-        end
-      end
-    end
-
     context 'instance' do
-      subject { described_class.new(current_user: user, **params) }
-
-      context 'user is admin' do
-        let(:user) { create(:user, :admin) }
-
-        with_them do
-          let(:params) { { os: commands.each_key.first, arch: 'foo' } }
+      subject { described_class.new(**params) }
 
-          it 'have correct configurations' do
-            result = subject.register_command
-
-            expect(result).to include("#{commands[commands.each_key.first]} register")
-            expect(result).to include("--registration-token $REGISTRATION_TOKEN")
-            expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
-          end
-        end
-      end
+      with_them do
+        let(:params) { { os: commands.each_key.first, arch: 'foo' } }
 
-      context 'user is not admin' do
-        it 'raises error' do
+        it 'have correct configurations' do
           result = subject.register_command
 
-          expect(result).to be_nil
-          expect(subject.errors).to include("Gitlab::Access::AccessDeniedError")
+          expect(result).to include("#{commands[commands.each_key.first]} register")
+          expect(result).to include("--registration-token $REGISTRATION_TOKEN")
+          expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}")
         end
       end
     end