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

feat: Add migration webhooks #137

Closed
buehler opened this issue Jan 22, 2021 · 11 comments · Fixed by #639
Closed

feat: Add migration webhooks #137

buehler opened this issue Jan 22, 2021 · 11 comments · Fixed by #639
Labels
enhancement New feature or request

Comments

@buehler
Copy link
Owner

buehler commented Jan 22, 2021

https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#configure-customresourcedefinition-to-use-conversion-webhooks

@buehler buehler added the enhancement New feature or request label Jan 22, 2021
@buehler
Copy link
Owner Author

buehler commented Jan 23, 2021

@anekdoti
Copy link

Hello Christoph, is there currently a schedule to implement conversion webhooks? All the best!

@buehler
Copy link
Owner Author

buehler commented Nov 12, 2021

Hey @anekdoti

Currently not. I'm not certain how I'd implement them at the moment. (And I'm on vacation roght now :)

The thing with the webhooks is, that they need one store version and they must convert all other versions from and to that storageversion. Currently O've no idea on how I conceptually implement this.

Regards

@anekdoti
Copy link

Hello @buehler, I hope you enjoyed your well-deserved vacation!

I also thought about starting to implement conversion webhooks - but I just did not find the time for it until now.

Wouldn't it maybe be the easiest to define the interface for a conversion webhook with an argument of type object, and then to case distinctions based on runtime type information in the implementation?

@buehler
Copy link
Owner Author

buehler commented Nov 30, 2021

Yes, I thought about that too.

It may be a feasible idea to create an interface like the other webhooks. I thought about something like in Newtonsoft JSON or yamldotnet: an interface that defines "can convert" on an object.

interface IConversionWebhook<TEntity>
{
  bool CanConvert(Type type);
  // ...
}

Now as I understand the matter, the conversion always takes place when a version is received that is not in the "storage version".

So, another possible option could be:
Let us assume we have an entity V1CustomUser that contains a username and password field. The V2CustomUser version renames those fields to "user" and "pass" and V3CustomUser removes "pass" for security reasons.
The storage version is v3.

If (by documentation and convention) we create a base interface for all those entities (e.g. ICustomUser), we could implement a conversion webhook that handles all types that inherit ICustomUser and furthermore can reflect the current storage type. Problem is, that we do not know the exact way of translating the versions.

As far as I know, if we want to translate a v1 to v2 (in the example above), Kubernetes translates the v1 to v3 (since v3 is the storage version) and then converts it to v2 again.

https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#webhook-conversion

@buehler
Copy link
Owner Author

buehler commented Nov 30, 2021

Further reading in the Kubernetes docs; there is a possible scenario written:

  1. The storage version is v1beta1. You create an object. It is persisted in storage at version v1beta1
  2. You add version v1 to your CustomResourceDefinition and designate it as the storage version.
  3. You read your object at version v1beta1, then you read the object again at version v1. Both returned objects are identical except for the apiVersion field.
  4. You create a new object. It is persisted in storage at version v1. You now have two objects, one of which is at v1beta1, and the other of which is at v1.
  5. You update the first object. It is now persisted at version v1 since that is the current storage version.

@anekdoti
Copy link

anekdoti commented Nov 30, 2021

As I understood, this version in the middle (in the Kubebuilder documentation called "hub") does not even have to be the storage version - but maybe this restriction is typical.

How about the following approach: the user can supply implementations of an interface IConvert<From, To>. The conversion webhook itself is not implemented by the developer of the operator but provided in KubeOps and it uses DI / assembly scanning to find implementations of IConvert. It's conversion method gets the resource to be converted as a plain object or even a JSON fragment, casts it approriately, and calls two converters with the storage version or some other chosen version as the hub.

@buehler
Copy link
Owner Author

buehler commented Dec 1, 2021

Hmm that may be an option.

Another idea that came to my mind last evening: If I understand the documentation correctly, Kubernetes is not required to convert everything to the "stored version". It tries to store a newly created object, or an updated one in the "storage version". What I don't understand is this section:

 webhook:
      # conversionReviewVersions indicates what ConversionReview versions are understood/preferred by the webhook.
      # The first version in the list understood by the API server is sent to the webhook.
      # The webhook must respond with a ConversionReview object in the same version it received.
      conversionReviewVersions: ["v1","v1beta1"]

(souce: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#webhook-conversion)

Does this mean, we could "trick" Kubernetes to always use a dummy/"know-it-all" object and convert them to other versions? (e.g. one would create an artificial entity VwhateverObject that contains all fields of other versions?)

Or does this field just configure which versions the webhook is able to convert? If so, it could be possible to let the developer implement IConversionWebhook<FROM,TO> or IConversionWebhook<Entity> and generate the possible list from those webhooks per entity.

@anekdoti
Copy link

anekdoti commented Dec 1, 2021

I think this is not about the versions of the custom resource definition, but about the supported versions of the ConversionReview object that is used to call the webhook.

@anekdoti
Copy link

anekdoti commented Dec 2, 2021

I think the source and target versions in ConversionReview object sent to the webhook can be arbitrary (because e.g. there are situations where a custom resource is requested in a different version than the storage version. The artifical entity with all fields you mention could be something like the "Hub" I mentioned above, but I think it would be more one efficient strategy to implement a conversion webhook in a specific operator.

@buehler
Copy link
Owner Author

buehler commented Oct 17, 2023

This is now implemented in #639.

I need some feedback about the way I implemented it. Maybe there is a nicer way to register these webhooks

buehler added a commit that referenced this issue Dec 6, 2023
This closes #137. This adds the preview version of conversion webhooks.
The API is not stable yet and changes may happen in the future. They
require the preview features flag to be set.

BREAKING CHANGE: The CLI (which broke anyway...) does now
generate everyting together and determines based on the
project if there are webhooks or not. This results in one
config folder instead of separate folders for CRD, RBAC, etc.
With this, conversion webhooks can inject their configuration
into the CRDs. Otherwise, this would be a cumbersome process.
If the need for single topic generation arises again (for example only CRDs),
then these generators can be added again.
buehler added a commit that referenced this issue Jan 17, 2024
This closes #137. This adds the preview version of conversion webhooks.
The API is not stable yet and changes may happen in the future. They
require the preview features flag to be set.

BREAKING CHANGE: The CLI (which broke anyway...) does now
generate everyting together and determines based on the
project if there are webhooks or not. This results in one
config folder instead of separate folders for CRD, RBAC, etc.
With this, conversion webhooks can inject their configuration
into the CRDs. Otherwise, this would be a cumbersome process.
If the need for single topic generation arises again (for example only CRDs),
then these generators can be added again.
buehler added a commit that referenced this issue Jan 17, 2024
This closes #137. This adds the preview version of conversion webhooks.
The API is not stable yet and changes may happen in the future. They
require the preview features flag to be set.

BREAKING CHANGE: The CLI (which broke anyway...) does now
generate everyting together and determines based on the
project if there are webhooks or not. This results in one
config folder instead of separate folders for CRD, RBAC, etc.
With this, conversion webhooks can inject their configuration
into the CRDs. Otherwise, this would be a cumbersome process.
If the need for single topic generation arises again (for example only CRDs),
then these generators can be added again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants