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

CRDGenerator: Allow to configure conversion webhooks #5794

Open
baloo42 opened this issue Mar 11, 2024 · 10 comments
Open

CRDGenerator: Allow to configure conversion webhooks #5794

baloo42 opened this issue Mar 11, 2024 · 10 comments
Labels
component/crd-generator Related to the CRD generator

Comments

@baloo42
Copy link
Contributor

baloo42 commented Mar 11, 2024

Is your enhancement related to a problem? Please describe

At the moment there is no easy way to configure conversion webhooks for the generated CRDs:

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

Describe the solution you'd like

A solution could be to create one or more additional annotations which allows a user to configure the webhook:

Annotation @WebhookConversion:

/**
 * Decorates the CRD with the "Webhook" conversion strategy.
 * <p>
 * If this strategy is used, the Kubernetes API server will request an external webhook to do the conversion.
 * </p>
 * <p>
 * Note that only one strategy can be used at the same time for a single CRD.
 * If the "Webhook" strategy doesn't fit and no field has changed, use the {@link NoneConversion} strategy.
 * </p>
 *
 * @see <a href=
 *      "https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#webhook-conversion">
 *      Kubernetes Docs - CRD Versioning - Webhook Conversion
 *      </a>
 */
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface WebhookConversion {
  /**
   * The name of this strategy.
   */
  String NAME = "Webhook";

  /**
   * The ConversionReviewVersions.
   * <p>
   * ConversionReviewVersions is an ordered list of preferred `ConversionReview` versions the Webhook expects.
   * The API server will use the first version in the list which it supports. If none of the versions specified
   * in this list are supported by API server, conversion will fail for the custom resource. If a persisted Webhook
   * configuration specifies allowed versions and does not include any versions known to the API Server,
   * calls to the webhook will fail.
   * </p>
   *
   * @return the conversion review versions.
   */
  String[] versions();

  /**
   * The location of the webhook, in standard URL form (`scheme://host:port/path`).
   * Exactly one of `url` or `svcNamespace` / `svcName` must be specified.
   *
   * @return the URL of the webhook.
   * @see <a href=
   *      "https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#url">
   *      Kubernetes Docs - CRD Versioning - Webhook Conversion - URL
   *      </a>
   */
  String url() default "";

  /**
   * The name of the service.
   * Required if service instead of a URL is used.
   *
   * @return the service name.
   * @see <a href=
   *      "https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#service-reference">
   *      Kubernetes Docs - CRD Versioning - Webhook Conversion - Service Reference
   *      </a>
   */
  String serviceName() default "";

  /**
   * The namespace of the service.
   * Required if the service instead of a URL is used.
   *
   * @return the namespace of the service.
   * @see <a href=
   *      "https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#service-reference">
   *      Kubernetes Docs - CRD Versioning - Webhook Conversion - Service Reference
   *      </a>
   */
  String serviceNamespace() default "";

  /**
   * Service path is an optional URL path at which the webhook will be contacted.
   *
   * @return the path;
   */
  String servicePath() default "";

  /**
   * The service port.
   *
   * @return the service port
   */
  int servicePort() default 443;

}

Annotation @NoneConversion:

/**
 * Decorates the CRD with the "None" conversion strategy.
 * <p>
 * If this strategy is used, the built-in converter of the Kubernetes API server changes the apiVersion to the version
 * which is marked as stored and would not touch any other field in the custom resource.
 * Note that only one strategy can be used at the same time for a single CRD.
 * If the "None" strategy doesn't fit, use the {@link WebhookConversion} strategy.
 * </p>
 *
 * @see <a href=
 *      "https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#specify-multiple-versions">
 *      Kubernetes Docs - CRD Versioning
 *      </a>
 */
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface NoneConversion {

  /**
   * The name of this strategy.
   */
  String NAME = "None";
}

Describe alternatives you've considered

No response

Additional context

See also: #4692

Example Approval tests:
https://github.com/baloo42/crd-generator-victools/blob/main/test/src/test/java/io/fabric8/crd/generator/victools/approvaltests/conversion/NoneConversionExample.java
https://github.com/baloo42/crd-generator-victools/blob/main/test/src/test/resources/io/fabric8/crd/generator/victools/approvaltests/CRDGeneratorVictoolsApprovalTest.approvalTest.noneconversions.samples.fabric8.io.v1.approved.yml
https://github.com/baloo42/crd-generator-victools/blob/main/test/src/test/java/io/fabric8/crd/generator/victools/approvaltests/conversion/v2/WebhookConversionExample.java
https://github.com/baloo42/crd-generator-victools/blob/main/test/src/test/resources/io/fabric8/crd/generator/victools/approvaltests/CRDGeneratorVictoolsApprovalTest.approvalTest.webhookconversions.samples.fabric8.io.v1.approved.yml

@manusa manusa added the component/crd-generator Related to the CRD generator label Mar 12, 2024
@baloo42
Copy link
Contributor Author

baloo42 commented Apr 1, 2024

PoC: baloo42#3

@andreaTP
Copy link
Member

andreaTP commented Apr 1, 2024

You might want to checkout this repo too:
https://github.com/operator-framework/josdk-webhooks

cc. @csviri

@baloo42
Copy link
Contributor Author

baloo42 commented Apr 1, 2024

I already used JOSDK-webhooks as well as JOSDK, QOSDK and of course fabric8/kubernetes-client in a few real world projects. And I love it ;)
My last project for example was developing a Keycloak Entity Operator.

There are only some small things missing which I would like to contribute in the near future. To name a few planned improvements:

  • Support for the whole lifecycle of an Java Operator using the code-first approach and without the need of further kustomizing the generated CRDs. (Multi-Version CRDs, ConversionWebhooks, Version Deprecation, Deterministic Output)
  • Better support for the code-first approach / CRDGenerator:
    • Kubernetes Validations / CEL
    • Size constraints
    • List Type (x-kubernetes-list-type)
    • Map Type (x-kubernetes-map-type)
    • ListMapKeys (x-kubernetes-list-map-keys)
    • Embedded Resources (x-kubernetes-embedded-resource)
    • External Docs
    • ...
  • JOSDK: Better support for dynamic read-only dependant resources
  • JOSDK: Better support for events and status aggregators
  • Support for generating documentation websites for operators (CRDs, Metrics, Events, Status...)
  • Better support for Spring-Boot based operators

@csviri
Copy link
Contributor

csviri commented Apr 1, 2024

Hi @baloo42

JOSDK: Better support for dynamic read-only dependant resources
JOSDK: Better support for events and status aggregators

feel free to create issues for those and pls describe the enhancement for the current approach. Note that currently we are working on v5, so we can make some breaking changes.

Better support for Spring-Boot based operators

That would be more than welcome, unfortunately, we have limited time, basically spring boot starter is now in maintenance mode.

@andreaTP
Copy link
Member

andreaTP commented Apr 1, 2024

I already used JOSDK-webhooks as well as JOSDK, QOSDK and of course fabric8/kubernetes-client in a few real world projects. And I love it ;)

Nice! 🎉 And many thanks for engaging with us and contributing back, much appreciated!

My last project for example was developing a Keycloak Entity Operator.

Is it Open Source? If not, would you share details of its design? I'm interested and probably also @shawkins

Kubernetes Validations / CEL

FYI this exists: https://github.com/projectnessie/cel-java
I would love to see something happening in the area!

the code-first approach

This is completely out of curiosity for me, any specific reason why you prefer code first as opposed to contract first?

@baloo42
Copy link
Contributor Author

baloo42 commented Apr 1, 2024

feel free to create issues for those and pls describe the enhancement for the current approach. Note that currently we are working on v5, so we can make some breaking changes.

I will do before I start, thanks for the hint. We don't need those changes in the near future, so it makes sense to me contribute directly to v5. In short: We have produced a lot of NPEs in the past because status, status conditions and events must be passed through sometimes. I'm looking for a base implementation to avoid those often made mistakes.

That would be more than welcome, unfortunately, we have limited time, basically spring boot starter is now in maintenance mode.

We noticed that too. We started with the starter but had to switch to our own base. Maybe I can help here to add some missing features, make it easier to maintain and if you wish i can also help you with maintenance. Started a discussion in operator-framework/josdk-spring-boot-starter#142

Is it Open Source? If not, would you share details of its design? I'm interested and probably also @shawkins

Unfortunatly it's not open source. But my customer has an Open Source program and they might discuss this in the future. Until that, we can discuss some general design decisions. Can you suggest a place for it?
We had to implement a lot of workarounds for Keycloak's Admin REST API e.g. for Authentication Flows or Permissions for Token-Exchanges, which made me think of improving those pain points first and directly in Keycloak itself.

Kubernetes Validations / CEL
FYI this exists: https://github.com/projectnessie/cel-java
I would love to see something happening in the area!

Let's discuss it! --> #5851

This is completely out of curiosity for me, any specific reason why you prefer code first as opposed to contract first?

Good question 😃 It's often not my decision alone and I used both approaches in the past. I also think it heavily depends on the team and on the use case. But if I have the choice and if it fits to the use case, I prefer the code-first strategy.
I think the main reason behind of this decision is, that I hope to onboard other developers faster. My team members have made often good experiences by using the Code-First stategy on REST APIs (springdoc, smallrye, CXF OpenAPI Feature). On the other side I have often seen broken Contract-First approaches, because at some time they gave up generating code and worked on by directly manipulating the code.

@andreaTP
Copy link
Member

andreaTP commented Apr 2, 2024

Can you suggest a place for it?
We had to implement a lot of workarounds for Keycloak's Admin REST API e.g. for Authentication Flows or Permissions for Token-Exchanges, which made me think of improving those pain points first and directly in Keycloak itself.

Probably, the best place to have this discussion in public is here: https://github.com/keycloak/keycloak/discussions
If you feel uncomfortable having this discussion in the open, my personal email address is public on my GH profile, feel free to drop me an email 🙂

And thanks for the feedback on the rest of the subjects!

Copy link

stale bot commented Jul 18, 2024

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@BramMeerten
Copy link

You might want to checkout this repo too:
https://github.com/operator-framework/josdk-webhooks

I'm using the JOSDK framework as well, and it's useful for creating the conversion webhooks. But they still need to be configured in the CRD.
For the creation of CRD I use crd-generator, they are programmatically generated and deployed.

So I'm also intereseted in this proposal, because currently I can't configure the webhook for the CRD.

@baloo42
Copy link
Contributor Author

baloo42 commented Nov 26, 2024

Updated the description with the outcome of my experiments so far. Most of it is straight forward, but how should we deal with the more "dynamic" parts of the configuration?

The service namespace, service name and the caBundle attribute are often overridden when deployed to a specific environment.

conversion:
    strategy: Webhook
    webhook:
      conversionReviewVersions: ["v1","v1beta1"]
      clientConfig:
        service:
          namespace: default
          name: example-conversion-webhook-server
          path: /crdconvert
        caBundle: "Ci0tLS0tQk...<base64-encoded PEM bundle>...tLS0K"

Should we make e.g. the caBundle configurable via the annotation? Or would it be better to restrict configuration via annotation to only the meaningful parts and instead allow the user to inject a value by CLI, Maven, Gradle parameters?

Another approach would be to provide an utility method (API)/ subcommand (CLI)/ mojo (maven plugin) to patch existing CRDs with a conversion strategy. For example a ConversionWebhookConfigurerer.
This way a once created CRD could be further customized for different environments.

@BramMeerten Do you have an opinion on that topic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/crd-generator Related to the CRD generator
Projects
None yet
Development

No branches or pull requests

5 participants