From 7738c98bddd0d0839a15a8b41333f3c5f60bd80a Mon Sep 17 00:00:00 2001 From: Jeremy Asher Date: Fri, 4 Nov 2016 14:41:16 -0700 Subject: [PATCH 1/3] 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 ad6c318459e3..bbb566195f88 100644 --- a/builtin/providers/aws/provider.go +++ b/builtin/providers/aws/provider.go @@ -343,6 +343,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 000000000000..3a5797cb7028 --- /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 5441c3fe511200ef369890f815034640b0e88278 Mon Sep 17 00:00:00 2001 From: Jeremy Asher Date: Fri, 4 Nov 2016 14:41:23 -0700 Subject: [PATCH 2/3] 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 000000000000..571c4c2ea961 --- /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 28f0b20684d440551a7f1a11b6254f3ac3adf330 Mon Sep 17 00:00:00 2001 From: Jeremy Asher Date: Thu, 10 Nov 2016 14:11:49 -0800 Subject: [PATCH 3/3] 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 | 68 ++++++++++++++++--- 1 file changed, 58 insertions(+), 10 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 3a5797cb7028..6a7fd40a1832 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 } + 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 true, nil + for _, vp := range attrs.CreateVolumePermissions { + if *vp.UserId == account_id { + return attrs, "granted", nil + } } + return attrs, "denied", nil } - return false, nil }