Always create `gitlab` service account and service account token regardless of ABAC/RBAC

This also solves the async nature of the automatic creation of default
service tokens for service accounts. It also makes explicit which
service account token we always use.

create cluster role binding only if the provider has legacy_abac
disabled.

10 files changed, 195 insertions, 172 deletions

diff --git a/app/services/clusters/gcp/finalize_creation_service.rb b/app/services/clusters/gcp/finalize_creation_service.rb
index 8170e732d48..3ae0a4a19d0 100644
--- a/app/services/clusters/gcp/finalize_creation_service.rb
+++ b/app/services/clusters/gcp/finalize_creation_service.rb
@@ -8,9 +8,8 @@ module Clusters
       def execute(provider)
         @provider = provider
 
-        create_gitlab_service_account!
-
         configure_provider
+        create_gitlab_service_account!
         configure_kubernetes
 
         cluster.save!
@@ -25,9 +24,7 @@ module Clusters
       private
 
       def create_gitlab_service_account!
-        if create_rbac_cluster?
-          Clusters::Gcp::Kubernetes::CreateServiceAccountService.new(kube_client).execute
-        end
+        Clusters::Gcp::Kubernetes::CreateServiceAccountService.new(kube_client, rbac: create_rbac_cluster?).execute
       end
 
       def configure_provider
@@ -47,9 +44,7 @@ module Clusters
       end
 
       def request_kubernetes_token
-        service_account_name = create_rbac_cluster? ? Clusters::Gcp::Kubernetes::SERVICE_ACCOUNT_NAME : 'default'
-
-        Clusters::Gcp::Kubernetes::FetchKubernetesTokenService.new(kube_client, service_account_name).execute
+        Clusters::Gcp::Kubernetes::FetchKubernetesTokenService.new(kube_client).execute
       end
 
       def authorization_type
diff --git a/app/services/clusters/gcp/kubernetes.rb b/app/services/clusters/gcp/kubernetes.rb
index 74ef68eb58f..21a09891ac4 100644
--- a/app/services/clusters/gcp/kubernetes.rb
+++ b/app/services/clusters/gcp/kubernetes.rb
@@ -4,6 +4,7 @@ module Clusters
   module Gcp
     module Kubernetes
       SERVICE_ACCOUNT_NAME = 'gitlab'
+      SERVICE_ACCOUNT_TOKEN_NAME = 'gitlab-token'
       CLUSTER_ROLE_BINDING_NAME = 'gitlab-admin'
       CLUSTER_ROLE_NAME = 'cluster-admin'
     end
diff --git a/app/services/clusters/gcp/kubernetes/create_service_account_service.rb b/app/services/clusters/gcp/kubernetes/create_service_account_service.rb
index 8d87bd7b5c8..4c43b94d911 100644
--- a/app/services/clusters/gcp/kubernetes/create_service_account_service.rb
+++ b/app/services/clusters/gcp/kubernetes/create_service_account_service.rb
@@ -4,25 +4,32 @@ module Clusters
   module Gcp
     module Kubernetes
       class CreateServiceAccountService
-        attr_reader :kubeclient
+        attr_reader :kubeclient, :rbac
 
-        def initialize(kubeclient)
+        def initialize(kubeclient, rbac:)
           @kubeclient = kubeclient
+          @rbac = rbac
         end
 
         def execute
           kubeclient.create_service_account(service_account_resource)
-          kubeclient.create_cluster_role_binding(cluster_role_binding_resource)
+          kubeclient.create_secret(service_account_token_resource)
+          kubeclient.create_cluster_role_binding(cluster_role_binding_resource) if rbac
         end
 
         private
 
         def service_account_resource
-          Gitlab::Kubernetes::ServiceAccount.new(SERVICE_ACCOUNT_NAME, 'default').generate
+          Gitlab::Kubernetes::ServiceAccount.new(service_account_name, namespace).generate
+        end
+
+        def service_account_token_resource
+          Gitlab::Kubernetes::ServiceAccountToken.new(
+            SERVICE_ACCOUNT_TOKEN_NAME, service_account_name, namespace).generate
         end
 
         def cluster_role_binding_resource
-          subjects = [{ kind: 'ServiceAccount', name: SERVICE_ACCOUNT_NAME, namespace: 'default' }]
+          subjects = [{ kind: 'ServiceAccount', name: service_account_name, namespace: namespace }]
 
           Gitlab::Kubernetes::ClusterRoleBinding.new(
             CLUSTER_ROLE_BINDING_NAME,
@@ -30,6 +37,14 @@ module Clusters
             subjects
           ).generate
         end
+
+        def service_account_name
+          SERVICE_ACCOUNT_NAME
+        end
+
+        def namespace
+          'default'
+        end
       end
     end
   end
diff --git a/app/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service.rb b/app/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service.rb
index c16ce451aaf..877dc1de89b 100644
--- a/app/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service.rb
+++ b/app/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service.rb
@@ -4,37 +4,25 @@ module Clusters
   module Gcp
     module Kubernetes
       class FetchKubernetesTokenService
-        attr_reader :kubeclient, :service_account_name
+        attr_reader :kubeclient
 
-        def initialize(kubeclient, service_account_name)
+        def initialize(kubeclient)
           @kubeclient = kubeclient
-          @service_account_name = service_account_name
         end
 
         def execute
-          read_secrets.each do |secret|
-            name = secret.dig('metadata', 'name')
-            if token_regex =~ name
-              token_base64 = secret.dig('data', 'token')
-              return Base64.decode64(token_base64) if token_base64
-            end
-          end
-
-          nil
+          token_base64 = get_secret&.dig('data', 'token')
+          Base64.decode64(token_base64) if token_base64
         end
 
         private
 
-        def token_regex
-          /#{service_account_name}-token/
-        end
-
-        def read_secrets
-          kubeclient.get_secrets.as_json
+        def get_secret
+          kubeclient.get_secret(SERVICE_ACCOUNT_TOKEN_NAME).as_json
         rescue Kubeclient::HttpError => err
           raise err unless err.error_code == 404
 
-          []
+          nil
         end
       end
     end
diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb
index 2f03d73dfb4..588238de608 100644
--- a/lib/gitlab/kubernetes/kube_client.rb
+++ b/lib/gitlab/kubernetes/kube_client.rb
@@ -25,6 +25,7 @@ module Gitlab
         :get_config_map,
         :get_namespace,
         :get_pod,
+        :get_secret,
         :get_service,
         :get_service_account,
         :delete_pod,
diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
index 7c057e66673..53c5a4e7c94 100644
--- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb
+++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
@@ -116,6 +116,7 @@ describe Gitlab::Kubernetes::KubeClient do
       :get_config_map,
       :get_pod,
       :get_namespace,
+      :get_secret,
       :get_service,
       :get_service_account,
       :delete_pod,
diff --git a/spec/services/clusters/gcp/finalize_creation_service_spec.rb b/spec/services/clusters/gcp/finalize_creation_service_spec.rb
index 1ea41b41771..0f484222228 100644
--- a/spec/services/clusters/gcp/finalize_creation_service_spec.rb
+++ b/spec/services/clusters/gcp/finalize_creation_service_spec.rb
@@ -12,9 +12,11 @@ describe Clusters::Gcp::FinalizeCreationService do
     let(:zone) { provider.zone }
     let(:cluster_name) { cluster.name }
 
+    subject { described_class.new.execute(provider) }
+
     shared_examples 'success' do
       it 'configures provider and kubernetes' do
-        described_class.new.execute(provider)
+        subject
 
         expect(provider).to be_created
       end
@@ -22,7 +24,7 @@ describe Clusters::Gcp::FinalizeCreationService do
 
     shared_examples 'error' do
       it 'sets an error to provider object' do
-        described_class.new.execute(provider)
+        subject
 
         expect(provider.reload).to be_errored
       end
@@ -33,6 +35,7 @@ describe Clusters::Gcp::FinalizeCreationService do
       let(:api_url) { 'https://' + endpoint }
       let(:username) { 'sample-username' }
       let(:password) { 'sample-password' }
+      let(:secret_name) { 'gitlab-token' }
 
       before do
         stub_cloud_platform_get_zone_cluster(
@@ -43,124 +46,98 @@ describe Clusters::Gcp::FinalizeCreationService do
             password: password
           }
         )
-
-        stub_kubeclient_discover(api_url)
       end
 
-      context 'when suceeded to fetch kuberenetes token' do
-        let(:secret_name) { 'default-token-Y1a' }
-        let(:token) { 'sample-token' }
-
+      context 'service account and token created' do
         before do
-          stub_kubeclient_get_secrets(
-            api_url,
-            {
-              metadata_name: secret_name,
-              token: Base64.encode64(token)
-            } )
-        end
-
-        it_behaves_like 'success'
-
-        it 'has corresponded data' do
-          described_class.new.execute(provider)
-          cluster.reload
-          provider.reload
-          platform.reload
-
-          expect(provider.endpoint).to eq(endpoint)
-          expect(platform.api_url).to eq(api_url)
-          expect(platform.ca_cert).to eq(Base64.decode64(load_sample_cert))
-          expect(platform.username).to eq(username)
-          expect(platform.password).to eq(password)
-          expect(platform.authorization_type).to eq('abac')
-          expect(platform.token).to eq(token)
+          stub_kubeclient_discover(api_url)
+          stub_kubeclient_create_service_account(api_url)
+          stub_kubeclient_create_secret(api_url)
         end
 
-        context 'rbac_clusters feature enabled' do
-          let(:secret_name) { 'gitlab-token-Y1a' }
+        shared_context 'kubernetes token successfully fetched' do
+          let(:token) { 'sample-token' }
 
           before do
-            provider.legacy_abac = false
-
-            stub_kubeclient_create_service_account(api_url)
-            stub_kubeclient_create_cluster_role_binding(api_url)
+            stub_kubeclient_get_secret(
+              api_url,
+              {
+                metadata_name: secret_name,
+                token: Base64.encode64(token)
+              } )
           end
+        end
+
+        context 'provider legacy_abac is enabled' do
+          include_context 'kubernetes token successfully fetched'
 
           it_behaves_like 'success'
 
-          it 'has corresponded data' do
-            described_class.new.execute(provider)
+          it 'properly configures database models' do
+            subject
+
             cluster.reload
-            provider.reload
-            platform.reload
 
             expect(provider.endpoint).to eq(endpoint)
             expect(platform.api_url).to eq(api_url)
             expect(platform.ca_cert).to eq(Base64.decode64(load_sample_cert))
             expect(platform.username).to eq(username)
             expect(platform.password).to eq(password)
-            expect(platform.authorization_type).to eq('rbac')
+            expect(platform).to be_abac
+            expect(platform.authorization_type).to eq('abac')
             expect(platform.token).to eq(token)
           end
         end
-      end
-
-      context 'when no matching token is found' do
-        before do
-          stub_kubeclient_get_secrets(api_url, metadata_name: 'not-default-not-gitlab')
-        end
 
-        it_behaves_like 'error'
-
-        context 'rbac_clusters feature enabled' do
+        context 'provider legacy_abac is disabled' do
           before do
             provider.legacy_abac = false
-
-            stub_kubeclient_create_service_account(api_url)
-            stub_kubeclient_create_cluster_role_binding(api_url)
           end
 
-          it_behaves_like 'error'
-        end
-      end
+          include_context 'kubernetes token successfully fetched'
 
-      context 'when token is empty' do
-        let(:secret_name) { 'default-token-123' }
+          context 'cluster role binding created' do
+            before do
+              stub_kubeclient_create_cluster_role_binding(api_url)
+            end
 
-        before do
-          stub_kubeclient_get_secrets(api_url, token: '', metadata_name: secret_name)
-        end
+            it_behaves_like 'success'
 
-        it_behaves_like 'error'
+            it 'properly configures database models' do
+              subject
 
-        context 'rbac_clusters feature enabled' do
-          let(:secret_name) { 'gitlab-token-321' }
+              cluster.reload
 
-          before do
-            provider.legacy_abac = false
+              expect(provider.endpoint).to eq(endpoint)
+              expect(platform.api_url).to eq(api_url)
+              expect(platform.ca_cert).to eq(Base64.decode64(load_sample_cert))
+              expect(platform.username).to eq(username)
+              expect(platform.password).to eq(password)
+              expect(platform).to be_rbac
+              expect(platform.token).to eq(token)
+            end
+          end
+        end
 
-            stub_kubeclient_create_service_account(api_url)
-            stub_kubeclient_create_cluster_role_binding(api_url)
+        context 'when token is empty' do
+          before do
+            stub_kubeclient_get_secret(api_url, token: '', metadata_name: secret_name)
           end
 
           it_behaves_like 'error'
         end
-      end
 
-      context 'when failed to fetch kuberenetes token' do
-        before do
-          stub_kubeclient_get_secrets_error(api_url)
-        end
+        context 'when failed to fetch kubernetes token' do
+          before do
+            stub_kubeclient_get_secret_error(api_url, secret_name)
+          end
 
-        it_behaves_like 'error'
+          it_behaves_like 'error'
+        end
 
-        context 'rbac_clusters feature enabled' do
+        context 'when service account fails to create' do
           before do
-            provider.legacy_abac = false
-
-            stub_kubeclient_create_service_account(api_url)
-            stub_kubeclient_create_cluster_role_binding(api_url)
+            stub_kubeclient_create_service_account_error(api_url)
           end
 
           it_behaves_like 'error'
diff --git a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb b/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb
index 2dd4eedaf56..5268ae8a6d7 100644
--- a/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb
+++ b/spec/services/clusters/gcp/kubernetes/create_service_account_service_spec.rb
@@ -5,11 +5,12 @@ require 'spec_helper'
 describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do
   include KubernetesHelpers
 
-  let(:service) { described_class.new(kubeclient) }
+  let(:service) { described_class.new(kubeclient, rbac: rbac) }
 
   describe '#execute' do
     subject { service.execute }
 
+    let(:rbac) { false }
     let(:api_url) { 'http://111.111.111.111' }
     let(:username) { 'admin' }
     let(:password) { 'xxx' }
@@ -25,29 +26,69 @@ describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do
       before do
         stub_kubeclient_discover(api_url)
         stub_kubeclient_create_service_account(api_url)
-        stub_kubeclient_create_cluster_role_binding(api_url)
+        stub_kubeclient_create_secret(api_url)
       end
 
-      it 'creates a kubernetes service account' do
-        subject
+      shared_examples 'creates service account and token' do
+        it 'creates a kubernetes service account' do
+          subject
 
-        expect(WebMock).to have_requested(:post, api_url + '/api/v1/namespaces/default/serviceaccounts').with(
-          body: hash_including(
-            metadata: { name: 'gitlab', namespace: 'default' }
+          expect(WebMock).to have_requested(:post, api_url + '/api/v1/namespaces/default/serviceaccounts').with(
+            body: hash_including(
+              kind: 'ServiceAccount',
+              metadata: { name: 'gitlab', namespace: 'default' }
+            )
           )
-        )
+        end
+
+        it 'creates a kubernetes secret of type ServiceAccountToken' do
+          subject
+
+          expect(WebMock).to have_requested(:post, api_url + '/api/v1/namespaces/default/secrets').with(
+            body: hash_including(
+              kind: 'Secret',
+              metadata: {
+                name: 'gitlab-token',
+                namespace: 'default',
+                annotations: {
+                  'kubernetes.io/service-account.name': 'gitlab'
+                }
+              },
+              type: 'kubernetes.io/service-account-token'
+            )
+          )
+        end
+      end
+
+      context 'abac enabled cluster' do
+        it_behaves_like 'creates service account and token'
       end
 
-      it 'creates a kubernetes cluster role binding' do
-        subject
+      context 'rbac enabled cluster' do
+        let(:rbac) { true }
+
+        before do
+          stub_kubeclient_create_cluster_role_binding(api_url)
+        end
+
+        it_behaves_like 'creates service account and token'
+
+        it 'creates a kubernetes cluster role binding' do
+          subject
 
-        expect(WebMock).to have_requested(:post, api_url + '/apis/rbac.authorization.k8s.io/v1/clusterrolebindings').with(
-          body: hash_including(
-            metadata: { name: 'gitlab-admin' },
-            roleRef: { apiGroup: 'rbac.authorization.k8s.io', kind: 'ClusterRole', name: 'cluster-admin' },
-            subjects: [{ kind: 'ServiceAccount', namespace: 'default', name: 'gitlab' }]
+          expect(WebMock).to have_requested(:post, api_url + '/apis/rbac.authorization.k8s.io/v1/clusterrolebindings').with(
+            body: hash_including(
+              kind: 'ClusterRoleBinding',
+              metadata: { name: 'gitlab-admin' },
+              roleRef: {
+                apiGroup: 'rbac.authorization.k8s.io',
+                kind: 'ClusterRole',
+                name: 'cluster-admin'
+              },
+              subjects: [{ kind: 'ServiceAccount', namespace: 'default', name: 'gitlab' }]
+            )
           )
-        )
+        end
       end
     end
   end
diff --git a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb
index 74d58a6d206..4c34f21c1bc 100644
--- a/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb
+++ b/spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb
@@ -1,10 +1,9 @@
-require 'spec_helper'
+require 'fast_spec_helper'
 
 describe Clusters::Gcp::Kubernetes::FetchKubernetesTokenService do
   describe '#execute' do
-    subject { described_class.new(kubeclient, service_account_name).execute }
+    subject { described_class.new(kubeclient).execute }
 
-    let(:service_account_name) { 'gitlab-sa' }
     let(:api_url) { 'http://111.111.111.111' }
     let(:username) { 'admin' }
     let(:password) { 'xxx' }
@@ -18,42 +17,39 @@ describe Clusters::Gcp::Kubernetes::FetchKubernetesTokenService do
     end
 
     context 'when params correct' do
-      let(:token) { 'xxx.token.xxx' }
-
-      let(:secrets_json) do
-        [
-          {
-            'metadata': {
-              name: 'default-token-123'
-            },
-            'data': {
-              'token': Base64.encode64('yyy.token.yyy')
-            }
+      let(:decoded_token) { 'xxx.token.xxx' }
+      let(:token) { Base64.encode64(decoded_token) }
+
+      let(:secret_json) do
+        {
+          'metadata': {
+            name: 'gitlab-token'
           },
-          {
-            'metadata': {
-              name: metadata_name
-            },
-            'data': {
-              'token': Base64.encode64(token)
-            }
+          'data': {
+            'token': token
           }
-        ]
+        }
       end
 
       before do
         allow_any_instance_of(Kubeclient::Client)
-          .to receive(:get_secrets).and_return(secrets_json)
+          .to receive(:get_secret).and_return(secret_json)
       end
 
-      context 'when token for service account exists' do
-        let(:metadata_name) { 'gitlab-sa-token-123' }
+      context 'when gitlab-token exists' do
+        let(:metadata_name) { 'gitlab-token' }
 
-        it { is_expected.to eq(token) }
+        it { is_expected.to eq(decoded_token) }
       end
 
       context 'when gitlab-token does not exist' do
-        let(:metadata_name) { 'another-token-123' }
+        let(:secret_json) { {} }
+
+        it { is_expected.to be_nil }
+      end
+
+      context 'when token is nil' do
+        let(:token) { nil }
 
         it { is_expected.to be_nil }
       end
diff --git a/spec/support/helpers/kubernetes_helpers.rb b/spec/support/helpers/kubernetes_helpers.rb
index 30af1e7928c..2fde5c8fde4 100644
--- a/spec/support/helpers/kubernetes_helpers.rb
+++ b/spec/support/helpers/kubernetes_helpers.rb
@@ -33,13 +33,15 @@ module KubernetesHelpers
     WebMock.stub_request(:get, deployments_url).to_return(response || kube_deployments_response)
   end
 
-  def stub_kubeclient_get_secrets(api_url, **options)
-    WebMock.stub_request(:get, api_url + '/api/v1/secrets')
-      .to_return(kube_response(kube_v1_secrets_body(options)))
+  def stub_kubeclient_get_secret(api_url, **options)
+    options[:metadata_name] ||= "default-token-1"
+
+    WebMock.stub_request(:get, api_url + "/api/v1/secrets/#{options[:metadata_name]}")
+      .to_return(kube_response(kube_v1_secret_body(options)))
   end
 
-  def stub_kubeclient_get_secrets_error(api_url)
-    WebMock.stub_request(:get, api_url + '/api/v1/secrets')
+  def stub_kubeclient_get_secret_error(api_url, name)
+    WebMock.stub_request(:get, api_url + "/api/v1/secrets/#{name}")
       .to_return(status: [404, "Internal Server Error"])
   end
 
@@ -48,26 +50,32 @@ module KubernetesHelpers
       .to_return(kube_response({}))
   end
 
+  def stub_kubeclient_create_service_account_error(api_url, namespace: 'default')
+    WebMock.stub_request(:post, api_url + "/api/v1/namespaces/#{namespace}/serviceaccounts")
+      .to_return(status: [500, "Internal Server Error"])
+  end
+
+  def stub_kubeclient_create_secret(api_url, namespace: 'default')
+    WebMock.stub_request(:post, api_url + "/api/v1/namespaces/#{namespace}/secrets")
+      .to_return(kube_response({}))
+  end
+
   def stub_kubeclient_create_cluster_role_binding(api_url)
     WebMock.stub_request(:post, api_url + '/apis/rbac.authorization.k8s.io/v1/clusterrolebindings')
       .to_return(kube_response({}))
   end
 
-  def kube_v1_secrets_body(**options)
+  def kube_v1_secret_body(**options)
     {
       "kind" => "SecretList",
       "apiVersion": "v1",
-      "items" => [
-        {
-          "metadata": {
-            "name": options[:metadata_name] || "default-token-1",
-            "namespace": "kube-system"
-          },
-          "data": {
-            "token": options[:token] || Base64.encode64('token-sample-123')
-          }
-        }
-      ]
+      "metadata": {
+        "name": options[:metadata_name] || "default-token-1",
+        "namespace": "kube-system"
+      },
+      "data": {
+        "token": options[:token] || Base64.encode64('token-sample-123')
+      }
     }
   end