Incorrect Descriptions are generated for an Aliased Type defined in multiple API versions #292

shashankram · 2025-05-12T14:51:01Z

gengo produces flaking descriptions for the same type that is defined in multiple pkgs (using an alias), when used with openapi-gen (from kube-openapi).

In v1alpha2:

// BackendRef defines how a Route should forward a request to a Kubernetes
// resource.
//
// Note that when a namespace different than the local namespace is specified, a
// ReferenceGrant object is required in the referent namespace to allow that
// namespace's owner to accept the reference. See the ReferenceGrant
// documentation for details.
// +k8s:deepcopy-gen=false
type BackendRef = v1.BackendRef

In v1:

// BackendRef defines how a Route should forward a request to a Kubernetes
// resource.
//
// Note that when a namespace different than the local namespace is specified, a
// ReferenceGrant object is required in the referent namespace to allow that
// namespace's owner to accept the reference. See the ReferenceGrant
// documentation for details.
//
// <gateway:experimental:description>
//
// When the BackendRef points to a Kubernetes Service, implementations SHOULD
// honor the appProtocol field if it is set for the target Service Port.
//
// Implementations supporting appProtocol SHOULD recognize the Kubernetes
// Standard Application Protocols defined in KEP-3726.
//
// If a Service appProtocol isn't specified, an implementation MAY infer the
// backend protocol through its own means. Implementations MAY infer the
// protocol from the Route type referring to the backend Service.
//
// If a Route is not able to send traffic to the backend using the specified
// protocol then the backend is considered invalid. Implementations MUST set the
// "ResolvedRefs" condition to "False" with the "UnsupportedProtocol" reason.
//
// </gateway:experimental:description>
//
// Note that when the BackendTLSPolicy object is enabled by the implementation,
// there are some extra rules about validity to consider here. See the fields
// where this struct is used for more information about the exact behavior.
type BackendRef struct {

The following command generates different Descriptions for the same type in zz_generated.openapi.go:

go tool openapi-gen --output-file zz_generated.openapi.go --report-filename /tmp/generate.sh.api_violations.CUlQnn --output-dir /home/runner/work/kgateway/kgateway/pkg/generated/openapi --output-pkg github.com/kgateway-dev/kgateway/v2/pkg/generated/openapi github.com/kgateway-dev/kgateway/v2/api/v1alpha1 sigs.k8s.io/gateway-api/apis/v1 sigs.k8s.io/gateway-api/apis/v1alpha2 k8s.io/apimachinery/pkg/apis/meta/v1 k8s.io/api/core/v1 k8s.io/apimachinery/pkg/runtime k8s.io/apimachinery/pkg/util/intstr k8s.io/apimachinery/pkg/api/resource k8s.io/apimachinery/pkg/version

The diff:

diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go
index 52786b7..8a7e159 100644
--- a/pkg/generated/openapi/zz_generated.openapi.go
+++ b/pkg/generated/openapi/zz_generated.openapi.go
@@ -20767,7 +20767,7 @@ func schema_sigsk8sio_gateway_api_apis_v1_BackendRef(ref common.ReferenceCallbac
 	return common.OpenAPIDefinition{
 		Schema: spec.Schema{
 			SchemaProps: spec.SchemaProps{
-				Description: "BackendRef defines how a Route should forward a request to a Kubernetes resource.\n\nNote that when a namespace different than the local namespace is specified, a ReferenceGrant object is required in the referent namespace to allow that namespace's owner to accept the reference. See the ReferenceGrant documentation for details.",
+				Description: "BackendRef defines how a Route should forward a request to a Kubernetes resource.\n\nNote that when a namespace different than the local namespace is specified, a ReferenceGrant object is required in the referent namespace to allow that namespace's owner to accept the reference. See the ReferenceGrant documentation for details.\n\n<gateway:experimental:description>\n\nWhen the BackendRef points to a Kubernetes Service, implementations SHOULD honor the appProtocol field if it is set for the target Service Port.\n\nImplementations supporting appProtocol SHOULD recognize the Kubernetes Standard Application Protocols defined in KEP-3726.\n\nIf a Service appProtocol isn't specified, an implementation MAY infer the backend protocol through its own means. Implementations MAY infer the protocol from the Route type referring to the backend Service.\n\nIf a Route is not able to send traffic to the backend using the specified protocol then the backend is considered invalid. Implementations MUST set the \"ResolvedRefs\" condition to \"False\" with the \"UnsupportedProtocol\" reason.\n\n</gateway:experimental:description>\n\nNote that when the BackendTLSPolicy object is enabled by the implementation, there are some extra rules about validity to consider here. See the fields where this struct is used for more information about the exact behavior.",
 				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
 					"group": {
@@ -23242,7 +23242,7 @@ func schema_sigsk8sio_gateway_api_apis_v1_ParentReference(ref common.ReferenceCa
 	return common.OpenAPIDefinition{
 		Schema: spec.Schema{
 			SchemaProps: spec.SchemaProps{
-				Description: "ParentReference identifies an API object (usually a Gateway) that can be considered a parent of this resource (usually a route). The only kind of parent resource with \"Core\" support is Gateway. This API may be extended in the future to support additional kinds of parent resources, such as HTTPRoute.\n\nNote that there are specific rules for ParentRefs which cross namespace boundaries. Cross-namespace references are only valid if they are explicitly allowed by something in the namespace they are referring to. For example: Gateway has the AllowedRoutes field, and ReferenceGrant provides a generic way to enable any other kind of cross-namespace reference.\n\nThe API object must be valid in the cluster; the Group and Kind must be registered in the cluster for this reference to be valid.",
+				Description: "ParentReference identifies an API object (usually a Gateway) that can be considered a parent of this resource (usually a route). There are two kinds of parent resources with \"Core\" support:\n\n* Gateway (Gateway conformance profile) * Service (Mesh conformance profile, ClusterIP Services only)\n\nThis API may be extended in the future to support additional kinds of parent resources.\n\nThe API object must be valid in the cluster; the Group and Kind must be registered in the cluster for this reference to be valid.",
 				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
 					"group": {

The description being generated switches between the descriptions defined in the v1 and v1alph2 versions of the API. I am not sure if this is a bug in openapi-gen or gengo.

The text was updated successfully, but these errors were encountered:

shashankram · 2025-05-12T14:51:08Z

/sig api-machinery

shashankram · 2025-05-12T15:51:35Z

/cc @Jefftree @jpbetz @aaron-prindle @yongruilin from kubernetes/kube-openapi#535

shashankram · 2025-05-12T15:54:59Z

I root caused this to gengo overwriting the parsed CommentLines from the actual Type when processing an aliased Type later with a different description:

gengo/v2/parser/parse.go

Line 491 in 1244d31

p.addCommentsToType(obj, t)

The following line overwrites the comments corresponding when it encounters an aliased type with a different comment:

			t := p.walkType(*u, nil, obj.Type())
			p.addCommentsToType(obj, t)

Not sure why the generation of Type descriptions is non-deterministic though (gengo produces descriptions from v1 sometimes and from v1alpha2 otherwise) when UserRequestedPackages() returns a sorted list.

shashankram · 2025-05-12T19:08:42Z

So this is a result of calling walkAliasType on an aliased type and then updating the original/unaliased type's comments with the aliased obj:

func (p *Parser) walkType(u types.Universe, useName *types.Name, in gotypes.Type) *types.Type {
---
	if out := p.walkAliasType(u, in); out != nil {
		return out
	}

func (p *Parser) addPkgToUniverse(pkg *packages.Package, u *types.Universe) error {
---
			t := p.walkType(*u, nil, obj.Type())
			p.addCommentsToType(obj, t) // this overwrites the original type's comment with the aliased type

This fixed it for me:

--- a/v2/parser/parse.go
+++ b/v2/parser/parse.go
@@ -385,6 +385,12 @@ func (p *Parser) NewUniverse() (types.Universe, error) {
 // addCommentsToType takes any accumulated comment lines prior to obj and
 // attaches them to the type t.
 func (p *Parser) addCommentsToType(obj gotypes.Object, t *types.Type) {
+       if _, isAlias := obj.Type().(*gotypes.Alias); isAlias {
+               // Don't overwrite the parsed comments on 't' if 'obj' is an alias
+               // as it would overwrite the comments on the original type 't' with
+               // the comments on the aliased type.
+               return
+       }
        t.CommentLines = p.docComment(obj.Pos())
        t.SecondClosestCommentLines = p.priorDetachedComment(obj.Pos())
 }

cc @liggitt (ref: #281)

yongruilin · 2025-05-12T19:32:32Z

Thanks for reporting the issue. Feel free to open the PR for the fix you proposed.

shashankram · 2025-05-12T23:05:37Z

Thanks for reporting the issue. Feel free to open the PR for the fix you proposed.

I am unable to repro this using a unit test within the repo. Hard to tell why the aliased types are not parsed as *gotypes.Alias.

shashankram · 2025-05-13T21:31:11Z

Thanks for reporting the issue. Feel free to open the PR for the fix you proposed.

I am unable to repro this using a unit test within the repo. Hard to tell why the aliased types are not parsed as *gotypes.Alias.

I am able to repro this with a test, so I'll open a PR.

yongruilin · 2025-05-13T21:35:37Z

Cool! I believe the inconsistent comments attached is due to the recursive call of the pkg.Imports, which is a map

gengo/v2/parser/parse.go

Lines 513 to 518 in 1244d31

    
           for _, imp := range pkg.Imports { 
        
           	if err := p.addPkgToUniverse(imp, u); err != nil { 
        
           		return err 
        
           	} 
        
           	importedPkgs = append(importedPkgs, imp.PkgPath) 
        
           }

yongruilin · 2025-05-13T21:40:41Z

FYI, I'm able to reproduce on my end, PTAL yongruilin@d83797a in case you need it.

yongruilin · 2025-05-13T21:41:18Z

/assign @shashankram

Fixes the comment parsing on a Type so that it does not get overwritten by an aliased Type with a different comment. Resolves kubernetes#292 Signed-off-by: Shashank Ram <[email protected]>

Fixes the comment parsing on a Type so that it does not get overwritten by an aliased Type with a different comment. Resolves kubernetes#292 Signed-off-by: Shashank Ram <[email protected]> Signed-off-by: Shashank Ram <[email protected]>

Fixes the comment parsing on a Type so that it does not get overwritten by an aliased Type with a different comment. Resolves kubernetes#292 Signed-off-by: Shashank Ram <[email protected]>

shashankram · 2025-05-13T22:45:30Z

FYI, I'm able to reproduce on my end, PTAL yongruilin@d83797a in case you need it.

Thanks. #294 fixes this

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label May 12, 2025

shashankram marked this as a duplicate of kubernetes/kube-openapi#535 May 12, 2025

shashankram mentioned this issue May 12, 2025

Incorrect Descriptions are generated for an Aliased Type defined in multiple API versions kubernetes/kube-openapi#535

Closed

shashankram mentioned this issue May 12, 2025

[ci] Flake static analysis test in CI workflows kgateway-dev/kgateway#11166

Closed

k8s-ci-robot assigned shashankram May 13, 2025

shashankram linked a pull request May 13, 2025 that will close this issue

v2/parser: prefer original comment when evaluating aliased Type #294

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Incorrect Descriptions are generated for an Aliased Type defined in multiple API versions #292

Incorrect Descriptions are generated for an Aliased Type defined in multiple API versions #292

shashankram commented May 12, 2025

shashankram commented May 12, 2025

Uh oh!

shashankram commented May 12, 2025

Uh oh!

shashankram commented May 12, 2025 •

edited

Loading

Uh oh!

shashankram commented May 12, 2025

Uh oh!

yongruilin commented May 12, 2025

Uh oh!

shashankram commented May 12, 2025

Uh oh!

shashankram commented May 13, 2025

Uh oh!

yongruilin commented May 13, 2025

Uh oh!

yongruilin commented May 13, 2025

Uh oh!

yongruilin commented May 13, 2025

Uh oh!

shashankram commented May 13, 2025

Uh oh!

Incorrect Descriptions are generated for an Aliased Type defined in multiple API versions #292

Incorrect Descriptions are generated for an Aliased Type defined in multiple API versions #292

Comments

shashankram commented May 12, 2025

shashankram commented May 12, 2025

Uh oh!

shashankram commented May 12, 2025

Uh oh!

shashankram commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shashankram commented May 12, 2025

Uh oh!

yongruilin commented May 12, 2025

Uh oh!

shashankram commented May 12, 2025

Uh oh!

shashankram commented May 13, 2025

Uh oh!

yongruilin commented May 13, 2025

Uh oh!

yongruilin commented May 13, 2025

Uh oh!

yongruilin commented May 13, 2025

Uh oh!

shashankram commented May 13, 2025

Uh oh!

shashankram commented May 12, 2025 •

edited

Loading