Fix bug with InClusterConfig requiring bearer token.

[timothysc]

What this PR does / why we need it:
In cluster configs were failing b/c of changes made by #67359

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #69234

Special notes for your reviewer:
I've verified this PR fixes the issue, and is critical priority.

Release note:

Fix bug caused by #67359 that caused some client-go generated InCluster KubeConfigs to fail.  

/sig testing
/sig auth
/sig apimachinery

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews @dims
/assign @liggitt @mikedanese @smarterclayton

[dims]

@timothysc based on discussion in #69270 @liggitt and @darxkies would like us to try this #69273 instead of trying to go back to initializing the bearertoken during initialization.

[dims]

/hold

[timothysc]

@dims @liggitt @mikedanese - there is still a weird problem now in that function is no longer self-contained, and anyone who was previously using it is now going to have an issue.

[liggitt]

@dims @liggitt @mikedanese - there is still a weird problem now in that function is no longer self-contained, and anyone who was previously using it is now going to have an issue.

the returned config works correctly with transport/client construction... there have always been unserializable fields in the client config struct, so it cannot be assumed to be able to be serialized. extracting a statically read token and persisting it elsewhere is not going to work well as we gradually move to a world in which expiration/rotation occurs.

an alternative approach could be to set the tokenfile attribute on the returned InClusterConfig, and adjust transport construction to set up the refreshing token source at point of construction.

[timothysc]

an alternative approach could be to set the tokenfile attribute on the returned InClusterConfig, and adjust transport construction to set up the refreshing token source at point of construction.

That's a pretty non-intuitive UX from a client library. It's what @dims did to patch the tests, but for general consumers of the library, that are not core k8s developers, it's going to be non-intuitive.

[smarterclayton]

I really don't like this.

[k8s-ci-robot]

@timothysc: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: timothysc
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: liggitt

If they are not already assigned, you can assign the PR to them by writing /assign @liggitt in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• staging/src/k8s.io/client-go/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[smarterclayton]

We broke our ecosystem on a fairly critical token ithout a way to preserve compatibility. That's not what we do.

[smarterclayton]

extracting a statically read token and persisting it elsewhere is not going to work well as we gradually move to a world in which expiration/rotation occurs.

That is an argument that people have to do work in that future, which they already had to do. This seems like we pulled this bandaid prematurely.

[k8s-ci-robot]

@timothysc: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-bazel-build	1e4c010	link	/test pull-kubernetes-bazel-build
pull-kubernetes-integration	1e4c010	link	/test pull-kubernetes-integration
pull-kubernetes-bazel-test	1e4c010	link	/test pull-kubernetes-bazel-test
pull-kubernetes-e2e-gce-100-performance	1e4c010	link	/test pull-kubernetes-e2e-gce-100-performance
pull-kubernetes-kubemark-e2e-gce-big	1e4c010	link	/test pull-kubernetes-kubemark-e2e-gce-big
pull-kubernetes-typecheck	1e4c010	link	/test pull-kubernetes-typecheck
pull-kubernetes-e2e-kops-aws	1e4c010	link	/test pull-kubernetes-e2e-kops-aws
pull-kubernetes-e2e-gce	1e4c010	link	/test pull-kubernetes-e2e-gce
pull-kubernetes-node-e2e	1e4c010	link	/test pull-kubernetes-node-e2e
pull-kubernetes-e2e-gce-device-plugin-gpu	1e4c010	link	/test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-verify	1e4c010	link	/test pull-kubernetes-verify

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[smarterclayton]

I don't know that I would fix it this way - I might say we should fix the token transport to handle this, remove the need for wrap transport, and fix the token.

[liggitt]

I don't know that I would fix it this way - I might say we should fix the token transport to handle this, remove the need for wrap transport, and fix the token.

opened #71713

[smarterclayton]

Follow up with Jordan in #71713 - I think this preserves the desired legacy behavior and is even slightly cleaner.