Clone Kubernetes objects in API Server before encoding them

This works around a race condition present in the encoder for Kubernetes objects, as detailed in kubernetes/kubernetes#82497 Fixes googleforgames#2086
highlyunavailable · May 1, 2021 · 734f0c4 · 734f0c4
1 parent 8452b46
commit 734f0c4
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 3 deletions.
diff --git a/pkg/util/apiserver/apiserver.go b/pkg/util/apiserver/apiserver.go
@@ -167,8 +167,9 @@ func (as *APIServer) addSerializedHandler(pattern string, m k8sruntime.Object) {
 				return err
 			}
 
+			objCopy := m.DeepCopyObject()
 			w.Header().Set(ContentTypeHeader, info.MediaType)
-			err = Codecs.EncoderForVersion(info.Serializer, unversionedVersion).Encode(m, w)
+			err = Codecs.EncoderForVersion(info.Serializer, unversionedVersion).Encode(objCopy, w)
 			if err != nil {
 				return errors.New("error marshalling")
 			}

diff --git a/pkg/util/apiserver/apiserver_test.go b/pkg/util/apiserver/apiserver_test.go
@@ -158,13 +158,110 @@ func TestAPIServerAddAPIResourceDiscovery(t *testing.T) {
 		_, _, err = info.Serializer.Decode(b, &gvk, list)
 		assert.NoError(t, err)
 
-		// not testing for version/kind, seems not to come through. Strange.
-		// we're not building anything that needs this, so not our issue.
+		assert.Equal(t, gvk.Version, list.GroupVersionKind().Version)
+		assert.Equal(t, gvk.Kind, list.GroupVersionKind().Kind)
 		assert.Equal(t, gv.String(), list.GroupVersion)
 		assert.Equal(t, resource, list.APIResources[0])
 	})
 }
 
+func TestAPIServerAddAPIResourceDiscoveryParallel(t *testing.T) {
+	t.Parallel()
+	mux := http.NewServeMux()
+	ts := httptest.NewUnstartedServer(mux)
+	api := NewAPIServer(mux)
+
+	api.AddAPIResource(gv.String(), resource, func(_ http.ResponseWriter, _ *http.Request, _ string) error {
+		return nil
+	})
+
+	ts.Start()
+	defer ts.Close()
+
+	t.Run("Parallel Tests", func(t *testing.T) {
+		t.Run("No Accept Header", func(t *testing.T) {
+			t.Parallel()
+			client := ts.Client()
+			path := ts.URL + "/apis/allocation.agones.dev/v1"
+			resp, err := client.Get(path)
+			if resp != nil {
+				defer resp.Body.Close() // nolint: errcheck
+			}
+			if !assert.NoError(t, err) {
+				assert.FailNow(t, "should not error")
+			}
+			assert.Equal(t, k8sruntime.ContentTypeJSON, resp.Header.Get("Content-Type"))
+
+			// default is json
+			list := &metav1.APIResourceList{}
+			err = json.NewDecoder(resp.Body).Decode(list)
+			assert.NoError(t, err)
+
+			assert.Equal(t, "v1", list.TypeMeta.APIVersion)
+			assert.Equal(t, "APIResourceList", list.TypeMeta.Kind)
+			assert.Equal(t, gv.String(), list.GroupVersion)
+			assert.Equal(t, resource, list.APIResources[0])
+		})
+
+		t.Run("Accept */*", func(t *testing.T) {
+			t.Parallel()
+			client := ts.Client()
+			path := ts.URL + "/apis/allocation.agones.dev/v1"
+			request, err := http.NewRequest(http.MethodGet, path, nil)
+			assert.NoError(t, err)
+
+			request.Header.Set("Accept", "*/*")
+			resp, err := client.Do(request)
+			assert.NoError(t, err)
+			if resp != nil {
+				defer resp.Body.Close() // nolint: errcheck
+			}
+			assert.Equal(t, k8sruntime.ContentTypeJSON, resp.Header.Get("Content-Type"))
+
+			list := &metav1.APIResourceList{}
+			err = json.NewDecoder(resp.Body).Decode(list)
+			assert.NoError(t, err)
+
+			assert.Equal(t, "v1", list.TypeMeta.APIVersion)
+			assert.Equal(t, "APIResourceList", list.TypeMeta.Kind)
+			assert.Equal(t, gv.String(), list.GroupVersion)
+			assert.Equal(t, resource, list.APIResources[0])
+		})
+
+		t.Run("Accept Protobuf, */*", func(t *testing.T) {
+			t.Parallel()
+			client := ts.Client()
+			path := ts.URL + "/apis/allocation.agones.dev/v1"
+			request, err := http.NewRequest(http.MethodGet, path, nil)
+			assert.NoError(t, err)
+
+			request.Header.Set("Accept", "application/vnd.kubernetes.protobuf, */*")
+			resp, err := client.Do(request)
+			assert.NoError(t, err)
+			if resp != nil {
+				defer resp.Body.Close() // nolint: errcheck
+			}
+			assert.Equal(t, "application/vnd.kubernetes.protobuf", resp.Header.Get("Content-Type"))
+
+			list := &metav1.APIResourceList{}
+			b, err := ioutil.ReadAll(resp.Body)
+			assert.NoError(t, err)
+
+			info, ok := k8sruntime.SerializerInfoForMediaType(Codecs.SupportedMediaTypes(), "application/vnd.kubernetes.protobuf")
+			assert.True(t, ok)
+
+			gvk := unversionedVersion.WithKind("APIResourceList")
+			_, _, err = info.Serializer.Decode(b, &gvk, list)
+			assert.NoError(t, err)
+
+			assert.Equal(t, gvk.Version, list.GroupVersionKind().Version)
+			assert.Equal(t, gvk.Kind, list.GroupVersionKind().Kind)
+			assert.Equal(t, gv.String(), list.GroupVersion)
+			assert.Equal(t, resource, list.APIResources[0])
+		})
+	})
+}
+
 func TestSplitNameSpaceResource(t *testing.T) {
 	type expected struct {
 		namespace string