client-go's InClusterConfig does not implement documented API discovery heuristics

[olix0r]

What happened?

client-go uses KUBERNETES_SERVICE_HOST to discover the API service address:

kubernetes/staging/src/k8s.io/client-go/rest/config.go

Lines 507 to 541 in 67bde9a

	// InClusterConfig returns a config object which uses the service account
	// kubernetes gives to pods. It's intended for clients that expect to be
	// running inside a pod running on kubernetes. It will return ErrNotInCluster
	// if called from a process not running in a kubernetes environment.
	func InClusterConfig() (*Config, error) {
	const (
	tokenFile = "/var/run/secrets/kubernetes.io/serviceaccount/token"
	rootCAFile = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
	)
	host, port := os.Getenv("KUBERNETES_SERVICE_HOST"), os.Getenv("KUBERNETES_SERVICE_PORT")
	if len(host) == 0 || len(port) == 0 {
	return nil, ErrNotInCluster
	}
	token, err := os.ReadFile(tokenFile)
	if err != nil {
	return nil, err
	}
	tlsClientConfig := TLSClientConfig{}
	if _, err := certutil.NewPool(rootCAFile); err != nil {
	klog.Errorf("Expected to load root CA config from %s, but got err: %v", rootCAFile, err)
	} else {
	tlsClientConfig.CAFile = rootCAFile
	}
	return &Config{
	// TODO: switch to using cluster DNS.
	Host: "https://" + net.JoinHostPort(host, port),
	TLSClientConfig: tlsClientConfig,
	BearerToken: string(token),
	BearerTokenFile: tokenFile,
	}, nil
	}

This at odds with the documentation at https://kubernetes.io/docs/tasks/run-application/access-api-from-pod/#directly-accessing-the-rest-api which states:

While running in a Pod, the Kubernetes apiserver is accessible via a Service named kubernetes in the default namespace. Therefore, Pods can use the kubernetes.default.svc hostname to query the API server. Official client libraries do this automatically.

client-go does not do this. Though, it includes a TODO:

		// TODO: switch to using cluster DNS.
		Host:            "https://" + net.JoinHostPort(host, port),

Ambiguity around the documented client discovery policy causes problems for client libraries; and cloud providers continue to promulgate the (now undocumented) legacy env configuration, e.g. Azure/AKS#3183.

Would you accept a PR to remove the environment based in-cluster configuration in favor of always using kubernetes.default.svc?

What did you expect to happen?

client-go should use kubernetes.default.svc and it should not honor the KUBERNETES_SERVICE_HOST environment variable.

OR the client docs should be updated with the proper heuristics.

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

...

Anything else we need to know?

No response

Kubernetes version

1.24

Cloud provider

AKS

OS version

No response

Install tools

No response

Container runtime (CRI) and version (if applicable)

No response

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

No response

[olix0r]

/sig api-machinery

[leilajal]

/cc @Jefftree
/triage accepted

[liggitt]

The client docs should be updated, rather than change the client-go implementation. Making client-go depend on DNS is not a completely compatible change

[olix0r]

Looking at some of the officially supported kubernetes client libraries, all appear to use the environment variables and none use the in-cluster DNS

• https://github.com/kubernetes-client/javascript/blob/98414841064cbc322b617538a34bf0f5e31a5122/src/config.ts#L194-L219
• https://github.com/kubernetes-client/python/blob/16935036252be0fdf164f8ce624bd71eb8d35e89/kubernetes/base/config/incluster_config.py#L68-L70
• https://github.com/kubernetes-client/c/blob/2bb4393cc827e4c105fe57ded59e787bc3a4fbf0/kubernetes/config/incluster_config.c#L44-L67
• https://github.com/kubernetes-client/csharp/blob/f0d15cb39ef377370b219f0aa5f860764f0cc12a/src/KubernetesClient/KubernetesClientConfiguration.InCluster.cs#L51-L52
• https://github.com/kubernetes-client/haskell/blob/e848bc0c5c8d4fe4d2c9e8555331c219814c8ec9/kubernetes-client/src/Kubernetes/Client/Config.hs#L97-L99
• https://github.com/kubernetes-client/java/blob/73ae39a97dfdec9936944c23015edc1ab6742589/util/src/main/java/io/kubernetes/client/util/ClientBuilder.java#L242-L253
• https://github.com/kubernetes-client/ruby/blob/33861097e2eab9954b3f07f6898e5e8199199731/kubernetes/src/kubernetes/config/incluster_config.rb#L22-L46

So, with this in mind, the documentation should definitely be updated.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[olix0r]

/remove-lifecycle stale

kubernetes/website#36691 has been open since September. Has approvals. Still waiting for a merge...