Fix OptionalEnum transform for complex enums

[doxxx93]

Fix OptionalEnum transform for complex enums

The OptionalEnum transform from #1908 only checked for first.contains_key("enum"), which handled simple enums but skipped complex enums (those using oneOf). This caused invalid CRDs when optional complex enum fields were used.

Motivation

Fixes additional case reported in #1906 (comment)

After #1908 was merged, @anmazzotti reported that Option<ComplexEnum> still generates invalid CRDs. The OptionalEnum transform only checked for first.contains_key("enum"), which handled simple enums but skipped complex enums (those using oneOf).

When an optional complex enum field is used, the transform would skip it, leaving an invalid anyOf structure in the CRD:

anyOf:
  - oneOf: [...]
  - enum: [null]
    nullable: true
description: "..."  # ❌ Forbidden with anyOf
properties: {...}   # ❌ Forbidden with anyOf
type: object        # ❌ Forbidden with anyOf

Solution

Remove the first.contains_key("enum") check from the OptionalEnum transform. The remaining conditions are sufficient to identify Option<T> patterns:

• anyOf with exactly 2 elements
• First element does not have nullable
• Second element is { "enum": [null], "nullable": true }

This works for all types of optional enums (simple and complex):

description: "..."
nullable: true      # ✅ Correctly added
oneOf: [...]        # ✅ Hoisted to root
properties: {...}
type: object

Added test cases for Option<ComplexEnum> in kube-derive/tests/crd_complex_enum_test.rs.

Note

The flattened optional enum case (#[serde(flatten)] foo: Option<Enum>) reported in the same comment is a schemars limitation, not a kube issue. According to serde documentation, flatten is only supported "within structs that have named fields."

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.3%. Comparing base (8bcdcb5) to head (f605219).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1934     +/-   ##
=======================================
+ Coverage   76.3%   76.3%   +0.1%     
=======================================
  Files         88      88             
  Lines       8408    8409      +1     
=======================================
+ Hits        6408    6412      +4     
+ Misses      2000    1997      -3     

Files with missing lines	Coverage Δ
kube-core/src/schema.rs	99.1% <100.0%> (+1.0%)	⬆️
kube-derive/tests/crd_complex_enum_test.rs	100.0% <100.0%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[clux]

looks good to me, thank you!

[clux]

not necessary now, but i think if we did

-        OptionalComplexEnumTest::crd(),
+        OptionalComplexEnumTest::crd().spec.versions?[0].schema?.open_api_v3_schema?,

on the left hand side of the assert in the future, these tests would be a lot shorter, less indented, and more focused on the actual statement we want to test

[NickLarsenNZ]

I don't think we shouldn't assume a fixed order of keys.

Just linking the solution from my first PR that also fixed this: https://github.com/stackabletech/kube-rs/blob/71707abf1109bad01bbf2c46e3b161846049767a/kube-core/src/schema/transform_any_of.rs#L52-L97

Replace the schema with the anyOf subschema and set to nullable when the
only other subschema is the nullable entry.

Used for correcting the schema for optional tagged unit enums.
The non-null subschema is hoisted, and nullable will be set to true.

This will return early without modifications unless:

• There are exactly 2 anyOf subschemas.
• One subschema represents the nullability (ie: it has an enum with a single
  null entry, and nullable set to true).

Regardless, once this goes in I can run and check the remaining tests from #1839 which I explicitly didn't bring in with #1921 (because they failed until this fix went in).

[doxxx93]

@NickLarsenNZ Thanks for the feedback! You're right that assuming array order isn't ideal. I checked schemars' current behavior and it consistently generates [schema, null] order, so this works for now. But I agree your approach of explicitly identifying which subschema represents nullability is more robust. Would be great to improve this in a follow-up PR.

[NickLarsenNZ]

Yeah, that is indeed the current behaviour, but I'm more concerned about it not breaking in the future (since that has already happened before).

So I have no objection to this PR going in as-is, and I can raise a/some follow up issues and subsequent PR(s) to improve that based on the original PRs I raised (if @clux is happy with that).

[clux]

Yeah, that is indeed the current behaviour, but I'm more concerned about it not breaking in the future (since that has already happened before).

So I have no objection to this PR going in as-is, and I can raise a/some follow up issues and subsequent PR(s) to improve that based on the original PRs I raised (if @clux is happy with that).

yeah, i agree with the concern. follow-ups would be appreciated!