OpenAPIDefinitionGetter interface is incorrect

[Airblader]

kube-openapi has two interfaces

type OpenAPIDefinitionGetter interface {
	OpenAPIDefinition() *OpenAPIDefinition
}

type OpenAPIV3DefinitionGetter interface {
	OpenAPIV3Definition() *OpenAPIDefinition
}

However, implementing this doesn't actually work, because they're incorrectly specified. The methods actually need to return the structs directly, not a pointer of the struct, for it to work. Since the generator never actually checks for the interface, but purely for the existence of the method, this goes unnoticed.

The interfaces actually should be declared as

type OpenAPIDefinitionGetter interface {
	OpenAPIDefinition() OpenAPIDefinition
}

type OpenAPIV3DefinitionGetter interface {
	OpenAPIV3Definition() OpenAPIDefinition
}

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[Jefftree]

What exactly is breaking by returning a pointer? This would be a backwards incompatible change so I'd be quite skeptical unless something is broken in a major way.

[Airblader]

As it stands, implementing the interface means your code won’t work — that is broken. We can either change the code to actually expect what the interface declares, or we can change the interface to match what the code expects, but having these things not match makes no sense IMO.

[Jefftree]

Ah I see, it looks like we have a mix of usages of the pointer and struct

kube-openapi/pkg/builder/openapi_test.go

Line 62 in 2c72e55

func (_ TestExtensionV2Schema) OpenAPIDefinition() *openapi.OpenAPIDefinition {

kube-openapi/test/integration/testdata/custom/v2.go

Line 27 in 2c72e55

func (_ Bak) OpenAPIDefinition() common.OpenAPIDefinition {

and since nothing validates the interface implementation, both tests pass 😓 . Definitely seems like a bug