Set the maximum size increase the copy operations in a json patch can cause

This commit is contained in:
Chao Xu 2019-02-04 11:15:16 -08:00
parent b8911b8d79
commit f001f9e1db
5 changed files with 72 additions and 16 deletions

View File

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

View File

@ -108,6 +108,7 @@ go_library(
"//staging/src/k8s.io/component-base/logs:go_default_library", "//staging/src/k8s.io/component-base/logs:go_default_library",
"//vendor/github.com/coreos/go-systemd/daemon: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/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/go-openapi/spec:go_default_library",
"//vendor/github.com/pborman/uuid:go_default_library", "//vendor/github.com/pborman/uuid:go_default_library",
"//vendor/golang.org/x/net/http2:go_default_library", "//vendor/golang.org/x/net/http2:go_default_library",

View File

@ -26,8 +26,10 @@ import (
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"sync/atomic"
"time" "time"
jsonpatch "github.com/evanphx/json-patch"
"github.com/go-openapi/spec" "github.com/go-openapi/spec"
"github.com/pborman/uuid" "github.com/pborman/uuid"
"k8s.io/klog" "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 // 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. // twice this value. Note that it is up to the request handlers to ignore or honor this timeout. In seconds.
MinRequestTimeout int 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 // MaxRequestsInFlight is the maximum number of parallel non-long-running requests. Every further
// request has to wait. Applies only to non-mutating requests. // request has to wait. Applies only to non-mutating requests.
MaxRequestsInFlight int MaxRequestsInFlight int
@ -243,20 +249,26 @@ type AuthorizationInfo struct {
// NewConfig returns a Config struct with the default values // NewConfig returns a Config struct with the default values
func NewConfig(codecs serializer.CodecFactory) *Config { func NewConfig(codecs serializer.CodecFactory) *Config {
return &Config{ return &Config{
Serializer: codecs, Serializer: codecs,
BuildHandlerChainFunc: DefaultBuildHandlerChain, BuildHandlerChainFunc: DefaultBuildHandlerChain,
HandlerChainWaitGroup: new(utilwaitgroup.SafeWaitGroup), HandlerChainWaitGroup: new(utilwaitgroup.SafeWaitGroup),
LegacyAPIGroupPrefixes: sets.NewString(DefaultLegacyAPIPrefix), LegacyAPIGroupPrefixes: sets.NewString(DefaultLegacyAPIPrefix),
DisabledPostStartHooks: sets.NewString(), DisabledPostStartHooks: sets.NewString(),
HealthzChecks: []healthz.HealthzChecker{healthz.PingHealthz, healthz.LogHealthz}, HealthzChecks: []healthz.HealthzChecker{healthz.PingHealthz, healthz.LogHealthz},
EnableIndex: true, EnableIndex: true,
EnableDiscovery: true, EnableDiscovery: true,
EnableProfiling: true, EnableProfiling: true,
EnableMetrics: true, EnableMetrics: true,
MaxRequestsInFlight: 400, MaxRequestsInFlight: 400,
MaxMutatingRequestsInFlight: 200, MaxMutatingRequestsInFlight: 200,
RequestTimeout: time.Duration(60) * time.Second, RequestTimeout: time.Duration(60) * time.Second,
MinRequestTimeout: 1800, 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), EnableAPIResponseCompression: utilfeature.DefaultFeatureGate.Enabled(features.APIResponseCompression),
// Default to treating watch as a long-running operation // 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, 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() { for k, v := range delegationTarget.PostStartHooks() {
s.postStartHooks[k] = v s.postStartHooks[k] = v
} }

View File

@ -42,7 +42,10 @@ type ServerRunOptions struct {
MaxMutatingRequestsInFlight int MaxMutatingRequestsInFlight int
RequestTimeout time.Duration RequestTimeout time.Duration
MinRequestTimeout int 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 { func NewServerRunOptions() *ServerRunOptions {
@ -52,6 +55,7 @@ func NewServerRunOptions() *ServerRunOptions {
MaxMutatingRequestsInFlight: defaults.MaxMutatingRequestsInFlight, MaxMutatingRequestsInFlight: defaults.MaxMutatingRequestsInFlight,
RequestTimeout: defaults.RequestTimeout, RequestTimeout: defaults.RequestTimeout,
MinRequestTimeout: defaults.MinRequestTimeout, MinRequestTimeout: defaults.MinRequestTimeout,
JSONPatchMaxCopyBytes: defaults.JSONPatchMaxCopyBytes,
} }
} }
@ -63,6 +67,7 @@ func (s *ServerRunOptions) ApplyTo(c *server.Config) error {
c.MaxMutatingRequestsInFlight = s.MaxMutatingRequestsInFlight c.MaxMutatingRequestsInFlight = s.MaxMutatingRequestsInFlight
c.RequestTimeout = s.RequestTimeout c.RequestTimeout = s.RequestTimeout
c.MinRequestTimeout = s.MinRequestTimeout c.MinRequestTimeout = s.MinRequestTimeout
c.JSONPatchMaxCopyBytes = s.JSONPatchMaxCopyBytes
c.PublicAddress = s.AdvertiseAddress c.PublicAddress = s.AdvertiseAddress
return nil return nil
@ -107,10 +112,14 @@ func (s *ServerRunOptions) Validate() []error {
errors = append(errors, fmt.Errorf("--min-request-timeout can not be negative value")) 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 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) { func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) {
// Note: the weird ""+ in below lines seems to be the only way to get gofmt to // Note: the weird ""+ in below lines seems to be the only way to get gofmt to
// arrange these text blocks sensibly. Grrr. // arrange these text blocks sensibly. Grrr.

View File

@ -40,6 +40,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
MaxMutatingRequestsInFlight: 200, MaxMutatingRequestsInFlight: 200,
RequestTimeout: time.Duration(2) * time.Minute, RequestTimeout: time.Duration(2) * time.Minute,
MinRequestTimeout: 1800, MinRequestTimeout: 1800,
JSONPatchMaxCopyBytes: 10 * 1024 * 1024,
TargetRAMMB: -65536, TargetRAMMB: -65536,
}, },
expectErr: "--target-ram-mb can not be negative value", expectErr: "--target-ram-mb can not be negative value",
@ -53,6 +54,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
MaxMutatingRequestsInFlight: 200, MaxMutatingRequestsInFlight: 200,
RequestTimeout: time.Duration(2) * time.Minute, RequestTimeout: time.Duration(2) * time.Minute,
MinRequestTimeout: 1800, MinRequestTimeout: 1800,
JSONPatchMaxCopyBytes: 10 * 1024 * 1024,
TargetRAMMB: 65536, TargetRAMMB: 65536,
}, },
expectErr: "--max-requests-inflight can not be negative value", expectErr: "--max-requests-inflight can not be negative value",
@ -66,6 +68,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
MaxMutatingRequestsInFlight: -200, MaxMutatingRequestsInFlight: -200,
RequestTimeout: time.Duration(2) * time.Minute, RequestTimeout: time.Duration(2) * time.Minute,
MinRequestTimeout: 1800, MinRequestTimeout: 1800,
JSONPatchMaxCopyBytes: 10 * 1024 * 1024,
TargetRAMMB: 65536, TargetRAMMB: 65536,
}, },
expectErr: "--max-mutating-requests-inflight can not be negative value", expectErr: "--max-mutating-requests-inflight can not be negative value",
@ -79,6 +82,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
MaxMutatingRequestsInFlight: 200, MaxMutatingRequestsInFlight: 200,
RequestTimeout: -time.Duration(2) * time.Minute, RequestTimeout: -time.Duration(2) * time.Minute,
MinRequestTimeout: 1800, MinRequestTimeout: 1800,
JSONPatchMaxCopyBytes: 10 * 1024 * 1024,
TargetRAMMB: 65536, TargetRAMMB: 65536,
}, },
expectErr: "--request-timeout can not be negative value", expectErr: "--request-timeout can not be negative value",
@ -92,10 +96,25 @@ func TestServerRunOptionsValidate(t *testing.T) {
MaxMutatingRequestsInFlight: 200, MaxMutatingRequestsInFlight: 200,
RequestTimeout: time.Duration(2) * time.Minute, RequestTimeout: time.Duration(2) * time.Minute,
MinRequestTimeout: -1800, MinRequestTimeout: -1800,
JSONPatchMaxCopyBytes: 10 * 1024 * 1024,
TargetRAMMB: 65536, TargetRAMMB: 65536,
}, },
expectErr: "--min-request-timeout can not be negative value", 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", name: "Test when ServerRunOptions is valid",
testOptions: &ServerRunOptions{ testOptions: &ServerRunOptions{
@ -105,6 +124,7 @@ func TestServerRunOptionsValidate(t *testing.T) {
MaxMutatingRequestsInFlight: 200, MaxMutatingRequestsInFlight: 200,
RequestTimeout: time.Duration(2) * time.Minute, RequestTimeout: time.Duration(2) * time.Minute,
MinRequestTimeout: 1800, MinRequestTimeout: 1800,
JSONPatchMaxCopyBytes: 10 * 1024 * 1024,
TargetRAMMB: 65536, TargetRAMMB: 65536,
}, },
}, },