Skip to content

Commit

Permalink
fix: arguments accept units
Browse files Browse the repository at this point in the history 
`max-body-size` and `read-buffer-size` now accept units as defined in the docs.

Fixes dapr#1489

Signed-off-by: Mike Nguyen <hey@mike.ee>
  • Loading branch information
@mikeee
mikeee committed Feb 20, 2025
1 parent a968b18 commit ae325cd
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 30 deletions.
27 changes: 19 additions & 8 deletions cmd/annotate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"bytes"
"fmt"
"io"
"k8s.io/apimachinery/pkg/api/resource"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -60,8 +61,8 @@ var (
annotateReadinessProbeThreshold int
annotateDaprImage string
annotateAppSSL bool
annotateMaxRequestBodySize int
annotateReadBufferSize int
annotateMaxRequestBodySize string
annotateReadBufferSize string
annotateHTTPStreamRequestBody bool
annotateGracefulShutdownSeconds int
annotateEnableAPILogging bool
Expand Down Expand Up @@ -318,12 +319,22 @@ func getOptionsFromFlags() kubernetes.AnnotateOptions {
if annotateAppSSL {
o = append(o, kubernetes.WithAppSSL())
}
if annotateMaxRequestBodySize != -1 {
o = append(o, kubernetes.WithMaxRequestBodySize(annotateMaxRequestBodySize))
if annotateMaxRequestBodySize != "-1" {
if q, err := resource.ParseQuantity(annotateMaxRequestBodySize); err != nil {
panic(err)
} else {
o = append(o, kubernetes.WithMaxRequestBodySize(int(q.Value())))
}

}
if annotateReadBufferSize != -1 {
o = append(o, kubernetes.WithReadBufferSize(annotateReadBufferSize))
if annotateReadBufferSize != "-1" {
if q, err := resource.ParseQuantity(annotateReadBufferSize); err != nil {
panic(err)
} else {
o = append(o, kubernetes.WithReadBufferSize(int(q.Value())))
}
}

if annotateHTTPStreamRequestBody {
o = append(o, kubernetes.WithHTTPStreamRequestBody())
}
Expand Down Expand Up @@ -385,8 +396,8 @@ func init() {
AnnotateCmd.Flags().StringVar(&annotateDaprImage, "dapr-image", "", "The image to use for the dapr sidecar container")
AnnotateCmd.Flags().BoolVar(&annotateAppSSL, "app-ssl", false, "Enable SSL for the app")
AnnotateCmd.Flags().MarkDeprecated("app-ssl", "This flag is deprecated and will be removed in a future release. Use \"app-protocol\" flag with https or grpcs instead")
AnnotateCmd.Flags().IntVar(&annotateMaxRequestBodySize, "max-body-size", -1, "The maximum request body size to use")
AnnotateCmd.Flags().IntVar(&annotateReadBufferSize, "read-buffer-size", -1, "The maximum size of HTTP header read buffer in kilobytes")
AnnotateCmd.Flags().StringVar(&annotateMaxRequestBodySize, "max-body-size", "-1", "The maximum request body size to use")
AnnotateCmd.Flags().StringVar(&annotateReadBufferSize, "read-buffer-size", "-1", "The maximum size of HTTP header read buffer in kilobytes")
AnnotateCmd.Flags().BoolVar(&annotateHTTPStreamRequestBody, "http-stream-request-body", false, "Enable streaming request body for HTTP")
AnnotateCmd.Flags().IntVar(&annotateGracefulShutdownSeconds, "graceful-shutdown-seconds", -1, "The number of seconds to wait for the app to shutdown")
AnnotateCmd.Flags().BoolVar(&annotateEnableAPILogging, "enable-api-logging", false, "Enable API logging for the Dapr sidecar")
Expand Down
8 changes: 4 additions & 4 deletions cmd/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ var (
resourcesPaths []string
appSSL bool
metricsPort int
maxRequestBodySize int
readBufferSize int
maxRequestBodySize string
readBufferSize string
unixDomainSocket string
enableAppHealth bool
appHealthPath string
Expand Down Expand Up @@ -473,8 +473,8 @@ func init() {
RunCmd.Flags().MarkDeprecated("app-ssl", "This flag is deprecated and will be removed in the future releases. Use \"app-protocol\" flag with https or grpcs values instead")
RunCmd.Flags().IntVarP(&metricsPort, "metrics-port", "M", -1, "The port of metrics on dapr")
RunCmd.Flags().BoolP("help", "h", false, "Print this help message")
RunCmd.Flags().IntVarP(&maxRequestBodySize, "max-body-size", "", -1, "Max size of request body in MB")
RunCmd.Flags().IntVarP(&readBufferSize, "read-buffer-size", "", -1, "HTTP header read buffer in KB")
RunCmd.Flags().StringVarP(&maxRequestBodySize, "max-body-size", "", "-1", "Max size of request body in MB")
RunCmd.Flags().StringVarP(&readBufferSize, "read-buffer-size", "", "-1", "HTTP header read buffer in KB")
RunCmd.Flags().StringVarP(&unixDomainSocket, "unix-domain-socket", "u", "", "Path to a unix domain socket dir. If specified, Dapr API servers will use Unix Domain Sockets")
RunCmd.Flags().BoolVar(&enableAppHealth, "enable-app-health-check", false, "Enable health checks for the application using the protocol defined with app-protocol")
RunCmd.Flags().StringVar(&appHealthPath, "app-health-check-path", "", "Path used for health checks; HTTP only")
Expand Down
53 changes: 45 additions & 8 deletions pkg/runexec/runexec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package runexec

import (
"fmt"
"github.com/stretchr/testify/require"
"os"
"regexp"
"strings"
Expand Down Expand Up @@ -180,8 +181,8 @@ func TestRun(t *testing.T) {
AppProtocol: "http",
ComponentsPath: componentsDir,
AppSSL: true,
MaxRequestBodySize: -1,
HTTPReadBufferSize: -1,
MaxRequestBodySize: "-1",
HTTPReadBufferSize: "-1",
EnableAPILogging: true,
APIListenAddresses: "127.0.0.1",
}
Expand Down Expand Up @@ -294,8 +295,6 @@ func TestRun(t *testing.T) {
basicConfig.ProfilePort = 0
basicConfig.EnableProfiling = true
basicConfig.MaxConcurrency = 0
basicConfig.MaxRequestBodySize = 0
basicConfig.HTTPReadBufferSize = 0
basicConfig.AppProtocol = ""

basicConfig.SetDefaultFromSchema()
Expand All @@ -307,8 +306,8 @@ func TestRun(t *testing.T) {
assert.Equal(t, -1, basicConfig.ProfilePort)
assert.True(t, basicConfig.EnableProfiling)
assert.Equal(t, -1, basicConfig.MaxConcurrency)
assert.Equal(t, -1, basicConfig.MaxRequestBodySize)
assert.Equal(t, -1, basicConfig.HTTPReadBufferSize)
assert.Equal(t, "-1", basicConfig.MaxRequestBodySize)
assert.Equal(t, "-1", basicConfig.HTTPReadBufferSize)
assert.Equal(t, "http", basicConfig.AppProtocol)

// Test after Validate gets called.
Expand All @@ -322,8 +321,46 @@ func TestRun(t *testing.T) {
assert.Positive(t, basicConfig.ProfilePort)
assert.True(t, basicConfig.EnableProfiling)
assert.Equal(t, -1, basicConfig.MaxConcurrency)
assert.Equal(t, -1, basicConfig.MaxRequestBodySize)
assert.Equal(t, -1, basicConfig.HTTPReadBufferSize)
assert.Equal(t, "-1", basicConfig.MaxRequestBodySize)
assert.Equal(t, "-1", basicConfig.HTTPReadBufferSize)
assert.Equal(t, "http", basicConfig.AppProtocol)
})

t.Run("run with max body size without units", func(t *testing.T) {
basicConfig.MaxRequestBodySize = "4000000"

output, err := NewOutput(basicConfig)
require.NoError(t, err)
assertArgumentEqual(t, "max-body-size", "4M", output.DaprCMD.Args)
})

t.Run("run with max body size with units", func(t *testing.T) {
basicConfig.MaxRequestBodySize = "4Mi"

output, err := NewOutput(basicConfig)
require.NoError(t, err)
assertArgumentEqual(t, "max-body-size", "4Mi", output.DaprCMD.Args)

basicConfig.MaxRequestBodySize = "5M"

output, err = NewOutput(basicConfig)
require.NoError(t, err)
assertArgumentEqual(t, "max-body-size", "5M", output.DaprCMD.Args)
})

t.Run("run with read buffer size set without units", func(t *testing.T) {
basicConfig.HTTPReadBufferSize = "16001"

output, err := NewOutput(basicConfig)
require.NoError(t, err)
assertArgumentEqual(t, "read-buffer-size", "16001", output.DaprCMD.Args)
})

t.Run("run with read buffer size set with units", func(t *testing.T) {
basicConfig.HTTPReadBufferSize = "4Ki"

output, err := NewOutput(basicConfig)
require.NoError(t, err)
assertArgumentEqual(t, "read-buffer-size", "4Ki", output.DaprCMD.Args)
})
}
39 changes: 29 additions & 10 deletions pkg/standalone/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package standalone
import (
"context"
"fmt"
"k8s.io/apimachinery/pkg/api/resource"
"net"
"os"
"os/exec"
Expand Down Expand Up @@ -79,8 +80,8 @@ type SharedRunConfig struct {
ResourcesPaths []string `arg:"resources-path" yaml:"resourcesPaths"`
// Speicifcally omitted from annotations as appSSL is deprecated.
AppSSL bool `arg:"app-ssl" yaml:"appSSL"`
MaxRequestBodySize int `arg:"max-body-size" annotation:"dapr.io/max-body-size" yaml:"maxBodySize" default:"-1"`
HTTPReadBufferSize int `arg:"read-buffer-size" annotation:"dapr.io/read-buffer-size" yaml:"readBufferSize" default:"-1"`
MaxRequestBodySize string `arg:"max-body-size" annotation:"dapr.io/max-body-size" yaml:"maxBodySize" default:"-1"`
HTTPReadBufferSize string `arg:"read-buffer-size" annotation:"dapr.io/read-buffer-size" yaml:"readBufferSize" default:"-1"`
EnableAppHealth bool `arg:"enable-app-health-check" annotation:"dapr.io/enable-app-health-check" yaml:"enableAppHealthCheck"`
AppHealthPath string `arg:"app-health-check-path" annotation:"dapr.io/app-health-check-path" yaml:"appHealthCheckPath"`
AppHealthInterval int `arg:"app-health-probe-interval" annotation:"dapr.io/app-health-probe-interval" ifneq:"0" yaml:"appHealthProbeInterval"`
Expand Down Expand Up @@ -226,12 +227,21 @@ func (config *RunConfig) Validate() error {
if config.MaxConcurrency < 1 {
config.MaxConcurrency = -1
}
if config.MaxRequestBodySize < 0 {
config.MaxRequestBodySize = -1

if q, err := resource.ParseQuantity(config.MaxRequestBodySize); err != nil {
return fmt.Errorf("invalid max request body size: %w", err)
} else if q.Value() < 0 {
config.MaxRequestBodySize = "-1"
} else {
config.MaxRequestBodySize = q.String()
}

if config.HTTPReadBufferSize < 0 {
config.HTTPReadBufferSize = -1
if q, err := resource.ParseQuantity(config.HTTPReadBufferSize); err != nil {
return fmt.Errorf("invalid http read buffer size: %w", err)
} else if q.Value() < 0 {
config.HTTPReadBufferSize = "-1"
} else {
config.HTTPReadBufferSize = q.String()
}

err = config.validatePlacementHostAddr()
Expand Down Expand Up @@ -265,12 +275,21 @@ func (config *RunConfig) ValidateK8s() error {
if config.MaxConcurrency < 1 {
config.MaxConcurrency = -1
}
if config.MaxRequestBodySize < 0 {
config.MaxRequestBodySize = -1

if q, err := resource.ParseQuantity(config.MaxRequestBodySize); err != nil {
return fmt.Errorf("invalid max request body size: %w", err)
} else if q.Value() <= 0 {
config.MaxRequestBodySize = "-1"
} else {
config.MaxRequestBodySize = q.String()
}

if config.HTTPReadBufferSize < 0 {
config.HTTPReadBufferSize = -1
if q, err := resource.ParseQuantity(config.HTTPReadBufferSize); err != nil {
return fmt.Errorf("invalid http read buffer size: %w", err)
} else if q.Value() <= 0 {
config.HTTPReadBufferSize = "-1"
} else {
config.HTTPReadBufferSize = q.String()
}

return nil
Expand Down

0 comments on commit ae325cd

Please sign in to comment.