Merge branch 'cluster_agent_name_validation' into 'master'

Restrict allowed chars for Kubernetes Agent's name

See merge request gitlab-org/gitlab!37564

app/models/clusters/agent.rb

@@ -8,6 +8,13 @@ module Clusters
 
     has_many :agent_tokens, class_name: 'Clusters::AgentToken'
 
-    validates :name, presence: true, length: { maximum: 255 }, uniqueness: { scope: :project_id }
+    validates :name,
+      presence: true,
+      length: { maximum: 63 },
+      uniqueness: { scope: :project_id },
+      format: {
+        with: Gitlab::Regex.cluster_agent_name_regex,
+        message: Gitlab::Regex.cluster_agent_name_regex_message
+      }
   end
 end

lib/gitlab/regex.rb

@@ -160,6 +160,15 @@ module Gitlab
       "can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.', '*' and spaces"
     end
 
+    # https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/doc/identity_and_auth.md#agent-identity-and-name
+    def cluster_agent_name_regex
+      /\A[a-z0-9]([-a-z0-9]*[a-z0-9])?\z/
+    end
+
+    def cluster_agent_name_regex_message
+      %q{can contain only lowercase letters, digits, and '-', but cannot start or end with '-'}
+    end
+
     def kubernetes_namespace_regex
       /\A[a-z0-9]([-a-z0-9]*[a-z0-9])?\z/
     end

spec/factories/clusters/agents.rb

@@ -4,6 +4,6 @@ FactoryBot.define do
   factory :cluster_agent, class: 'Clusters::Agent' do
     project
 
-    sequence(:name) { |n| "agent_#{n}" }
+    sequence(:name) { |n| "agent-#{n}" }
   end
 end

spec/lib/gitlab/regex_spec.rb

@@ -131,6 +131,22 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('9/9/2018') }
   end
 
+  describe '.cluster_agent_name_regex' do
+    subject { described_class.cluster_agent_name_regex }
+
+    it { is_expected.to match('foo') }
+    it { is_expected.to match('foo-bar') }
+    it { is_expected.to match('1foo-bar') }
+    it { is_expected.to match('foo-bar2') }
+    it { is_expected.to match('foo-1bar') }
+    it { is_expected.not_to match('foo.bar') }
+    it { is_expected.not_to match('Foo') }
+    it { is_expected.not_to match('FoO') }
+    it { is_expected.not_to match('FoO-') }
+    it { is_expected.not_to match('-foo-') }
+    it { is_expected.not_to match('foo/bar') }
+  end
+
   describe '.kubernetes_namespace_regex' do
     subject { described_class.kubernetes_namespace_regex }
 

spec/models/clusters/agent_spec.rb

@@ -9,6 +9,27 @@ RSpec.describe Clusters::Agent do
   it { is_expected.to have_many(: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(255) }
+  it { is_expected.to validate_length_of(:name).is_at_most(63) }
   it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) }
+
+  describe 'validation' do
+    describe 'name validation' do
+      it 'rejects names that do not conform to RFC 1123', :aggregate_failures do
+        %w[Agent agentA agentAagain gent- -agent agent.a agent/a agent>a].each do |name|
+          agent = build(:cluster_agent, name: name)
+
+          expect(agent).not_to be_valid
+          expect(agent.errors[:name]).to eq(["can contain only lowercase letters, digits, and '-', but cannot start or end with '-'"])
+        end
+      end
+
+      it 'accepts valid names', :aggregate_failures do
+        %w[agent agent123 agent-123].each do |name|
+          agent = build(:cluster_agent, name: name)
+
+          expect(agent).to be_valid
+        end
+      end
+    end
+  end
 end