Update dependencies to v0.26.10 tag

[1.26][CVE-2023-39325] .: bump golang.org/x/net to v0.17.0

Kubernetes-commit: 3ec9fd3fbe2873205939935052b04f2668a3db7e

applyconfigurations/internal/internal.go

@@ -6553,6 +6553,8 @@ var schemaYAML = typed.YAMLObject(`types:
           elementType:
             namedType: io.k8s.api.core.v1.ResourceClaim
           elementRelationship: associative
+          keys:
+          - name
     - name: limits
       type:
         map:
@@ -11659,6 +11661,8 @@ var schemaYAML = typed.YAMLObject(`types:
           elementType:
             namedType: io.k8s.api.resource.v1alpha1.ResourceClaimConsumerReference
           elementRelationship: associative
+          keys:
+          - uid
 - name: io.k8s.api.resource.v1alpha1.ResourceClaimTemplate
   map:
     fields:

discovery/aggregated_discovery.go

@@ -24,19 +24,36 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
+// StaleGroupVersionError encasulates failed GroupVersion marked "stale"
+// in the returned AggregatedDiscovery format.
+type StaleGroupVersionError struct {
+	gv schema.GroupVersion
+}
+
+func (s StaleGroupVersionError) Error() string {
+	return fmt.Sprintf("stale GroupVersion discovery: %v", s.gv)
+}
+
 // SplitGroupsAndResources transforms "aggregated" discovery top-level structure into
 // the previous "unaggregated" discovery groups and resources.
-func SplitGroupsAndResources(aggregatedGroups apidiscovery.APIGroupDiscoveryList) (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList) {
+func SplitGroupsAndResources(aggregatedGroups apidiscovery.APIGroupDiscoveryList) (
+	*metav1.APIGroupList,
+	map[schema.GroupVersion]*metav1.APIResourceList,
+	map[schema.GroupVersion]error) {
 	// Aggregated group list will contain the entirety of discovery, including
-	// groups, versions, and resources.
+	// groups, versions, and resources. GroupVersions marked "stale" are failed.
 	groups := []*metav1.APIGroup{}
+	failedGVs := map[schema.GroupVersion]error{}
 	resourcesByGV := map[schema.GroupVersion]*metav1.APIResourceList{}
 	for _, aggGroup := range aggregatedGroups.Items {
-		group, resources := convertAPIGroup(aggGroup)
+		group, resources, failed := convertAPIGroup(aggGroup)
 		groups = append(groups, group)
 		for gv, resourceList := range resources {
 			resourcesByGV[gv] = resourceList
 		}
+		for gv, err := range failed {
+			failedGVs[gv] = err
+		}
 	}
 	// Transform slice of groups to group list before returning.
 	groupList := &metav1.APIGroupList{}
@@ -44,65 +61,96 @@ func SplitGroupsAndResources(aggregatedGroups apidiscovery.APIGroupDiscoveryList
 	for _, group := range groups {
 		groupList.Groups = append(groupList.Groups, *group)
 	}
-	return groupList, resourcesByGV
+	return groupList, resourcesByGV, failedGVs
 }
 
 // convertAPIGroup tranforms an "aggregated" APIGroupDiscovery to an "legacy" APIGroup,
 // also returning the map of APIResourceList for resources within GroupVersions.
-func convertAPIGroup(g apidiscovery.APIGroupDiscovery) (*metav1.APIGroup, map[schema.GroupVersion]*metav1.APIResourceList) {
+func convertAPIGroup(g apidiscovery.APIGroupDiscovery) (
+	*metav1.APIGroup,
+	map[schema.GroupVersion]*metav1.APIResourceList,
+	map[schema.GroupVersion]error) {
 	// Iterate through versions to convert to group and resources.
 	group := &metav1.APIGroup{}
 	gvResources := map[schema.GroupVersion]*metav1.APIResourceList{}
+	failedGVs := map[schema.GroupVersion]error{}
 	group.Name = g.ObjectMeta.Name
-	for i, v := range g.Versions {
-		version := metav1.GroupVersionForDiscovery{}
+	for _, v := range g.Versions {
 		gv := schema.GroupVersion{Group: g.Name, Version: v.Version}
+		if v.Freshness == apidiscovery.DiscoveryFreshnessStale {
+			failedGVs[gv] = StaleGroupVersionError{gv: gv}
+			continue
+		}
+		version := metav1.GroupVersionForDiscovery{}
 		version.GroupVersion = gv.String()
 		version.Version = v.Version
 		group.Versions = append(group.Versions, version)
-		if i == 0 {
+		// PreferredVersion is first non-stale Version
+		if group.PreferredVersion == (metav1.GroupVersionForDiscovery{}) {
 			group.PreferredVersion = version
 		}
 		resourceList := &metav1.APIResourceList{}
 		resourceList.GroupVersion = gv.String()
 		for _, r := range v.Resources {
-			resource := convertAPIResource(r)
-			resourceList.APIResources = append(resourceList.APIResources, resource)
+			resource, err := convertAPIResource(r)
+			if err == nil {
+				resourceList.APIResources = append(resourceList.APIResources, resource)
+			}
 			// Subresources field in new format get transformed into full APIResources.
+			// It is possible a partial result with an error was returned to be used
+			// as the parent resource for the subresource.
 			for _, subresource := range r.Subresources {
-				sr := convertAPISubresource(resource, subresource)
-				resourceList.APIResources = append(resourceList.APIResources, sr)
+				sr, err := convertAPISubresource(resource, subresource)
+				if err == nil {
+					resourceList.APIResources = append(resourceList.APIResources, sr)
+				}
 			}
 		}
 		gvResources[gv] = resourceList
 	}
-	return group, gvResources
+	return group, gvResources, failedGVs
 }
 
-// convertAPIResource tranforms a APIResourceDiscovery to an APIResource.
-func convertAPIResource(in apidiscovery.APIResourceDiscovery) metav1.APIResource {
-	return metav1.APIResource{
+var emptyKind = metav1.GroupVersionKind{}
+
+// convertAPIResource tranforms a APIResourceDiscovery to an APIResource. We are
+// resilient to missing GVK, since this resource might be the parent resource
+// for a subresource. If the parent is missing a GVK, it is not returned in
+// discovery, and the subresource MUST have the GVK.
+func convertAPIResource(in apidiscovery.APIResourceDiscovery) (metav1.APIResource, error) {
+	result := metav1.APIResource{
 		Name:         in.Resource,
 		SingularName: in.SingularResource,
 		Namespaced:   in.Scope == apidiscovery.ScopeNamespace,
-		Group:        in.ResponseKind.Group,
-		Version:      in.ResponseKind.Version,
-		Kind:         in.ResponseKind.Kind,
 		Verbs:        in.Verbs,
 		ShortNames:   in.ShortNames,
 		Categories:   in.Categories,
 	}
+	var err error
+	if in.ResponseKind != nil && (*in.ResponseKind) != emptyKind {
+		result.Group = in.ResponseKind.Group
+		result.Version = in.ResponseKind.Version
+		result.Kind = in.ResponseKind.Kind
+	} else {
+		err = fmt.Errorf("discovery resource %s missing GVK", in.Resource)
+	}
+	// Can return partial result with error, which can be the parent for a
+	// subresource. Do not add this result to the returned discovery resources.
+	return result, err
 }
 
 // convertAPISubresource tranforms a APISubresourceDiscovery to an APIResource.
-func convertAPISubresource(parent metav1.APIResource, in apidiscovery.APISubresourceDiscovery) metav1.APIResource {
-	return metav1.APIResource{
-		Name:         fmt.Sprintf("%s/%s", parent.Name, in.Subresource),
-		SingularName: parent.SingularName,
-		Namespaced:   parent.Namespaced,
-		Group:        in.ResponseKind.Group,
-		Version:      in.ResponseKind.Version,
-		Kind:         in.ResponseKind.Kind,
-		Verbs:        in.Verbs,
+func convertAPISubresource(parent metav1.APIResource, in apidiscovery.APISubresourceDiscovery) (metav1.APIResource, error) {
+	result := metav1.APIResource{}
+	if in.ResponseKind == nil || (*in.ResponseKind) == emptyKind {
+		return result, fmt.Errorf("subresource %s/%s missing GVK", parent.Name, in.Subresource)
 	}
+	result.Name = fmt.Sprintf("%s/%s", parent.Name, in.Subresource)
+	result.SingularName = parent.SingularName
+	result.Namespaced = parent.Namespaced
+	result.Group = in.ResponseKind.Group
+	result.Version = in.ResponseKind.Version
+	result.Kind = in.ResponseKind.Kind
+	result.Verbs = in.Verbs
+	return result, nil
 }

discovery/aggregated_discovery_test.go

@@ -31,6 +31,7 @@ func TestSplitGroupsAndResources(t *testing.T) {
 		agg                 apidiscovery.APIGroupDiscoveryList
 		expectedGroups      metav1.APIGroupList
 		expectedGVResources map[schema.GroupVersion]*metav1.APIResourceList
+		expectedFailedGVs   map[schema.GroupVersion]error
 	}{
 		{
 			name: "Aggregated discovery: core/v1 group and pod resource",
@@ -90,6 +91,7 @@ func TestSplitGroupsAndResources(t *testing.T) {
 					},
 				},
 			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
 		},
 		{
 			name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources at /apis",
@@ -179,6 +181,7 @@ func TestSplitGroupsAndResources(t *testing.T) {
 					},
 				},
 			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
 		},
 		{
 			name: "Aggregated discovery: 1 group/2 resources at /api, 1 group/2 resources at /apis",
@@ -313,6 +316,7 @@ func TestSplitGroupsAndResources(t *testing.T) {
 					},
 				},
 			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
 		},
 		{
 			name: "Aggregated discovery: multiple groups with cluster-scoped resources",
@@ -447,6 +451,7 @@ func TestSplitGroupsAndResources(t *testing.T) {
 					},
 				},
 			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
 		},
 		{
 			name: "Aggregated discovery with single subresource",
@@ -534,6 +539,146 @@ func TestSplitGroupsAndResources(t *testing.T) {
 					},
 				},
 			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
+		},
+		{
+			name: "Aggregated discovery with single subresource and parent missing GVK",
+			agg: apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "external.metrics.k8s.io",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										// resilient to nil GVK for parent
+										Resource:         "*",
+										Scope:            apidiscovery.ScopeNamespace,
+										SingularResource: "",
+										Subresources: []apidiscovery.APISubresourceDiscovery{
+											{
+												Subresource: "other-external-metric",
+												ResponseKind: &metav1.GroupVersionKind{
+													Kind: "MetricValueList",
+												},
+												Verbs: []string{"get"},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedGroups: metav1.APIGroupList{
+				Groups: []metav1.APIGroup{
+					{
+						Name: "external.metrics.k8s.io",
+						Versions: []metav1.GroupVersionForDiscovery{
+							{
+								GroupVersion: "external.metrics.k8s.io/v1beta1",
+								Version:      "v1beta1",
+							},
+						},
+						PreferredVersion: metav1.GroupVersionForDiscovery{
+							GroupVersion: "external.metrics.k8s.io/v1beta1",
+							Version:      "v1beta1",
+						},
+					},
+				},
+			},
+			expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{
+				{Group: "external.metrics.k8s.io", Version: "v1beta1"}: {
+					GroupVersion: "external.metrics.k8s.io/v1beta1",
+					APIResources: []metav1.APIResource{
+						// Since parent GVK was nil, it is NOT returned--only the subresource.
+						{
+							Name:         "*/other-external-metric",
+							SingularName: "",
+							Namespaced:   true,
+							Group:        "",
+							Version:      "",
+							Kind:         "MetricValueList",
+							Verbs:        []string{"get"},
+						},
+					},
+				},
+			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
+		},
+		{
+			name: "Aggregated discovery with single subresource and parent empty GVK",
+			agg: apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "external.metrics.k8s.io",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										// resilient to empty GVK for parent
+										Resource:         "*",
+										Scope:            apidiscovery.ScopeNamespace,
+										SingularResource: "",
+										ResponseKind:     &metav1.GroupVersionKind{},
+										Subresources: []apidiscovery.APISubresourceDiscovery{
+											{
+												Subresource: "other-external-metric",
+												ResponseKind: &metav1.GroupVersionKind{
+													Kind: "MetricValueList",
+												},
+												Verbs: []string{"get"},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedGroups: metav1.APIGroupList{
+				Groups: []metav1.APIGroup{
+					{
+						Name: "external.metrics.k8s.io",
+						Versions: []metav1.GroupVersionForDiscovery{
+							{
+								GroupVersion: "external.metrics.k8s.io/v1beta1",
+								Version:      "v1beta1",
+							},
+						},
+						PreferredVersion: metav1.GroupVersionForDiscovery{
+							GroupVersion: "external.metrics.k8s.io/v1beta1",
+							Version:      "v1beta1",
+						},
+					},
+				},
+			},
+			expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{
+				{Group: "external.metrics.k8s.io", Version: "v1beta1"}: {
+					GroupVersion: "external.metrics.k8s.io/v1beta1",
+					APIResources: []metav1.APIResource{
+						// Since parent GVK was nil, it is NOT returned--only the subresource.
+						{
+							Name:         "*/other-external-metric",
+							SingularName: "",
+							Namespaced:   true,
+							Group:        "",
+							Version:      "",
+							Kind:         "MetricValueList",
+							Verbs:        []string{"get"},
+						},
+					},
+				},
+			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
 		},
 		{
 			name: "Aggregated discovery with multiple subresources",
@@ -633,11 +778,185 @@ func TestSplitGroupsAndResources(t *testing.T) {
 					},
 				},
 			},
+			expectedFailedGVs: map[schema.GroupVersion]error{},
+		},
+		{
+			name: "Aggregated discovery: single failed GV at /api",
+			agg: apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "pods",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "",
+											Version: "v1",
+											Kind:    "Pod",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "services",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "",
+											Version: "v1",
+											Kind:    "Service",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
+						},
+					},
+				},
+			},
+			// Single core Group/Version is stale, so no Version within Group.
+			expectedGroups: metav1.APIGroupList{
+				Groups: []metav1.APIGroup{{Name: ""}},
+			},
+			// Single core Group/Version is stale, so there are no expected resources.
+			expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{},
+			expectedFailedGVs: map[schema.GroupVersion]error{
+				{Group: "", Version: "v1"}: StaleGroupVersionError{gv: schema.GroupVersion{Group: "", Version: "v1"}},
+			},
+		},
+		{
+			name: "Aggregated discovery: single failed GV at /apis",
+			agg: apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "apps",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "statefulsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1",
+											Kind:    "StatefulSets",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
+						},
+					},
+				},
+			},
+			// Single apps/v1 Group/Version is stale, so no Version within Group.
+			expectedGroups: metav1.APIGroupList{
+				Groups: []metav1.APIGroup{{Name: "apps"}},
+			},
+			// Single apps/v1 Group/Version is stale, so there are no expected resources.
+			expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{},
+			expectedFailedGVs: map[schema.GroupVersion]error{
+				{Group: "apps", Version: "v1"}: StaleGroupVersionError{gv: schema.GroupVersion{Group: "apps", Version: "v1"}},
+			},
+		},
+		{
+			name: "Aggregated discovery: 1 group/2 versions/1 failed GV at /apis",
+			agg: apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "apps",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							// Stale v2 should report failed GV.
+							{
+								Version: "v2",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "daemonsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v2",
+											Kind:    "DaemonSets",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			// Only apps/v1 is non-stale expected Group/Version
+			expectedGroups: metav1.APIGroupList{
+				Groups: []metav1.APIGroup{
+					{
+						Name: "apps",
+						Versions: []metav1.GroupVersionForDiscovery{
+							{
+								GroupVersion: "apps/v1",
+								Version:      "v1",
+							},
+						},
+						// PreferredVersion must be apps/v1
+						PreferredVersion: metav1.GroupVersionForDiscovery{
+							GroupVersion: "apps/v1",
+							Version:      "v1",
+						},
+					},
+				},
+			},
+			// Only apps/v1 resources expected.
+			expectedGVResources: map[schema.GroupVersion]*metav1.APIResourceList{
+				{Group: "apps", Version: "v1"}: {
+					GroupVersion: "apps/v1",
+					APIResources: []metav1.APIResource{
+						{
+							Name:       "deployments",
+							Namespaced: true,
+							Group:      "apps",
+							Version:    "v1",
+							Kind:       "Deployment",
+						},
+					},
+				},
+			},
+			expectedFailedGVs: map[schema.GroupVersion]error{
+				{Group: "apps", Version: "v2"}: StaleGroupVersionError{gv: schema.GroupVersion{Group: "apps", Version: "v2"}},
+			},
 		},
 	}
 
 	for _, test := range tests {
-		apiGroups, resourcesByGV := SplitGroupsAndResources(test.agg)
+		apiGroups, resourcesByGV, failedGVs := SplitGroupsAndResources(test.agg)
+		assert.Equal(t, test.expectedFailedGVs, failedGVs)
 		assert.Equal(t, test.expectedGroups, *apiGroups)
 		assert.Equal(t, test.expectedGVResources, resourcesByGV)
 	}

discovery/cached/memory/memcache.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/client-go/openapi"
 	cachedopenapi "k8s.io/client-go/openapi/cached"
 	restclient "k8s.io/client-go/rest"
+	"k8s.io/klog/v2"
 )
 
 type cacheEntry struct {
@@ -61,6 +62,15 @@ var (
 	ErrCacheNotFound = errors.New("not found")
 )
 
+// Server returning empty ResourceList for Group/Version.
+type emptyResponseError struct {
+	gv string
+}
+
+func (e *emptyResponseError) Error() string {
+	return fmt.Sprintf("received empty response for: %s", e.gv)
+}
+
 var _ discovery.CachedDiscoveryInterface = &memCacheClient{}
 
 // isTransientConnectionError checks whether given error is "Connection refused" or
@@ -103,7 +113,13 @@ func (d *memCacheClient) ServerResourcesForGroupVersion(groupVersion string) (*m
 	if cachedVal.err != nil && isTransientError(cachedVal.err) {
 		r, err := d.serverResourcesForGroupVersion(groupVersion)
 		if err != nil {
-			utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", groupVersion, err))
+			// Don't log "empty response" as an error; it is a common response for metrics.
+			if _, emptyErr := err.(*emptyResponseError); emptyErr {
+				// Log at same verbosity as disk cache.
+				klog.V(3).Infof("%v", err)
+			} else {
+				utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", groupVersion, err))
+			}
 		}
 		cachedVal = &cacheEntry{r, err}
 		d.groupToServerResources[groupVersion] = cachedVal
@@ -120,32 +136,38 @@ func (d *memCacheClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*meta
 // GroupsAndMaybeResources returns the list of APIGroups, and possibly the map of group/version
 // to resources. The returned groups will never be nil, but the resources map can be nil
 // if there are no cached resources.
-func (d *memCacheClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) {
+func (d *memCacheClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 
 	if !d.cacheValid {
 		if err := d.refreshLocked(); err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 	}
 	// Build the resourceList from the cache?
 	var resourcesMap map[schema.GroupVersion]*metav1.APIResourceList
+	var failedGVs map[schema.GroupVersion]error
 	if d.receivedAggregatedDiscovery && len(d.groupToServerResources) > 0 {
 		resourcesMap = map[schema.GroupVersion]*metav1.APIResourceList{}
+		failedGVs = map[schema.GroupVersion]error{}
 		for gv, cacheEntry := range d.groupToServerResources {
 			groupVersion, err := schema.ParseGroupVersion(gv)
 			if err != nil {
-				return nil, nil, fmt.Errorf("failed to parse group version (%v): %v", gv, err)
+				return nil, nil, nil, fmt.Errorf("failed to parse group version (%v): %v", gv, err)
+			}
+			if cacheEntry.err != nil {
+				failedGVs[groupVersion] = cacheEntry.err
+			} else {
+				resourcesMap[groupVersion] = cacheEntry.resourceList
 			}
-			resourcesMap[groupVersion] = cacheEntry.resourceList
 		}
 	}
-	return d.groupList, resourcesMap, nil
+	return d.groupList, resourcesMap, failedGVs, nil
 }
 
 func (d *memCacheClient) ServerGroups() (*metav1.APIGroupList, error) {
-	groups, _, err := d.GroupsAndMaybeResources()
+	groups, _, _, err := d.GroupsAndMaybeResources()
 	if err != nil {
 		return nil, err
 	}
@@ -219,7 +241,8 @@ func (d *memCacheClient) refreshLocked() error {
 
 	if ad, ok := d.delegate.(discovery.AggregatedDiscoveryInterface); ok {
 		var resources map[schema.GroupVersion]*metav1.APIResourceList
-		gl, resources, err = ad.GroupsAndMaybeResources()
+		var failedGVs map[schema.GroupVersion]error
+		gl, resources, failedGVs, err = ad.GroupsAndMaybeResources()
 		if resources != nil && err == nil {
 			// Cache the resources.
 			d.groupToServerResources = map[string]*cacheEntry{}
@@ -227,6 +250,10 @@ func (d *memCacheClient) refreshLocked() error {
 			for gv, resources := range resources {
 				d.groupToServerResources[gv.String()] = &cacheEntry{resources, nil}
 			}
+			// Cache GroupVersion discovery errors
+			for gv, err := range failedGVs {
+				d.groupToServerResources[gv.String()] = &cacheEntry{nil, err}
+			}
 			d.receivedAggregatedDiscovery = true
 			d.cacheValid = true
 			return nil
@@ -252,7 +279,13 @@ func (d *memCacheClient) refreshLocked() error {
 
 				r, err := d.serverResourcesForGroupVersion(gv)
 				if err != nil {
-					utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", gv, err))
+					// Don't log "empty response" as an error; it is a common response for metrics.
+					if _, emptyErr := err.(*emptyResponseError); emptyErr {
+						// Log at same verbosity as disk cache.
+						klog.V(3).Infof("%v", err)
+					} else {
+						utilruntime.HandleError(fmt.Errorf("couldn't get resource list for %v: %v", gv, err))
+					}
 				}
 
 				resultLock.Lock()
@@ -274,7 +307,7 @@ func (d *memCacheClient) serverResourcesForGroupVersion(groupVersion string) (*m
 		return r, err
 	}
 	if len(r.APIResources) == 0 {
-		return r, fmt.Errorf("Got empty response for: %v", groupVersion)
+		return r, &emptyResponseError{gv: groupVersion}
 	}
 	return r, nil
 }

discovery/cached/memory/memcache_test.go

@@ -32,6 +32,7 @@ import (
 	errorsutil "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/discovery/fake"
@@ -599,10 +600,11 @@ func TestMemCacheGroupsAndMaybeResources(t *testing.T) {
 			groupToServerResources: map[string]*cacheEntry{},
 		}
 		assert.False(t, memClient.Fresh())
-		apiGroupList, resourcesMap, err := memClient.GroupsAndMaybeResources()
+		apiGroupList, resourcesMap, failedGVs, err := memClient.GroupsAndMaybeResources()
 		require.NoError(t, err)
 		// "Unaggregated" discovery always returns nil for resources.
 		assert.Nil(t, resourcesMap)
+		assert.True(t, len(failedGVs) == 0, "expected empty failed GroupVersions, got (%d)", len(failedGVs))
 		assert.False(t, memClient.receivedAggregatedDiscovery)
 		assert.True(t, memClient.Fresh())
 		// Test the expected groups are returned for the aggregated format.
@@ -618,7 +620,7 @@ func TestMemCacheGroupsAndMaybeResources(t *testing.T) {
 		// Invalidate the cache and retrieve the server groups and resources again.
 		memClient.Invalidate()
 		assert.False(t, memClient.Fresh())
-		apiGroupList, resourcesMap, err = memClient.GroupsAndMaybeResources()
+		apiGroupList, resourcesMap, _, err = memClient.GroupsAndMaybeResources()
 		require.NoError(t, err)
 		assert.Nil(t, resourcesMap)
 		assert.False(t, memClient.receivedAggregatedDiscovery)
@@ -638,6 +640,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 		expectedGroupNames    []string
 		expectedGroupVersions []string
 		expectedGVKs          []string
+		expectedFailedGVs     []string
 	}{
 		{
 			name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/1 resources at /apis",
@@ -694,9 +697,10 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 				"/v1/Pod",
 				"apps/v1/Deployment",
 			},
+			expectedFailedGVs: []string{},
 		},
 		{
-			name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources at /apis",
+			name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources/1 stale GV at /apis",
 			corev1: &apidiscovery.APIGroupDiscoveryList{
 				Items: []apidiscovery.APIGroupDiscovery{
 					{
@@ -741,6 +745,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 								},
 							},
 							{
+								// Stale Version is not included in discovery.
 								Version: "v2",
 								Resources: []apidiscovery.APIResourceDiscovery{
 									{
@@ -753,18 +758,19 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 										Scope: apidiscovery.ScopeNamespace,
 									},
 								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
 							},
 						},
 					},
 				},
 			},
 			expectedGroupNames:    []string{"", "apps"},
-			expectedGroupVersions: []string{"v1", "apps/v1", "apps/v2"},
+			expectedGroupVersions: []string{"v1", "apps/v1"},
 			expectedGVKs: []string{
 				"/v1/Pod",
 				"apps/v1/Deployment",
-				"apps/v2/Deployment",
 			},
+			expectedFailedGVs: []string{"apps/v2"},
 		},
 		{
 			name: "Aggregated discovery: 1 group/2 resources at /api, 1 group/2 resources at /apis",
@@ -841,9 +847,10 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 				"apps/v1/Deployment",
 				"apps/v1/StatefulSet",
 			},
+			expectedFailedGVs: []string{},
 		},
 		{
-			name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources at /apis",
+			name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources/1 stale GV at /apis",
 			corev1: &apidiscovery.APIGroupDiscoveryList{
 				Items: []apidiscovery.APIGroupDiscovery{
 					{
@@ -905,6 +912,31 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 									},
 								},
 							},
+							{
+								// Stale version is not included in discovery.
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "statefulsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "StatefulSet",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
 						},
 					},
 					{
@@ -949,9 +981,10 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 				"batch/v1/Job",
 				"batch/v1/CronJob",
 			},
+			expectedFailedGVs: []string{"apps/v1beta1"},
 		},
 		{
-			name:   "Aggregated discovery: /api returns nothing, 2 groups/2 resources at /apis",
+			name:   "Aggregated discovery: /api returns nothing, 2 groups/2 resources/2 stale GV at /apis",
 			corev1: &apidiscovery.APIGroupDiscoveryList{},
 			apis: &apidiscovery.APIGroupDiscoveryList{
 				Items: []apidiscovery.APIGroupDiscovery{
@@ -960,6 +993,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 							Name: "apps",
 						},
 						Versions: []apidiscovery.APIVersionDiscovery{
+							// Statel "v1" Version is not included in discovery.
 							{
 								Version: "v1",
 								Resources: []apidiscovery.APIResourceDiscovery{
@@ -982,6 +1016,30 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 										Scope: apidiscovery.ScopeNamespace,
 									},
 								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
+							{
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "statefulsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "StatefulSet",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
 							},
 						},
 					},
@@ -1013,18 +1071,35 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 									},
 								},
 							},
+							{
+								// Stale Version is not included in discovery.
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "jobs",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "batch",
+											Version: "v1beta1",
+											Kind:    "Job",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
 						},
 					},
 				},
 			},
 			expectedGroupNames:    []string{"apps", "batch"},
-			expectedGroupVersions: []string{"apps/v1", "batch/v1"},
+			expectedGroupVersions: []string{"apps/v1beta1", "batch/v1"},
 			expectedGVKs: []string{
-				"apps/v1/Deployment",
-				"apps/v1/StatefulSet",
+				"apps/v1beta1/Deployment",
+				"apps/v1beta1/StatefulSet",
 				"batch/v1/Job",
 				"batch/v1/CronJob",
 			},
+			expectedFailedGVs: []string{"apps/v1", "batch/v1beta1"},
 		},
 	}
 
@@ -1054,7 +1129,7 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 			groupToServerResources: map[string]*cacheEntry{},
 		}
 		assert.False(t, memClient.Fresh())
-		apiGroupList, resourcesMap, err := memClient.GroupsAndMaybeResources()
+		apiGroupList, resourcesMap, failedGVs, err := memClient.GroupsAndMaybeResources()
 		require.NoError(t, err)
 		assert.True(t, memClient.receivedAggregatedDiscovery)
 		assert.True(t, memClient.Fresh())
@@ -1077,10 +1152,15 @@ func TestAggregatedMemCacheGroupsAndMaybeResources(t *testing.T) {
 		actualGVKs := sets.NewString(groupVersionKinds(resources)...)
 		assert.True(t, expectedGVKs.Equal(actualGVKs),
 			"%s: Expected GVKs (%s), got (%s)", test.name, expectedGVKs.List(), actualGVKs.List())
+		// Test the returned failed GroupVersions are correct.
+		expectedFailedGVs := sets.NewString(test.expectedFailedGVs...)
+		actualFailedGVs := sets.NewString(failedGroupVersions(failedGVs)...)
+		assert.True(t, expectedFailedGVs.Equal(actualFailedGVs),
+			"%s: Expected Failed GroupVersions (%s), got (%s)", test.name, expectedFailedGVs.List(), actualFailedGVs.List())
 		// Invalidate the cache and retrieve the server groups again.
 		memClient.Invalidate()
 		assert.False(t, memClient.Fresh())
-		apiGroupList, _, err = memClient.GroupsAndMaybeResources()
+		apiGroupList, _, _, err = memClient.GroupsAndMaybeResources()
 		require.NoError(t, err)
 		// Test the expected groups are returned for the aggregated format.
 		actualGroupNames = sets.NewString(groupNamesFromList(apiGroupList)...)
@@ -1410,3 +1490,11 @@ func groupVersionKinds(resources []*metav1.APIResourceList) []string {
 	}
 	return result
 }
+
+func failedGroupVersions(gvs map[schema.GroupVersion]error) []string {
+	result := []string{}
+	for gv := range gvs {
+		result = append(result, gv.String())
+	}
+	return result
+}

discovery/discovery_client.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"mime"
 	"net/http"
 	"net/url"
 	"sort"
@@ -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
@@ -86,7 +88,7 @@ type DiscoveryInterface interface {
 type AggregatedDiscoveryInterface interface {
 	DiscoveryInterface
 
-	GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error)
+	GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, map[schema.GroupVersion]error, error)
 }
 
 // CachedDiscoveryInterface is a DiscoveryInterface with cache invalidation and freshness.
@@ -186,18 +188,23 @@ func apiVersionsToAPIGroup(apiVersions *metav1.APIVersions) (apiGroup metav1.API
 // and resources from /api and /apis (either aggregated or not). Legacy groups
 // must be ordered first. The server will either return both endpoints (/api, /apis)
 // as aggregated discovery format or legacy format. For safety, resources will only
-// be returned if both endpoints returned resources.
-func (d *DiscoveryClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) {
+// be returned if both endpoints returned resources. Returned "failedGVs" can be
+// empty, but will only be nil in the case an error is returned.
+func (d *DiscoveryClient) GroupsAndMaybeResources() (
+	*metav1.APIGroupList,
+	map[schema.GroupVersion]*metav1.APIResourceList,
+	map[schema.GroupVersion]error,
+	error) {
 	// Legacy group ordered first (there is only one -- core/v1 group). Returned groups must
 	// be non-nil, but it could be empty. Returned resources, apiResources map could be nil.
-	groups, resources, err := d.downloadLegacy()
+	groups, resources, failedGVs, err := d.downloadLegacy()
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	// Discovery groups and (possibly) resources downloaded from /apis.
-	apiGroups, apiResources, aerr := d.downloadAPIs()
-	if err != nil {
-		return nil, nil, aerr
+	apiGroups, apiResources, failedApisGVs, aerr := d.downloadAPIs()
+	if aerr != nil {
+		return nil, nil, nil, aerr
 	}
 	// Merge apis groups into the legacy groups.
 	for _, group := range apiGroups.Groups {
@@ -211,14 +218,23 @@ func (d *DiscoveryClient) GroupsAndMaybeResources() (*metav1.APIGroupList, map[s
 	} else if resources != nil {
 		resources = nil
 	}
-	return groups, resources, err
+	// Merge failed GroupVersions from /api and /apis
+	for gv, err := range failedApisGVs {
+		failedGVs[gv] = err
+	}
+	return groups, resources, failedGVs, err
 }
 
 // downloadLegacy returns the discovery groups and possibly resources
 // for the legacy v1 GVR at /api, or an error if one occurred. It is
 // possible for the resource map to be nil if the server returned
-// the unaggregated discovery.
-func (d *DiscoveryClient) downloadLegacy() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) {
+// the unaggregated discovery. Returned "failedGVs" can be empty, but
+// will only be nil in the case of a returned error.
+func (d *DiscoveryClient) downloadLegacy() (
+	*metav1.APIGroupList,
+	map[schema.GroupVersion]*metav1.APIResourceList,
+	map[schema.GroupVersion]error,
+	error) {
 	accept := acceptDiscoveryFormats
 	if d.UseLegacyDiscovery {
 		accept = AcceptV1
@@ -230,48 +246,55 @@ func (d *DiscoveryClient) downloadLegacy() (*metav1.APIGroupList, map[schema.Gro
 		Do(context.TODO()).
 		ContentType(&responseContentType).
 		Raw()
-	// Special error handling for 403 or 404 to be compatible with older v1.0 servers.
-	// Return empty group list to be merged with /apis.
-	if err != nil && !errors.IsNotFound(err) && !errors.IsForbidden(err) {
-		return nil, nil, err
-	}
-	if err != nil && (errors.IsNotFound(err) || errors.IsForbidden(err)) {
-		return &metav1.APIGroupList{}, nil, nil
+	apiGroupList := &metav1.APIGroupList{}
+	failedGVs := map[schema.GroupVersion]error{}
+	if err != nil {
+		// Tolerate 404, since aggregated api servers can return it.
+		if errors.IsNotFound(err) {
+			// Return empty structures and no error.
+			emptyGVMap := map[schema.GroupVersion]*metav1.APIResourceList{}
+			return apiGroupList, emptyGVMap, failedGVs, nil
+		} else {
+			return nil, nil, nil, err
+		}
 	}
 
-	apiGroupList := &metav1.APIGroupList{}
 	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 {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 		apiGroup := metav1.APIGroup{}
 		if len(v.Versions) != 0 {
 			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, err
-		}
-		apiGroupList, resourcesByGV = SplitGroupsAndResources(aggregatedDiscovery)
-	default:
-		return nil, nil, fmt.Errorf("Unknown discovery response content-type: %s", responseContentType)
 	}
 
-	return apiGroupList, resourcesByGV, nil
+	return apiGroupList, resourcesByGV, failedGVs, nil
 }
 
 // downloadAPIs returns the discovery groups and (if aggregated format) the
 // discovery resources. The returned groups will always exist, but the
-// resources map may be nil.
-func (d *DiscoveryClient) downloadAPIs() (*metav1.APIGroupList, map[schema.GroupVersion]*metav1.APIResourceList, error) {
+// resources map may be nil. Returned "failedGVs" can be empty, but will
+// only be nil in the case of a returned error.
+func (d *DiscoveryClient) downloadAPIs() (
+	*metav1.APIGroupList,
+	map[schema.GroupVersion]*metav1.APIResourceList,
+	map[schema.GroupVersion]error,
+	error) {
 	accept := acceptDiscoveryFormats
 	if d.UseLegacyDiscovery {
 		accept = AcceptV1
@@ -283,42 +306,59 @@ func (d *DiscoveryClient) downloadAPIs() (*metav1.APIGroupList, map[schema.Group
 		Do(context.TODO()).
 		ContentType(&responseContentType).
 		Raw()
-	// Special error handling for 403 or 404 to be compatible with older v1.0 servers.
-	// Return empty group list to be merged with /api.
-	if err != nil && !errors.IsNotFound(err) && !errors.IsForbidden(err) {
-		return nil, nil, err
-	}
-	if err != nil && (errors.IsNotFound(err) || errors.IsForbidden(err)) {
-		return &metav1.APIGroupList{}, nil, nil
+	if err != nil {
+		return nil, nil, nil, err
 	}
 
 	apiGroupList := &metav1.APIGroupList{}
+	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, err
-		}
-	case AcceptV2Beta1:
+	switch {
+	case isV2Beta1ContentType(responseContentType):
 		var aggregatedDiscovery apidiscovery.APIGroupDiscoveryList
 		err = json.Unmarshal(body, &aggregatedDiscovery)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
-		apiGroupList, resourcesByGV = SplitGroupsAndResources(aggregatedDiscovery)
+		apiGroupList, resourcesByGV, failedGVs = SplitGroupsAndResources(aggregatedDiscovery)
 	default:
-		return 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, nil
+	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) {
-	groups, _, err := d.GroupsAndMaybeResources()
+	groups, _, _, err := d.GroupsAndMaybeResources()
 	if err != nil {
 		return nil, err
 	}
@@ -341,8 +381,10 @@ func (d *DiscoveryClient) ServerResourcesForGroupVersion(groupVersion string) (r
 	}
 	err = d.restClient.Get().AbsPath(url.String()).Do(context.TODO()).Into(resources)
 	if err != nil {
-		// ignore 403 or 404 error to be compatible with an v1.0 server.
-		if groupVersion == "v1" && (errors.IsNotFound(err) || errors.IsForbidden(err)) {
+		// Tolerate core/v1 not found response by returning empty resource list;
+		// this probably should not happen. But we should verify all callers are
+		// not depending on this toleration before removal.
+		if groupVersion == "v1" && errors.IsNotFound(err) {
 			return resources, nil
 		}
 		return nil, err
@@ -383,13 +425,14 @@ func IsGroupDiscoveryFailedError(err error) bool {
 func ServerGroupsAndResources(d DiscoveryInterface) ([]*metav1.APIGroup, []*metav1.APIResourceList, error) {
 	var sgs *metav1.APIGroupList
 	var resources []*metav1.APIResourceList
+	var failedGVs map[schema.GroupVersion]error
 	var err error
 
 	// If the passed discovery object implements the wider AggregatedDiscoveryInterface,
 	// then attempt to retrieve aggregated discovery with both groups and the resources.
 	if ad, ok := d.(AggregatedDiscoveryInterface); ok {
 		var resourcesByGV map[schema.GroupVersion]*metav1.APIResourceList
-		sgs, resourcesByGV, err = ad.GroupsAndMaybeResources()
+		sgs, resourcesByGV, failedGVs, err = ad.GroupsAndMaybeResources()
 		for _, resourceList := range resourcesByGV {
 			resources = append(resources, resourceList)
 		}
@@ -404,8 +447,15 @@ func ServerGroupsAndResources(d DiscoveryInterface) ([]*metav1.APIGroup, []*meta
 	for i := range sgs.Groups {
 		resultGroups = append(resultGroups, &sgs.Groups[i])
 	}
+	// resources is non-nil if aggregated discovery succeeded.
 	if resources != nil {
-		return resultGroups, resources, nil
+		// Any stale Group/Versions returned by aggregated discovery
+		// must be surfaced to the caller as failed Group/Versions.
+		var ferr error
+		if len(failedGVs) > 0 {
+			ferr = &ErrGroupDiscoveryFailed{Groups: failedGVs}
+		}
+		return resultGroups, resources, ferr
 	}
 
 	groupVersionResources, failedGroups := fetchGroupVersionResources(d, sgs)
@@ -436,16 +486,18 @@ func ServerPreferredResources(d DiscoveryInterface) ([]*metav1.APIResourceList,
 	var err error
 
 	// If the passed discovery object implements the wider AggregatedDiscoveryInterface,
-	// then it is attempt to retrieve both the groups and the resources.
+	// then it is attempt to retrieve both the groups and the resources. "failedGroups"
+	// are Group/Versions returned as stale in AggregatedDiscovery format.
 	ad, ok := d.(AggregatedDiscoveryInterface)
 	if ok {
-		serverGroupList, groupVersionResources, err = ad.GroupsAndMaybeResources()
+		serverGroupList, groupVersionResources, failedGroups, err = ad.GroupsAndMaybeResources()
 	} else {
 		serverGroupList, err = d.ServerGroups()
 	}
 	if err != nil {
 		return nil, err
 	}
+	// Non-aggregated discovery must fetch resources from Groups.
 	if groupVersionResources == nil {
 		groupVersionResources, failedGroups = fetchGroupVersionResources(d, serverGroupList)
 	}

discovery/discovery_client_test.go

@@ -110,7 +110,6 @@ func TestGetServerGroupsWithV1Server(t *testing.T) {
 	}))
 	defer server.Close()
 	client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
-	// ServerGroups should not return an error even if server returns error at /api and /apis
 	apiGroupList, err := client.ServerGroups()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -121,32 +120,49 @@ func TestGetServerGroupsWithV1Server(t *testing.T) {
 	}
 }
 
-func TestGetServerGroupsWithBrokenServer(t *testing.T) {
-	for _, statusCode := range []int{http.StatusNotFound, http.StatusForbidden} {
-		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-			w.WriteHeader(statusCode)
-		}))
-		defer server.Close()
-		client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
-		// ServerGroups should not return an error even if server returns Not Found or Forbidden error at all end points
-		apiGroupList, err := client.ServerGroups()
+func TestDiscoveryToleratesMissingCoreGroup(t *testing.T) {
+	// Discovery tolerates 404 from /api. Aggregated api servers can do this.
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		var obj interface{}
+		switch req.URL.Path {
+		case "/api":
+			w.WriteHeader(http.StatusNotFound)
+		case "/apis":
+			obj = &metav1.APIGroupList{
+				Groups: []metav1.APIGroup{
+					{
+						Name: "extensions",
+						Versions: []metav1.GroupVersionForDiscovery{
+							{GroupVersion: "extensions/v1beta1"},
+						},
+					},
+				},
+			}
+		}
+		output, err := json.Marshal(obj)
 		if err != nil {
-			t.Fatalf("unexpected error: %v", err)
-		}
-		groupVersions := metav1.ExtractGroupVersions(apiGroupList)
-		if len(groupVersions) != 0 {
-			t.Errorf("expected empty list, got: %q", groupVersions)
+			t.Fatalf("unexpected encoding error: %v", err)
+			return
 		}
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		w.Write(output)
+	}))
+	defer server.Close()
+	client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
+	// ServerGroups should not return an error even if server returns 404 at /api.
+	apiGroupList, err := client.ServerGroups()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	groupVersions := metav1.ExtractGroupVersions(apiGroupList)
+	if !reflect.DeepEqual(groupVersions, []string{"extensions/v1beta1"}) {
+		t.Errorf("expected: %q, got: %q", []string{"extensions/v1beta1"}, groupVersions)
 	}
 }
 
-func TestTimeoutIsSet(t *testing.T) {
-	cfg := &restclient.Config{}
-	setDiscoveryDefaults(cfg)
-	assert.Equal(t, defaultTimeout, cfg.Timeout)
-}
-
-func TestGetServerResourcesWithV1Server(t *testing.T) {
+func TestDiscoveryFailsWhenNonCoreGroupsMissing(t *testing.T) {
+	// Discovery fails when /apis returns 404.
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		var obj interface{}
 		switch req.URL.Path {
@@ -156,13 +172,12 @@ func TestGetServerResourcesWithV1Server(t *testing.T) {
 					"v1",
 				},
 			}
-		default:
+		case "/apis":
 			w.WriteHeader(http.StatusNotFound)
-			return
 		}
 		output, err := json.Marshal(obj)
 		if err != nil {
-			t.Errorf("unexpected encoding error: %v", err)
+			t.Fatalf("unexpected encoding error: %v", err)
 			return
 		}
 		w.Header().Set("Content-Type", "application/json")
@@ -171,17 +186,34 @@ func TestGetServerResourcesWithV1Server(t *testing.T) {
 	}))
 	defer server.Close()
 	client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
-	// ServerResources should not return an error even if server returns error at /api/v1.
-	_, serverResources, err := client.ServerGroupsAndResources()
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
+	_, err := client.ServerGroups()
+	if err == nil {
+		t.Fatal("expected error, received none")
 	}
-	gvs := groupVersions(serverResources)
-	if !sets.NewString(gvs...).Has("v1") {
-		t.Errorf("missing v1 in resource list: %v", serverResources)
+}
+
+func TestGetServerGroupsWithBrokenServer(t *testing.T) {
+	// 404 Not Found errors because discovery at /apis returns an error.
+	// 403 Forbidden errors because discovery at both /api and /apis returns error.
+	for _, statusCode := range []int{http.StatusNotFound, http.StatusForbidden} {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			w.WriteHeader(statusCode)
+		}))
+		defer server.Close()
+		client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
+		_, err := client.ServerGroups()
+		if err == nil {
+			t.Fatal("expected error, received none")
+		}
 	}
 }
 
+func TestTimeoutIsSet(t *testing.T) {
+	cfg := &restclient.Config{}
+	setDiscoveryDefaults(cfg)
+	assert.Equal(t, defaultTimeout, cfg.Timeout)
+}
+
 func TestGetServerResourcesForGroupVersion(t *testing.T) {
 	stable := metav1.APIResourceList{
 		GroupVersion: "v1",
@@ -964,17 +996,33 @@ func TestServerPreferredNamespacedResources(t *testing.T) {
 		expected map[schema.GroupVersionResource]struct{}
 	}{
 		{
+			// Combines discovery for /api and /apis.
 			response: func(w http.ResponseWriter, req *http.Request) {
 				var list interface{}
 				switch req.URL.Path {
-				case "/api/v1":
-					list = &stable
 				case "/api":
 					list = &metav1.APIVersions{
 						Versions: []string{
 							"v1",
 						},
 					}
+				case "/api/v1":
+					list = &stable
+				case "/apis":
+					list = &metav1.APIGroupList{
+						Groups: []metav1.APIGroup{
+							{
+								Name: "batch",
+								Versions: []metav1.GroupVersionForDiscovery{
+									{GroupVersion: "batch/v1", Version: "v1"},
+								},
+								PreferredVersion: metav1.GroupVersionForDiscovery{GroupVersion: "batch/v1", Version: "v1"},
+							},
+						},
+					}
+				case "/apis/batch/v1":
+					list = &batchv1
+
 				default:
 					t.Logf("unexpected request: %s", req.URL.Path)
 					w.WriteHeader(http.StatusNotFound)
@@ -990,11 +1038,14 @@ func TestServerPreferredNamespacedResources(t *testing.T) {
 				w.Write(output)
 			},
 			expected: map[schema.GroupVersionResource]struct{}{
-				{Group: "", Version: "v1", Resource: "pods"}:     {},
-				{Group: "", Version: "v1", Resource: "services"}: {},
+				{Group: "", Version: "v1", Resource: "pods"}:      {},
+				{Group: "", Version: "v1", Resource: "services"}:  {},
+				{Group: "batch", Version: "v1", Resource: "jobs"}: {},
 			},
 		},
 		{
+			// Only return /apis (not legacy /api); does not error. 404 for legacy
+			// core/v1 at /api is tolerated.
 			response: func(w http.ResponseWriter, req *http.Request) {
 				var list interface{}
 				switch req.URL.Path {
@@ -1349,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)
 		}))
@@ -1384,6 +1436,7 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 		expectedGroupNames    []string
 		expectedGroupVersions []string
 		expectedGVKs          []string
+		expectedFailedGVs     []string
 	}{
 		{
 			name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/1 resources at /apis",
@@ -1512,6 +1565,78 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 				"apps/v2/Deployment",
 			},
 		},
+		{
+			name: "Aggregated discovery: 1 group/1 resources at /api, 1 group/2 versions/1 resources at /apis",
+			corev1: &apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "pods",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "",
+											Version: "v1",
+											Kind:    "Pod",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			apis: &apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "apps",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+							},
+							{
+								Version: "v2",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v2",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
+						},
+					},
+				},
+			},
+			expectedGroupNames:    []string{"", "apps"},
+			expectedGroupVersions: []string{"v1", "apps/v1"},
+			expectedGVKs: []string{
+				"/v1/Pod",
+				"apps/v1/Deployment",
+			},
+			expectedFailedGVs: []string{"apps/v2"},
+		},
 		{
 			name: "Aggregated discovery: 1 group/2 resources at /api, 1 group/2 resources at /apis",
 			corev1: &apidiscovery.APIGroupDiscoveryList{
@@ -1552,6 +1677,31 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 							Name: "apps",
 						},
 						Versions: []apidiscovery.APIVersionDiscovery{
+							// Stale "v2" version not included.
+							{
+								Version: "v2",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v2",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "statefulsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v2",
+											Kind:    "StatefulSet",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
 							{
 								Version: "v1",
 								Resources: []apidiscovery.APIResourceDiscovery{
@@ -1587,9 +1737,10 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 				"apps/v1/Deployment",
 				"apps/v1/StatefulSet",
 			},
+			expectedFailedGVs: []string{"apps/v2"},
 		},
 		{
-			name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources at /apis",
+			name: "Aggregated discovery: 1 group/2 resources at /api, 2 group/2 resources/1 stale GV at /apis",
 			corev1: &apidiscovery.APIGroupDiscoveryList{
 				Items: []apidiscovery.APIGroupDiscovery{
 					{
@@ -1658,6 +1809,7 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 							Name: "batch",
 						},
 						Versions: []apidiscovery.APIVersionDiscovery{
+							// Stale Group/Version is not included
 							{
 								Version: "v1",
 								Resources: []apidiscovery.APIResourceDiscovery{
@@ -1680,21 +1832,46 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 										Scope: apidiscovery.ScopeNamespace,
 									},
 								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
+							{
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "jobs",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "batch",
+											Version: "v1beta1",
+											Kind:    "Job",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "cronjobs",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "batch",
+											Version: "v1beta1",
+											Kind:    "CronJob",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
 							},
 						},
 					},
 				},
 			},
 			expectedGroupNames:    []string{"", "apps", "batch"},
-			expectedGroupVersions: []string{"v1", "apps/v1", "batch/v1"},
+			expectedGroupVersions: []string{"v1", "apps/v1", "batch/v1beta1"},
 			expectedGVKs: []string{
 				"/v1/Pod",
 				"/v1/Service",
 				"apps/v1/Deployment",
 				"apps/v1/StatefulSet",
-				"batch/v1/Job",
-				"batch/v1/CronJob",
+				"batch/v1beta1/Job",
+				"batch/v1beta1/CronJob",
 			},
+			expectedFailedGVs: []string{"batch/v1"},
 		},
 		{
 			name:   "Aggregated discovery: /api returns nothing, 2 groups/2 resources at /apis",
@@ -1759,6 +1936,31 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 									},
 								},
 							},
+							{
+								// Stale "v1beta1" not included.
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "jobs",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "batch",
+											Version: "v1beta1",
+											Kind:    "Job",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "cronjobs",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "batch",
+											Version: "v1beta1",
+											Kind:    "CronJob",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
 						},
 					},
 				},
@@ -1771,6 +1973,7 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 				"batch/v1/Job",
 				"batch/v1/CronJob",
 			},
+			expectedFailedGVs: []string{"batch/v1beta1"},
 		},
 	}
 
@@ -1788,15 +1991,24 @@ 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)
 		}))
 		defer server.Close()
 		client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
 		apiGroups, resources, err := client.ServerGroupsAndResources()
-		require.NoError(t, err)
+		if len(test.expectedFailedGVs) > 0 {
+			require.Error(t, err)
+			expectedFailedGVs := sets.NewString(test.expectedFailedGVs...)
+			actualFailedGVs := sets.NewString(failedGroupVersions(err)...)
+			assert.True(t, expectedFailedGVs.Equal(actualFailedGVs),
+				"%s: Expected Failed GVs (%s), got (%s)", test.name, expectedFailedGVs, actualFailedGVs)
+		} else {
+			require.NoError(t, err)
+		}
 		// Test the expected groups are returned for the aggregated format.
 		expectedGroupNames := sets.NewString(test.expectedGroupNames...)
 		actualGroupNames := sets.NewString(groupNames(apiGroups)...)
@@ -1823,12 +2035,139 @@ func TestAggregatedServerGroupsAndResources(t *testing.T) {
 	}
 }
 
+func TestAggregatedServerGroupsAndResourcesWithErrors(t *testing.T) {
+	tests := []struct {
+		name              string
+		corev1            *apidiscovery.APIGroupDiscoveryList
+		coreHttpStatus    int
+		apis              *apidiscovery.APIGroupDiscoveryList
+		apisHttpStatus    int
+		expectedGroups    []string
+		expectedResources []string
+		expectedErr       bool
+	}{
+		{
+			name:           "Aggregated Discovery: 404 for core/v1 is tolerated",
+			corev1:         &apidiscovery.APIGroupDiscoveryList{},
+			coreHttpStatus: http.StatusNotFound,
+			apis: &apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "apps",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "daemonsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1",
+											Kind:    "DaemonSet",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			apisHttpStatus:    http.StatusOK,
+			expectedGroups:    []string{"apps"},
+			expectedResources: []string{"apps/v1/Deployment", "apps/v1/DaemonSet"},
+			expectedErr:       false,
+		},
+		{
+			name:           "Aggregated Discovery: 403 for core/v1 causes error",
+			corev1:         &apidiscovery.APIGroupDiscoveryList{},
+			coreHttpStatus: http.StatusForbidden,
+			apis:           &apidiscovery.APIGroupDiscoveryList{},
+			apisHttpStatus: http.StatusOK,
+			expectedErr:    true,
+		},
+		{
+			name:           "Aggregated Discovery: 404 for /apis causes error",
+			corev1:         &apidiscovery.APIGroupDiscoveryList{},
+			coreHttpStatus: http.StatusOK,
+			apis:           &apidiscovery.APIGroupDiscoveryList{},
+			apisHttpStatus: http.StatusNotFound,
+			expectedErr:    true,
+		},
+		{
+			name:           "Aggregated Discovery: 403 for /apis causes error",
+			corev1:         &apidiscovery.APIGroupDiscoveryList{},
+			coreHttpStatus: http.StatusOK,
+			apis:           &apidiscovery.APIGroupDiscoveryList{},
+			apisHttpStatus: http.StatusForbidden,
+			expectedErr:    true,
+		},
+	}
+
+	for _, test := range tests {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			var agg *apidiscovery.APIGroupDiscoveryList
+			var status int
+			switch req.URL.Path {
+			case "/api":
+				agg = test.corev1
+				status = test.coreHttpStatus
+			case "/apis":
+				agg = test.apis
+				status = test.apisHttpStatus
+			default:
+				w.WriteHeader(http.StatusNotFound)
+				return
+			}
+			output, err := json.Marshal(agg)
+			require.NoError(t, err)
+			// 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)
+		}))
+		defer server.Close()
+		client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
+		apiGroups, resources, err := client.ServerGroupsAndResources()
+		if test.expectedErr {
+			require.Error(t, err)
+			require.Nil(t, apiGroups)
+			require.Nil(t, resources)
+			continue
+		}
+		require.NoError(t, err)
+		// First check the returned groups
+		expectedGroups := sets.NewString(test.expectedGroups...)
+		actualGroups := sets.NewString(groupNames(apiGroups)...)
+		assert.True(t, expectedGroups.Equal(actualGroups),
+			"%s: Expected GVKs (%s), got (%s)", test.name, expectedGroups.List(), actualGroups.List())
+		// Next check the returned resources
+		expectedGVKs := sets.NewString(test.expectedResources...)
+		actualGVKs := sets.NewString(groupVersionKinds(resources)...)
+		assert.True(t, expectedGVKs.Equal(actualGVKs),
+			"%s: Expected GVKs (%s), got (%s)", test.name, expectedGVKs.List(), actualGVKs.List())
+	}
+}
+
 func TestAggregatedServerPreferredResources(t *testing.T) {
 	tests := []struct {
-		name         string
-		corev1       *apidiscovery.APIGroupDiscoveryList
-		apis         *apidiscovery.APIGroupDiscoveryList
-		expectedGVKs []string
+		name              string
+		corev1            *apidiscovery.APIGroupDiscoveryList
+		apis              *apidiscovery.APIGroupDiscoveryList
+		expectedGVKs      []string
+		expectedFailedGVs []string
 	}{
 		{
 			name: "Aggregated discovery: basic corev1 and apps/v1 preferred resources returned",
@@ -1954,6 +2293,78 @@ func TestAggregatedServerPreferredResources(t *testing.T) {
 				"apps/v2/Deployment",
 			},
 		},
+		{
+			name: "Aggregated discovery: stale Group/Version can not produce preferred version",
+			corev1: &apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						Versions: []apidiscovery.APIVersionDiscovery{
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "pods",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "",
+											Version: "v1",
+											Kind:    "Pod",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			apis: &apidiscovery.APIGroupDiscoveryList{
+				Items: []apidiscovery.APIGroupDiscovery{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "apps",
+						},
+						Versions: []apidiscovery.APIVersionDiscovery{
+							// v2 is "stale", so it can not be "preferred".
+							{
+								Version: "v2",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v2",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
+							{
+								Version: "v1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			// Only v1 resources from apps group; v2 would be preferred but it is "stale".
+			expectedGVKs: []string{
+				"/v1/Pod",
+				"apps/v1/Deployment",
+			},
+			expectedFailedGVs: []string{"apps/v2"},
+		},
 		{
 			name: "Aggregated discovery: preferred multiple resources from multiple group/versions",
 			corev1: &apidiscovery.APIGroupDiscoveryList{
@@ -2017,6 +2428,30 @@ func TestAggregatedServerPreferredResources(t *testing.T) {
 									},
 								},
 							},
+							{
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "statefulsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "StatefulSet",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+								Freshness: apidiscovery.DiscoveryFreshnessStale,
+							},
 						},
 					},
 				},
@@ -2027,6 +2462,7 @@ func TestAggregatedServerPreferredResources(t *testing.T) {
 				"apps/v1/Deployment",
 				"apps/v1/StatefulSet",
 			},
+			expectedFailedGVs: []string{"apps/v1beta1"},
 		},
 		{
 			name: "Aggregated discovery: resources from multiple preferred group versions at /apis",
@@ -2091,6 +2527,30 @@ func TestAggregatedServerPreferredResources(t *testing.T) {
 									},
 								},
 							},
+							{
+								// Not included because "v1" is preferred.
+								Version: "v1beta1",
+								Resources: []apidiscovery.APIResourceDiscovery{
+									{
+										Resource: "deployments",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "Deployment",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+									{
+										Resource: "statefulsets",
+										ResponseKind: &metav1.GroupVersionKind{
+											Group:   "apps",
+											Version: "v1beta1",
+											Kind:    "StatefulSet",
+										},
+										Scope: apidiscovery.ScopeNamespace,
+									},
+								},
+							},
 						},
 					},
 					{
@@ -2228,6 +2688,7 @@ func TestAggregatedServerPreferredResources(t *testing.T) {
 								},
 							},
 							{
+								// Not included, since "v1" is preferred.
 								Version: "v1beta1",
 								Resources: []apidiscovery.APIResourceDiscovery{
 									{
@@ -2280,15 +2741,24 @@ 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)
 		}))
 		defer server.Close()
 		client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
 		resources, err := client.ServerPreferredResources()
-		require.NoError(t, err)
+		if len(test.expectedFailedGVs) > 0 {
+			require.Error(t, err)
+			expectedFailedGVs := sets.NewString(test.expectedFailedGVs...)
+			actualFailedGVs := sets.NewString(failedGroupVersions(err)...)
+			assert.True(t, expectedFailedGVs.Equal(actualFailedGVs),
+				"%s: Expected Failed GVs (%s), got (%s)", test.name, expectedFailedGVs, actualFailedGVs)
+		} else {
+			require.NoError(t, err)
+		}
 		// Test the expected preferred GVKs are returned from the aggregated discovery.
 		expectedGVKs := sets.NewString(test.expectedGVKs...)
 		actualGVKs := sets.NewString(groupVersionKinds(resources)...)
@@ -2297,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) {
@@ -2370,3 +2892,15 @@ func groupVersionKinds(resources []*metav1.APIResourceList) []string {
 	}
 	return result
 }
+
+func failedGroupVersions(err error) []string {
+	result := []string{}
+	ferr, ok := err.(*ErrGroupDiscoveryFailed)
+	if !ok {
+		return result
+	}
+	for gv := range ferr.Groups {
+		result = append(result, gv.String())
+	}
+	return result
+}

discovery/helper_blackbox_test.go

@@ -65,6 +65,22 @@ func TestServerSupportsVersion(t *testing.T) {
 			expectErr:       func(err error) bool { return strings.Contains(err.Error(), `server does not support API version "v1"`) },
 			statusCode:      http.StatusOK,
 		},
+		{
+			name:            "Status 403 Forbidden for core/v1 group returns error and is unsupported",
+			requiredVersion: schema.GroupVersion{Version: "v1"},
+			serverVersions:  []string{"/version1", v1.SchemeGroupVersion.String()},
+			expectErr:       func(err error) bool { return strings.Contains(err.Error(), "unknown") },
+			statusCode:      http.StatusForbidden,
+		},
+		{
+			name:            "Status 404 Not Found for core/v1 group returns empty and is unsupported",
+			requiredVersion: schema.GroupVersion{Version: "v1"},
+			serverVersions:  []string{"/version1", v1.SchemeGroupVersion.String()},
+			expectErr: func(err error) bool {
+				return strings.Contains(err.Error(), "server could not find the requested resource")
+			},
+			statusCode: http.StatusNotFound,
+		},
 		{
 			name:           "connection refused error",
 			serverVersions: []string{"version1"},
@@ -72,11 +88,6 @@ func TestServerSupportsVersion(t *testing.T) {
 			expectErr:      func(err error) bool { return strings.Contains(err.Error(), "connection refused") },
 			statusCode:     http.StatusOK,
 		},
-		{
-			name:            "discovery fails due to 404 Not Found errors and thus serverVersions is empty, use requested GroupVersion",
-			requiredVersion: schema.GroupVersion{Version: "version1"},
-			statusCode:      http.StatusNotFound,
-		},
 	}
 
 	for _, test := range tests {

go.mod

@@ -13,19 +13,19 @@ require (
 	github.com/google/gnostic v0.5.7-v3refs
 	github.com/google/go-cmp v0.5.9
 	github.com/google/gofuzz v1.1.0
-	github.com/google/uuid v1.1.2
+	github.com/google/uuid v1.3.0
 	github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7
 	github.com/imdario/mergo v0.3.6
 	github.com/peterbourgon/diskv v2.0.1+incompatible
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.0
-	golang.org/x/net v0.1.1-0.20221027164007-c63010009c80
+	golang.org/x/net v0.17.0
 	golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b
-	golang.org/x/term v0.1.0
+	golang.org/x/term v0.13.0
 	golang.org/x/time v0.0.0-20220210224613-90d013bbcef8
 	google.golang.org/protobuf v1.28.1
-	k8s.io/api v0.0.0-20221112014728-9e1815a99d4f
-	k8s.io/apimachinery v0.0.0-20221108055230-fd8a60496be5
+	k8s.io/api v0.26.10
+	k8s.io/apimachinery v0.26.10
 	k8s.io/klog/v2 v2.80.1
 	k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280
 	k8s.io/utils v0.0.0-20221107191617-1a15be271d1d
@@ -49,8 +49,8 @@ require (
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	golang.org/x/sys v0.1.0 // indirect
-	golang.org/x/text v0.4.0 // indirect
+	golang.org/x/sys v0.13.0 // indirect
+	golang.org/x/text v0.13.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
@@ -59,6 +59,6 @@ require (
 )
 
 replace (
-	k8s.io/api => k8s.io/api v0.0.0-20221112014728-9e1815a99d4f
-	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20221108055230-fd8a60496be5
+	k8s.io/api => k8s.io/api v0.26.10
+	k8s.io/apimachinery => k8s.io/apimachinery v0.26.10
 )

go.sum

@@ -128,8 +128,8 @@ github.com/google/pprof v0.0.0-20200229191704-1ebb73c60ed3/go.mod h1:ZgVRPoUq/hf
 github.com/google/pprof v0.0.0-20200430221834-fc25d7d30c6d/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM=
 github.com/google/pprof v0.0.0-20200708004538-1a94d8640e99/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
-github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y=
-github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
+github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
 github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
 github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
@@ -263,8 +263,8 @@ golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81R
 golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
 golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
-golang.org/x/net v0.1.1-0.20221027164007-c63010009c80 h1:CtRWmqbiPSOXwJV1JoY7pWiTx2xzVKQ813bvU+Y/9jI=
-golang.org/x/net v0.1.1-0.20221027164007-c63010009c80/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco=
+golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM=
+golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -309,19 +309,19 @@ golang.org/x/sys v0.0.0-20200803210538-64077c9b5642/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U=
-golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
+golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
-golang.org/x/term v0.1.0 h1:g6Z6vPFA9dYBAF7DWcH6sCcOntplXsDKcliusYijMlw=
-golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
+golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek=
+golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
-golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg=
-golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
+golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
+golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
 golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
@@ -476,10 +476,10 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
-k8s.io/api v0.0.0-20221112014728-9e1815a99d4f h1:ktcfuKz8wVGjfjJ+qyGhcepyyYcbsxLXwP41rZwHvGA=
-k8s.io/api v0.0.0-20221112014728-9e1815a99d4f/go.mod h1:j2jT1HZpNN4eUpl6xrwjWC1amreYNCdsevVdZMhBz5o=
-k8s.io/apimachinery v0.0.0-20221108055230-fd8a60496be5 h1:iFAMJ1evvrO6X7dS7EKujS6An+bp3u/dD6opu8rn0QA=
-k8s.io/apimachinery v0.0.0-20221108055230-fd8a60496be5/go.mod h1:VXMmlsE7YRJ5vyAyWpkKIfFkEbDNpVs0ObpkuQf1WfM=
+k8s.io/api v0.26.10 h1:skTnrDR0r8dg4MMLf6YZIzugxNM0BjFsWKPkNc5kOvk=
+k8s.io/api v0.26.10/go.mod h1:ou/H3yviqrHtP/DSPVTfsc7qNfmU06OhajytJfYXkXw=
+k8s.io/apimachinery v0.26.10 h1:aE+J2KIbjctFqPp3Y0q4Wh2PD+l1p2g3Zp4UYjSvtGU=
+k8s.io/apimachinery v0.26.10/go.mod h1:iT1ZP4JBP34wwM+ZQ8ByPEQ81u043iqAcsJYftX9amM=
 k8s.io/klog/v2 v2.80.1 h1:atnLQ121W371wYYFawwYx1aEY2eUfs4l3J72wtgAwV4=
 k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
 k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 h1:+70TFaan3hfJzs+7VK2o+OGxg8HsuBr/5f6tVAjDu6E=

openapi/client.go

@@ -19,6 +19,7 @@ package openapi
 import (
 	"context"
 	"encoding/json"
+	"strings"
 
 	"k8s.io/client-go/rest"
 	"k8s.io/kube-openapi/pkg/handler3"
@@ -58,7 +59,11 @@ func (c *client) Paths() (map[string]GroupVersion, error) {
 	// Create GroupVersions for each element of the result
 	result := map[string]GroupVersion{}
 	for k, v := range discoMap.Paths {
-		result[k] = newGroupVersion(c, v)
+		// If the server returned a URL rooted at /openapi/v3, preserve any additional client-side prefix.
+		// If the server returned a URL not rooted at /openapi/v3, treat it as an actual server-relative URL.
+		// See https://github.com/kubernetes/kubernetes/issues/117463 for details
+		useClientPrefix := strings.HasPrefix(v.ServerRelativeURL, "/openapi/v3")
+		result[k] = newGroupVersion(c, v, useClientPrefix)
 	}
 	return result, nil
 }

openapi/groupversion.go

@@ -18,6 +18,7 @@ package openapi
 
 import (
 	"context"
+	"net/url"
 
 	"k8s.io/kube-openapi/pkg/handler3"
 )
@@ -29,18 +30,41 @@ type GroupVersion interface {
 }
 
 type groupversion struct {
-	client *client
-	item   handler3.OpenAPIV3DiscoveryGroupVersion
+	client          *client
+	item            handler3.OpenAPIV3DiscoveryGroupVersion
+	useClientPrefix bool
 }
 
-func newGroupVersion(client *client, item handler3.OpenAPIV3DiscoveryGroupVersion) *groupversion {
-	return &groupversion{client: client, item: item}
+func newGroupVersion(client *client, item handler3.OpenAPIV3DiscoveryGroupVersion, useClientPrefix bool) *groupversion {
+	return &groupversion{client: client, item: item, useClientPrefix: useClientPrefix}
 }
 
 func (g *groupversion) Schema(contentType string) ([]byte, error) {
-	return g.client.restClient.Get().
-		RequestURI(g.item.ServerRelativeURL).
-		SetHeader("Accept", contentType).
-		Do(context.TODO()).
-		Raw()
+	if !g.useClientPrefix {
+		return g.client.restClient.Get().
+			RequestURI(g.item.ServerRelativeURL).
+			SetHeader("Accept", contentType).
+			Do(context.TODO()).
+			Raw()
+	}
+
+	locator, err := url.Parse(g.item.ServerRelativeURL)
+	if err != nil {
+		return nil, err
+	}
+
+	path := g.client.restClient.Get().
+		AbsPath(locator.Path).
+		SetHeader("Accept", contentType)
+
+	// Other than root endpoints(openapiv3/apis), resources have hash query parameter to support etags.
+	// However, absPath does not support handling query parameters internally,
+	// so that hash query parameter is added manually
+	for k, value := range locator.Query() {
+		for _, v := range value {
+			path.Param(k, v)
+		}
+	}
+
+	return path.Do(context.TODO()).Raw()
 }

openapi/groupversion_test.go

@@ -0,0 +1,106 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package openapi
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	appsv1 "k8s.io/api/apps/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
+)
+
+func TestGroupVersion(t *testing.T) {
+	tests := []struct {
+		name                string
+		prefix              string
+		serverReturnsPrefix bool
+	}{
+		{
+			name:                "no prefix",
+			prefix:              "",
+			serverReturnsPrefix: false,
+		},
+		{
+			name:                "prefix not in discovery",
+			prefix:              "/test-endpoint",
+			serverReturnsPrefix: false,
+		},
+		{
+			name:                "prefix in discovery",
+			prefix:              "/test-endpoint",
+			serverReturnsPrefix: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				switch {
+				case r.URL.Path == test.prefix+"/openapi/v3/apis/apps/v1" && r.URL.RawQuery == "hash=014fbff9a07c":
+					w.Header().Set("Content-Type", "application/json")
+					w.WriteHeader(http.StatusOK)
+					w.Write([]byte(`{"openapi":"3.0.0","info":{"title":"Kubernetes","version":"unversioned"}}`))
+				case r.URL.Path == test.prefix+"/openapi/v3":
+					// return root content
+					w.Header().Set("Content-Type", "application/json")
+					w.WriteHeader(http.StatusOK)
+					if test.serverReturnsPrefix {
+						w.Write([]byte(fmt.Sprintf(`{"paths":{"apis/apps/v1":{"serverRelativeURL":"%s/openapi/v3/apis/apps/v1?hash=014fbff9a07c"}}}`, test.prefix)))
+					} else {
+						w.Write([]byte(`{"paths":{"apis/apps/v1":{"serverRelativeURL":"/openapi/v3/apis/apps/v1?hash=014fbff9a07c"}}}`))
+					}
+				default:
+					t.Errorf("unexpected request: %s", r.URL.String())
+					w.WriteHeader(http.StatusNotFound)
+					return
+				}
+			}))
+			defer server.Close()
+
+			c, err := rest.RESTClientFor(&rest.Config{
+				Host: server.URL + test.prefix,
+				ContentConfig: rest.ContentConfig{
+					NegotiatedSerializer: scheme.Codecs,
+					GroupVersion:         &appsv1.SchemeGroupVersion,
+				},
+			})
+
+			if err != nil {
+				t.Fatalf("unexpected error occurred: %v", err)
+			}
+
+			openapiClient := NewClient(c)
+			paths, err := openapiClient.Paths()
+			if err != nil {
+				t.Fatalf("unexpected error occurred: %v", err)
+			}
+			schema, err := paths["apis/apps/v1"].Schema(runtime.ContentTypeJSON)
+			if err != nil {
+				t.Fatalf("unexpected error occurred: %v", err)
+			}
+			expectedResult := `{"openapi":"3.0.0","info":{"title":"Kubernetes","version":"unversioned"}}`
+			if string(schema) != expectedResult {
+				t.Fatalf("unexpected result actual: %s expected: %s", string(schema), expectedResult)
+			}
+		})
+	}
+}

rest/request.go

@@ -34,6 +34,7 @@ import (
 	"time"
 
 	"golang.org/x/net/http2"
+
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -116,8 +117,11 @@ type Request struct {
 	subresource  string
 
 	// output
-	err  error
-	body io.Reader
+	err error
+
+	// only one of body / bodyBytes may be set. requests using body are not retriable.
+	body      io.Reader
+	bodyBytes []byte
 
 	retryFn requestRetryFunc
 }
@@ -443,12 +447,15 @@ func (r *Request) Body(obj interface{}) *Request {
 			return r
 		}
 		glogBody("Request Body", data)
-		r.body = bytes.NewReader(data)
+		r.body = nil
+		r.bodyBytes = data
 	case []byte:
 		glogBody("Request Body", t)
-		r.body = bytes.NewReader(t)
+		r.body = nil
+		r.bodyBytes = t
 	case io.Reader:
 		r.body = t
+		r.bodyBytes = nil
 	case runtime.Object:
 		// callers may pass typed interface pointers, therefore we must check nil with reflection
 		if reflect.ValueOf(t).IsNil() {
@@ -465,7 +472,8 @@ func (r *Request) Body(obj interface{}) *Request {
 			return r
 		}
 		glogBody("Request Body", data)
-		r.body = bytes.NewReader(data)
+		r.body = nil
+		r.bodyBytes = data
 		r.SetHeader("Content-Type", r.c.content.ContentType)
 	default:
 		r.err = fmt.Errorf("unknown type used for body: %+v", obj)
@@ -825,9 +833,6 @@ func (r *Request) Stream(ctx context.Context) (io.ReadCloser, error) {
 		if err != nil {
 			return nil, err
 		}
-		if r.body != nil {
-			req.Body = io.NopCloser(r.body)
-		}
 		resp, err := client.Do(req)
 		updateURLMetrics(ctx, r, resp, err)
 		retry.After(ctx, r, resp, err)
@@ -889,8 +894,20 @@ func (r *Request) requestPreflightCheck() error {
 }
 
 func (r *Request) newHTTPRequest(ctx context.Context) (*http.Request, error) {
+	var body io.Reader
+	switch {
+	case r.body != nil && r.bodyBytes != nil:
+		return nil, fmt.Errorf("cannot set both body and bodyBytes")
+	case r.body != nil:
+		body = r.body
+	case r.bodyBytes != nil:
+		// Create a new reader specifically for this request.
+		// Giving each request a dedicated reader allows retries to avoid races resetting the request body.
+		body = bytes.NewReader(r.bodyBytes)
+	}
+
 	url := r.URL().String()
-	req, err := http.NewRequest(r.verb, url, r.body)
+	req, err := http.NewRequest(r.verb, url, body)
 	if err != nil {
 		return nil, err
 	}

rest/request_test.go

@@ -1122,42 +1122,6 @@ func TestRequestWatch(t *testing.T) {
 			},
 			Empty: true,
 		},
-		{
-			name: "max retries 1, server returns a retry-after response, request body seek error",
-			Request: &Request{
-				body: &readSeeker{err: io.EOF},
-				c: &RESTClient{
-					base: &url.URL{},
-				},
-			},
-			maxRetries:       1,
-			attemptsExpected: 1,
-			serverReturns: []responseErr{
-				{response: retryAfterResponse(), err: nil},
-			},
-			Err: true,
-			ErrFn: func(err error) bool {
-				return !apierrors.IsInternalError(err) && strings.Contains(err.Error(), "failed to reset the request body while retrying a request: EOF")
-			},
-		},
-		{
-			name: "max retries 1, server returns a retryable error, request body seek error",
-			Request: &Request{
-				body: &readSeeker{err: io.EOF},
-				c: &RESTClient{
-					base: &url.URL{},
-				},
-			},
-			maxRetries:       1,
-			attemptsExpected: 1,
-			serverReturns: []responseErr{
-				{response: nil, err: io.EOF},
-			},
-			Err: true,
-			ErrFn: func(err error) bool {
-				return !apierrors.IsInternalError(err)
-			},
-		},
 		{
 			name: "max retries 2, server always returns a response with Retry-After header",
 			Request: &Request{
@@ -1319,7 +1283,7 @@ func TestRequestStream(t *testing.T) {
 			},
 		},
 		{
-			name: "max retries 1, server returns a retry-after response, request body seek error",
+			name: "max retries 1, server returns a retry-after response, non-bytes request, no retry",
 			Request: &Request{
 				body: &readSeeker{err: io.EOF},
 				c: &RESTClient{
@@ -1332,9 +1296,6 @@ func TestRequestStream(t *testing.T) {
 				{response: retryAfterResponse(), err: nil},
 			},
 			Err: true,
-			ErrFn: func(err error) bool {
-				return !apierrors.IsInternalError(err) && strings.Contains(err.Error(), "failed to reset the request body while retrying a request: EOF")
-			},
 		},
 		{
 			name: "max retries 2, server always returns a response with Retry-After header",
@@ -2016,20 +1977,24 @@ func TestBody(t *testing.T) {
 			}
 		}
 
-		if r.body == nil {
+		req, err := r.newHTTPRequest(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if req.Body == nil {
 			if len(tt.expected) != 0 {
-				t.Errorf("%d: r.body = %q; want %q", i, r.body, tt.expected)
+				t.Errorf("%d: req.Body = %q; want %q", i, req.Body, tt.expected)
 			}
 			continue
 		}
 		buf := make([]byte, len(tt.expected))
-		if _, err := r.body.Read(buf); err != nil {
-			t.Errorf("%d: r.body.Read error: %v", i, err)
+		if _, err := req.Body.Read(buf); err != nil {
+			t.Errorf("%d: req.Body.Read error: %v", i, err)
 			continue
 		}
 		body := string(buf)
 		if body != tt.expected {
-			t.Errorf("%d: r.body = %q; want %q", i, body, tt.expected)
+			t.Errorf("%d: req.Body = %q; want %q", i, body, tt.expected)
 		}
 	}
 }
@@ -2640,6 +2605,7 @@ func TestRequestWithRetry(t *testing.T) {
 	tests := []struct {
 		name                         string
 		body                         io.Reader
+		bodyBytes                    []byte
 		serverReturns                responseErr
 		errExpected                  error
 		errContains                  string
@@ -2647,53 +2613,53 @@ func TestRequestWithRetry(t *testing.T) {
 		roundTripInvokedExpected     int
 	}{
 		{
-			name:                         "server returns retry-after response, request body is not io.Seeker, retry goes ahead",
-			body:                         io.NopCloser(bytes.NewReader([]byte{})),
+			name:                         "server returns retry-after response, no request body, retry goes ahead",
+			bodyBytes:                    nil,
 			serverReturns:                responseErr{response: retryAfterResponse(), err: nil},
 			errExpected:                  nil,
 			transformFuncInvokedExpected: 1,
 			roundTripInvokedExpected:     2,
 		},
 		{
-			name:                         "server returns retry-after response, request body Seek returns error, retry aborted",
-			body:                         &readSeeker{err: io.EOF},
-			serverReturns:                responseErr{response: retryAfterResponse(), err: nil},
-			errExpected:                  nil,
-			transformFuncInvokedExpected: 0,
-			roundTripInvokedExpected:     1,
-		},
-		{
-			name:                         "server returns retry-after response, request body Seek returns no error, retry goes ahead",
-			body:                         &readSeeker{err: nil},
+			name:                         "server returns retry-after response, bytes request body, retry goes ahead",
+			bodyBytes:                    []byte{},
 			serverReturns:                responseErr{response: retryAfterResponse(), err: nil},
 			errExpected:                  nil,
 			transformFuncInvokedExpected: 1,
 			roundTripInvokedExpected:     2,
 		},
 		{
-			name:                         "server returns retryable err, request body is not io.Seek, retry goes ahead",
-			body:                         io.NopCloser(bytes.NewReader([]byte{})),
-			serverReturns:                responseErr{response: nil, err: io.ErrUnexpectedEOF},
-			errExpected:                  io.ErrUnexpectedEOF,
-			transformFuncInvokedExpected: 0,
-			roundTripInvokedExpected:     2,
-		},
-		{
-			name:                         "server returns retryable err, request body Seek returns error, retry aborted",
-			body:                         &readSeeker{err: io.EOF},
-			serverReturns:                responseErr{response: nil, err: io.ErrUnexpectedEOF},
-			errContains:                  "failed to reset the request body while retrying a request: EOF",
-			transformFuncInvokedExpected: 0,
+			name:                         "server returns retry-after response, opaque request body, retry aborted",
+			body:                         &readSeeker{},
+			serverReturns:                responseErr{response: retryAfterResponse(), err: nil},
+			errExpected:                  nil,
+			transformFuncInvokedExpected: 1,
 			roundTripInvokedExpected:     1,
 		},
 		{
-			name:                         "server returns retryable err, request body Seek returns no err, retry goes ahead",
-			body:                         &readSeeker{err: nil},
+			name:                         "server returns retryable err, no request body, retry goes ahead",
+			bodyBytes:                    nil,
 			serverReturns:                responseErr{response: nil, err: io.ErrUnexpectedEOF},
 			errExpected:                  io.ErrUnexpectedEOF,
 			transformFuncInvokedExpected: 0,
 			roundTripInvokedExpected:     2,
 		},
+		{
+			name:                         "server returns retryable err, bytes request body, retry goes ahead",
+			bodyBytes:                    []byte{},
+			serverReturns:                responseErr{response: nil, err: io.ErrUnexpectedEOF},
+			errExpected:                  io.ErrUnexpectedEOF,
+			transformFuncInvokedExpected: 0,
+			roundTripInvokedExpected:     2,
+		},
+		{
+			name:                         "server returns retryable err, opaque request body, retry aborted",
+			body:                         &readSeeker{},
+			serverReturns:                responseErr{response: nil, err: io.ErrUnexpectedEOF},
+			errExpected:                  io.ErrUnexpectedEOF,
+			transformFuncInvokedExpected: 0,
+			roundTripInvokedExpected:     1,
+		},
 	}
 
 	for _, test := range tests {
@@ -2864,7 +2830,8 @@ func testRequestWithRetry(t *testing.T, key string, doFunc func(ctx context.Cont
 	tests := []struct {
 		name          string
 		verb          string
-		body          func() io.Reader
+		body          io.Reader
+		bodyBytes     []byte
 		maxRetries    int
 		serverReturns []responseErr
 
@@ -2874,7 +2841,7 @@ func testRequestWithRetry(t *testing.T, key string, doFunc func(ctx context.Cont
 		{
 			name:       "server always returns retry-after response",
 			verb:       "GET",
-			body:       func() io.Reader { return bytes.NewReader([]byte{}) },
+			bodyBytes:  []byte{},
 			maxRetries: 2,
 			serverReturns: []responseErr{
 				{response: retryAfterResponse(), err: nil},
@@ -2902,7 +2869,7 @@ func testRequestWithRetry(t *testing.T, key string, doFunc func(ctx context.Cont
 		{
 			name:       "server always returns retryable error",
 			verb:       "GET",
-			body:       func() io.Reader { return bytes.NewReader([]byte{}) },
+			bodyBytes:  []byte{},
 			maxRetries: 2,
 			serverReturns: []responseErr{
 				{response: nil, err: io.EOF},
@@ -2931,7 +2898,7 @@ func testRequestWithRetry(t *testing.T, key string, doFunc func(ctx context.Cont
 		{
 			name:       "server returns success on the final retry",
 			verb:       "GET",
-			body:       func() io.Reader { return bytes.NewReader([]byte{}) },
+			bodyBytes:  []byte{},
 			maxRetries: 2,
 			serverReturns: []responseErr{
 				{response: retryAfterResponse(), err: nil},
@@ -2978,13 +2945,10 @@ func testRequestWithRetry(t *testing.T, key string, doFunc func(ctx context.Cont
 				return resp, test.serverReturns[attempts].err
 			})
 
-			reqCountGot := newCount()
-			reqRecorder := newReadTracker(reqCountGot)
-			reqRecorder.delegated = test.body()
-
 			req := &Request{
-				verb: test.verb,
-				body: reqRecorder,
+				verb:      test.verb,
+				body:      test.body,
+				bodyBytes: test.bodyBytes,
 				c: &RESTClient{
 					content: defaultContentConfig(),
 					Client:  client,
@@ -3004,9 +2968,6 @@ func testRequestWithRetry(t *testing.T, key string, doFunc func(ctx context.Cont
 				t.Errorf("Expected retries: %d, but got: %d", expected.attempts, attempts)
 			}
 
-			if !reflect.DeepEqual(expected.reqCount.seeks, reqCountGot.seeks) {
-				t.Errorf("Expected request body to have seek invocation: %v, but got: %v", expected.reqCount.seeks, reqCountGot.seeks)
-			}
 			if expected.respCount.closes != respCountGot.getCloseCount() {
 				t.Errorf("Expected response body Close to be invoked %d times, but got: %d", expected.respCount.closes, respCountGot.getCloseCount())
 			}
@@ -3263,8 +3224,8 @@ func testRetryWithRateLimiterBackoffAndMetrics(t *testing.T, key string, doFunc
 				t.Fatalf("Wrong test setup - did not find expected for: %s", key)
 			}
 			req := &Request{
-				verb: "GET",
-				body: bytes.NewReader([]byte{}),
+				verb:      "GET",
+				bodyBytes: []byte{},
 				c: &RESTClient{
 					base:        base,
 					content:     defaultContentConfig(),
@@ -3399,8 +3360,8 @@ func testWithRetryInvokeOrder(t *testing.T, key string, doFunc func(ctx context.
 				t.Fatalf("Wrong test setup - did not find expected for: %s", key)
 			}
 			req := &Request{
-				verb: "GET",
-				body: bytes.NewReader([]byte{}),
+				verb:      "GET",
+				bodyBytes: []byte{},
 				c: &RESTClient{
 					base:    base,
 					content: defaultContentConfig(),
@@ -3574,8 +3535,8 @@ func testWithWrapPreviousError(t *testing.T, doFunc func(ctx context.Context, r
 				t.Fatalf("Failed to create new HTTP request - %v", err)
 			}
 			req := &Request{
-				verb: "GET",
-				body: bytes.NewReader([]byte{}),
+				verb:      "GET",
+				bodyBytes: []byte{},
 				c: &RESTClient{
 					base:    base,
 					content: defaultContentConfig(),
@@ -3810,104 +3771,3 @@ func TestTransportConcurrency(t *testing.T) {
 		})
 	}
 }
-
-// TODO: see if we can consolidate the other trackers into one.
-type requestBodyTracker struct {
-	io.ReadSeeker
-	f func(string)
-}
-
-func (t *requestBodyTracker) Read(p []byte) (int, error) {
-	t.f("Request.Body.Read")
-	return t.ReadSeeker.Read(p)
-}
-
-func (t *requestBodyTracker) Seek(offset int64, whence int) (int64, error) {
-	t.f("Request.Body.Seek")
-	return t.ReadSeeker.Seek(offset, whence)
-}
-
-type responseBodyTracker struct {
-	io.ReadCloser
-	f func(string)
-}
-
-func (t *responseBodyTracker) Read(p []byte) (int, error) {
-	t.f("Response.Body.Read")
-	return t.ReadCloser.Read(p)
-}
-
-func (t *responseBodyTracker) Close() error {
-	t.f("Response.Body.Close")
-	return t.ReadCloser.Close()
-}
-
-type recorder struct {
-	order []string
-}
-
-func (r *recorder) record(call string) {
-	r.order = append(r.order, call)
-}
-
-func TestRequestBodyResetOrder(t *testing.T) {
-	recorder := &recorder{}
-	respBodyTracker := &responseBodyTracker{
-		ReadCloser: nil, // the server will fill it
-		f:          recorder.record,
-	}
-
-	var attempts int
-	client := clientForFunc(func(req *http.Request) (*http.Response, error) {
-		defer func() {
-			attempts++
-		}()
-
-		// read the request body.
-		io.ReadAll(req.Body)
-
-		// first attempt, we send a retry-after
-		if attempts == 0 {
-			resp := retryAfterResponse()
-			respBodyTracker.ReadCloser = io.NopCloser(bytes.NewReader([]byte{}))
-			resp.Body = respBodyTracker
-			return resp, nil
-		}
-
-		return &http.Response{StatusCode: http.StatusOK}, nil
-	})
-
-	reqBodyTracker := &requestBodyTracker{
-		ReadSeeker: bytes.NewReader([]byte{}), // empty body ensures one Read operation at most.
-		f:          recorder.record,
-	}
-	req := &Request{
-		verb: "POST",
-		body: reqBodyTracker,
-		c: &RESTClient{
-			content: defaultContentConfig(),
-			Client:  client,
-		},
-		backoff:    &noSleepBackOff{},
-		maxRetries: 1,
-		retryFn:    defaultRequestRetryFn,
-	}
-
-	req.Do(context.Background())
-
-	expected := []string{
-		// 1st attempt: the server handler reads the request body
-		"Request.Body.Read",
-		// the server sends a retry-after, client reads the
-		// response body, and closes it
-		"Response.Body.Read",
-		"Response.Body.Close",
-		// client retry logic seeks to the beginning of the request body
-		"Request.Body.Seek",
-		// 2nd attempt: the server reads the request body
-		"Request.Body.Read",
-	}
-	if !reflect.DeepEqual(expected, recorder.order) {
-		t.Errorf("Expected invocation request and response body operations for retry do not match: %s", cmp.Diff(expected, recorder.order))
-	}
-}

rest/with_retry.go

@@ -153,6 +153,11 @@ func (r *withRetry) IsNextRetry(ctx context.Context, restReq *Request, httpReq *
 		return false
 	}
 
+	if restReq.body != nil {
+		// we have an opaque reader, we can't safely reset it
+		return false
+	}
+
 	r.attempts++
 	r.retryAfter = &RetryAfter{Attempt: r.attempts}
 	if r.attempts > r.maxRetries {
@@ -209,18 +214,6 @@ func (r *withRetry) Before(ctx context.Context, request *Request) error {
 		return nil
 	}
 
-	// At this point we've made atleast one attempt, post which the response
-	// body should have been fully read and closed in order for it to be safe
-	// to reset the request body before we reconnect, in order for us to reuse
-	// the same TCP connection.
-	if seeker, ok := request.body.(io.Seeker); ok && request.body != nil {
-		if _, err := seeker.Seek(0, io.SeekStart); err != nil {
-			err = fmt.Errorf("failed to reset the request body while retrying a request: %v", err)
-			r.trackPreviousError(err)
-			return err
-		}
-	}
-
 	// if we are here, we have made attempt(s) at least once before.
 	if request.backoff != nil {
 		delay := request.backoff.CalculateBackoff(url)

rest/with_retry_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package rest
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -212,7 +211,7 @@ func TestIsNextRetry(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			restReq := &Request{
-				body: bytes.NewReader([]byte{}),
+				bodyBytes: []byte{},
 				c: &RESTClient{
 					base: &url.URL{},
 				},

tools/cache/controller.go

@@ -353,17 +353,6 @@ func NewIndexerInformer(
 	return clientState, newInformer(lw, objType, resyncPeriod, h, clientState, nil)
 }
 
-// TransformFunc allows for transforming an object before it will be processed
-// and put into the controller cache and before the corresponding handlers will
-// be called on it.
-// TransformFunc (similarly to ResourceEventHandler functions) should be able
-// to correctly handle the tombstone of type cache.DeletedFinalStateUnknown
-//
-// The most common usage pattern is to clean-up some parts of the object to
-// reduce component memory usage if a given component doesn't care about them.
-// given controller doesn't care for them
-type TransformFunc func(interface{}) (interface{}, error)
-
 // NewTransformingInformer returns a Store and a controller for populating
 // the store while also providing event notifications. You should only used
 // the returned Store for Get/List operations; Add/Modify/Deletes will cause
@@ -411,19 +400,11 @@ func processDeltas(
 	// Object which receives event notifications from the given deltas
 	handler ResourceEventHandler,
 	clientState Store,
-	transformer TransformFunc,
 	deltas Deltas,
 ) error {
 	// from oldest to newest
 	for _, d := range deltas {
 		obj := d.Object
-		if transformer != nil {
-			var err error
-			obj, err = transformer(obj)
-			if err != nil {
-				return err
-			}
-		}
 
 		switch d.Type {
 		case Sync, Replaced, Added, Updated:
@@ -475,6 +456,7 @@ func newInformer(
 	fifo := NewDeltaFIFOWithOptions(DeltaFIFOOptions{
 		KnownObjects:          clientState,
 		EmitDeltaTypeReplaced: true,
+		Transformer:           transformer,
 	})
 
 	cfg := &Config{
@@ -486,7 +468,7 @@ func newInformer(
 
 		Process: func(obj interface{}) error {
 			if deltas, ok := obj.(Deltas); ok {
-				return processDeltas(h, clientState, transformer, deltas)
+				return processDeltas(h, clientState, deltas)
 			}
 			return errors.New("object given as Process argument is not Deltas")
 		},

tools/cache/controller_test.go

@@ -23,7 +23,7 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -32,7 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/watch"
 	fcache "k8s.io/client-go/tools/cache/testing"
 
-	"github.com/google/gofuzz"
+	fuzz "github.com/google/gofuzz"
 )
 
 func Example() {

tools/cache/delta_fifo.go

@@ -51,6 +51,10 @@ type DeltaFIFOOptions struct {
 	// When true, `Replaced` events will be sent for items passed to a Replace() call.
 	// When false, `Sync` events will be sent instead.
 	EmitDeltaTypeReplaced bool
+
+	// If set, will be called for objects before enqueueing them. Please
+	// see the comment on TransformFunc for details.
+	Transformer TransformFunc
 }
 
 // DeltaFIFO is like FIFO, but differs in two ways.  One is that the
@@ -129,8 +133,32 @@ type DeltaFIFO struct {
 	// emitDeltaTypeReplaced is whether to emit the Replaced or Sync
 	// DeltaType when Replace() is called (to preserve backwards compat).
 	emitDeltaTypeReplaced bool
+
+	// Called with every object if non-nil.
+	transformer TransformFunc
 }
 
+// TransformFunc allows for transforming an object before it will be processed.
+// TransformFunc (similarly to ResourceEventHandler functions) should be able
+// to correctly handle the tombstone of type cache.DeletedFinalStateUnknown.
+//
+// New in v1.27: In such cases, the contained object will already have gone
+// through the transform object separately (when it was added / updated prior
+// to the delete), so the TransformFunc can likely safely ignore such objects
+// (i.e., just return the input object).
+//
+// The most common usage pattern is to clean-up some parts of the object to
+// reduce component memory usage if a given component doesn't care about them.
+//
+// New in v1.27: unless the object is a DeletedFinalStateUnknown, TransformFunc
+// sees the object before any other actor, and it is now safe to mutate the
+// object in place instead of making a copy.
+//
+// Note that TransformFunc is called while inserting objects into the
+// notification queue and is therefore extremely performance sensitive; please
+// do not do anything that will take a long time.
+type TransformFunc func(interface{}) (interface{}, error)
+
 // DeltaType is the type of a change (addition, deletion, etc)
 type DeltaType string
 
@@ -227,6 +255,7 @@ func NewDeltaFIFOWithOptions(opts DeltaFIFOOptions) *DeltaFIFO {
 		knownObjects: opts.KnownObjects,
 
 		emitDeltaTypeReplaced: opts.EmitDeltaTypeReplaced,
+		transformer:           opts.Transformer,
 	}
 	f.cond.L = &f.lock
 	return f
@@ -411,6 +440,21 @@ func (f *DeltaFIFO) queueActionLocked(actionType DeltaType, obj interface{}) err
 	if err != nil {
 		return KeyError{obj, err}
 	}
+
+	// Every object comes through this code path once, so this is a good
+	// place to call the transform func.  If obj is a
+	// DeletedFinalStateUnknown tombstone, then the containted inner object
+	// will already have gone through the transformer, but we document that
+	// this can happen. In cases involving Replace(), such an object can
+	// come through multiple times.
+	if f.transformer != nil {
+		var err error
+		obj, err = f.transformer(obj)
+		if err != nil {
+			return err
+		}
+	}
+
 	oldDeltas := f.items[id]
 	newDeltas := append(oldDeltas, Delta{actionType, obj})
 	newDeltas = dedupDeltas(newDeltas)
@@ -566,12 +610,11 @@ func (f *DeltaFIFO) Pop(process PopProcessFunc) (interface{}, error) {
 // using the Sync or Replace DeltaType and then (2) it does some deletions.
 // In particular: for every pre-existing key K that is not the key of
 // an object in `list` there is the effect of
-// `Delete(DeletedFinalStateUnknown{K, O})` where O is current object
-// of K.  If `f.knownObjects == nil` then the pre-existing keys are
-// those in `f.items` and the current object of K is the `.Newest()`
-// of the Deltas associated with K.  Otherwise the pre-existing keys
-// are those listed by `f.knownObjects` and the current object of K is
-// what `f.knownObjects.GetByKey(K)` returns.
+// `Delete(DeletedFinalStateUnknown{K, O})` where O is the latest known
+// object of K. The pre-existing keys are those in the union set of the keys in
+// `f.items` and `f.knownObjects` (if not nil). The last known object for key K is
+// the one present in the last delta in `f.items`. If there is no delta for K
+// in `f.items`, it is the object in `f.knownObjects`
 func (f *DeltaFIFO) Replace(list []interface{}, _ string) error {
 	f.lock.Lock()
 	defer f.lock.Unlock()
@@ -595,56 +638,54 @@ func (f *DeltaFIFO) Replace(list []interface{}, _ string) error {
 		}
 	}
 
-	if f.knownObjects == nil {
-		// Do deletion detection against our own list.
-		queuedDeletions := 0
-		for k, oldItem := range f.items {
+	// Do deletion detection against objects in the queue
+	queuedDeletions := 0
+	for k, oldItem := range f.items {
+		if keys.Has(k) {
+			continue
+		}
+		// Delete pre-existing items not in the new list.
+		// This could happen if watch deletion event was missed while
+		// disconnected from apiserver.
+		var deletedObj interface{}
+		if n := oldItem.Newest(); n != nil {
+			deletedObj = n.Object
+
+			// if the previous object is a DeletedFinalStateUnknown, we have to extract the actual Object
+			if d, ok := deletedObj.(DeletedFinalStateUnknown); ok {
+				deletedObj = d.Obj
+			}
+		}
+		queuedDeletions++
+		if err := f.queueActionLocked(Deleted, DeletedFinalStateUnknown{k, deletedObj}); err != nil {
+			return err
+		}
+	}
+
+	if f.knownObjects != nil {
+		// Detect deletions for objects not present in the queue, but present in KnownObjects
+		knownKeys := f.knownObjects.ListKeys()
+		for _, k := range knownKeys {
 			if keys.Has(k) {
 				continue
 			}
-			// Delete pre-existing items not in the new list.
-			// This could happen if watch deletion event was missed while
-			// disconnected from apiserver.
-			var deletedObj interface{}
-			if n := oldItem.Newest(); n != nil {
-				deletedObj = n.Object
+			if len(f.items[k]) > 0 {
+				continue
+			}
+
+			deletedObj, exists, err := f.knownObjects.GetByKey(k)
+			if err != nil {
+				deletedObj = nil
+				klog.Errorf("Unexpected error %v during lookup of key %v, placing DeleteFinalStateUnknown marker without object", err, k)
+			} else if !exists {
+				deletedObj = nil
+				klog.Infof("Key %v does not exist in known objects store, placing DeleteFinalStateUnknown marker without object", k)
 			}
 			queuedDeletions++
 			if err := f.queueActionLocked(Deleted, DeletedFinalStateUnknown{k, deletedObj}); err != nil {
 				return err
 			}
 		}
-
-		if !f.populated {
-			f.populated = true
-			// While there shouldn't be any queued deletions in the initial
-			// population of the queue, it's better to be on the safe side.
-			f.initialPopulationCount = keys.Len() + queuedDeletions
-		}
-
-		return nil
-	}
-
-	// Detect deletions not already in the queue.
-	knownKeys := f.knownObjects.ListKeys()
-	queuedDeletions := 0
-	for _, k := range knownKeys {
-		if keys.Has(k) {
-			continue
-		}
-
-		deletedObj, exists, err := f.knownObjects.GetByKey(k)
-		if err != nil {
-			deletedObj = nil
-			klog.Errorf("Unexpected error %v during lookup of key %v, placing DeleteFinalStateUnknown marker without object", err, k)
-		} else if !exists {
-			deletedObj = nil
-			klog.Infof("Key %v does not exist in known objects store, placing DeleteFinalStateUnknown marker without object", k)
-		}
-		queuedDeletions++
-		if err := f.queueActionLocked(Deleted, DeletedFinalStateUnknown{k, deletedObj}); err != nil {
-			return err
-		}
 	}
 
 	if !f.populated {

tools/cache/delta_fifo_test.go

@@ -121,6 +121,130 @@ func TestDeltaFIFO_replaceWithDeleteDeltaIn(t *testing.T) {
 	}
 }
 
+func TestDeltaFIFOW_ReplaceMakesDeletionsForObjectsOnlyInQueue(t *testing.T) {
+	obj := mkFifoObj("foo", 2)
+	objV2 := mkFifoObj("foo", 3)
+	table := []struct {
+		name           string
+		operations     func(f *DeltaFIFO)
+		expectedDeltas Deltas
+	}{
+		{
+			name: "Added object should be deleted on Replace",
+			operations: func(f *DeltaFIFO) {
+				f.Add(obj)
+				f.Replace([]interface{}{}, "0")
+			},
+			expectedDeltas: Deltas{
+				{Added, obj},
+				{Deleted, DeletedFinalStateUnknown{Key: "foo", Obj: obj}},
+			},
+		},
+		{
+			name: "Replaced object should have only a single Delete",
+			operations: func(f *DeltaFIFO) {
+				f.emitDeltaTypeReplaced = true
+				f.Add(obj)
+				f.Replace([]interface{}{obj}, "0")
+				f.Replace([]interface{}{}, "0")
+			},
+			expectedDeltas: Deltas{
+				{Added, obj},
+				{Replaced, obj},
+				{Deleted, DeletedFinalStateUnknown{Key: "foo", Obj: obj}},
+			},
+		},
+		{
+			name: "Deleted object should have only a single Delete",
+			operations: func(f *DeltaFIFO) {
+				f.Add(obj)
+				f.Delete(obj)
+				f.Replace([]interface{}{}, "0")
+			},
+			expectedDeltas: Deltas{
+				{Added, obj},
+				{Deleted, obj},
+			},
+		},
+		{
+			name: "Synced objects should have a single delete",
+			operations: func(f *DeltaFIFO) {
+				f.Add(obj)
+				f.Replace([]interface{}{obj}, "0")
+				f.Replace([]interface{}{obj}, "0")
+				f.Replace([]interface{}{}, "0")
+			},
+			expectedDeltas: Deltas{
+				{Added, obj},
+				{Sync, obj},
+				{Sync, obj},
+				{Deleted, DeletedFinalStateUnknown{Key: "foo", Obj: obj}},
+			},
+		},
+		{
+			name: "Added objects should have a single delete on multiple Replaces",
+			operations: func(f *DeltaFIFO) {
+				f.Add(obj)
+				f.Replace([]interface{}{}, "0")
+				f.Replace([]interface{}{}, "1")
+			},
+			expectedDeltas: Deltas{
+				{Added, obj},
+				{Deleted, DeletedFinalStateUnknown{Key: "foo", Obj: obj}},
+			},
+		},
+		{
+			name: "Added and deleted and added object should be deleted",
+			operations: func(f *DeltaFIFO) {
+				f.Add(obj)
+				f.Delete(obj)
+				f.Add(objV2)
+				f.Replace([]interface{}{}, "0")
+			},
+			expectedDeltas: Deltas{
+				{Added, obj},
+				{Deleted, obj},
+				{Added, objV2},
+				{Deleted, DeletedFinalStateUnknown{Key: "foo", Obj: objV2}},
+			},
+		},
+	}
+	for _, tt := range table {
+		tt := tt
+
+		t.Run(tt.name, func(t *testing.T) {
+			// Test with a DeltaFIFO with a backing KnownObjects
+			fWithKnownObjects := NewDeltaFIFOWithOptions(DeltaFIFOOptions{
+				KeyFunction: testFifoObjectKeyFunc,
+				KnownObjects: literalListerGetter(func() []testFifoObject {
+					return []testFifoObject{}
+				}),
+			})
+			tt.operations(fWithKnownObjects)
+			actualDeltasWithKnownObjects := Pop(fWithKnownObjects)
+			if !reflect.DeepEqual(tt.expectedDeltas, actualDeltasWithKnownObjects) {
+				t.Errorf("expected %#v, got %#v", tt.expectedDeltas, actualDeltasWithKnownObjects)
+			}
+			if len(fWithKnownObjects.items) != 0 {
+				t.Errorf("expected no extra deltas (empty map), got %#v", fWithKnownObjects.items)
+			}
+
+			// Test with a DeltaFIFO without a backing KnownObjects
+			fWithoutKnownObjects := NewDeltaFIFOWithOptions(DeltaFIFOOptions{
+				KeyFunction: testFifoObjectKeyFunc,
+			})
+			tt.operations(fWithoutKnownObjects)
+			actualDeltasWithoutKnownObjects := Pop(fWithoutKnownObjects)
+			if !reflect.DeepEqual(tt.expectedDeltas, actualDeltasWithoutKnownObjects) {
+				t.Errorf("expected %#v, got %#v", tt.expectedDeltas, actualDeltasWithoutKnownObjects)
+			}
+			if len(fWithoutKnownObjects.items) != 0 {
+				t.Errorf("expected no extra deltas (empty map), got %#v", fWithoutKnownObjects.items)
+			}
+		})
+	}
+}
+
 func TestDeltaFIFO_requeueOnPop(t *testing.T) {
 	f := NewDeltaFIFOWithOptions(DeltaFIFOOptions{KeyFunction: testFifoObjectKeyFunc})
 
@@ -203,6 +327,88 @@ func TestDeltaFIFO_addUpdate(t *testing.T) {
 	}
 }
 
+type rvAndXfrm struct {
+	rv   int
+	xfrm int
+}
+
+func TestDeltaFIFO_transformer(t *testing.T) {
+	mk := func(name string, rv int) testFifoObject {
+		return mkFifoObj(name, &rvAndXfrm{rv, 0})
+	}
+	xfrm := TransformFunc(func(obj interface{}) (interface{}, error) {
+		switch v := obj.(type) {
+		case testFifoObject:
+			v.val.(*rvAndXfrm).xfrm++
+		case DeletedFinalStateUnknown:
+			if x := v.Obj.(testFifoObject).val.(*rvAndXfrm).xfrm; x != 1 {
+				return nil, fmt.Errorf("object has been transformed wrong number of times: %#v", obj)
+			}
+		default:
+			return nil, fmt.Errorf("unexpected object: %#v", obj)
+		}
+		return obj, nil
+	})
+
+	must := func(err error) {
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	f := NewDeltaFIFOWithOptions(DeltaFIFOOptions{
+		KeyFunction: testFifoObjectKeyFunc,
+		Transformer: xfrm,
+	})
+	must(f.Add(mk("foo", 10)))
+	must(f.Add(mk("bar", 11)))
+	must(f.Update(mk("foo", 12)))
+	must(f.Delete(mk("foo", 15)))
+	must(f.Replace([]interface{}{}, ""))
+	must(f.Add(mk("bar", 16)))
+	must(f.Replace([]interface{}{}, ""))
+
+	// Should be empty
+	if e, a := []string{"foo", "bar"}, f.ListKeys(); !reflect.DeepEqual(e, a) {
+		t.Errorf("Expected %+v, got %+v", e, a)
+	}
+
+	for i := 0; i < 2; i++ {
+		obj, err := f.Pop(func(o interface{}) error { return nil })
+		if err != nil {
+			t.Fatalf("got nothing on try %v?", i)
+		}
+		obj = obj.(Deltas).Newest().Object
+		switch v := obj.(type) {
+		case testFifoObject:
+			if v.name != "foo" {
+				t.Errorf("expected regular deletion of foo, got %q", v.name)
+			}
+			rx := v.val.(*rvAndXfrm)
+			if rx.rv != 15 {
+				t.Errorf("expected last message, got %#v", obj)
+			}
+			if rx.xfrm != 1 {
+				t.Errorf("obj %v transformed wrong number of times.", obj)
+			}
+		case DeletedFinalStateUnknown:
+			tf := v.Obj.(testFifoObject)
+			rx := tf.val.(*rvAndXfrm)
+			if tf.name != "bar" {
+				t.Errorf("expected tombstone deletion of bar, got %q", tf.name)
+			}
+			if rx.rv != 16 {
+				t.Errorf("expected last message, got %#v", obj)
+			}
+			if rx.xfrm != 1 {
+				t.Errorf("tombstoned obj %v transformed wrong number of times.", obj)
+			}
+		default:
+			t.Errorf("unknown item %#v", obj)
+		}
+	}
+}
+
 func TestDeltaFIFO_enqueueingNoLister(t *testing.T) {
 	f := NewDeltaFIFOWithOptions(DeltaFIFOOptions{KeyFunction: testFifoObjectKeyFunc})
 	f.Add(mkFifoObj("foo", 10))
@@ -371,7 +577,7 @@ func TestDeltaFIFO_ReplaceMakesDeletions(t *testing.T) {
 
 	expectedList = []Deltas{
 		{{Added, mkFifoObj("baz", 10)},
-			{Deleted, DeletedFinalStateUnknown{Key: "baz", Obj: mkFifoObj("baz", 7)}}},
+			{Deleted, DeletedFinalStateUnknown{Key: "baz", Obj: mkFifoObj("baz", 10)}}},
 		{{Sync, mkFifoObj("foo", 5)}},
 		// Since "bar" didn't have a delete event and wasn't in the Replace list
 		// it should get a tombstone key with the right Obj.
@@ -385,6 +591,67 @@ func TestDeltaFIFO_ReplaceMakesDeletions(t *testing.T) {
 		}
 	}
 
+	// Now try deleting and recreating the object in the queue, then delete it by a Replace call
+	f = NewDeltaFIFOWithOptions(DeltaFIFOOptions{
+		KeyFunction: testFifoObjectKeyFunc,
+		KnownObjects: literalListerGetter(func() []testFifoObject {
+			return []testFifoObject{mkFifoObj("foo", 5), mkFifoObj("bar", 6), mkFifoObj("baz", 7)}
+		}),
+	})
+	f.Delete(mkFifoObj("bar", 6))
+	f.Add(mkFifoObj("bar", 100))
+	f.Replace([]interface{}{mkFifoObj("foo", 5)}, "0")
+
+	expectedList = []Deltas{
+		{
+			{Deleted, mkFifoObj("bar", 6)},
+			{Added, mkFifoObj("bar", 100)},
+			// Since "bar" has a newer object in the queue than in the state,
+			// it should get a tombstone key with the latest object from the queue
+			{Deleted, DeletedFinalStateUnknown{Key: "bar", Obj: mkFifoObj("bar", 100)}},
+		},
+		{{Sync, mkFifoObj("foo", 5)}},
+		{{Deleted, DeletedFinalStateUnknown{Key: "baz", Obj: mkFifoObj("baz", 7)}}},
+	}
+
+	for _, expected := range expectedList {
+		cur := Pop(f).(Deltas)
+		if e, a := expected, cur; !reflect.DeepEqual(e, a) {
+			t.Errorf("Expected %#v, got %#v", e, a)
+		}
+	}
+
+	// Now try syncing it first to ensure the delete use the latest version
+	f = NewDeltaFIFOWithOptions(DeltaFIFOOptions{
+		KeyFunction: testFifoObjectKeyFunc,
+		KnownObjects: literalListerGetter(func() []testFifoObject {
+			return []testFifoObject{mkFifoObj("foo", 5), mkFifoObj("bar", 6), mkFifoObj("baz", 7)}
+		}),
+	})
+	f.Replace([]interface{}{mkFifoObj("bar", 100), mkFifoObj("foo", 5)}, "0")
+	f.Replace([]interface{}{mkFifoObj("foo", 5)}, "0")
+
+	expectedList = []Deltas{
+		{
+			{Sync, mkFifoObj("bar", 100)},
+			// Since "bar" didn't have a delete event and wasn't in the Replace list
+			// it should get a tombstone key with the right Obj.
+			{Deleted, DeletedFinalStateUnknown{Key: "bar", Obj: mkFifoObj("bar", 100)}},
+		},
+		{
+			{Sync, mkFifoObj("foo", 5)},
+			{Sync, mkFifoObj("foo", 5)},
+		},
+		{{Deleted, DeletedFinalStateUnknown{Key: "baz", Obj: mkFifoObj("baz", 7)}}},
+	}
+
+	for _, expected := range expectedList {
+		cur := Pop(f).(Deltas)
+		if e, a := expected, cur; !reflect.DeepEqual(e, a) {
+			t.Errorf("Expected %#v, got %#v", e, a)
+		}
+	}
+
 	// Now try starting without an explicit KeyListerGetter
 	f = NewDeltaFIFOWithOptions(DeltaFIFOOptions{KeyFunction: testFifoObjectKeyFunc})
 	f.Add(mkFifoObj("baz", 10))

tools/cache/shared_informer.go

@@ -198,10 +198,7 @@ type SharedInformer interface {
 	//
 	// Must be set before starting the informer.
 	//
-	// Note: Since the object given to the handler may be already shared with
-	//	other goroutines, it is advisable to copy the object being
-	//  transform before mutating it at all and returning the copy to prevent
-	//	data races.
+	// Please see the comment on TransformFunc for more details.
 	SetTransform(handler TransformFunc) error
 
 	// IsStopped reports whether the informer has already been stopped.
@@ -422,6 +419,7 @@ func (s *sharedIndexInformer) Run(stopCh <-chan struct{}) {
 	fifo := NewDeltaFIFOWithOptions(DeltaFIFOOptions{
 		KnownObjects:          s.indexer,
 		EmitDeltaTypeReplaced: true,
+		Transformer:           s.transform,
 	})
 
 	cfg := &Config{
@@ -585,7 +583,7 @@ func (s *sharedIndexInformer) HandleDeltas(obj interface{}) error {
 	defer s.blockDeltas.Unlock()
 
 	if deltas, ok := obj.(Deltas); ok {
-		return processDeltas(s, s.indexer, s.transform, deltas)
+		return processDeltas(s, s.indexer, deltas)
 	}
 	return errors.New("object given as Process argument is not Deltas")
 }

tools/cache/shared_informer_test.go

@@ -395,9 +395,8 @@ func TestSharedInformerTransformer(t *testing.T) {
 			name := pod.GetName()
 
 			if upper := strings.ToUpper(name); upper != name {
-				copied := pod.DeepCopyObject().(*v1.Pod)
-				copied.SetName(upper)
-				return copied, nil
+				pod.SetName(upper)
+				return pod, nil
 			}
 		}
 		return obj, nil

tools/events/event_broadcaster.go

@@ -181,22 +181,24 @@ func (e *eventBroadcasterImpl) recordToSink(event *eventsv1.Event, clock clock.C
 					return nil
 				}
 				isomorphicEvent.Series = &eventsv1.EventSeries{
-					Count:            1,
+					Count:            2,
 					LastObservedTime: metav1.MicroTime{Time: clock.Now()},
 				}
-				return isomorphicEvent
+				// Make a copy of the Event to make sure that recording it
+				// doesn't mess with the object stored in cache.
+				return isomorphicEvent.DeepCopy()
 			}
 			e.eventCache[eventKey] = eventCopy
-			return eventCopy
+			// Make a copy of the Event to make sure that recording it doesn't
+			// mess with the object stored in cache.
+			return eventCopy.DeepCopy()
 		}()
 		if evToRecord != nil {
-			recordedEvent := e.attemptRecording(evToRecord)
-			if recordedEvent != nil {
-				recordedEventKey := getKey(recordedEvent)
-				e.mu.Lock()
-				defer e.mu.Unlock()
-				e.eventCache[recordedEventKey] = recordedEvent
-			}
+			// TODO: Add a metric counting the number of recording attempts
+			e.attemptRecording(evToRecord)
+			// We don't want the new recorded Event to be reflected in the
+			// client's cache because server-side mutations could mess with the
+			// aggregation mechanism used by the client.
 		}
 	}()
 }
@@ -248,6 +250,14 @@ func recordEvent(sink EventSink, event *eventsv1.Event) (*eventsv1.Event, bool)
 		return nil, false
 	case *errors.StatusError:
 		if errors.IsAlreadyExists(err) {
+			// If we tried to create an Event from an EventSerie, it means that
+			// the original Patch request failed because the Event we were
+			// trying to patch didn't exist. If the creation failed because the
+			// Event now exists, it is safe to retry.  This occurs when a new
+			// Event is emitted twice in a very short period of time.
+			if isEventSeries {
+				return nil, true
+			}
 			klog.V(5).Infof("Server rejected event '%#v': '%v' (will not retry!)", event, err)
 		} else {
 			klog.Errorf("Server rejected event '%#v': '%v' (will not retry!)", event, err)

tools/events/event_broadcaster_test.go

@@ -0,0 +1,103 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package events
+
+import (
+	"context"
+	"reflect"
+	"testing"
+
+	eventsv1 "k8s.io/api/events/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/client-go/kubernetes/fake"
+)
+
+func TestRecordEventToSink(t *testing.T) {
+	nonIsomorphicEvent := eventsv1.Event{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: metav1.NamespaceDefault,
+		},
+		Series: nil,
+	}
+
+	isomorphicEvent := *nonIsomorphicEvent.DeepCopy()
+	isomorphicEvent.Series = &eventsv1.EventSeries{Count: 2}
+
+	testCases := []struct {
+		name                  string
+		eventsToRecord        []eventsv1.Event
+		expectedRecordedEvent eventsv1.Event
+	}{
+		{
+			name: "record one Event",
+			eventsToRecord: []eventsv1.Event{
+				nonIsomorphicEvent,
+			},
+			expectedRecordedEvent: nonIsomorphicEvent,
+		},
+		{
+			name: "record one Event followed by an isomorphic one",
+			eventsToRecord: []eventsv1.Event{
+				nonIsomorphicEvent,
+				isomorphicEvent,
+			},
+			expectedRecordedEvent: isomorphicEvent,
+		},
+		{
+			name: "record one isomorphic Event before the original",
+			eventsToRecord: []eventsv1.Event{
+				isomorphicEvent,
+				nonIsomorphicEvent,
+			},
+			expectedRecordedEvent: isomorphicEvent,
+		},
+		{
+			name: "record one isomorphic Event without one already existing",
+			eventsToRecord: []eventsv1.Event{
+				isomorphicEvent,
+			},
+			expectedRecordedEvent: isomorphicEvent,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			kubeClient := fake.NewSimpleClientset()
+			eventSink := &EventSinkImpl{Interface: kubeClient.EventsV1()}
+
+			for _, ev := range tc.eventsToRecord {
+				recordEvent(eventSink, &ev)
+			}
+
+			recordedEvents, err := kubeClient.EventsV1().Events(metav1.NamespaceDefault).List(context.TODO(), metav1.ListOptions{})
+			if err != nil {
+				t.Errorf("expected to be able to list Events from fake client")
+			}
+
+			if len(recordedEvents.Items) != 1 {
+				t.Errorf("expected one Event to be recorded, found: %d", len(recordedEvents.Items))
+			}
+
+			recordedEvent := recordedEvents.Items[0]
+			if !reflect.DeepEqual(recordedEvent, tc.expectedRecordedEvent) {
+				t.Errorf("expected to have recorded Event: %#+v, got: %#+v\n diff: %s", tc.expectedRecordedEvent, recordedEvent, diff.ObjectReflectDiff(tc.expectedRecordedEvent, recordedEvent))
+			}
+		})
+	}
+}

tools/events/eventseries_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package events
 
 import (
+	"context"
 	"strconv"
 	"testing"
 	"time"
@@ -29,6 +30,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8sruntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
+	fake "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/kubernetes/scheme"
 	restclient "k8s.io/client-go/rest"
 	ref "k8s.io/client-go/tools/reference"
@@ -106,7 +108,7 @@ func TestEventSeriesf(t *testing.T) {
 	nonIsomorphicEvent := expectedEvent.DeepCopy()
 	nonIsomorphicEvent.Action = "stopped"
 
-	expectedEvent.Series = &eventsv1.EventSeries{Count: 1}
+	expectedEvent.Series = &eventsv1.EventSeries{Count: 2}
 	table := []struct {
 		regarding    k8sruntime.Object
 		related      k8sruntime.Object
@@ -185,6 +187,44 @@ func TestEventSeriesf(t *testing.T) {
 	close(stopCh)
 }
 
+// TestEventSeriesWithEventSinkImplRace verifies that when Events are emitted to
+// an EventSink consecutively there is no data race.  This test is meant to be
+// run with the `-race` option.
+func TestEventSeriesWithEventSinkImplRace(t *testing.T) {
+	kubeClient := fake.NewSimpleClientset()
+
+	eventSink := &EventSinkImpl{Interface: kubeClient.EventsV1()}
+	eventBroadcaster := NewBroadcaster(eventSink)
+
+	stopCh := make(chan struct{})
+	eventBroadcaster.StartRecordingToSink(stopCh)
+
+	recorder := eventBroadcaster.NewRecorder(scheme.Scheme, "test")
+
+	recorder.Eventf(&v1.ObjectReference{}, nil, v1.EventTypeNormal, "reason", "action", "", "")
+	recorder.Eventf(&v1.ObjectReference{}, nil, v1.EventTypeNormal, "reason", "action", "", "")
+
+	err := wait.PollImmediate(100*time.Millisecond, 5*time.Second, func() (done bool, err error) {
+		events, err := kubeClient.EventsV1().Events(metav1.NamespaceDefault).List(context.TODO(), metav1.ListOptions{})
+		if err != nil {
+			return false, err
+		}
+
+		if len(events.Items) != 1 {
+			return false, nil
+		}
+
+		if events.Items[0].Series == nil {
+			return false, nil
+		}
+
+		return true, nil
+	})
+	if err != nil {
+		t.Fatal("expected that 2 identical Eventf calls would result in the creation of an Event with a Serie")
+	}
+}
+
 func validateEvent(messagePrefix string, expectedUpdate bool, actualEvent *eventsv1.Event, expectedEvent *eventsv1.Event, t *testing.T) {
 	recvEvent := *actualEvent
 

tools/record/event.go

@@ -344,6 +344,9 @@ func (recorder *recorderImpl) generateEvent(object runtime.Object, annotations m
 	event := recorder.makeEvent(ref, annotations, eventtype, reason, message)
 	event.Source = recorder.source
 
+	event.ReportingInstance = recorder.source.Host
+	event.ReportingController = recorder.source.Component
+
 	// NOTE: events should be a non-blocking operation, but we also need to not
 	// put this in a goroutine, otherwise we'll race to write to a closed channel
 	// when we go to shut down this broadcaster.  Just drop events if we get overloaded,

tools/record/event_test.go

@@ -178,11 +178,12 @@ func TestEventf(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[2]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[2]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: false,
@@ -205,11 +206,12 @@ func TestEventf(t *testing.T) {
 					UID:        "bar",
 					APIVersion: "v1",
 				},
-				Reason:  "Killed",
-				Message: "some other verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Killed",
+				Message:             "some other verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:""}): type: 'Normal' reason: 'Killed' some other verbose message: 1`,
 			expectUpdate: false,
@@ -233,11 +235,12 @@ func TestEventf(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[2]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   2,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               2,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[2]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: true,
@@ -261,11 +264,12 @@ func TestEventf(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[3]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[3]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: false,
@@ -289,11 +293,12 @@ func TestEventf(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[2]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   3,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               3,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[2]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: true,
@@ -317,11 +322,12 @@ func TestEventf(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[3]",
 				},
-				Reason:  "Stopped",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Stopped",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[3]"}): type: 'Normal' reason: 'Stopped' some verbose message: 1`,
 			expectUpdate: false,
@@ -345,11 +351,12 @@ func TestEventf(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[3]",
 				},
-				Reason:  "Stopped",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   2,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Stopped",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               2,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[3]"}): type: 'Normal' reason: 'Stopped' some verbose message: 1`,
 			expectUpdate: true,
@@ -697,11 +704,12 @@ func TestMultiSinkCache(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[2]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[2]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: false,
@@ -724,11 +732,12 @@ func TestMultiSinkCache(t *testing.T) {
 					UID:        "bar",
 					APIVersion: "v1",
 				},
-				Reason:  "Killed",
-				Message: "some other verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Killed",
+				Message:             "some other verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:""}): type: 'Normal' reason: 'Killed' some other verbose message: 1`,
 			expectUpdate: false,
@@ -752,11 +761,12 @@ func TestMultiSinkCache(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[2]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   2,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               2,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[2]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: true,
@@ -780,11 +790,12 @@ func TestMultiSinkCache(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[3]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[3]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: false,
@@ -808,11 +819,12 @@ func TestMultiSinkCache(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[2]",
 				},
-				Reason:  "Started",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   3,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Started",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               3,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"bar", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[2]"}): type: 'Normal' reason: 'Started' some verbose message: 1`,
 			expectUpdate: true,
@@ -836,11 +848,12 @@ func TestMultiSinkCache(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[3]",
 				},
-				Reason:  "Stopped",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   1,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Stopped",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               1,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[3]"}): type: 'Normal' reason: 'Stopped' some verbose message: 1`,
 			expectUpdate: false,
@@ -864,11 +877,12 @@ func TestMultiSinkCache(t *testing.T) {
 					APIVersion: "v1",
 					FieldPath:  "spec.containers[3]",
 				},
-				Reason:  "Stopped",
-				Message: "some verbose message: 1",
-				Source:  v1.EventSource{Component: "eventTest"},
-				Count:   2,
-				Type:    v1.EventTypeNormal,
+				Reason:              "Stopped",
+				Message:             "some verbose message: 1",
+				Source:              v1.EventSource{Component: "eventTest"},
+				ReportingController: "eventTest",
+				Count:               2,
+				Type:                v1.EventTypeNormal,
 			},
 			expectLog:    `Event(v1.ObjectReference{Kind:"Pod", Namespace:"baz", Name:"foo", UID:"differentUid", APIVersion:"v1", ResourceVersion:"", FieldPath:"spec.containers[3]"}): type: 'Normal' reason: 'Stopped' some verbose message: 1`,
 			expectUpdate: true,

tools/record/events_cache_test.go

@@ -115,6 +115,9 @@ func validateEvent(messagePrefix string, actualEvent *v1.Event, expectedEvent *v
 	// Temp clear time stamps for comparison because actual values don't matter for comparison
 	recvEvent.FirstTimestamp = expectedEvent.FirstTimestamp
 	recvEvent.LastTimestamp = expectedEvent.LastTimestamp
+
+	recvEvent.ReportingController = expectedEvent.ReportingController
+
 	// Check that name has the right prefix.
 	if n, en := recvEvent.Name, expectedEvent.Name; !strings.HasPrefix(n, en) {
 		t.Errorf("%v - Name '%v' does not contain prefix '%v'", messagePrefix, n, en)

transport/cache.go

@@ -109,7 +109,7 @@ func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) {
 
 	// If we use are reloading files, we need to handle certificate rotation properly
 	// TODO(jackkleeman): We can also add rotation here when config.HasCertCallback() is true
-	if config.TLS.ReloadTLSFiles {
+	if config.TLS.ReloadTLSFiles && tlsConfig != nil && tlsConfig.GetClientCertificate != nil {
 		dynamicCertDialer := certRotatingDialer(tlsConfig.GetClientCertificate, dial)
 		tlsConfig.GetClientCertificate = dynamicCertDialer.GetClientCertificate
 		dial = dynamicCertDialer.connDialer.DialContext

util/cert/cert.go

@@ -25,6 +25,7 @@ import (
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"fmt"
+	"math"
 	"math/big"
 	"net"
 	"os"
@@ -44,6 +45,7 @@ type Config struct {
 	Organization []string
 	AltNames     AltNames
 	Usages       []x509.ExtKeyUsage
+	NotBefore    time.Time
 }
 
 // AltNames contains the domain names and IP addresses that will be added
@@ -57,14 +59,24 @@ type AltNames struct {
 // NewSelfSignedCACert creates a CA certificate
 func NewSelfSignedCACert(cfg Config, key crypto.Signer) (*x509.Certificate, error) {
 	now := time.Now()
+	// returns a uniform random value in [0, max-1), then add 1 to serial to make it a uniform random value in [1, max).
+	serial, err := cryptorand.Int(cryptorand.Reader, new(big.Int).SetInt64(math.MaxInt64-1))
+	if err != nil {
+		return nil, err
+	}
+	serial = new(big.Int).Add(serial, big.NewInt(1))
+	notBefore := now.UTC()
+	if !cfg.NotBefore.IsZero() {
+		notBefore = cfg.NotBefore.UTC()
+	}
 	tmpl := x509.Certificate{
-		SerialNumber: new(big.Int).SetInt64(0),
+		SerialNumber: serial,
 		Subject: pkix.Name{
 			CommonName:   cfg.CommonName,
 			Organization: cfg.Organization,
 		},
 		DNSNames:              []string{cfg.CommonName},
-		NotBefore:             now.UTC(),
+		NotBefore:             notBefore,
 		NotAfter:              now.Add(duration365d * 10).UTC(),
 		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
 		BasicConstraintsValid: true,
@@ -116,9 +128,14 @@ func GenerateSelfSignedCertKeyWithFixtures(host string, alternateIPs []net.IP, a
 	if err != nil {
 		return nil, nil, err
 	}
-
+	// returns a uniform random value in [0, max-1), then add 1 to serial to make it a uniform random value in [1, max).
+	serial, err := cryptorand.Int(cryptorand.Reader, new(big.Int).SetInt64(math.MaxInt64-1))
+	if err != nil {
+		return nil, nil, err
+	}
+	serial = new(big.Int).Add(serial, big.NewInt(1))
 	caTemplate := x509.Certificate{
-		SerialNumber: big.NewInt(1),
+		SerialNumber: serial,
 		Subject: pkix.Name{
 			CommonName: fmt.Sprintf("%s-ca@%d", host, time.Now().Unix()),
 		},
@@ -144,9 +161,14 @@ func GenerateSelfSignedCertKeyWithFixtures(host string, alternateIPs []net.IP, a
 	if err != nil {
 		return nil, nil, err
 	}
-
+	// returns a uniform random value in [0, max-1), then add 1 to serial to make it a uniform random value in [1, max).
+	serial, err = cryptorand.Int(cryptorand.Reader, new(big.Int).SetInt64(math.MaxInt64-1))
+	if err != nil {
+		return nil, nil, err
+	}
+	serial = new(big.Int).Add(serial, big.NewInt(1))
 	template := x509.Certificate{
-		SerialNumber: big.NewInt(2),
+		SerialNumber: serial,
 		Subject: pkix.Name{
 			CommonName: fmt.Sprintf("%s@%d", host, time.Now().Unix()),
 		},