Repository: dapr/cli (go)

Issue:
`max-body-size` and `read-buffer-size` do not accept units
## Expected Behavior

<!-- Briefly describe what you expect to happen -->
I should be able to define the arguments `max-body-size` and `read-buffer-size` with units, e.g. "4Mi" as per docs.
https://v1-15.docs.dapr.io/reference/arguments-annotations-overview/

## Actual Behavior

<!-- Briefly describe what is actually happening -->
At the moment, the flag is being parsed as an int rather than a string before invoking the runtime.

## Steps to Reproduce the Problem

<!-- How can a maintainer reproduce this issue (be detailed) -->
```
dapr run --app-id custom-grpc-client \
		 -d ./config \
		 --max-body-size 4Mi \
		 --log-level debug \
		 go run ./custom-grpc-client/main.go
```

## Release Note

<!-- How should the fix for this issue be communicated in our release notes? It can be populated later. -->
<!-- Keep it as a single line. Examples: -->

<!-- RELEASE NOTE: **ADD** New feature in Dapr. -->
<!-- RELEASE NOTE: **FIX** Bug in runtime. -->
<!-- RELEASE NOTE: **UPDATE** Runtime dependency. -->

RELEASE NOTE: **FIX** Allow `max-body-size` and `read-buffer-size` to be defined with units

Pull request:
fix: arguments accept units
`max-body-size` and `read-buffer-size` now accept units as defined in the docs.

Defaults of 4Mi and 4Ki respectively are also set

Fixes #1489

# Description

_Please explain the changes you've made_

## Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #_[issue number]_

## Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

* [ ] Code compiles correctly
* [ ] Created/updated tests
* [ ] Extended the documentation


Patch:
diff --git a/cmd/annotate.go b/cmd/annotate.go
index 6dd5737..c68c172 100644
--- a/cmd/annotate.go
+++ b/cmd/annotate.go
@@ -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"
 
@@ -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
@@ -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())
 	}
@@ -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")
diff --git a/cmd/run.go b/cmd/run.go
index c6dfdd9..31c4f6f 100644
--- a/cmd/run.go
+++ b/cmd/run.go
@@ -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"
@@ -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
@@ -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")
diff --git a/pkg/standalone/run.go b/pkg/standalone/run.go
index 2752eda..fddfa6b 100644
--- a/pkg/standalone/run.go
+++ b/pkg/standalone/run.go
@@ -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"
@@ -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"`
@@ -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()
@@ -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


Test patch:
diff --git a/pkg/runexec/runexec_test.go b/pkg/runexec/runexec_test.go
index 14ff619..740cdb1 100644
--- a/pkg/runexec/runexec_test.go
+++ b/pkg/runexec/runexec_test.go
@@ -20,6 +20,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/stretchr/testify/assert"
 
 	"github.com/dapr/cli/pkg/standalone"
@@ -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",
 	}
@@ -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()
@@ -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.
@@ -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)
+	})
 }