Merge pull request #6512 from sharmaanshul2102/5786-use-describe-regions-for-validation-aws

Use DescribeRegions for aws region validation

builder/amazon/chroot/builder_test.go

@@ -11,6 +11,8 @@ func testConfig() map[string]interface{} {
 		"ami_name":   "foo",
 		"source_ami": "foo",
 		"region":     "us-east-1",
+		// region validation logic is checked in ami_config_test
+		"skip_region_validation": true,
 	}
 }
 
@@ -28,6 +30,7 @@ func TestBuilderPrepare_AMIName(t *testing.T) {
 
 	// Test good
 	config["ami_name"] = "foo"
+	config["skip_region_validation"] = true
 	warnings, err := b.Prepare(config)
 	if len(warnings) > 0 {
 		t.Fatalf("bad: %#v", warnings)

builder/amazon/common/access_config.go

@@ -148,7 +148,8 @@ func (c *AccessConfig) Prepare(ctx *interpolate.Context) []error {
 	}
 
 	if c.RawRegion != "" && !c.SkipValidation {
-		if valid := ValidateRegion(c.RawRegion); !valid {
+		ec2conn := getValidationSession()
+		if valid := ValidateRegion(c.RawRegion, ec2conn); !valid {
 			errs = append(errs, fmt.Errorf("Unknown region: %s", c.RawRegion))
 		}
 	}

builder/amazon/common/access_config_test.go

@@ -13,33 +13,40 @@ func testAccessConfig() *AccessConfig {
 
 func TestAccessConfigPrepare_Region(t *testing.T) {
 	c := testAccessConfig()
-	c.RawRegion = ""
+
-	if err := c.Prepare(nil); err != nil {
+	mockConn := &mockEC2Client{}
-		t.Fatalf("shouldn't have err: %s", err)
-	}
 
 	c.RawRegion = "us-east-12"
-	if err := c.Prepare(nil); err == nil {
+	valid := ValidateRegion(c.RawRegion, mockConn)
-		t.Fatal("should have error")
+	if valid {
+		t.Fatalf("should have region validation err: %s", c.RawRegion)
 	}
 
 	c.RawRegion = "us-east-1"
-	if err := c.Prepare(nil); err != nil {
+	valid = ValidateRegion(c.RawRegion, mockConn)
-		t.Fatalf("shouldn't have err: %s", err)
+	if !valid {
+		t.Fatalf("shouldn't have region validation err: %s", c.RawRegion)
 	}
 
 	c.RawRegion = "custom"
-	if err := c.Prepare(nil); err == nil {
+	valid = ValidateRegion(c.RawRegion, mockConn)
-		t.Fatalf("should have err")
+	if valid {
+		t.Fatalf("should have region validation err: %s", c.RawRegion)
 	}
 
 	c.RawRegion = "custom"
 	c.SkipValidation = true
+	// testing whole prepare func here; this is checking that validation is
+	// skipped, so we don't need a mock connection
 	if err := c.Prepare(nil); err != nil {
 		t.Fatalf("shouldn't have err: %s", err)
 	}
-	c.SkipValidation = false
 
+	c.SkipValidation = false
+	c.RawRegion = ""
+	if err := c.Prepare(nil); err != nil {
+		t.Fatalf("shouldn't have err: %s", err)
+	}
 }
 
 func TestAccessConfigPrepare_RegionRestricted(t *testing.T) {

builder/amazon/common/ami_config.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"log"
 
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/hashicorp/packer/template/interpolate"
 )
 
@@ -56,44 +57,8 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context
 		}
 	}
 
-	if len(c.AMIRegions) > 0 {
+	ec2conn := getValidationSession()
-		regionSet := make(map[string]struct{})
+	errs = c.prepareRegions(ec2conn, accessConfig, errs)
-		regions := make([]string, 0, len(c.AMIRegions))
-
-		for _, region := range c.AMIRegions {
-			// If we already saw the region, then don't look again
-			if _, ok := regionSet[region]; ok {
-				continue
-			}
-
-			// Mark that we saw the region
-			regionSet[region] = struct{}{}
-
-			if !c.AMISkipRegionValidation {
-				// Verify the region is real
-				if valid := ValidateRegion(region); !valid {
-					errs = append(errs, fmt.Errorf("Unknown region: %s", region))
-				}
-			}
-
-			// Make sure that if we have region_kms_key_ids defined,
-			// the regions in ami_regions are also in region_kms_key_ids
-			if len(c.AMIRegionKMSKeyIDs) > 0 {
-				if _, ok := c.AMIRegionKMSKeyIDs[region]; !ok {
-					errs = append(errs, fmt.Errorf("Region %s is in ami_regions but not in region_kms_key_ids", region))
-				}
-			}
-			if (accessConfig != nil) && (region == accessConfig.RawRegion) {
-				// make sure we don't try to copy to the region we originally
-				// create the AMI in.
-				log.Printf("Cannot copy AMI to AWS session region '%s', deleting it from `ami_regions`.", region)
-				continue
-			}
-			regions = append(regions, region)
-		}
-
-		c.AMIRegions = regions
-	}
 
 	if len(c.AMIUsers) > 0 && c.AMIEncryptBootVolume {
 		errs = append(errs, fmt.Errorf("Cannot share AMI with encrypted boot volume"))
@@ -130,3 +95,45 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context
 
 	return nil
 }
+
+func (c *AMIConfig) prepareRegions(ec2conn ec2iface.EC2API, accessConfig *AccessConfig, errs []error) []error {
+	if len(c.AMIRegions) > 0 {
+		regionSet := make(map[string]struct{})
+		regions := make([]string, 0, len(c.AMIRegions))
+
+		for _, region := range c.AMIRegions {
+			// If we already saw the region, then don't look again
+			if _, ok := regionSet[region]; ok {
+				continue
+			}
+
+			// Mark that we saw the region
+			regionSet[region] = struct{}{}
+
+			if !c.AMISkipRegionValidation {
+				// Verify the region is real
+				if valid := ValidateRegion(region, ec2conn); !valid {
+					errs = append(errs, fmt.Errorf("Unknown region: %s", region))
+				}
+			}
+
+			// Make sure that if we have region_kms_key_ids defined,
+			// the regions in ami_regions are also in region_kms_key_ids
+			if len(c.AMIRegionKMSKeyIDs) > 0 {
+				if _, ok := c.AMIRegionKMSKeyIDs[region]; !ok {
+					errs = append(errs, fmt.Errorf("Region %s is in ami_regions but not in region_kms_key_ids", region))
+				}
+			}
+			if (accessConfig != nil) && (region == accessConfig.RawRegion) {
+				// make sure we don't try to copy to the region we originally
+				// create the AMI in.
+				log.Printf("Cannot copy AMI to AWS session region '%s', deleting it from `ami_regions`.", region)
+				continue
+			}
+			regions = append(regions, region)
+		}
+
+		c.AMIRegions = regions
+	}
+	return errs
+}

builder/amazon/common/ami_config_test.go

@@ -1,8 +1,13 @@
 package common
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 )
 
 func testAMIConfig() *AMIConfig {
@@ -19,6 +24,7 @@ func getFakeAccessConfig(region string) *AccessConfig {
 
 func TestAMIConfigPrepare_name(t *testing.T) {
 	c := testAMIConfig()
+	c.AMISkipRegionValidation = true
 	if err := c.Prepare(nil, nil); err != nil {
 		t.Fatalf("shouldn't have err: %s", err)
 	}
@@ -29,26 +35,46 @@ func TestAMIConfigPrepare_name(t *testing.T) {
 	}
 }
 
+type mockEC2Client struct {
+	ec2iface.EC2API
+}
+
+func (m *mockEC2Client) DescribeRegions(*ec2.DescribeRegionsInput) (*ec2.DescribeRegionsOutput, error) {
+	return &ec2.DescribeRegionsOutput{
+		Regions: []*ec2.Region{
+			{RegionName: aws.String("us-east-1")},
+			{RegionName: aws.String("us-east-2")},
+			{RegionName: aws.String("us-west-1")},
+		},
+	}, nil
+}
+
 func TestAMIConfigPrepare_regions(t *testing.T) {
 	c := testAMIConfig()
 	c.AMIRegions = nil
-	if err := c.Prepare(nil, nil); err != nil {
+	c.AMISkipRegionValidation = true
-		t.Fatalf("shouldn't have err: %s", err)
+
+	var errs []error
+	mockConn := &mockEC2Client{}
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
+		t.Fatalf("shouldn't have err: %#v", errs)
 	}
 
-	c.AMIRegions = listEC2Regions()
+	c.AMISkipRegionValidation = false
-	if err := c.Prepare(nil, nil); err != nil {
+	c.AMIRegions = listEC2Regions(mockConn)
-		t.Fatalf("shouldn't have err: %s", err)
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
+		t.Fatalf("shouldn't have err: %#v", errs)
 	}
 
 	c.AMIRegions = []string{"foo"}
-	if err := c.Prepare(nil, nil); err == nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) == 0 {
 		t.Fatal("should have error")
 	}
+	errs = errs[:0]
 
 	c.AMIRegions = []string{"us-east-1", "us-west-1", "us-east-1"}
-	if err := c.Prepare(nil, nil); err != nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
-		t.Fatalf("bad: %s", err)
+		t.Fatalf("bad: %s", errs[0])
 	}
 
 	expected := []string{"us-east-1", "us-west-1"}
@@ -58,7 +84,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 
 	c.AMIRegions = []string{"custom"}
 	c.AMISkipRegionValidation = true
-	if err := c.Prepare(nil, nil); err != nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
 		t.Fatal("shouldn't have error")
 	}
 	c.AMISkipRegionValidation = false
@@ -69,8 +95,8 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 		"us-west-1": "789-012-3456",
 		"us-east-2": "456-789-0123",
 	}
-	if err := c.Prepare(nil, nil); err != nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
-		t.Fatal("shouldn't have error")
+		t.Fatal(fmt.Sprintf("shouldn't have error: %s", errs[0]))
 	}
 
 	c.AMIRegions = []string{"us-east-1", "us-east-2", "us-west-1"}
@@ -79,7 +105,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 		"us-west-1": "789-012-3456",
 		"us-east-2": "",
 	}
-	if err := c.Prepare(nil, nil); err != nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
 		t.Fatal("should have passed; we are able to use default KMS key if not sharing")
 	}
 
@@ -90,7 +116,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 		"us-west-1": "789-012-3456",
 		"us-east-2": "",
 	}
-	if err := c.Prepare(nil, nil); err == nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
 		t.Fatal("should have an error b/c can't use default KMS key if sharing")
 	}
 
@@ -100,7 +126,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 		"us-west-1": "789-012-3456",
 		"us-east-2": "456-789-0123",
 	}
-	if err := c.Prepare(nil, nil); err == nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
 		t.Fatal("should have error b/c theres a region in the key map that isn't in ami_regions")
 	}
 
@@ -109,9 +135,12 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 		"us-east-1": "123-456-7890",
 		"us-west-1": "789-012-3456",
 	}
+
+	c.AMISkipRegionValidation = true
 	if err := c.Prepare(nil, nil); err == nil {
 		t.Fatal("should have error b/c theres a region in in ami_regions that isn't in the key map")
 	}
+	c.AMISkipRegionValidation = false
 
 	c.SnapshotUsers = []string{"foo", "bar"}
 	c.AMIKmsKeyId = "123-abc-456"
@@ -121,7 +150,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 		"us-east-1": "123-456-7890",
 		"us-west-1": "",
 	}
-	if err := c.Prepare(nil, nil); err == nil {
+	if errs = c.prepareRegions(mockConn, nil, errs); len(errs) > 0 {
 		t.Fatal("should have error b/c theres a region in in ami_regions that isn't in the key map")
 	}
 
@@ -129,7 +158,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 	accessConf := getFakeAccessConfig("us-east-1")
 	c.AMIRegions = []string{"us-east-1", "us-west-1", "us-east-2"}
 	c.AMIRegionKMSKeyIDs = nil
-	if err := c.Prepare(accessConf, nil); err != nil {
+	if errs = c.prepareRegions(mockConn, accessConf, errs); len(errs) > 0 {
 		t.Fatal("should allow user to have the raw region in ami_regions")
 	}
 

builder/amazon/common/regions.go

@@ -1,35 +1,34 @@
 package common
 
-// aws ec2 describe-regions --query 'Regions[].{Name:RegionName}' --output text | sed 's/\(.*\)/"\1",/' | sort
+import (
-func listEC2Regions() []string {
+	"github.com/aws/aws-sdk-go/aws/session"
-	return []string{
+	"github.com/aws/aws-sdk-go/service/ec2"
-		"ap-northeast-1",
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
-		"ap-northeast-2",
+)
-		"ap-northeast-3",
+
-		"ap-south-1",
+func getValidationSession() *ec2.EC2 {
-		"ap-southeast-1",
+	sess := session.Must(session.NewSessionWithOptions(session.Options{
-		"ap-southeast-2",
+		SharedConfigState: session.SharedConfigEnable,
-		"ca-central-1",
+	}))
-		"eu-central-1",
+
-		"eu-west-1",
+	ec2conn := ec2.New(sess)
-		"eu-west-2",
+	return ec2conn
-		"eu-west-3",
+}
-		"sa-east-1",
+
-		"us-east-1",
+func listEC2Regions(ec2conn ec2iface.EC2API) []string {
-		"us-east-2",
+	var regions []string
-		"us-west-1",
+	resultRegions, _ := ec2conn.DescribeRegions(nil)
-		"us-west-2",
+	for _, region := range resultRegions.Regions {
-		// not part of autogenerated list
+		regions = append(regions, *region.RegionName)
-		"us-gov-west-1",
-		"cn-north-1",
-		"cn-northwest-1",
 	}
+
+	return regions
 }
 
 // ValidateRegion returns true if the supplied region is a valid AWS
 // region and false if it's not.
-func ValidateRegion(region string) bool {
+func ValidateRegion(region string, ec2conn ec2iface.EC2API) bool {
-	for _, valid := range listEC2Regions() {
+	for _, valid := range listEC2Regions(ec2conn) {
 		if region == valid {
 			return true
 		}

builder/amazon/ebs/builder_test.go

@@ -47,6 +47,7 @@ func TestBuilderPrepare_AMIName(t *testing.T) {
 
 	// Test good
 	config["ami_name"] = "foo"
+	config["skip_region_validation"] = true
 	warnings, err := b.Prepare(config)
 	if len(warnings) > 0 {
 		t.Fatalf("bad: %#v", warnings)
@@ -99,6 +100,7 @@ func TestBuilderPrepare_InvalidShutdownBehavior(t *testing.T) {
 
 	// Test good
 	config["shutdown_behavior"] = "terminate"
+	config["skip_region_validation"] = true
 	warnings, err := b.Prepare(config)
 	if len(warnings) > 0 {
 		t.Fatalf("bad: %#v", warnings)

builder/amazon/ebsvolume/builder_test.go

@@ -61,6 +61,7 @@ func TestBuilderPrepare_InvalidShutdownBehavior(t *testing.T) {
 
 	// Test good
 	config["shutdown_behavior"] = "terminate"
+	config["skip_region_validation"] = true
 	warnings, err := b.Prepare(config)
 	if len(warnings) > 0 {
 		t.Fatalf("bad: %#v", warnings)

builder/amazon/instance/builder_test.go

@@ -41,6 +41,8 @@ func TestBuilder_ImplementsBuilder(t *testing.T) {
 func TestBuilderPrepare_AccountId(t *testing.T) {
 	b := &Builder{}
 	config, tempfile := testConfig()
+	config["skip_region_validation"] = true
+
 	defer os.Remove(tempfile.Name())
 	defer tempfile.Close()
 
@@ -84,6 +86,7 @@ func TestBuilderPrepare_AMIName(t *testing.T) {
 
 	// Test good
 	config["ami_name"] = "foo"
+	config["skip_region_validation"] = true
 	warnings, err := b.Prepare(config)
 	if len(warnings) > 0 {
 		t.Fatalf("bad: %#v", warnings)
@@ -118,6 +121,7 @@ func TestBuilderPrepare_AMIName(t *testing.T) {
 func TestBuilderPrepare_BundleDestination(t *testing.T) {
 	b := &Builder{}
 	config, tempfile := testConfig()
+	config["skip_region_validation"] = true
 	defer os.Remove(tempfile.Name())
 	defer tempfile.Close()
 
@@ -138,6 +142,7 @@ func TestBuilderPrepare_BundleDestination(t *testing.T) {
 func TestBuilderPrepare_BundlePrefix(t *testing.T) {
 	b := &Builder{}
 	config, tempfile := testConfig()
+	config["skip_region_validation"] = true
 	defer os.Remove(tempfile.Name())
 	defer tempfile.Close()
 
@@ -174,6 +179,7 @@ func TestBuilderPrepare_InvalidKey(t *testing.T) {
 func TestBuilderPrepare_S3Bucket(t *testing.T) {
 	b := &Builder{}
 	config, tempfile := testConfig()
+	config["skip_region_validation"] = true
 	defer os.Remove(tempfile.Name())
 	defer tempfile.Close()
 
@@ -199,6 +205,7 @@ func TestBuilderPrepare_S3Bucket(t *testing.T) {
 func TestBuilderPrepare_X509CertPath(t *testing.T) {
 	b := &Builder{}
 	config, tempfile := testConfig()
+	config["skip_region_validation"] = true
 	defer os.Remove(tempfile.Name())
 	defer tempfile.Close()
 
@@ -240,6 +247,7 @@ func TestBuilderPrepare_X509CertPath(t *testing.T) {
 func TestBuilderPrepare_X509KeyPath(t *testing.T) {
 	b := &Builder{}
 	config, tempfile := testConfig()
+	config["skip_region_validation"] = true
 	defer os.Remove(tempfile.Name())
 	defer tempfile.Close()
 
@@ -281,6 +289,7 @@ func TestBuilderPrepare_X509KeyPath(t *testing.T) {
 func TestBuilderPrepare_X509UploadPath(t *testing.T) {
 	b := &Builder{}
 	config, tempfile := testConfig()
+	config["skip_region_validation"] = true
 	defer os.Remove(tempfile.Name())
 	defer tempfile.Close()
 

vendor/vendor.json

@@ -531,6 +531,14 @@
 			"version": "v1.12.72",
 			"versionExact": "v1.12.72"
 		},
+		{
+			"checksumSHA1": "u+wyWp/GWHHINKLeXP0eQmbfQ38=",
+			"path": "github.com/aws/aws-sdk-go/service/ec2/ec2iface",
+			"revision": "586c9ba6027a527800564282bb843d7e6e7985c9",
+			"revisionTime": "2018-02-07T00:16:19Z",
+			"version": "v1.12.72",
+			"versionExact": "v1.12.72"
+		},
 		{
 			"checksumSHA1": "kEgV0dSAj3M3M1waEkC27JS7VnU=",
 			"comment": "v1.7.1",
@@ -1005,12 +1013,6 @@
 			"revision": "0fb14efe8c47ae851c0034ed7a448854d3d34cf3",
 			"revisionTime": "2018-02-01T23:52:37Z"
 		},
-		{
-			"checksumSHA1": "0PeWsO2aI+2PgVYlYlDPKfzCLEQ=",
-			"path": "github.com/hashicorp/serf/coordinate",
-			"revision": "984a73625de3138f44deb38d00878fab39eb6447",
-			"revisionTime": "2018-05-30T15:59:58Z"
-		},
 		{
 			"checksumSHA1": "HtpYAWHvd9mq+mHkpo7z8PGzMik=",
 			"path": "github.com/hashicorp/hcl",
@@ -1065,6 +1067,12 @@
 			"revision": "ef8a98b0bbce4a65b5aa4c368430a80ddc533168",
 			"revisionTime": "2018-04-04T17:41:02Z"
 		},
+		{
+			"checksumSHA1": "0PeWsO2aI+2PgVYlYlDPKfzCLEQ=",
+			"path": "github.com/hashicorp/serf/coordinate",
+			"revision": "984a73625de3138f44deb38d00878fab39eb6447",
+			"revisionTime": "2018-05-30T15:59:58Z"
+		},
 		{
 			"checksumSHA1": "oi8fnfAPPTf2nN7zptlyu/EsMMs=",
 			"path": "github.com/hashicorp/vault/api",