diff --git a/internal/pkg/manifest/errors.go b/internal/pkg/manifest/errors.go index 4575c58e199..734b541de83 100644 --- a/internal/pkg/manifest/errors.go +++ b/internal/pkg/manifest/errors.go @@ -5,7 +5,6 @@ package manifest import ( "fmt" - "strings" "github.com/dustin/go-humanize/english" ) @@ -64,7 +63,7 @@ func (e *errFieldMustBeSpecified) Error() string { if len(e.conditionalFields) == 0 { return errMsg } - return fmt.Sprintf(`%s if "%s" %s specified`, errMsg, strings.Join(e.conditionalFields, `", "`), + return fmt.Sprintf(`%s if "%s" %s specified`, errMsg, english.WordSeries(e.conditionalFields, "or"), english.PluralWord(len(e.conditionalFields), "is", "are")) } diff --git a/internal/pkg/manifest/storage.go b/internal/pkg/manifest/storage.go index 11387f4a50c..782df8f1b20 100644 --- a/internal/pkg/manifest/storage.go +++ b/internal/pkg/manifest/storage.go @@ -11,7 +11,7 @@ import ( ) var ( - errUnmarshalEFSOpts = errors.New(`cannot unmarshal efs field into bool or map`) + errUnmarshalEFSOpts = errors.New(`cannot unmarshal "efs" field into bool or map`) ) // Storage represents the options for external and native storage. diff --git a/internal/pkg/manifest/svc.go b/internal/pkg/manifest/svc.go index 6eac7b28749..374802a5f08 100644 --- a/internal/pkg/manifest/svc.go +++ b/internal/pkg/manifest/svc.go @@ -33,7 +33,7 @@ var ServiceTypes = []string{ WorkerServiceType, } -// Range contains either a Range or a range configuration for Autoscaling ranges +// Range contains either a Range or a range configuration for Autoscaling ranges. type Range struct { Value *IntRangeBand // Mutually exclusive with RangeConfig RangeConfig RangeConfig @@ -44,20 +44,12 @@ func (r *Range) IsEmpty() bool { return r.Value == nil && r.RangeConfig.IsEmpty() } -// Parse extracts the min and max from RangeOpts +// Parse extracts the min and max from RangeOpts. func (r *Range) Parse() (min int, max int, err error) { - if r.Value != nil && !r.RangeConfig.IsEmpty() { - return 0, 0, &errFieldMutualExclusive{ - firstField: "range", - secondField: "min/max", - } - } - if r.Value != nil { return r.Value.Parse() } - - return *r.RangeConfig.Min, *r.RangeConfig.Max, nil + return aws.IntValue(r.RangeConfig.Min), aws.IntValue(r.RangeConfig.Max), nil } // UnmarshalYAML overrides the default YAML unmarshaling logic for the RangeOpts diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 4933d91c1fc..a60a8deda19 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -97,7 +97,10 @@ func (i *Image) Validate() error { // Validate returns if BuildArgsOrString is configured correctly. func (b *BuildArgsOrString) Validate() error { - return b.BuildArgs.Validate() + if !b.BuildArgs.isEmpty() { + return b.BuildArgs.Validate() + } + return nil } // Validate returns if DockerBuildArgs is configured correctly. @@ -221,8 +224,7 @@ func (a *AdvancedCount) Validate() error { if err := a.Range.Validate(); err != nil { return fmt.Errorf(`validate "range": %w`, err) } - } - if a.CPU != nil || a.Memory != nil || a.Requests != nil || a.ResponseTime != nil { + } else if a.CPU != nil || a.Memory != nil || a.Requests != nil || a.ResponseTime != nil { return &errFieldMustBeSpecified{ missingField: "range", conditionalFields: []string{"cpu_percentage", "memory_percentage", "requests", "response_time"}, @@ -267,6 +269,18 @@ func (r *IntRangeBand) Validate() error { // Validate returns if RangeConfig is configured correctly. func (r *RangeConfig) Validate() error { + if r.Min == nil && r.Max != nil { + return &errFieldMustBeSpecified{ + missingField: "min", + conditionalFields: []string{"max"}, + } + } + if r.Min != nil && r.Max == nil { + return &errFieldMustBeSpecified{ + missingField: "max", + conditionalFields: []string{"min"}, + } + } min, max := aws.IntValue(r.Min), aws.IntValue(r.Max) if min <= max { return nil @@ -344,12 +358,12 @@ func (e *EFSVolumeConfiguration) Validate() error { if err := e.AuthConfig.Validate(); err != nil { return fmt.Errorf(`validate "auth": %w`, err) } - if aws.StringValue(e.AuthConfig.AccessPointID) != "" { + if e.AuthConfig.AccessPointID != nil { if (aws.StringValue(e.RootDirectory) == "" || aws.StringValue(e.RootDirectory) == "/") && aws.BoolValue(e.AuthConfig.IAM) { return nil } - return fmt.Errorf("root_dir must be either empty or / and auth.iam must be true when access_point_id is in used") + return fmt.Errorf(`"root_dir" must be either empty or "/" and "auth.iam" must be true when "access_point_id" is used`) } return nil } diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 68bbdec4183..a066548a288 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -243,7 +243,7 @@ func TestAdvancedCount_Validate(t *testing.T) { AdvancedCount: AdvancedCount{ Requests: aws.Int(123), }, - wantedError: fmt.Errorf(`"range" must be specified if "cpu_percentage", "memory_percentage", "requests", "response_time" are specified`), + wantedError: fmt.Errorf(`"range" must be specified if "cpu_percentage, memory_percentage, requests or response_time" are specified`), }, } for name, tc := range testCases { @@ -297,6 +297,18 @@ func TestRangeConfig_Validate(t *testing.T) { wantedError error }{ + "error if min is set but max is not": { + RangeConfig: RangeConfig{ + Min: aws.Int(2), + }, + wantedError: fmt.Errorf(`"max" must be specified if "min" is specified`), + }, + "error if max is set but min is not": { + RangeConfig: RangeConfig{ + Max: aws.Int(1), + }, + wantedError: fmt.Errorf(`"min" must be specified if "max" is specified`), + }, "error if range min is greater than max": { RangeConfig: RangeConfig{ Min: aws.Int(2), @@ -420,7 +432,7 @@ func TestEFSVolumeConfiguration_Validate(t *testing.T) { AccessPointID: aws.String("mockID"), }, }, - wantedError: fmt.Errorf("root_dir must be either empty or / and auth.iam must be true when access_point_id is in used"), + wantedError: fmt.Errorf(`"root_dir" must be either empty or "/" and "auth.iam" must be true when "access_point_id" is used`), }, } for name, tc := range testCases {