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

chore: version the crd #661

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

chore: version the crd #661

wants to merge 8 commits into from

Conversation

NickLarsenNZ
Copy link
Member

@NickLarsenNZ NickLarsenNZ commented Dec 4, 2024

Description

Note

This PR serves as a model for how to convert CRD definitions to stackable-versioned CRDs.
See the explanation of the commits below, and also see the extended commit messages.

  1. 4000684: (Technically optional) Migrate the crd workspace member crate to be a module of the operator-binary member crate. Run cargo check to make sure everything is fine before continuing.
    • If it is used by other member crates (eg: in this case there is the user-info-fetcher), then export it via a lib.rs in the operator-binary crate.
  2. 6ae92e4: Add stackable-versioned as a workspace dependency, and refer to it from relevant crates.
  3. 4eaf36c: Split impls from the data structures for the CRD (or any types or groups of types that will be versioned). This makes the next step easier...
  4. d8715b3: Wrap the CRD data structures in a versioned module. Tweak the top level CRD struct to work with stackable-versioned.
  5. 19d9328: Optionally version other data structures. In this case, we independently version the user_info_fetcher types that are used from the main CRD.
  6. e9d201c: Fix all of the references to the new versioned types.
    • Ideally we should qualify usages of the structures with the version (eg: v1alpha1::Foo, rather than just Foo to make is clear).
      In some cases this might not be possible, for example if you needed both crd::v1alpha1 and crd::user_info_fetcher::v1alpha1 imported - they would conflict. In that case, import crd::v1alpha1 for the crd, and crd::user_info_fetcher (without the version module) for the other. The code would then use types qualified like: v1alpha1::Foo and user_info_fetcher::v1alpha1::Bar.

Tip

A separate PR will show how to make changes to versioned types.

This makes way for the versioned module we will soon introduce
At this point, errors will appear in any crates using the crd.
It has only been done separately to illustrate the ease in versioning a CRD without all of the other necessary changes.
This is helpful for later crd version sharing substructures that might not change.
For example: v1alpha2::OpaCluster might still use user_info_fetcher::v1alpha1::Config,
or perhaps it uses user_info_fetcher::v1beta1::Config.

Similarly, shared structures from stackable-operators can then be
versioned in the same way.
The versioned module is imported rather than the individual structs and enums (when there is no conflict, eg: if also importing a versioned shared struct) so that that usages show the version explicitly.

There might be times where this isn't possible, for example, once structs and enums are versioned in stackable-operator, there could be multiple modules with the same name.

In this case, user-info-fetcher is also versioned with v1alpha1, so it is referred to as user_info_fetcher::v1alpha1 in crd/mod.rs so as to not conflict with the crds v1alpha1.
@NickLarsenNZ NickLarsenNZ force-pushed the chore/add-crd-versioning branch from f9bea09 to e9d201c Compare December 5, 2024 09:59
@NickLarsenNZ NickLarsenNZ requested a review from Techassi December 5, 2024 12:30
@NickLarsenNZ NickLarsenNZ marked this pull request as ready for review December 5, 2024 12:30
@NickLarsenNZ NickLarsenNZ force-pushed the chore/add-crd-versioning branch from c834ee3 to 6e061f2 Compare December 5, 2024 16:05
Co-authored-by: Techassi <[email protected]>
@NickLarsenNZ NickLarsenNZ force-pushed the chore/add-crd-versioning branch from 6e061f2 to e319f4b Compare December 5, 2024 16:06
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.

1 participant