Skip to content

Commit

Permalink
Refactors discovery content-type and helper functions
Browse files Browse the repository at this point in the history 
Kubernetes-commit: 3ce0c108fe9587be2e5195ad872578877970a7a9
  • Loading branch information
@seans3 @k8s-publishing-bot
seans3 authored and k8s-publishing-bot committed Apr 25, 2023
1 parent 559da62 commit a16d525
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 29 deletions.
64 changes: 43 additions & 21 deletions discovery/discovery_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"fmt"
"mime"
"net/http"
"net/url"
"sort"
Expand Down Expand Up @@ -58,8 +59,9 @@ const (
defaultBurst = 300

AcceptV1 = runtime.ContentTypeJSON
// Aggregated discovery content-type (currently v2beta1). NOTE: Currently, we are assuming the order
// for "g", "v", and "as" from the server. We can only compare this string if we can make that assumption.
// Aggregated discovery content-type (v2beta1). NOTE: content-type parameters
// MUST be ordered (g, v, as) for server in "Accept" header (BUT we are resilient
// to ordering when comparing returned values in "Content-Type" header).
AcceptV2Beta1 = runtime.ContentTypeJSON + ";" + "g=apidiscovery.k8s.io;v=v2beta1;as=APIGroupDiscoveryList"
// Prioritize aggregated discovery by placing first in the order of discovery accept types.
acceptDiscoveryFormats = AcceptV2Beta1 + "," + AcceptV1
Expand Down Expand Up @@ -259,8 +261,16 @@ func (d *DiscoveryClient) downloadLegacy() (

var resourcesByGV map[schema.GroupVersion]*metav1.APIResourceList
// Switch on content-type server responded with: aggregated or unaggregated.
switch responseContentType {
case AcceptV1:
switch {
case isV2Beta1ContentType(responseContentType):
var aggregatedDiscovery apidiscovery.APIGroupDiscoveryList
err = json.Unmarshal(body, &aggregatedDiscovery)
if err != nil {
return nil, nil, nil, err
}
apiGroupList, resourcesByGV, failedGVs = SplitGroupsAndResources(aggregatedDiscovery)
default:
// Default is unaggregated discovery v1.
var v metav1.APIVersions
err = json.Unmarshal(body, &v)
if err != nil {
Expand All @@ -271,15 +281,6 @@ func (d *DiscoveryClient) downloadLegacy() (
apiGroup = apiVersionsToAPIGroup(&v)
}
apiGroupList.Groups = []metav1.APIGroup{apiGroup}
case AcceptV2Beta1:
var aggregatedDiscovery apidiscovery.APIGroupDiscoveryList
err = json.Unmarshal(body, &aggregatedDiscovery)
if err != nil {
return nil, nil, nil, err
}
apiGroupList, resourcesByGV, failedGVs = SplitGroupsAndResources(aggregatedDiscovery)
default:
return nil, nil, nil, fmt.Errorf("Unknown discovery response content-type: %s", responseContentType)
}

return apiGroupList, resourcesByGV, failedGVs, nil
Expand Down Expand Up @@ -313,26 +314,47 @@ func (d *DiscoveryClient) downloadAPIs() (
failedGVs := map[schema.GroupVersion]error{}
var resourcesByGV map[schema.GroupVersion]*metav1.APIResourceList
// Switch on content-type server responded with: aggregated or unaggregated.
switch responseContentType {
case AcceptV1:
err = json.Unmarshal(body, apiGroupList)
if err != nil {
return nil, nil, nil, err
}
case AcceptV2Beta1:
switch {
case isV2Beta1ContentType(responseContentType):
var aggregatedDiscovery apidiscovery.APIGroupDiscoveryList
err = json.Unmarshal(body, &aggregatedDiscovery)
if err != nil {
return nil, nil, nil, err
}
apiGroupList, resourcesByGV, failedGVs = SplitGroupsAndResources(aggregatedDiscovery)
default:
return nil, nil, nil, fmt.Errorf("Unknown discovery response content-type: %s", responseContentType)
// Default is unaggregated discovery v1.
err = json.Unmarshal(body, apiGroupList)
if err != nil {
return nil, nil, nil, err
}
}

return apiGroupList, resourcesByGV, failedGVs, nil
}

// isV2Beta1ContentType checks of the content-type string is both
// "application/json" and contains the v2beta1 content-type params.
// NOTE: This function is resilient to the ordering of the
// content-type parameters, as well as parameters added by
// intermediaries such as proxies or gateways. Examples:
//
// "application/json; g=apidiscovery.k8s.io;v=v2beta1;as=APIGroupDiscoveryList" = true
// "application/json; as=APIGroupDiscoveryList;v=v2beta1;g=apidiscovery.k8s.io" = true
// "application/json; as=APIGroupDiscoveryList;v=v2beta1;g=apidiscovery.k8s.io;charset=utf-8" = true
// "application/json" = false
// "application/json; charset=UTF-8" = false
func isV2Beta1ContentType(contentType string) bool {
base, params, err := mime.ParseMediaType(contentType)
if err != nil {
return false
}
return runtime.ContentTypeJSON == base &&
params["g"] == "apidiscovery.k8s.io" &&
params["v"] == "v2beta1" &&
params["as"] == "APIGroupDiscoveryList"
}

// ServerGroups returns the supported groups, with information like supported versions and the
// preferred version.
func (d *DiscoveryClient) ServerGroups() (*metav1.APIGroupList, error) {
Expand Down
72 changes: 64 additions & 8 deletions discovery/discovery_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1395,8 +1395,9 @@ func TestAggregatedServerGroups(t *testing.T) {
}
output, err := json.Marshal(agg)
require.NoError(t, err)
// Content-type is "aggregated" discovery format.
w.Header().Set("Content-Type", AcceptV2Beta1)
// Content-Type is "aggregated" discovery format. Add extra parameter
// to ensure we are resilient to these extra parameters.
w.Header().Set("Content-Type", AcceptV2Beta1+"; charset=utf-8")
w.WriteHeader(http.StatusOK)
w.Write(output)
}))
Expand Down Expand Up @@ -1985,8 +1986,9 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
}
output, err := json.Marshal(agg)
require.NoError(t, err)
// Content-type is "aggregated" discovery format.
w.Header().Set("Content-Type", AcceptV2Beta1)
// Content-type is "aggregated" discovery format. Add extra parameter
// to ensure we are resilient to these extra parameters.
w.Header().Set("Content-Type", AcceptV2Beta1+"; charset=utf-8")
w.WriteHeader(http.StatusOK)
w.Write(output)
}))
Expand Down Expand Up @@ -2125,8 +2127,9 @@ func TestAggregatedServerGroupsAndResourcesWithErrors(t *testing.T) {
}
output, err := json.Marshal(agg)
require.NoError(t, err)
// Content-type is "aggregated" discovery format.
w.Header().Set("Content-Type", AcceptV2Beta1)
// Content-type is "aggregated" discovery format. Add extra parameter
// to ensure we are resilient to these extra parameters.
w.Header().Set("Content-Type", AcceptV2Beta1+"; charset=utf-8")
w.WriteHeader(status)
w.Write(output)
}))
Expand Down Expand Up @@ -2733,8 +2736,9 @@ func TestAggregatedServerPreferredResources(t *testing.T) {
}
output, err := json.Marshal(agg)
require.NoError(t, err)
// Content-type is "aggregated" discovery format.
w.Header().Set("Content-Type", AcceptV2Beta1)
// Content-type is "aggregated" discovery format. Add extra parameter
// to ensure we are resilient to these extra parameters.
w.Header().Set("Content-Type", AcceptV2Beta1+"; charset=utf-8")
w.WriteHeader(http.StatusOK)
w.Write(output)
}))
Expand All @@ -2758,6 +2762,58 @@ func TestAggregatedServerPreferredResources(t *testing.T) {
}
}

func TestDiscoveryContentTypeVersion(t *testing.T) {
tests := []struct {
contentType string
isV2Beta1 bool
}{
{
contentType: "application/json; g=apidiscovery.k8s.io;v=v2beta1;as=APIGroupDiscoveryList",
isV2Beta1: true,
},
{
// content-type parameters are not in correct order, but comparison ignores order.
contentType: "application/json; v=v2beta1;as=APIGroupDiscoveryList;g=apidiscovery.k8s.io",
isV2Beta1: true,
},
{
// content-type parameters are not in correct order, but comparison ignores order.
contentType: "application/json; as=APIGroupDiscoveryList;g=apidiscovery.k8s.io;v=v2beta1",
isV2Beta1: true,
},
{
// Ignores extra parameter "charset=utf-8"
contentType: "application/json; g=apidiscovery.k8s.io;v=v2beta1;as=APIGroupDiscoveryList;charset=utf-8",
isV2Beta1: true,
},
{
contentType: "application/json",
isV2Beta1: false,
},
{
contentType: "application/json; charset=UTF-8",
isV2Beta1: false,
},
{
contentType: "text/json",
isV2Beta1: false,
},
{
contentType: "text/html",
isV2Beta1: false,
},
{
contentType: "",
isV2Beta1: false,
},
}

for _, test := range tests {
isV2Beta1 := isV2Beta1ContentType(test.contentType)
assert.Equal(t, test.isV2Beta1, isV2Beta1)
}
}

func TestUseLegacyDiscovery(t *testing.T) {
// Default client sends aggregated discovery accept format (first) as well as legacy format.
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
Expand Down

0 comments on commit a16d525

Please sign in to comment.