Skip to content

Commit

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

Fixes #1489

Signed-off-by: Mike Nguyen <hey@mike.ee>

* chore: gofumpt

Signed-off-by: Mike Nguyen <hey@mike.ee>

* refactor: modify logic to comply with vetting

Signed-off-by: Mike Nguyen <hey@mike.ee>

* chore: gofumpt -w .

Signed-off-by: Mike Nguyen <hey@mike.ee>

* refactor: set defaults
`max-body-size` is defaulted to 4Mi
`request-buffer-size` is defaulted to 4Ki

This is inline with the runtime.

Signed-off-by: Mike Nguyen <hey@mike.ee>

* fix: set defaults in run and annotate

Signed-off-by: Mike Nguyen <hey@mike.ee>

* chore: gofumpt

Signed-off-by: Mike Nguyen <hey@mike.ee>

* refactor: exit with error rather than panic

Co-authored-by: Anton Troshin <troll.sic@gmail.com>
Signed-off-by: Mike Nguyen <hey@mike.ee>

---------

Signed-off-by: Mike Nguyen <hey@mike.ee>
Co-authored-by: Anton Troshin <troll.sic@gmail.com>
  • Loading branch information
@mikeee @antontroshin
mikeee and antontroshin authored Feb 21, 2025
1 parent a968b18 commit 0cd0585
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 30 deletions.
32 changes: 24 additions & 8 deletions cmd/annotate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import (
"net/url"
"os"
"path/filepath"
"strconv"

"github.com/dapr/dapr/pkg/runtime"

"k8s.io/apimachinery/pkg/api/resource"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -60,8 +65,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 +323,23 @@ 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 {
print.FailureStatusEvent(os.Stderr, "error parsing value of max-body-size: %s, error: %s", annotateMaxRequestBodySize, err.Error())
os.Exit(1)
} 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 {
print.FailureStatusEvent(os.Stderr, "error parsing value of read-buffer-size: %s, error: %s", annotateMaxRequestBodySize, err.Error())
os.Exit(1)
} else {
o = append(o, kubernetes.WithReadBufferSize(int(q.Value())))
}
}

if annotateHTTPStreamRequestBody {
o = append(o, kubernetes.WithHTTPStreamRequestBody())
}
Expand Down Expand Up @@ -385,8 +401,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", strconv.Itoa(runtime.DefaultMaxRequestBodySize>>20)+"Mi", "The maximum request body size to use")
AnnotateCmd.Flags().StringVar(&annotateReadBufferSize, "read-buffer-size", strconv.Itoa(runtime.DefaultReadBufferSize>>10)+"Ki", "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
10 changes: 6 additions & 4 deletions cmd/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/viper"

daprRuntime "github.com/dapr/dapr/pkg/runtime"

"github.com/dapr/cli/pkg/kubernetes"
"github.com/dapr/cli/pkg/metadata"
"github.com/dapr/cli/pkg/print"
Expand All @@ -55,8 +57,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 +475,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", "", strconv.Itoa(daprRuntime.DefaultMaxRequestBodySize>>20)+"Mi", "Max size of request body in MB")
RunCmd.Flags().StringVarP(&readBufferSize, "read-buffer-size", "", strconv.Itoa(daprRuntime.DefaultReadBufferSize>>10)+"Ki", "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
56 changes: 48 additions & 8 deletions pkg/runexec/runexec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"strings"
"testing"

"github.com/stretchr/testify/require"

"github.com/stretchr/testify/assert"

"github.com/dapr/cli/pkg/standalone"
Expand Down Expand Up @@ -180,8 +182,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 +296,8 @@ func TestRun(t *testing.T) {
basicConfig.ProfilePort = 0
basicConfig.EnableProfiling = true
basicConfig.MaxConcurrency = 0
basicConfig.MaxRequestBodySize = 0
basicConfig.HTTPReadBufferSize = 0
basicConfig.MaxRequestBodySize = ""
basicConfig.HTTPReadBufferSize = ""
basicConfig.AppProtocol = ""

basicConfig.SetDefaultFromSchema()
Expand All @@ -307,8 +309,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, "4Mi", basicConfig.MaxRequestBodySize)
assert.Equal(t, "4Ki", basicConfig.HTTPReadBufferSize)
assert.Equal(t, "http", basicConfig.AppProtocol)

// Test after Validate gets called.
Expand All @@ -322,8 +324,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, "4Mi", basicConfig.MaxRequestBodySize)
assert.Equal(t, "4Ki", 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)
})
}
52 changes: 42 additions & 10 deletions pkg/standalone/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"strconv"
"strings"

"k8s.io/apimachinery/pkg/api/resource"

"github.com/Pallinder/sillyname-go"
"github.com/phayes/freeport"
"gopkg.in/yaml.v2"
Expand Down Expand Up @@ -79,8 +81,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:"4Mi"`
HTTPReadBufferSize string `arg:"read-buffer-size" annotation:"dapr.io/read-buffer-size" yaml:"readBufferSize" default:"4Ki"`
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 +228,27 @@ func (config *RunConfig) Validate() error {
if config.MaxConcurrency < 1 {
config.MaxConcurrency = -1
}
if config.MaxRequestBodySize < 0 {
config.MaxRequestBodySize = -1

qBody, err := resource.ParseQuantity(config.MaxRequestBodySize)
if err != nil {
return fmt.Errorf("invalid max request body size: %w", err)
}

if qBody.Value() < 0 {
config.MaxRequestBodySize = "-1"
} else {
config.MaxRequestBodySize = qBody.String()
}

qBuffer, err := resource.ParseQuantity(config.HTTPReadBufferSize)
if err != nil {
return fmt.Errorf("invalid http read buffer size: %w", err)
}

if config.HTTPReadBufferSize < 0 {
config.HTTPReadBufferSize = -1
if qBuffer.Value() < 0 {
config.HTTPReadBufferSize = "-1"
} else {
config.HTTPReadBufferSize = qBuffer.String()
}

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

qBody, err := resource.ParseQuantity(config.MaxRequestBodySize)
if err != nil {
return fmt.Errorf("invalid max request body size: %w", err)
}

if qBody.Value() < 0 {
config.MaxRequestBodySize = "-1"
} else {
config.MaxRequestBodySize = qBody.String()
}

qBuffer, err := resource.ParseQuantity(config.HTTPReadBufferSize)
if err != nil {
return fmt.Errorf("invalid http read buffer size: %w", err)
}

if config.HTTPReadBufferSize < 0 {
config.HTTPReadBufferSize = -1
if qBuffer.Value() < 0 {
config.HTTPReadBufferSize = "-1"
} else {
config.HTTPReadBufferSize = qBuffer.String()
}

return nil
Expand Down

0 comments on commit 0cd0585

Please sign in to comment.