Skip to content

Commit

Permalink
Discovery client and aggregator downloader use /openapi/v2 endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
roycaihw committed Feb 22, 2018
1 parent 2eb3d04 commit 1791794
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 6 deletions.
21 changes: 17 additions & 4 deletions staging/src/k8s.io/client-go/discovery/discovery_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
51 changes: 50 additions & 1 deletion staging/src/k8s.io/client-go/discovery/discovery_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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 {
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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\""))

}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down

0 comments on commit 1791794

Please sign in to comment.