Merge branch 'ee-refactor_gitlab_kube_client' into 'master'

Refactor Gitlab::KubeClient (EE)

See merge request gitlab-org/gitlab-ee!7831

app/models/clusters/platforms/kubernetes.rb

@@ -109,7 +109,7 @@ module Clusters
       end
 
       def kubeclient
-        @kubeclient ||= build_kube_client!(api_groups: ['api', 'apis/rbac.authorization.k8s.io'])
+        @kubeclient ||= build_kube_client!
       end
 
       private
@@ -138,7 +138,7 @@ module Clusters
         Gitlab::NamespaceSanitizer.sanitize(slug)
       end
 
-      def build_kube_client!(api_groups: ['api'], api_version: 'v1')
+      def build_kube_client!
         raise "Incomplete settings" unless api_url && actual_namespace
 
         unless (username && password) || token
@@ -147,8 +147,6 @@ module Clusters
 
         Gitlab::Kubernetes::KubeClient.new(
           api_url,
-          api_groups,
-          api_version,
           auth_options: kubeclient_auth_options,
           ssl_options: kubeclient_ssl_options,
           http_proxy_uri: ENV['http_proxy']

app/models/project_services/kubernetes_service.rb

@@ -146,7 +146,7 @@ class KubernetesService < DeploymentService
   end
 
   def kubeclient
-    @kubeclient ||= build_kube_client!(api_groups: ['api', 'apis/rbac.authorization.k8s.io'])
+    @kubeclient ||= build_kube_client!
   end
 
   def deprecated?
@@ -184,13 +184,11 @@ class KubernetesService < DeploymentService
     slug.gsub(/[^-a-z0-9]/, '-').gsub(/^-+/, '')
   end
 
-  def build_kube_client!(api_groups: ['api'], api_version: 'v1')
+  def build_kube_client!
     raise "Incomplete settings" unless api_url && actual_namespace && token
 
     Gitlab::Kubernetes::KubeClient.new(
       api_url,
-      api_groups,
-      api_version,
       auth_options: kubeclient_auth_options,
       ssl_options: kubeclient_ssl_options,
       http_proxy_uri: ENV['http_proxy']

app/services/clusters/gcp/finalize_creation_service.rb

@@ -60,18 +60,15 @@ module Clusters
           'https://' + gke_cluster.endpoint,
           Base64.decode64(gke_cluster.master_auth.cluster_ca_certificate),
           gke_cluster.master_auth.username,
-          gke_cluster.master_auth.password,
-          api_groups: ['api', 'apis/rbac.authorization.k8s.io']
+          gke_cluster.master_auth.password
         )
       end
 
-      def build_kube_client!(api_url, ca_pem, username, password, api_groups: ['api'], api_version: 'v1')
+      def build_kube_client!(api_url, ca_pem, username, password)
         raise "Incomplete settings" unless api_url && username && password
 
         Gitlab::Kubernetes::KubeClient.new(
           api_url,
-          api_groups,
-          api_version,
           auth_options: { username: username, password: password },
           ssl_options: kubeclient_ssl_options(ca_pem),
           http_proxy_uri: ENV['http_proxy']

ee/app/models/concerns/ee/kubernetes_service.rb

@@ -31,8 +31,6 @@ module EE
     end
 
     def read_deployments
-      kubeclient = build_kube_client!(api_groups: ['apis/extensions'], api_version: 'v1beta1')
-
       kubeclient.get_deployments(namespace: actual_namespace).as_json
     rescue KubeException => err
       raise err unless err.error_code == 404

lib/gitlab/kubernetes/kube_client.rb

@@ -13,11 +13,21 @@ module Gitlab
     class KubeClient
       include Gitlab::Utils::StrongMemoize
 
-      SUPPORTED_API_GROUPS = [
-        'api',
-        'apis/rbac.authorization.k8s.io',
-        'apis/extensions'
-      ].freeze
+      SUPPORTED_API_GROUPS = {
+        core: { group: 'api', version: 'v1' },
+        rbac: { group: 'apis/rbac.authorization.k8s.io', version: 'v1' },
+        extensions: { group: 'apis/extensions', version: 'v1beta1' }
+      }.freeze
+
+      SUPPORTED_API_GROUPS.each do |name, params|
+        client_method_name = "#{name}_client".to_sym
+
+        define_method(client_method_name) do
+          strong_memoize(client_method_name) do
+            build_kubeclient(params[:group], params[:version])
+          end
+        end
+      end
 
       # Core API methods delegates to the core api group client
       delegate :get_pods,
@@ -62,48 +72,21 @@ module Gitlab
         :watch_pod_log,
         to: :core_client
 
-      def initialize(api_prefix, api_groups = ['api'], api_version = 'v1', **kubeclient_options)
-        raise ArgumentError unless check_api_groups_supported?(api_groups)
+      attr_reader :api_prefix, :kubeclient_options
 
+      def initialize(api_prefix, **kubeclient_options)
         @api_prefix = api_prefix
-        @api_groups = api_groups
-        @api_version = api_version
         @kubeclient_options = kubeclient_options
       end
 
-      def discover!
-        clients.each(&:discover)
-      end
-
-      def clients
-        hashed_clients.values
-      end
-
-      def core_client
-        hashed_clients['api']
-      end
-
-      def rbac_client
-        hashed_clients['apis/rbac.authorization.k8s.io']
-      end
-
-      def extensions_client
-        hashed_clients['apis/extensions']
-      end
-
-      def hashed_clients
-        strong_memoize(:hashed_clients) do
-          @api_groups.map do |api_group|
-            api_url = join_api_url(@api_prefix, api_group)
-            [api_group, ::Kubeclient::Client.new(api_url, @api_version, **@kubeclient_options)]
-          end.to_h
-        end
-      end
-
       private
 
-      def check_api_groups_supported?(api_groups)
-        api_groups.all? {|api_group| SUPPORTED_API_GROUPS.include?(api_group) }
+      def build_kubeclient(api_group, api_version)
+        ::Kubeclient::Client.new(
+          join_api_url(api_prefix, api_group),
+          api_version,
+          **kubeclient_options
+        )
       end
 
       def join_api_url(api_prefix, api_path)

spec/lib/gitlab/kubernetes/kube_client_spec.rb

@@ -6,104 +6,63 @@ describe Gitlab::Kubernetes::KubeClient do
   include KubernetesHelpers
 
   let(:api_url) { 'https://kubernetes.example.com/prefix' }
-  let(:api_groups) { ['api', 'apis/rbac.authorization.k8s.io'] }
-  let(:api_version) { 'v1' }
   let(:kubeclient_options) { { auth_options: { bearer_token: 'xyz' } } }
 
-  let(:client) { described_class.new(api_url, api_groups, api_version, kubeclient_options) }
+  let(:client) { described_class.new(api_url, kubeclient_options) }
 
   before do
     stub_kubeclient_discover(api_url)
   end
 
-  describe '#hashed_clients' do
-    subject { client.hashed_clients }
-
-    it 'has keys from api groups' do
-      expect(subject.keys).to match_array api_groups
-    end
-
-    it 'has values of Kubeclient::Client' do
-      expect(subject.values).to all(be_an_instance_of Kubeclient::Client)
-    end
-  end
-
-  describe '#clients' do
-    subject { client.clients }
-
-    it 'is not empty' do
-      is_expected.to be_present
-    end
-
-    it 'is an array of Kubeclient::Client objects' do
-      is_expected.to all(be_an_instance_of Kubeclient::Client)
-    end
-
-    it 'has each API group url' do
-      expected_urls = api_groups.map { |group| "#{api_url}/#{group}" }
-
-      expect(subject.map(&:api_endpoint).map(&:to_s)).to match_array(expected_urls)
+  shared_examples 'a Kubeclient' do
+    it 'is a Kubeclient::Client' do
+      is_expected.to be_an_instance_of Kubeclient::Client
     end
 
     it 'has the kubeclient options' do
-      subject.each do |client|
-        expect(client.auth_options).to eq({ bearer_token: 'xyz' })
-      end
-    end
-
-    it 'has the api_version' do
-      subject.each do |client|
-        expect(client.instance_variable_get(:@api_version)).to eq('v1')
-      end
+      expect(subject.auth_options).to eq({ bearer_token: 'xyz' })
     end
   end
 
   describe '#core_client' do
     subject { client.core_client }
 
-    it 'is a Kubeclient::Client' do
-      is_expected.to be_an_instance_of Kubeclient::Client
-    end
+    it_behaves_like 'a Kubeclient'
 
     it 'has the core API endpoint' do
       expect(subject.api_endpoint.to_s).to match(%r{\/api\Z})
     end
+
+    it 'has the api_version' do
+      expect(subject.instance_variable_get(:@api_version)).to eq('v1')
+    end
   end
 
   describe '#rbac_client' do
     subject { client.rbac_client }
 
-    it 'is a Kubeclient::Client' do
-      is_expected.to be_an_instance_of Kubeclient::Client
-    end
+    it_behaves_like 'a Kubeclient'
 
     it 'has the RBAC API group endpoint' do
       expect(subject.api_endpoint.to_s).to match(%r{\/apis\/rbac.authorization.k8s.io\Z})
     end
+
+    it 'has the api_version' do
+      expect(subject.instance_variable_get(:@api_version)).to eq('v1')
+    end
   end
 
   describe '#extensions_client' do
     subject { client.extensions_client }
 
-    let(:api_groups) { ['apis/extensions'] }
-
-    it 'is a Kubeclient::Client' do
-      is_expected.to be_an_instance_of Kubeclient::Client
-    end
+    it_behaves_like 'a Kubeclient'
 
     it 'has the extensions API group endpoint' do
       expect(subject.api_endpoint.to_s).to match(%r{\/apis\/extensions\Z})
     end
-  end
 
-  describe '#discover!' do
-    it 'makes a discovery request for each API group' do
-      client.discover!
-
-      api_groups.each do |api_group|
-        discovery_url = api_url + '/' + api_group + '/v1'
-        expect(WebMock).to have_requested(:get, discovery_url).once
-      end
+    it 'has the api_version' do
+      expect(subject.instance_variable_get(:@api_version)).to eq('v1beta1')
     end
   end
 
@@ -156,21 +115,12 @@ describe Gitlab::Kubernetes::KubeClient do
         it 'responds to the method' do
           expect(client).to respond_to method
         end
-
-        context 'no rbac client' do
-          let(:api_groups) { ['api'] }
-
-          it 'throws an error' do
-            expect { client.public_send(method) }.to raise_error(Module::DelegationError)
-          end
-        end
       end
     end
   end
 
   describe 'extensions API group' do
     let(:api_groups) { ['apis/extensions'] }
-    let(:api_version) { 'v1beta1' }
     let(:extensions_client) { client.extensions_client }
 
     describe '#get_deployments' do
@@ -181,22 +131,11 @@ describe Gitlab::Kubernetes::KubeClient do
       it 'responds to the method' do
         expect(client).to respond_to :get_deployments
       end
-
-      context 'no extensions client' do
-        let(:api_groups) { ['api'] }
-        let(:api_version) { 'v1' }
-
-        it 'throws an error' do
-          expect { client.get_deployments }.to raise_error(Module::DelegationError)
-        end
-      end
     end
   end
 
   describe 'non-entity methods' do
     it 'does not proxy for non-entity methods' do
-      expect(client.clients.first).to respond_to :proxy_url
-
       expect(client).not_to respond_to :proxy_url
     end
 
@@ -211,14 +150,6 @@ describe Gitlab::Kubernetes::KubeClient do
     it 'is delegated to the core client' do
       expect(client).to delegate_method(:get_pod_log).to(:core_client)
     end
-
-    context 'when no core client' do
-      let(:api_groups) { ['apis/extensions'] }
-
-      it 'throws an error' do
-        expect { client.get_pod_log('pod-name') }.to raise_error(Module::DelegationError)
-      end
-    end
   end
 
   describe '#watch_pod_log' do
@@ -227,14 +158,6 @@ describe Gitlab::Kubernetes::KubeClient do
     it 'is delegated to the core client' do
       expect(client).to delegate_method(:watch_pod_log).to(:core_client)
     end
-
-    context 'when no core client' do
-      let(:api_groups) { ['apis/extensions'] }
-
-      it 'throws an error' do
-        expect { client.watch_pod_log('pod-name') }.to raise_error(Module::DelegationError)
-      end
-    end
   end
 
   describe 'methods that do not exist on any client' do

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

@@ -16,7 +16,6 @@ describe Clusters::Gcp::Kubernetes::CreateServiceAccountService do
     let(:kubeclient) do
       Gitlab::Kubernetes::KubeClient.new(
         api_url,
-        ['api', 'apis/rbac.authorization.k8s.io'],
         auth_options: { username: username, password: password }
       )
     end

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

@@ -11,7 +11,6 @@ describe Clusters::Gcp::Kubernetes::FetchKubernetesTokenService do
     let(:kubeclient) do
       Gitlab::Kubernetes::KubeClient.new(
         api_url,
-        ['api', 'apis/rbac.authorization.k8s.io'],
         auth_options: { username: username, password: password }
       )
     end