builder/amazon/step_pre_validate: Add check for non-default VPCs

Subnet information is only really needed when the specified `vpc_id` is
not the default VPC for the region where the builder is being executed.
This change uses the AWS API to determine if the VPC provided is a
non-default VPC and only validates the existence of a `subnet_id` if a
user has provided a non-default `vpc_id`.

Tests after change
```
> make test TEST=./builder/amazon/... TESTARGS='-count=1 -v -run=TestStepPreValidate_checkVpc'
...

=== RUN   TestStepPreValidate_checkVpc
=== RUN   TestStepPreValidate_checkVpc/DefaultVpc
=== RUN   TestStepPreValidate_checkVpc/NonDefaultVpcNoSubnet
=== RUN   TestStepPreValidate_checkVpc/NonDefaultVpcWithSubnet
=== RUN   TestStepPreValidate_checkVpc/SubnetWithNoVpc
=== RUN   TestStepPreValidate_checkVpc/NoVpcInformation
--- PASS: TestStepPreValidate_checkVpc (0.00s)
    --- PASS: TestStepPreValidate_checkVpc/DefaultVpc (0.00s)
    --- PASS: TestStepPreValidate_checkVpc/NonDefaultVpcNoSubnet (0.00s)
    --- PASS: TestStepPreValidate_checkVpc/NonDefaultVpcWithSubnet (0.00s)
    --- PASS: TestStepPreValidate_checkVpc/SubnetWithNoVpc (0.00s)
    --- PASS: TestStepPreValidate_checkVpc/NoVpcInformation (0.00s)
PASS

...
```
This commit is contained in:
nywilken 2019-11-08 16:13:45 -05:00
parent ad34a4551b
commit f9f4726eff
6 changed files with 121 additions and 3 deletions

View File

@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
"github.com/hashicorp/packer/common/retry"
"github.com/hashicorp/packer/helper/multistep"
"github.com/hashicorp/packer/packer"
@ -20,6 +21,8 @@ type StepPreValidate struct {
DestAmiName string
ForceDeregister bool
AMISkipBuildRegion bool
VpcId string
SubnetId string
}
func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
@ -76,6 +79,7 @@ func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) mul
ui.Say("Force Deregister flag found, skipping prevalidating AMI Name")
return multistep.ActionContinue
}
if s.AMISkipBuildRegion {
ui.Say("skip_build_region was set; not prevalidating AMI name")
return multistep.ActionContinue
@ -83,6 +87,14 @@ func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) mul
ec2conn := state.Get("ec2").(*ec2.EC2)
// Validate VPC settings for non-default VPCs
ui.Say("Prevalidating any provided VPC information")
if err := s.checkVpc(ec2conn); err != nil {
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
}
ui.Say(fmt.Sprintf("Prevalidating AMI Name: %s", s.DestAmiName))
req, resp := ec2conn.DescribeImagesRequest(&ec2.DescribeImagesInput{
Filters: []*ec2.Filter{{
@ -91,13 +103,13 @@ func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) mul
}}})
req.RetryCount = 11
err := req.Send()
if err != nil {
err := fmt.Errorf("Error querying AMI: %s", err)
if err := req.Send(); err != nil {
err = fmt.Errorf("Error querying AMI: %s", err)
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
}
if len(resp.Images) > 0 {
err := fmt.Errorf("Error: AMI Name: '%s' is used by an existing AMI: %s", *resp.Images[0].Name, *resp.Images[0].ImageId)
state.Put("error", err)
@ -108,4 +120,24 @@ func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) mul
return multistep.ActionContinue
}
func (s *StepPreValidate) checkVpc(conn ec2iface.EC2API) error {
if s.VpcId != "" && s.SubnetId != "" {
// skip validation if both VpcId and SubnetId are provided; AWS API will error if something is wrong.
return nil
}
res, err := conn.DescribeVpcs(&ec2.DescribeVpcsInput{VpcIds: []*string{aws.String(s.VpcId)}})
if isAWSErr(err, "InvalidVpcID.NotFound", "") || err != nil {
return fmt.Errorf("Error retrieving VPC information for vpc_id %q", s.VpcId)
}
if res != nil && len(res.Vpcs) == 1 && res.Vpcs[0] != nil {
if isDefault := aws.BoolValue(res.Vpcs[0].IsDefault); !isDefault {
return fmt.Errorf("Error: subnet_id must be provided for non-default VPCs (%s)", s.VpcId)
}
}
return nil
}
// Cleanup ...
func (s *StepPreValidate) Cleanup(multistep.StateBag) {}

View File

@ -0,0 +1,72 @@
package common
import (
"fmt"
"strings"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
)
//DescribeVpcs mocks an ec2.DescribeVpcsOutput for a given input
func (m *mockEC2Conn) DescribeVpcs(input *ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error) {
m.lock.Lock()
m.copyImageCount++
m.lock.Unlock()
if input == nil || len(input.VpcIds) == 0 {
return nil, fmt.Errorf("oops looks like we need more input")
}
var isDefault bool
vpcID := aws.StringValue(input.VpcIds[0])
//only one default VPC per region
if strings.Contains("vpc-default-id", vpcID) {
isDefault = true
}
output := &ec2.DescribeVpcsOutput{
Vpcs: []*ec2.Vpc{
&ec2.Vpc{IsDefault: aws.Bool(isDefault),
VpcId: aws.String(vpcID),
},
},
}
return output, nil
}
func TestStepPreValidate_checkVpc(t *testing.T) {
tt := []struct {
name string
step StepPreValidate
errorExpected bool
}{
{"DefaultVpc", StepPreValidate{VpcId: "vpc-default-id"}, false},
{"NonDefaultVpcNoSubnet", StepPreValidate{VpcId: "vpc-1234567890"}, true},
{"NonDefaultVpcWithSubnet", StepPreValidate{VpcId: "vpc-1234567890", SubnetId: "subnet-1234567890"}, false},
{"SubnetWithNoVpc", StepPreValidate{SubnetId: "subnet-1234567890"}, false},
{"NoVpcInformation", StepPreValidate{}, false},
}
mockConn, err := getMockConn(nil, "")
if err != nil {
t.Fatal("unable to get a mock connection")
}
for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
err := tc.step.checkVpc(mockConn)
if tc.errorExpected && err == nil {
t.Errorf("expected a validation error for %q but got %q", tc.name, err)
}
if !tc.errorExpected && err != nil {
t.Errorf("expected a validation to pass for %q but got %q", tc.name, err)
}
})
}
}

View File

@ -177,6 +177,14 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa
runReq, runResp := ec2conn.RunInstancesRequest(runOpts)
runReq.RetryCount = 11
err = runReq.Send()
if isAWSErr(err, "VPCIdNotSpecified", "No default VPC for this user") && subnetId == "" {
err := fmt.Errorf("Error launching source instance: a valid Subnet Id was not specified")
state.Put("error", err)
ui.Error(err.Error())
return multistep.ActionHalt
}
if err != nil {
err := fmt.Errorf("Error launching source instance: %s", err)
state.Put("error", err)

View File

@ -190,6 +190,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
DestAmiName: b.config.AMIName,
ForceDeregister: b.config.AMIForceDeregister,
AMISkipBuildRegion: b.config.AMISkipBuildRegion,
VpcId: b.config.VpcId,
SubnetId: b.config.SubnetId,
},
&awscommon.StepSourceAMIInfo{
SourceAmi: b.config.SourceAmi,

View File

@ -232,6 +232,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
&awscommon.StepPreValidate{
DestAmiName: b.config.AMIName,
ForceDeregister: b.config.AMIForceDeregister,
VpcId: b.config.VpcId,
SubnetId: b.config.SubnetId,
},
&awscommon.StepSourceAMIInfo{
SourceAmi: b.config.SourceAmi,

View File

@ -286,6 +286,8 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
&awscommon.StepPreValidate{
DestAmiName: b.config.AMIName,
ForceDeregister: b.config.AMIForceDeregister,
VpcId: b.config.VpcId,
SubnetId: b.config.SubnetId,
},
&awscommon.StepSourceAMIInfo{
SourceAmi: b.config.SourceAmi,