Skip to content

Commit

Permalink
aws_ami: Add root_snapshot_id attribute (#1572)
Browse files Browse the repository at this point in the history
Current limitations in HCL prevent it from being possible to reference
an AMI's root snapshot ID via its ID. This is because the snapshot ID is
deeply nested in the `block_device_mappings` set. HCL is not
yet flexible enough to allow a lookup expression to extract the snapshot
ID embedded within that structure.

This makes it impossible to use resources like
`aws_snapshot_create_volume_permission` without manually doing the AMI
ID -> Snapshot ID translation outside of Terraform.

Here, we promote the Snapshot ID of the root volume to a top-level
`root_snapshot_id` attribute for AMI data sources and resources.

This field will be blank for instance store AMIs or for weird conditions
where there's no snapshot ID recorded (See commit message here:
bed5f62), but it enables usage of the
resource referenced above for the majority of scenarios.
  • Loading branch information
phinze authored and radeksimko committed Oct 10, 2017
1 parent babe606 commit 493a305
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 10 deletions.
21 changes: 21 additions & 0 deletions aws/data_source_aws_ami.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ func dataSourceAwsAmi() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"root_snapshot_id": {
Type: schema.TypeString,
Computed: true,
},
"sriov_net_support": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -284,6 +288,7 @@ func amiDescriptionAttributes(d *schema.ResourceData, image *ec2.Image) error {
d.Set("root_device_name", image.RootDeviceName)
}
d.Set("root_device_type", image.RootDeviceType)
d.Set("root_snapshot_id", amiRootSnapshotId(image))
if image.SriovNetSupport != nil {
d.Set("sriov_net_support", image.SriovNetSupport)
}
Expand Down Expand Up @@ -358,6 +363,22 @@ func amiProductCodes(m []*ec2.ProductCode) *schema.Set {
return s
}

// Returns the root snapshot ID for an image, if it has one
func amiRootSnapshotId(image *ec2.Image) string {
if image.RootDeviceName == nil {
return ""
}
for _, bdm := range image.BlockDeviceMappings {
if bdm.DeviceName == nil || *bdm.DeviceName != *image.RootDeviceName {
continue
}
if bdm.Ebs != nil && bdm.Ebs.SnapshotId != nil {
return *bdm.Ebs.SnapshotId
}
}
return ""
}

// Returns the state reason.
func amiStateReason(m *ec2.StateReason) map[string]interface{} {
s := make(map[string]interface{})
Expand Down
3 changes: 3 additions & 0 deletions aws/data_source_aws_ami_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestAccAWSAmiDataSource_natInstance(t *testing.T) {
resource.TestCheckResourceAttr("data.aws_ami.nat_ami", "product_codes.#", "0"),
resource.TestCheckResourceAttr("data.aws_ami.nat_ami", "root_device_name", "/dev/xvda"),
resource.TestCheckResourceAttr("data.aws_ami.nat_ami", "root_device_type", "ebs"),
resource.TestMatchResourceAttr("data.aws_ami.nat_ami", "root_snapshot_id", regexp.MustCompile("^snap-")),
resource.TestCheckResourceAttr("data.aws_ami.nat_ami", "sriov_net_support", "simple"),
resource.TestCheckResourceAttr("data.aws_ami.nat_ami", "state", "available"),
resource.TestCheckResourceAttr("data.aws_ami.nat_ami", "state_reason.code", "UNSET"),
Expand Down Expand Up @@ -78,6 +79,7 @@ func TestAccAWSAmiDataSource_windowsInstance(t *testing.T) {
resource.TestCheckResourceAttr("data.aws_ami.windows_ami", "product_codes.#", "0"),
resource.TestCheckResourceAttr("data.aws_ami.windows_ami", "root_device_name", "/dev/sda1"),
resource.TestCheckResourceAttr("data.aws_ami.windows_ami", "root_device_type", "ebs"),
resource.TestMatchResourceAttr("data.aws_ami.windows_ami", "root_snapshot_id", regexp.MustCompile("^snap-")),
resource.TestCheckResourceAttr("data.aws_ami.windows_ami", "sriov_net_support", "simple"),
resource.TestCheckResourceAttr("data.aws_ami.windows_ami", "state", "available"),
resource.TestCheckResourceAttr("data.aws_ami.windows_ami", "state_reason.code", "UNSET"),
Expand Down Expand Up @@ -112,6 +114,7 @@ func TestAccAWSAmiDataSource_instanceStore(t *testing.T) {
resource.TestCheckResourceAttr("data.aws_ami.instance_store_ami", "public", "true"),
resource.TestCheckResourceAttr("data.aws_ami.instance_store_ami", "product_codes.#", "0"),
resource.TestCheckResourceAttr("data.aws_ami.instance_store_ami", "root_device_type", "instance-store"),
resource.TestCheckResourceAttr("data.aws_ami.instance_store_ami", "root_snapshot_id", ""),
resource.TestCheckResourceAttr("data.aws_ami.instance_store_ami", "sriov_net_support", "simple"),
resource.TestCheckResourceAttr("data.aws_ami.instance_store_ami", "state", "available"),
resource.TestCheckResourceAttr("data.aws_ami.instance_store_ami", "state_reason.code", "UNSET"),
Expand Down
5 changes: 5 additions & 0 deletions aws/resource_aws_ami.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ func resourceAwsAmiRead(d *schema.ResourceData, meta interface{}) error {
d.Set("kernel_id", image.KernelId)
d.Set("ramdisk_id", image.RamdiskId)
d.Set("root_device_name", image.RootDeviceName)
d.Set("root_snapshot_id", amiRootSnapshotId(image))
d.Set("sriov_net_support", image.SriovNetSupport)
d.Set("virtualization_type", image.VirtualizationType)

Expand Down Expand Up @@ -441,6 +442,10 @@ func resourceAwsAmiCommonSchema(computed bool) map[string]*schema.Schema {
Computed: computed,
ForceNew: !computed,
},
"root_snapshot_id": {
Type: schema.TypeString,
Computed: true,
},
"sriov_net_support": {
Type: schema.TypeString,
Optional: !computed,
Expand Down
27 changes: 17 additions & 10 deletions aws/resource_aws_ami_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package aws
import (
"fmt"
"log"
"regexp"
"testing"
"time"

Expand All @@ -29,6 +30,8 @@ func TestAccAWSAMI_basic(t *testing.T) {
testAccCheckAmiExists("aws_ami.foo", &ami),
resource.TestCheckResourceAttr(
"aws_ami.foo", "name", fmt.Sprintf("tf-testing-%d", rInt)),
resource.TestMatchResourceAttr(
"aws_ami.foo", "root_snapshot_id", regexp.MustCompile("^snap-")),
),
},
},
Expand Down Expand Up @@ -202,12 +205,14 @@ func testAccCheckAmiEbsBlockDevice(bd *ec2.BlockDeviceMapping, ed *ec2.EbsBlockD

func testAccAmiConfig_basic(rInt int) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {}
resource "aws_ebs_volume" "foo" {
availability_zone = "us-west-2a"
size = 8
tags {
Name = "testAccAmiConfig_basic"
}
availability_zone = "${data.aws_availability_zones.available.names[0]}"
size = 8
tags {
Name = "testAccAmiConfig_basic"
}
}
resource "aws_ebs_snapshot" "foo" {
Expand All @@ -232,12 +237,14 @@ resource "aws_ami" "foo" {

func testAccAmiConfig_snapshotSize(rInt int) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {}
resource "aws_ebs_volume" "foo" {
availability_zone = "us-west-2a"
size = 20
tags {
Name = "testAccAmiConfig_snapshotSize"
}
availability_zone = "${data.aws_availability_zones.available.names[0]}"
size = 20
tags {
Name = "testAccAmiConfig_snapshotSize"
}
}
resource "aws_ebs_snapshot" "foo" {
Expand Down
2 changes: 2 additions & 0 deletions website/docs/d/ami.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ interpolation.
for machine images.
* `root_device_name` - The device name of the root device.
* `root_device_type` - The type of root device (ie: `ebs` or `instance-store`).
* `root_snapshot_id` - The snapshot id associated with the root device, if any
(only applies to `ebs` root devices).
* `sriov_net_support` - Specifies whether enhanced networking is enabled.
* `state` - The current state of the AMI. If the state is `available`, the image
is successfully registered and can be used to launch an instance.
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/ami.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ The following arguments are supported:

* `name` - (Required) A region-unique name for the AMI.
* `description` - (Optional) A longer, human-readable description for the AMI.
* `root_device_name` - (Optional) The name of the root device (for example, `/dev/sda1`, or `/dev/xvda`).
* `virtualization_type` - (Optional) Keyword to choose what virtualization mode created instances
will use. Can be either "paravirtual" (the default) or "hvm". The choice of virtualization type
changes the set of further arguments that are required, as described below.
Expand Down Expand Up @@ -98,3 +99,4 @@ Nested `ephemeral_block_device` blocks have the following structure:
The following attributes are exported:

* `id` - The ID of the created AMI.
* `root_snapshot_id` - The Snapshot ID for the root volume (for EBS-backed AMIs)

0 comments on commit 493a305

Please sign in to comment.