RESTMapper randomly returns API versions

[stefanprodan]

After #2901 we are observing in Flux that the RESTMapper can not be used reliably to determine the preferred API version of a custom resource.

	// RESTMapping identifies a preferred resource mapping for the provided group kind.
	RESTMapping(gk schema.GroupKind, versions ...string) (*RESTMapping, error)

When calling RESTMapping for a GroupKind without specified versions (this is what kubernetes-sigs/cli-utils/pkg/kstatus does here), the controller-runtime RESTMapper will randomly return deprecated versions as the preferred resource mapping.

Given that the RESTMapping implementation picks the first mapping from the version array, it suggests that the controller-runtime RESTMapper using the Aggregated Discovery reorders the array or somehow it only contains a single recored even if in etcd the CRD has multiple versions.

func (m *DefaultRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*RESTMapping, error) {
	mappings, err := m.RESTMappings(gk, versions...)
	if err != nil {
		return nil, err
	}
	if len(mappings) == 0 {
		return nil, &NoKindMatchError{GroupKind: gk, SearchedVersions: versions}
	}
	// since we rely on RESTMappings method
	// take the first match and return to the caller
	// as this was the existing behavior.
	return mappings[0], nil
}

Logs from Flux kustomize-controller:

{"level":"info","ts":"2025-02-24T13:28:10.312Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:30:10.365Z","logger":"KubeAPIWarningLogger","msg":"v2beta2 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:32:10.423Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:34:10.497Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:36:10.555Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:38:10.631Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:40:10.699Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:42:10.760Z","logger":"KubeAPIWarningLogger","msg":"v2beta2 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:44:10.823Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:46:10.950Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:48:11.022Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:50:11.082Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:52:11.140Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:54:11.210Z","logger":"KubeAPIWarningLogger","msg":"v2beta2 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:56:11.268Z","logger":"KubeAPIWarningLogger","msg":"v2beta2 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T13:58:11.339Z","logger":"KubeAPIWarningLogger","msg":"v2beta2 HelmRelease is deprecated, upgrade to v2"}
{"level":"info","ts":"2025-02-24T14:00:11.591Z","logger":"KubeAPIWarningLogger","msg":"v2beta1 HelmRelease is deprecated, upgrade to v2"}

To fix this in Flux, we decided to vendor the RESTMapper from controller-runtime v0.19 in fluxcd/pkg#873.

[erikgb]

I can add that even after vendoring the RESTMapper from controller-runtime v0.19 in Flux, I see warnings about incorrect API versions being selected/preferred by Flux. Example from a test-bootstrap in an ephemeral cluster:

$ k logs -n flux-system kustomize-controller-6b478c49db-l682l | grep deprecated
{"level":"info","ts":"2025-02-25T07:58:21.495Z","logger":"KubeAPIWarningLogger","msg":"kyverno.io/v2beta1 ClusterCleanupPolicy is deprecated; use kyverno.io/v2 ClusterCleanupPolicy"}
{"level":"info","ts":"2025-02-25T07:58:21.499Z","logger":"KubeAPIWarningLogger","msg":"kyverno.io/v2beta1 ClusterCleanupPolicy is deprecated; use kyverno.io/v2 ClusterCleanupPolicy"}

And all our ClusterCleanupPolicy resources are declared as v2 in Git.

[caozhuozi]

To address the issue, do we need an option in the restmapper of controller-runtime to keep the preferred version info?

[alvaroaleman]

To address the issue, do we need an option in the restmapper of controller-runtime to keep the preferred version info?

We don't want to duplicate the complexity of the upstream restmapper regarding preferred version selection. Controller-Runtime itself has no use-case where the api version isn't known. The best compromise I can think of is to construct an upstream restmapper when someone calls one of the methods that select a version on our restmapper

[alvaroaleman]

We don't want to duplicate the complexity of the upstream restmapper regarding preferred version selection. Controller-Runtime itself has no use-case where the api version isn't known. The best compromise I can think of is to construct an upstream restmapper when someone calls one of the methods that select a version on our restmapper

It turns out this isn't needed, I misremembered the related code, the server tells us what the preferred version is. I posted a fix for this issue in #3151

What confuses me a lot here though is that this issue definitely predates the introduction of aggregated discovery, backporting the testcase I added to 0.19 makes it fail the same way. @stefanprodan what makes you think this is related to aggregated discovery?

From what I can tell this issue exists since the introduction of a truly lazy resmapper in v0.15

[stefanprodan]

what makes you think this is related to aggregated discovery?

I assumed it's the aggregated discovery because we didn't had this issue in older versions of Flux using controller-runtime 0.19 and older. It may well be that the latest RESTMapper made the version flipping happen more often. I will test the fix and see if we can switch back. Thanks a lot for looking into this.