search

Azure / azure-service-operator

Azure Service Operator allows you to create Azure resources using kubectl
https://azure.github.io/azure-service-operator/
MIT License
773 stars 199 forks source link

Handle additional property formats #2503

Open theunrepentantgeek opened 2 years ago

theunrepentantgeek commented 2 years ago

Describe the current behavior

During run of the code generator, we get warnings about unsupported property formats:

W0919 10:16:53.861756   37696 jsonast.go:339] unknown format "base64url"
W0919 10:16:53.908731   37696 jsonast.go:339] unknown format "uri"
W0919 10:17:24.732709   37696 jsonast.go:339] unknown format "byte"

Describe the improvement

We should add proper support for these formats so that our CRDs properly validate up front.

Additional context

This may technically be a breaking change, as we'll be rejecting values previously accepted - but given they would then be rejected by ARM, I don't think this will negatively impact any current users.

PawelHaracz commented 1 year ago

@matthchr can I take it and do it? should I know something more about these fields?

theunrepentantgeek commented 1 year ago

You're welcome to take this issue and do it.

We already handle a bunch of formats - typically, a format gets translated into an underlying type (e.g. IntType) and some validation rules. For example, byte probably becomes IntType + Min: 0 + Max: 255

PawelHaracz commented 1 year ago

@theunrepentantgeek can you help me and tell me where or how I can generate a config file for the code generator? or can you attach some an example?

theunrepentantgeek commented 1 year ago

Apologies @PawelHaracz, somehow I missed your message. 😢

If you're still interested (no problem if you're not), I'd suggest reusing the existing configuration file for the generator - v2/azure-arm.yaml. The commandline to run the generator would look like this:

$ generator gen-types azure-arm.yaml

When the data format is handled, you should see a few small changes in the generated code under v2/api.

PawelHaracz commented 1 year ago

Thank you for your respons. Of course, I want to still try to help you. I gonna try to use this clip and resolve the issue 😃

matthchr commented 1 year ago

We're still interested in doing this, although to date we haven't seen many issues with lack of this support.

matthchr commented 7 months ago

No change from above.

theunrepentantgeek commented 1 month ago

We still want this. The code has moved to line 270.