From 9a404b590b1e451e8349eb9730d93167c9650fed Mon Sep 17 00:00:00 2001 From: Jefftree Date: Wed, 28 Feb 2024 23:17:40 +0000 Subject: [PATCH] Filter aggregated apiservice gv --- .../openapi/aggregator/aggregator.go | 4 +- .../openapi/aggregator/aggregator_test.go | 113 +++++++++++++++++- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator.go index c35ac49094c..b41e268b8ab 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator.go @@ -197,7 +197,9 @@ func (s *specAggregator) updateServiceLocked(name string) error { if err != nil { return nil, "", err } - return aggregator.FilterSpecByPathsWithoutSideEffects(result, []string{"/apis/"}), etag, nil + group := specInfo.apiService.Spec.Group + version := specInfo.apiService.Spec.Version + return aggregator.FilterSpecByPathsWithoutSideEffects(result, []string{"/apis/" + group + "/" + version}), etag, nil }, cached.Result[*spec.Swagger]{Value: result, Etag: etag, Err: err}) specInfo.spec.Store(filteredResult) return err diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator_test.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator_test.go index 1b366e12de1..34c586bab09 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator/aggregator_test.go @@ -73,6 +73,8 @@ func TestAddUpdateAPIService(t *testing.T) { apiService := &v1.APIService{ Spec: v1.APIServiceSpec{ + Group: "apiservicegroup", + Version: "v1", Service: &v1.ServiceReference{Name: "dummy"}, }, } @@ -82,7 +84,7 @@ func TestAddUpdateAPIService(t *testing.T) { SwaggerProps: spec.SwaggerProps{ Paths: &spec.Paths{ Paths: map[string]spec.PathItem{ - "/apis/apiservicegroup/v1": {}, + "/apis/apiservicegroup/v1/path1": {}, }, }, }, @@ -100,7 +102,7 @@ func TestAddUpdateAPIService(t *testing.T) { t.Error(err) } - expectPath(t, swagger, "/apis/apiservicegroup/v1") + expectPath(t, swagger, "/apis/apiservicegroup/v1/path1") expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1") t.Log("Update APIService OpenAPI") @@ -108,7 +110,7 @@ func TestAddUpdateAPIService(t *testing.T) { SwaggerProps: spec.SwaggerProps{ Paths: &spec.Paths{ Paths: map[string]spec.PathItem{ - "/apis/apiservicegroup/v2": {}, + "/apis/apiservicegroup/v1/path2": {}, }, }, }, @@ -123,7 +125,100 @@ func TestAddUpdateAPIService(t *testing.T) { } // Ensure that the if the APIService OpenAPI is updated, the // aggregated OpenAPI is also updated. + expectPath(t, swagger, "/apis/apiservicegroup/v1/path2") + expectNoPath(t, swagger, "/apis/apiservicegroup/v1/path1") + expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1") +} + +// Tests that an APIService that registers OpenAPI will only have the OpenAPI +// for its specific group version served registered. +// See https://github.com/kubernetes/kubernetes/pull/123570 for full context. +func TestAPIServiceOpenAPIServiceMismatch(t *testing.T) { + mux := http.NewServeMux() + var delegationHandlers []http.Handler + delegate1 := &openAPIHandler{openapi: &spec.Swagger{ + SwaggerProps: spec.SwaggerProps{ + Paths: &spec.Paths{ + Paths: map[string]spec.PathItem{ + "/apis/foo/v1": {}, + }, + }, + }, + }} + delegationHandlers = append(delegationHandlers, delegate1) + + s := buildAndRegisterSpecAggregator(delegationHandlers, mux) + + apiService := &v1.APIService{ + Spec: v1.APIServiceSpec{ + Group: "apiservicegroup", + Version: "v1", + Service: &v1.ServiceReference{Name: "dummy"}, + }, + } + apiService.Name = "apiservice" + + apiService2 := &v1.APIService{ + Spec: v1.APIServiceSpec{ + Group: "apiservicegroup", + Version: "v2", + Service: &v1.ServiceReference{Name: "dummy2"}, + }, + } + apiService2.Name = "apiservice2" + + handler := &openAPIHandler{openapi: &spec.Swagger{ + SwaggerProps: spec.SwaggerProps{ + Paths: &spec.Paths{ + Paths: map[string]spec.PathItem{ + "/apis/apiservicegroup/v1": {}, + }, + }, + }, + }} + + handler2 := &openAPIHandler{openapi: &spec.Swagger{ + SwaggerProps: spec.SwaggerProps{ + Paths: &spec.Paths{ + Paths: map[string]spec.PathItem{ + "/apis/a": {}, + "/apis/apiservicegroup/v1": {}, + "/apis/apiservicegroup/v2": {}, + }, + }, + }, + }} + + if err := s.AddUpdateAPIService(apiService, handler); err != nil { + t.Error(err) + } + if err := s.UpdateAPIServiceSpec(apiService.Name); err != nil { + t.Error(err) + } + + if err := s.AddUpdateAPIService(apiService2, handler2); err != nil { + t.Error(err) + } + if err := s.UpdateAPIServiceSpec(apiService2.Name); err != nil { + t.Error(err) + } + + swagger, err := fetchOpenAPI(mux) + if err != nil { + t.Error(err) + } + expectPath(t, swagger, "/apis/apiservicegroup/v1") expectPath(t, swagger, "/apis/apiservicegroup/v2") + expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1") + + t.Logf("Remove APIService %s", apiService.Name) + s.RemoveAPIService(apiService.Name) + + swagger, err = fetchOpenAPI(mux) + if err != nil { + t.Error(err) + } + // Ensure that the if the APIService is added then removed, the OpenAPI disappears from the aggregated OpenAPI as well. expectNoPath(t, swagger, "/apis/apiservicegroup/v1") expectPath(t, swagger, "/apis/apiregistration.k8s.io/v1") } @@ -146,6 +241,8 @@ func TestAddRemoveAPIService(t *testing.T) { apiService := &v1.APIService{ Spec: v1.APIServiceSpec{ + Group: "apiservicegroup", + Version: "v1", Service: &v1.ServiceReference{Name: "dummy"}, }, } @@ -205,6 +302,8 @@ func TestUpdateAPIService(t *testing.T) { apiService := &v1.APIService{ Spec: v1.APIServiceSpec{ + Group: "apiservicegroup", + Version: "v1", Service: &v1.ServiceReference{Name: "dummy"}, }, } @@ -277,6 +376,8 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) { apiServiceFailed := &v1.APIService{ Spec: v1.APIServiceSpec{ + Group: "failed", + Version: "v1", Service: &v1.ServiceReference{Name: "dummy"}, }, } @@ -297,6 +398,8 @@ func TestFailingAPIServiceSkippedAggregation(t *testing.T) { apiServiceSuccess := &v1.APIService{ Spec: v1.APIServiceSpec{ + Group: "success", + Version: "v1", Service: &v1.ServiceReference{Name: "dummy2"}, }, } @@ -355,6 +458,8 @@ func TestAPIServiceFailSuccessTransition(t *testing.T) { apiService := &v1.APIService{ Spec: v1.APIServiceSpec{ + Group: "apiservicegroup", + Version: "v1", Service: &v1.ServiceReference{Name: "dummy"}, }, } @@ -419,6 +524,8 @@ func TestFailingAPIServiceDoesNotBlockAdd(t *testing.T) { apiServiceFailed := &v1.APIService{ Spec: v1.APIServiceSpec{ + Group: "failed", + Version: "v1", Service: &v1.ServiceReference{Name: "dummy"}, }, }