Merge branch '323708-agent-authorized-groups' into 'master'

Persist groups configured to use an Agent

See merge request gitlab-org/gitlab!68023

app/models/clusters/agent.rb

@@ -10,6 +10,9 @@ class Agent < ApplicationRecord
     has_many :agent_tokens, class_name: 'Clusters::AgentToken'
     has_many :last_used_agent_tokens, -> { order_last_used_at_desc }, class_name: 'Clusters::AgentToken', inverse_of: :agent
+    has_many :group_authorizations, class_name: 'Clusters::Agents::GroupAuthorization'
+    has_many :authorized_groups, class_name: '::Group', through: :group_authorizations, source: :group
+
     scope :ordered_by_name, -> { order(:name) }
     scope :with_name, -> (name) { where(name: name) }

app/models/clusters/agents/group_authorization.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Agents
+    class GroupAuthorization < ApplicationRecord
+      self.table_name = 'agent_group_authorizations'
+
+      belongs_to :agent, class_name: 'Clusters::Agent', optional: false
+      belongs_to :group, class_name: '::Group', optional: false
+
+      validates :config, json_schema: { filename: 'cluster_agent_authorization_configuration' }
+    end
+  end
+end

app/services/clusters/agents/refresh_authorization_service.rb

+# frozen_string_literal: true
+
+module Clusters
+  module Agents
+    class RefreshAuthorizationService
+      include Gitlab::Utils::StrongMemoize
+
+      AUTHORIZED_GROUP_LIMIT = 100
+
+      delegate :project, to: :agent, private: true
+
+      def initialize(agent, config:)
+        @agent = agent
+        @config = config
+      end
+
+      def execute
+        if allowed_group_configurations.present?
+          group_ids = allowed_group_configurations.map { |config| config.fetch(:group_id) }
+
+          agent.with_lock do
+            agent.group_authorizations.upsert_all(allowed_group_configurations, unique_by: [:agent_id, :group_id])
+            agent.group_authorizations.where.not(group_id: group_ids).delete_all # rubocop: disable CodeReuse/ActiveRecord
+          end
+        else
+          agent.group_authorizations.delete_all(:delete_all)
+        end
+
+        true
+      end
+
+      private
+
+      attr_reader :agent, :config
+
+      def allowed_group_configurations
+        strong_memoize(:allowed_group_configurations) do
+          group_entries = config.dig('ci_access', 'groups')&.first(AUTHORIZED_GROUP_LIMIT)
+
+          if group_entries
+            groups_by_path = group_entries.index_by { |config| config.delete('id') }
+
+            allowed_groups.where_full_path_in(groups_by_path.keys).map do |group|
+              { group_id: group.id, config: groups_by_path[group.full_path] }
+            end
+          end
+        end
+      end
+
+      def allowed_groups
+        if project.root_ancestor.group?
+          project.root_ancestor.self_and_descendants
+        else
+          ::Group.none
+        end
+      end
+    end
+  end
+end

app/validators/json_schemas/cluster_agent_authorization_configuration.json

+{
+  "$schema": "http://json-schema.org/draft-07/schema#",
+  "description": "Cluster Agent configuration for an authorized project or group",
+  "type": "object",
+  "additionalProperties": true
+}

db/migrate/20210809014850_create_agent_group_authorizations.rb

+# frozen_string_literal: true
+
+class CreateAgentGroupAuthorizations < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  def change
+    create_table :agent_group_authorizations do |t|
+      t.bigint :group_id, null: false
+      t.bigint :agent_id, null: false
+      t.jsonb :config, null: false
+
+      t.index :group_id
+      t.index [:agent_id, :group_id], unique: true
+    end
+  end
+end

db/migrate/20210809014918_add_agent_group_authorizations_foreign_keys.rb

+# frozen_string_literal: true
+
+class AddAgentGroupAuthorizationsForeignKeys < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :agent_group_authorizations, :namespaces, column: :group_id
+    add_concurrent_foreign_key :agent_group_authorizations, :cluster_agents, column: :agent_id
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key_if_exists :agent_group_authorizations, column: :group_id
+    end
+
+    with_lock_retries do
+      remove_foreign_key_if_exists :agent_group_authorizations, column: :agent_id
+    end
+  end
+end

db/schema_migrations/20210809014850

+6f67e2bba5f42d48a9b21f8ab4d9abf4495ef7e0226ea903d51e77eed85ad0cb
\ No newline at end of file

db/schema_migrations/20210809014918

+d282a027d03920a53d49444f54745ab7d2c8bcccc485ac9407ff9dbbef77981f
\ No newline at end of file

db/structure.sql

@@ -8958,6 +8958,22 @@ CREATE SEQUENCE abuse_reports_id_seq
 ALTER SEQUENCE abuse_reports_id_seq OWNED BY abuse_reports.id;
+CREATE TABLE agent_group_authorizations (
+    id bigint NOT NULL,
+    group_id bigint NOT NULL,
+    agent_id bigint NOT NULL,
+    config jsonb NOT NULL
+);
+
+CREATE SEQUENCE agent_group_authorizations_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE agent_group_authorizations_id_seq OWNED BY agent_group_authorizations.id;
+
 CREATE TABLE alert_management_alert_assignees (
     id bigint NOT NULL,
     user_id bigint NOT NULL,
@@ -20066,6 +20082,8 @@ ALTER SEQUENCE zoom_meetings_id_seq OWNED BY zoom_meetings.id;
 ALTER TABLE ONLY abuse_reports ALTER COLUMN id SET DEFAULT nextval('abuse_reports_id_seq'::regclass);
+ALTER TABLE ONLY agent_group_authorizations ALTER COLUMN id SET DEFAULT nextval('agent_group_authorizations_id_seq'::regclass);
+
 ALTER TABLE ONLY alert_management_alert_assignees ALTER COLUMN id SET DEFAULT nextval('alert_management_alert_assignees_id_seq'::regclass);
 ALTER TABLE ONLY alert_management_alert_user_mentions ALTER COLUMN id SET DEFAULT nextval('alert_management_alert_user_mentions_id_seq'::regclass);
@@ -21190,6 +21208,9 @@ ALTER TABLE ONLY gitlab_partitions_static.product_analytics_events_experimental_
 ALTER TABLE ONLY abuse_reports
     ADD CONSTRAINT abuse_reports_pkey PRIMARY KEY (id);
+ALTER TABLE ONLY agent_group_authorizations
+    ADD CONSTRAINT agent_group_authorizations_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY alert_management_alert_assignees
     ADD CONSTRAINT alert_management_alert_assignees_pkey PRIMARY KEY (id);
@@ -23113,6 +23134,10 @@ CREATE UNIQUE INDEX idx_vulnerability_issue_links_on_vulnerability_id_and_link_t
 CREATE INDEX index_abuse_reports_on_user_id ON abuse_reports USING btree (user_id);
+CREATE UNIQUE INDEX index_agent_group_authorizations_on_agent_id_and_group_id ON agent_group_authorizations USING btree (agent_id, group_id);
+
+CREATE INDEX index_agent_group_authorizations_on_group_id ON agent_group_authorizations USING btree (group_id);
+
 CREATE INDEX index_alert_assignees_on_alert_id ON alert_management_alert_assignees USING btree (alert_id);
 CREATE UNIQUE INDEX index_alert_assignees_on_user_id_and_alert_id ON alert_management_alert_assignees USING btree (user_id, alert_id);
@@ -26293,6 +26318,9 @@ ALTER TABLE ONLY geo_event_log
 ALTER TABLE ONLY deployments
     ADD CONSTRAINT fk_289bba3222 FOREIGN KEY (cluster_id) REFERENCES clusters(id) ON DELETE SET NULL;
+ALTER TABLE ONLY agent_group_authorizations
+    ADD CONSTRAINT fk_2c9f941965 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY ci_freeze_periods
     ADD CONSTRAINT fk_2e02bbd1a6 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
@@ -26953,6 +26981,9 @@ ALTER TABLE ONLY dep_ci_build_trace_section_names
 ALTER TABLE ONLY ci_stages
     ADD CONSTRAINT fk_fb57e6cc56 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;
+ALTER TABLE ONLY agent_group_authorizations
+    ADD CONSTRAINT fk_fb70782616 FOREIGN KEY (agent_id) REFERENCES cluster_agents(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY system_note_metadata
     ADD CONSTRAINT fk_fbd87415c9 FOREIGN KEY (description_version_id) REFERENCES description_versions(id) ON DELETE SET NULL;

lib/api/internal/kubernetes.rb

@@ -100,6 +100,23 @@ def check_agent_token
           end
         end
+        namespace 'kubernetes/agent_configuration' do
+          desc 'POST agent configuration' do
+            detail 'Store configuration for an agent'
+          end
+          params do
+            requires :agent_id, type: Integer, desc: 'ID of the configured Agent'
+            requires :agent_config, type: JSON, desc: 'Configuration for the Agent'
+          end
+          post '/' do
+            agent = Clusters::Agent.find(params[:agent_id])
+
+            Clusters::Agents::RefreshAuthorizationService.new(agent, config: params[:agent_config]).execute
+
+            no_content!
+          end
+        end
+
         namespace 'kubernetes/usage_metrics' do
           desc 'POST usage metrics' do
             detail 'Updates usage metrics for agent'

spec/factories/clusters/agents/group_authorizations.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :agent_group_authorization, class: 'Clusters::Agents::GroupAuthorization' do
+    association :agent, factory: :cluster_agent
+    group
+
+    config { { default_namespace: 'production' } }
+  end
+end

spec/models/clusters/agent_spec.rb

@@ -9,6 +9,8 @@
   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 have_many(:group_authorizations).class_name('Clusters::Agents::GroupAuthorization') }
+  it { is_expected.to have_many(:authorized_groups).through(:group_authorizations) }
   it { is_expected.to validate_presence_of(:name) }
   it { is_expected.to validate_length_of(:name).is_at_most(63) }

spec/models/clusters/agents/group_authorization_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Clusters::Agents::GroupAuthorization do
+  it { is_expected.to belong_to(:agent).class_name('Clusters::Agent').required }
+  it { is_expected.to belong_to(:group).class_name('::Group').required }
+
+  it { expect(described_class).to validate_jsonb_schema(['config']) }
+end

spec/requests/api/internal/kubernetes_spec.rb

@@ -93,6 +93,44 @@ def send_request(headers: {}, params: {})
     end
   end
+  describe 'POST /internal/kubernetes/agent_configuration' do
+    def send_request(headers: {}, params: {})
+      post api('/internal/kubernetes/agent_configuration'), params: params, headers: headers.reverse_merge(jwt_auth_headers)
+    end
+
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, namespace: group) }
+    let_it_be(:agent) { create(:cluster_agent, project: project) }
+    let_it_be(:config) do
+      {
+        ci_access: {
+          groups: [
+            { id: group.full_path, default_namespace: 'production' }
+          ]
+        }
+      }
+    end
+
+    include_examples 'authorization'
+
+    context 'agent exists' do
+      it 'configures the agent and returns a 204' do
+        send_request(params: { agent_id: agent.id, agent_config: config })
+
+        expect(response).to have_gitlab_http_status(:no_content)
+        expect(agent.authorized_groups).to contain_exactly(group)
+      end
+    end
+
+    context 'agent does not exist' do
+      it 'returns a 404' do
+        send_request(params: { agent_id: -1, agent_config: config })
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+
   describe 'GET /internal/kubernetes/agent_info' do
     def send_request(headers: {}, params: {})
       get api('/internal/kubernetes/agent_info'), params: params, headers: headers.reverse_merge(jwt_auth_headers)

spec/services/clusters/agents/refresh_authorization_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Clusters::Agents::RefreshAuthorizationService do
+  describe '#execute' do
+    let_it_be(:root_ancestor) { create(:group) }
+    let_it_be(:removed_group) { create(:group, parent: root_ancestor) }
+    let_it_be(:modified_group) { create(:group, parent: root_ancestor) }
+    let_it_be(:added_group) { create(:group, parent: root_ancestor) }
+
+    let(:project) { create(:project, namespace: root_ancestor) }
+    let(:agent) { create(:cluster_agent, project: project) }
+
+    let(:config) do
+      {
+        ci_access: {
+          groups: [
+            { id: added_group.full_path, default_namespace: 'default' },
+            { id: modified_group.full_path, default_namespace: 'new-namespace' }
+          ]
+        }
+      }.deep_stringify_keys
+    end
+
+    subject { described_class.new(agent, config: config).execute }
+
+    before do
+      default_config = { default_namespace: 'default' }
+
+      agent.group_authorizations.create!(group: removed_group, config: default_config)
+      agent.group_authorizations.create!(group: modified_group, config: default_config)
+    end
+
+    it 'refreshes authorizations for the agent' do
+      expect(subject).to be_truthy
+      expect(agent.authorized_groups).to contain_exactly(added_group, modified_group)
+
+      added_authorization = agent.group_authorizations.find_by(group: added_group)
+      expect(added_authorization.config).to eq({ 'default_namespace' => 'default' })
+
+      modified_authorization = agent.group_authorizations.find_by(group: modified_group)
+      expect(modified_authorization.config).to eq({ 'default_namespace' => 'new-namespace' })
+    end
+
+    context 'config contains no groups' do
+      let(:config) { {} }
+
+      it 'removes all authorizations' do
+        expect(subject).to be_truthy
+        expect(agent.authorized_groups).to be_empty
+      end
+    end
+
+    context 'config contains groups outside of the configuration project hierarchy' do
+      let(:project) { create(:project, namespace: create(:group)) }
+
+      it 'removes all authorizations' do
+        expect(subject).to be_truthy
+        expect(agent.authorized_groups).to be_empty
+      end
+    end
+
+    context 'configuration project does not belong to a group' do
+      let(:project) { create(:project) }
+
+      it 'removes all authorizations' do
+        expect(subject).to be_truthy
+        expect(agent.authorized_groups).to be_empty
+      end
+    end
+
+    context 'config contains too many groups' do
+      before do
+        stub_const("#{described_class}::AUTHORIZED_GROUP_LIMIT", 1)
+      end
+
+      it 'authorizes groups up to the limit' do
+        expect(subject).to be_truthy
+        expect(agent.authorized_groups).to contain_exactly(added_group)
+      end
+    end
+  end
+end