Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #117571: Refactors discovery content-type and helper functions #117638

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 43 additions & 21 deletions staging/src/k8s.io/client-go/discovery/discovery_client.go
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 staging/src/k8s.io/client-go/discovery/discovery_client_test.go
Expand Up @@ -1400,8 +1400,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 @@ -1990,8 +1991,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 @@ -2130,8 +2132,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 @@ -2738,8 +2741,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 @@ -2763,6 +2767,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