CRD Generator not filtering ENUM$VALUES synthetic field

[shawkins]

Describe the bug

There seems to be some inconsistency with the VM reflection reporting $VALUES vs ENUM$VALUES - https://stackoverflow.com/questions/46924470/java-enums-enumvalues-or-values

If ENUM$VALUES is reported - and I don't quite see why it is as the same jre gives me different behavior at times - then it won't be filtered by the logic looking for a name starting with a $ -

kubernetes-client/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java

Line 668 in 683336b

.filter(n -> !n.startsWith("$"))

Fabric8 Kubernetes Client version

6.6.2

Steps to reproduce

Not quite sure the behavior seems inconsistent.

Expected behavior

That the VALUES field will be omitted regardless.

Ideally ClassToTypeDef would check / filter synthetic properties rather than just relying upon the $ convention.

Runtime

other (please specify in additional context)

Kubernetes API Server version

other (please specify in additional context)

Environment

Linux

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

[andreaTP]

The bug might be legit, have you attempted to explicitly add @JsonIgnore as a temporary workaround to values ?
It would be great to reproduce it in the test suite.

[shawkins]

have you attempted to explicitly add @JsonIgnore as a temporary workaround to values

Where would I add that on an enum?

It would be great to reproduce it in the test suite.

The stack overflow post also complains about the unpredictable nature of seeing both representations.

[shawkins]

Where would I add that on an enum?

If I for example add an explicit ENUM$VALUES field or enum value with a jsonignore, then when this issue is occurring another
ENUM$VALUES_0 synthetic field will be added automatically. There doesn't seem to be a way to apply an annotation to synthethic field.

A couple of additional observations:

• there's no support JsonIgnoreProperties, which could be applied at the class level.
• The logic shouldn't assume a field starting with $ should be ignored, since it's a generally valid identifier - the checks should be based upon other information such as whether it is synthetic.

[andreaTP]

Thanks a lot for the additional information @shawkins ! I'll take a closer look into this early next week, I hope that sundrio exposes the API to identify synthetic fields 🤞 otherwise is going to be a much slower fix.

In the test suite we are already generating the Realm import CRD from Keycloak, have you already verified if the bug reproduces locally to this codebase?

[shawkins]

I hope that sundrio exposes the API to identify synthetic fields crossed_fingers otherwise is going to be a much slower fix.

Unfortunately it does not - it probably should to then not use starting with $ as a filter. However it does convey that the field is static and private - that should not be included with the rest of the enums. Adding a filter like:

.filter(p -> !p.isStatic() || !p.isPrivate())

Will address this specific case.

It does bring up that only simple enums are supported - if you also include state in the enum, then it will be mistakenly included in the schema:

public enum Category {
    Any,
    Misc,
    Programming,
    Dark,
    Pun,
    Spooky,
    Christmas;

    private int rating = 1;

    public int getRating() {
      return rating;
    }

  }

causes rating to show up as an enum value. Maybe only public static fields should be included (it is of course possible with jackson to change the serialization of enums https://www.baeldung.com/jackson-serialize-enums but none of that is supported either).

In the test suite we are already generating the Realm import CRD from Keycloak, have you already verified if the bug reproduces locally to this codebase?

I was able to reproduce it eventually just with the JokeRequestSpec - ENUM$VALUES showed up as an enum value. However I still don't know what causes that. Trying different jres, etc., doesn't seem to guarentee that it will happen - it didn't initially happen when testing the SchemaSwap either.

Whatever causes it, it is known as a possibility in the jdk source: https://github.com/openjdk/jdk11u/blob/1000b1c0074af4b954c5f210d1684b5623e9723b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/spi/JavaConstantFieldProvider.java#L131

[andreaTP]

This issue might have the same solution as: #4225

[shawkins]

@andreaTP for now let's combine the filtering from that pr with the public static requirement:

.filter(p -> p.isStatic() && p.isPublic() && def.getFullyQualifiedName().equals(p.getTypeRef().toString()))

That will leave the corner case of a public static field on the enum that is typed the same as the enum (should be extremely rare). The only way to account for that is to expand sundrio to expose Field.isEnumConstant() - since that's such a corner case though I'd vote on adding the filtering now rather than waiting for the sundrio fix.

[andreaTP]

@shawkins I agree that we need to get a fix for this.

should be extremely rare

This is the only detail I disagree with since a getDefault() method in enums is pretty common.
Still, I don't want to get stuck on this, let's time-box the "proper" fix, if we can get a new sundrio release including the surfacing for isEnumConstant and isSynthetic within a week with the help of @iocanel and @metacosm it would be great.

Meanwhile, if you want to submit the proposed workaround I'll accept it to make sure we improve for the next release.

[shawkins]

This is the only detail I disagree with since a getDefault() method in enums is pretty common.

This would have to be expressed as:

enum Something {
  VALUE;
  public static final Something DEFAULT_VALUE = VALUE;
}

Using a method would be fine because we're only looking at the fields.

Meanwhile, if you want to submit the proposed workaround I'll accept it to make sure we improve for the next release.

#4230 has been updated.

[shawkins]

Marking resolved by #4230 - there will be another improvement after sundrio/sundrio#389