From 2ea4c98270a76bcee6076e62e34cb848373397c7 Mon Sep 17 00:00:00 2001 From: Taka Nishida <tnishida@gitlab.com> Date: Thu, 13 Mar 2025 11:54:49 +0900 Subject: [PATCH] Fix: Skip agent authorization check for environment creation in REST API This change fixes a regression introduced in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182221 which broke the cluster agent bootstrapping workflow. When creating environments via the REST API, we now skip checking for cluster agent authorization using a new `skip_agent_auth` parameter. During bootstrapping, environments are created before the agent connects for the first time, which means there are no DB entries for agent authorization yet. The previous behavior caused a 400 Unauthorized error, breaking glab cluster agent bootstrap commands. Related to https://gitlab.com/gitlab-org/cli/-/issues/7786. Changelog: fixed --- app/services/environments/create_service.rb | 8 +- lib/api/environments.rb | 1 + spec/requests/api/environments_spec.rb | 111 ++++++++---------- .../environments/create_service_spec.rb | 52 +++++--- 4 files changed, 95 insertions(+), 77 deletions(-) diff --git a/app/services/environments/create_service.rb b/app/services/environments/create_service.rb index 6736e98404ff..387d5cab613a 100644 --- a/app/services/environments/create_service.rb +++ b/app/services/environments/create_service.rb @@ -13,7 +13,13 @@ def execute ) end - if unauthorized_cluster_agent? + # Note: We skip checking the cluster agent authorization if the `skip_agent_auth` + # parameter is present. This allows creating environments that reference a cluster agent + # before the agent has connected for the first time. + # This is necessary to support the bootstrapping workflow, where environments + # are created before the agent has connected for the first time. + # See https://gitlab.com/gitlab-org/cli/-/issues/7786 for context. + if params[:skip_agent_auth].blank? && unauthorized_cluster_agent? return ServiceResponse.error( message: _('Unauthorized to access the cluster agent in this project'), payload: { environment: nil }) diff --git a/lib/api/environments.rb b/lib/api/environments.rb index 9530fbde5f9c..764fc4a02843 100644 --- a/lib/api/environments.rb +++ b/lib/api/environments.rb @@ -88,6 +88,7 @@ class Environments < ::API::Base params[:cluster_agent] = agent end + params[:skip_agent_auth] = true response = ::Environments::CreateService.new(user_project, current_user, params).execute if response.success? diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 0e4bfb776921..c393cff2cd93 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -167,86 +167,73 @@ let_it_be(:cluster_agent) { create(:cluster_agent, project: project) } let_it_be(:foreign_cluster_agent) { create(:cluster_agent) } - context 'when user access authorization exists' do - let_it_be(:user_access_auth) { create(:agent_user_access_project_authorization, project: project, agent: cluster_agent) } + it 'creates an environment with associated cluster agent' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: cluster_agent.id } - it 'creates an environment with associated cluster agent' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: cluster_agent.id } - - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('public_api/v4/environment') - expect(json_response['cluster_agent']).to be_present - end - - it 'creates an environment with associated kubernetes namespace' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: cluster_agent.id, kubernetes_namespace: 'flux-system' } - - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('public_api/v4/environment') - expect(json_response['cluster_agent']).to be_present - expect(json_response['kubernetes_namespace']).to eq('flux-system') - end + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/environment') + expect(json_response['cluster_agent']).to be_present + end - it 'creates an environment with associated flux resource path' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: cluster_agent.id, kubernetes_namespace: 'flux-system', flux_resource_path: 'HelmRelease/flux-system' } + it 'creates an environment with associated kubernetes namespace' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: cluster_agent.id, kubernetes_namespace: 'flux-system' } - expect(response).to have_gitlab_http_status(:created) - expect(response).to match_response_schema('public_api/v4/environment') - expect(json_response['cluster_agent']).to be_present - expect(json_response['kubernetes_namespace']).to eq('flux-system') - expect(json_response['flux_resource_path']).to eq('HelmRelease/flux-system') - end + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/environment') + expect(json_response['cluster_agent']).to be_present + expect(json_response['kubernetes_namespace']).to eq('flux-system') + end - it 'fails to create environment with kubernetes namespace but no cluster agent' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", kubernetes_namespace: 'flux-system' } + it 'creates an environment with associated flux resource path' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: cluster_agent.id, kubernetes_namespace: 'flux-system', flux_resource_path: 'HelmRelease/flux-system' } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq(['Kubernetes namespace cannot be set without a cluster agent']) - end + expect(response).to have_gitlab_http_status(:created) + expect(response).to match_response_schema('public_api/v4/environment') + expect(json_response['cluster_agent']).to be_present + expect(json_response['kubernetes_namespace']).to eq('flux-system') + expect(json_response['flux_resource_path']).to eq('HelmRelease/flux-system') + end - it 'fails to create environment with flux resource path but no cluster agent and kubernetes namespace' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", flux_resource_path: 'HelmRelease/flux-system' } + it 'fails to create environment with kubernetes namespace but no cluster agent' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", kubernetes_namespace: 'flux-system' } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq(['Flux resource path cannot be set without a kubernetes namespace']) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(['Kubernetes namespace cannot be set without a cluster agent']) + end - it 'fails to create environment with flux resource path but no cluster agent' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", kubernetes_namespace: 'flux-system', flux_resource_path: 'HelmRelease/flux-system' } + it 'fails to create environment with flux resource path but no cluster agent and kubernetes namespace' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", flux_resource_path: 'HelmRelease/flux-system' } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq(['Kubernetes namespace cannot be set without a cluster agent']) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(['Flux resource path cannot be set without a kubernetes namespace']) + end - it 'fails to create environment with cluster agent and flux resource path but no kubernetes namespace' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent: cluster_agent.id, flux_resource_path: 'HelmRelease/flux-system' } + it 'fails to create environment with flux resource path but no cluster agent' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", kubernetes_namespace: 'flux-system', flux_resource_path: 'HelmRelease/flux-system' } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq(['Flux resource path cannot be set without a kubernetes namespace']) - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(['Kubernetes namespace cannot be set without a cluster agent']) + end - it 'fails to create environment with a non existing cluster agent' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: non_existing_record_id } + it 'fails to create environment with cluster agent and flux resource path but no kubernetes namespace' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent: cluster_agent.id, flux_resource_path: 'HelmRelease/flux-system' } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq("400 Bad request - cluster agent doesn't exist or cannot be associated with this environment") - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(['Flux resource path cannot be set without a kubernetes namespace']) + end - it 'fails to create environment with a foreign cluster agent' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: foreign_cluster_agent.id } + it 'fails to create environment with a non existing cluster agent' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: non_existing_record_id } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq("400 Bad request - cluster agent doesn't exist or cannot be associated with this environment") - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("400 Bad request - cluster agent doesn't exist or cannot be associated with this environment") end - context 'when user access authorization does not exist' do - it 'fails to create environment with associated cluster agent' do - post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: cluster_agent.id } + it 'fails to create environment with a foreign cluster agent' do + post api("/projects/#{project.id}/environments", user), params: { name: "mepmep", cluster_agent_id: foreign_cluster_agent.id } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq('Unauthorized to access the cluster agent in this project') - end + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("400 Bad request - cluster agent doesn't exist or cannot be associated with this environment") end end diff --git a/spec/services/environments/create_service_spec.rb b/spec/services/environments/create_service_spec.rb index 00ca4dda1b33..8ce468cc9a42 100644 --- a/spec/services/environments/create_service_spec.rb +++ b/spec/services/environments/create_service_spec.rb @@ -151,7 +151,6 @@ let_it_be(:agent_management_project) { create(:project) } let_it_be(:cluster_agent) { create(:cluster_agent, project: agent_management_project) } - let!(:authorization) { create(:agent_user_access_project_authorization, project: project, agent: cluster_agent) } let(:params) do { name: 'production', @@ -161,24 +160,49 @@ } end - it 'returns successful response' do - response = subject + context 'when skip_agent_auth is true' do + let(:params) { { name: 'production', skip_agent_auth: true } } + let!(:authorization) { nil } - expect(response).to be_success - expect(response.payload[:environment].cluster_agent).to eq(cluster_agent) - expect(response.payload[:environment].kubernetes_namespace).to eq('default') - expect(response.payload[:environment].flux_resource_path).to eq('path/to/flux/resource') + it 'creates an environment without checking the cluster agent authorization' do + expect { subject }.to change { ::Environment.count }.by(1) + end + + it 'returns successful response' do + response = subject + + expect(response).to be_success + expect(response.payload[:environment].name).to eq('production') + expect(response.payload[:environment].cluster_agent).to be_nil + end end - context 'when user does not have permission to read the agent' do - let!(:authorization) { nil } + context 'when skip_agent_auth is nil' do + context 'when user has permission to read the agent' do + let!(:authorization) do + create(:agent_user_access_project_authorization, project: project, agent: cluster_agent) + end - it 'returns an error' do - response = subject + it 'returns successful response' do + response = subject + + expect(response).to be_success + expect(response.payload[:environment].cluster_agent).to eq(cluster_agent) + expect(response.payload[:environment].kubernetes_namespace).to eq('default') + expect(response.payload[:environment].flux_resource_path).to eq('path/to/flux/resource') + end + end + + context 'when user does not have permission to read the agent' do + let!(:authorization) { nil } - expect(response).to be_error - expect(response.message).to eq('Unauthorized to access the cluster agent in this project') - expect(response.payload[:environment]).to be_nil + it 'returns an error' do + response = subject + + expect(response).to be_error + expect(response.message).to eq('Unauthorized to access the cluster agent in this project') + expect(response.payload[:environment]).to be_nil + end end end end -- GitLab