azure-service-operator
azure-service-operator copied to clipboard
Fix handling of "empty" types
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:
- https://schema.management.azure.com/schemas/2019-03-01/Microsoft.Compute.Galleries.json
- GalleryProperties.Identifier
- 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
@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)
GalleryIdentifier doesn’t even have a type so that is something we can’t generate properly (I assume it is meant to be string).
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.
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.
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.
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.
Can we close this in favor of #2084 @Porges?
We still need to confirm that #2084 fixes this. I think it should but will leave this open until we can confirm.