From 7216185f0d5f0740abbc95c31a3b90993911f74f Mon Sep 17 00:00:00 2001 From: Jeremy Asher Date: Fri, 4 Nov 2016 14:41:16 -0700 Subject: [PATCH 1/5] implement aws_snapshot_create_volume_permission This adds the new resource aws_snapshot_create_volume_permission which manages the createVolumePermission attribute of snapshots. This allows granting an AWS account permissions to create a volume from a particular snapshot. This is often required to allow another account to copy a private AMI. --- builtin/providers/aws/provider.go | 1 + ...e_aws_snapshot_create_volume_permission.go | 104 ++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go diff --git a/builtin/providers/aws/provider.go b/builtin/providers/aws/provider.go index 7b7aaabd5..60b2f4e08 100644 --- a/builtin/providers/aws/provider.go +++ b/builtin/providers/aws/provider.go @@ -351,6 +351,7 @@ func Provider() terraform.ResourceProvider { "aws_spot_fleet_request": resourceAwsSpotFleetRequest(), "aws_sqs_queue": resourceAwsSqsQueue(), "aws_sqs_queue_policy": resourceAwsSqsQueuePolicy(), + "aws_snapshot_create_volume_permission": resourceAwsSnapshotCreateVolumePermission(), "aws_sns_topic": resourceAwsSnsTopic(), "aws_sns_topic_policy": resourceAwsSnsTopicPolicy(), "aws_sns_topic_subscription": resourceAwsSnsTopicSubscription(), diff --git a/builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go new file mode 100644 index 000000000..3a5797cb7 --- /dev/null +++ b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go @@ -0,0 +1,104 @@ +package aws + +import ( + "fmt" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/terraform/helper/schema" +) + +func resourceAwsSnapshotCreateVolumePermission() *schema.Resource { + return &schema.Resource{ + Exists: resourceAwsSnapshotCreateVolumePermissionExists, + Create: resourceAwsSnapshotCreateVolumePermissionCreate, + Read: resourceAwsSnapshotCreateVolumePermissionRead, + Delete: resourceAwsSnapshotCreateVolumePermissionDelete, + + Schema: map[string]*schema.Schema{ + "snapshot_id": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "account_id": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + }, + } +} + +func resourceAwsSnapshotCreateVolumePermissionExists(d *schema.ResourceData, meta interface{}) (bool, error) { + conn := meta.(*AWSClient).ec2conn + + snapshot_id := d.Get("snapshot_id").(string) + account_id := d.Get("account_id").(string) + return hasCreateVolumePermission(conn, snapshot_id, account_id) +} + +func resourceAwsSnapshotCreateVolumePermissionCreate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).ec2conn + + snapshot_id := d.Get("snapshot_id").(string) + account_id := d.Get("account_id").(string) + + _, err := conn.ModifySnapshotAttribute(&ec2.ModifySnapshotAttributeInput{ + SnapshotId: aws.String(snapshot_id), + Attribute: aws.String("createVolumePermission"), + CreateVolumePermission: &ec2.CreateVolumePermissionModifications{ + Add: []*ec2.CreateVolumePermission{ + &ec2.CreateVolumePermission{UserId: aws.String(account_id)}, + }, + }, + }) + if err != nil { + return fmt.Errorf("error creating snapshot volume permission: %s", err) + } + + d.SetId(fmt.Sprintf("%s-%s", snapshot_id, account_id)) + return nil +} + +func resourceAwsSnapshotCreateVolumePermissionRead(d *schema.ResourceData, meta interface{}) error { + return nil +} + +func resourceAwsSnapshotCreateVolumePermissionDelete(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).ec2conn + + snapshot_id := d.Get("snapshot_id").(string) + account_id := d.Get("account_id").(string) + + _, err := conn.ModifySnapshotAttribute(&ec2.ModifySnapshotAttributeInput{ + SnapshotId: aws.String(snapshot_id), + Attribute: aws.String("createVolumePermission"), + CreateVolumePermission: &ec2.CreateVolumePermissionModifications{ + Remove: []*ec2.CreateVolumePermission{ + &ec2.CreateVolumePermission{UserId: aws.String(account_id)}, + }, + }, + }) + if err != nil { + return fmt.Errorf("error removing snapshot volume permission: %s", err) + } + + return nil +} + +func hasCreateVolumePermission(conn *ec2.EC2, snapshot_id string, account_id string) (bool, error) { + attrs, err := conn.DescribeSnapshotAttribute(&ec2.DescribeSnapshotAttributeInput{ + SnapshotId: aws.String(snapshot_id), + Attribute: aws.String("createVolumePermission"), + }) + if err != nil { + return false, err + } + + for _, vp := range attrs.CreateVolumePermissions { + if *vp.UserId == account_id { + return true, nil + } + } + return false, nil +} From f20d1c3caafa84cc6ac866af0976b0bf6f9fe5a1 Mon Sep 17 00:00:00 2001 From: Jeremy Asher Date: Fri, 4 Nov 2016 14:41:23 -0700 Subject: [PATCH 2/5] WIP aws_snapshot_create_volume_permission tests --- ..._snapshot_create_volume_permission_test.go | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go diff --git a/builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go new file mode 100644 index 000000000..571c4c2ea --- /dev/null +++ b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go @@ -0,0 +1,88 @@ +package aws + +import ( + "fmt" + "os" + "testing" + + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" +) + +func TestAccAWSSnapshotCreateVolumePermission_Basic(t *testing.T) { + snapshot_id := "" + account_id := os.Getenv("AWS_ACCOUNT_ID") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + if os.Getenv("AWS_ACCOUNT_ID") == "" { + t.Fatal("AWS_ACCOUNT_ID must be set") + } + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + // Scaffold everything + resource.TestStep{ + Config: testAccAWSSnapshotCreateVolumePermissionConfig(account_id, true), + Check: resource.ComposeTestCheckFunc( + testCheckResourceGetAttr("aws_ami_copy.test", "block_device_mappings.0.ebs.snapshot_id", &snapshot_id), + testAccAWSSnapshotCreateVolumePermissionExists(account_id, &snapshot_id), + ), + }, + // Drop just create volume permission to test destruction + resource.TestStep{ + Config: testAccAWSSnapshotCreateVolumePermissionConfig(account_id, false), + Check: resource.ComposeTestCheckFunc( + testAccAWSSnapshotCreateVolumePermissionDestroyed(account_id, &snapshot_id), + ), + }, + }, + }) +} + +func testAccAWSSnapshotCreateVolumePermissionExists(account_id string, snapshot_id *string) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).ec2conn + if has, err := hasCreateVolumePermission(conn, *snapshot_id, account_id); err != nil { + return err + } else if !has { + return fmt.Errorf("create volume permission does not exist for '%s' on '%s'", account_id, *snapshot_id) + } + return nil + } +} + +func testAccAWSSnapshotCreateVolumePermissionDestroyed(account_id string, snapshot_id *string) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).ec2conn + if has, err := hasCreateVolumePermission(conn, *snapshot_id, account_id); err != nil { + return err + } else if has { + return fmt.Errorf("create volume permission still exists for '%s' on '%s'", account_id, *snapshot_id) + } + return nil + } +} + +func testAccAWSSnapshotCreateVolumePermissionConfig(account_id string, includeCreateVolumePermission bool) string { + base := ` +resource "aws_ami_copy" "test" { + name = "create-volume-permission-test" + description = "Create Volume Permission Test Copy" + source_ami_id = "ami-7172b611" + source_ami_region = "us-west-2" +} +` + + if !includeCreateVolumePermission { + return base + } + + return base + fmt.Sprintf(` +resource "aws_snapshot_create_volume_permission" "self-test" { + snapshot_id = "${lookup(lookup(element(aws_ami_copy.test.block_device_mappings, 0), "ebs"), "snapshot_id")}" + account_id = "%s" +} +`, account_id) +} From 185ee439daf7225102d4c23bb321f576f9275525 Mon Sep 17 00:00:00 2001 From: Jeremy Asher Date: Thu, 10 Nov 2016 14:11:49 -0800 Subject: [PATCH 3/5] add wait after AWS snapshot attr modification This adds up to a 5 minute wait after issuing an add or remove request to adjust a snapshot's createVolumePermission attribute. --- ...e_aws_snapshot_create_volume_permission.go | 72 +++++++++++++++---- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go index 3a5797cb7..6a7fd40a1 100644 --- a/builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go +++ b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission.go @@ -2,8 +2,11 @@ package aws import ( "fmt" + "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" ) @@ -53,10 +56,26 @@ func resourceAwsSnapshotCreateVolumePermissionCreate(d *schema.ResourceData, met }, }) if err != nil { - return fmt.Errorf("error creating snapshot volume permission: %s", err) + return fmt.Errorf("Error adding snapshot createVolumePermission: %s", err) } d.SetId(fmt.Sprintf("%s-%s", snapshot_id, account_id)) + + // Wait for the account to appear in the permission list + stateConf := &resource.StateChangeConf{ + Pending: []string{"denied"}, + Target: []string{"granted"}, + Refresh: resourceAwsSnapshotCreateVolumePermissionStateRefreshFunc(conn, snapshot_id, account_id), + Timeout: 5 * time.Minute, + Delay: 10 * time.Second, + MinTimeout: 10 * time.Second, + } + if _, err := stateConf.WaitForState(); err != nil { + return fmt.Errorf( + "Error waiting for snapshot createVolumePermission (%s) to be added: %s", + d.Id(), err) + } + return nil } @@ -80,25 +99,54 @@ func resourceAwsSnapshotCreateVolumePermissionDelete(d *schema.ResourceData, met }, }) if err != nil { - return fmt.Errorf("error removing snapshot volume permission: %s", err) + return fmt.Errorf("Error removing snapshot createVolumePermission: %s", err) + } + + // Wait for the account to disappear from the permission list + stateConf := &resource.StateChangeConf{ + Pending: []string{"granted"}, + Target: []string{"denied"}, + Refresh: resourceAwsSnapshotCreateVolumePermissionStateRefreshFunc(conn, snapshot_id, account_id), + Timeout: 5 * time.Minute, + Delay: 10 * time.Second, + MinTimeout: 10 * time.Second, + } + if _, err := stateConf.WaitForState(); err != nil { + return fmt.Errorf( + "Error waiting for snapshot createVolumePermission (%s) to be removed: %s", + d.Id(), err) } return nil } func hasCreateVolumePermission(conn *ec2.EC2, snapshot_id string, account_id string) (bool, error) { - attrs, err := conn.DescribeSnapshotAttribute(&ec2.DescribeSnapshotAttributeInput{ - SnapshotId: aws.String(snapshot_id), - Attribute: aws.String("createVolumePermission"), - }) + _, state, err := resourceAwsSnapshotCreateVolumePermissionStateRefreshFunc(conn, snapshot_id, account_id)() if err != nil { return false, err } - - for _, vp := range attrs.CreateVolumePermissions { - if *vp.UserId == account_id { - return true, nil - } + if state == "granted" { + return true, nil + } else { + return false, nil + } +} + +func resourceAwsSnapshotCreateVolumePermissionStateRefreshFunc(conn *ec2.EC2, snapshot_id string, account_id string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + attrs, err := conn.DescribeSnapshotAttribute(&ec2.DescribeSnapshotAttributeInput{ + SnapshotId: aws.String(snapshot_id), + Attribute: aws.String("createVolumePermission"), + }) + if err != nil { + return nil, "", fmt.Errorf("Error refreshing snapshot createVolumePermission state: %s", err) + } + + for _, vp := range attrs.CreateVolumePermissions { + if *vp.UserId == account_id { + return attrs, "granted", nil + } + } + return attrs, "denied", nil } - return false, nil } From 42057045fff63bd3471c1003e5b94c7580a9ad94 Mon Sep 17 00:00:00 2001 From: clint shryock Date: Thu, 8 Dec 2016 16:43:03 -0600 Subject: [PATCH 4/5] refactor the test to use caller_identity data source, and new ebs_snapshot resource --- ..._snapshot_create_volume_permission_test.go | 59 ++++++++++--------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go index 571c4c2ea..53691350c 100644 --- a/builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go +++ b/builtin/providers/aws/resource_aws_snapshot_create_volume_permission_test.go @@ -2,7 +2,6 @@ package aws import ( "fmt" - "os" "testing" "github.com/hashicorp/terraform/helper/resource" @@ -10,68 +9,70 @@ import ( ) func TestAccAWSSnapshotCreateVolumePermission_Basic(t *testing.T) { - snapshot_id := "" - account_id := os.Getenv("AWS_ACCOUNT_ID") + var snapshotId, accountId string resource.Test(t, resource.TestCase{ - PreCheck: func() { - testAccPreCheck(t) - if os.Getenv("AWS_ACCOUNT_ID") == "" { - t.Fatal("AWS_ACCOUNT_ID must be set") - } - }, Providers: testAccProviders, Steps: []resource.TestStep{ // Scaffold everything resource.TestStep{ - Config: testAccAWSSnapshotCreateVolumePermissionConfig(account_id, true), + Config: testAccAWSSnapshotCreateVolumePermissionConfig(true), Check: resource.ComposeTestCheckFunc( - testCheckResourceGetAttr("aws_ami_copy.test", "block_device_mappings.0.ebs.snapshot_id", &snapshot_id), - testAccAWSSnapshotCreateVolumePermissionExists(account_id, &snapshot_id), + testCheckResourceGetAttr("aws_ebs_snapshot.example_snapshot", "id", &snapshotId), + testCheckResourceGetAttr("data.aws_caller_identity.current", "account_id", &accountId), + testAccAWSSnapshotCreateVolumePermissionExists(&accountId, &snapshotId), ), }, // Drop just create volume permission to test destruction resource.TestStep{ - Config: testAccAWSSnapshotCreateVolumePermissionConfig(account_id, false), + Config: testAccAWSSnapshotCreateVolumePermissionConfig(false), Check: resource.ComposeTestCheckFunc( - testAccAWSSnapshotCreateVolumePermissionDestroyed(account_id, &snapshot_id), + testAccAWSSnapshotCreateVolumePermissionDestroyed(&accountId, &snapshotId), ), }, }, }) } -func testAccAWSSnapshotCreateVolumePermissionExists(account_id string, snapshot_id *string) resource.TestCheckFunc { +func testAccAWSSnapshotCreateVolumePermissionExists(accountId, snapshotId *string) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ec2conn - if has, err := hasCreateVolumePermission(conn, *snapshot_id, account_id); err != nil { + if has, err := hasCreateVolumePermission(conn, *snapshotId, *accountId); err != nil { return err } else if !has { - return fmt.Errorf("create volume permission does not exist for '%s' on '%s'", account_id, *snapshot_id) + return fmt.Errorf("create volume permission does not exist for '%s' on '%s'", *accountId, *snapshotId) } return nil } } -func testAccAWSSnapshotCreateVolumePermissionDestroyed(account_id string, snapshot_id *string) resource.TestCheckFunc { +func testAccAWSSnapshotCreateVolumePermissionDestroyed(accountId, snapshotId *string) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ec2conn - if has, err := hasCreateVolumePermission(conn, *snapshot_id, account_id); err != nil { + if has, err := hasCreateVolumePermission(conn, *snapshotId, *accountId); err != nil { return err } else if has { - return fmt.Errorf("create volume permission still exists for '%s' on '%s'", account_id, *snapshot_id) + return fmt.Errorf("create volume permission still exists for '%s' on '%s'", *accountId, *snapshotId) } return nil } } -func testAccAWSSnapshotCreateVolumePermissionConfig(account_id string, includeCreateVolumePermission bool) string { +func testAccAWSSnapshotCreateVolumePermissionConfig(includeCreateVolumePermission bool) string { base := ` -resource "aws_ami_copy" "test" { - name = "create-volume-permission-test" - description = "Create Volume Permission Test Copy" - source_ami_id = "ami-7172b611" - source_ami_region = "us-west-2" +data "aws_caller_identity" "current" {} + +resource "aws_ebs_volume" "example" { + availability_zone = "us-west-2a" + size = 40 + + tags { + Name = "ebs_snap_perm" + } +} + +resource "aws_ebs_snapshot" "example_snapshot" { + volume_id = "${aws_ebs_volume.example.id}" } ` @@ -81,8 +82,8 @@ resource "aws_ami_copy" "test" { return base + fmt.Sprintf(` resource "aws_snapshot_create_volume_permission" "self-test" { - snapshot_id = "${lookup(lookup(element(aws_ami_copy.test.block_device_mappings, 0), "ebs"), "snapshot_id")}" - account_id = "%s" + snapshot_id = "${aws_ebs_snapshot.example_snapshot.id}" + account_id = "${data.aws_caller_identity.current.account_id}" } -`, account_id) +`) } From c54dba65b0cf06c60ba27098fa60b91c82ecdb15 Mon Sep 17 00:00:00 2001 From: clint shryock Date: Fri, 9 Dec 2016 08:36:40 -0600 Subject: [PATCH 5/5] document snapshot_create_volume_permissions --- ...hot_create_volume_permission.html.markdown | 42 +++++++++++++++++++ website/source/layouts/aws.erb | 6 ++- 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 website/source/docs/providers/aws/r/snapshot_create_volume_permission.html.markdown diff --git a/website/source/docs/providers/aws/r/snapshot_create_volume_permission.html.markdown b/website/source/docs/providers/aws/r/snapshot_create_volume_permission.html.markdown new file mode 100644 index 000000000..6d571725a --- /dev/null +++ b/website/source/docs/providers/aws/r/snapshot_create_volume_permission.html.markdown @@ -0,0 +1,42 @@ +--- +layout: "aws" +page_title: "AWS: aws_snapshot_create_volume_permission" +sidebar_current: "docs-aws-resource-snapshot-create-volume-permission" +description: |- + Adds create volume permission to an EBS Snapshot +--- + +# aws\_snapshot\_create\_volume\_permission + +Adds permission to create volumes off of a given EBS Snapshot. + +## Example Usage + +``` +resource "aws_snapshot_create_volume_permission" "example_perm" { + snapshot_id = "${aws_ebs_snapshot.example_snapshot.id}" + account_id = "12345678" +} + +resource "aws_ebs_volume" "example" { + availability_zone = "us-west-2a" + size = 40 +} + +resource "aws_ebs_snapshot" "example_snapshot" { + volume_id = "${aws_ebs_volume.example.id}" +} +``` + +## Argument Reference + +The following arguments are supported: + + * `snapshot_id` - (required) A snapshot ID + * `account_id` - (required) An AWS Account ID to add create volume permissions + +## Attributes Reference + +The following attributes are exported: + + * `id` - A combination of "`snapshot_id`-`account_id`". diff --git a/website/source/layouts/aws.erb b/website/source/layouts/aws.erb index c28fbdb06..74b17a0a3 100644 --- a/website/source/layouts/aws.erb +++ b/website/source/layouts/aws.erb @@ -266,7 +266,7 @@ - > + > EC2 Resources