Merge branch '300417-agent-token-description' into 'master'

Add description field to cluster agent token

See merge request gitlab-org/gitlab!54091

app/models/clusters/agent_token.rb

@@ -7,9 +7,11 @@ module Clusters
 
     self.table_name = 'cluster_agent_tokens'
 
-    belongs_to :agent, class_name: 'Clusters::Agent'
+    belongs_to :agent, class_name: 'Clusters::Agent', optional: false
     belongs_to :created_by_user, class_name: 'User', optional: true
 
     before_save :ensure_token
+
+    validates :description, length: { maximum: 1024 }
   end
 end

changelogs/unreleased/300417-agent-token-description.yml

+---
+title: Add description field to cluster agent token
+merge_request: 54091
+author:
+type: changed

db/migrate/20210216193620_add_description_to_cluster_token.rb

+# frozen_string_literal: true
+
+class AddDescriptionToClusterToken < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless column_exists?(:cluster_agent_tokens, :description)
+      add_column :cluster_agent_tokens, :description, :text
+    end
+
+    add_text_limit :cluster_agent_tokens, :description, 1024
+  end
+
+  def down
+    remove_column :cluster_agent_tokens, :description
+  end
+end

db/schema_migrations/20210216193620

+3587ba61d003385ea63ce900c1dd1c2bd1f2386abd921615b50421f1b798f553
\ No newline at end of file

db/structure.sql

@@ -11026,6 +11026,8 @@ CREATE TABLE cluster_agent_tokens (
     agent_id bigint NOT NULL,
     token_encrypted text NOT NULL,
     created_by_user_id bigint,
+    description text,
+    CONSTRAINT check_4e4ec5070a CHECK ((char_length(description) <= 1024)),
     CONSTRAINT check_c60daed227 CHECK ((char_length(token_encrypted) <= 255))
 );
 

doc/api/graphql/reference/index.md

@@ -717,6 +717,7 @@ Autogenerated return type of ClusterAgentDelete.
 | `clusterAgent` | ClusterAgent | Cluster agent this token is associated with. |
 | `createdAt` | Time | Timestamp the token was created. |
 | `createdByUser` | User | The user who created the token. |
+| `description` | String | Description of the token. |
 | `id` | ClustersAgentTokenID! | Global ID of the token. |
 
 ### ClusterAgentTokenCreatePayload

ee/app/graphql/mutations/clusters/agent_tokens/create.rb

@@ -10,10 +10,16 @@ module Mutations
 
         ClusterAgentID = ::Types::GlobalIDType[::Clusters::Agent]
 
-        argument :cluster_agent_id, ClusterAgentID,
+        argument :cluster_agent_id,
+                 ClusterAgentID,
                  required: true,
                  description: 'Global ID of the cluster agent that will be associated with the new token.'
 
+        argument :description,
+                 GraphQL::STRING_TYPE,
+                 required: false,
+                 description: 'Description of the token.'
+
         field :secret,
               GraphQL::STRING_TYPE,
               null: true,
@@ -24,12 +30,16 @@ module Mutations
               null: true,
               description: 'Token created after mutation.'
 
-        def resolve(cluster_agent_id:)
-          cluster_agent = authorized_find!(id: cluster_agent_id)
+        def resolve(args)
+          cluster_agent = authorized_find!(id: args[:cluster_agent_id])
 
           result = ::Clusters::AgentTokens::CreateService
-            .new(container: cluster_agent.project, current_user: current_user)
-            .execute(cluster_agent)
+            .new(
+              container: cluster_agent.project,
+              current_user: current_user,
+              params: args.merge(agent_id: cluster_agent.id)
+            )
+            .execute
 
           payload = result.payload
 

ee/app/graphql/types/clusters/agent_token_type.rb

@@ -24,6 +24,11 @@ module Types
             null: true,
             description: 'The user who created the token.'
 
+      field :description,
+            GraphQL::STRING_TYPE,
+            null: true,
+            description: 'Description of the token.'
+
       field :id,
             ::Types::GlobalIDType[::Clusters::AgentToken],
             null: false,

ee/app/services/clusters/agent_tokens/create_service.rb

@@ -3,11 +3,13 @@
 module Clusters
   module AgentTokens
     class CreateService < ::BaseContainerService
-      def execute(cluster_agent)
+      ALLOWED_PARAMS = %i[agent_id description].freeze
+
+      def execute
         return error_feature_not_available unless container.feature_available?(:cluster_agents)
         return error_no_permissions unless current_user.can?(:create_cluster, container)
 
-        token = ::Clusters::AgentToken.new(agent: cluster_agent, created_by_user: current_user)
+        token = ::Clusters::AgentToken.new(filtered_params.merge(created_by_user: current_user))
 
         if token.save
           ServiceResponse.success(payload: { secret: token.token, token: token })
@@ -25,6 +27,10 @@ module Clusters
       def error_no_permissions
         ServiceResponse.error(message: s_('ClusterAgent|User has insufficient permissions to create a token for this project'))
       end
+
+      def filtered_params
+        params.slice(*ALLOWED_PARAMS)
+      end
     end
   end
 end

ee/spec/graphql/mutations/clusters/agent_tokens/create_spec.rb

@@ -18,7 +18,9 @@ RSpec.describe Mutations::Clusters::AgentTokens::Create do
   specify { expect(described_class).to require_graphql_authorizations(:create_cluster) }
 
   describe '#resolve' do
-    subject { mutation.resolve(cluster_agent_id: cluster_agent.to_global_id) }
+    let(:description) { 'new token!' }
+
+    subject { mutation.resolve(cluster_agent_id: cluster_agent.to_global_id, description: description) }
 
     context 'without token permissions' do
       it 'raises an error if the resource is not accessible to the user' do
@@ -44,10 +46,14 @@ RSpec.describe Mutations::Clusters::AgentTokens::Create do
 
       it 'creates a new token', :aggregate_failures do
         expect { subject }.to change { ::Clusters::AgentToken.count }.by(1)
-        expect(subject[:secret]).not_to be_nil
         expect(subject[:errors]).to eq([])
       end
 
+      it 'returns token information', :aggregate_failures do
+        expect(subject[:secret]).not_to be_nil
+        expect(subject[:token].description).to eq(description)
+      end
+
       context 'invalid params' do
         subject { mutation.resolve(cluster_agent_id: cluster_agent.id) }
 

ee/spec/graphql/types/clusters/agent_token_type_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe GitlabSchema.types['ClusterAgentToken'] do
-  let(:fields) { %i[cluster_agent created_at created_by_user id] }
+  let(:fields) { %i[cluster_agent created_at created_by_user description id] }
 
   it { expect(described_class.graphql_name).to eq('ClusterAgentToken') }
 

ee/spec/requests/api/graphql/mutations/clusters/agent_tokens/create_spec.rb

@@ -8,10 +8,11 @@ RSpec.describe 'Create a new cluster agent token' do
   let_it_be(:cluster_agent) { create(:cluster_agent) }
   let_it_be(:current_user) { create(:user) }
 
+  let(:description) { 'create token' }
   let(:mutation) do
     graphql_mutation(
       :cluster_agent_token_create,
-      { cluster_agent_id: cluster_agent.to_global_id.to_s }
+      { cluster_agent_id: cluster_agent.to_global_id.to_s, description: description }
     )
   end
 
@@ -49,8 +50,14 @@ RSpec.describe 'Create a new cluster agent token' do
 
     it 'creates a new token', :aggregate_failures do
       expect { post_graphql_mutation(mutation, current_user: current_user) }.to change { Clusters::AgentToken.count }.by(1)
-      expect(mutation_response['secret']).not_to be_nil
       expect(mutation_response['errors']).to eq([])
     end
+
+    it 'returns token information', :aggregate_failures do
+      post_graphql_mutation(mutation, current_user: current_user)
+
+      expect(mutation_response['secret']).not_to be_nil
+      expect(mutation_response.dig('token', 'description')).to eq(description)
+    end
   end
 end

ee/spec/services/clusters/agent_tokens/create_service_spec.rb

@@ -3,43 +3,45 @@
 require 'spec_helper'
 
 RSpec.describe Clusters::AgentTokens::CreateService do
-  subject(:service) { described_class.new(container: project, current_user: user) }
+  subject(:service) { described_class.new(container: project, current_user: user, params: params) }
 
   let_it_be(:user) { create(:user) }
   let(:cluster_agent) { create(:cluster_agent) }
   let(:project) { cluster_agent.project }
+  let(:params) { { agent_id: cluster_agent.id } }
 
   before do
     stub_licensed_features(cluster_agents: false)
   end
 
   describe '#execute' do
+    subject { service.execute }
+
     context 'without premium plan' do
       it 'does not create a new token' do
-        expect { service.execute(cluster_agent) }.not_to change(Clusters::AgentToken, :count)
+        expect { subject }.not_to change(Clusters::AgentToken, :count)
       end
 
       it 'returns missing license error' do
-        result = service.execute(cluster_agent)
-
-        expect(result.status).to eq(:error)
-        expect(result.message).to eq('This feature is only available for premium plans')
+        expect(subject.status).to eq(:error)
+        expect(subject.message).to eq('This feature is only available for premium plans')
       end
 
       context 'with premium plan' do
+        let(:description) { 'New token description' }
+        let(:params) { { agent_id: cluster_agent.id, description: description } }
+
         before do
           stub_licensed_features(cluster_agents: true)
         end
 
         it 'does not create a new token due to user permissions' do
-          expect { service.execute(cluster_agent) }.not_to change(::Clusters::AgentToken, :count)
+          expect { subject }.not_to change(::Clusters::AgentToken, :count)
         end
 
         it 'returns permission errors', :aggregate_failures do
-          result = service.execute(cluster_agent)
-
-          expect(result.status).to eq(:error)
-          expect(result.message).to eq('User has insufficient permissions to create a token for this project')
+          expect(subject.status).to eq(:error)
+          expect(subject.message).to eq('User has insufficient permissions to create a token for this project')
         end
 
         context 'with user permissions' do
@@ -48,21 +50,34 @@ RSpec.describe Clusters::AgentTokens::CreateService do
           end
 
           it 'creates a new token' do
-            expect { service.execute(cluster_agent) }.to change { ::Clusters::AgentToken.count }.by(1)
+            expect { subject }.to change { ::Clusters::AgentToken.count }.by(1)
           end
 
           it 'returns success status', :aggregate_failures do
-            result = service.execute(cluster_agent)
-
-            expect(result.status).to eq(:success)
-            expect(result.message).to be_nil
+            expect(subject.status).to eq(:success)
+            expect(subject.message).to be_nil
           end
 
           it 'returns token information', :aggregate_failures do
-            result = service.execute(cluster_agent)
+            token = subject.payload[:token]
+
+            expect(subject.payload[:secret]).not_to be_nil
+
+            expect(token.created_by_user).to eq(user)
+            expect(token.description).to eq(description)
+          end
+
+          context 'when params are invalid' do
+            let(:params) { { agent_id: 'bad_id' } }
+
+            it 'does not create a new token' do
+              expect { subject }.not_to change(::Clusters::AgentToken, :count)
+            end
 
-            expect(result.payload[:secret]).not_to be_nil
-            expect(result.payload[:token].created_by_user).to eq(user)
+            it 'returns validation errors', :aggregate_failures do
+              expect(subject.status).to eq(:error)
+              expect(subject.message).to eq(['Agent must exist'])
+            end
           end
         end
       end

spec/models/clusters/agent_token_spec.rb

@@ -3,8 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Clusters::AgentToken do
-  it { is_expected.to belong_to(:agent).class_name('Clusters::Agent') }
+  it { is_expected.to belong_to(:agent).class_name('Clusters::Agent').required }
   it { is_expected.to belong_to(:created_by_user).class_name('User').optional }
+  it { is_expected.to validate_length_of(:description).is_at_most(1024) }
 
   describe '#token' do
     it 'is generated on save' do