Skip to content

Commit 0ff135f

Browse files
Jefftreek8s-publishing-bot
Jefftree
authored and
k8s-publishing-bot
committed
fix aggregator path filtering to include /
Kubernetes-commit: 8373f3035ade14b3c4723aae82672709fba6d372
1 parent 0484f16 commit 0ff135f

File tree

2 files changed

+47
-45
lines changed

2 files changed

+47
-45
lines changed

‎pkg/controllers/openapi/aggregator/aggregator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func (s *specAggregator) updateServiceLocked(name string) error {
199199
}
200200
group := specInfo.apiService.Spec.Group
201201
version := specInfo.apiService.Spec.Version
202-
return aggregator.FilterSpecByPathsWithoutSideEffects(result, []string{"/apis/" + group + "/" + version}), etag, nil
202+
return aggregator.FilterSpecByPathsWithoutSideEffects(result, []string{"/apis/" + group + "/" + version + "/"}), etag, nil
203203
}, cached.Result[*spec.Swagger]{Value: result, Etag: etag, Err: err})
204204
specInfo.spec.Store(filteredResult)
205205
return err

‎pkg/controllers/openapi/aggregator/aggregator_test.go

Lines changed: 46 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func TestBasicPathsMerged(t *testing.T) {
3939
SwaggerProps: spec.SwaggerProps{
4040
Paths: &spec.Paths{
4141
Paths: map[string]spec.PathItem{
42-
"/apis/foo/v1": {},
42+
"/apis/foo/v1/": {},
4343
},
4444
},
4545
},
@@ -52,8 +52,8 @@ func TestBasicPathsMerged(t *testing.T) {
5252
if err != nil {
5353
t.Error(err)
5454
}
55-
expectPath(t, swagger, "/apis/foo/v1")
56-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
55+
expectPath(t, swagger, "/apis/foo/v1/")
56+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
5757
}
5858

5959
func TestAddUpdateAPIService(t *testing.T) {
@@ -103,7 +103,7 @@ func TestAddUpdateAPIService(t *testing.T) {
103103
}
104104

105105
expectPath(t, swagger, "/apis/apiservicegroup/v1/path1")
106-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
106+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
107107

108108
t.Log("Update APIService OpenAPI")
109109
handler.openapi = &spec.Swagger{
@@ -127,7 +127,7 @@ func TestAddUpdateAPIService(t *testing.T) {
127127
// aggregated OpenAPI is also updated.
128128
expectPath(t, swagger, "/apis/apiservicegroup/v1/path2")
129129
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/path1")
130-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
130+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
131131
}
132132

133133
// Tests that an APIService that registers OpenAPI will only have the OpenAPI
@@ -140,7 +140,7 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
140140
SwaggerProps: spec.SwaggerProps{
141141
Paths: &spec.Paths{
142142
Paths: map[string]spec.PathItem{
143-
"/apis/foo/v1": {},
143+
"/apis/foo/v1/": {},
144144
},
145145
},
146146
},
@@ -171,7 +171,8 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
171171
SwaggerProps: spec.SwaggerProps{
172172
Paths: &spec.Paths{
173173
Paths: map[string]spec.PathItem{
174-
"/apis/apiservicegroup/v1": {},
174+
"/apis/apiservicegroup/v1/": {},
175+
"/apis/apiservicegroup/v1beta1/": {},
175176
},
176177
},
177178
},
@@ -181,9 +182,9 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
181182
SwaggerProps: spec.SwaggerProps{
182183
Paths: &spec.Paths{
183184
Paths: map[string]spec.PathItem{
184-
"/apis/a": {},
185-
"/apis/apiservicegroup/v1": {},
186-
"/apis/apiservicegroup/v2": {},
185+
"/apis/a/": {},
186+
"/apis/apiservicegroup/v1/": {},
187+
"/apis/apiservicegroup/v2/": {},
187188
},
188189
},
189190
},
@@ -207,10 +208,11 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
207208
if err != nil {
208209
t.Error(err)
209210
}
210-
expectPath(t, swagger, "/apis/apiservicegroup/v1")
211-
expectPath(t, swagger, "/apis/apiservicegroup/v2")
212-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
213-
expectNoPath(t, swagger, "/apis/a")
211+
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
212+
expectPath(t, swagger, "/apis/apiservicegroup/v2/")
213+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
214+
expectNoPath(t, swagger, "/apis/a/")
215+
expectNoPath(t, swagger, "/apis/apiservicegroup/v1beta1/")
214216

215217
t.Logf("Remove APIService %s", apiService.Name)
216218
s.RemoveAPIService(apiService.Name)
@@ -221,7 +223,7 @@ func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) {
221223
}
222224
// Ensure that the if the APIService is added then removed, the OpenAPI disappears from the aggregated OpenAPI as well.
223225
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
224-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
226+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
225227
expectNoPath(t, swagger, "/apis/a")
226228
}
227229

@@ -232,7 +234,7 @@ func TestAddRemoveAPIService(t *testing.T) {
232234
SwaggerProps: spec.SwaggerProps{
233235
Paths: &spec.Paths{
234236
Paths: map[string]spec.PathItem{
235-
"/apis/foo/v1": {},
237+
"/apis/foo/v1/": {},
236238
},
237239
},
238240
},
@@ -254,7 +256,7 @@ func TestAddRemoveAPIService(t *testing.T) {
254256
SwaggerProps: spec.SwaggerProps{
255257
Paths: &spec.Paths{
256258
Paths: map[string]spec.PathItem{
257-
"/apis/apiservicegroup/v1": {},
259+
"/apis/apiservicegroup/v1/": {},
258260
},
259261
},
260262
},
@@ -271,8 +273,8 @@ func TestAddRemoveAPIService(t *testing.T) {
271273
if err != nil {
272274
t.Error(err)
273275
}
274-
expectPath(t, swagger, "/apis/apiservicegroup/v1")
275-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
276+
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
277+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
276278

277279
t.Logf("Remove APIService %s", apiService.Name)
278280
s.RemoveAPIService(apiService.Name)
@@ -282,8 +284,8 @@ func TestAddRemoveAPIService(t *testing.T) {
282284
t.Error(err)
283285
}
284286
// Ensure that the if the APIService is added then removed, the OpenAPI disappears from the aggregated OpenAPI as well.
285-
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
286-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
287+
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/")
288+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
287289
}
288290

289291
func TestUpdateAPIService(t *testing.T) {
@@ -293,7 +295,7 @@ func TestUpdateAPIService(t *testing.T) {
293295
SwaggerProps: spec.SwaggerProps{
294296
Paths: &spec.Paths{
295297
Paths: map[string]spec.PathItem{
296-
"/apis/foo/v1": {},
298+
"/apis/foo/v1/": {},
297299
},
298300
},
299301
},
@@ -315,7 +317,7 @@ func TestUpdateAPIService(t *testing.T) {
315317
SwaggerProps: spec.SwaggerProps{
316318
Paths: &spec.Paths{
317319
Paths: map[string]spec.PathItem{
318-
"/apis/apiservicegroup/v1": {},
320+
"/apis/apiservicegroup/v1/": {},
319321
},
320322
},
321323
},
@@ -340,8 +342,8 @@ func TestUpdateAPIService(t *testing.T) {
340342
if err != nil {
341343
t.Error(err)
342344
}
343-
expectPath(t, swagger, "/apis/apiservicegroup/v1")
344-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
345+
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
346+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
345347

346348
t.Logf("Updating APIService %s", apiService.Name)
347349
if err := s.AddUpdateAPIService(apiService, handler2); err != nil {
@@ -356,8 +358,8 @@ func TestUpdateAPIService(t *testing.T) {
356358
t.Error(err)
357359
}
358360
// Ensure that the if the APIService is added and then handler is modified, the new data is reflected in the aggregated OpenAPI.
359-
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
360-
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1")
361+
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/")
362+
expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1/")
361363
}
362364

363365
func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
@@ -367,7 +369,7 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
367369
SwaggerProps: spec.SwaggerProps{
368370
Paths: &spec.Paths{
369371
Paths: map[string]spec.PathItem{
370-
"/apis/foo/v1": {},
372+
"/apis/foo/v1/": {},
371373
},
372374
},
373375
},
@@ -391,7 +393,7 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
391393
SwaggerProps: spec.SwaggerProps{
392394
Paths: &spec.Paths{
393395
Paths: map[string]spec.PathItem{
394-
"/apis/failed/v1": {},
396+
"/apis/failed/v1/": {},
395397
},
396398
},
397399
},
@@ -412,7 +414,7 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
412414
SwaggerProps: spec.SwaggerProps{
413415
Paths: &spec.Paths{
414416
Paths: map[string]spec.PathItem{
415-
"/apis/success/v1": {},
417+
"/apis/success/v1/": {},
416418
},
417419
},
418420
},
@@ -437,9 +439,9 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) {
437439
if err != nil {
438440
t.Error(err)
439441
}
440-
expectPath(t, swagger, "/apis/foo/v1")
441-
expectNoPath(t, swagger, "/apis/failed/v1")
442-
expectPath(t, swagger, "/apis/success/v1")
442+
expectPath(t, swagger, "/apis/foo/v1/")
443+
expectNoPath(t, swagger, "/apis/failed/v1/")
444+
expectPath(t, swagger, "/apis/success/v1/")
443445
}
444446

445447
func TestAPIServiceFailSuccessTransition(t *testing.T) {
@@ -449,7 +451,7 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
449451
SwaggerProps: spec.SwaggerProps{
450452
Paths: &spec.Paths{
451453
Paths: map[string]spec.PathItem{
452-
"/apis/foo/v1": {},
454+
"/apis/foo/v1/": {},
453455
},
454456
},
455457
},
@@ -473,7 +475,7 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
473475
SwaggerProps: spec.SwaggerProps{
474476
Paths: &spec.Paths{
475477
Paths: map[string]spec.PathItem{
476-
"/apis/apiservicegroup/v1": {},
478+
"/apis/apiservicegroup/v1/": {},
477479
},
478480
},
479481
},
@@ -491,8 +493,8 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
491493
if err != nil {
492494
t.Error(err)
493495
}
494-
expectPath(t, swagger, "/apis/foo/v1")
495-
expectNoPath(t, swagger, "/apis/apiservicegroup/v1")
496+
expectPath(t, swagger, "/apis/foo/v1/")
497+
expectNoPath(t, swagger, "/apis/apiservicegroup/v1/")
496498

497499
t.Log("Transition APIService to not return error")
498500
handler.returnErr = false
@@ -504,8 +506,8 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) {
504506
if err != nil {
505507
t.Error(err)
506508
}
507-
expectPath(t, swagger, "/apis/foo/v1")
508-
expectPath(t, swagger, "/apis/apiservicegroup/v1")
509+
expectPath(t, swagger, "/apis/foo/v1/")
510+
expectPath(t, swagger, "/apis/apiservicegroup/v1/")
509511
}
510512

511513
func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
@@ -515,7 +517,7 @@ func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
515517
SwaggerProps: spec.SwaggerProps{
516518
Paths: &spec.Paths{
517519
Paths: map[string]spec.PathItem{
518-
"/apis/foo/v1": {},
520+
"/apis/foo/v1/": {},
519521
},
520522
},
521523
},
@@ -542,7 +544,7 @@ func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
542544
SwaggerProps: spec.SwaggerProps{
543545
Paths: &spec.Paths{
544546
Paths: map[string]spec.PathItem{
545-
"/apis/failed/v1": {},
547+
"/apis/failed/v1/": {},
546548
},
547549
},
548550
},
@@ -567,8 +569,8 @@ func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) {
567569
if err != nil {
568570
t.Error(err)
569571
}
570-
expectPath(t, swagger, "/apis/foo/v1")
571-
expectNoPath(t, swagger, "/apis/failed/v1")
572+
expectPath(t, swagger, "/apis/foo/v1/")
573+
expectNoPath(t, swagger, "/apis/failed/v1/")
572574
}
573575

574576
type openAPIHandler struct {
@@ -619,7 +621,7 @@ func buildAndRegisterSpecAggregator(delegationHandlers []http.Handler, mux commo
619621
SwaggerProps: spec.SwaggerProps{
620622
Paths: &spec.Paths{
621623
Paths: map[string]spec.PathItem{
622-
"/apis/apiregistration.k8s.io/v1": {},
624+
"/apis/apiregistration.k8s.io/v1/": {},
623625
},
624626
},
625627
},

0 commit comments

Comments
 (0)