mode KmsKeyId to chroot block device, as it's only used there

builder/amazon/chroot/block_device.go

@@ -0,0 +1,77 @@
+//go:generate struct-markdown
+
+package chroot
+
+import (
+	"fmt"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	awscommon "github.com/hashicorp/packer/builder/amazon/common"
+	"github.com/hashicorp/packer/template/interpolate"
+)
+
+type BlockDevice struct {
+	awscommon.BlockDevice `mapstructure:",squash"`
+	// ID, alias or ARN of the KMS key to use for boot volume encryption. This
+	// only applies to the main region, other regions where the AMI will be
+	// copied will be encrypted by the default EBS KMS key. For valid formats
+	// see KmsKeyId in the [AWS API docs -
+	// CopyImage](https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CopyImage.html)
+	// This field is validated by Packer, when using an alias, you will have to
+	// prefix kms_key_id with alias/.
+	KmsKeyId string `mapstructure:"kms_key_id" required:"false"`
+}
+
+type BlockDevices []BlockDevice
+
+func (bds BlockDevices) BuildEC2BlockDeviceMappings() []*ec2.BlockDeviceMapping {
+	var blockDevices []*ec2.BlockDeviceMapping
+
+	for _, blockDevice := range bds {
+		blockDevices = append(blockDevices, blockDevice.BuildEC2BlockDeviceMapping())
+	}
+	return blockDevices
+}
+
+func (blockDevice BlockDevice) BuildEC2BlockDeviceMapping() *ec2.BlockDeviceMapping {
+	mapping := blockDevice.BlockDevice.BuildEC2BlockDeviceMapping()
+
+	if blockDevice.KmsKeyId != "" {
+		mapping.Ebs.KmsKeyId = aws.String(blockDevice.KmsKeyId)
+	}
+	return mapping
+}
+
+func (b *BlockDevice) Prepare(ctx *interpolate.Context) error {
+	if b.DeviceName == "" {
+		return fmt.Errorf("The `device_name` must be specified " +
+			"for every device in the block device mapping.")
+	}
+	// Warn that encrypted must be true when setting kms_key_id
+	if b.KmsKeyId != "" && b.Encrypted != nil && *b.Encrypted == false {
+		return fmt.Errorf("The device %v, must also have `encrypted: "+
+			"true` when setting a kms_key_id.", b.DeviceName)
+	}
+
+	return nil
+}
+
+func (bds BlockDevices) Prepare(ctx *interpolate.Context) (errs []error) {
+	for _, block := range bds {
+		if err := block.Prepare(ctx); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	return errs
+}
+
+func (b BlockDevices) GetOmissions() map[string]bool {
+	omitMap := make(map[string]bool)
+
+	for _, blockDevice := range b {
+		omitMap[blockDevice.DeviceName] = blockDevice.OmitFromArtifact
+	}
+
+	return omitMap
+}

builder/amazon/chroot/builder.go

@@ -1,9 +1,9 @@
 //go:generate struct-markdown
 
-// The chroot package is able to create an Amazon AMI without requiring
+// The chroot package is able to create an Amazon AMI without requiring the
-// the launch of a new instance for every build. It does this by attaching
+// launch of a new instance for every build. It does this by attaching and
-// and mounting the root volume of another AMI and chrooting into that
+// mounting the root volume of another AMI and chrooting into that directory.
-// directory. It then creates an AMI from that attached drive.
+// It then creates an AMI from that attached drive.
 package chroot
 
 import (
@@ -23,111 +23,114 @@ import (
 // The unique ID for this builder
 const BuilderId = "mitchellh.amazon.chroot"
 
-// Config is the configuration that is chained through the steps and
+// Config is the configuration that is chained through the steps and settable
-// settable from the template.
+// from the template.
 type Config struct {
 	common.PackerConfig    `mapstructure:",squash"`
-	AMIMappings            awscommon.BlockDevices `mapstructure:"ami_block_device_mappings" required:"false"`
-	LaunchMappings         awscommon.BlockDevices `mapstructure:"launch_block_device_mappings" required:"false"`
 	awscommon.AMIConfig    `mapstructure:",squash"`
 	awscommon.AccessConfig `mapstructure:",squash"`
-	// This is a list of devices to
+	// Add one or more block device mappings to the AMI. These will be attached
-	// mount into the chroot environment. This configuration parameter requires
+	// when booting a new instance from your AMI. To add a block device during
-	// some additional documentation which is in the Chroot
+	// the Packer build see launch_block_device_mappings below. Your options
-	// Mounts section. Please read that section for more
+	// here may vary depending on the type of VM you use. See the BlockDevices
+	// documentation for fields.
+	AMIMappings BlockDevices `mapstructure:"ami_block_device_mappings" required:"false"`
+	// Add one or more block device mappings to the AMI. These will be attached
+	// when booting a new instance from your AMI. To add a block device during
+	// the Packer build see launch_block_device_mappings below. Your options
+	// here may vary depending on the type of VM you use. See the BlockDevices
+	// documentation for fields.
+	LaunchMappings BlockDevices `mapstructure:"launch_block_device_mappings" required:"false"`
+	// This is a list of devices to mount into the chroot environment. This
+	// configuration parameter requires some additional documentation which is
+	// in the Chroot Mounts section. Please read that section for more
 	// information on how to use this.
 	ChrootMounts [][]string `mapstructure:"chroot_mounts" required:"false"`
-	// How to run shell commands. This defaults to
+	// How to run shell commands. This defaults to {{.Command}}. This may be
-	// {{.Command}}. This may be useful to set if you want to set environmental
+	// useful to set if you want to set environmental variables or perhaps run
-	// variables or perhaps run it with sudo or so on. This is a configuration
+	// it with sudo or so on. This is a configuration template where the
-	// template where the .Command variable is replaced with the command to be
+	// .Command variable is replaced with the command to be run. Defaults to
-	// run. Defaults to {{.Command}}.
+	// {{.Command}}.
 	CommandWrapper string `mapstructure:"command_wrapper" required:"false"`
-	// Paths to files on the running EC2
+	// Paths to files on the running EC2 instance that will be copied into the
-	// instance that will be copied into the chroot environment prior to
+	// chroot environment prior to provisioning. Defaults to /etc/resolv.conf
-	// provisioning. Defaults to /etc/resolv.conf so that DNS lookups work. Pass
+	// so that DNS lookups work. Pass an empty list to skip copying
-	// an empty list to skip copying /etc/resolv.conf. You may need to do this
+	// /etc/resolv.conf. You may need to do this if you're building an image
-	// if you're building an image that uses systemd.
+	// that uses systemd.
 	CopyFiles []string `mapstructure:"copy_files" required:"false"`
-	// The path to the device where the root volume of
+	// The path to the device where the root volume of the source AMI will be
-	// the source AMI will be attached. This defaults to "" (empty string), which
+	// attached. This defaults to "" (empty string), which forces Packer to
-	// forces Packer to find an open device automatically.
+	// find an open device automatically.
 	DevicePath string `mapstructure:"device_path" required:"false"`
-	// When we call the mount command (by default
+	// When we call the mount command (by default mount -o device dir), the
-	// mount -o device dir), the string provided in nvme_mount_path will
+	// string provided in nvme_mount_path will replace device in that command.
-	// replace device in that command. When this option is not set, device in
+	// When this option is not set, device in that command will be something
-	// that command will be something like /dev/sdf1, mirroring the attached
+	// like /dev/sdf1, mirroring the attached device name. This assumption
-	// device name. This assumption works for most instances but will fail with c5
+	// works for most instances but will fail with c5 and m5 instances. In
-	// and m5 instances. In order to use the chroot builder with c5 and m5
+	// order to use the chroot builder with c5 and m5 instances, you must
-	// instances, you must manually set nvme_device_path and device_path.
+	// manually set nvme_device_path and device_path.
 	NVMEDevicePath string `mapstructure:"nvme_device_path" required:"false"`
-	// Build a new volume instead of starting from an
+	// Build a new volume instead of starting from an existing AMI root volume
-	// existing AMI root volume snapshot. Default false. If true, source_ami
+	// snapshot. Default false. If true, source_ami is no longer used and the
-	// is no longer used and the following options become required:
+	// following options become required: ami_virtualization_type,
-	// ami_virtualization_type, pre_mount_commands and root_volume_size. The
+	// pre_mount_commands and root_volume_size. The below options are also
-	// below options are also required in this mode only:
+	// required in this mode only:
 	FromScratch bool `mapstructure:"from_scratch" required:"false"`
-	// Options to supply the mount command
+	// Options to supply the mount command when mounting devices. Each option
-	// when mounting devices. Each option will be prefixed with -o and supplied
+	// will be prefixed with -o and supplied to the mount command ran by
-	// to the mount command ran by Packer. Because this command is ran in a
+	// Packer. Because this command is ran in a shell, user discretion is
-	// shell, user discretion is advised. See this manual page for the mount
+	// advised. See this manual page for the mount command for valid file
-	// command for valid file
 	// system specific options.
 	MountOptions []string `mapstructure:"mount_options" required:"false"`
-	// The partition number containing the /
+	// The partition number containing the / partition. By default this is the
-	// partition. By default this is the first partition of the volume, (for
+	// first partition of the volume, (for example, xvda1) but you can
-	// example, xvda1) but you can designate the entire block device by setting
+	// designate the entire block device by setting "mount_partition": "0" in
-	// "mount_partition": "0" in your config, which will mount xvda instead.
+	// your config, which will mount xvda instead.
 	MountPartition string `mapstructure:"mount_partition" required:"false"`
-	// The path where the volume will be mounted. This is
+	// The path where the volume will be mounted. This is where the chroot
-	// where the chroot environment will be. This defaults to
+	// environment will be. This defaults to
 	// /mnt/packer-amazon-chroot-volumes/{{.Device}}. This is a configuration
 	// template where the .Device variable is replaced with the name of the
 	// device where the volume is attached.
 	MountPath string `mapstructure:"mount_path" required:"false"`
-	// As pre_mount_commands, but the
+	// As pre_mount_commands, but the commands are executed after mounting the
-	// commands are executed after mounting the root device and before the extra
+	// root device and before the extra mount and copy steps. The device and
-	// mount and copy steps. The device and mount path are provided by
+	// mount path are provided by {{.Device}} and {{.MountPath}}.
-	// {{.Device}} and {{.MountPath}}.
 	PostMountCommands []string `mapstructure:"post_mount_commands" required:"false"`
-	// A series of commands to execute
+	// A series of commands to execute after attaching the root volume and
-	// after attaching the root volume and before mounting the chroot. This is not
+	// before mounting the chroot. This is not required unless using
-	// required unless using from_scratch. If so, this should include any
+	// from_scratch. If so, this should include any partitioning and filesystem
-	// partitioning and filesystem creation commands. The path to the device is
+	// creation commands. The path to the device is provided by {{.Device}}.
-	// provided by {{.Device}}.
 	PreMountCommands []string `mapstructure:"pre_mount_commands" required:"false"`
 	// The root device name. For example, xvda.
 	RootDeviceName string `mapstructure:"root_device_name" required:"false"`
-	// The size of the root volume in GB for the
+	// The size of the root volume in GB for the chroot environment and the
-	// chroot environment and the resulting AMI. Default size is the snapshot size
+	// resulting AMI. Default size is the snapshot size of the source_ami
-	// of the source_ami unless from_scratch is true, in which case this
+	// unless from_scratch is true, in which case this field must be defined.
-	// field must be defined.
 	RootVolumeSize int64 `mapstructure:"root_volume_size" required:"false"`
-	// The type of EBS volume for the chroot
+	// The type of EBS volume for the chroot environment and resulting AMI. The
-	// environment and resulting AMI. The default value is the type of the
+	// default value is the type of the source_ami, unless from_scratch is
-	// source_ami, unless from_scratch is true, in which case the default
+	// true, in which case the default value is gp2. You can only specify io1
-	// value is gp2. You can only specify io1 if building based on top of a
+	// if building based on top of a source_ami which is also io1.
-	// source_ami which is also io1.
 	RootVolumeType string `mapstructure:"root_volume_type" required:"false"`
-	// The source AMI whose root volume will be copied and
+	// The source AMI whose root volume will be copied and provisioned on the
-	// provisioned on the currently running instance. This must be an EBS-backed
+	// currently running instance. This must be an EBS-backed AMI with a root
-	// AMI with a root volume snapshot that you have access to. Note: this is not
+	// volume snapshot that you have access to. Note: this is not used when
-	// used when from_scratch is set to true.
+	// from_scratch is set to true.
 	SourceAmi string `mapstructure:"source_ami" required:"true"`
-	// Filters used to populate the source_ami
+	// Filters used to populate the source_ami field. Example:
-	// field. Example:
 	//
 	//
 	//     ``` json
 	//     {
 	//       "source_ami_filter": {
-	//     	 "filters": {
+	//       "filters": {
-	//     	  "virtualization-type": "hvm",
+	//        "virtualization-type": "hvm",
-	//     	  "name": "ubuntu/images/*ubuntu-xenial-16.04-amd64-server-*",
+	//        "name": "ubuntu/images/*ubuntu-xenial-16.04-amd64-server-*",
-	//     	  "root-device-type": "ebs"
+	//        "root-device-type": "ebs"
-	//     	},
+	//      },
-	//     	"owners": ["099720109477"],
+	//      "owners": ["099720109477"],
-	//     	"most_recent": true
+	//      "most_recent": true
 	//       }
 	//     }
 	//     ```
@@ -157,13 +160,12 @@ type Config struct {
 	//     criteria provided in `source_ami_filter`; this pins the AMI returned by the
 	//     filter, but will cause Packer to fail if the `source_ami` does not exist.
 	SourceAmiFilter awscommon.AmiFilterOptions `mapstructure:"source_ami_filter" required:"false"`
-	// Tags to apply to the
+	// Tags to apply to the volumes that are *launched*. This is a [template
-	// volumes that are *launched*. This is a [template
 	// engine](/docs/templates/engine.html), see [Build template
 	// data](#build-template-data) for more information.
 	RootVolumeTags awscommon.TagMap `mapstructure:"root_volume_tags" required:"false"`
-	// what architecture to use when registering the
+	// what architecture to use when registering the final AMI; valid options
-	// final AMI; valid options are "x86_64" or "arm64". Defaults to "x86_64".
+	// are "x86_64" or "arm64". Defaults to "x86_64".
 	Architecture string `mapstructure:"ami_architecture" required:"false"`
 
 	ctx interpolate.Context
@@ -289,8 +291,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 		}
 		if len(b.config.AMIMappings) > 0 && b.config.RootDeviceName != "" {
 			if b.config.RootVolumeSize == 0 {
-				// Although, they can specify the device size in the block device mapping, it's easier to
+				// Although, they can specify the device size in the block
-				// be specific here.
+				// device mapping, it's easier to be specific here.
 				errs = packer.MultiErrorAppend(
 					errs, errors.New("root_volume_size is required if ami_block_device_mappings is specified"))
 			}

builder/amazon/chroot/step_register_ami_test.go

@@ -92,10 +92,10 @@ func TestStepRegisterAmi_buildRegisterOptsFromScratch(t *testing.T) {
 	config := Config{
 		FromScratch:  true,
 		PackerConfig: common.PackerConfig{},
-		AMIMappings: []amazon.BlockDevice{
+		AMIMappings: []BlockDevice{
-			{
+			{BlockDevice: amazon.BlockDevice{
 				DeviceName: rootDeviceName,
-			},
+			}},
 		},
 		RootDeviceName: rootDeviceName,
 	}
@@ -167,10 +167,10 @@ func TestStepRegisterAmi_buildRegisterOptFromExistingImageWithBlockDeviceMapping
 	config := Config{
 		FromScratch:  false,
 		PackerConfig: common.PackerConfig{},
-		AMIMappings: []amazon.BlockDevice{
+		AMIMappings: []BlockDevice{
-			{
+			{BlockDevice: amazon.BlockDevice{
 				DeviceName: rootDeviceName,
-			},
+			}},
 		},
 		RootDeviceName: rootDeviceName,
 	}

builder/amazon/common/block_device.go

@@ -44,14 +44,6 @@ type BlockDevice struct {
 	// The size of the volume, in GiB. Required if not specifying a
 	// snapshot_id.
 	VolumeSize int64 `mapstructure:"volume_size" required:"false"`
-	// ID, alias or ARN of the KMS key to use for boot volume encryption. This
-	// only applies to the main region, other regions where the AMI will be
-	// copied will be encrypted by the default EBS KMS key. For valid formats
-	// see KmsKeyId in the [AWS API docs -
-	// CopyImage](https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CopyImage.html)
-	// This field is validated by Packer, when using an alias, you will have to
-	// prefix kms_key_id with alias/.
-	KmsKeyId string `mapstructure:"kms_key_id" required:"false"`
 	// ebssurrogate only
 	OmitFromArtifact bool `mapstructure:"omit_from_artifact"`
 }
@@ -106,10 +98,6 @@ func (blockDevice BlockDevice) BuildEC2BlockDeviceMapping() *ec2.BlockDeviceMapp
 	}
 	ebsBlockDevice.Encrypted = blockDevice.Encrypted
 
-	if blockDevice.KmsKeyId != "" {
-		ebsBlockDevice.KmsKeyId = aws.String(blockDevice.KmsKeyId)
-	}
-
 	mapping.Ebs = ebsBlockDevice
 
 	return mapping
@@ -120,11 +108,6 @@ func (b *BlockDevice) Prepare(ctx *interpolate.Context) error {
 		return fmt.Errorf("The `device_name` must be specified " +
 			"for every device in the block device mapping.")
 	}
-	// Warn that encrypted must be true when setting kms_key_id
-	if b.KmsKeyId != "" && b.Encrypted != nil && *b.Encrypted == false {
-		return fmt.Errorf("The device %v, must also have `encrypted: "+
-			"true` when setting a kms_key_id.", b.DeviceName)
-	}
 
 	return nil
 }

builder/amazon/common/block_device_test.go

@@ -91,7 +91,6 @@ func TestBlockDevice(t *testing.T) {
 				VolumeSize:          8,
 				DeleteOnTermination: true,
 				Encrypted:           aws.Bool(true),
-				KmsKeyId:            "2Fa48a521f-3aff-4b34-a159-376ac5d37812",
 			},
 
 			Result: &ec2.BlockDeviceMapping{
@@ -101,7 +100,6 @@ func TestBlockDevice(t *testing.T) {
 					VolumeSize:          aws.Int64(8),
 					DeleteOnTermination: aws.Bool(true),
 					Encrypted:           aws.Bool(true),
-					KmsKeyId:            aws.String("2Fa48a521f-3aff-4b34-a159-376ac5d37812"),
 				},
 			},
 		},