JsonSubTypes not properly handled while generating CRD from Java annotated class

[renannprado]

Describe the bug

Disclaimer: I've tried to check whether that's a limitation on the CRD spec, but I don't know for sure whether it's the case or not, so I decided to create a bug anyway.

I'm trying to generate the CRD from a Java class, but I need to use a generic type and while generating the CRD, it comes out as object instead of having the actual properties. Both for a List<T> and for property of the generic type itself

Output:

# Generated by Fabric8 CRDGenerator, manual edits might get overwritten!
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: examples.dev.renann
spec:
  group: dev.renann
  names:
    kind: example
    plural: examples
    singular: example
  scope: Namespaced
  versions:
  - name: v1alpha1
    schema:
      openAPIV3Schema:
        properties:
          spec:
            properties:
              myList:
                items:
                  type: object
                type: array
              myField:
                type: object
              status:
                properties:
                  error:
                    type: boolean
                  message:
                    type: string
                type: object
            type: object
          status:
            properties:
              error:
                type: boolean
              message:
                type: string
            type: object
        type: object
    served: true
    storage: true
    subresources:
      status: {}

Fabric8 Kubernetes Client version

6.0.0

Steps to reproduce

Create a class like the below and try to generate the CRD definition.

package dev.renann.kubernetes.operator;

import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.annotation.JsonTypeResolver;
import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.model.annotation.*;

import java.util.List;

@Group("dev.renann")
@Version("v1alpha1")
@Kind("example")
public class ExampleCrd extends CustomResource<ExampleCrd.MySpecArgs, ExampleCrd.ExampleStatus> implements Namespaced {
    public ExampleCrd() {
    }

    public static class ExampleStatus {
        boolean error;
        String message;

        public ExampleStatus(boolean error, String message) {
            this.error = error;
            this.message = message;
        }
    }

    public static class MySpecArgs {
        private final List<MyInterface> myList;
        private final MyInterface myField;
        private final ExampleStatus status;
        public MySpecArgs(List<MyInterface> myList, MyInterface myField, ExampleStatus status) {
            this.myList = myList;
            this.myField = myField;
            this.status = status;
        }

        public List<MyInterface> getMyList() {
            return myList;
        }

        public ExampleStatus getStatus() {
            return status;
        }

        @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type")
        @JsonSubTypes({
                @JsonSubTypes.Type(
                        value = MyImpl.class,
                        name = "the-impl")
        })
        private interface MyInterface {
            String getType();
        }

        public static class MyImpl implements MyInterface {
            private final String name;

            public MyImpl(String name) {
                this.name = name;
            }

            public String getName() {
                return name;
            }

            @Override
            public String getType() {
                return "the-type";
            }
        }
    }
}

Expected behavior

As I said, I'm not 100% sure whether this is supported by the CRD spec, but I would expect that the generic types properties are present on the spec and that a certain field support multiple types.

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

1.23

Environment

macOS

Fabric8 Kubernetes Client Logs

No response

Additional context

No response

[metacosm]

I'm not sure if polymorphic lists are supported by the OpenAPI schemas either… Just a quick question, though, shouldn't the name of the sub-type in the @JsonSubTypes.Type annotation match what's returned by MyImpl.getType?

[andreaTP]

Hi @renannprado , thanks for opening this issue! Unfortunately, at the moment, generic types are not translated into the CRD, and is probably some major piece of work to actually support them.

The best workaround you have at the moment is to specify a concrete type annotating the field with SchemaFrom

[andreaTP]

Example:

class MyImplList extends List<MyImpl> {}

... 

    public static class MySpecArgs {
        @SchemaFrom(MyImplList.class)
        private final List<MyInterface> myList;

[manusa]

We're not planning on providing support for this at the moment. Does the suggested workaround work for you?

[renannprado]

@manusa I didn't try to use this workaround yet, but what I did was to manually add x-kubernetes-preserve-unknown-fields to the relevant part of the CRD definition.

How can I add x-kubernetes-preserve-unknown-fields in this situation? Is there a way to do it via annotations?

I also don't see how this annotation would help as there are multiple implementations of MyInterface in my case.

[andreaTP]

@renannprado you are right, in the case of multiple implementations, it's not going to work. Here you can find the documentation on how to automatically generate x-kubernetes-preserve-unknown-fields

[renannprado]

@andreaTP thanks for the hint.

Maybe that's out of scope and should have it's own feature, but don't you think that the x-kubernetes-preserve-unknown-fields should be added automatically for fields that use JsonSubTypes and/or JsonTypeInfo? IMHO that already implies x-kubernetes-preserve-unknown-fields. There could be some situations where you don't want to add the x-kubernetes-preserve-unknown-fields, however perhaps those situations could be inferred by checking the parameter values of the annotations.

[andreaTP]

Adding x-kubernetes-preserve-unknown-fields is basically a workaround and I'm reluctant to automate it.

In various edge cases (e.g. JsonSubTypes with only one implementation) adding x-kubernetes-preserve-unknown-fields might mask other serialization issues and not be the correct thing to do.

could be inferred by checking the parameter values of the annotations

@renannprado do you have a concrete proposal that can cover all the invariants? Or, would you like to submit a PR to show this idea in action?

[renannprado]

@andreaTP Fair enough. If you say it's not that easy, I believe in you.

But I'm intrigued by your response. If the x-kubernetes-preserve-unknown-fields is a workaround, what would be a proper solution here in your opinion?

[andreaTP]

If you say it's not that easy, I believe in you.

This is not what I'm saying 😃 I haven't looked carefully at this, it might be easier or harder than expected.

what would be a proper solution here in your opinion?

I'm not sure, but I can share my thoughts. Since this feature is not supported by the CRD definition I'm tempted to say that it should be completely unsupported (e.g. throwing an exception) even by the generator. Adding x-kubernetes-preserve-unknown-fields is a workaround as it allows any unknown field to be included, preventing any kind of reasonable validation. But, that said, it might be possible to define a reasonable semantic for some use cases:

• In the case of a single implementation class the implementation is straightforward
• if the presence or absence of fields clearly discriminates all the possible implementations an implementation is possible (possibly with a few limitations around required fields?)
• if the type is serialized along in an explicit field it's again doable with a few minor restrictions

The biggest unknown, for me, is how those options will play with the default serialization used all around in this codebase (IIRC you should instruct the Jackson mapper to have the desired behavior).

All in all, I'm happy to listen to proposals and review POCs or WIPs in this direction as I do not have, at the moment, a compelling use case to reason about/spend time on.

I hope this helps @renannprado

[renannprado]

Adding x-kubernetes-preserve-unknown-fields is a workaround as it allows any unknown field to be included, preventing any kind of reasonable validation.

The validation that is built-in is not enough from my POV. For example, I have a field where you can input a URL, which is a string. But not any string is a valid URL, so even though the type is correct, the value might still be wrong.

Moreover,

if the presence or absence of fields clearly discriminates all the possible implementations an implementation is possible (possibly with a few limitations around required fields?)

Are you suggesting to squash all fields under the same object? I mean, that could work, but at this point I would favor your last suggestion.

if the type is serialized along in an explicit field it's again doable with a few minor restrictions

If I understood correctly, let's say you have 3 implementations of the interface, then you'd have

array-type1:
  - obj-type1...
array-type2:
  - obj-type2...
array-type3:
  - obj-type3...

I'm not sure if I like that, but I'll think about it.

The biggest unknown, for me, is how those options will play with the default serialization used all around in this codebase (IIRC you should instruct the Jackson mapper to have the desired behavior). All in all, I'm happy to listen to proposals and review POCs or WIPs in this direction as I do not have, at the moment, a compelling use case to reason about/spend time on.

I get what you mean. I don't have capacity to propose something now unfortunately, but I'll come back to this in the future. I also have just got started with using the library and implementing operators, so I bet I have lots of things to learn before I can make a good proposal.

But thanks for being open for it!

[andreaTP]

For example, I have a field where you can input a URL, which is a string. But not any string is a valid URL, so even though the type is correct, the value might still be wrong.

I agree that the "built-in" validation is not going to cover a lot of cases, but I just wanna mention that simple validation like URL is supported using the built-in pattern.

If I understood correctly, let's say you have 3 implementations of the interface, then you'd have

Let me say that my thoughts are not settled here, but I'm looking for something like:

my-array:
  oneOf:
    - array-type1
    ...
    - array-type2
    ...
    - array-type3
    ...

I get what you mean. I don't have capacity to propose something now unfortunately, but I'll come back to this in the future. I also have just got started with using the library and implementing operators, so I bet I have lots of things to learn before I can make a good proposal.

Enjoy the journey!!! 🎉

But thanks for being open for it!

no problem at all, thanks for the discussion, for the moment I would prefer to close this issue as it seems to be:

• a niche enough use-case
• in need of a more "structured" solution/proposal

Feel free to re-open it for any compelling reason, or, open a new issue when you come to it 🙂