-
-
Notifications
You must be signed in to change notification settings - Fork 15
feat(stackable-versioned)!: Integrate with ConversionReviews #1050
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
Conversation
crates/stackable-versioned-macros/src/codegen/flux_converter.rs
Outdated
Show resolved
Hide resolved
These tests are removed for now, because we are not able to roundtrip CRD conversions without data loss yet. This feature will be re-added in a follow-up PR in an improved form once loss-less roundtripping is supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left comments and suggestions
@@ -1,5 +1,6 @@ | |||
{ | |||
"rust-analyzer.cargo.features": "all", | |||
"rust-analyzer.imports.granularity.group": "crate", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: check if this is the same in our other rust repos:
- operator-templating
- stackablectl
- docker-images (patchable)
- containerdebug
- actions (interu)
- trino-lb
- product-config (any reason this can't just be a crate inside operator-rs?)
- config-utils
There are more, but they are less used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing to action here, this was just me thinking. I will check it out later.
Co-authored-by: Nick <[email protected]>
Co-authored-by: Nick <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds utilities to convert a CRD from one version into a different version. This is tightly integrated with CRD conversions, which send
ConversionReview
s to registered webhooks and return that review to the K8s apiserver.TLDR
#[versioned(k8s(skip(merged_crd)))]
flag has been removedmerged_crd
is now suffixed withVersion
try_convert
function to convert objects received via a ConversionReviewenable_tracing
option to#[versioned(k8s(options(...)))]
Detailed Overview
There are multiple different parts to make this whole process work. All of these parts are automatically generated by the macro and for the user it is a single function call:
Foo::try_convert
. The individual parts are explained below.Note
The CRD spec currently does not contain any fields. During the development of the conversion mechanism, this is emitted to more easily focus on the newly generated code. It will eventually contain fields to fully test the implementation.
Updated Version Enum
The generated code tries to abstract away as much of the underlying code as possible. For this use-case, a top-level version enum exists. Before this PR, it only provided an easy way to produce a single, merged CRD based on all defined CRD versions via
Foo::merged_crd(Foo::V1)
. The variants described the available versions and forced users to use well-defined versions instead of&str
which can be anything (eg. an invalid version).This PR changes the inner workings of this enum. Instead of only providing plain variants without any data, each variant now holds the appropriate version of the custom resource.
This change enables us to directly deserialize an object of a custom resource into the enum. This then enables us to generate further (in addition to
merged_crd
) top-level abstractions on the enum.With this change it is not possible to use
Self
to specify the stored apiversion viaFoo::merged_crd()
. Instead, a new enum for this purpose only is generated.(De)serialization from/to JSON values
Most (if not all) conversion will be handled automatically by a CRD conversion webhook. Before the K8s apiserver either stores the custom resource in etcd or responds to clients, it will send a
ConversionReview
to the registered webhook. A list of to be converted objects is provided as a JSON object. This is also represented in Rust code, more specifically aserde_json::Value
.As such, the code needs to be able to turn the JSON object into a Rust representation (in the correct version). Once conversion is performed, the opposite operation needs to be performed: turning the Rust struct back into serialized JSON.
This PR introduces two new (private) methods to do this:
Conversion function
Currently, conversion of custom objects (defined by versioned CRDs) is achieved by receiving the ConversionReview via a webhook (which will be run alongside the operator in the future). This ConversionReview is then handed over to the
try_convert
function. Internally, this will construct strictly typed objects based on the current apiVersion. They are then converted to the desired apiVersion and added to the list of converted objects. If all objects were able to be converted, a successful ConversionReview is returned. Otherwise, a failure is indicated....
Partial expanded code