Skip to content

Commit 8b922c9

Browse files
committed
Merge remote-tracking branch 'origin/main' into 70-implement-multi-tenant-search-support
2 parents fe72506 + 0bb17e8 commit 8b922c9

File tree

3 files changed

+383
-16
lines changed

3 files changed

+383
-16
lines changed

cmd/search/main.go

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"os"
7+
"strings"
78
"time"
89

910
"github.com/google/uuid"
@@ -25,6 +26,7 @@ import (
2526
"k8s.io/component-base/logs"
2627
logsapi "k8s.io/component-base/logs/api/v1"
2728
"k8s.io/klog/v2"
29+
openapicommon "k8s.io/kube-openapi/pkg/common"
2830
openapiutil "k8s.io/kube-openapi/pkg/util"
2931
"k8s.io/kube-openapi/pkg/validation/spec"
3032
runtimecache "sigs.k8s.io/controller-runtime/pkg/cache"
@@ -182,29 +184,56 @@ func (o *SearchServerOptions) Config() (*searchapiserver.Config, error) {
182184
// Set effective version to match the Kubernetes version we're built against.
183185
genericConfig.EffectiveVersion = basecompatibility.NewEffectiveVersionFromString("1.35", "", "")
184186

187+
// Configure OpenAPI for SSA compatibility.
188+
//
189+
// The DefinitionNamer uses scheme.ToOpenAPIDefinitionName() which returns
190+
// REST-friendly names. GVK extensions are only returned when the name matches
191+
// what's in DefinitionNamer's internal map.
192+
//
193+
// The OpenAPI builder uses reflection to get type names (Go module paths like
194+
// "go.miloapis.net/search/...") and looks them up in the definitions map. So
195+
// definition keys must remain in Go module path format for lookups to work.
196+
//
197+
// Our approach:
198+
// 1. GetOpenAPIDefinitionsWithUnstructured keeps keys in Go module path format
199+
// 2. Custom GetDefinitionName transforms Go module paths to REST-friendly format
200+
// before looking up in DefinitionNamer (to get GVK extensions)
201+
// 3. Re-compute Definitions so $refs use REST-friendly names matching the
202+
// definition names, which is what SSA TypeConverter expects
185203
namer := apiopenapi.NewDefinitionNamer(searchapiserver.Scheme)
204+
205+
// Custom GetDefinitionName that transforms Go module paths to REST-friendly format
206+
// before looking up in DefinitionNamer. This ensures GVK extensions are returned.
207+
getDefinitionName := func(name string) (string, spec.Extensions) {
208+
if strings.Contains(name, "/") {
209+
name = openapiutil.ToRESTFriendlyName(name)
210+
}
211+
return namer.GetDefinitionName(name)
212+
}
213+
214+
// OpenAPI v3 config
186215
genericConfig.OpenAPIV3Config = genericapiserver.DefaultOpenAPIV3Config(openapi.GetOpenAPIDefinitionsWithUnstructured, namer)
187216
genericConfig.OpenAPIV3Config.Info.Title = "Search"
188217
genericConfig.OpenAPIV3Config.Info.Version = version.Version
189-
genericConfig.OpenAPIV3Config.GetDefinitionName = func(name string) (string, spec.Extensions) {
190-
friendlyName, extensions := namer.GetDefinitionName(name)
191-
return openapiutil.ToRESTFriendlyName(friendlyName), extensions
192-
}
193-
// Nil out the pre-computed Definitions map so the OpenAPI builder re-invokes
194-
// GetDefinitions with a fresh ref callback that uses the updated GetDefinitionName
195-
// above. Without this, the builder uses the pre-computed $refs (which were built
196-
// before GetDefinitionName was overridden) and the schema component keys diverge
197-
// from the $ref values in the generated spec.
198-
genericConfig.OpenAPIV3Config.Definitions = nil
199-
200-
// Configure OpenAPI v2
218+
genericConfig.OpenAPIV3Config.GetDefinitionName = getDefinitionName
219+
// Re-compute Definitions with our custom getDefinitionName so $refs use REST-friendly names.
220+
// DefaultOpenAPIV3Config computes Definitions with the original namer, but we need refs
221+
// to match our transformed definition names for SSA TypeConverter lookups.
222+
genericConfig.OpenAPIV3Config.Definitions = openapi.GetOpenAPIDefinitionsWithUnstructured(func(name string) spec.Ref {
223+
defName, _ := getDefinitionName(name)
224+
return spec.MustCreateRef("#/components/schemas/" + openapicommon.EscapeJsonPointer(defName))
225+
})
226+
227+
// OpenAPI v2 config
201228
genericConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(openapi.GetOpenAPIDefinitionsWithUnstructured, namer)
202229
genericConfig.OpenAPIConfig.Info.Title = "Search"
203230
genericConfig.OpenAPIConfig.Info.Version = version.Version
204-
genericConfig.OpenAPIConfig.GetDefinitionName = func(name string) (string, spec.Extensions) {
205-
friendlyName, extensions := namer.GetDefinitionName(name)
206-
return openapiutil.ToRESTFriendlyName(friendlyName), extensions
207-
}
231+
genericConfig.OpenAPIConfig.GetDefinitionName = getDefinitionName
232+
// Re-compute Definitions for v2 as well
233+
genericConfig.OpenAPIConfig.Definitions = openapi.GetOpenAPIDefinitionsWithUnstructured(func(name string) spec.Ref {
234+
defName, _ := getDefinitionName(name)
235+
return spec.MustCreateRef("#/definitions/" + openapicommon.EscapeJsonPointer(defName))
236+
})
208237

209238
if err := o.RecommendedOptions.ApplyTo(genericConfig); err != nil {
210239
return nil, fmt.Errorf("failed to apply recommended options: %w", err)

cmd/search/openapi_test.go

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
package main
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
apiopenapi "k8s.io/apiserver/pkg/endpoints/openapi"
8+
openapicommon "k8s.io/kube-openapi/pkg/common"
9+
openapiutil "k8s.io/kube-openapi/pkg/util"
10+
"k8s.io/kube-openapi/pkg/validation/spec"
11+
12+
searchapiserver "go.miloapis.net/search/internal/apiserver"
13+
"go.miloapis.net/search/pkg/generated/openapi"
14+
)
15+
16+
// TestOpenAPIGVKExtensions verifies that OpenAPI schemas include x-kubernetes-group-version-kind
17+
// extensions required for Server-Side Apply (SSA) to work correctly.
18+
//
19+
// This is a regression test for the SSA failure:
20+
// "no corresponding type for search.miloapis.com/v1alpha1, Kind=ResourceIndexPolicy"
21+
//
22+
// Root cause: The DefinitionNamer only returns GVK extensions when looking up names
23+
// in REST-friendly format. The OpenAPI builder uses reflection to get Go module paths,
24+
// so we need GetDefinitionName to transform Go module paths before DefinitionNamer lookup.
25+
func TestOpenAPIGVKExtensions(t *testing.T) {
26+
namer := apiopenapi.NewDefinitionNamer(searchapiserver.Scheme)
27+
28+
// Custom GetDefinitionName that transforms Go module paths to REST-friendly format
29+
// before looking up in DefinitionNamer. This ensures GVK extensions are returned.
30+
getDefinitionName := func(name string) (string, spec.Extensions) {
31+
if strings.Contains(name, "/") {
32+
name = openapiutil.ToRESTFriendlyName(name)
33+
}
34+
return namer.GetDefinitionName(name)
35+
}
36+
37+
defs := openapi.GetOpenAPIDefinitionsWithUnstructured(func(path string) spec.Ref {
38+
return spec.Ref{}
39+
})
40+
41+
testCases := []struct {
42+
goModulePath string
43+
expectedGroup string
44+
expectedKind string
45+
}{
46+
{
47+
goModulePath: "go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceIndexPolicy",
48+
expectedGroup: "search.miloapis.com",
49+
expectedKind: "ResourceIndexPolicy",
50+
},
51+
{
52+
goModulePath: "go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceIndexPolicyList",
53+
expectedGroup: "search.miloapis.com",
54+
expectedKind: "ResourceIndexPolicyList",
55+
},
56+
{
57+
goModulePath: "go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceSearchQuery",
58+
expectedGroup: "search.miloapis.com",
59+
expectedKind: "ResourceSearchQuery",
60+
},
61+
{
62+
goModulePath: "go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceSearchQueryList",
63+
expectedGroup: "search.miloapis.com",
64+
expectedKind: "ResourceSearchQueryList",
65+
},
66+
}
67+
68+
for _, tc := range testCases {
69+
t.Run(tc.goModulePath, func(t *testing.T) {
70+
// Verify the definition exists with Go module path key
71+
if _, ok := defs[tc.goModulePath]; !ok {
72+
t.Fatalf("Type %q not found in OpenAPI definitions", tc.goModulePath)
73+
}
74+
75+
// Verify GetDefinitionName returns GVK extensions after transformation
76+
defName, extensions := getDefinitionName(tc.goModulePath)
77+
if extensions == nil {
78+
t.Fatalf("No extensions returned for %q (transformed to %q) - GVK extension missing! "+
79+
"This will cause SSA to fail with 'no corresponding type' error", tc.goModulePath, defName)
80+
}
81+
82+
gvkExt, ok := extensions["x-kubernetes-group-version-kind"]
83+
if !ok {
84+
t.Fatalf("x-kubernetes-group-version-kind extension not found for %q", tc.goModulePath)
85+
}
86+
87+
gvks, ok := gvkExt.([]any)
88+
if !ok {
89+
t.Fatalf("GVK extension is not an array: %T", gvkExt)
90+
}
91+
92+
found := false
93+
for _, gvk := range gvks {
94+
gvkMap, ok := gvk.(map[string]any)
95+
if !ok {
96+
continue
97+
}
98+
if gvkMap["group"] == tc.expectedGroup &&
99+
gvkMap["version"] == "v1alpha1" &&
100+
gvkMap["kind"] == tc.expectedKind {
101+
found = true
102+
break
103+
}
104+
}
105+
106+
if !found {
107+
t.Errorf("Expected GVK {group: %q, version: v1alpha1, kind: %q} not found in extensions: %v",
108+
tc.expectedGroup, tc.expectedKind, gvkExt)
109+
}
110+
})
111+
}
112+
}
113+
114+
// TestUnstructuredTypeIncluded verifies that the Unstructured type is included
115+
// in the definitions (required for SearchResult which embeds Unstructured).
116+
func TestUnstructuredTypeIncluded(t *testing.T) {
117+
defs := openapi.GetOpenAPIDefinitionsWithUnstructured(func(path string) spec.Ref {
118+
return spec.Ref{}
119+
})
120+
121+
unstructuredKey := "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured.Unstructured"
122+
if _, ok := defs[unstructuredKey]; !ok {
123+
t.Errorf("Unstructured type %q not found in definitions", unstructuredKey)
124+
}
125+
}
126+
127+
// TestOldGetDefinitionNameMissesGVK demonstrates that the old approach (passing
128+
// Go module paths directly to DefinitionNamer without transformation) fails to
129+
// return GVK extensions. This proves the fix is necessary.
130+
func TestOldGetDefinitionNameMissesGVK(t *testing.T) {
131+
namer := apiopenapi.NewDefinitionNamer(searchapiserver.Scheme)
132+
133+
// The OLD approach: pass the Go module path directly to namer without
134+
// transforming to REST-friendly format first. This was the pre-fix behavior
135+
// where GetDefinitionName did: namer.GetDefinitionName(name) -> ToRESTFriendlyName()
136+
// instead of: ToRESTFriendlyName(name) -> namer.GetDefinitionName()
137+
goModulePath := "go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceIndexPolicy"
138+
139+
// DefinitionNamer's internal map uses REST-friendly names (from scheme), so
140+
// looking up a Go module path directly returns nil extensions
141+
_, extensions := namer.GetDefinitionName(goModulePath)
142+
if extensions != nil {
143+
if _, ok := extensions["x-kubernetes-group-version-kind"]; ok {
144+
t.Fatal("Expected the old approach to NOT return GVK extensions, but it did. " +
145+
"If this passes, the old code was actually fine and the fix is unnecessary.")
146+
}
147+
}
148+
149+
// Now verify the fixed approach DOES return extensions
150+
fixedName := openapiutil.ToRESTFriendlyName(goModulePath)
151+
_, fixedExtensions := namer.GetDefinitionName(fixedName)
152+
if fixedExtensions == nil {
153+
t.Fatal("Fixed approach should return extensions")
154+
}
155+
if _, ok := fixedExtensions["x-kubernetes-group-version-kind"]; !ok {
156+
t.Fatal("Fixed approach should return x-kubernetes-group-version-kind extension")
157+
}
158+
}
159+
160+
// TestOpenAPIV3RefConsistency verifies that $refs in OpenAPI definitions use
161+
// REST-friendly names that match the definition names produced by GetDefinitionName.
162+
// When these diverge, SSA's TypeConverter cannot resolve types.
163+
func TestOpenAPIV3RefConsistency(t *testing.T) {
164+
namer := apiopenapi.NewDefinitionNamer(searchapiserver.Scheme)
165+
166+
getDefinitionName := func(name string) (string, spec.Extensions) {
167+
if strings.Contains(name, "/") {
168+
name = openapiutil.ToRESTFriendlyName(name)
169+
}
170+
return namer.GetDefinitionName(name)
171+
}
172+
173+
// Build definitions the same way the server should — refs use getDefinitionName
174+
defs := openapi.GetOpenAPIDefinitionsWithUnstructured(func(name string) spec.Ref {
175+
defName, _ := getDefinitionName(name)
176+
return spec.MustCreateRef("#/components/schemas/" + openapicommon.EscapeJsonPointer(defName))
177+
})
178+
179+
// Check that all Search types are present with Go module path keys
180+
searchTypes := []string{
181+
"go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceIndexPolicy",
182+
"go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceIndexPolicyList",
183+
"go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceSearchQuery",
184+
"go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceSearchQueryList",
185+
}
186+
187+
for _, typeName := range searchTypes {
188+
if _, ok := defs[typeName]; !ok {
189+
keys := make([]string, 0, len(defs))
190+
for k := range defs {
191+
if strings.Contains(k, "search") {
192+
keys = append(keys, k)
193+
}
194+
}
195+
t.Errorf("Type %q not found in definitions. Search-related keys: %v", typeName, keys)
196+
}
197+
}
198+
199+
// Verify that $refs within definitions resolve to valid definition names.
200+
// Pick ResourceIndexPolicy which references ResourceIndexPolicySpec etc.
201+
policyDef := defs["go.miloapis.net/search/pkg/apis/search/v1alpha1.ResourceIndexPolicy"]
202+
for propName, prop := range policyDef.Schema.Properties {
203+
if prop.Ref.String() != "" {
204+
ref := prop.Ref.String()
205+
// The ref should use REST-friendly naming (com.miloapis... not go.miloapis.net/...)
206+
if strings.Contains(ref, "go.miloapis.net/") {
207+
t.Errorf("Property %q has $ref using Go module path format %q — "+
208+
"should use REST-friendly format for SSA compatibility", propName, ref)
209+
}
210+
}
211+
}
212+
}

0 commit comments

Comments
 (0)