Don't use Kubernetes namespaces with no token63079-exclude-k8s-namespaces-with-no-service-account-token

Whenever we are selecting a namespace to use for a
deployment or to query a cluster we want to exclude
Kubernetes namespace records that don't have a token
set as they will not have the required permissions.
However when configuring clusters, we want to
use the original namespace record even if it has no
token, as a namespace has to be unique on a cluster.

5 files changed, 101 insertions, 5 deletions

diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb
index ccc877fb924..0206ce81c5f 100644
--- a/app/models/clusters/cluster.rb
+++ b/app/models/clusters/cluster.rb
@@ -193,15 +193,34 @@ module Clusters
       platform_kubernetes.kubeclient if kubernetes?
     end
 
+    ##
+    # This is subtly different to #find_or_initialize_kubernetes_namespace_for_project
+    # below because it will ignore any namespaces that have not got a service account
+    # token. This provides a guarantee that any namespace selected here can be used
+    # for cluster operations - a namespace needs to have a service account configured
+    # before it it can be used.
+    #
+    # This is used for selecting a namespace to use when querying a cluster, or
+    # generating variables to pass to CI.
     def kubernetes_namespace_for(project)
-      find_or_initialize_kubernetes_namespace_for_project(project).namespace
+      find_or_initialize_kubernetes_namespace_for_project(
+        project, scope: kubernetes_namespaces.has_service_account_token
+      ).namespace
     end
 
-    def find_or_initialize_kubernetes_namespace_for_project(project)
+    ##
+    # This is subtly different to #kubernetes_namespace_for because it will include
+    # namespaces that have yet to receive a service account token. This allows
+    # the namespace configuration process to be repeatable - if a namespace has
+    # already been created without a token we don't need to create another
+    # record entirely, just set the token on the pre-existing namespace.
+    #
+    # This is used for configuring cluster namespaces.
+    def find_or_initialize_kubernetes_namespace_for_project(project, scope: kubernetes_namespaces)
       attributes = { project: project }
       attributes[:cluster_project] = cluster_project if project_type?
 
-      kubernetes_namespaces.find_or_initialize_by(attributes).tap do |namespace|
+      scope.find_or_initialize_by(attributes).tap do |namespace|
         namespace.set_defaults
       end
     end
diff --git a/changelogs/unreleased/63079-exclude-k8s-namespaces-with-no-service-account-token.yml b/changelogs/unreleased/63079-exclude-k8s-namespaces-with-no-service-account-token.yml
new file mode 100644
index 00000000000..9dc99c8a62f
--- /dev/null
+++ b/changelogs/unreleased/63079-exclude-k8s-namespaces-with-no-service-account-token.yml
@@ -0,0 +1,6 @@
+---
+title: Ensure a Kubernetes namespace is not used for deployments if there is no service
+  account token associated with it
+merge_request: 29643
+author:
+type: fixed
diff --git a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb
index 5387863bd07..b1cfb8e1c62 100644
--- a/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb
+++ b/spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb
@@ -35,7 +35,7 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do
         end
 
         context 'and a namespace is already created for this project' do
-          let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster, project: build.project) }
+          let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster, project: build.project) }
 
           it { is_expected.to be_falsey }
         end
diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb
index f206bb41f45..c302b7a15f4 100644
--- a/spec/models/clusters/cluster_spec.rb
+++ b/spec/models/clusters/cluster_spec.rb
@@ -555,6 +555,63 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '#find_or_initialize_kubernetes_namespace_for_project' do
+    let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+    let(:project) { cluster.projects.first }
+
+    subject { cluster.find_or_initialize_kubernetes_namespace_for_project(project) }
+
+    context 'kubernetes namespace exists' do
+      context 'with no service account token' do
+        let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, project: project, cluster: cluster) }
+
+        it { is_expected.to eq kubernetes_namespace }
+      end
+
+      context 'with a service account token' do
+        let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, project: project, cluster: cluster) }
+
+        it { is_expected.to eq kubernetes_namespace }
+      end
+    end
+
+    context 'kubernetes namespace does not exist' do
+      it 'initializes a new namespace and sets default values' do
+        expect(subject).to be_new_record
+        expect(subject.project).to eq project
+        expect(subject.cluster).to eq cluster
+        expect(subject.namespace).to be_present
+        expect(subject.service_account_name).to be_present
+      end
+    end
+
+    context 'a custom scope is provided' do
+      let(:scope) { cluster.kubernetes_namespaces.has_service_account_token }
+
+      subject { cluster.find_or_initialize_kubernetes_namespace_for_project(project, scope: scope) }
+
+      context 'kubernetes namespace exists' do
+        context 'with no service account token' do
+          let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, project: project, cluster: cluster) }
+
+          it 'initializes a new namespace and sets default values' do
+            expect(subject).to be_new_record
+            expect(subject.project).to eq project
+            expect(subject.cluster).to eq cluster
+            expect(subject.namespace).to be_present
+            expect(subject.service_account_name).to be_present
+          end
+        end
+
+        context 'with a service account token' do
+          let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, project: project, cluster: cluster) }
+
+          it { is_expected.to eq kubernetes_namespace }
+        end
+      end
+    end
+  end
+
   describe '#predefined_variables' do
     subject { cluster.predefined_variables }
 
diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb
index c485850c16e..f4dd457c3d3 100644
--- a/spec/models/clusters/platforms/kubernetes_spec.rb
+++ b/spec/models/clusters/platforms/kubernetes_spec.rb
@@ -223,19 +223,33 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
       let(:namespace) { 'namespace-123' }
 
       it { is_expected.to eq(namespace) }
+
+      context 'kubernetes namespace is present but has no service account token' do
+        let!(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) }
+
+        it { is_expected.to eq(namespace) }
+      end
     end
 
     context 'with no namespace assigned' do
       let(:namespace) { nil }
 
       context 'when kubernetes namespace is present' do
-        let(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, cluster: cluster) }
+        let(:kubernetes_namespace) { create(:cluster_kubernetes_namespace, :with_token, cluster: cluster) }
 
         before do
           kubernetes_namespace
         end
 
         it { is_expected.to eq(kubernetes_namespace.namespace) }
+
+        context 'kubernetes namespace has no service account token' do
+          before do
+            kubernetes_namespace.update!(namespace: 'old-namespace', service_account_token: nil)
+          end
+
+          it { is_expected.to eq("#{project.path}-#{project.id}") }
+        end
       end
 
       context 'when kubernetes namespace is not present' do