From 1944f38985d9b405432218f0ea29ffeefd455296 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 31 May 2017 13:41:32 -0700 Subject: [PATCH] update docs and clean up --- builder/amazon/common/ami_config.go | 32 +++++++++---------- builder/amazon/common/ami_config_test.go | 10 +++--- builder/amazon/common/step_ami_region_copy.go | 5 ++- builder/amazon/ebs/builder.go | 2 +- .../source/docs/builders/amazon-ebs.html.md | 13 +++++--- 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/builder/amazon/common/ami_config.go b/builder/amazon/common/ami_config.go index 6fd884412..e61701d54 100644 --- a/builder/amazon/common/ami_config.go +++ b/builder/amazon/common/ami_config.go @@ -22,14 +22,14 @@ type AMIConfig struct { AMIForceDeleteSnapshot bool `mapstructure:"force_delete_snapshot"` AMIEncryptBootVolume bool `mapstructure:"encrypt_boot"` AMIKmsKeyId string `mapstructure:"kms_key_id"` - AMIRegionKmsKeyIds map[string]string `mapstructure:"region_kms_key_ids"` + AMIRegionKMSKeyIDs map[string]string `mapstructure:"region_kms_key_ids"` SnapshotTags map[string]string `mapstructure:"snapshot_tags"` SnapshotUsers []string `mapstructure:"snapshot_users"` SnapshotGroups []string `mapstructure:"snapshot_groups"` } -func stringInSlice(searchstr string, searchslice []string) bool { - for _, item := range searchslice { +func stringInSlice(s []string, searchstr string) bool { + for _, item := range s { if item == searchstr { return true } @@ -64,13 +64,10 @@ func (c *AMIConfig) Prepare(ctx *interpolate.Context) []error { } } - // 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 { - regionsInKeyMap := make([]string, 0, len(c.AMIRegionKmsKeyIds)) - for reg := range c.AMIRegionKmsKeyIds { - regionsInKeyMap = append(regionsInKeyMap, reg) - } - if regionsMatch := stringInSlice(region, regionsInKeyMap); !regionsMatch { + // 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)) } } @@ -80,10 +77,11 @@ func (c *AMIConfig) Prepare(ctx *interpolate.Context) []error { c.AMIRegions = regions } - // Make sure that if we have region_kms_key_ids defined the regions in region_kms_key_ids are also in ami_regions - if len(c.AMIRegionKmsKeyIds) > 0 { - for kmsKeyRegion := range c.AMIRegionKmsKeyIds { - if regionsMatch := stringInSlice(kmsKeyRegion, c.AMIRegions); !regionsMatch { + // Make sure that if we have region_kms_key_ids defined, + // the regions in region_kms_key_ids are also in ami_regions + if len(c.AMIRegionKMSKeyIDs) > 0 { + for kmsKeyRegion := range c.AMIRegionKMSKeyIDs { + if !stringInSlice(c.AMIRegions, kmsKeyRegion) { errs = append(errs, fmt.Errorf("Region %s is in region_kms_key_ids but not in ami_regions", kmsKeyRegion)) } } @@ -97,9 +95,9 @@ func (c *AMIConfig) Prepare(ctx *interpolate.Context) []error { if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume { errs = append(errs, fmt.Errorf("Cannot share snapshot encrypted with default KMS key")) } - if len(c.AMIRegionKmsKeyIds) > 0 { - for _, kmsKeyRegion := range c.AMIRegionKmsKeyIds { - if len(kmsKeyRegion) == 0 { + if len(c.AMIRegionKMSKeyIDs) > 0 { + for _, kmsKey := range c.AMIRegionKMSKeyIDs { + if len(kmsKey) == 0 { errs = append(errs, fmt.Errorf("Cannot share snapshot encrypted with default KMS key")) } } diff --git a/builder/amazon/common/ami_config_test.go b/builder/amazon/common/ami_config_test.go index 5cdff587c..29371ddae 100644 --- a/builder/amazon/common/ami_config_test.go +++ b/builder/amazon/common/ami_config_test.go @@ -58,7 +58,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { c.AMISkipRegionValidation = false c.AMIRegions = []string{"us-east-1", "us-east-2", "us-west-1"} - c.AMIRegionKmsKeyIds = map[string]string{ + c.AMIRegionKMSKeyIDs = map[string]string{ "us-east-1": "123-456-7890", "us-west-1": "789-012-3456", "us-east-2": "456-789-0123", @@ -69,7 +69,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { c.SnapshotUsers = []string{"user-foo", "user-bar"} c.AMIRegions = []string{"us-east-1", "us-east-2", "us-west-1"} - c.AMIRegionKmsKeyIds = map[string]string{ + c.AMIRegionKMSKeyIDs = map[string]string{ "us-east-1": "123-456-7890", "us-west-1": "789-012-3456", "us-east-2": "", @@ -79,7 +79,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { } c.AMIRegions = []string{"us-east-1", "us-west-1"} - c.AMIRegionKmsKeyIds = map[string]string{ + c.AMIRegionKMSKeyIDs = map[string]string{ "us-east-1": "123-456-7890", "us-west-1": "789-012-3456", "us-east-2": "456-789-0123", @@ -89,7 +89,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { } c.AMIRegions = []string{"us-east-1", "us-west-1", "us-east-2"} - c.AMIRegionKmsKeyIds = map[string]string{ + c.AMIRegionKMSKeyIDs = map[string]string{ "us-east-1": "123-456-7890", "us-west-1": "789-012-3456", } @@ -101,7 +101,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { c.AMIKmsKeyId = "123-abc-456" c.AMIEncryptBootVolume = true c.AMIRegions = []string{"us-east-1", "us-west-1"} - c.AMIRegionKmsKeyIds = map[string]string{ + c.AMIRegionKMSKeyIDs = map[string]string{ "us-east-1": "123-456-7890", "us-west-1": "", } diff --git a/builder/amazon/common/step_ami_region_copy.go b/builder/amazon/common/step_ami_region_copy.go index 37fec8be2..1eb82653a 100644 --- a/builder/amazon/common/step_ami_region_copy.go +++ b/builder/amazon/common/step_ami_region_copy.go @@ -47,9 +47,8 @@ func (s *StepAMIRegionCopy) Run(state multistep.StateBag) multistep.StepAction { wg.Add(1) ui.Message(fmt.Sprintf("Copying to: %s", region)) - regKeyID = s.RegionKeyIds[region] - if !s.EncryptBootVolume { - regKeyID = "" + if s.EncryptBootVolume { + regKeyID = s.RegionKeyIds[region] } go func(region string) { diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index fb30ff535..9b3b00897 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -200,7 +200,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &awscommon.StepAMIRegionCopy{ AccessConfig: &b.config.AccessConfig, Regions: b.config.AMIRegions, - RegionKeyIds: b.config.AMIRegionKmsKeyIds, + RegionKeyIds: b.config.AMIRegionKMSKeyIDs, EncryptBootVolume: b.config.AMIEncryptBootVolume, Name: b.config.AMIName, }, diff --git a/website/source/docs/builders/amazon-ebs.html.md b/website/source/docs/builders/amazon-ebs.html.md index 9869bbb34..8ff595ee3 100644 --- a/website/source/docs/builders/amazon-ebs.html.md +++ b/website/source/docs/builders/amazon-ebs.html.md @@ -128,10 +128,6 @@ builder. Tags and attributes are copied along with the AMI. AMI copying takes time depending on the size of the AMI, but will generally take many minutes. -- `region_kms_key_ids` (map of strings) - a map of regions to copy the ami to, - along with the custom kms key id to use for encryption for that region. - Keys must match the regions provided in `ami_regions` - - `ami_users` (array of strings) - A list of account IDs that have access to launch the resulting AMI(s). By default no additional users other than the user creating the AMI has permissions to launch it. @@ -199,6 +195,15 @@ builder. preserved when booting from the AMI built with Packer. See `ami_block_device_mappings`, above, for details. +- `region_kms_key_ids` (map of strings) - a map of regions to copy the ami to, + along with the custom kms key id to use for encryption for that region. + Keys must match the regions provided in `ami_regions`. If you just want to + encrypt using a default ID, you can stick with `kms_key_id` and `ami_regions`. + If you want a region to be encrypted with that region's default key ID, you can + use an empty string `""` instead of a key id in this map. (e.g. `"us-east-1": ""`) + However, you cannot use default key IDs if you are using this in conjunction with + `snapshot_users` -- in that situation you must use custom keys. + - `run_tags` (object of key/value strings) - Tags to apply to the instance that is *launched* to create the AMI. These tags are *not* applied to the resulting AMI unless they're duplicated in `tags`. This is a