diff options
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 |