From 1312f3ae53332af8a202b354db03e9917015dc3a Mon Sep 17 00:00:00 2001 From: Hartley McGuire Date: Tue, 28 Feb 2023 17:18:56 -0500 Subject: [PATCH] Enable refreshing of auth_provider bearer_token Previously, once a Kubeclient::Context was created, auth_provider bearer_tokens would be set and never refreshed. This commit enables automatic refreshing of auth_provider bearer_tokens by returning callables instead of static tokens. When used with the faraday client, the callable is automatically called for each request, and for the non-faraday client the callable is called before setting the header manually. Tests have been updated to cover this functionality for both client types, and an additional test was added for refreshing bearer_token_file because it did not appear to be covered before. Co-authored-by: Jonathan Gnagy Co-authored-by: Hartley McGuire --- lib/kubeclient/config.rb | 4 +-- lib/kubeclient/watch_stream.rb | 27 +++++++++------- test/test_config.rb | 58 ++++++++++++++++++++++++---------- test/test_kubeclient.rb | 32 +++++++++++++++++++ test/test_watch.rb | 36 ++++++++++++++++++--- 5 files changed, 123 insertions(+), 34 deletions(-) diff --git a/lib/kubeclient/config.rb b/lib/kubeclient/config.rb index f9aa79ae..de3873c0 100644 --- a/lib/kubeclient/config.rb +++ b/lib/kubeclient/config.rb @@ -199,9 +199,9 @@ def fetch_token_from_provider(auth_provider) case auth_provider['name'] when 'gcp' config = expand_command_option(auth_provider['config'], 'cmd-path') - Kubeclient::GCPAuthProvider.token(config) + -> { Kubeclient::GCPAuthProvider.token(config) } when 'oidc' - Kubeclient::OIDCAuthProvider.token(auth_provider['config']) + -> { Kubeclient::OIDCAuthProvider.token(auth_provider['config']) } end end diff --git a/lib/kubeclient/watch_stream.rb b/lib/kubeclient/watch_stream.rb index 738d11c6..3602464f 100644 --- a/lib/kubeclient/watch_stream.rb +++ b/lib/kubeclient/watch_stream.rb @@ -65,22 +65,25 @@ def build_client ) end - bearer_token = nil - if @http_options[:bearer_token_file] - bearer_token_file = @http_options[:bearer_token_file] + if (bearer_token = extract_bearer_token) + client = client.auth("Bearer #{bearer_token}") + end + + client + end + + def extract_bearer_token + if (bearer_token_file = @http_options[:bearer_token_file]) if File.file?(bearer_token_file) && File.readable?(bearer_token_file) - token = File.read(bearer_token_file).chomp - bearer_token = "Bearer #{token}" + File.read(bearer_token_file).chomp end elsif @http_options[:bearer_token] - bearer_token = "Bearer #{@http_options[:bearer_token]}" - end - - if bearer_token - client = client.auth(bearer_token) + if @http_options[:bearer_token].respond_to?(:call) + @http_options[:bearer_token].call + else + @http_options[:bearer_token] + end end - - client end def using_proxy diff --git a/test/test_config.rb b/test/test_config.rb index da9e7333..038d6a85 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -177,26 +177,17 @@ def test_user_exec_nopath end def test_gcp_default_auth - Kubeclient::GoogleApplicationDefaultCredentials.expects(:token).returns('token1').once - parsed = load_yaml(config_file('gcpauth.kubeconfig')) - config = Kubeclient::Config.new(parsed, nil) - config.context(config.contexts.first) - end - - # Each call to .context() obtains a new token, calling .auth_options doesn't change anything. - # NOTE: this is not a guarantee, may change, just testing current behavior. - def test_gcp_default_auth_renew Kubeclient::GoogleApplicationDefaultCredentials.expects(:token).returns('token1').once parsed = load_yaml(config_file('gcpauth.kubeconfig')) config = Kubeclient::Config.new(parsed, nil) context = config.context(config.contexts.first) - assert_equal({ bearer_token: 'token1' }, context.auth_options) - assert_equal({ bearer_token: 'token1' }, context.auth_options) + + assert_respond_to(context.auth_options[:bearer_token], :call) + assert_equal('token1', context.auth_options[:bearer_token].call) Kubeclient::GoogleApplicationDefaultCredentials.expects(:token).returns('token2').once - context2 = config.context(config.contexts.first) - assert_equal({ bearer_token: 'token2' }, context2.auth_options) - assert_equal({ bearer_token: 'token1' }, context.auth_options) + + assert_equal('token2', context.auth_options[:bearer_token].call) end def test_gcp_command_auth @@ -213,7 +204,25 @@ def test_gcp_command_auth .returns('token1') .once config = Kubeclient::Config.read(config_file('gcpcmdauth.kubeconfig')) - config.context(config.contexts.first) + context = config.context(config.contexts.first) + + assert_respond_to(context.auth_options[:bearer_token], :call) + assert_equal('token1', context.auth_options[:bearer_token].call) + + Kubeclient::GCPCommandCredentials + .expects(:token) + .with( + 'access-token' => '', + 'cmd-args' => 'config config-helper --format=json', + 'cmd-path' => '/path/to/gcloud', + 'expiry' => '2019-04-09 19:26:18 UTC', + 'expiry-key' => '{.credential.token_expiry}', + 'token-key' => '{.credential.access_token}' + ) + .returns('token2') + .once + + assert_equal('token2', context.auth_options[:bearer_token].call) end def test_oidc_auth_provider @@ -230,7 +239,24 @@ def test_oidc_auth_provider .once parsed = YAML.safe_load(File.read(config_file('oidcauth.kubeconfig'))) config = Kubeclient::Config.new(parsed, nil) - config.context(config.contexts.first) + context = config.context(config.contexts.first) + + assert_respond_to(context.auth_options[:bearer_token], :call) + assert_equal('token1', context.auth_options[:bearer_token].call) + + Kubeclient::OIDCAuthProvider + .expects(:token) + .with( + 'client-id' => 'fake-client-id', + 'client-secret' => 'fake-client-secret', + 'id-token' => 'fake-id-token', + 'idp-issuer-url' => 'https://accounts.google.com', + 'refresh-token' => 'fake-refresh-token' + ) + .returns('token2') + .once + + assert_equal('token2', context.auth_options[:bearer_token].call) end def test_impersonate diff --git a/test/test_kubeclient.rb b/test/test_kubeclient.rb index 6d41a682..255dcea7 100644 --- a/test/test_kubeclient.rb +++ b/test/test_kubeclient.rb @@ -841,6 +841,38 @@ def test_api_bearer_token_file_success assert_equal(1, pods.size) end + def test_api_bearer_token_file_refreshes + stub_core_api_list + + file = File.join(File.dirname(__FILE__), 'valid_token_file') + client = Kubeclient::Client.new( + 'http://localhost:8080/api/', 'v1', + auth_options: { bearer_token_file: file } + ) + + first_request = stub_request(:get, 'http://localhost:8080/api/v1/pods') + .with(headers: { Authorization: 'Bearer valid_token' }) + .to_return(body: '{}', status: 200) + + client.get_pods + + assert_requested(first_request) + + WebMock.reset! + + File.open(file, 'w') { |f| f.puts('another_valid_token') } + + second_request = stub_request(:get, 'http://localhost:8080/api/v1/pods') + .with(headers: { Authorization: 'Bearer another_valid_token' }) + .to_return(body: '{}', status: 200) + + client.get_pods + + assert_requested(second_request) + ensure + File.open(file, 'w') { |f| f.puts('valid_token') } + end + def test_impersonate pods_stub = stub_request(:get, 'http://localhost:8080/api/v1/pods') .with( diff --git a/test/test_watch.rb b/test/test_watch.rb index a2d1a36e..8e36d5c6 100644 --- a/test/test_watch.rb +++ b/test/test_watch.rb @@ -142,7 +142,6 @@ def test_watch_pod_api_bearer_token_file_success def test_watch_pod_api_bearer_token_success stub_core_api_list - file = Tempfile.new('token') client = Kubeclient::Client.new( 'http://localhost:8080/api/', 'v1', auth_options: { bearer_token: 'valid_token' } @@ -155,9 +154,38 @@ def test_watch_pod_api_bearer_token_success got = nil client.watch_pods(as: :raw).each { |notice| got = notice } assert_match(/\A{"type":"DELETED"/, got) - ensure - file.close - file.unlink # deletes the temp file + end + + def test_watch_pod_api_callable_bearer_token + stub_core_api_list + + token = 'valid_token' + + client = Kubeclient::Client.new( + 'http://localhost:8080/api/', 'v1', + auth_options: { bearer_token: -> { token } } + ) + + watcher = client.watch_pods(as: :raw) + + stub_token = stub_request(:get, %r{/watch/pods}) + .with(headers: { Authorization: 'Bearer valid_token' }) + .to_return(body: open_test_file('watch_stream.json'), status: 200) + + got = nil + watcher.each { |notice| got = notice } + assert_match(/\A{"type":"DELETED"/, got) + remove_request_stub(stub_token) + + stub_request(:get, %r{/watch/pods}) + .with(headers: { Authorization: 'Bearer rotated_token' }) + .to_return(body: open_test_file('watch_stream.json'), status: 200) + + token = 'rotated_token' + + got = nil + watcher.each { |notice| got = notice } + assert_match(/\A{"type":"DELETED"/, got) end # Ensure that WatchStream respects a format that's not JSON