for a CAFile config, this reads/freezes the data from the file at TransportConfig() construction time rather than transport construction time... probably not a big deal, but notable... could end up with different content here than is used in the transport itself.

it's also maybe slightly unexpected that TransportConfig() would do file reads...

I could pass in the logic as a closure to delay the computation if you think the extra complexity would be worth it?

I missed some of the discussion around this, so asked a question at https://github.com/kubernetes/enhancements/pull/1711/files#r445956764, echoing here:

I can see use cases for cluster hostname and custom config bits, but I was surprised by the ServerName and CAData bits... are we expecting exec plugins to call out to the server we are asking them to provide credentials for?

If so, don't we need to include ProxyURL and InsecureSkipTLSVerify?

is there a reason not to just use the serialized version of the cluster stanza from the kubeconfig file more or less as-is?

I was surprised by the ServerName and CAData bits... are we expecting exec plugins to call out to the server we are asking them to provide credentials for?

Yes, similar to what oc login does today to determine the location of the OAuth server (i.e. read this well known config map that tells you something dynamic about the cluster).

If so, don't we need to include ProxyURL and InsecureSkipTLSVerify?

The per-cluster ProxyURL was added in v1.19 without me noticing, but yeah, we should include that. I left out InsecureSkipTLSVerify on purpose to discourage its use...

is there a reason not to just use the serialized version of the cluster stanza from the kubeconfig file more or less as-is?

I had that in the original draft of the KEP updates. See kubernetes/enhancements#1711 (comment). @mikedanese leaned towards a new struct, and I liked the idea of just having a single well-known extension key to pass through instead of just dumping everything.

Opened a new PR into the KEP to try to address this design update: kubernetes/enhancements#1927.

I'm not sure it's ok to declare an unqualified name (no kubernetes.io/ prefix) reserved after the fact like this.

would it be better for the exec config to opt into getting cluster info (and maybe even identify the named cluster extension it wants)?

clusters:
- name: my-cluster
  cluster:
    ...
    extensions:
    - name: myexec
      extension:
        audience: 06e3fbd18de8  # arbitrary config
users:
- name: myuser
  user:
    exec:
      ...
      includeClusterConfig: true
      includeClusterExtension: myexec

that would:

1. avoid having a magic extension name
2. avoid clusters only working with a single plugin
3. avoid sending exec plugins data they did not expect/want (CA data specifically could be quite large to include in the env)
4. avoid multiple calls to an exec plugin for different clusters when it didn't care about the cluster and the already-fetched credential could have been used

I would personally lean towards just changing exec to client.authentication.k8s.io/exec to prevent any possible collision instead of expanding ExecConfig (though I suspect that we would be fine using exec).

avoid having a magic extension name

I am not sure this is much different from the exec: stanza under user:.

avoid clusters only working with a single plugin

Somehow I do not see a user having the same cluster with multiple distinct exec plugins configured. I realize it is technically possible, but not exactly practical or common.

avoid sending exec plugins data they did not expect/want (CA data specifically could be quite large to include in the env)

I think we can handle the extra 2 KB via the env var...

avoid multiple calls to an exec plugin for different clusters when it didn't care about the cluster and the already-fetched credential could have been used

Using the same credential across clusters is an incredibly unsafe pattern. If you are using a bearer token, you are at risk of a compromised cluster replaying your credentials. For both tokens and certs, if the credential is stolen from disk, it can be used against multiple clusters. Combine that with no revocation of certs, and you are really asking for trouble. I am okay adding the latency of exec-ing the binary per cluster as the binary will likely need to do some form of caching itself to support clients like kubectl.

Opened a new PR into the KEP to try to address this design update: kubernetes/enhancements#1927.

is there a reason not to just use the serialized version of the cluster stanza from the kubeconfig file more or less as-is?

does this do the right thing printing the runtime.Config object?

Yeah, the new test cases in TestCacheKey should cover that.