fix: Produce valid CRDs containing flattened untagged enums

[NickLarsenNZ]

Fixes #1941

Note

This is a minimal fix to make review easier and close the issue.
A follow up PR is intended to replace the properties transformation into a proper transformer like in #1839.

• 1a40d62: Add a test for flattened untagged enums.
• 2caf34b: Ensure description and type fields are dropped on untagged variants without fields.
• 596bb93: Stop untagged variant descriptions being pushed into properties (which are for the fields)

[codecov]

Codecov Report

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

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1942     +/-   ##
=======================================
+ Coverage   76.2%   76.3%   +0.1%     
=======================================
  Files         89      89             
  Lines       8487    8495      +8     
=======================================
+ Hits        6465    6474      +9     
+ Misses      2022    2021      -1     

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

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[doxxx93]

Verified with a kind cluster:

• The original bug reproduces on main (anyOf[2].description: Forbidden, anyOf[2].type: Forbidden)
• With this fix, CRD registration and CR instance creation all pass (tested struct variants, empty variants, newtype variants, Option<>, Vec<>, nested untagged enums)
• No regression on tagged enums (oneOf)

One minor nit left inline about test coverage for a newtype variant hitting the same else if branch.

LGTM otherwise.

[doxxx93]

Sorry for the ping @clux — I've verified the normal behavior through the tests. One thing I noticed: the new else if branch (removing type/metadata from variants without fields) runs for both one_of (tagged) and any_of (untagged) paths, even though the comment says "untagged variants." Is this fine?

Could you take a look when you get a chance?

[NickLarsenNZ]

One thing I noticed: the new else if branch (removing type/metadata from variants without fields) runs for both one_of (tagged) and any_of (untagged) paths, even though the comment says "untagged variants." Is this fine?

No this isn't fine. I will fix that (by only doing it when push_description_to_property == true - maybe rename the variable).
This PR is only dealing with untagged enums and should not change anything else.

I didn't see any test failing. Can you share how you tested that?
I tested on one of our CRDs and didn't see a regression, so I'd like to ensure that path is covered.

[doxxx93]

@NickLarsenNZ I didn't find a specific failing test case — it was more of a code review observation that the else if branch could theoretically run in the one_of (tagged) path as well. The existing tests with tagged enums pass fine, but I wasn't confident there isn't an uncovered edge case.

[NickLarsenNZ]

@doxxx93 I see what you mean.

These extra fields need to go regardless of oneOf or anyOf, as they are not accepted by Kubernetes.

My original implementation does this (and only preserves the descriptions in the case of a oneOf by moving them into the applicable property).
For the untagged variants, the doc-comment on the variant shouldn't be pushed into the properties because the comment is for the variant and not the field(s) within.

See here: https://github.com/stackabletech/kube-rs/blob/71707abf1109bad01bbf2c46e3b161846049767a/kube-core/src/schema/transform_properties.rs#L89-L95

I would keep it as-is for now, and raise a separate refactor PR based on the properties hoisting in #1839 which is easier to test/debug IMO - if you're ok with that.

[clux]

logic itself looks sensible to me. thanks a lot!

an extra comment on the if here would be nice though.

[clux]

this signature is getting a little awkard and is probably another refactoring candidate for refactoring later.

[NickLarsenNZ]

Yes, this was properly handled in the original refactor: https://github.com/stackabletech/kube-rs/blob/71707abf1109bad01bbf2c46e3b161846049767a/kube-core/src/schema/transform_properties.rs#L50-L57

[clux]

Thank you!