Merge pull request #73713 from caesarxuchao/bump-json-patch-again

Importing the latest json patch and set the accumulated copy size limit

Godeps/Godeps.json

@@ -1576,8 +1576,8 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Comment": "v4.1.0-11-gd4020504c68b6b",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Comment": "v4.1.0-19-g5858425f75500d",
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/exponent-io/jsonpath",

cmd/kube-apiserver/app/options/options_test.go

@@ -129,6 +129,7 @@ func TestAddFlags(t *testing.T) {
 			MaxMutatingRequestsInFlight: 200,
 			RequestTimeout:              time.Duration(2) * time.Minute,
 			MinRequestTimeout:           1800,
+			JSONPatchMaxCopyBytes:       int64(10 * 1024 * 1024),
 		},
 		Admission: &kubeoptions.AdmissionOptions{
 			GenericAdmission: &apiserveroptions.AdmissionOptions{

staging/src/k8s.io/apiextensions-apiserver/Godeps/Godeps.json

@@ -372,7 +372,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/ghodss/yaml",

staging/src/k8s.io/apimachinery/Godeps/Godeps.json

@@ -24,7 +24,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

staging/src/k8s.io/apiserver/Godeps/Godeps.json

@@ -372,7 +372,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/ghodss/yaml",

staging/src/k8s.io/apiserver/pkg/server/BUILD

@@ -108,6 +108,7 @@ go_library(
         "//staging/src/k8s.io/component-base/logs:go_default_library",
         "//vendor/github.com/coreos/go-systemd/daemon:go_default_library",
         "//vendor/github.com/emicklei/go-restful:go_default_library",
+        "//vendor/github.com/evanphx/json-patch:go_default_library",
         "//vendor/github.com/go-openapi/spec:go_default_library",
         "//vendor/github.com/pborman/uuid:go_default_library",
         "//vendor/golang.org/x/net/http2:go_default_library",

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -26,8 +26,10 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"time"
 
+	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/go-openapi/spec"
 	"github.com/pborman/uuid"
 	"k8s.io/klog"
@@ -153,6 +155,10 @@ type Config struct {
 	// If specified, long running requests such as watch will be allocated a random timeout between this value, and
 	// twice this value.  Note that it is up to the request handlers to ignore or honor this timeout. In seconds.
 	MinRequestTimeout int
+	// The limit on the total size increase all "copy" operations in a json
+	// patch may cause.
+	// This affects all places that applies json patch in the binary.
+	JSONPatchMaxCopyBytes int64
 	// MaxRequestsInFlight is the maximum number of parallel non-long-running requests. Every further
 	// request has to wait. Applies only to non-mutating requests.
 	MaxRequestsInFlight int
@@ -243,20 +249,26 @@ type AuthorizationInfo struct {
 // NewConfig returns a Config struct with the default values
 func NewConfig(codecs serializer.CodecFactory) *Config {
 	return &Config{
-		Serializer:                   codecs,
-		BuildHandlerChainFunc:        DefaultBuildHandlerChain,
-		HandlerChainWaitGroup:        new(utilwaitgroup.SafeWaitGroup),
-		LegacyAPIGroupPrefixes:       sets.NewString(DefaultLegacyAPIPrefix),
-		DisabledPostStartHooks:       sets.NewString(),
-		HealthzChecks:                []healthz.HealthzChecker{healthz.PingHealthz, healthz.LogHealthz},
-		EnableIndex:                  true,
-		EnableDiscovery:              true,
-		EnableProfiling:              true,
-		EnableMetrics:                true,
-		MaxRequestsInFlight:          400,
-		MaxMutatingRequestsInFlight:  200,
-		RequestTimeout:               time.Duration(60) * time.Second,
-		MinRequestTimeout:            1800,
+		Serializer:                  codecs,
+		BuildHandlerChainFunc:       DefaultBuildHandlerChain,
+		HandlerChainWaitGroup:       new(utilwaitgroup.SafeWaitGroup),
+		LegacyAPIGroupPrefixes:      sets.NewString(DefaultLegacyAPIPrefix),
+		DisabledPostStartHooks:      sets.NewString(),
+		HealthzChecks:               []healthz.HealthzChecker{healthz.PingHealthz, healthz.LogHealthz},
+		EnableIndex:                 true,
+		EnableDiscovery:             true,
+		EnableProfiling:             true,
+		EnableMetrics:               true,
+		MaxRequestsInFlight:         400,
+		MaxMutatingRequestsInFlight: 200,
+		RequestTimeout:              time.Duration(60) * time.Second,
+		MinRequestTimeout:           1800,
+		// 10MB is the recommended maximum client request size in bytes
+		// the etcd server should accept. Thus, we set it as the limit
+		// on the size increase the "copy" operations in a json patch
+		// can cause.  See
+		// https://github.com/etcd-io/etcd/blob/release-3.3/etcdserver/server.go#L90.
+		JSONPatchMaxCopyBytes:        int64(10 * 1024 * 1024),
 		EnableAPIResponseCompression: utilfeature.DefaultFeatureGate.Enabled(features.APIResponseCompression),
 
 		// Default to treating watch as a long-running operation
@@ -451,6 +463,19 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G
 		enableAPIResponseCompression: c.EnableAPIResponseCompression,
 	}
 
+	for {
+		if c.JSONPatchMaxCopyBytes <= 0 {
+			break
+		}
+		existing := atomic.LoadInt64(&jsonpatch.AccumulatedCopySizeLimit)
+		if existing > 0 && existing < c.JSONPatchMaxCopyBytes {
+			break
+		}
+		if atomic.CompareAndSwapInt64(&jsonpatch.AccumulatedCopySizeLimit, existing, c.JSONPatchMaxCopyBytes) {
+			break
+		}
+	}
+
 	for k, v := range delegationTarget.PostStartHooks() {
 		s.postStartHooks[k] = v
 	}

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go

@@ -42,7 +42,10 @@ type ServerRunOptions struct {
 	MaxMutatingRequestsInFlight int
 	RequestTimeout              time.Duration
 	MinRequestTimeout           int
-	TargetRAMMB                 int
+	// We intentionally did not add a flag for this option. Users of the
+	// apiserver library can wire it to a flag.
+	JSONPatchMaxCopyBytes int64
+	TargetRAMMB           int
 }
 
 func NewServerRunOptions() *ServerRunOptions {
@@ -52,6 +55,7 @@ func NewServerRunOptions() *ServerRunOptions {
 		MaxMutatingRequestsInFlight: defaults.MaxMutatingRequestsInFlight,
 		RequestTimeout:              defaults.RequestTimeout,
 		MinRequestTimeout:           defaults.MinRequestTimeout,
+		JSONPatchMaxCopyBytes:       defaults.JSONPatchMaxCopyBytes,
 	}
 }
 
@@ -63,6 +67,7 @@ func (s *ServerRunOptions) ApplyTo(c *server.Config) error {
 	c.MaxMutatingRequestsInFlight = s.MaxMutatingRequestsInFlight
 	c.RequestTimeout = s.RequestTimeout
 	c.MinRequestTimeout = s.MinRequestTimeout
+	c.JSONPatchMaxCopyBytes = s.JSONPatchMaxCopyBytes
 	c.PublicAddress = s.AdvertiseAddress
 
 	return nil
@@ -107,10 +112,14 @@ func (s *ServerRunOptions) Validate() []error {
 		errors = append(errors, fmt.Errorf("--min-request-timeout can not be negative value"))
 	}
 
+	if s.JSONPatchMaxCopyBytes < 0 {
+		errors = append(errors, fmt.Errorf("--json-patch-max-copy-bytes can not be negative value"))
+	}
+
 	return errors
 }
 
-// AddFlags adds flags for a specific APIServer to the specified FlagSet
+// AddUniversalFlags adds flags for a specific APIServer to the specified FlagSet
 func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) {
 	// Note: the weird ""+ in below lines seems to be the only way to get gofmt to
 	// arrange these text blocks sensibly. Grrr.

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go

@@ -40,6 +40,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MaxMutatingRequestsInFlight: 200,
 				RequestTimeout:              time.Duration(2) * time.Minute,
 				MinRequestTimeout:           1800,
+				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				TargetRAMMB:                 -65536,
 			},
 			expectErr: "--target-ram-mb can not be negative value",
@@ -53,6 +54,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MaxMutatingRequestsInFlight: 200,
 				RequestTimeout:              time.Duration(2) * time.Minute,
 				MinRequestTimeout:           1800,
+				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				TargetRAMMB:                 65536,
 			},
 			expectErr: "--max-requests-inflight can not be negative value",
@@ -66,6 +68,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MaxMutatingRequestsInFlight: -200,
 				RequestTimeout:              time.Duration(2) * time.Minute,
 				MinRequestTimeout:           1800,
+				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				TargetRAMMB:                 65536,
 			},
 			expectErr: "--max-mutating-requests-inflight can not be negative value",
@@ -79,6 +82,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MaxMutatingRequestsInFlight: 200,
 				RequestTimeout:              -time.Duration(2) * time.Minute,
 				MinRequestTimeout:           1800,
+				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				TargetRAMMB:                 65536,
 			},
 			expectErr: "--request-timeout can not be negative value",
@@ -92,10 +96,25 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MaxMutatingRequestsInFlight: 200,
 				RequestTimeout:              time.Duration(2) * time.Minute,
 				MinRequestTimeout:           -1800,
+				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				TargetRAMMB:                 65536,
 			},
 			expectErr: "--min-request-timeout can not be negative value",
 		},
+		{
+			name: "Test when JSONPatchMaxCopyBytes is negative value",
+			testOptions: &ServerRunOptions{
+				AdvertiseAddress:            net.ParseIP("192.168.10.10"),
+				CorsAllowedOriginList:       []string{"10.10.10.100", "10.10.10.200"},
+				MaxRequestsInFlight:         400,
+				MaxMutatingRequestsInFlight: 200,
+				RequestTimeout:              time.Duration(2) * time.Minute,
+				MinRequestTimeout:           1800,
+				JSONPatchMaxCopyBytes:       -10 * 1024 * 1024,
+				TargetRAMMB:                 65536,
+			},
+			expectErr: "--json-patch-max-copy-bytes can not be negative value",
+		},
 		{
 			name: "Test when ServerRunOptions is valid",
 			testOptions: &ServerRunOptions{
@@ -105,6 +124,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
 				MaxMutatingRequestsInFlight: 200,
 				RequestTimeout:              time.Duration(2) * time.Minute,
 				MinRequestTimeout:           1800,
+				JSONPatchMaxCopyBytes:       10 * 1024 * 1024,
 				TargetRAMMB:                 65536,
 			},
 		},

staging/src/k8s.io/cli-runtime/Godeps/Godeps.json

@@ -12,7 +12,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

staging/src/k8s.io/client-go/Godeps/Godeps.json

@@ -56,7 +56,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

staging/src/k8s.io/csi-api/Godeps/Godeps.json

@@ -12,7 +12,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

staging/src/k8s.io/kube-aggregator/Godeps/Godeps.json

@@ -96,7 +96,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/go-openapi/jsonpointer",

staging/src/k8s.io/metrics/Godeps/Godeps.json

@@ -12,7 +12,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

staging/src/k8s.io/node-api/Godeps/Godeps.json

@@ -12,7 +12,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

staging/src/k8s.io/sample-apiserver/Godeps/Godeps.json

@@ -88,7 +88,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/go-openapi/jsonpointer",

staging/src/k8s.io/sample-cli-plugin/Godeps/Godeps.json

@@ -12,7 +12,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

staging/src/k8s.io/sample-controller/Godeps/Godeps.json

@@ -12,7 +12,7 @@
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",
-			"Rev": "d4020504c68b6bfa818032bedfb48e33e9638506"
+			"Rev": "5858425f75500d40c52783dce87d085a483ce135"
 		},
 		{
 			"ImportPath": "github.com/gogo/protobuf/proto",

vendor/github.com/evanphx/json-patch/BUILD

@@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")
 go_library(
     name = "go_default_library",
     srcs = [
+        "errors.go",
         "merge.go",
         "patch.go",
     ],

vendor/github.com/evanphx/json-patch/README.md

@@ -34,13 +34,9 @@ go get -u github.com/evanphx/json-patch
   functionality can be disabled by setting `jsonpatch.SupportNegativeIndices =
   false`.
 
-* There is a global configuration variable `jsonpatch.ArraySizeLimit`, which
-  limits the length of any array the patched object can have. It defaults to 0,
-  which means there is no limit.
-
-* There is a global configuration variable `jsonpatch.ArraySizeAdditionLimit`,
-  which limits the increase of array length caused by each operation. It
-  defaults to 0, which means there is no limit.
+* There is a global configuration variable `jsonpatch.AccumulatedCopySizeLimit`,
+  which limits the total size increase in bytes caused by "copy" operations in a
+  patch. It defaults to 0, which means there is no limit.
 
 ## Create and apply a merge patch
 Given both an original JSON document and a modified JSON document, you can create

vendor/github.com/evanphx/json-patch/errors.go

@@ -0,0 +1,38 @@
+package jsonpatch
+
+import "fmt"
+
+// AccumulatedCopySizeError is an error type returned when the accumulated size
+// increase caused by copy operations in a patch operation has exceeded the
+// limit.
+type AccumulatedCopySizeError struct {
+	limit       int64
+	accumulated int64
+}
+
+// NewAccumulatedCopySizeError returns an AccumulatedCopySizeError.
+func NewAccumulatedCopySizeError(l, a int64) *AccumulatedCopySizeError {
+	return &AccumulatedCopySizeError{limit: l, accumulated: a}
+}
+
+// Error implements the error interface.
+func (a *AccumulatedCopySizeError) Error() string {
+	return fmt.Sprintf("Unable to complete the copy, the accumulated size increase of copy is %d, exceeding the limit %d", a.accumulated, a.limit)
+}
+
+// ArraySizeError is an error type returned when the array size has exceeded
+// the limit.
+type ArraySizeError struct {
+	limit int
+	size  int
+}
+
+// NewArraySizeError returns an ArraySizeError.
+func NewArraySizeError(l, s int) *ArraySizeError {
+	return &ArraySizeError{limit: l, size: s}
+}
+
+// Error implements the error interface.
+func (a *ArraySizeError) Error() string {
+	return fmt.Sprintf("Unable to create array of size %d, limit is %d", a.size, a.limit)
+}

vendor/github.com/evanphx/json-patch/patch.go

@@ -14,9 +14,15 @@ const (
 	eAry
 )
 
-var SupportNegativeIndices bool = true
-var ArraySizeLimit int = 0
-var ArraySizeAdditionLimit int = 0
+var (
+	// SupportNegativeIndices decides whether to support non-standard practice of
+	// allowing negative indices to mean indices starting at the end of an array.
+	// Default to true.
+	SupportNegativeIndices bool = true
+	// AccumulatedCopySizeLimit limits the total size increase in bytes caused by
+	// "copy" operations in a patch.
+	AccumulatedCopySizeLimit int64 = 0
+)
 
 type lazyNode struct {
 	raw   *json.RawMessage
@@ -65,17 +71,18 @@ func (n *lazyNode) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
-func deepCopy(src *lazyNode) (*lazyNode, error) {
+func deepCopy(src *lazyNode) (*lazyNode, int, error) {
 	if src == nil {
-		return nil, nil
+		return nil, 0, nil
 	}
 	a, err := src.MarshalJSON()
 	if err != nil {
-		return nil, err
+		return nil, 0, err
 	}
-	ra := make(json.RawMessage, len(a))
+	sz := len(a)
+	ra := make(json.RawMessage, sz)
 	copy(ra, a)
-	return newLazyNode(&ra), nil
+	return newLazyNode(&ra), sz, nil
 }
 
 func (n *lazyNode) intoDoc() (*partialDoc, error) {
@@ -359,44 +366,14 @@ func (d *partialDoc) remove(key string) error {
 	return nil
 }
 
+// set should only be used to implement the "replace" operation, so "key" must
+// be an already existing index in "d".
 func (d *partialArray) set(key string, val *lazyNode) error {
-	if key == "-" {
-		*d = append(*d, val)
-		return nil
-	}
-
 	idx, err := strconv.Atoi(key)
 	if err != nil {
 		return err
 	}
-
-	sz := len(*d)
-
-	if diff := idx + 1 - sz; ArraySizeAdditionLimit > 0 && diff > ArraySizeAdditionLimit {
-		return fmt.Errorf("Unable to increase the array size by %d, the limit is %d", diff, ArraySizeAdditionLimit)
-	}
-
-	if idx+1 > sz {
-		sz = idx + 1
-	}
-
-	if ArraySizeLimit > 0 && sz > ArraySizeLimit {
-		return fmt.Errorf("Unable to create array of size %d, limit is %d", sz, ArraySizeLimit)
-	}
-
-	ary := make([]*lazyNode, sz)
-
-	cur := *d
-
-	copy(ary, cur)
-
-	if idx >= len(ary) {
-		return fmt.Errorf("Unable to access invalid index: %d", idx)
-	}
-
-	ary[idx] = val
-
-	*d = ary
+	(*d)[idx] = val
 	return nil
 }
 
@@ -412,9 +389,6 @@ func (d *partialArray) add(key string, val *lazyNode) error {
 	}
 
 	sz := len(*d) + 1
-	if ArraySizeLimit > 0 && sz > ArraySizeLimit {
-		return fmt.Errorf("Unable to create array of size %d, limit is %d", sz, ArraySizeLimit)
-	}
 
 	ary := make([]*lazyNode, sz)
 
@@ -556,7 +530,7 @@ func (p Patch) move(doc *container, op operation) error {
 		return fmt.Errorf("jsonpatch move operation does not apply: doc is missing destination path: %s", path)
 	}
 
-	return con.set(key, val)
+	return con.add(key, val)
 }
 
 func (p Patch) test(doc *container, op operation) error {
@@ -590,7 +564,7 @@ func (p Patch) test(doc *container, op operation) error {
 	return fmt.Errorf("Testing value %s failed", path)
 }
 
-func (p Patch) copy(doc *container, op operation) error {
+func (p Patch) copy(doc *container, op operation, accumulatedCopySize *int64) error {
 	from := op.from()
 
 	con, key := findObject(doc, from)
@@ -612,10 +586,14 @@ func (p Patch) copy(doc *container, op operation) error {
 		return fmt.Errorf("jsonpatch copy operation does not apply: doc is missing destination path: %s", path)
 	}
 
-	valCopy, err := deepCopy(val)
+	valCopy, sz, err := deepCopy(val)
 	if err != nil {
 		return err
 	}
+	(*accumulatedCopySize) += int64(sz)
+	if AccumulatedCopySizeLimit > 0 && *accumulatedCopySize > AccumulatedCopySizeLimit {
+		return NewAccumulatedCopySizeError(AccumulatedCopySizeLimit, *accumulatedCopySize)
+	}
 
 	return con.add(key, valCopy)
 }
@@ -670,6 +648,8 @@ func (p Patch) ApplyIndent(doc []byte, indent string) ([]byte, error) {
 
 	err = nil
 
+	var accumulatedCopySize int64
+
 	for _, op := range p {
 		switch op.kind() {
 		case "add":
@@ -683,7 +663,7 @@ func (p Patch) ApplyIndent(doc []byte, indent string) ([]byte, error) {
 		case "test":
 			err = p.test(&pd, op)
 		case "copy":
-			err = p.copy(&pd, op)
+			err = p.copy(&pd, op, &accumulatedCopySize)
 		default:
 			err = fmt.Errorf("Unexpected kind: %s", op.kind())
 		}