From 6d2e81dfbe7e623d5fbf306c466a43fcca526f35 Mon Sep 17 00:00:00 2001 From: Paul Stack Date: Tue, 16 Aug 2016 17:55:06 +0100 Subject: [PATCH] provider/aws: `aws_spot_fleet_request` throws panic on missing subnet_id (#8217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit or availability_zone Fixes #8000 There was a hard coded panic in the code!!! ``` panic( fmt.Sprintf( "Must set one of:\navailability_zone %#v\nsubnet_id: %#v", m["availability_zone"], m["subnet_id"]) ) ``` This was causing issues when we set neither an availability zone or a subnet id. This has been removed and is now handled with an error rather than a panic. This was what happened with the new test before the fix: ``` === RUN TestAccAWSSpotFleetRequest_brokenLaunchSpecification panic: Must set one of: availability_zone "" subnet_id: "" goroutine 129 [running]: panic(0x11377a0, 0xc8202abfc0) /opt/boxen/homebrew/Cellar/go/1.6.2/libexec/src/runtime/panic.go:481 +0x3e6 github.com/hashicorp/terraform/builtin/providers/aws.hashLaunchSpecification(0x11361a0, 0xc8202e07e0, 0xc800000001) /Users/stacko/Code/go/src/github.com/hashicorp/terraform/builtin/providers/aws/resource_aws_spot_fleet_request.go:953 +0x685 github.com/hashicorp/terraform/helper/schema.(*Set).hash(0xc82005ae00, 0x11361a0, 0xc8202e07e0, 0x0, 0x0) /Users/stacko/Code/go/src/github.com/hashicorp/terraform/helper/schema/set.go:180 +0x40 github.com/hashicorp/terraform/helper/schema.(*Set).add(0xc82005ae00, 0x11361a0, 0xc8202e07e0, 0xc820276900, 0x0, 0x0) ``` The test then ran fine after the fix: ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSpotFleetRequest_brokenLaunchSpecification' ==> Checking that code complies with gofmt requirements... /Users/stacko/Code/go/bin/stringer go generate $(go list ./... | grep -v /terraform/vendor/) 2016/08/16 08:03:18 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSpotFleetRequest_brokenLaunchSpecification -timeout 120m === RUN TestAccAWSSpotFleetRequest_brokenLaunchSpecification --- PASS: TestAccAWSSpotFleetRequest_brokenLaunchSpecification (32.37s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 32.384s ``` Full test run looks as follows: ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSpotFleetRequest_' ✹ ==> Checking that code complies with gofmt requirements... /Users/stacko/Code/go/bin/stringer go generate $(go list ./... | grep -v /terraform/vendor/) 2016/08/16 08:04:34 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSpotFleetRequest_ -timeout 120m === RUN TestAccAWSSpotFleetRequest_basic --- PASS: TestAccAWSSpotFleetRequest_basic (33.78s) === RUN TestAccAWSSpotFleetRequest_brokenLaunchSpecification --- PASS: TestAccAWSSpotFleetRequest_brokenLaunchSpecification (33.59s) === RUN TestAccAWSSpotFleetRequest_launchConfiguration --- PASS: TestAccAWSSpotFleetRequest_launchConfiguration (35.26s) === RUN TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName --- PASS: TestAccAWSSpotFleetRequest_CannotUseEmptyKeyName (0.00s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 102.648s ``` --- .../aws/resource_aws_spot_fleet_request.go | 13 ++-- .../resource_aws_spot_fleet_request_test.go | 60 +++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/builtin/providers/aws/resource_aws_spot_fleet_request.go b/builtin/providers/aws/resource_aws_spot_fleet_request.go index 7fb1a05ef8..669a8239aa 100644 --- a/builtin/providers/aws/resource_aws_spot_fleet_request.go +++ b/builtin/providers/aws/resource_aws_spot_fleet_request.go @@ -290,6 +290,13 @@ func resourceAwsSpotFleetRequest() *schema.Resource { func buildSpotFleetLaunchSpecification(d map[string]interface{}, meta interface{}) (*ec2.SpotFleetLaunchSpecification, error) { conn := meta.(*AWSClient).ec2conn + + _, hasSubnet := d["subnet_id"] + _, hasAZ := d["availability_zone"] + if !hasAZ && !hasSubnet { + return nil, fmt.Errorf("LaunchSpecification must include a subnet_id or an availability_zone") + } + opts := &ec2.SpotFleetLaunchSpecification{ ImageId: aws.String(d["ami"].(string)), InstanceType: aws.String(d["instance_type"].(string)), @@ -938,12 +945,6 @@ func hashLaunchSpecification(v interface{}) int { buf.WriteString(fmt.Sprintf("%s-", m["availability_zone"].(string))) } else if m["subnet_id"] != nil && m["subnet_id"] != "" { buf.WriteString(fmt.Sprintf("%s-", m["subnet_id"].(string))) - } else { - panic( - fmt.Sprintf( - "Must set one of:\navailability_zone %#v\nsubnet_id: %#v", - m["availability_zone"], - m["subnet_id"])) } buf.WriteString(fmt.Sprintf("%s-", m["instance_type"].(string))) buf.WriteString(fmt.Sprintf("%s-", m["spot_price"].(string))) diff --git a/builtin/providers/aws/resource_aws_spot_fleet_request_test.go b/builtin/providers/aws/resource_aws_spot_fleet_request_test.go index d6a2968e6d..ccb240542d 100644 --- a/builtin/providers/aws/resource_aws_spot_fleet_request_test.go +++ b/builtin/providers/aws/resource_aws_spot_fleet_request_test.go @@ -3,6 +3,7 @@ package aws import ( "encoding/base64" "fmt" + "regexp" "testing" "github.com/aws/aws-sdk-go/aws" @@ -33,6 +34,20 @@ func TestAccAWSSpotFleetRequest_basic(t *testing.T) { }) } +func TestAccAWSSpotFleetRequest_brokenLaunchSpecification(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSpotFleetRequestDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSSpotFleetRequestConfigBroken, + ExpectError: regexp.MustCompile("LaunchSpecification must include a subnet_id or an availability_zone"), + }, + }, + }) +} + func TestAccAWSSpotFleetRequest_launchConfiguration(t *testing.T) { var sfr ec2.SpotFleetRequestConfig @@ -208,6 +223,51 @@ resource "aws_spot_fleet_request" "foo" { } ` +const testAccAWSSpotFleetRequestConfigBroken = ` +resource "aws_key_pair" "debugging" { + key_name = "tmp-key" + public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com" +} + +resource "aws_iam_policy_attachment" "test-attach" { + name = "test-attachment" + roles = ["${aws_iam_role.test-role.name}"] + policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonEC2SpotFleetRole" +} + +resource "aws_iam_role" "test-role" { + name = "test-role" + assume_role_policy = <