config: write api_key['BearerToken'] so v36+ SDK auth works#2585
Conversation
k8s-ci-robot
added
do-not-merge/invalid-commit-message
|
|
k8s-ci-robot
added
the
needs-kind
|
Welcome @Jmacek! |
k8s-ci-robot
added
size/M
Jmacek
force-pushed
the
fix-incluster-auth-v36-bearertoken-key
branch
from
f651938 to
322667b
Compare
k8s-ci-robot
added
release-note
|
/kind bug |
k8s-ci-robot
added
kind/bug
v36 rewrote Configuration.auth_settings() to look up the bearer-token credential under api_key['BearerToken'], matching the OpenAPI security scheme name. The in-cluster and kubeconfig loaders were not updated - they still write api_key['authorization'], the v35 lookup key. The same gap exists in kubernetes_asyncio/config/incluster_config.py. As a result, on v36 every call to load_incluster_config() (and load_kube_config() with a static token, including the async equivalents) produces a Configuration whose auth_settings() yields no bearer credential, so outgoing API requests are sent without an Authorization header and the apiserver treats them as system:anonymous. Write the token under both 'authorization' (v35) and 'BearerToken' (v36+) in all three affected loaders so requests carry the expected header. The old key is preserved as a backward-compatibility hedge for any third-party code introspecting api_key['authorization'] directly. Add a regression test in incluster_config_test that drives a real ApiClient.update_params_for_auth() against a freshly-loaded Configuration and asserts the resulting headers contain an Authorization entry - the end-to-end invariant that v36 quietly broke.
Jmacek
force-pushed
the
fix-incluster-auth-v36-bearertoken-key
branch
from
322667b to
c3608a4
Compare
| client_configuration.ssl_ca_cert = self.ssl_ca_cert | ||
| if self.token is not None: | ||
| client_configuration.api_key['authorization'] = self.token | ||
| client_configuration.api_key['BearerToken'] = self.token |
There was a problem hiding this comment.
Does it make sense to have 2 configurations with that same token?
Either one or the other would be fine. It has to be consistent everywhere.
Do we know why that BearerToken was introduced? What was the intention?
There was a problem hiding this comment.
I think this is related https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#breaking-change-from-upgrading-openapi-generator-to-v660. It was introduced in upstream client generator. Same question here. I think upstream did a replacement, rather than keeping both. cc @yliaog
There was a problem hiding this comment.
Thanks both - the original intent of writing both keys was to hedge backward compatibility for any third-party code introspecting api_key['authorization'] directly. But since the rename was already documented as a breaking change in the v6.6.0 generator upgrade, aligning the loaders with that decision is the right call. Switching to a straight replacement.
Address review feedback: the openapi-generator v6.6.0 upgrade declared this rename as a breaking change in the project CHANGELOG, so the loaders should follow the rename rather than preserve the v35 key for backward compatibility. Removes the api_key['authorization'] write in all three affected loaders (sync incluster, sync kube_config, async incluster) and updates the corresponding test assertions to read 'BearerToken'.
|
thanks for the fix /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jmacek, yliaog The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
5 participants
What this PR does
Updates the in-cluster and kubeconfig loaders in
kubernetes/base/config/, pluskubernetes_asyncio/config/incluster_config.py, to write the bearer token underapi_key['BearerToken'](the v36+ lookup key) instead ofapi_key['authorization'](the v35 lookup key). Updates the corresponding test assertions to read the new key. Adds a regression test that exercises the end-to-end flow (Configuration → ApiClient → outgoing Authorization header) which the existing tests did not cover.Why
v36 rewrote
Configuration.auth_settings()to look up the bearer-token credential underapi_key['BearerToken'], matching the OpenAPI security scheme name. This rename was declared a breaking change in the project CHANGELOG as part of the openapi-generator v6.6.0 upgrade. The hand-written loaders inkubernetes/base/config/andkubernetes_asyncio/config/incluster_config.pywere not updated to follow the rename - they still writeapi_key['authorization']. (Asyncio'skube_config.pyalready writes'BearerToken', so it isn't affected.)Net effect on v36: every call to
load_incluster_config()(andload_kube_config()with a static token, including the async equivalents) produces aConfigurationwhoseauth_settings()yields no bearer credential, so outgoing API requests are sent without anAuthorizationheader and the apiserver treats them assystem:anonymous. The failure mode is silent - no warning, no exception, just 401s from every API call.See #2582 for the user-side report and a minimal repro.
Test plan
test_load_incluster_sets_request_authorization_headertokubernetes/base/config/incluster_config_test.py. It drives a realApiClient.update_params_for_auth()against a freshly-loadedConfigurationand asserts the resulting headers contain anAuthorizationentry - the end-to-end invariant that v36 broke.kubernetes==36.0.0against the unfixed loader (AssertionError: 'authorization' not found in {}) and passes with the fix.kubernetes/base/config/and 72 async tests inkubernetes_asyncio/config/pass on v36 with the change applied.Fixes #2582