From ad8274fa47868d4324e66da18d6ac0326231b15a Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Fri, 8 Apr 2022 00:37:37 -0400 Subject: [PATCH 1/4] chore: add lambda platform config validation - add tests --- builtin/aws/lambda/platform.go | 39 +++++++++++++++++- builtin/aws/lambda/platform_test.go | 62 ++++++++++++++--------------- 2 files changed, 68 insertions(+), 33 deletions(-) diff --git a/builtin/aws/lambda/platform.go b/builtin/aws/lambda/platform.go index 5607e7d477a..565cb714434 100644 --- a/builtin/aws/lambda/platform.go +++ b/builtin/aws/lambda/platform.go @@ -19,6 +19,7 @@ import ( "github.com/aws/aws-sdk-go/service/elbv2" "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/lambda" + validation "github.com/go-ozzo/ozzo-validation/v4" "github.com/hashicorp/go-hclog" "github.com/hashicorp/waypoint/builtin/aws/ecr" "github.com/hashicorp/waypoint/builtin/aws/utils" @@ -38,12 +39,46 @@ type Platform struct { // ConfigSet is called after a configuration has been decoded // we can use this to validate the config func (p *Platform) ConfigSet(config interface{}) error { - _, ok := config.(*Config) + c, ok := config.(*Config) if !ok { // this should never happen return fmt.Errorf("Invalid configuration, expected *lambda.Config, got %s", reflect.TypeOf(config)) } + // validate architecture + if c.Architecture != "" { + architectures := make([]interface{}, len(lambda.Architecture_Values())) + + for i, ca := range lambda.Architecture_Values() { + architectures[i] = ca + } + + var validArchitectures []string + for _, arch := range lambda.Architecture_Values() { + validArchitectures = append(validArchitectures, fmt.Sprintf("\"%s\"", arch)) + } + + if err := utils.Error(validation.ValidateStruct(c, + validation.Field(&c.Architecture, + validation.In(architectures...).Error(fmt.Sprintf("Unsupported function architecture \"%s\". Must be one of [%s], or left blank", c.Architecture, strings.Join(validArchitectures, ", "))), + ), + )); err != nil { + return err + } + } + + // validate timeout - max is 900 (15 minutes) + if c.Timeout != 0 { + if err := utils.Error(validation.ValidateStruct(c, + validation.Field(&c.Timeout, + validation.Min(0).Error("Timeout must not be negative"), + validation.Max(900).Error("Timeout must be less than or equal to 15 minutes"), + ), + )); err != nil { + return err + } + } + return nil } @@ -822,7 +857,7 @@ type Config struct { Memory int `hcl:"memory,optional"` // The number of seconds to wait for a function to complete it's work. - // Defaults to 256 + // Defaults to 60 Timeout int `hcl:"timeout,optional"` // The instruction set architecture that the function supports. diff --git a/builtin/aws/lambda/platform_test.go b/builtin/aws/lambda/platform_test.go index dd80d84ff36..9ee9ef9507f 100644 --- a/builtin/aws/lambda/platform_test.go +++ b/builtin/aws/lambda/platform_test.go @@ -1,40 +1,40 @@ package lambda import ( - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "encoding/base64" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/crypto/ssh" ) -// Just to validate that the key armoring works properly -func TestKeys(t *testing.T) { - hostkey, err := rsa.GenerateKey(rand.Reader, 4096) - require.NoError(t, err) - - hoststr := base64.StdEncoding.EncodeToString(x509.MarshalPKCS1PrivateKey(hostkey)) - - hostbytes, err := base64.StdEncoding.DecodeString(hoststr) - require.NoError(t, err) - - hkey, err := x509.ParsePKCS1PrivateKey(hostbytes) - require.NoError(t, err) - - assert.True(t, hostkey.Equal(hkey)) - - userstr := base64.StdEncoding.EncodeToString(x509.MarshalPKCS1PublicKey(&hostkey.PublicKey)) - - userbytes, err := base64.StdEncoding.DecodeString(userstr) - require.NoError(t, err) - - userKey, err := x509.ParsePKCS1PublicKey(userbytes) - require.NoError(t, err) - - _, err = ssh.NewPublicKey(userKey) - require.NoError(t, err) +func TestPlatformConfig(t *testing.T) { + t.Run("empty is fine", func(t *testing.T) { + var p Platform + cfg := &Config{} + require.NoError(t, p.ConfigSet(cfg)) + }) + + t.Run("disallows unsupported architecture", func(t *testing.T) { + var p Platform + cfg := &Config{ + Architecture: "foobar", + } + require.Error(t, p.ConfigSet(cfg)) + }) + + t.Run("disallows invalid timeout", func(t *testing.T) { + var p Platform + { + cfg := &Config{ + Timeout: 901, + } + require.Error(t, p.ConfigSet(cfg)) + } + + { + cfg := &Config{ + Timeout: -1, + } + require.Error(t, p.ConfigSet(cfg)) + } + }) } From 3e88537fc0920a883650cc61fde5eb285191217a Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Fri, 8 Apr 2022 00:56:03 -0400 Subject: [PATCH 2/4] chore: changelog --- .changelog/3193.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/3193.txt diff --git a/.changelog/3193.txt b/.changelog/3193.txt new file mode 100644 index 00000000000..7996a5f8791 --- /dev/null +++ b/.changelog/3193.txt @@ -0,0 +1,3 @@ +```release-note:improvement +plugin/aws-lambda: Add platform config validation. +``` From eb41392459850a2c88225b65ff098b66b203195f Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 10 Apr 2022 01:42:04 -0400 Subject: [PATCH 3/4] Apply suggestions from code review use `%q` over `\"%s\"` Co-authored-by: Brian Cain --- builtin/aws/lambda/platform.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/aws/lambda/platform.go b/builtin/aws/lambda/platform.go index 565cb714434..f695e328571 100644 --- a/builtin/aws/lambda/platform.go +++ b/builtin/aws/lambda/platform.go @@ -42,7 +42,7 @@ func (p *Platform) ConfigSet(config interface{}) error { c, ok := config.(*Config) if !ok { // this should never happen - return fmt.Errorf("Invalid configuration, expected *lambda.Config, got %s", reflect.TypeOf(config)) + return fmt.Errorf("Invalid configuration, expected *lambda.Config, got %q", reflect.TypeOf(config)) } // validate architecture @@ -55,12 +55,12 @@ func (p *Platform) ConfigSet(config interface{}) error { var validArchitectures []string for _, arch := range lambda.Architecture_Values() { - validArchitectures = append(validArchitectures, fmt.Sprintf("\"%s\"", arch)) + validArchitectures = append(validArchitectures, fmt.Sprintf("%q", arch)) } if err := utils.Error(validation.ValidateStruct(c, validation.Field(&c.Architecture, - validation.In(architectures...).Error(fmt.Sprintf("Unsupported function architecture \"%s\". Must be one of [%s], or left blank", c.Architecture, strings.Join(validArchitectures, ", "))), + validation.In(architectures...).Error(fmt.Sprintf("Unsupported function architecture %q. Must be one of [%s], or left blank", c.Architecture, strings.Join(validArchitectures, ", "))), ), )); err != nil { return err From 269be1b33f17b289ae2be52caa898c5597f4175b Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 10 Apr 2022 23:04:54 -0400 Subject: [PATCH 4/4] chore: explicit errors --- builtin/aws/lambda/platform_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/aws/lambda/platform_test.go b/builtin/aws/lambda/platform_test.go index 9ee9ef9507f..ec7d8cb0bf5 100644 --- a/builtin/aws/lambda/platform_test.go +++ b/builtin/aws/lambda/platform_test.go @@ -18,7 +18,8 @@ func TestPlatformConfig(t *testing.T) { cfg := &Config{ Architecture: "foobar", } - require.Error(t, p.ConfigSet(cfg)) + + require.EqualError(t, p.ConfigSet(cfg), "rpc error: code = InvalidArgument desc = Architecture: Unsupported function architecture \"foobar\". Must be one of [\"x86_64\", \"arm64\"], or left blank.") }) t.Run("disallows invalid timeout", func(t *testing.T) { @@ -27,14 +28,14 @@ func TestPlatformConfig(t *testing.T) { cfg := &Config{ Timeout: 901, } - require.Error(t, p.ConfigSet(cfg)) + require.EqualError(t, p.ConfigSet(cfg), "rpc error: code = InvalidArgument desc = Timeout: Timeout must be less than or equal to 15 minutes.") } { cfg := &Config{ Timeout: -1, } - require.Error(t, p.ConfigSet(cfg)) + require.EqualError(t, p.ConfigSet(cfg), "rpc error: code = InvalidArgument desc = Timeout: Timeout must not be negative.") } }) }