diff --git a/builder/amazon/common/access_config.go b/builder/amazon/common/access_config.go index 59ef90c2b..bf295e535 100644 --- a/builder/amazon/common/access_config.go +++ b/builder/amazon/common/access_config.go @@ -80,8 +80,7 @@ func (c *AccessConfig) Session() (*session.Session, error) { return c.session, nil } -// Region returns the aws.Region object for access to AWS services, requesting -// the region from the instance metadata if possible. +// region returns either the region from config or region from metadata service func (c *AccessConfig) region() (string, error) { if c.RawRegion != "" { if !c.SkipValidation { diff --git a/builder/amazon/common/ami_config.go b/builder/amazon/common/ami_config.go index 09b25479a..7dfe1af88 100644 --- a/builder/amazon/common/ami_config.go +++ b/builder/amazon/common/ami_config.go @@ -41,20 +41,21 @@ func stringInSlice(s []string, searchstr string) bool { func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context) []error { var errs []error - session, err := accessConfig.Session() - if err != nil { - errs = append(errs, err) + if accessConfig != nil { + session, err := accessConfig.Session() + if err != nil { + errs = append(errs, err) + } + region := *session.Config.Region + if stringInSlice(c.AMIRegions, region) { + errs = append(errs, fmt.Errorf("Cannot copy AMI to AWS session region '%s', please remove it from `ami_regions`.", region)) + } } - region := *session.Config.Region if c.AMIName == "" { errs = append(errs, fmt.Errorf("ami_name must be specified")) } - if stringInSlice(c.AMIRegions, region) { - errs = append(errs, fmt.Errorf("Cannot copy AMI to AWS session region '%s', please remove it from `ami_regions`.", region)) - } - if len(c.AMIRegions) > 0 { regionSet := make(map[string]struct{}) regions := make([]string, 0, len(c.AMIRegions)) diff --git a/builder/amazon/common/ami_config_test.go b/builder/amazon/common/ami_config_test.go index faa67c56d..5f130130c 100644 --- a/builder/amazon/common/ami_config_test.go +++ b/builder/amazon/common/ami_config_test.go @@ -13,12 +13,12 @@ func testAMIConfig() *AMIConfig { func TestAMIConfigPrepare_name(t *testing.T) { c := testAMIConfig() - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatalf("shouldn't have err: %s", err) } c.AMIName = "" - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("should have error") } } @@ -26,22 +26,22 @@ func TestAMIConfigPrepare_name(t *testing.T) { func TestAMIConfigPrepare_regions(t *testing.T) { c := testAMIConfig() c.AMIRegions = nil - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatalf("shouldn't have err: %s", err) } c.AMIRegions = listEC2Regions() - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatalf("shouldn't have err: %s", err) } c.AMIRegions = []string{"foo"} - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("should have error") } c.AMIRegions = []string{"us-east-1", "us-west-1", "us-east-1"} - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatalf("bad: %s", err) } @@ -52,7 +52,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { c.AMIRegions = []string{"custom"} c.AMISkipRegionValidation = true - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatal("shouldn't have error") } c.AMISkipRegionValidation = false @@ -63,7 +63,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { "us-west-1": "789-012-3456", "us-east-2": "456-789-0123", } - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatal("shouldn't have error") } @@ -73,7 +73,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { "us-west-1": "789-012-3456", "us-east-2": "", } - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatal("should have passed; we are able to use default KMS key if not sharing") } @@ -84,7 +84,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { "us-west-1": "789-012-3456", "us-east-2": "", } - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("should have an error b/c can't use default KMS key if sharing") } @@ -94,7 +94,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { "us-west-1": "789-012-3456", "us-east-2": "456-789-0123", } - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("should have error b/c theres a region in the key map that isn't in ami_regions") } @@ -103,7 +103,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { "us-east-1": "123-456-7890", "us-west-1": "789-012-3456", } - if err := c.Prepare(nil); err == nil { + 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") } @@ -115,7 +115,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { "us-east-1": "123-456-7890", "us-west-1": "", } - if err := c.Prepare(nil); err == nil { + 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") } } @@ -126,12 +126,12 @@ func TestAMIConfigPrepare_Share_EncryptedBoot(t *testing.T) { c.AMIEncryptBootVolume = true c.AMIKmsKeyId = "" - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("shouldn't be able to share ami with encrypted boot volume") } c.AMIKmsKeyId = "89c3fb9a-de87-4f2a-aedc-fddc5138193c" - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("shouldn't be able to share ami with encrypted boot volume") } } @@ -140,7 +140,7 @@ func TestAMINameValidation(t *testing.T) { c := testAMIConfig() c.AMIName = "aa" - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("shouldn't be able to have an ami name with less than 3 characters") } @@ -149,22 +149,22 @@ func TestAMINameValidation(t *testing.T) { longAmiName += "a" } c.AMIName = longAmiName - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("shouldn't be able to have an ami name with great than 128 characters") } c.AMIName = "+aaa" - if err := c.Prepare(nil); err == nil { + if err := c.Prepare(nil, nil); err == nil { t.Fatal("shouldn't be able to have an ami name with invalid characters") } c.AMIName = "fooBAR1()[] ./-'@_" - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatal("should be able to use all of the allowed AMI characters") } c.AMIName = `xyz-base-2017-04-05-1934` - if err := c.Prepare(nil); err != nil { + if err := c.Prepare(nil, nil); err != nil { t.Fatalf("expected `xyz-base-2017-04-05-1934` to pass validation.") }