Make KUBECONFIG nil if KUBE_TOKEN is nil

Having an invalid KUBECONFIG without a token in it is not helpful. This
only became possible recently now that we are creating a separate
namespace and service account (and hence token) to send to the runners.
This led to somewhat surprising results when troubleshooting
https://gitlab.com/gitlab-org/gitlab-ce/issues/53879 as I found that the
KUBECONFIG was still being passed but KUBE_TOKEN was not. These things
really should have been linked.

Furthermore now that we are also using the [presence of KUBECONFIG to
decide whether or not to run build steps in Auto
DevOps](https://gitlab.com/gitlab-org/gitlab-ce/blob/294d15be3e9497e7b67e1f9131ce9d5c0d68406c/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml#L164)
I think it makes even more sense to ensure that KUBECONFIG is a complete
config if passed to a job.

app/models/clusters/kubernetes_namespace.rb

@@ -33,14 +33,12 @@ module Clusters
     end
 
     def predefined_variables
-      config = YAML.dump(kubeconfig)
-
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
         variables
           .append(key: 'KUBE_SERVICE_ACCOUNT', value: service_account_name.to_s)
           .append(key: 'KUBE_NAMESPACE', value: namespace.to_s)
           .append(key: 'KUBE_TOKEN', value: service_account_token.to_s, public: false)
-          .append(key: 'KUBECONFIG', value: config, public: false, file: true)
+          .append(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true)
       end
     end
 

app/models/clusters/platforms/kubernetes.rb

@@ -90,13 +90,11 @@ module Clusters
             # Clusters::KubernetesNamespace, so once migration has been completed,
             # this 'else' branch will be removed. For more information, please see
             # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22433
-            config = YAML.dump(kubeconfig)
-
             variables
               .append(key: 'KUBE_URL', value: api_url)
               .append(key: 'KUBE_TOKEN', value: token, public: false)
               .append(key: 'KUBE_NAMESPACE', value: actual_namespace)
-              .append(key: 'KUBECONFIG', value: config, public: false, file: true)
+              .append(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true)
           end
         end
       end

app/models/project_services/kubernetes_service.rb

@@ -110,14 +110,12 @@ class KubernetesService < DeploymentService
   # Clusters::Platforms::Kubernetes, it won't be used on this method
   # as it's only needed for Clusters::Cluster.
   def predefined_variables(project:)
-    config = YAML.dump(kubeconfig)
-
     Gitlab::Ci::Variables::Collection.new.tap do |variables|
       variables
         .append(key: 'KUBE_URL', value: api_url)
         .append(key: 'KUBE_TOKEN', value: token, public: false)
         .append(key: 'KUBE_NAMESPACE', value: actual_namespace)
-        .append(key: 'KUBECONFIG', value: config, public: false, file: true)
+        .append(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true)
 
       if ca_pem.present?
         variables

changelogs/unreleased/set-kubeconfig-nil-when-token-nil.yml

+---
+title: Make KUBECONFIG nil if KUBE_TOKEN is nil
+merge_request: 23414
+author:
+type: fixed

lib/gitlab/kubernetes.rb

@@ -85,6 +85,8 @@ module Gitlab
     end
 
     def to_kubeconfig(url:, namespace:, token:, ca_pem: nil)
+      return unless token.present?
+
       config = {
         apiVersion: 'v1',
         clusters: [
@@ -113,7 +115,7 @@ module Gitlab
 
       kubeconfig_embed_ca_pem(config, ca_pem) if ca_pem
 
-      config.deep_stringify_keys
+      YAML.dump(config.deep_stringify_keys)
     end
 
     private

spec/lib/gitlab/kubernetes_spec.rb

@@ -48,26 +48,30 @@ describe Gitlab::Kubernetes do
   end
 
   describe '#to_kubeconfig' do
+    let(:token) { 'TOKEN' }
+    let(:ca_pem) { 'PEM' }
+
     subject do
       to_kubeconfig(
         url: 'https://kube.domain.com',
         namespace: 'NAMESPACE',
-        token: 'TOKEN',
-        ca_pem: ca_pem)
+        token: token,
+        ca_pem: ca_pem
+      )
     end
 
-    context 'when CA PEM is provided' do
-      let(:ca_pem) { 'PEM' }
-      let(:path) { expand_fixture_path('config/kubeconfig.yml') }
-
-      it { is_expected.to eq(YAML.load_file(path)) }
-    end
+    it { expect(YAML.safe_load(subject)).to eq(YAML.load_file(expand_fixture_path('config/kubeconfig.yml'))) }
 
     context 'when CA PEM is not provided' do
       let(:ca_pem) { nil }
-      let(:path) { expand_fixture_path('config/kubeconfig-without-ca.yml') }
 
-      it { is_expected.to eq(YAML.load_file(path)) }
+      it { expect(YAML.safe_load(subject)).to eq(YAML.load_file(expand_fixture_path('config/kubeconfig-without-ca.yml'))) }
+    end
+
+    context 'when token is not provided' do
+      let(:token) { nil }
+
+      it { is_expected.to be_nil }
     end
   end