From cef28c0f9acab5d318fb71a9a80e574b61744c49 Mon Sep 17 00:00:00 2001
From: Emily Ring <ering@gitlab.com>
Date: Fri, 9 Apr 2021 07:56:13 +0000
Subject: [PATCH] Order cluster token by last used
---
app/models/clusters/agent.rb | 1 +
app/models/clusters/agent_token.rb | 4 +++-
changelogs/unreleased/322128-token-order.yml | 5 +++++
...52925_add_cluster_agent_token_last_used.rb | 20 +++++++++++++++++++
db/schema_migrations/20210407152925 | 1 +
db/structure.sql | 2 ++
.../clusters/agent_tokens_resolver.rb | 2 +-
.../resolvers/clusters/agents_resolver.rb | 2 +-
.../clusters/agent_tokens_resolver_spec.rb | 8 ++++----
.../graphql/project/cluster_agents_spec.rb | 20 +++++++++----------
spec/models/clusters/agent_spec.rb | 1 +
spec/models/clusters/agent_token_spec.rb | 13 ++++++++++++
12 files changed, 62 insertions(+), 17 deletions(-)
create mode 100644 changelogs/unreleased/322128-token-order.yml
create mode 100644 db/migrate/20210407152925_add_cluster_agent_token_last_used.rb
create mode 100644 db/schema_migrations/20210407152925
diff --git a/app/models/clusters/agent.rb b/app/models/clusters/agent.rb
index c5b9dddb1daea..7c42fc28a8de2 100644
--- a/app/models/clusters/agent.rb
+++ b/app/models/clusters/agent.rb
@@ -8,6 +8,7 @@ class Agent < ApplicationRecord
belongs_to :project, class_name: '::Project' # Otherwise, it will load ::Clusters::Project
has_many :agent_tokens, class_name: 'Clusters::AgentToken'
+ has_many :last_used_agent_tokens, -> { order_last_used_at_desc }, class_name: 'Clusters::AgentToken'
scope :ordered_by_name, -> { order(:name) }
scope :with_name, -> (name) { where(name: name) }
diff --git a/app/models/clusters/agent_token.rb b/app/models/clusters/agent_token.rb
index d42279502c5dd..27a3cd8d13d57 100644
--- a/app/models/clusters/agent_token.rb
+++ b/app/models/clusters/agent_token.rb
@@ -6,7 +6,7 @@ class AgentToken < ApplicationRecord
include TokenAuthenticatable
add_authentication_token_field :token, encrypted: :required, token_generator: -> { Devise.friendly_token(50) }
- cached_attr_reader :last_contacted_at
+ cached_attr_reader :last_used_at
self.table_name = 'cluster_agent_tokens'
@@ -21,6 +21,8 @@ class AgentToken < ApplicationRecord
validates :description, length: { maximum: 1024 }
validates :name, presence: true, length: { maximum: 255 }
+ scope :order_last_used_at_desc, -> { order(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) }
+
def track_usage
track_values = { last_used_at: Time.current.utc }
diff --git a/changelogs/unreleased/322128-token-order.yml b/changelogs/unreleased/322128-token-order.yml
new file mode 100644
index 0000000000000..cf2ccd5789817
--- /dev/null
+++ b/changelogs/unreleased/322128-token-order.yml
@@ -0,0 +1,5 @@
+---
+title: Add ability to order cluster token by last used
+merge_request: 57520
+author:
+type: changed
diff --git a/db/migrate/20210407152925_add_cluster_agent_token_last_used.rb b/db/migrate/20210407152925_add_cluster_agent_token_last_used.rb
new file mode 100644
index 0000000000000..8ade2971539c3
--- /dev/null
+++ b/db/migrate/20210407152925_add_cluster_agent_token_last_used.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class AddClusterAgentTokenLastUsed < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ INDEX = 'index_cluster_agent_tokens_on_last_used_at'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :cluster_agent_tokens,
+ :last_used_at,
+ name: INDEX,
+ order: { last_used_at: 'DESC NULLS LAST' }
+ end
+
+ def down
+ remove_concurrent_index_by_name :cluster_agent_tokens, INDEX
+ end
+end
diff --git a/db/schema_migrations/20210407152925 b/db/schema_migrations/20210407152925
new file mode 100644
index 0000000000000..1a248d78c8f84
--- /dev/null
+++ b/db/schema_migrations/20210407152925
@@ -0,0 +1 @@
+079ca92ac58519ce8f575c4cb94bfe6cf209e0c9eac20d3d3a294f5b468bc586
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index a553b47ff84dd..98f3206ab404b 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -22309,6 +22309,8 @@ CREATE INDEX index_cluster_agent_tokens_on_agent_id ON cluster_agent_tokens USIN
CREATE INDEX index_cluster_agent_tokens_on_created_by_user_id ON cluster_agent_tokens USING btree (created_by_user_id);
+CREATE INDEX index_cluster_agent_tokens_on_last_used_at ON cluster_agent_tokens USING btree (last_used_at DESC NULLS LAST);
+
CREATE UNIQUE INDEX index_cluster_agent_tokens_on_token_encrypted ON cluster_agent_tokens USING btree (token_encrypted);
CREATE INDEX index_cluster_agents_on_created_by_user_id ON cluster_agents USING btree (created_by_user_id);
diff --git a/ee/app/graphql/resolvers/clusters/agent_tokens_resolver.rb b/ee/app/graphql/resolvers/clusters/agent_tokens_resolver.rb
index b0aae648b90ce..758010239dfef 100644
--- a/ee/app/graphql/resolvers/clusters/agent_tokens_resolver.rb
+++ b/ee/app/graphql/resolvers/clusters/agent_tokens_resolver.rb
@@ -12,7 +12,7 @@ class AgentTokensResolver < BaseResolver
def resolve(**args)
return ::Clusters::AgentToken.none unless can_read_agent_tokens?
- agent.agent_tokens
+ agent.last_used_agent_tokens
end
private
diff --git a/ee/app/graphql/resolvers/clusters/agents_resolver.rb b/ee/app/graphql/resolvers/clusters/agents_resolver.rb
index fca1258befc0f..f2630297f069f 100644
--- a/ee/app/graphql/resolvers/clusters/agents_resolver.rb
+++ b/ee/app/graphql/resolvers/clusters/agents_resolver.rb
@@ -28,7 +28,7 @@ def resolve_with_lookahead(**args)
private
def preloads
- { tokens: :agent_tokens }
+ { tokens: [:last_used_agent_tokens, { last_used_agent_tokens: :agent }] }
end
end
end
diff --git a/ee/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb b/ee/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb
index 7eeb63d2503a8..1a58100441131 100644
--- a/ee/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb
+++ b/ee/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb
@@ -14,8 +14,8 @@
let(:feature_available) { true }
let(:ctx) { Hash(current_user: user) }
- let!(:matching_token1) { create(:cluster_agent_token, agent: agent) }
- let!(:mathcing_token2) { create(:cluster_agent_token, agent: agent) }
+ let!(:matching_token1) { create(:cluster_agent_token, agent: agent, last_used_at: 5.days.ago) }
+ let!(:matching_token2) { create(:cluster_agent_token, agent: agent, last_used_at: 2.days.ago) }
let!(:other_token) { create(:cluster_agent_token) }
subject { resolve(described_class, obj: agent, ctx: ctx) }
@@ -24,8 +24,8 @@
stub_licensed_features(cluster_agents: feature_available)
end
- it 'returns tokens associated with the agent' do
- expect(subject).to contain_exactly(matching_token1, mathcing_token2)
+ it 'returns tokens associated with the agent, ordered by last_used_at' do
+ expect(subject).to eq([matching_token2, matching_token1])
end
context 'feature is not available' do
diff --git a/ee/spec/requests/api/graphql/project/cluster_agents_spec.rb b/ee/spec/requests/api/graphql/project/cluster_agents_spec.rb
index 65ec967d82e05..95cd4306c3067 100644
--- a/ee/spec/requests/api/graphql/project/cluster_agents_spec.rb
+++ b/ee/spec/requests/api/graphql/project/cluster_agents_spec.rb
@@ -50,22 +50,22 @@
end
context 'selecting tokens' do
- let(:cluster_agents_fields) { [:id, query_nodes(:tokens, of: 'ClusterAgentToken')] }
+ let_it_be(:token_1) { create(:cluster_agent_token, agent: agents.first) }
+ let_it_be(:token_2) { create(:cluster_agent_token, agent: agents.second) }
+ let_it_be(:token_3) { create(:cluster_agent_token, agent: agents.second, last_used_at: 2.days.ago) }
- before do
- create(:cluster_agent_token, agent: agents.first)
- create(:cluster_agent_token, agent: agents.second)
- end
+ let(:cluster_agents_fields) { [:id, query_nodes(:tokens, of: 'ClusterAgentToken')] }
- it 'can select tokens' do
+ it 'can select tokens in last_used_at order' do
post_graphql(query, current_user: current_user)
tokens = graphql_data_at(:project, :cluster_agents, :nodes, :tokens, :nodes)
- expect(tokens).to contain_exactly(
- a_hash_including('id' => be_present),
- a_hash_including('id' => be_present)
- )
+ expect(tokens).to match([
+ a_hash_including('id' => global_id_of(token_1)),
+ a_hash_including('id' => global_id_of(token_3)),
+ a_hash_including('id' => global_id_of(token_2))
+ ])
end
it 'does not suffer from N+1 performance issues' do
diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb
index a85a72eba0b71..ea7a55480a8a6 100644
--- a/spec/models/clusters/agent_spec.rb
+++ b/spec/models/clusters/agent_spec.rb
@@ -8,6 +8,7 @@
it { is_expected.to belong_to(:created_by_user).class_name('User').optional }
it { is_expected.to belong_to(:project).class_name('::Project') }
it { is_expected.to have_many(:agent_tokens).class_name('Clusters::AgentToken') }
+ it { is_expected.to have_many(:last_used_agent_tokens).class_name('Clusters::AgentToken') }
it { is_expected.to validate_presence_of(:name) }
it { is_expected.to validate_length_of(:name).is_at_most(63) }
diff --git a/spec/models/clusters/agent_token_spec.rb b/spec/models/clusters/agent_token_spec.rb
index 680b351d24a7f..bde4798abecb3 100644
--- a/spec/models/clusters/agent_token_spec.rb
+++ b/spec/models/clusters/agent_token_spec.rb
@@ -9,6 +9,19 @@
it { is_expected.to validate_length_of(:name).is_at_most(255) }
it { is_expected.to validate_presence_of(:name) }
+ describe 'scopes' do
+ describe '.order_last_used_at_desc' do
+ let_it_be(:token_1) { create(:cluster_agent_token, last_used_at: 7.days.ago) }
+ let_it_be(:token_2) { create(:cluster_agent_token, last_used_at: nil) }
+ let_it_be(:token_3) { create(:cluster_agent_token, last_used_at: 2.days.ago) }
+
+ it 'sorts by last_used_at descending, with null values at last' do
+ expect(described_class.order_last_used_at_desc)
+ .to eq([token_3, token_1, token_2])
+ end
+ end
+ end
+
describe '#token' do
it 'is generated on save' do
agent_token = build(:cluster_agent_token, token_encrypted: nil)
--
GitLab