Repository: kubernetes
Issue: Unclear concurrency safety requirements of `k8s.io/cli-runtime/pkg/resource.NewBuilder` argument
Labels: kind/bug,sig/cli,needs-triage

### What happened?

[NewBuilder](https://github.com/kubernetes/kubernetes/blob/bd9444c1cfd438690dc21c31404dd79ab60dc6cb/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go#L207) takes a [RESTClientGetter](https://github.com/kubernetes/kubernetes/blob/bd9444c1cfd438690dc21c31404dd79ab60dc6cb/staging/src/k8s.io/cli-runtime/pkg/resource/interfaces.go#L27) interface as argument. Neither has **any** docstring.

The latter has a `ToRESTConfig` method which returns a pointer to a `rest.Config`. It turns out that the implementation of `Builder` (specifically the [methods of `ClientConfigFunc`](https://github.com/kubernetes/kubernetes/blob/bd9444c1cfd438690dc21c31404dd79ab60dc6cb/staging/src/k8s.io/cli-runtime/pkg/resource/client.go#L26-L69)) assume ownership of the `rest.Config` returned by this method: it gets mutated.

This is fine with the "stock" implementation of `RESTClientGetter`, namely [`ConfigFlags`](https://github.com/kubernetes/kubernetes/blob/bd9444c1cfd438690dc21c31404dd79ab60dc6cb/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go#L74) whose `ToRESTConfig` implementation is [careful enough to return a fresh copy](https://github.com/kubernetes/cli-runtime/blob/ba42b4014925e8e17241197c8b3a1ba2e0795c97/pkg/genericclioptions/config_flags.go#L130) of `rest.Config` on each call.

However it so happened, that at least one user of this library (the `helm-operator-plugins` framework) provided an implementation of `RESTClientGetter` that simply returned a pointer to a shared `rest.Config` object. As things are with shared mutable state, a data race [bug](https://github.com/operator-framework/helm-operator-plugins/issues/189) in some scenarios ensued, which was [quite tricky to track down](https://github.com/porridge/config-race-repro) though [easy to fix](https://github.com/operator-framework/helm-operator-plugins/pull/190) for that particular use case.

### What did you expect to happen?

It is not unreasonable to assume that other users of `cli-runtime` might also stumble upon this deficiency.

For the sake of future users, the concurrency safety requirements of `NewBuilder` should be clearly documented. Arguably the definition of the `RESTClientGetter` interfaces (I found at least a couple in the codebase) should also have at least a mention of this.

For the current users (who are extremely unlikely to see a change in a docstring of a method they already use) it might make sense to consider something more:
- proactively make a copy of the returned `rest.Config` inside [all those methods](https://github.com/kubernetes/cli-runtime/blob/9139cfdcab39ff4b6b1c6a42af95855c2b5e57ed/pkg/resource/client.go#L26-L69) of `ClientConfigFunc` which call the receiver. However since these methods [are chain-called](https://github.com/kubernetes/cli-runtime/blob/9139cfdcab39ff4b6b1c6a42af95855c2b5e57ed/pkg/resource/builder.go#L954-L956) this might be tricky to do without needlessly double-copying the config.
- (wild idea) calling `ToRestConfig` a couple of times in `NewBuilder` and comparing the return values to make sure they are different objects, and panicking if they are not

### How can we reproduce it (as minimally and precisely as possible)?

https://github.com/porridge/config-race-repro

### Anything else we need to know?

_No response_

### Kubernetes version

<details>

```console
k8s.io/cli-runtime@v0.23.1
```

</details>


### Cloud provider

<details>
Irrelavant
</details>


### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>