Merge branch '323708-enable-authorized-group-agents' into 'master'

Include group authorized agents in allowed agents

See merge request gitlab-org/gitlab!69047

app/models/clusters/agents/group_authorization.rb

@@ -9,6 +9,8 @@ module Clusters
       belongs_to :group, class_name: '::Group', optional: false
 
       validates :config, json_schema: { filename: 'cluster_agent_authorization_configuration' }
+
+      delegate :project, to: :agent
     end
   end
 end

app/models/clusters/agents/implicit_authorization.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Agents
+    class ImplicitAuthorization
+      attr_reader :agent
+
+      delegate :id, to: :agent, prefix: true
+      delegate :project, to: :agent
+
+      def initialize(agent:)
+        @agent = agent
+      end
+
+      def config
+        nil
+      end
+    end
+  end
+end

config/feature_flags/development/group_authorized_agents.yml

+---
+name: group_authorized_agents
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69047
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340166
+milestone: '14.3'
+type: development
+group: group::configure
+default_enabled: false

ee/app/finders/clusters/agent_authorizations_finder.rb

+# frozen_string_literal: true
+
+module Clusters
+  class AgentAuthorizationsFinder
+    def initialize(project)
+      @project = project
+    end
+
+    def execute
+      return [] unless feature_available?
+
+      implicit_authorizations + group_authorizations
+    end
+
+    private
+
+    attr_reader :project
+
+    def feature_available?
+      project.licensed_feature_available?(:cluster_agents)
+    end
+
+    def implicit_authorizations
+      project.cluster_agents.map do |agent|
+        Clusters::Agents::ImplicitAuthorization.new(agent: agent)
+      end
+    end
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def group_authorizations
+      return [] unless project.group
+
+      authorizations = Clusters::Agents::GroupAuthorization.arel_table
+
+      ordered_ancestors_cte = Gitlab::SQL::CTE.new(
+        :ordered_ancestors,
+        project.group.self_and_ancestors(hierarchy_order: :asc).reselect(:id)
+      )
+
+      cte_join_sources = authorizations.join(ordered_ancestors_cte.table).on(
+        authorizations[:group_id].eq(ordered_ancestors_cte.table[:id])
+      ).join_sources
+
+      Clusters::Agents::GroupAuthorization
+        .with(ordered_ancestors_cte.to_arel)
+        .joins(cte_join_sources)
+        .joins(agent: :project)
+        .where('projects.namespace_id IN (SELECT id FROM ordered_ancestors)')
+        .order(Arel.sql('agent_id, array_position(ARRAY(SELECT id FROM ordered_ancestors)::bigint[], agent_group_authorizations.group_id)'))
+        .select('DISTINCT ON (agent_id) agent_group_authorizations.*')
+        .preload(agent: :project)
+        .to_a
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end

ee/app/models/ee/ci/pipeline.rb

@@ -175,9 +175,13 @@ module EE
 
       def authorized_cluster_agents
         strong_memoize(:authorized_cluster_agents) do
+          if ::Feature.enabled?(:group_authorized_agents, project, default_enabled: :yaml)
+            ::Clusters::AgentAuthorizationsFinder.new(project).execute.map(&:agent)
+          else
             ::Clusters::DeployableAgentsFinder.new(project).execute
           end
         end
+      end
 
       private
 

ee/lib/ee/api/ci/jobs.rb

@@ -19,10 +19,18 @@ module EE
 
               pipeline = current_authenticated_job.pipeline
               project = current_authenticated_job.project
-              allowed_agents = ::Clusters::DeployableAgentsFinder.new(project).execute
+
+              allowed_agents =
+                if ::Feature.enabled?(:group_authorized_agents, project, default_enabled: :yaml)
+                  agent_authorizations = ::Clusters::AgentAuthorizationsFinder.new(project).execute
+                  ::API::Entities::Clusters::AgentAuthorization.represent(agent_authorizations)
+                else
+                  associated_agents = ::Clusters::DeployableAgentsFinder.new(project).execute
+                  ::API::Entities::Clusters::Agent.represent(associated_agents)
+                end
 
               {
-                allowed_agents: ::API::Entities::Clusters::Agent.represent(allowed_agents),
+                allowed_agents: allowed_agents,
                 job: ::API::Entities::Ci::JobRequest::JobInfo.represent(current_authenticated_job),
                 pipeline: ::API::Entities::Ci::PipelineBasic.represent(pipeline),
                 project: ::API::Entities::ProjectIdentity.represent(project),

ee/spec/finders/clusters/agent_authorizations_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Clusters::AgentAuthorizationsFinder do
+  describe '#execute' do
+    let_it_be(:top_level_group) { create(:group) }
+    let_it_be(:subgroup1) { create(:group, parent: top_level_group) }
+    let_it_be(:subgroup2) { create(:group, parent: subgroup1) }
+    let_it_be(:bottom_level_group) { create(:group, parent: subgroup2) }
+
+    let_it_be(:agent_configuration_project) { create(:project, namespace: subgroup1) }
+    let_it_be(:requesting_project, reload: true) { create(:project, namespace: bottom_level_group) }
+
+    let_it_be(:staging_agent) { create(:cluster_agent, project: agent_configuration_project) }
+    let_it_be(:production_agent) { create(:cluster_agent, project: agent_configuration_project) }
+
+    let(:feature_available) { true }
+
+    subject { described_class.new(requesting_project).execute }
+
+    before do
+      stub_licensed_features(cluster_agents: feature_available)
+    end
+
+    context 'feature is not available' do
+      let(:feature_available) { false }
+
+      it { is_expected.to be_empty }
+    end
+
+    describe 'implicit authorizations' do
+      let!(:associated_agent) { create(:cluster_agent, project: requesting_project) }
+
+      it 'returns authorazations for agents directly associated with the project' do
+        expect(subject.count).to eq(1)
+
+        authorazation = subject.first
+        expect(authorazation).to be_a(Clusters::Agents::ImplicitAuthorization)
+        expect(authorazation.agent).to eq(associated_agent)
+      end
+    end
+
+    describe 'authorized groups' do
+      context 'agent configuration project is outside the requesting project hierarchy' do
+        let(:unrelated_agent) { create(:cluster_agent) }
+
+        before do
+          create(:agent_group_authorization, agent: unrelated_agent, group: top_level_group)
+        end
+
+        it { is_expected.to be_empty }
+      end
+
+      context 'multiple agents are authorized for the same group' do
+        let!(:staging_auth) { create(:agent_group_authorization, agent: staging_agent, group: bottom_level_group) }
+        let!(:production_auth) { create(:agent_group_authorization, agent: production_agent, group: bottom_level_group) }
+
+        it 'returns authorizations for all agents' do
+          expect(subject).to contain_exactly(staging_auth, production_auth)
+        end
+      end
+
+      context 'a single agent is authorized to more than one matching group' do
+        let!(:bottom_level_auth) { create(:agent_group_authorization, agent: production_agent, group: bottom_level_group) }
+        let!(:top_level_auth) { create(:agent_group_authorization, agent: production_agent, group: top_level_group) }
+
+        it 'picks the authorization for the closest group to the requesting project' do
+          expect(subject).to contain_exactly(bottom_level_auth)
+        end
+      end
+    end
+  end
+end

ee/spec/models/ci/pipeline_spec.rb

@@ -634,16 +634,34 @@ RSpec.describe Ci::Pipeline do
   end
 
   describe '#authorized_cluster_agents' do
-    let(:finder) { double(execute: agents) }
-    let(:agents) { double }
+    let(:agent) { instance_double(Clusters::Agent) }
+    let(:authorization) { instance_double(Clusters::Agents::GroupAuthorization, agent: agent) }
+    let(:finder) { double(execute: [authorization]) }
 
     it 'retrieves agent records from the finder and caches the result' do
+      expect(Clusters::AgentAuthorizationsFinder).to receive(:new).once
+      .with(pipeline.project)
+      .and_return(finder)
+
+      expect(pipeline.authorized_cluster_agents).to contain_exactly(agent)
+      expect(pipeline.authorized_cluster_agents).to contain_exactly(agent) # cached
+    end
+
+    context 'group_authorized_agents feature flag is disabled' do
+      let(:finder) { double(execute: [agent]) }
+
+      before do
+        stub_feature_flags(group_authorized_agents: false)
+      end
+
+      it 'retrieves agent records from the legacy finder and caches the result' do
         expect(Clusters::DeployableAgentsFinder).to receive(:new).once
         .with(pipeline.project)
         .and_return(finder)
 
-      expect(pipeline.authorized_cluster_agents).to eq(agents)
-      expect(pipeline.authorized_cluster_agents).to eq(agents) # cached
+        expect(pipeline.authorized_cluster_agents).to contain_exactly(agent)
+        expect(pipeline.authorized_cluster_agents).to contain_exactly(agent) # cached
+      end
     end
   end
 end

ee/spec/requests/api/ci/jobs_spec.rb

@@ -26,13 +26,18 @@ RSpec.describe API::Ci::Jobs do
   end
 
   describe 'GET /job/allowed_agents' do
-    let_it_be(:agent) { create(:cluster_agent, project: project) }
+    let_it_be(:group_authorization) { create(:agent_group_authorization) }
+    let_it_be(:associated_agent) { create(:cluster_agent, project: project) }
+
+    let(:implicit_authorization) { Clusters::Agents::ImplicitAuthorization.new(agent: associated_agent) }
+    let(:authorizations_finder) { double(execute: [implicit_authorization, group_authorization]) }
 
     let(:api_user) { developer }
     let(:headers) { { API::Ci::Helpers::Runner::JOB_TOKEN_HEADER => job.token } }
     let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user, status: job_status) }
     let(:job_status) { 'running' }
     let(:params) { {} }
+    let(:group_authorized_agents_enabled) { true }
 
     subject do
       get api('/job/allowed_agents'), headers: headers, params: params
@@ -40,7 +45,9 @@ RSpec.describe API::Ci::Jobs do
 
     before do
       stub_licensed_features(cluster_agents: true)
-      agent
+      stub_feature_flags(group_authorized_agents: group_authorized_agents_enabled)
+      allow(Clusters::AgentAuthorizationsFinder).to receive(:new).with(project).and_return(authorizations_finder)
+
       subject
     end
 
@@ -53,7 +60,16 @@ RSpec.describe API::Ci::Jobs do
         expect(json_response.dig('project', 'id')).to eq(job.project_id)
         expect(json_response.dig('user', 'username')).to eq(api_user.username)
         expect(json_response['allowed_agents']).to match_array([
-          { 'id' => agent.id, 'config_project' => a_hash_including('id' => agent.project_id) }
+          {
+            'id' => implicit_authorization.agent_id,
+            'config_project' => hash_including('id' => implicit_authorization.agent.project_id),
+            'configuration' => implicit_authorization.config
+          },
+          {
+            'id' => group_authorization.agent_id,
+            'config_project' => hash_including('id' => group_authorization.agent.project_id),
+            'configuration' => group_authorization.config
+          }
         ])
       end
 
@@ -69,7 +85,40 @@ RSpec.describe API::Ci::Jobs do
           expect(json_response.dig('project', 'id')).to eq(job.project_id)
           expect(json_response.dig('user', 'username')).to eq(api_user.username)
           expect(json_response['allowed_agents']).to match_array([
-            { 'id' => agent.id, 'config_project' => a_hash_including('id' => agent.project_id) }
+            {
+              'id' => implicit_authorization.agent_id,
+              'config_project' => hash_including('id' => implicit_authorization.agent.project_id),
+              'configuration' => implicit_authorization.config
+            },
+            {
+              'id' => group_authorization.agent_id,
+              'config_project' => a_hash_including('id' => group_authorization.agent.project_id),
+              'configuration' => group_authorization.config
+            }
+          ])
+        end
+      end
+
+      context 'group_authorized_agents feature flag is disabled' do
+        let(:group_authorized_agents_enabled) { false }
+        let(:agents_finder) { double(execute: [associated_agent]) }
+
+        before do
+          allow(Clusters::DeployableAgentsFinder).to receive(:new).with(project).and_return(agents_finder)
+        end
+
+        it 'returns agent info', :aggregate_failures do
+          expect(response).to have_gitlab_http_status(:ok)
+
+          expect(json_response.dig('job', 'id')).to eq(job.id)
+          expect(json_response.dig('pipeline', 'id')).to eq(job.pipeline_id)
+          expect(json_response.dig('project', 'id')).to eq(job.project_id)
+          expect(json_response.dig('user', 'username')).to eq(api_user.username)
+          expect(json_response['allowed_agents']).to match_array([
+            {
+              'id' => associated_agent.id,
+              'config_project' => hash_including('id' => associated_agent.project_id)
+            }
           ])
         end
       end

lib/api/entities/clusters/agent_authorization.rb

+# frozen_string_literal: true
+
+module API
+  module Entities
+    module Clusters
+      class AgentAuthorization < Grape::Entity
+        expose :agent_id, as: :id
+        expose :project, with: Entities::ProjectIdentity, as: :config_project
+        expose :config, as: :configuration
+      end
+    end
+  end
+end

spec/lib/api/entities/clusters/agent_authorization_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe API::Entities::Clusters::AgentAuthorization do
+  let_it_be(:authorization) { create(:agent_group_authorization) }
+
+  subject { described_class.new(authorization).as_json }
+
+  it 'includes basic fields' do
+    expect(subject).to include(
+      id: authorization.agent_id,
+      config_project: a_hash_including(id: authorization.agent.project_id),
+      configuration: authorization.config
+    )
+  end
+end

spec/models/clusters/agents/implicit_authorization_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Clusters::Agents::ImplicitAuthorization do
+  let_it_be(:agent) { create(:cluster_agent) }
+
+  subject { described_class.new(agent: agent) }
+
+  it { expect(subject.agent).to eq(agent) }
+  it { expect(subject.agent_id).to eq(agent.id) }
+  it { expect(subject.project).to eq(agent.project) }
+  it { expect(subject.config).to be_nil }
+end