Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Requests from inside the cluster misuse the API to refresh tokens #1948

Closed
AlexisZam opened this issue Nov 10, 2022 · 1 comment · Fixed by #1949
Closed

Requests from inside the cluster misuse the API to refresh tokens #1948

AlexisZam opened this issue Nov 10, 2022 · 1 comment · Fixed by #1949
Assignees

Comments

@AlexisZam
Copy link
Contributor

AlexisZam commented Nov 10, 2022

Problem statement

The OpenAPITools/openapi-generator/pull/3594 PR added the refresh_api_key_hook() hook to Configuration() objects to refresh expired (or expiring) tokens. This hook is undefined by default. If defined, the get_api_key_with_prefix() method will call it to refresh the token before returning it.

Out-of-cluster

The kubernetes-client/python-base/pull/250 PR defined the __refresh_api_key() function and used it to override the refresh_api_key_hook() hook to refresh tokens from outside the cluster.

This is the proper usage of the API.

In-cluster

The kubernetes-client/python-base/pull/191 and kubernetes-client/python-base/pull/193 PRs defined the load_token_from_file() function and used it to override the get_api_key_with_prefix() method to refresh tokens from inside the cluster.

That is, a well-defined higher-level function is overridden, instead of the undefined lower-level hook, which is designed to be overridden. Put simply, the API is misused.

A side-effect of this is that the token of the client is never updated. I.e., the client always has the (possibly stale) token that was loaded by the last call to load_config() at the time it was created. This works just because the token of the client is not used for the requests to Kubernetes (even though it should be).

Note that by 'client' we refer to an XXXApi object (e.g. CoreV1Api), which we use to make requests to Kubernetes.

Proposed fix

To use the API properly, update the tokens of clients making requests to Kubernetes, and use these tokens for requests, we can mirror the way tokens are refreshed from outside the cluster, i.e.,

  • define a _refresh_api_key function (instead of load_token_from_file), which
  • updates the token of the client (instead of simply returning a token), and
  • override the refresh_api_key_hook hook (instead of get_api_key_with_prefix).

The diff is

diff --git a/kubernetes/base/config/incluster_config.py b/kubernetes/base/config/incluster_config.py
index 5dabd4b7c..86070df43 100644
--- a/kubernetes/base/config/incluster_config.py
+++ b/kubernetes/base/config/incluster_config.py
@@ -92,12 +92,12 @@ class InClusterConfigLoader(object):
         if not self._try_refresh_token:
             return
 
-        def load_token_from_file(*args):
+        def _refresh_api_key(client_configuration):
             if self.token_expires_at <= datetime.datetime.now():
                 self._read_token_file()
-            return self.token
+            self._set_config(client_configuration)
 
-        client_configuration.get_api_key_with_prefix = load_token_from_file
+        client_configuration.refresh_api_key_hook = _refresh_api_key
 
     def _read_token_file(self):
         with open(self._token_filename) as f:
@roycaihw
Copy link
Member

roycaihw commented Dec 5, 2022

cc @yliaog
/assign @AlexisZam

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants