kube-rs / kopium

Kubernetes OPenapI UnMangler
Apache License 2.0
113 stars 21 forks source link

Disambiguate duplicate `Member` names #206

Closed clux closed 8 months ago

clux commented 8 months ago

This adds a safer name mutation strategy in output.rs, by checking to see if a pascal/snake rename would generate a duplicate name, and then suffixing the name if it does to keep it rust compatible.

This is to fix certain pathological cases like #165.

This solution does not drop information, and always manages to include everything in the struct (even if we don't know which one will eventually go away, like the case with relabeling actions).

Annotated Example

Using the PodMonitor CRD with the clashing relabeling actions, this is what we now produce on the the struct by default (minus my explaining comments about where the member name came from):

#[derive(Serialize, Deserialize, Clone, Debug)]
pub enum PodMonitorPodMetricsEndpointsMetricRelabelingsAction {
    #[serde(rename = "replace")]
    Replace, // originally named "replace" in the spec (a deprecated name)
    #[serde(rename = "Replace")]
    ReplaceX, // originally named "Replace" in the spec
    #[serde(rename = "keep")]
    Keep, // originally named "keep" in the spec (deprecated)
    #[serde(rename = "Keep")]
    KeepX, // originally named "Keep" in the spec

    #[serde(rename = "labelmap")]
    Labelmap, // originally named "labelmap" in the spec (deprecated)
    LabelMap, // originally named "LabelMap" in the spec
    /// ....
}

Caveats

Maybe in the future if the OpenAPI spec can include deprecated attrs, we can remove this logic.

(This PR was originally trying to deduplicate members by name, but this ended up picking the wrong names, leaving the good ones unreachable by chance.)

sebhoss commented 8 months ago

I like this a lot & it completely removes the need for my home grown solution (I have used yq to remove duplicate fields before passing the modified YAML to kopium). Running this against all CRDs in kube-custom-resources-rs yields a single warning though: The natOutgoing and nat-outgoing fields in an Calico IPPool resource are not enums and thus should use snake_case. The generated code is here: https://github.com/metio/kube-custom-resources-rs/blob/kopium-206/kube-custom-resources-rs/src/crd_projectcalico_org/v1/ippools.rs#L38

clux commented 8 months ago

thanks @sebhoss , have added a smarter suffixing var to deal with the differences between struct and enum members now!

clux commented 8 months ago

also nice to see all that manual override code go away :smile:

sebhoss commented 8 months ago

yeah this solution here is a lot better since it retains all enums/fields so people can actually use all possible values (even though they are deprecated upstream).

Great stuff - thanks a lot!