Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

not handling non-string enum #91

Closed
shaneutt opened this issue Nov 7, 2022 · 5 comments · Fixed by #100
Closed

not handling non-string enum #91

shaneutt opened this issue Nov 7, 2022 · 5 comments · Fixed by #100
Assignees

Comments

@shaneutt
Copy link

shaneutt commented Nov 7, 2022

Hi 👋

When using kopium to generate Kubernetes Gateway API resources, I found that it's not capable of generating one of the APIs, HTTPRoute specifically:

$ curl -sSL 'https://raw.githubusercontent.com/kubernetes-sigs/gateway-api/main/config/crd/standard/gateway.networking.k8s.io_httproutes.yaml?ref=v0.5.1' | kopium -Af -
// WARNING: generated by kopium - manual changes will be overwritten
Error: not handling non-string enum

I found the line here. Looks like there's just more TODO here to add support? Is there already work tracking this TODO?

@clux
Copy link
Member

clux commented Nov 8, 2022

Hey there. Thanks for the issue. I originally thought this was a duplicate of #39, since non-string enums will be described in oneOf sections for more complicated enums. However, go does not tend to use complicated enums as it's not in the language.

I think the problem here is the statusCode using an int enum:

                                    statusCode:
                                      default: 302
                                      description: "StatusCode is the HTTP status
                                        code to be used in response. \n Support: Core
                                        \n Note that values may be added to this enum,
                                        implementations must ensure that unknown values
                                        will not cause a crash. \n Unknown values
                                        here must result in the implementation setting
                                        the Accepted Condition for the Route to `status:
                                        False`, with a Reason of `UnsupportedValue`."
                                      enum:
                                      - 301
                                      - 302
                                      type: integer

and i think we could add support for that before we add full complex-enum support, so going to keep this issue open.

Initial thoughts on how to handle this:

  • parse struct differently if we find integer enum
  • need to store the int value as the member's discriminant
  • extend Member to carry the optional discriminants
  • unit test for the statusCode above could be added in analyzer
  • serialize discriminants in enums in main when found

It might not be too bad. Feel free to take a stab at it if you want, otherwise i'll get to it eventually™

@clux clux added good first issue Good for newcomers help wanted Extra attention is needed and removed good first issue Good for newcomers help wanted Extra attention is needed labels Nov 8, 2022
@clux clux self-assigned this Nov 11, 2022
@clux clux linked a pull request Nov 11, 2022 that will close this issue
@clux clux closed this as completed in #100 Nov 11, 2022
@shaneutt
Copy link
Author

shaneutt commented Nov 11, 2022

Built from main and tested this, I can confirm it's working now with HTTPRoute. Thank you @clux! 🖖

@clux
Copy link
Member

clux commented Nov 11, 2022

Hehe excellent. I'll put it into a proper kopium release on the weekend or early next. Have one more fix I want in first.

@shaneutt
Copy link
Author

Hehe excellent. I'll put it into a proper kopium release on the weekend or early next. Have one more fix I want in first.

Much appreciated! Extra context: In Kubernetes SIG Network we're looking into providing official bindings for Gateway API in Rust using Kopium to generate them from the CRDs built by our existing Go library (for low maintenance Rust support). The project is at https://github.com/shaneutt/gateway-api-rs and https://crates.io/crates/gateway-api, but the hope is to move it into https://github.com/kubernetes-sigs/ soon. This was the only hard blocker I ran into so far, so again thank you very much for knocking it out.

@clux
Copy link
Member

clux commented Nov 11, 2022

Woah, that is really cool! I'll keep an eye on that one, and if it makes it in will definitely link to it in the README!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants