Merge pull request #107061 from alexzielenski/oav3-bug-incorrect-definitions-version

Fix openapi v3 bug which causes incorrect schema version to be looked up

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go

@@ -80,6 +80,7 @@ func refForOpenAPIVersion(schemaRef string, v2 bool) string {
 }
 
 var definitions map[string]common.OpenAPIDefinition
+var definitionsV3 map[string]common.OpenAPIDefinition
 var buildDefinitions sync.Once
 var namer *openapi.DefinitionNamer
 
@@ -460,26 +461,31 @@ func addEmbeddedProperties(s *spec.Schema, opts Options) {
 // getDefinition gets definition for given Kubernetes type. This function is extracted from
 // kube-openapi builder logic
 func getDefinition(name string, v2 bool) spec.Schema {
-	buildDefinitions.Do(generateBuildDefinitionsFunc(v2))
+	buildDefinitions.Do(generateBuildDefinitionsFunc)
+
+	if v2 {
 		return definitions[name].Schema
 	}
+	return definitionsV3[name].Schema
+}
 
 func withDescription(s spec.Schema, desc string) spec.Schema {
 	return *s.WithDescription(desc)
 }
 
-func generateBuildDefinitionsFunc(v2 bool) func() {
+func generateBuildDefinitionsFunc() {
-	return func() {
 	namer = openapi.NewDefinitionNamer(runtime.NewScheme())
-		definitions = utilopenapi.GetOpenAPIDefinitionsWithoutDisabledFeatures(generatedopenapi.GetOpenAPIDefinitions)(func(name string) spec.Ref {
+	definitionsV3 = utilopenapi.GetOpenAPIDefinitionsWithoutDisabledFeatures(generatedopenapi.GetOpenAPIDefinitions)(func(name string) spec.Ref {
 		defName, _ := namer.GetDefinitionName(name)
 		prefix := v3DefinitionPrefix
-			if v2 {
-				prefix = definitionPrefix
-			}
 		return spec.MustCreateRef(prefix + common.EscapeJsonPointer(defName))
 	})
-	}
+
+	definitions = utilopenapi.GetOpenAPIDefinitionsWithoutDisabledFeatures(generatedopenapi.GetOpenAPIDefinitions)(func(name string) spec.Ref {
+		defName, _ := namer.GetDefinitionName(name)
+		prefix := definitionPrefix
+		return spec.MustCreateRef(prefix + common.EscapeJsonPointer(defName))
+	})
 }
 
 // addTypeMetaProperties adds Kubernetes-specific type meta properties to input schema:
@@ -528,7 +534,7 @@ func (b *builder) getOpenAPIConfig(v2 bool) *common.Config {
 		},
 		GetOperationIDAndTags: openapi.GetOperationIDAndTags,
 		GetDefinitionName: func(name string) (string, spec.Extensions) {
-			buildDefinitions.Do(generateBuildDefinitionsFunc(v2))
+			buildDefinitions.Do(generateBuildDefinitionsFunc)
 			return namer.GetDefinitionName(name)
 		},
 		GetDefinitions: func(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition {

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go

@@ -18,6 +18,7 @@ package builder
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -802,3 +803,30 @@ func TestBuildOpenAPIV3(t *testing.T) {
 		})
 	}
 }
+
+// Tests that getDefinition's ref building function respects the v2 flag for v2
+// vs v3 operations
+// This bug did not surface since we only so far look up types which do not make
+// use of refs
+func TestGetDefinitionRefPrefix(t *testing.T) {
+	// A bug was triggered by generating the cached definition map for one version,
+	// but then performing a looking on another. The map is generated upon
+	// the first call to getDefinition
+
+	// ManagedFieldsEntry's Time field is known to use arefs
+	managedFieldsTypePath := "k8s.io/apimachinery/pkg/apis/meta/v1.ManagedFieldsEntry"
+
+	v2Ref := getDefinition(managedFieldsTypePath, true).SchemaProps.Properties["time"].SchemaProps.Ref
+	v3Ref := getDefinition(managedFieldsTypePath, false).SchemaProps.Properties["time"].SchemaProps.Ref
+
+	v2String := v2Ref.String()
+	v3String := v3Ref.String()
+
+	if !strings.HasPrefix(v3String, v3DefinitionPrefix) {
+		t.Errorf("v3 ref (%v) does not have the correct prefix (%v)", v3String, v3DefinitionPrefix)
+	}
+
+	if !strings.HasPrefix(v2String, definitionPrefix) {
+		t.Errorf("v2 ref (%v) does not have the correct prefix (%v)", v2String, definitionPrefix)
+	}
+}