diff --git a/app/graphql/mutations/environments/update.rb b/app/graphql/mutations/environments/update.rb index dc1fb9b23af207545a5d8a1b27da5822f1975a5e..431a7add00e8a5aa6995b137aaee4f7dcd4f19cc 100644 --- a/app/graphql/mutations/environments/update.rb +++ b/app/graphql/mutations/environments/update.rb @@ -23,6 +23,11 @@ class Update < ::Mutations::BaseMutation required: false, description: 'Tier of the environment.' + argument :cluster_agent_id, + ::Types::GlobalIDType[::Clusters::Agent], + required: false, + description: 'Cluster agent of the environment.' + field :environment, Types::EnvironmentType, null: true, @@ -31,6 +36,8 @@ class Update < ::Mutations::BaseMutation def resolve(id:, **kwargs) environment = authorized_find!(id: id) + convert_cluster_agent_id(kwargs) + response = ::Environments::UpdateService.new(environment.project, current_user, kwargs).execute(environment) if response.success? @@ -39,6 +46,16 @@ def resolve(id:, **kwargs) { environment: response.payload[:environment], errors: response.errors } end end + + private + + def convert_cluster_agent_id(kwargs) + return unless kwargs.key?(:cluster_agent_id) + + kwargs[:cluster_agent] = if kwargs[:cluster_agent_id] + ::Clusters::Agent.find_by_id(kwargs[:cluster_agent_id].model_id) + end + end end end end diff --git a/app/services/environments/update_service.rb b/app/services/environments/update_service.rb index e02b239842681852ded5d79d25449dd32a500dc0..5eb4880ec4b4dd381e66a637f8b8d95f308cc46a 100644 --- a/app/services/environments/update_service.rb +++ b/app/services/environments/update_service.rb @@ -2,6 +2,8 @@ module Environments class UpdateService < BaseService + ALLOWED_ATTRIBUTES = %i[external_url tier cluster_agent].freeze + def execute(environment) unless can?(current_user, :update_environment, environment) return ServiceResponse.error( @@ -10,7 +12,13 @@ def execute(environment) ) end - if environment.update(**params) + if unauthorized_cluster_agent? + return ServiceResponse.error( + message: _('Unauthorized to access the cluster agent in this project'), + payload: { environment: environment }) + end + + if environment.update(**params.slice(*ALLOWED_ATTRIBUTES)) ServiceResponse.success(payload: { environment: environment }) else ServiceResponse.error( @@ -19,5 +27,16 @@ def execute(environment) ) end end + + private + + def unauthorized_cluster_agent? + return false unless params[:cluster_agent] + + ::Clusters::Agents::Authorizations::UserAccess::Finder + .new(current_user, agent: params[:cluster_agent], project: project) + .execute + .empty? + end end end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 369fa2bbf98ec98b6ce9efc124f292de17843894..6384d896292fea39c70303bd219a69c23c704ca9 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -3141,6 +3141,7 @@ Input type: `EnvironmentUpdateInput` | Name | Type | Description | | ---- | ---- | ----------- | | <a id="mutationenvironmentupdateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationenvironmentupdateclusteragentid"></a>`clusterAgentId` | [`ClustersAgentID`](#clustersagentid) | Cluster agent of the environment. | | <a id="mutationenvironmentupdateexternalurl"></a>`externalUrl` | [`String`](#string) | External URL of the environment. | | <a id="mutationenvironmentupdateid"></a>`id` | [`EnvironmentID!`](#environmentid) | Global ID of the environment to update. | | <a id="mutationenvironmentupdatetier"></a>`tier` | [`DeploymentTier`](#deploymenttier) | Tier of the environment. | diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2b638c4e322e79f75cbe2f7d8eea08e9d2c68d7f..e687b6a745636b249a7acde6f1ea4b3922866ac1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -48006,6 +48006,9 @@ msgstr "" msgid "Unauthenticated web rate limit period in seconds" msgstr "" +msgid "Unauthorized to access the cluster agent in this project" +msgstr "" + msgid "Unauthorized to create an environment" msgstr "" diff --git a/spec/graphql/mutations/environments/update_spec.rb b/spec/graphql/mutations/environments/update_spec.rb index 87c1bd5a44bb8cdb17b6d2ae7916015802e3f48a..5c61b3c5dbe55d6f9d752bf051888e7739a33d10 100644 --- a/spec/graphql/mutations/environments/update_spec.rb +++ b/spec/graphql/mutations/environments/update_spec.rb @@ -18,10 +18,10 @@ end describe '#resolve' do - subject { mutation.resolve(id: environment_id, external_url: external_url) } + subject { mutation.resolve(id: environment_id, **kwargs) } let(:environment_id) { environment.to_global_id } - let(:external_url) { 'https://gitlab.com/' } + let(:kwargs) { { external_url: 'https://gitlab.com/' } } context 'when service execution succeeded' do it 'returns no errors' do @@ -29,12 +29,12 @@ end it 'updates the environment' do - expect(subject[:environment][:external_url]).to eq(external_url) + expect(subject[:environment][:external_url]).to eq('https://gitlab.com/') end end context 'when service cannot update the attribute' do - let(:external_url) { 'http://${URL}' } + let(:kwargs) { { external_url: 'http://${URL}' } } it 'returns an error' do expect(subject) @@ -45,6 +45,46 @@ end end + context 'when setting cluster agent ID to the environment' do + let_it_be(:cluster_agent) { create(:cluster_agent, project: project) } + + let!(:authorization) { create(:agent_user_access_project_authorization, project: project, agent: cluster_agent) } + + let(:kwargs) { { cluster_agent_id: cluster_agent.to_global_id } } + + it 'sets the cluster agent to the environment' do + expect(subject[:environment].cluster_agent).to eq(cluster_agent) + end + end + + context 'when unsetting cluster agent ID to the environment' do + let_it_be(:cluster_agent) { create(:cluster_agent, project: project) } + + let(:kwargs) { { cluster_agent_id: nil } } + + before do + environment.update!(cluster_agent: cluster_agent) + end + + it 'removes the cluster agent from the environment' do + expect(subject[:environment].cluster_agent).to be_nil + end + end + + context 'when the cluster agent is not updated' do + let_it_be(:cluster_agent) { create(:cluster_agent, project: project) } + + let(:kwargs) { { external_url: 'https://dev.gitlab.com/' } } + + before do + environment.update!(cluster_agent: cluster_agent) + end + + it 'does not change the environment cluster agent' do + expect(subject[:environment].cluster_agent).to eq(cluster_agent) + end + end + context 'when user is reporter who does not have permission to access the environment' do let(:user) { reporter } diff --git a/spec/services/environments/update_service_spec.rb b/spec/services/environments/update_service_spec.rb index 72ace3b050ebd374dfd9e4d4e8b49a6fbf38219e..84220c0930ba9ce169fa67c1d484ef32f79f3ce2 100644 --- a/spec/services/environments/update_service_spec.rb +++ b/spec/services/environments/update_service_spec.rb @@ -28,6 +28,50 @@ expect(response.payload[:environment]).to eq(environment) end + context 'when setting a cluster agent to the environment' do + 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) { { cluster_agent: cluster_agent } } + + it 'returns successful response' do + response = subject + + expect(response).to be_success + expect(response.payload[:environment].cluster_agent).to eq(cluster_agent) + end + + context 'when user does not have permission to read the agent' do + let!(:authorization) { 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 eq(environment) + end + end + end + + context 'when unsetting a cluster agent of the environment' do + let_it_be(:cluster_agent) { create(:cluster_agent, project: project) } + + let(:params) { { cluster_agent: nil } } + + before do + environment.update!(cluster_agent: cluster_agent) + end + + it 'returns successful response' do + response = subject + + expect(response).to be_success + expect(response.payload[:environment].cluster_agent).to be_nil + end + end + context 'when params contain invalid value' do let(:params) { { external_url: 'http://${URL}' } } @@ -40,6 +84,18 @@ end end + context 'when disallowed parameter is passed' do + let(:params) { { external_url: 'https://gitlab.com/', slug: 'prod' } } + + it 'ignores the parameter' do + response = subject + + expect(response).to be_success + expect(response.payload[:environment].external_url).to eq('https://gitlab.com/') + expect(response.payload[:environment].slug).not_to eq('prod') + end + end + context 'when user is reporter' do let(:current_user) { reporter }