Merge branch...

Merge branch '467936-nwittstruck-limit-kubernetes-agent-access-to-protected-branches' into 'master' 

Allow the restriction of Kubernetes agent access to protected branches

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156626



Merged-by: Fabio Pitino <fpitino@gitlab.com>
Approved-by: Tiger Watson <twatson@gitlab.com>
Approved-by: Fabio Pitino <fpitino@gitlab.com>
Reviewed-by: Fabio Pitino <fpitino@gitlab.com>
Reviewed-by: Vitali Tatarintev <vtatarintev@gitlab.com>
Co-authored-by: Nicholas <1494491-nwittstruck@users.noreply.gitlab.com>
Co-authored-by: Nicholas Wittstruck <nicholas@b310.de>

app/services/ci/generate_kubeconfig_service.rb

@@ -43,7 +43,9 @@ def execute
     def agent_authorizations
       ::Clusters::Agents::Authorizations::CiAccess::FilterService.new(
         pipeline.cluster_agent_authorizations,
-        environment: environment
+        { environment: environment,
+          protected_ref: pipeline.protected_ref? },
+        pipeline.project
       ).execute
     end
 

app/services/clusters/agents/authorizations/ci_access/filter_service.rb

@@ -5,20 +5,26 @@ module Agents
     module Authorizations
       module CiAccess
         class FilterService
-          def initialize(authorizations, filter_params)
+          def initialize(authorizations, filter_params, project)
             @authorizations = authorizations
             @filter_params = filter_params
+            @project = project
 
             @environments_matcher = {}
           end
 
           def execute
-            filter_by_environment(authorizations)
+            filtered_authorizations = filter_by_environment(authorizations)
+            if Feature.enabled?(:kubernetes_agent_protected_branches, project)
+              filtered_authorizations = filter_protected_ref(filtered_authorizations)
+            end
+
+            filtered_authorizations
           end
 
           private
 
-          attr_reader :authorizations, :filter_params
+          attr_reader :authorizations, :filter_params, :project
 
           def filter_by_environment(auths)
             return auths unless filter_by_environment?
@@ -47,6 +53,26 @@ def matches_environment?(environment_pattern)
           def environments_matcher(environment_pattern)
             @environments_matcher[environment_pattern] ||= ::Gitlab::Ci::EnvironmentMatcher.new(environment_pattern)
           end
+
+          def filter_protected_ref(authorizations)
+            # we deny all if the protected_ref is not set, since we can't check if the branch is protected:
+            return [] unless protected_ref_filter_present?
+
+            # when the branch is protected we don't need to check the authorization settings
+            return authorizations if filter_params[:protected_ref]
+
+            authorizations.reject do |authorization|
+              only_run_on_protected_ref?(authorization)
+            end
+          end
+
+          def protected_ref_filter_present?
+            filter_params.has_key?(:protected_ref)
+          end
+
+          def only_run_on_protected_ref?(authorization)
+            authorization.config['protected_branches_only']
+          end
         end
       end
     end

config/feature_flags/beta/kubernetes_agent_protected_branches.yml

+---
+name: kubernetes_agent_protected_branches
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/388323
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156626
+rollout_issue_url:
+milestone: '17.3'
+group: group::environments
+type: beta
+default_enabled: false

doc/user/clusters/agent/ci_cd_workflow.md

@@ -332,6 +332,55 @@ In this example:
   - `*` is a wildcard, so `review/*` matches all environments under `review`.
 - CI/CD jobs for projects under `group-1` with `production` environments can access the agent.
 
+## Restrict access to the agent to protected branches
+
+DETAILS:
+**Tier:** Free, Premium, Ultimate
+**Offering:** Self-managed, GitLab Dedicated
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/388323) in GitLab 17.3.
+
+To restrict access to the agent to only jobs run on [protected branches](../../project/protected_branches.md):
+
+- Add `protected_branches_only: true` to `ci_access.projects` or `ci_access.groups`.
+  For example:
+
+  ```yaml
+  ci_access:
+    projects:
+      - id: path/to/project-1
+        protected_branches_only: true
+    groups:
+      - id: path/to/group-1
+        protected_branches_only: true
+        environments:
+          - production
+  ```
+
+By default, `protected_branches_only` is set to `false`, and the agent can be accessed from unprotected and protected branches.
+
+For additional security, you can combine this feature with [environment restrictions](#restrict-project-and-group-access-to-specific-environments).
+
+If a project has multiple configurations, only the most specific configuration is used.
+For example, the following configuration grants access to unprotected branches in `example/my-project`, even though the `example` group is configured to grant access to only protected branches:
+
+```yaml
+# .gitlab/agents/my-agent/config.yaml
+ci_access:
+  project:
+    - id: example/my-project # Project of the group below
+      protected_branches_only: false # This configuration supercedes the group configuration
+      environments:
+        - dev
+  groups:
+    - id: example
+      protected_branches_only: true
+      environments:
+        - dev
+```
+
+For more details, see [Access to Kubernetes from CI](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/b601fa21cac24f0cdedc8b8eb59ebcba0b70f459/doc/kubernetes_ci_access.md#apiv4joballowed_agents-api).
+
 ## Related topics
 
 - [Self-paced classroom workshop](https://gitlab-for-eks.awsworkshop.io) (Uses AWS EKS, but you can use for other Kubernetes clusters)

lib/api/ci/jobs.rb

@@ -269,7 +269,9 @@ class Jobs < ::API::Base
 
           agent_authorizations = ::Clusters::Agents::Authorizations::CiAccess::FilterService.new(
             ::Clusters::Agents::Authorizations::CiAccess::Finder.new(project).execute,
-            environment: persisted_environment&.name
+            { environment: persisted_environment&.name,
+              protected_ref: pipeline.protected_ref? },
+            pipeline.project
           ).execute
 
           # See https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/doc/kubernetes_ci_access.md#apiv4joballowed_agents-api

spec/factories/clusters/agents/authorizations/ci_access/group_authorizations.rb

@@ -4,14 +4,15 @@
   factory :agent_ci_access_group_authorization, class: 'Clusters::Agents::Authorizations::CiAccess::GroupAuthorization' do
     association :agent, factory: :cluster_agent
     group
-
     transient do
       environments { nil }
+      protected_branches_only { false }
     end
 
     config do
       { default_namespace: 'production' }.tap do |c|
         c[:environments] = environments if environments
+        c[:protected_branches_only] = protected_branches_only
       end
     end
   end

spec/factories/clusters/agents/authorizations/ci_access/project_authorizations.rb

@@ -7,11 +7,13 @@
 
     transient do
       environments { nil }
+      protected_branches_only { false }
     end
 
     config do
       { default_namespace: 'production' }.tap do |c|
         c[:environments] = environments if environments
+        c[:protected_branches_only] = protected_branches_only
       end
     end
   end

spec/requests/api/graphql/project/ci_access_authorized_agents_spec.rb

@@ -50,7 +50,8 @@
 
       expect(authorized_agent['agent']['id']).to eq(agent.to_global_id.to_s)
       expect(authorized_agent['agent']['name']).to eq(agent.name)
-      expect(authorized_agent['config']).to eq({ "default_namespace" => "production" })
+      expect(authorized_agent['config']).to eq({ "default_namespace" => "production",
+                                                 "protected_branches_only" => false })
       expect(authorized_agent['agent']['project']).to be_nil # User is not authorized to read other resources.
     end
 
@@ -87,7 +88,8 @@
 
       expect(authorized_agent['agent']['id']).to eq(agent.to_global_id.to_s)
       expect(authorized_agent['agent']['name']).to eq(agent.name)
-      expect(authorized_agent['config']).to eq({ "default_namespace" => "production" })
+      expect(authorized_agent['config']).to eq({ "default_namespace" => "production",
+                                                "protected_branches_only" => false })
       expect(authorized_agent['agent']['project']).to be_nil # User is not authorized to read other resources.
     end
 

spec/services/ci/generate_kubeconfig_service_spec.rb

@@ -61,7 +61,9 @@
     it "filters the pipeline's agents by `nil` environment" do
       expect(::Clusters::Agents::Authorizations::CiAccess::FilterService).to receive(:new).with(
         pipeline.cluster_agent_authorizations,
-        environment: nil
+        { environment: nil,
+          protected_ref: false },
+        project
       )
 
       execute
@@ -91,7 +93,9 @@
       it "filters the pipeline's agents by the specified environment" do
         expect(::Clusters::Agents::Authorizations::CiAccess::FilterService).to receive(:new).with(
           pipeline.cluster_agent_authorizations,
-          environment: 'production'
+          { environment: 'production',
+            protected_ref: false },
+          project
         )
 
         execute

spec/services/clusters/agents/authorizations/ci_access/filter_service_spec.rb

@@ -15,9 +15,9 @@
       ]
     end
 
-    let(:filter_params) { {} }
+    let(:filter_params) { { protected_ref: true } }
 
-    subject(:execute_filter) { described_class.new(agent_authorizations, filter_params).execute }
+    subject(:execute_filter) { described_class.new(agent_authorizations, filter_params, project).execute }
 
     context 'when there are no filters' do
       let(:agent_authorizations) { agent_authorizations_without_env }
@@ -34,13 +34,15 @@
             :agent_ci_access_project_authorization,
             project: project,
             agent: build(:cluster_agent, project: project),
-            environments: ['staging', 'review/*', 'production']
+            environments: ['staging', 'review/*', 'production'],
+            protected_branches_only: false
           ),
           build(
             :agent_ci_access_group_authorization,
             group: group,
             agent: build(:cluster_agent, project: project),
-            environments: ['staging', 'review/*', 'production']
+            environments: ['staging', 'review/*', 'production'],
+            protected_branches_only: false
           )
         ]
       end
@@ -62,15 +64,35 @@
         ]
       end
 
+      let(:agent_authorizations_with_env_and_protected_branches) do
+        [
+          build(
+            :agent_ci_access_project_authorization,
+            project: project,
+            agent: build(:cluster_agent, project: project),
+            environments: ['staging', 'review/*', 'production'],
+            protected_branches_only: true
+          ),
+          build(
+            :agent_ci_access_group_authorization,
+            group: group,
+            agent: build(:cluster_agent, project: project),
+            environments: ['staging', 'review/*', 'production'],
+            protected_branches_only: true
+          )
+        ]
+      end
+
       let(:agent_authorizations) do
         (
           agent_authorizations_without_env +
           agent_authorizations_with_env +
-          agent_authorizations_with_different_env
+          agent_authorizations_with_different_env +
+          agent_authorizations_with_env_and_protected_branches
         )
       end
 
-      let(:filter_params) { { environment: 'production' } }
+      let(:filter_params) { { environment: 'production', protected_ref: false } }
 
       it 'returns the authorizations with the given environment AND authorizations without any environment' do
         expected_authorizations = agent_authorizations_with_env + agent_authorizations_without_env
@@ -79,7 +101,7 @@
       end
 
       context 'when environment filter has a wildcard' do
-        let(:filter_params) { { environment: 'review/123' } }
+        let(:filter_params) { { environment: 'review/123', protected_ref: false } }
 
         it 'returns the authorizations with matching environments AND authorizations without any environment' do
           expected_authorizations = agent_authorizations_with_env + agent_authorizations_without_env
@@ -89,12 +111,85 @@
       end
 
       context 'when environment filter is nil' do
-        let(:filter_params) { { environment: nil } }
+        let(:filter_params) { { environment: nil, protected_ref: false } }
 
         it 'returns the authorizations without any environment' do
           expect(execute_filter).to match_array agent_authorizations_without_env
         end
       end
+
+      context 'when executed on protected branch' do
+        let(:filter_params) { { environment: 'production', protected_ref: true } }
+
+        it 'returns the authorizations with the given environment AND authorizations without any environment AND the authorizations with protected branches' do
+          expected_authorizations = agent_authorizations_with_env + agent_authorizations_without_env + agent_authorizations_with_env_and_protected_branches
+
+          expect(execute_filter).to match_array expected_authorizations
+        end
+      end
+    end
+
+    context 'when filtering protected branches' do
+      let(:agent_authorizations_with_protected_agent) do
+        build(
+          :agent_ci_access_project_authorization,
+          project: project,
+          agent: build(:cluster_agent, project: project),
+          protected_branches_only: protected
+        )
+      end
+
+      let(:agent_authorizations) { [agent_authorizations_with_protected_agent] }
+
+      context 'with protected agent' do
+        let(:protected) { true }
+
+        context 'on protected branch' do
+          let(:filter_params) { { protected_ref: true } }
+
+          it 'does return the authorizations as is' do
+            expect(execute_filter).to match_array agent_authorizations_with_protected_agent
+          end
+        end
+
+        context 'on unprotected branch' do
+          let(:filter_params) { { protected_ref: false } }
+
+          it 'does not return any authorizations' do
+            expect(execute_filter).to eq []
+          end
+
+          context 'when kubernetes_agent_protected_branches is disabled' do
+            before do
+              stub_feature_flags(kubernetes_agent_protected_branches: false)
+            end
+
+            it 'does not filter for protected_ref' do
+              expect(execute_filter).to match_array agent_authorizations_with_protected_agent
+            end
+          end
+        end
+      end
+
+      context 'with unprotected agent' do
+        let(:protected) { false }
+
+        context 'on protected branch' do
+          let(:filter_params) { { protected_ref: true } }
+
+          it 'does return the authorizations as is' do
+            expect(execute_filter).to match_array agent_authorizations_with_protected_agent
+          end
+        end
+
+        context 'on unprotected branch' do
+          let(:filter_params) { { protected_ref: false } }
+
+          it 'does return the authorizations as is' do
+            expect(execute_filter).to match_array agent_authorizations_with_protected_agent
+          end
+        end
+      end
     end
   end
 end

spec/services/clusters/agents/authorizations/ci_access/refresh_service_spec.rb

@@ -23,12 +23,13 @@
           groups: [
             { id: added_group.full_path, default_namespace: 'default' },
             # Uppercase path verifies case-insensitive matching.
-            { id: modified_group.full_path.upcase, default_namespace: 'new-namespace' }
+            { id: modified_group.full_path.upcase, default_namespace: 'new-namespace', protected_branches_only: 'true' }
           ],
           projects: [
             { id: added_project.full_path, default_namespace: 'default' },
             # Uppercase path verifies case-insensitive matching.
-            { id: modified_project.full_path.upcase, default_namespace: 'new-namespace' }
+            { id: modified_project.full_path.upcase, default_namespace: 'new-namespace',
+              protected_branches_only: 'true' }
           ]
         }
       }.deep_stringify_keys
@@ -84,7 +85,8 @@
         expect(added_authorization.config).to eq({ 'default_namespace' => 'default' })
 
         modified_authorization = agent.ci_access_group_authorizations.find_by(group: modified_group)
-        expect(modified_authorization.config).to eq({ 'default_namespace' => 'new-namespace' })
+        expect(modified_authorization.config).to eq({ 'default_namespace' => 'new-namespace',
+                                                      'protected_branches_only' => 'true' })
       end
 
       context 'config contains too many groups' do
@@ -112,7 +114,8 @@
         expect(added_authorization.config).to eq({ 'default_namespace' => 'default' })
 
         modified_authorization = agent.ci_access_project_authorizations.find_by(project: modified_project)
-        expect(modified_authorization.config).to eq({ 'default_namespace' => 'new-namespace' })
+        expect(modified_authorization.config).to eq({ 'default_namespace' => 'new-namespace',
+                                                      'protected_branches_only' => 'true' })
       end
 
       context 'project does not belong to a group, and is in the same namespace as the agent' do