provider/aws: `aws_spot_fleet_request` throws panic on missing subnet_id (#8217)

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
```
This commit is contained in:
Paul Stack 2016-08-16 17:55:06 +01:00 committed by GitHub
parent ddb62d026e
commit 6d2e81dfbe
2 changed files with 67 additions and 6 deletions

View File

@ -290,6 +290,13 @@ func resourceAwsSpotFleetRequest() *schema.Resource {
func buildSpotFleetLaunchSpecification(d map[string]interface{}, meta interface{}) (*ec2.SpotFleetLaunchSpecification, error) { func buildSpotFleetLaunchSpecification(d map[string]interface{}, meta interface{}) (*ec2.SpotFleetLaunchSpecification, error) {
conn := meta.(*AWSClient).ec2conn 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{ opts := &ec2.SpotFleetLaunchSpecification{
ImageId: aws.String(d["ami"].(string)), ImageId: aws.String(d["ami"].(string)),
InstanceType: aws.String(d["instance_type"].(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))) buf.WriteString(fmt.Sprintf("%s-", m["availability_zone"].(string)))
} else if m["subnet_id"] != nil && m["subnet_id"] != "" { } else if m["subnet_id"] != nil && m["subnet_id"] != "" {
buf.WriteString(fmt.Sprintf("%s-", m["subnet_id"].(string))) 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["instance_type"].(string)))
buf.WriteString(fmt.Sprintf("%s-", m["spot_price"].(string))) buf.WriteString(fmt.Sprintf("%s-", m["spot_price"].(string)))

View File

@ -3,6 +3,7 @@ package aws
import ( import (
"encoding/base64" "encoding/base64"
"fmt" "fmt"
"regexp"
"testing" "testing"
"github.com/aws/aws-sdk-go/aws" "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) { func TestAccAWSSpotFleetRequest_launchConfiguration(t *testing.T) {
var sfr ec2.SpotFleetRequestConfig 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 = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Sid": "",
"Effect": "Allow",
"Principal": {
"Service": "spotfleet.amazonaws.com"
},
"Action": "sts:AssumeRole"
}
]
}
EOF
}
resource "aws_spot_fleet_request" "foo" {
iam_fleet_role = "${aws_iam_role.test-role.arn}"
spot_price = "0.005"
target_capacity = 2
valid_until = "2019-11-04T20:44:20Z"
launch_specification {
instance_type = "m1.small"
ami = "ami-d06a90b0"
key_name = "${aws_key_pair.debugging.key_name}"
}
depends_on = ["aws_iam_policy_attachment.test-attach"]
}
`
const testAccAWSSpotFleetRequestWithAdvancedLaunchSpecConfig = ` const testAccAWSSpotFleetRequestWithAdvancedLaunchSpecConfig = `
resource "aws_key_pair" "debugging" { resource "aws_key_pair" "debugging" {
key_name = "tmp-key" key_name = "tmp-key"