Skip to content

Commit

Permalink
chore(manifest): add manifest validation boilerplate (aws#2815)
Browse files Browse the repository at this point in the history
<!-- Provide summary of changes -->
Part of aws#2818. This PR adds boilerplate for new manifest validation for LB svc. Also added validation for mutually exclusive fields. Will add for the other workload types, advanced type validation, and consolidate existing validations in following PRs
<!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License..
  • Loading branch information
iamhopaul123 authored and thrau committed Dec 9, 2022
1 parent 3656ac9 commit 5f1016f
Show file tree
Hide file tree
Showing 7 changed files with 946 additions and 38 deletions.
38 changes: 38 additions & 0 deletions internal/pkg/manifest/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package manifest

import (
"fmt"

"github.com/dustin/go-humanize/english"
)

// ErrInvalidWorkloadType occurs when a user requested a manifest template type that doesn't exist.
Expand Down Expand Up @@ -50,3 +52,39 @@ func (e *ErrUnknownProvider) Is(target error) bool {
_, ok := target.(*ErrUnknownProvider)
return ok
}

type errFieldMustBeSpecified struct {
missingField string
conditionalFields []string
}

func (e *errFieldMustBeSpecified) Error() string {
errMsg := fmt.Sprintf(`"%s" must be specified`, e.missingField)
if len(e.conditionalFields) == 0 {
return errMsg
}
return fmt.Sprintf(`%s if "%s" %s specified`, errMsg, english.WordSeries(e.conditionalFields, "or"),
english.PluralWord(len(e.conditionalFields), "is", "are"))
}

type errFieldMutualExclusive struct {
firstField string
secondField string
mustExist bool
}

func (e *errFieldMutualExclusive) Error() string {
if e.mustExist {
return fmt.Sprintf(`must specify one of "%s" and "%s"`, e.firstField, e.secondField)
}
return fmt.Sprintf(`must specify one, not both, of "%s" and "%s"`, e.firstField, e.secondField)
}

type errMinGreaterThanMax struct {
min int
max int
}

func (e *errMinGreaterThanMax) Error() string {
return fmt.Sprintf("min value %d cannot be greater than max value %d", e.min, e.max)
}
8 changes: 5 additions & 3 deletions internal/pkg/manifest/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import (
)

var (
errUnmarshalEFSOpts = errors.New(`cannot unmarshal efs field into bool or map`)
errInvalidEFSConfiguration = errors.New(`must specify one, not both, of "uid/gid" and "id/root_dir/auth"`)
errUnmarshalEFSOpts = errors.New(`cannot unmarshal "efs" field into bool or map`)
)

// Storage represents the options for external and native storage.
Expand Down Expand Up @@ -154,7 +153,10 @@ func (e *EFSVolumeConfiguration) unsetUIDConfig() {

func (e *EFSVolumeConfiguration) isValid() error {
if !e.EmptyBYOConfig() && !e.EmptyUIDConfig() {
return errInvalidEFSConfiguration
return &errFieldMutualExclusive{
firstField: "uid/gid",
secondField: "id/root_dir/auth",
}
}
return nil
}
Expand Down
21 changes: 11 additions & 10 deletions internal/pkg/manifest/svc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -44,17 +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, errInvalidRangeOpts
}

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
Expand Down Expand Up @@ -206,12 +201,18 @@ func (a *AdvancedCount) hasAutoscaling() bool {
func (a *AdvancedCount) IsValid() error {
// Spot translates to desiredCount; cannot specify with autoscaling
if a.Spot != nil && a.hasAutoscaling() {
return errInvalidAdvancedCount
return &errFieldMutualExclusive{
firstField: "spot",
secondField: "range/cpu_percentage/memory_percentage/requests/response_time",
}
}

// Range must be specified if using autoscaling
if a.Range.IsEmpty() && (a.CPU != nil || a.Memory != nil || a.Requests != nil || a.ResponseTime != nil) {
return errInvalidAutoscaling
return &errFieldMustBeSpecified{
missingField: "range",
conditionalFields: []string{"cpu_percentage", "memory_percentage", "requests", "response_time"},
}
}

return nil
Expand Down
52 changes: 31 additions & 21 deletions internal/pkg/manifest/svc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,13 +474,19 @@ func TestCount_UnmarshalYAML(t *testing.T) {
range: 1-10
spot: 3
`),
wantedError: errInvalidAdvancedCount,
wantedError: &errFieldMutualExclusive{
firstField: "spot",
secondField: "range/cpu_percentage/memory_percentage/requests/response_time",
},
},
"Error if autoscaling specified without range": {
inContent: []byte(`count:
cpu_percentage: 30
`),
wantedError: errInvalidAutoscaling,
wantedError: &errFieldMustBeSpecified{
missingField: "range",
conditionalFields: []string{"cpu_percentage", "memory_percentage", "requests", "response_time"},
},
},
"Error if unmarshalable": {
inContent: []byte(`count: badNumber
Expand Down Expand Up @@ -562,20 +568,16 @@ func TestRange_Parse(t *testing.T) {

wantedMin int
wantedMax int
wantedErr error
}{
"error when both range and RangeConfig specified": {
"success with range value": {
input: Range{
Value: &mockRange,
RangeConfig: RangeConfig{
Min: aws.Int(1),
Max: aws.Int(3),
},
},

wantedErr: errInvalidRangeOpts,
wantedMin: 1,
wantedMax: 10,
},
"success": {
"success with range config": {
input: Range{
RangeConfig: RangeConfig{
Min: aws.Int(2),
Expand All @@ -591,13 +593,9 @@ func TestRange_Parse(t *testing.T) {
t.Run(name, func(t *testing.T) {
gotMin, gotMax, err := tc.input.Parse()

if tc.wantedErr != nil {
require.EqualError(t, err, tc.wantedErr.Error())
} else {
require.NoError(t, err)
require.Equal(t, tc.wantedMin, gotMin)
require.Equal(t, tc.wantedMax, gotMax)
}
require.NoError(t, err)
require.Equal(t, tc.wantedMin, gotMin)
require.Equal(t, tc.wantedMax, gotMax)
})
}
}
Expand Down Expand Up @@ -814,7 +812,10 @@ func TestAdvancedCount_IsValid(t *testing.T) {
Requests: aws.Int(1000),
},

expectedErr: errInvalidAdvancedCount,
expectedErr: &errFieldMutualExclusive{
firstField: "spot",
secondField: "range/cpu_percentage/memory_percentage/requests/response_time",
},
},
"invalid with spot count and range": {
input: &AdvancedCount{
Expand All @@ -824,7 +825,10 @@ func TestAdvancedCount_IsValid(t *testing.T) {
},
},

expectedErr: errInvalidAdvancedCount,
expectedErr: &errFieldMutualExclusive{
firstField: "spot",
secondField: "range/cpu_percentage/memory_percentage/requests/response_time",
},
},
"invalid with spot count and range config": {
input: &AdvancedCount{
Expand All @@ -838,7 +842,10 @@ func TestAdvancedCount_IsValid(t *testing.T) {
},
},

expectedErr: errInvalidAdvancedCount,
expectedErr: &errFieldMutualExclusive{
firstField: "spot",
secondField: "range/cpu_percentage/memory_percentage/requests/response_time",
},
},
"invalid with autoscaling fields and no range": {
input: &AdvancedCount{
Expand All @@ -847,7 +854,10 @@ func TestAdvancedCount_IsValid(t *testing.T) {
Requests: aws.Int(1000),
},

expectedErr: errInvalidAutoscaling,
expectedErr: &errFieldMustBeSpecified{
missingField: "range",
conditionalFields: []string{"cpu_percentage", "memory_percentage", "requests", "response_time"},
},
},
}
for name, tc := range testCases {
Expand Down
Loading

0 comments on commit 5f1016f

Please sign in to comment.