Merge branch 'log-activity-events-on-agent-token-revoke' into 'master'

Create activity event when agent token is revoked

See merge request gitlab-org/gitlab!113173



Merged-by: Tetiana Chupryna <tchupryna@gitlab.com>
Approved-by: Timo Furrer <tfurrer@gitlab.com>
Approved-by: Tetiana Chupryna <tchupryna@gitlab.com>
Reviewed-by: Tiger Watson <twatson@gitlab.com>
Co-authored-by: Tiger <twatson@gitlab.com>

app/services/clusters/agent_tokens/create_service.rb

@@ -19,7 +19,7 @@ def execute
         token = ::Clusters::AgentToken.new(filtered_params.merge(agent_id: agent.id, created_by_user: current_user))
 
         if token.save
-          log_activity_event!(token)
+          log_activity_event(token)
 
           ServiceResponse.success(payload: { secret: token.token, token: token })
         else
@@ -37,7 +37,7 @@ def filtered_params
         params.slice(*ALLOWED_PARAMS)
       end
 
-      def log_activity_event!(token)
+      def log_activity_event(token)
         Clusters::Agents::CreateActivityEventService.new(
           token.agent,
           kind: :token_created,

app/services/clusters/agent_tokens/revoke_service.rb

@@ -14,6 +14,8 @@ def execute
         return error_no_permissions unless current_user.can?(:create_cluster, token.agent.project)
 
         if token.update(status: token.class.statuses[:revoked])
+          log_activity_event(token)
+
           ServiceResponse.success
         else
           ServiceResponse.error(message: token.errors.full_messages)
@@ -26,6 +28,17 @@ def error_no_permissions
         ServiceResponse.error(
           message: s_('ClusterAgent|User has insufficient permissions to revoke the token for this project'))
       end
+
+      def log_activity_event(token)
+        Clusters::Agents::CreateActivityEventService.new(
+          token.agent,
+          kind: :token_revoked,
+          level: :info,
+          recorded_at: token.updated_at,
+          user: current_user,
+          agent_token: token
+        ).execute
+      end
     end
   end
 end

app/services/clusters/agents/create_activity_event_service.rb

@@ -14,6 +14,10 @@ def execute
         DeleteExpiredEventsWorker.perform_at(schedule_cleanup_at, agent.id)
 
         ServiceResponse.success
+      rescue StandardError => e
+        Gitlab::ErrorTracking.track_exception(e, agent_id: agent.id)
+
+        ServiceResponse.error(message: e.message)
       end
 
       private

spec/services/clusters/agent_tokens/revoke_service_spec.rb

@@ -4,6 +4,8 @@
 
 RSpec.describe Clusters::AgentTokens::RevokeService, feature_category: :kubernetes_management do
   describe '#execute' do
+    subject { described_class.new(token: agent_token, current_user: user).execute }
+
     let(:agent) { create(:cluster_agent) }
     let(:agent_token) { create(:cluster_agent_token, agent: agent) }
     let(:project) { agent.project }
@@ -20,10 +22,24 @@
 
       context 'when user revokes agent token' do
         it 'succeeds' do
-          described_class.new(token: agent_token, current_user: user).execute
+          subject
 
           expect(agent_token.revoked?).to be true
         end
+
+        it 'creates an activity event' do
+          expect { subject }.to change { ::Clusters::Agents::ActivityEvent.count }.by(1)
+
+          event = agent.activity_events.last
+
+          expect(event).to have_attributes(
+            kind: 'token_revoked',
+            level: 'info',
+            recorded_at: agent_token.reload.updated_at,
+            user: user,
+            agent_token: agent_token
+          )
+        end
       end
 
       context 'when there is a validation failure' do
@@ -32,24 +48,26 @@
         end
 
         it 'fails without raising an error', :aggregate_failures do
-          result = described_class.new(token: agent_token, current_user: user).execute
+          expect(subject[:status]).to eq(:error)
+          expect(subject[:message]).to eq(["Name can't be blank"])
+        end
 
-          expect(result[:status]).to eq(:error)
-          expect(result[:message]).to eq(["Name can't be blank"])
+        it 'does not create an activity event' do
+          expect { subject }.not_to change { ::Clusters::Agents::ActivityEvent.count }
         end
       end
     end
 
     context 'when user is not authorized' do
-      let(:unauthorized_user) { create(:user) }
+      let(:user) { create(:user) }
 
       before do
-        project.add_guest(unauthorized_user)
+        project.add_guest(user)
       end
 
       context 'when user attempts to revoke agent token' do
         it 'fails' do
-          described_class.new(token: agent_token, current_user: unauthorized_user).execute
+          subject
 
           expect(agent_token.revoked?).to be false
         end

spec/services/clusters/agents/create_activity_event_service_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Clusters::Agents::CreateActivityEventService do
+RSpec.describe Clusters::Agents::CreateActivityEventService, feature_category: :kubernetes_management do
   let_it_be(:agent) { create(:cluster_agent) }
   let_it_be(:token) { create(:cluster_agent_token, agent: agent) }
   let_it_be(:user) { create(:user) }
@@ -40,5 +40,16 @@
 
       subject
     end
+
+    context 'when activity event creation fails' do
+      let(:params) { {} }
+
+      it 'tracks the exception without raising' do
+        expect(Gitlab::ErrorTracking).to receive(:track_exception)
+          .with(instance_of(ActiveRecord::RecordInvalid), agent_id: agent.id)
+
+        subject
+      end
+    end
   end
 end