Use http_max_redirects opt to replace monkeypatch

http_max_redirects was introduced in 4.2.2, so upgrade kubeclient.

The monkey-patch was global so we will have to check that all instances
of Kubeclient::Client are handled.

Spec all methods of KubeClient

This should provide better confidence that we are indeed disallowing
redirection in all cases

6 files changed, 44 insertions, 26 deletions

diff --git a/Gemfile b/Gemfile
index b3eeb3ec0ec..943b0260dcb 100644
--- a/Gemfile
+++ b/Gemfile
@@ -224,7 +224,7 @@ gem 'asana', '~> 0.8.1'
 gem 'ruby-fogbugz', '~> 0.2.1'
 
 # Kubernetes integration
-gem 'kubeclient', '~> 4.0.0'
+gem 'kubeclient', '~> 4.2.2'
 
 # Sanitize user input
 gem 'sanitize', '~> 4.6'
diff --git a/Gemfile.lock b/Gemfile.lock
index 419a6831924..1c8222eb634 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -425,7 +425,7 @@ GEM
     kgio (2.10.0)
     knapsack (1.17.0)
       rake
-    kubeclient (4.0.0)
+    kubeclient (4.2.2)
       http (~> 3.0)
       recursive-open-struct (~> 1.0, >= 1.0.4)
       rest-client (~> 2.0)
@@ -1053,7 +1053,7 @@ DEPENDENCIES
   jwt (~> 2.1.0)
   kaminari (~> 1.0)
   knapsack (~> 1.17)
-  kubeclient (~> 4.0.0)
+  kubeclient (~> 4.2.2)
   letter_opener_web (~> 1.3.0)
   license_finder (~> 5.4)
   licensee (~> 8.9)
diff --git a/changelogs/unreleased/54250-upstream-kubeclient-redirect-patch.yml b/changelogs/unreleased/54250-upstream-kubeclient-redirect-patch.yml
new file mode 100644
index 00000000000..d1bdbccb20a
--- /dev/null
+++ b/changelogs/unreleased/54250-upstream-kubeclient-redirect-patch.yml
@@ -0,0 +1,5 @@
+---
+title: Upgrade kubeclient to 4.2.2 and swap out monkey-patch to disallow redirects
+merge_request: 24284
+author:
+type: other
diff --git a/config/initializers/kubeclient.rb b/config/initializers/kubeclient.rb
deleted file mode 100644
index f8fe1156aaa..00000000000
--- a/config/initializers/kubeclient.rb
+++ /dev/null
@@ -1,22 +0,0 @@
-class Kubeclient::Client
-  # Monkey patch to set `max_redirects: 0`, so that kubeclient
-  # does not follow redirects and expose internal services.
-  # See https://gitlab.com/gitlab-org/gitlab-ce/issues/53158
-  def create_rest_client(path = nil)
-    path ||= @api_endpoint.path
-    options = {
-      ssl_ca_file: @ssl_options[:ca_file],
-      ssl_cert_store: @ssl_options[:cert_store],
-      verify_ssl: @ssl_options[:verify_ssl],
-      ssl_client_cert: @ssl_options[:client_cert],
-      ssl_client_key: @ssl_options[:client_key],
-      proxy: @http_proxy_uri,
-      user: @auth_options[:username],
-      password: @auth_options[:password],
-      open_timeout: @timeouts[:open],
-      read_timeout: @timeouts[:read],
-      max_redirects: 0
-    }
-    RestClient::Resource.new(@api_endpoint.merge(path).to_s, options)
-  end
-end
diff --git a/lib/gitlab/kubernetes/kube_client.rb b/lib/gitlab/kubernetes/kube_client.rb
index fe839940f74..624c2c67551 100644
--- a/lib/gitlab/kubernetes/kube_client.rb
+++ b/lib/gitlab/kubernetes/kube_client.rb
@@ -76,9 +76,12 @@ module Gitlab
 
       attr_reader :api_prefix, :kubeclient_options
 
+      # We disable redirects through 'http_max_redirects: 0',
+      # so that KubeClient does not follow redirects and
+      # expose internal services.
       def initialize(api_prefix, **kubeclient_options)
         @api_prefix = api_prefix
-        @kubeclient_options = kubeclient_options
+        @kubeclient_options = kubeclient_options.merge(http_max_redirects: 0)
       end
 
       def create_or_update_cluster_role_binding(resource)
diff --git a/spec/lib/gitlab/kubernetes/kube_client_spec.rb b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
index 8fc85301304..02364e92149 100644
--- a/spec/lib/gitlab/kubernetes/kube_client_spec.rb
+++ b/spec/lib/gitlab/kubernetes/kube_client_spec.rb
@@ -24,6 +24,32 @@ describe Gitlab::Kubernetes::KubeClient do
     end
   end
 
+  shared_examples 'redirection not allowed' do |method_name|
+    before do
+      redirect_url = 'https://not-under-our-control.example.com/api/v1/pods'
+
+      stub_request(:get, %r{\A#{api_url}/})
+        .to_return(status: 302, headers: { location: redirect_url })
+
+      stub_request(:get, redirect_url)
+        .to_return(status: 200, body: '{}')
+    end
+
+    it 'does not follow redirects' do
+      method_call = -> do
+        case method_name
+        when /\A(get_|delete_)/
+          client.public_send(method_name)
+        when /\A(create_|update_)/
+          client.public_send(method_name, {})
+        else
+          raise "Unknown method name #{method_name}"
+        end
+      end
+      expect { method_call.call }.to raise_error(Kubeclient::HttpError)
+    end
+  end
+
   describe '#core_client' do
     subject { client.core_client }
 
@@ -103,6 +129,8 @@ describe Gitlab::Kubernetes::KubeClient do
       :update_service_account
     ].each do |method|
       describe "##{method}" do
+        include_examples 'redirection not allowed', method
+
         it 'delegates to the core client' do
           expect(client).to delegate_method(method).to(:core_client)
         end
@@ -123,6 +151,8 @@ describe Gitlab::Kubernetes::KubeClient do
       :update_cluster_role_binding
     ].each do |method|
       describe "##{method}" do
+        include_examples 'redirection not allowed', method
+
         it 'delegates to the rbac client' do
           expect(client).to delegate_method(method).to(:rbac_client)
         end
@@ -139,6 +169,8 @@ describe Gitlab::Kubernetes::KubeClient do
     let(:extensions_client) { client.extensions_client }
 
     describe '#get_deployments' do
+      include_examples 'redirection not allowed', 'get_deployments'
+
       it 'delegates to the extensions client' do
         expect(client).to delegate_method(:get_deployments).to(:extensions_client)
       end