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: Change format in @PrinterColumn to enum #6455

Closed
baloo42 opened this issue Oct 15, 2024 · 3 comments · Fixed by #6620
Closed

CRDGenerator: Change format in @PrinterColumn to enum #6455

baloo42 opened this issue Oct 15, 2024 · 3 comments · Fixed by #6620
Labels
component/crd-generator Related to the CRD generator
Milestone

Comments

@baloo42
Copy link
Contributor

baloo42 commented Oct 15, 2024

Is your task related to a problem? Please describe

To align with the upcoming @AdditionalPrinterColumn annotation and to make this typesafe, the type of format in @PrinterColumn should be changed to an enum.

Describe the solution you'd like

enum PrinterColumnFormat {
    NONE(null),
    INT32("int32"),
    INT64("int64"),
    FLOAT("float"),
    DOUBLE("double"),
    BYTE("byte"),
    DATE("date"),
    DATE_TIME("date-time"),
    PASSWORD("password");

    public final String value;

    Format(String value) {
      this.value = value;
    }

    public String getValue() {
      return value;
    }
  }

This enum should be shared between @AdditionalPrinterColumn and @PrinterColumn.

Describe alternatives you've considered

Phase it out:

  1. Add additional enum param formatStrict, deprecate format (with 7.0)
  2. Replace format with enum param, deprecate formatStrict (with 8.0)
  3. Remove formatStrict (with 9.0)

Additional context

First discussed here:

Note that this would be a breaking change.

@manusa manusa added the component/crd-generator Related to the CRD generator label Oct 22, 2024
@manusa manusa added this to the 7.0.0 milestone Oct 22, 2024
@baloo42
Copy link
Contributor Author

baloo42 commented Nov 18, 2024

I have already worked on this and discovered a blocker at least for me.

If we change the annotation param format to an enum, we also have to update the old implementation.
The problem is, I don't know how to get the enum value here:

https://github.com/fabric8io/kubernetes-client/blob/main/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractCustomResourceHandler.java#L94

If I run the unit tests, I can cast it to the enum. But if I run this via the annotation processor, I'm not able to cast it.
I guess casting is the wrong way. Can someone point me into the right direction?

How can I get the value of an enum from an annotation if sundrio's AptContext is used?

@manusa
Copy link
Member

manusa commented Nov 19, 2024

Can someone point me into the right direction?

I don't know about the right direction or the right approach when dealing with APT.

The only thing I can suggest is to do a hacky .from(Object value) in the enum and then deal with the two cases there (i.e. if it's instanceof the enum return it, if not parse it).

@baloo42
Copy link
Contributor Author

baloo42 commented Nov 19, 2024

Thanks, you got me somehow into the right direction. com.sun.tools.javac.code.Symbol$VarSymbol implements toString() which is in case of an enum value the name. The following method works:

  // o is in case of running as annotation processor an instance of com.sun.tools.javac.code.Symbol$VarSymbol
  private static Optional<PrinterColumnFormat> findPrinterColumnFormat(Object o) {
    if (o == null) {
      return Optional.empty();
    }

    if (o instanceof PrinterColumnFormat) {
      return Optional.of((PrinterColumnFormat) o);
    }

    String symbol = o.toString();
    return Arrays.stream(PrinterColumnFormat.values())
        .filter(f -> f.name().equals(symbol))
        .findFirst();
  }

Will polish the MR further in the next days.

@manusa manusa changed the title CRD-Generator: Change format in @PrinterColum to enum CRDGenerator: Change format in @PrinterColum to enum Nov 25, 2024
@baloo42 baloo42 changed the title CRDGenerator: Change format in @PrinterColum to enum CRDGenerator: Change format in @PrinterColumn to enum Nov 26, 2024
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.

2 participants