Skip to content

Commit

Permalink
Shallow copy Kubernetes objects in API Server before encoding them
Browse files Browse the repository at this point in the history
This works around a race condition present in the encoder for
Kubernetes objects, as detailed in
kubernetes/kubernetes#82497

Uses the fix from kubernetes/kubernetes#101123

Fixes googleforgames#2086
  • Loading branch information
highlyunavailable committed May 3, 2021
1 parent 8452b46 commit 136ba05
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 3 deletions.
14 changes: 13 additions & 1 deletion pkg/util/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"reflect"
"strings"

"agones.dev/agones/pkg/util/https"
Expand Down Expand Up @@ -167,8 +168,9 @@ func (as *APIServer) addSerializedHandler(pattern string, m k8sruntime.Object) {
return err
}

shallowCopy := shallowCopyObjectForTargetKind(m)
w.Header().Set(ContentTypeHeader, info.MediaType)
err = Codecs.EncoderForVersion(info.Serializer, unversionedVersion).Encode(m, w)
err = Codecs.EncoderForVersion(info.Serializer, unversionedVersion).Encode(shallowCopy, w)
if err != nil {
return errors.New("error marshalling")
}
Expand All @@ -180,6 +182,16 @@ func (as *APIServer) addSerializedHandler(pattern string, m k8sruntime.Object) {
}))
}

// shallowCopyObjectForTargetKind ensures obj is unique by performing a shallow copy
// of the struct Object points to (all Object must be a pointer to a struct in a scheme).
// Copied from https://github.com/kubernetes/kubernetes/pull/101123 until the referenced PR is merged
func shallowCopyObjectForTargetKind(obj k8sruntime.Object) k8sruntime.Object {
v := reflect.ValueOf(obj).Elem()
copied := reflect.New(v.Type())
copied.Elem().Set(v)
return copied.Interface().(k8sruntime.Object)
}

// AcceptedSerializer takes the request, and returns a serialiser (if it exists)
// for the given codec factory and
// for the Accepted media types. If not found, returns error
Expand Down
48 changes: 46 additions & 2 deletions pkg/util/apiserver/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,57 @@ 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, "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])
})
}

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) {
// Run 10 concurrent requests to exercise multithreading
for i := 0; i < 10; i++ {
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])
})
}
})
}

func TestSplitNameSpaceResource(t *testing.T) {
type expected struct {
namespace string
Expand Down

0 comments on commit 136ba05

Please sign in to comment.