Description
The issue
I'm using proto.NewOpenAPIV3Data() to parse an OpenAPI V3 spec coming from the /openapi/v3 endpoints of the apiserver.
I'd expect fields such as .metadata.ownerReferences to be a *proto.Ref that refers to the io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference schema, however they are instead returned as *proto.Arbitrary.
I'm not sure if this is the intended behavior, but I believe it should return a *proto.Ref.
Investigation
I observed a difference between the testdata for pkg/util/proto and the actual spec returned by /openapi/v3.
ownerReferences field from testdata
The $ref is directly under items.
...
"ownerReferences": {
"description": "List of objects depended by this object. If ALL objects in the list have been deleted, this object will be garbage collected. If this object is managed by a controller, then an entry in this list will point to this controller, with the controller field set to true. There cannot be more than one managing controller.",
"type": "array",
"items": {
"default": {},
"$ref": "#/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference"
},
"x-kubernetes-patch-merge-key": "uid",
"x-kubernetes-patch-strategy": "merge"
},
...ownerReferences field from k3s 1.30.0 (and 1.28.6)
The $ref is under an allOf, under items.
...
"ownerReferences": {
"description": "List of objects depended by this object. If ALL objects in the list have been deleted, this object will be garbage collected. If this object is managed by a controller, then an entry in this list will point to this controller, with the controller field set to true. There cannot be more than one managing controller.",
"type": "array",
"items": {
"default": {},
"allOf": [
{
"$ref": "#/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference"
}
]
},
"x-kubernetes-list-map-keys": [
"uid"
],
"x-kubernetes-list-type": "map",
"x-kubernetes-patch-merge-key": "uid",
"x-kubernetes-patch-strategy": "merge"
},
...Going further, this document is being parsed by gnostic's openapi_v3.ParseDocument. This results in the following different result (marshaled to JSON):
from testdata
items -> schema_or_reference -> oneof -> reference -> the ref
...
"name": "ownerReferences",
"value": {
"Oneof": {
"Schema": {
"type": "array",
"items": {
"schema_or_reference": [
{
"Oneof": {
"Reference": {
"_ref": "#/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference"
}
}
}
]
}
}
}
}
...from k3s 1.30.0
items -> schema_or_reference -> oneof -> allof ->oneof -> reference -> the ref
...
"name": "ownerReferences",
"value": {
"Oneof": {
"Schema": {
"type": "array",
"items": {
"schema_or_reference": [
{
"Oneof": {
"Schema": {
"all_of": [
{
"Oneof": {
"Reference": {
"_ref": "#/components/schemas/io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference"
}
}
}
],
"default": {
"Oneof": null
}
}
}
}
]
}
}
}
}
...Looking at the code, the first case is handled correctly. The calls are: proto.ParseSchemaV3 -> proto.parseV3Array -> proto.ParseV3SchemaOrReference -> proto.ParseV3SchemaReference -> returns a Ref.
The second case fails to parse as a ref properly because once it finds the Oneof which has type schema, so proto.ParseSchemaV3 is called. That function expects a type field to be defined, but since there isn't one, it defaults to returning an arbitrary.
Solution
From the looks of it, it seems like proto.ParseSchemaV3 should first check if there's an AllOf, before checking if there's a type. I have a small proof of concept that makes it work for my use case, though I'm not knowledgeable enough on the OpenAPI spec side to know whether that's a bullet proof fix.
Reproducing
I created a repo for easily reproducing this issue: https://github.com/tomleb/kube-openapi-issue/blob/master/main_test.go. Simply run go test -v ./.... I included a OpenAPI V3 spec from v1.30.0, v1.28.6 and from this repo's testdata.
I'd appreciate some guidance on whether this is a truly a bug (as opposed to say, misuse of the library?) and thoughts on the fix proposed above. I'd be willing to work on the fix and contribute to the repo once that's confirmed.