diff --git a/app/services/ci/runners/unregister_runner_manager_service.rb b/app/services/ci/runners/unregister_runner_manager_service.rb
index 9b3bd4a53e2feb2fb18c3d6697120e0099a931b5..b0d838d9f7d51ac478d279486706650e56b6ce3a 100644
--- a/app/services/ci/runners/unregister_runner_manager_service.rb
+++ b/app/services/ci/runners/unregister_runner_manager_service.rb
@@ -28,7 +28,7 @@ def execute
private
def system_id_missing_error
- ServiceResponse.error(message: '`system_id` needs to be specified for runners created in the UI.')
+ ServiceResponse.error(message: '`system_id` needs to be specified.')
end
end
end
diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb
index 8ef83e57b9aa65187dedebc49767891f5745cab3..d578405fe3376bf40b8b6558f5e3795d8738b5cd 100644
--- a/lib/api/ci/runner.rb
+++ b/lib/api/ci/runner.rb
@@ -98,11 +98,15 @@ class Runner < ::API::Base
delete '/managers', urgency: :low, feature_category: :fleet_visibility do
authenticate_runner!(ensure_runner_manager: false)
- destroy_conditionally!(current_runner) do
+ runner_manager = current_runner.runner_managers.find_by_system_xid(params[:system_id])
+ not_found!('Runner manager not found') unless runner_manager
+
+ destroy_conditionally!(runner_manager) do
::Ci::Runners::UnregisterRunnerManagerService.new(
current_runner,
params[:token],
- system_id: params[:system_id]).execute
+ system_id: params[:system_id]
+ ).execute
end
end
diff --git a/spec/requests/api/ci/runner/runners_delete_spec.rb b/spec/requests/api/ci/runner/runners_delete_spec.rb
index 7ece0bed5726278cfed07e76137b697b86745947..b46decc15185bcc8bce9ac7382a134ab674e08d0 100644
--- a/spec/requests/api/ci/runner/runners_delete_spec.rb
+++ b/spec/requests/api/ci/runner/runners_delete_spec.rb
@@ -102,75 +102,67 @@
context 'with created runner' do
let!(:runner) { create(:ci_runner, :with_runner_manager, registration_type: :authenticated_user) }
+ let(:delete_params) do
+ { token: runner.token, system_id: runner.runner_managers.first.system_xid }
+ end
- context 'with matching system_id' do
- context 'when no token is provided' do
- let(:delete_params) { { system_id: runner.runner_managers.first.system_xid } }
-
- it 'returns 400 error' do
- delete_request
-
- expect(response).to have_gitlab_http_status(:bad_request)
- end
- end
-
- context 'when invalid token is provided' do
- let(:delete_params) { { token: 'invalid', system_id: runner.runner_managers.first.system_xid } }
+ it 'deletes runner manager' do
+ expect do
+ delete_request
- it 'returns 403 error' do
- delete_request
+ expect(response).to have_gitlab_http_status(:no_content)
+ end.to change { runner.runner_managers.count }.from(1).to(0)
+ .and not_change { ::Ci::Runner.count }.from(1)
+ end
- expect(response).to have_gitlab_http_status(:forbidden)
- end
- end
+ it_behaves_like '412 response' do
+ let(:request) { api('/runners/managers') }
+ let(:params) { delete_params }
end
- end
- context 'when valid token is provided' do
- context 'with created runner' do
- let!(:runner) { create(:ci_runner, :with_runner_manager, registration_type: :authenticated_user) }
+ it_behaves_like 'storing arguments in the application context for the API' do
+ let(:expected_params) { { client_id: "runner/#{runner.id}" } }
+ end
- context 'with matching system_id' do
- let(:delete_params) { { token: runner.token, system_id: runner.runner_managers.first.system_xid } }
+ context 'with unknown system_id' do
+ let(:delete_params) { { token: runner.token, system_id: 'unknown_system_id' } }
- it 'deletes runner manager' do
- expect do
- delete_request
+ it 'returns 404 error' do
+ delete_request
- expect(response).to have_gitlab_http_status(:no_content)
- end.to change { runner.runner_managers.count }.from(1).to(0)
+ expect(response).to have_gitlab_http_status(:not_found)
+ expect(response.body).to include('Runner manager not found')
+ expect(::Ci::Runner.count).to eq(1)
+ end
+ end
- expect(::Ci::Runner.count).to eq(1)
- end
+ context 'without system_id' do
+ let(:delete_params) { { token: runner.token } }
- it_behaves_like '412 response' do
- let(:request) { api('/runners/managers') }
- let(:params) { delete_params }
- end
+ it 'does not delete runner manager nor runner' do
+ delete_request
- it_behaves_like 'storing arguments in the application context for the API' do
- let(:expected_params) { { client_id: "runner/#{runner.id}" } }
- end
+ expect(response).to have_gitlab_http_status(:bad_request)
end
+ end
- context 'with unknown system_id' do
- let(:delete_params) { { token: runner.token, system_id: 'unknown_system_id' } }
+ context 'when no token is provided' do
+ let(:delete_params) { { system_id: runner.runner_managers.first.system_xid } }
- it 'returns 404 error' do
- delete_request
+ it 'returns 400 error' do
+ delete_request
- expect(response).to have_gitlab_http_status(:not_found)
- end
+ expect(response).to have_gitlab_http_status(:bad_request)
end
+ end
- context 'without system_id' do
- let(:delete_params) { { token: runner.token } }
+ context 'when invalid token is provided' do
+ let(:delete_params) { { token: 'invalid', system_id: runner.runner_managers.first.system_xid } }
- it 'does not delete runner manager nor runner' do
- delete_request
+ it 'returns 403 error' do
+ delete_request
- expect(response).to have_gitlab_http_status(:bad_request)
- end
+ expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
diff --git a/spec/services/ci/runners/unregister_runner_manager_service_spec.rb b/spec/services/ci/runners/unregister_runner_manager_service_spec.rb
index 83b8a5473e7d25c1f906d48d937a52804c2a5765..367cff7863a77de803a9e8eae6283283d4e42c69 100644
--- a/spec/services/ci/runners/unregister_runner_manager_service_spec.rb
+++ b/spec/services/ci/runners/unregister_runner_manager_service_spec.rb
@@ -74,7 +74,7 @@
it 'returns error and leaves runner_manager1', :aggregate_failures do
expect do
expect(execute).to be_error
- expect(execute.message).to eq('`system_id` needs to be specified for runners created in the UI.')
+ expect(execute.message).to eq('`system_id` needs to be specified.')
end.to not_change { Ci::Runner.count }
.and not_change { Ci::RunnerManager.count }
end