From 17917940b5dfc516834980f0493cc241bcac4ab2 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Tue, 20 Feb 2018 09:22:40 -0800 Subject: [PATCH] Discovery client and aggregator downloader use /openapi/v2 endpoint --- .../client-go/discovery/discovery_client.go | 21 ++++++-- .../discovery/discovery_client_test.go | 51 ++++++++++++++++++- .../controllers/openapi/aggregator_test.go | 31 +++++++++++ .../pkg/controllers/openapi/downloader.go | 20 +++++++- 4 files changed, 117 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/client-go/discovery/discovery_client.go b/staging/src/k8s.io/client-go/discovery/discovery_client.go index 24c11f33bd8..3c685a95563 100644 --- a/staging/src/k8s.io/client-go/discovery/discovery_client.go +++ b/staging/src/k8s.io/client-go/discovery/discovery_client.go @@ -36,8 +36,12 @@ import ( restclient "k8s.io/client-go/rest" ) -// defaultRetries is the number of times a resource discovery is repeated if an api group disappears on the fly (e.g. ThirdPartyResources). -const defaultRetries = 2 +const ( + // defaultRetries is the number of times a resource discovery is repeated if an api group disappears on the fly (e.g. ThirdPartyResources). + defaultRetries = 2 + // protobuf mime type + mimePb = "application/com.github.proto-openapi.spec.v2@v1.0+protobuf" +) // DiscoveryInterface holds the methods that discover server-supported API groups, // versions and resources. @@ -329,9 +333,18 @@ func (d *DiscoveryClient) ServerVersion() (*version.Info, error) { // OpenAPISchema fetches the open api schema using a rest client and parses the proto. func (d *DiscoveryClient) OpenAPISchema() (*openapi_v2.Document, error) { - data, err := d.restClient.Get().AbsPath("/swagger-2.0.0.pb-v1").Do().Raw() + data, err := d.restClient.Get().AbsPath("/openapi/v2").SetHeader("Accept", mimePb).Do().Raw() if err != nil { - return nil, err + if errors.IsForbidden(err) || errors.IsNotFound(err) { + // single endpoint not found/registered in old server, try to fetch old endpoint + // TODO(roycaihw): remove this in 1.11 + data, err = d.restClient.Get().AbsPath("/swagger-2.0.0.pb-v1").Do().Raw() + if err != nil { + return nil, err + } + } else { + return nil, err + } } document := &openapi_v2.Document{} err = proto.Unmarshal(data, document) diff --git a/staging/src/k8s.io/client-go/discovery/discovery_client_test.go b/staging/src/k8s.io/client-go/discovery/discovery_client_test.go index 64249c88ad9..ad855139a05 100644 --- a/staging/src/k8s.io/client-go/discovery/discovery_client_test.go +++ b/staging/src/k8s.io/client-go/discovery/discovery_client_test.go @@ -326,9 +326,14 @@ var returnedOpenAPI = openapi_v2.Document{ }, } -func openapiSchemaFakeServer() (*httptest.Server, error) { +func openapiSchemaDeprecatedFakeServer() (*httptest.Server, error) { var sErr error server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + // old server returns 403 on new endpoint request + if req.URL.Path == "/openapi/v2" { + w.WriteHeader(http.StatusForbidden) + return + } if req.URL.Path != "/swagger-2.0.0.pb-v1" { sErr = fmt.Errorf("Unexpected url %v", req.URL) } @@ -349,6 +354,33 @@ func openapiSchemaFakeServer() (*httptest.Server, error) { return server, sErr } +func openapiSchemaFakeServer() (*httptest.Server, error) { + var sErr error + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.URL.Path != "/openapi/v2" { + sErr = fmt.Errorf("Unexpected url %v", req.URL) + } + if req.Method != "GET" { + sErr = fmt.Errorf("Unexpected method %v", req.Method) + } + decipherableFormat := req.Header.Get("Accept") + if decipherableFormat != "application/com.github.proto-openapi.spec.v2@v1.0+protobuf" { + sErr = fmt.Errorf("Unexpected accept mime type %v", decipherableFormat) + } + + mime.AddExtensionType(".pb-v1", "application/com.github.googleapis.gnostic.OpenAPIv2@68f4ded+protobuf") + + output, err := proto.Marshal(&returnedOpenAPI) + if err != nil { + sErr = err + return + } + w.WriteHeader(http.StatusOK) + w.Write(output) + })) + return server, sErr +} + func TestGetOpenAPISchema(t *testing.T) { server, err := openapiSchemaFakeServer() if err != nil { @@ -366,6 +398,23 @@ func TestGetOpenAPISchema(t *testing.T) { } } +func TestGetOpenAPISchemaFallback(t *testing.T) { + server, err := openapiSchemaDeprecatedFakeServer() + if err != nil { + t.Errorf("unexpected error starting fake server: %v", err) + } + defer server.Close() + + client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) + got, err := client.OpenAPISchema() + if err != nil { + t.Fatalf("unexpected error getting openapi: %v", err) + } + if e, a := returnedOpenAPI, *got; !reflect.DeepEqual(e, a) { + t.Errorf("expected %v, got %v", e, a) + } +} + func TestServerPreferredResources(t *testing.T) { stable := metav1.APIResourceList{ GroupVersion: "v1", diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator_test.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator_test.go index 4bebfe506b6..15d23e38043 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator_test.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/aggregator_test.go @@ -93,6 +93,32 @@ func (h handlerTest) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Write(h.data) } +type handlerDeprecatedTest struct { + etag string + data []byte +} + +var _ http.Handler = handlerDeprecatedTest{} + +func (h handlerDeprecatedTest) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // old server returns 403 on new endpoint + if r.URL.Path == "/openapi/v2" { + w.WriteHeader(http.StatusForbidden) + return + } + if len(h.etag) > 0 { + w.Header().Add("Etag", h.etag) + } + ifNoneMatches := r.Header["If-None-Match"] + for _, match := range ifNoneMatches { + if match == h.etag { + w.WriteHeader(http.StatusNotModified) + return + } + } + w.Write(h.data) +} + func assertDownloadedSpec(actualSpec *spec.Swagger, actualEtag string, err error, expectedSpecID string, expectedEtag string) error { if err != nil { @@ -132,4 +158,9 @@ func TestDownloadOpenAPISpec(t *testing.T) { actualSpec, actualEtag, _, err = s.Download( handlerTest{data: []byte("{\"id\": \"test\"}"), etag: "etag_test1"}, "etag_test2") assert.NoError(t, assertDownloadedSpec(actualSpec, actualEtag, err, "test", "etag_test1")) + + // Test old server fallback path + actualSpec, actualEtag, _, err = s.Download(handlerDeprecatedTest{data: []byte("{\"id\": \"test\"}")}, "") + assert.NoError(t, assertDownloadedSpec(actualSpec, actualEtag, err, "test", "\"6E8F849B434D4B98A569B9D7718876E9-356ECAB19D7FBE1336BABB1E70F8F3025050DE218BE78256BE81620681CFC9A268508E542B8B55974E17B2184BBFC8FFFAA577E51BE195D32B3CA2547818ABE4\"")) + } diff --git a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/downloader.go b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/downloader.go index 0da4e7e99af..4cd359edd05 100644 --- a/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/downloader.go +++ b/staging/src/k8s.io/kube-aggregator/pkg/controllers/openapi/downloader.go @@ -99,10 +99,11 @@ func (s *Downloader) Download(handler http.Handler, etag string) (returnSpec *sp handler = request.WithRequestContext(handler, s.contextMapper) handler = http.TimeoutHandler(handler, specDownloadTimeout, "request timed out") - req, err := http.NewRequest("GET", "/swagger.json", nil) + req, err := http.NewRequest("GET", "/openapi/v2", nil) if err != nil { return nil, "", 0, err } + req.Header.Add("Accept", "application/json") // Only pass eTag if it is not generated locally if len(etag) > 0 && !strings.HasPrefix(etag, locallyGeneratedEtagPrefix) { @@ -112,6 +113,23 @@ func (s *Downloader) Download(handler http.Handler, etag string) (returnSpec *sp writer := newInMemoryResponseWriter() handler.ServeHTTP(writer, req) + // single endpoint not found/registered in old server, try to fetch old endpoint + // TODO(roycaihw): remove this in 1.11 + if writer.respCode == http.StatusForbidden || writer.respCode == http.StatusNotFound { + req, err = http.NewRequest("GET", "/swagger.json", nil) + if err != nil { + return nil, "", 0, err + } + + // Only pass eTag if it is not generated locally + if len(etag) > 0 && !strings.HasPrefix(etag, locallyGeneratedEtagPrefix) { + req.Header.Add("If-None-Match", etag) + } + + writer = newInMemoryResponseWriter() + handler.ServeHTTP(writer, req) + } + switch writer.respCode { case http.StatusNotModified: if len(etag) == 0 {