From bb7fee08c5e492894282f904b33299a6fb784cb7 Mon Sep 17 00:00:00 2001 From: Pam Artiaga <partiaga@gitlab.com> Date: Mon, 22 May 2023 22:11:00 +0000 Subject: [PATCH] Limit the number of agent tokens created This lowers the number of agent tokens that need to be managed, which helps with 1) security, and 2) performance This change is behind a feature flag. Changelog: changed --- .../clusters/agent_tokens/create_service.rb | 12 +++++++++ .../cluster_agents_limit_tokens_created.yml | 8 ++++++ doc/api/cluster_agents.md | 5 +++- doc/user/clusters/agent/work_with_agent.md | 3 +++ locale/gitlab.pot | 3 +++ .../clusters/agent_tokens/create_spec.rb | 12 +++++++++ .../api/clusters/agent_tokens_spec.rb | 22 +++++++++++++++ .../agent_tokens/create_service_spec.rb | 27 +++++++++++++++++++ 8 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 config/feature_flags/development/cluster_agents_limit_tokens_created.yml diff --git a/app/services/clusters/agent_tokens/create_service.rb b/app/services/clusters/agent_tokens/create_service.rb index 66a3cb04d9808..efa9716d2c848 100644 --- a/app/services/clusters/agent_tokens/create_service.rb +++ b/app/services/clusters/agent_tokens/create_service.rb @@ -4,6 +4,7 @@ module Clusters module AgentTokens class CreateService ALLOWED_PARAMS = %i[agent_id description name].freeze + ACTIVE_TOKENS_LIMIT = 2 attr_reader :agent, :current_user, :params @@ -15,6 +16,7 @@ def initialize(agent:, current_user:, params:) def execute return error_no_permissions unless current_user.can?(:create_cluster, agent.project) + return error_active_tokens_limit_reached if active_tokens_limit_reached? token = ::Clusters::AgentToken.new(filtered_params.merge(agent_id: agent.id, created_by_user: current_user)) @@ -33,6 +35,16 @@ def error_no_permissions ServiceResponse.error(message: s_('ClusterAgent|User has insufficient permissions to create a token for this project')) end + def error_active_tokens_limit_reached + ServiceResponse.error(message: s_('ClusterAgent|An agent can have only two active tokens at a time')) + end + + def active_tokens_limit_reached? + return false unless Feature.enabled?(:cluster_agents_limit_tokens_created) + + ::Clusters::AgentTokensFinder.new(agent, current_user, status: :active).execute.count >= ACTIVE_TOKENS_LIMIT + end + def filtered_params params.slice(*ALLOWED_PARAMS) end diff --git a/config/feature_flags/development/cluster_agents_limit_tokens_created.yml b/config/feature_flags/development/cluster_agents_limit_tokens_created.yml new file mode 100644 index 0000000000000..1ad85185509e1 --- /dev/null +++ b/config/feature_flags/development/cluster_agents_limit_tokens_created.yml @@ -0,0 +1,8 @@ +--- +name: cluster_agents_limit_tokens_created +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120825 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/412399 +milestone: '16.1' +type: development +group: group::environments +default_enabled: false diff --git a/doc/api/cluster_agents.md b/doc/api/cluster_agents.md index 4bd16b88d92ee..1753757e5d992 100644 --- a/doc/api/cluster_agents.md +++ b/doc/api/cluster_agents.md @@ -365,12 +365,15 @@ Example response: ## Create an agent token -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/347046) in GitLab 15.0. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/347046) in GitLab 15.0. +> - Two-token limit [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/361030) in GitLab 16.1. Creates a new token for an agent. You must have at least the Maintainer role to use this endpoint. +An agent can have only two active tokens at one time. + ```plaintext POST /projects/:id/cluster_agents/:agent_id/tokens ``` diff --git a/doc/user/clusters/agent/work_with_agent.md b/doc/user/clusters/agent/work_with_agent.md index 2d54f67724e63..b2e8ac6ef16b3 100644 --- a/doc/user/clusters/agent/work_with_agent.md +++ b/doc/user/clusters/agent/work_with_agent.md @@ -91,6 +91,9 @@ For more information about debugging, see [troubleshooting documentation](troubl > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/327152) in GitLab 14.9. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/336641) in GitLab 14.10, the agent token can be revoked from the UI. +> - Two-token limit [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/361030) in GitLab 16.1. + +An agent can have only two active tokens at one time. To reset the agent token without downtime: diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f3bd7c3eef6c3..8b5bc0d8b5b4b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10415,6 +10415,9 @@ msgstr "" msgid "ClusterAgents|shared" msgstr "" +msgid "ClusterAgent|An agent can have only two active tokens at a time" +msgstr "" + msgid "ClusterAgent|User has insufficient permissions to create a token for this project" msgstr "" diff --git a/spec/graphql/mutations/clusters/agent_tokens/create_spec.rb b/spec/graphql/mutations/clusters/agent_tokens/create_spec.rb index 7998be19c2007..cb01ff64d5d28 100644 --- a/spec/graphql/mutations/clusters/agent_tokens/create_spec.rb +++ b/spec/graphql/mutations/clusters/agent_tokens/create_spec.rb @@ -50,6 +50,18 @@ expect(token.description).to eq(description) expect(token.name).to eq(name) end + + context 'when the active agent tokens limit is reached' do + before do + create(:cluster_agent_token, agent: cluster_agent) + create(:cluster_agent_token, agent: cluster_agent) + end + + it 'raises an error' do + expect { subject }.not_to change { ::Clusters::AgentToken.count } + expect(subject[:errors]).to eq(["An agent can have only two active tokens at a time"]) + end + end end end end diff --git a/spec/requests/api/clusters/agent_tokens_spec.rb b/spec/requests/api/clusters/agent_tokens_spec.rb index 2647684c9f812..c18ebf7d04436 100644 --- a/spec/requests/api/clusters/agent_tokens_spec.rb +++ b/spec/requests/api/clusters/agent_tokens_spec.rb @@ -162,6 +162,28 @@ expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'when the active agent tokens limit is reached' do + before do + # create an additional agent token to make it 2 + create(:cluster_agent_token, agent: agent) + end + + it 'returns a bad request (400) error' do + params = { + name: 'test-token', + description: 'Test description' + } + post(api("/projects/#{project.id}/cluster_agents/#{agent.id}/tokens", user), params: params) + + aggregate_failures "testing response" do + expect(response).to have_gitlab_http_status(:bad_request) + + error_message = json_response['message'] + expect(error_message).to eq('400 Bad request - An agent can have only two active tokens at a time') + end + end + end end describe 'DELETE /projects/:id/cluster_agents/:agent_id/tokens/:token_id' do diff --git a/spec/services/clusters/agent_tokens/create_service_spec.rb b/spec/services/clusters/agent_tokens/create_service_spec.rb index 803bd947629b7..431d7ce207994 100644 --- a/spec/services/clusters/agent_tokens/create_service_spec.rb +++ b/spec/services/clusters/agent_tokens/create_service_spec.rb @@ -78,6 +78,33 @@ expect(subject.message).to eq(["Name can't be blank"]) end end + + context 'when the active agent tokens limit is reached' do + before do + create(:cluster_agent_token, agent: cluster_agent) + create(:cluster_agent_token, agent: cluster_agent) + end + + it 'returns an error' do + expect(subject.status).to eq(:error) + expect(subject.message).to eq('An agent can have only two active tokens at a time') + end + + context 'when cluster_agents_limit_tokens_created feature flag is disabled' do + before do + stub_feature_flags(cluster_agents_limit_tokens_created: false) + end + + it 'creates a new token' do + expect { subject }.to change { ::Clusters::AgentToken.count }.by(1) + end + + it 'returns success status', :aggregate_failures do + expect(subject.status).to eq(:success) + expect(subject.message).to be_nil + end + end + end end end end -- GitLab