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

CRD-Generator v2: Maven Plugin #5944

Open
baloo42 opened this issue Apr 24, 2024 · 10 comments · May be fixed by #5979
Open

CRD-Generator v2: Maven Plugin #5944

baloo42 opened this issue Apr 24, 2024 · 10 comments · May be fixed by #5979
Labels
component/crd-generator Related to the CRD generator

Comments

@baloo42
Copy link
Contributor

baloo42 commented Apr 24, 2024

Is your enhancement related to a problem? Please describe

Because the CRD-Generator annotation processor is now deprecated, we need a replacement.
(see #5942)

Describe the solution you'd like

A CRD-Generator maven plugin could search for a top level annotation (or interface) and generate the CRDs using the CRD-Generator api.

Describe alternatives you've considered

No response

Additional context

  • Jandex could be used to find and index the relevant annotations/classes.
  • The following options should be supported:
    • Enable / disable parallel generation
    • CRD versions (v1, v1beta1)
      (TBD: There are thoughts to drop v1beta1 which makes this option irrelevant)
    • outputPath
    • header in output files
    • Allow to add additional labels
    • Allow to add additional annotations
    • Enable debug output

The also planned CRD-Generator gradle plugin (#5945) should support the same options.

@manusa manusa added the component/crd-generator Related to the CRD generator label Apr 24, 2024
@shawkins
Copy link
Contributor

As suggested by @manusa we can just start with a clone of https://github.com/fabric8io/kubernetes-client/tree/main/java-generator/maven-plugin

@baloo42
Copy link
Contributor Author

baloo42 commented Apr 25, 2024

I'm working on a prototype atm...

@manusa manusa changed the title CRD-Generator: Maven Plugin CRD-Generator v2: Maven Plugin Apr 25, 2024
@baloo42

This comment was marked as outdated.

@manusa
Copy link
Member

manusa commented Apr 26, 2024

Regarding the Maven Plugin implementation, I think it's OK, but we should strive to move any processing logic elsewhere so that it can be reused (for example for the Gradle Plugin). In the end, we want to have the plugins as just wrappers around functionality that's otherwise exposed.

Regarding the PR to Steven's branch, I'd try not to make that branch more complex and merge it ASAP as is. If we keep adding stuff to the branch, it'll get very hard to review.

@baloo42
Copy link
Contributor Author

baloo42 commented Apr 26, 2024

Regarding the PR to Steven's branch, I'd try not to make that branch more complex and merge it ASAP as is. If we keep adding stuff to the branch, it'll get very hard to review.

I'm totally fine with that decision. I just required a place to show the changes, so that I get initial feedback.

but we should strive to move any processing logic elsewhere so that it can be reused (for example for the Gradle Plugin)

The processing logic can be shared if we build an abstraction around org.codehaus.plexus.util.Scanner. But we need in this case another module shared by both plugins. If we put this into the crd-generator-api module it would introduce the jandex dependency to all subprojects.

IMHO it's probably the same amount of work. More complexity vs. a little bit duplicate code.

If we decide on a new intermediate module: What name should it have? indexer?

@manusa
Copy link
Member

manusa commented Apr 26, 2024

The processing logic can be shared if we build an abstraction around org.codehaus.plexus.util.Scanner

OK, I didn't see that we were leveraging Maven-specific capabilities to implement the mojo.

I think that if we implement first the Gradle plugin we'll be able to see if abstractions are needed or not. And if we need to follow a different path.

@baloo42
Copy link
Contributor Author

baloo42 commented Apr 26, 2024

Another approach would be to rely on a pre-existing jandex index which the user must provide. (e.g. by using the jandex-maven or gradle plugin). But I think it's not user friendly and should be only implemented as optional way.

I think that if we implement first the Gradle plugin we'll be able to see if abstractions are needed or not. And if we need to follow a different path.

Yes, let's start in parallel with the gradle plugin and see where it goes. I would try to implement it, but it will take some time as I haven't written any gradle plugins yet.

@manusa
Copy link
Member

manusa commented Apr 26, 2024

The ting is that regardless of the exposing method (Gradle plugin, Maven plugin, CLI main method) all of them should share the same processing logic to avoid any gaps across implementations. That's why I suggested in the first place to move processing logic elsewhere.

Maybe it must be easier to start first by exposing a public API method that can be exposed in a JBang script or a common CLI main method :)

@baloo42
Copy link
Contributor Author

baloo42 commented Apr 26, 2024

The ting is that regardless of the exposing method (Gradle plugin, Maven plugin, CLI main method) all of them should share the same processing logic to avoid any gaps across implementations.

👍

Then I think an intermediate module cannot be avoided if we don't want to introduce jandex to the api-v2 module.
The core problem remains the same. We have to scan class files, index them and find CustomResource classes.
Those classes must be loaded (including their dependencies) to work with Jackson's SchemaGenerator.

Or am I wrong?

I'll try to implement a CLI module next and try to find out what is required to share the code.

@baloo42
Copy link
Contributor Author

baloo42 commented May 3, 2024

The prototype can now be found here: baloo42#7

I extracted the shared code to a new module "crd-generator-collector" and refactored the maven plugin to use it.

@baloo42 baloo42 linked a pull request May 3, 2024 that will close this issue
11 tasks
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

Successfully merging a pull request may close this issue.

3 participants