From 7cf47fc463cd52a75d697878aeb1ebfa3b736d01 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 25 Oct 2019 16:09:20 -0700 Subject: [PATCH 1/2] add some extra layers of validation to make sure that people don't trip over magical encrypt_boot settings later in the build --- builder/amazon/common/ami_config.go | 17 ++++++++++++----- builder/amazon/common/step_ami_region_copy.go | 13 ++++++++++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/builder/amazon/common/ami_config.go b/builder/amazon/common/ami_config.go index 77d1418ee..74808cde1 100644 --- a/builder/amazon/common/ami_config.go +++ b/builder/amazon/common/ami_config.go @@ -170,17 +170,23 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context } } - var kmsKeys []string + kmsKeys := make([]string, 0) if len(c.AMIKmsKeyId) > 0 { kmsKeys = append(kmsKeys, c.AMIKmsKeyId) } if len(c.AMIRegionKMSKeyIDs) > 0 { for _, kmsKey := range c.AMIRegionKMSKeyIDs { - if len(kmsKey) == 0 { - kmsKeys = append(kmsKeys, c.AMIKmsKeyId) + if len(kmsKey) > 0 { + kmsKeys = append(kmsKeys, kmsKey) } } } + + if len(kmsKeys) > 0 && !c.AMIEncryptBootVolume.True() { + errs = append(errs, fmt.Errorf("If you have set either "+ + "region_kms_key_ids or kms_key_id, encrypt_boot must also be true.")) + + } for _, kmsKey := range kmsKeys { if !validateKmsKey(kmsKey) { errs = append(errs, fmt.Errorf("%s is not a valid KMS Key Id.", kmsKey)) @@ -188,8 +194,9 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context } if len(c.SnapshotUsers) > 0 { - if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume.True() { - errs = append(errs, fmt.Errorf("Cannot share snapshot encrypted with default KMS key")) + if len(c.AMIKmsKeyId) == 0 && len(c.AMIRegionKMSKeyIDs) == 0 && c.AMIEncryptBootVolume.True() { + errs = append(errs, fmt.Errorf("Cannot share snapshot encrypted "+ + "with default KMS key")) } if len(c.AMIRegionKMSKeyIDs) > 0 { for _, kmsKey := range c.AMIRegionKMSKeyIDs { diff --git a/builder/amazon/common/step_ami_region_copy.go b/builder/amazon/common/step_ami_region_copy.go index 323a9a0db..2ec108ff9 100644 --- a/builder/amazon/common/step_ami_region_copy.go +++ b/builder/amazon/common/step_ami_region_copy.go @@ -83,9 +83,16 @@ func (s *StepAMIRegionCopy) Run(ctx context.Context, state multistep.StateBag) m s.RegionKeyIds = make(map[string]string) } - // Make sure the kms_key_id for the original region is in the map - if _, ok := s.RegionKeyIds[s.OriginalRegion]; !ok { - s.RegionKeyIds[s.OriginalRegion] = s.AMIKmsKeyId + // Make sure the kms_key_id for the original region is in the map, as + // long as the AMIKmsKeyId isn't being defaulted. + if s.AMIKmsKeyId != "" { + if _, ok := s.RegionKeyIds[s.OriginalRegion]; !ok { + s.RegionKeyIds[s.OriginalRegion] = s.AMIKmsKeyId + } + } else { + if regionKey, ok := s.RegionKeyIds[s.OriginalRegion]; ok { + s.AMIKmsKeyId = regionKey + } } } From d084cd1895ab1c39272a435e6367c96abb9e9982 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 30 Oct 2019 09:34:42 -0700 Subject: [PATCH 2/2] Update builder/amazon/common/ami_config.go --- builder/amazon/common/ami_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/amazon/common/ami_config.go b/builder/amazon/common/ami_config.go index 74808cde1..28ddfdfa5 100644 --- a/builder/amazon/common/ami_config.go +++ b/builder/amazon/common/ami_config.go @@ -196,7 +196,7 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context if len(c.SnapshotUsers) > 0 { if len(c.AMIKmsKeyId) == 0 && len(c.AMIRegionKMSKeyIDs) == 0 && c.AMIEncryptBootVolume.True() { errs = append(errs, fmt.Errorf("Cannot share snapshot encrypted "+ - "with default KMS key")) + "with default KMS key, see https://www.packer.io/docs/builders/amazon-ebs.html#region_kms_key_ids for more information")) } if len(c.AMIRegionKMSKeyIDs) > 0 { for _, kmsKey := range c.AMIRegionKMSKeyIDs {