azure-service-operator icon indicating copy to clipboard operation
azure-service-operator copied to clipboard

Fix handling of "empty" types

Open matthchr opened this issue 5 years ago • 8 comments

There are a number of these that are due to conversion from Swagger where all of the fields of a type are read-only:

For example:

  1. https://schema.management.azure.com/schemas/2019-03-01/Microsoft.Compute.Galleries.json
    • GalleryProperties.Identifier
  2. https://schema.management.azure.com/schemas/2019-07-01/Microsoft.Compute.json
    • VirtualMachineIdentity.UserAssignedIdentities Both of the above are types that are entirely readonly

This causes at least a few errors when controller-gen runs that come out looking like this:

/home/matthchr/work/github/k8s-infra/hack/generator/apis/microsoft.compute.galleries/v20180601/galleries_types.go:75:15: unsupported AST kind *ast.InterfaceType

matthchr avatar Jun 01 '20 23:06 matthchr

@porges suggests that we could possibly limit our interpretation of what constitutes an allowable generic object to ones that are used with a field of tags or something (@matthchr note: I would've thought tags would just be a map[string]string though)

matthchr avatar Jun 01 '20 23:06 matthchr

GalleryIdentifier doesn’t even have a type so that is something we can’t generate properly (I assume it is meant to be string).

Porges avatar Jun 02 '20 03:06 Porges

It's a complex type with a single read-only property according to Swagger.

It looks like the JSON schema code generator just makes it out to be a really weird object because it's all readonly.

matthchr avatar Jun 02 '20 16:06 matthchr

This isn't exactly fixed by Azure/k8s-infra#203 but is side-stepped a little - we exclude all of the packages that generate interface{}. That lets us generate and controller-gen the other packages into something that builds successfully.

babbageclunk avatar Jul 31 '20 02:07 babbageclunk

This was un-fixed with the introduction of support for map[string]v1.JSON types. Unfortunately I am not sure how to differentiate between "this type is empty because all of its properties are readonly" and "this type is empty because we accept anything"

We may need the Swagger data.

matthchr avatar Mar 01 '21 23:03 matthchr

Moving to codegen-beta-0 milestone as we have a good-enough workaround for now (excluding the property); this may bite us later on though, so we still need to fix the issue.

theunrepentantgeek avatar Sep 22 '21 22:09 theunrepentantgeek

Can we close this in favor of #2084 @Porges?

matthchr avatar Mar 07 '22 19:03 matthchr

We still need to confirm that #2084 fixes this. I think it should but will leave this open until we can confirm.

matthchr avatar Mar 10 '22 21:03 matthchr