Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a versioned CustomResourceExt trait - fixes #497 #545

Merged
merged 4 commits into from
Jun 8, 2021
Merged

Conversation

clux
Copy link
Member

@clux clux commented Jun 6, 2021

Had a bit of a go at #497, and realised it's not quite that beginner worthy because of one hairy problem:

The new trait needs to hardcode the CustomResourceDefinition type from k8s_openapi if we want to keep enforcing the struct (and we probably should).
Unfortunately, doing this effectively hardcodes the apiextensions version and makes things using #[kube(apiextensions = "v1beta1")] not compile.

So, a couple of options present themselves:

  • immediately drop support for v1beta1 (minimally available EKS version supports v1)
  • parallel traits that users must select

I think for sake of stability guarantees and #508 parallel traits actually work here:

  • #[kube(apiextensions = "v1beta1")] can select the trait and implement the right one
  • users can pick out the trait (and it wont compile unless they pick the right one)
  • can export them from kube_core under versioned modules
  • can still have the "main" (most recent + available) version on the main path

So, if this seems acceptable. I propose this can perhaps serve as a guiding principle for #508

  • When we rely on versioned type definitions from kubernetes, we don't remove our old impls until they are removed from major cloud providers (provided we even implemented it in the first place)
  • When removing older versions, we mention it in the CHANGELOG

I.e. this case: we can remove kube_core::crd::v1beta1 when kubernetes 1.22 is the minimum EKS/AKS/GKE version.

@clux clux linked an issue Jun 6, 2021 that may be closed by this pull request
3 tasks
@clux clux mentioned this pull request Jun 6, 2021
6 tasks
Copy link
Contributor

@lfrancke lfrancke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I wouldn't have thought of the versioning issue at all.
Thank you for tackling this!

@clux clux merged commit 1f31d15 into master Jun 8, 2021
@clux clux deleted the crdext-trait branch June 8, 2021 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

define a CustomResourceExt in kube for kube-derive
2 participants