Support complex enums in CRDs

[nightkr]

Motivation

See GREsau/schemars#84

Currently #[derive(CustomResource)] generates invalid CRD objects for enum variants with fields, because the generated schema is rejected by K8s as not being structural.

Solution

This PR adds a Visitor that transparently rewrites enum schemas from:

properties:
  enum:
    oneOf:
      - type: object
      - properties:
          variantOne:
            # properties...
          required: [variantOne]
      - type: object
        properties:
          variantTwo:
            # properties...
        required: [variantTwo]

to:

properties:
  enum:
    type: object
    properties:
      variantOne:
        # properties...
      variantTwo:
        # properties...
    oneOf:
      - required: [variantOne]
      - required: [variantTwo]

[codecov-commenter]

Codecov Report

Merging #779 (aef0969) into master (5e6762d) will increase coverage by 0.10%.
The diff coverage is 88.46%.

@@            Coverage Diff             @@
##           master     #779      +/-   ##
==========================================
+ Coverage   71.89%   72.00%   +0.10%     
==========================================
  Files          54       55       +1     
  Lines        3700     3725      +25     
==========================================
+ Hits         2660     2682      +22     
- Misses       1040     1043       +3     

Impacted Files	Coverage Δ
kube-derive/src/custom_resource.rs	11.80% <ø> (ø)
kube-core/src/schema.rs	87.50% <87.50%> (ø)
kube-client/src/api/util.rs	81.57% <100.00%> (ø)
kube-derive/tests/crd_schema_test.rs	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e6762d...aef0969. Read the comment docs.

[nightkr]

cc @cassandracomar @LukeMathWalker @clux @webern who have expressed interest in GREsau/schemars#84
and @thedodd who reported this issue in #129 (comment)

[clux]

This looks great! And the api-server accepts this? Beautiful.
Left some quick comments/questions.

[nightkr]

And the api-server accepts this?

Yes :)

[soenkeliebau]

I just ran a few simple tests as well and it worked like a charm!

[webern]

I haven't checked the code (yet), but I thought of a couple of edge cases. You may have already tested these.

Does an enum like this work? The Empty variant would be some kind of empty object with no properties?

enum DoesThisWork {
    Empty,
    NotEmpty(MyStruct),
}

If we do the same thing with some nesting?

enum InnerEnum {
    Nope,
    Yep(SomeStruct)
}

enum Outer {
    Inner(Inner),
    Foo(AnotherStruct),
    Nothing,
}

I normally avoid having a variant and struct by the same name (even though that's actually what I want, but my IDE gets confused). But in the above example I thought having the same name for variant and struct makes for a good edge case.

[nightkr]

@webern

Does an enum like this work? The Empty variant would be some kind of empty object with no properties?

enum DoesThisWork {
   Empty,
    NotEmpty(MyStruct),
}

Not directly, no. That would serialize as:

# assuming this is wrapped in a field does_this_work
doesThisWork: empty
# or...
doesThisWork:
  notEmpty:
    # MyStruct contents

This violates the rule that all variants must have the same type. Changing Empty to an empty struct variant would ensure that both cases are serialized as objects:

enum DoesThisWork {
    Empty {},
    NotEmpty(MyStruct),
}

That causes the serialization to look like:

doesThisWork:
  empty: {}
  # or...
  notEmpty:
    # MyStruct contents

If we do the same thing with some nesting?

enum InnerEnum {
    Nope,
    Yep(SomeStruct)
}

enum Outer {
    Inner(Inner),
    Foo(AnotherStruct),
    Nothing,
}

Same thing here, it should work as long as you change the Nope and Nothing unit variants to empty struct variants instead.

I normally avoid having a variant and struct by the same name (even though that's actually what I want, but my IDE gets confused). But in the above example I thought having the same name for variant and struct makes for a good edge case.

Type names are ignored here anyway, only variant names matter.

[webern]

This violates the rule that all variants must have the same type.

I see, thank you for explaining this. So all of the following would work?

enum {
    A(String),
    B(String),
}

enum {
    A(u8),
    B(u8),
}

enum {
    A(SomeStruct{}),
    B(DifferentStruct{}),
    C{},
}

And this would not work?

enum {
    A(u8),
    B(String),
}

[nightkr]

I see, thank you for explaining this. So all of the following would work?

enum {
    A(String),
    B(String),
}

enum {
    A(u8),
    B(u8),
}

Yes.

enum {
    A(SomeStruct{}),
    B(DifferentStruct{}),
    C{},
}

After fixing SomeStruct{} -> SomeStruct, yes.

And this would not work?

enum {
    A(u8),
    B(String),
}

Yes it would, this would serialize as:

foo:
  a: 5
  # or...
  b: foo

Which is valid.

---

I think I explained this poorly, the enum itself must always serialize to the same JSON "type" (bool, number, string, array, object). For externally tagged enums (the default), the result of this is that either all or none of the variants must be unit variants (A,).

Each non-unit variant gets serialized as an object with a unique key, so there is no limit on what type those variants' values can have (apart from the usual constraint that they must also have a JsonSchema that we can turn structural).