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.

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

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

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

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

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,

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,

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'

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

spec/services/clusters/gcp/kubernetes/fetch_kubernetes_token_service_spec.rb

-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

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