diff --git a/.changelog/20677.txt b/.changelog/20677.txt new file mode 100644 index 000000000000..fb0d668a9670 --- /dev/null +++ b/.changelog/20677.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_ami_launch_permission: Add `group` argument +``` \ No newline at end of file diff --git a/.changelog/21694.txt b/.changelog/21694.txt new file mode 100644 index 000000000000..89ae21eb36cb --- /dev/null +++ b/.changelog/21694.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_ami_launch_permission: Add `organization_arn` and `organizational_unit_arn` arguments +``` \ No newline at end of file diff --git a/internal/service/ec2/ami_launch_permission.go b/internal/service/ec2/ami_launch_permission.go index dae4001de81b..4dd3a8ea4aa6 100644 --- a/internal/service/ec2/ami_launch_permission.go +++ b/internal/service/ec2/ami_launch_permission.go @@ -1,136 +1,264 @@ package ec2 import ( + "context" "fmt" "log" + "regexp" "strings" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/internal/verify" ) func ResourceAMILaunchPermission() *schema.Resource { return &schema.Resource{ - Create: resourceAMILaunchPermissionCreate, - Read: resourceAMILaunchPermissionRead, - Delete: resourceAMILaunchPermissionDelete, + CreateWithoutTimeout: resourceAMILaunchPermissionCreate, + ReadWithoutTimeout: resourceAMILaunchPermissionRead, + DeleteWithoutTimeout: resourceAMILaunchPermissionDelete, + Importer: &schema.ResourceImporter{ - State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - idParts := strings.Split(d.Id(), "/") - if len(idParts) != 2 || idParts[0] == "" || idParts[1] == "" { - return nil, fmt.Errorf("Unexpected format of ID (%q), expected ACCOUNT-ID/IMAGE-ID", d.Id()) - } - accountId := idParts[0] - imageId := idParts[1] - d.Set("account_id", accountId) - d.Set("image_id", imageId) - d.SetId(fmt.Sprintf("%s-%s", imageId, accountId)) - return []*schema.ResourceData{d}, nil - }, + StateContext: resourceAMILaunchPermissionImport, }, Schema: map[string]*schema.Schema{ + "account_id": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ExactlyOneOf: []string{"account_id", "group", "organization_arn", "organizational_unit_arn"}, + }, + "group": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: validation.StringInSlice(ec2.PermissionGroup_Values(), false), + ExactlyOneOf: []string{"account_id", "group", "organization_arn", "organizational_unit_arn"}, + }, "image_id": { Type: schema.TypeString, Required: true, ForceNew: true, }, - "account_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + "organization_arn": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, + ExactlyOneOf: []string{"account_id", "group", "organization_arn", "organizational_unit_arn"}, + }, + "organizational_unit_arn": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, + ExactlyOneOf: []string{"account_id", "group", "organization_arn", "organizational_unit_arn"}, }, }, } } -func resourceAMILaunchPermissionCreate(d *schema.ResourceData, meta interface{}) error { +func resourceAMILaunchPermissionCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).EC2Conn - image_id := d.Get("image_id").(string) - account_id := d.Get("account_id").(string) - - _, err := conn.ModifyImageAttribute(&ec2.ModifyImageAttributeInput{ - ImageId: aws.String(image_id), + imageID := d.Get("image_id").(string) + accountID := d.Get("account_id").(string) + group := d.Get("group").(string) + organizationARN := d.Get("organization_arn").(string) + organizationalUnitARN := d.Get("organizational_unit_arn").(string) + id := AMILaunchPermissionCreateResourceID(imageID, accountID, group, organizationARN, organizationalUnitARN) + input := &ec2.ModifyImageAttributeInput{ Attribute: aws.String(ec2.ImageAttributeNameLaunchPermission), + ImageId: aws.String(imageID), LaunchPermission: &ec2.LaunchPermissionModifications{ - Add: []*ec2.LaunchPermission{ - {UserId: aws.String(account_id)}, - }, + Add: expandLaunchPermissions(accountID, group, organizationARN, organizationalUnitARN), }, - }) + } + + log.Printf("[DEBUG] Creating AMI Launch Permission: %s", input) + _, err := conn.ModifyImageAttributeWithContext(ctx, input) + if err != nil { - return fmt.Errorf("error creating AMI launch permission: %w", err) + return diag.Errorf("creating AMI Launch Permission (%s): %s", id, err) } - d.SetId(fmt.Sprintf("%s-%s", image_id, account_id)) - return nil + d.SetId(id) + + return resourceAMILaunchPermissionRead(ctx, d, meta) } -func resourceAMILaunchPermissionRead(d *schema.ResourceData, meta interface{}) error { +func resourceAMILaunchPermissionRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).EC2Conn - exists, err := HasLaunchPermission(conn, d.Get("image_id").(string), d.Get("account_id").(string)) + imageID, accountID, group, organizationARN, organizationalUnitARN, err := AMILaunchPermissionParseResourceID(d.Id()) + if err != nil { - return fmt.Errorf("error reading AMI launch permission (%s): %w", d.Id(), err) + return diag.FromErr(err) } - if !exists { - if d.IsNewResource() { - return fmt.Errorf("error reading EC2 AMI Launch Permission (%s): not found", d.Id()) - } - log.Printf("[WARN] AMI launch permission (%s) not found, removing from state", d.Id()) + _, err = FindImageLaunchPermission(ctx, conn, imageID, accountID, group, organizationARN, organizationalUnitARN) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] AMI Launch Permission %s not found, removing from state", d.Id()) d.SetId("") return nil } + if err != nil { + return diag.Errorf("reading AMI Launch Permission (%s): %s", d.Id(), err) + } + + d.Set("account_id", accountID) + d.Set("group", group) + d.Set("image_id", imageID) + d.Set("organization_arn", organizationARN) + d.Set("organizational_unit_arn", organizationalUnitARN) + return nil } -func resourceAMILaunchPermissionDelete(d *schema.ResourceData, meta interface{}) error { +func resourceAMILaunchPermissionDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).EC2Conn - image_id := d.Get("image_id").(string) - account_id := d.Get("account_id").(string) + imageID, accountID, group, organizationARN, organizationalUnitARN, err := AMILaunchPermissionParseResourceID(d.Id()) + + if err != nil { + return diag.FromErr(err) + } - _, err := conn.ModifyImageAttribute(&ec2.ModifyImageAttributeInput{ - ImageId: aws.String(image_id), + input := &ec2.ModifyImageAttributeInput{ Attribute: aws.String(ec2.ImageAttributeNameLaunchPermission), + ImageId: aws.String(imageID), LaunchPermission: &ec2.LaunchPermissionModifications{ - Remove: []*ec2.LaunchPermission{ - {UserId: aws.String(account_id)}, - }, + Remove: expandLaunchPermissions(accountID, group, organizationARN, organizationalUnitARN), }, - }) + } + + log.Printf("[INFO] Deleting AMI Launch Permission: %s", d.Id()) + _, err = conn.ModifyImageAttributeWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, ErrCodeInvalidAMIIDNotFound, ErrCodeInvalidAMIIDUnavailable) { + return nil + } + if err != nil { - return fmt.Errorf("error deleting AMI launch permission (%s): %w", d.Id(), err) + return diag.Errorf("deleting AMI Launch Permission (%s): %s", d.Id(), err) } return nil } -func HasLaunchPermission(conn *ec2.EC2, image_id string, account_id string) (bool, error) { - attrs, err := conn.DescribeImageAttribute(&ec2.DescribeImageAttributeInput{ - ImageId: aws.String(image_id), - Attribute: aws.String(ec2.ImageAttributeNameLaunchPermission), - }) - if err != nil { - // When an AMI disappears out from under a launch permission resource, we will - // see either InvalidAMIID.NotFound or InvalidAMIID.Unavailable. - if ec2err, ok := err.(awserr.Error); ok && strings.HasPrefix(ec2err.Code(), "InvalidAMIID") { - log.Printf("[DEBUG] %s no longer exists, so we'll drop launch permission for %s from the state", image_id, account_id) - return false, nil +func resourceAMILaunchPermissionImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + const importIDSeparator = "/" + parts := strings.Split(d.Id(), importIDSeparator) + + // Heuristic to identify the permission type. + var ok bool + if n := len(parts); n >= 2 { + if permissionID, imageID := strings.Join(parts[:n-1], importIDSeparator), parts[n-1]; permissionID != "" && imageID != "" { + if regexp.MustCompile(`^\d{12}$`).MatchString(permissionID) { + // AWS account ID. + d.SetId(AMILaunchPermissionCreateResourceID(imageID, permissionID, "", "", "")) + ok = true + } else if arn.IsARN(permissionID) { + if v, _ := arn.Parse(permissionID); v.Service == "organizations" { + // See https://docs.aws.amazon.com/service-authorization/latest/reference/list_awsorganizations.html#awsorganizations-resources-for-iam-policies. + if strings.HasPrefix(v.Resource, "organization/") { + // Organization ARN. + d.SetId(AMILaunchPermissionCreateResourceID(imageID, "", "", permissionID, "")) + ok = true + } else if strings.HasPrefix(v.Resource, "ou/") { + // Organizational unit ARN. + d.SetId(AMILaunchPermissionCreateResourceID(imageID, "", "", "", permissionID)) + ok = true + } + } + } else { + // Group name. + d.SetId(AMILaunchPermissionCreateResourceID(imageID, "", permissionID, "", "")) + ok = true + } } - return false, err } - for _, lp := range attrs.LaunchPermissions { - if aws.StringValue(lp.UserId) == account_id { - return true, nil + if !ok { + return nil, fmt.Errorf("unexpected format for ID (%[1]s), expected [ACCOUNT-ID|GROUP-NAME|ORGANIZATION-ARN|ORGANIZATIONAL-UNIT-ARN]%[2]sIMAGE-ID", d.Id(), importIDSeparator) + } + + return []*schema.ResourceData{d}, nil +} + +func expandLaunchPermissions(accountID, group, organizationARN, organizationalUnitARN string) []*ec2.LaunchPermission { + apiObject := &ec2.LaunchPermission{} + + if accountID != "" { + apiObject.UserId = aws.String(accountID) + } + + if group != "" { + apiObject.Group = aws.String(group) + } + + if organizationARN != "" { + apiObject.OrganizationArn = aws.String(organizationARN) + } + + if organizationalUnitARN != "" { + apiObject.OrganizationalUnitArn = aws.String(organizationalUnitARN) + } + + return []*ec2.LaunchPermission{apiObject} +} + +const ( + amiLaunchPermissionIDSeparator = "-" + amiLaunchPermissionIDGroupIndicator = "group" + amiLaunchPermissionIDOrganizationIndicator = "org" + amiLaunchPermissionIDOrganizationalUnitIndicator = "ou" +) + +func AMILaunchPermissionCreateResourceID(imageID, accountID, group, organizationARN, organizationalUnitARN string) string { + parts := []string{imageID} + + if accountID != "" { + parts = append(parts, accountID) + } else if group != "" { + parts = append(parts, amiLaunchPermissionIDGroupIndicator, group) + } else if organizationARN != "" { + parts = append(parts, amiLaunchPermissionIDOrganizationIndicator, organizationARN) + } else if organizationalUnitARN != "" { + parts = append(parts, amiLaunchPermissionIDOrganizationalUnitIndicator, organizationalUnitARN) + } + + id := strings.Join(parts, amiLaunchPermissionIDSeparator) + + return id +} + +func AMILaunchPermissionParseResourceID(id string) (string, string, string, string, string, error) { + parts := strings.Split(id, amiLaunchPermissionIDSeparator) + + switch { + case len(parts) == 3 && parts[0] != "" && parts[1] != "" && parts[2] != "": + return strings.Join([]string{parts[0], parts[1]}, amiLaunchPermissionIDSeparator), parts[2], "", "", "", nil + case len(parts) > 3 && parts[0] != "" && parts[1] != "" && parts[3] != "": + switch parts[2] { + case amiLaunchPermissionIDGroupIndicator: + return strings.Join([]string{parts[0], parts[1]}, amiLaunchPermissionIDSeparator), "", strings.Join(parts[3:], amiLaunchPermissionIDSeparator), "", "", nil + case amiLaunchPermissionIDOrganizationIndicator: + return strings.Join([]string{parts[0], parts[1]}, amiLaunchPermissionIDSeparator), "", "", strings.Join(parts[3:], amiLaunchPermissionIDSeparator), "", nil + case amiLaunchPermissionIDOrganizationalUnitIndicator: + return strings.Join([]string{parts[0], parts[1]}, amiLaunchPermissionIDSeparator), "", "", "", strings.Join(parts[3:], amiLaunchPermissionIDSeparator), nil } } - return false, nil + + return "", "", "", "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected IMAGE-ID%[2]sACCOUNT-ID or IMAGE-ID%[2]s%[3]s%[2]sGROUP-NAME or IMAGE-ID%[2]s%[4]s%[2]sORGANIZATION-ARN or IMAGE-ID%[2]s%[5]s%[2]sORGANIZATIONAL-UNIT-ARN", id, amiLaunchPermissionIDSeparator, amiLaunchPermissionIDGroupIndicator, amiLaunchPermissionIDOrganizationIndicator, amiLaunchPermissionIDOrganizationalUnitIndicator) } diff --git a/internal/service/ec2/ami_launch_permission_test.go b/internal/service/ec2/ami_launch_permission_test.go index b3161111d5c8..7a93162d7af5 100644 --- a/internal/service/ec2/ami_launch_permission_test.go +++ b/internal/service/ec2/ami_launch_permission_test.go @@ -1,10 +1,10 @@ package ec2_test import ( + "context" "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfec2 "github.com/hashicorp/terraform-provider-aws/internal/service/ec2" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccEC2AMILaunchPermission_basic(t *testing.T) { @@ -25,9 +26,13 @@ func TestAccEC2AMILaunchPermission_basic(t *testing.T) { CheckDestroy: testAccCheckAMILaunchPermissionDestroy, Steps: []resource.TestStep{ { - Config: testAccAMILaunchPermissionConfig(rName), + Config: testAccAMILaunchPermissionAccountIDConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAMILaunchPermissionExists(resourceName), + acctest.CheckResourceAttrAccountID(resourceName, "account_id"), + resource.TestCheckResourceAttr(resourceName, "group", ""), + resource.TestCheckResourceAttr(resourceName, "organization_arn", ""), + resource.TestCheckResourceAttr(resourceName, "organizational_unit_arn", ""), ), }, { @@ -40,7 +45,7 @@ func TestAccEC2AMILaunchPermission_basic(t *testing.T) { }) } -func TestAccEC2AMILaunchPermission_Disappears_launchPermission(t *testing.T) { +func TestAccEC2AMILaunchPermission_disappears(t *testing.T) { resourceName := "aws_ami_launch_permission.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -51,10 +56,10 @@ func TestAccEC2AMILaunchPermission_Disappears_launchPermission(t *testing.T) { CheckDestroy: testAccCheckAMILaunchPermissionDestroy, Steps: []resource.TestStep{ { - Config: testAccAMILaunchPermissionConfig(rName), + Config: testAccAMILaunchPermissionAccountIDConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAMILaunchPermissionExists(resourceName), - testAccCheckAMILaunchPermissionDisappears(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceAMILaunchPermission(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -62,9 +67,7 @@ func TestAccEC2AMILaunchPermission_Disappears_launchPermission(t *testing.T) { }) } -// Bug reference: https://github.com/hashicorp/terraform-provider-aws/issues/6222 -// Images with all will not have and can cause a panic -func TestAccEC2AMILaunchPermission_DisappearsLaunchPermission_public(t *testing.T) { +func TestAccEC2AMILaunchPermission_Disappears_ami(t *testing.T) { resourceName := "aws_ami_launch_permission.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -75,11 +78,10 @@ func TestAccEC2AMILaunchPermission_DisappearsLaunchPermission_public(t *testing. CheckDestroy: testAccCheckAMILaunchPermissionDestroy, Steps: []resource.TestStep{ { - Config: testAccAMILaunchPermissionConfig(rName), + Config: testAccAMILaunchPermissionAccountIDConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAMILaunchPermissionExists(resourceName), - testAccCheckAMILaunchPermissionAddPublic(resourceName), - testAccCheckAMILaunchPermissionDisappears(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceAMICopy(), "aws_ami_copy.test"), ), ExpectNonEmptyPlan: true, }, @@ -87,8 +89,7 @@ func TestAccEC2AMILaunchPermission_DisappearsLaunchPermission_public(t *testing. }) } -func TestAccEC2AMILaunchPermission_Disappears_ami(t *testing.T) { - imageID := "" +func TestAccEC2AMILaunchPermission_group(t *testing.T) { resourceName := "aws_ami_launch_permission.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -99,212 +100,244 @@ func TestAccEC2AMILaunchPermission_Disappears_ami(t *testing.T) { CheckDestroy: testAccCheckAMILaunchPermissionDestroy, Steps: []resource.TestStep{ { - Config: testAccAMILaunchPermissionConfig(rName), + Config: testAccAMILaunchPermissionGroupConfig(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAMILaunchPermissionExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "account_id", ""), + resource.TestCheckResourceAttr(resourceName, "group", "all"), + resource.TestCheckResourceAttr(resourceName, "organization_arn", ""), + resource.TestCheckResourceAttr(resourceName, "organizational_unit_arn", ""), ), }, - // Here we delete the AMI to verify the follow-on refresh after this step - // should not error. { - Config: testAccAMILaunchPermissionConfig(rName), + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAMILaunchPermissionImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccEC2AMILaunchPermission_organizationARN(t *testing.T) { + resourceName := "aws_ami_launch_permission.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckOrganizationsEnabled(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckAMILaunchPermissionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAMILaunchPermissionOrganizationARNConfig(rName), Check: resource.ComposeTestCheckFunc( - testCheckResourceGetAttr("aws_ami_copy.test", "id", &imageID), - testAccAMIDisappears(&imageID), + testAccCheckAMILaunchPermissionExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "account_id", ""), + resource.TestCheckResourceAttr(resourceName, "group", ""), + resource.TestCheckResourceAttrSet(resourceName, "organization_arn"), + resource.TestCheckResourceAttr(resourceName, "organizational_unit_arn", ""), ), - ExpectNonEmptyPlan: true, + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAMILaunchPermissionImportStateIdFunc(resourceName), + ImportStateVerify: true, }, }, }) } -func testCheckResourceGetAttr(name, key string, value *string) resource.TestCheckFunc { - return func(s *terraform.State) error { - ms := s.RootModule() - rs, ok := ms.Resources[name] +func TestAccEC2AMILaunchPermission_organizationalUnitARN(t *testing.T) { + resourceName := "aws_ami_launch_permission.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckOrganizationsAccount(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckAMILaunchPermissionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAMILaunchPermissionOrganizationalUnitARNConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAMILaunchPermissionExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "account_id", ""), + resource.TestCheckResourceAttr(resourceName, "group", ""), + resource.TestCheckResourceAttr(resourceName, "organization_arn", ""), + resource.TestCheckResourceAttrSet(resourceName, "organizational_unit_arn"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateIdFunc: testAccAMILaunchPermissionImportStateIdFunc(resourceName), + ImportStateVerify: true, + }, + }, + }) +} + +func testAccAMILaunchPermissionImportStateIdFunc(resourceName string) resource.ImportStateIdFunc { + return func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources[resourceName] if !ok { - return fmt.Errorf("Not found: %s", name) + return "", fmt.Errorf("Not found: %s", resourceName) } - is := rs.Primary - if is == nil { - return fmt.Errorf("No primary instance: %s", name) - } + imageID := rs.Primary.Attributes["image_id"] - *value = is.Attributes[key] - return nil + if v := rs.Primary.Attributes["group"]; v != "" { + return fmt.Sprintf("%s/%s", v, imageID), nil + } else if v := rs.Primary.Attributes["organization_arn"]; v != "" { + return fmt.Sprintf("%s/%s", v, imageID), nil + } else if v := rs.Primary.Attributes["organizational_unit_arn"]; v != "" { + return fmt.Sprintf("%s/%s", v, imageID), nil + } else { + return fmt.Sprintf("%s/%s", rs.Primary.Attributes["account_id"], imageID), nil + } } } -func testAccCheckAMILaunchPermissionExists(resourceName string) resource.TestCheckFunc { +func testAccCheckAMILaunchPermissionExists(n string) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resourceName] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", resourceName) + return fmt.Errorf("Not found: %s", n) } if rs.Primary.ID == "" { - return fmt.Errorf("No resource ID is set") + return fmt.Errorf("No AMI Launch Permission ID is set") + } + + imageID, accountID, group, organizationARN, organizationalUnitARN, err := tfec2.AMILaunchPermissionParseResourceID(rs.Primary.ID) + + if err != nil { + return err } conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - accountID := rs.Primary.Attributes["account_id"] - imageID := rs.Primary.Attributes["image_id"] - if has, err := tfec2.HasLaunchPermission(conn, imageID, accountID); err != nil { + _, err = tfec2.FindImageLaunchPermission(context.TODO(), conn, imageID, accountID, group, organizationARN, organizationalUnitARN) + + if err != nil { return err - } else if !has { - return fmt.Errorf("launch permission does not exist for '%s' on '%s'", accountID, imageID) } + return nil } } func testAccCheckAMILaunchPermissionDestroy(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn + for _, rs := range s.RootModule().Resources { if rs.Type != "aws_ami_launch_permission" { continue } - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - accountID := rs.Primary.Attributes["account_id"] - imageID := rs.Primary.Attributes["image_id"] + imageID, accountID, group, organizationARN, organizationalUnitARN, err := tfec2.AMILaunchPermissionParseResourceID(rs.Primary.ID) - if has, err := tfec2.HasLaunchPermission(conn, imageID, accountID); err != nil { + if err != nil { return err - } else if has { - return fmt.Errorf("launch permission still exists for '%s' on '%s'", accountID, imageID) } - } - return nil -} - -func testAccCheckAMILaunchPermissionAddPublic(resourceName string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resourceName] - if !ok { - return fmt.Errorf("Not found: %s", resourceName) - } + _, err = tfec2.FindImageLaunchPermission(context.TODO(), conn, imageID, accountID, group, organizationARN, organizationalUnitARN) - if rs.Primary.ID == "" { - return fmt.Errorf("No resource ID is set") + if tfresource.NotFound(err) { + continue } - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - imageID := rs.Primary.Attributes["image_id"] - - input := &ec2.ModifyImageAttributeInput{ - ImageId: aws.String(imageID), - Attribute: aws.String(ec2.ImageAttributeNameLaunchPermission), - LaunchPermission: &ec2.LaunchPermissionModifications{ - Add: []*ec2.LaunchPermission{ - {Group: aws.String(ec2.PermissionGroupAll)}, - }, - }, + if err != nil { + return err } - _, err := conn.ModifyImageAttribute(input) - - return err + return fmt.Errorf("AMI Launch Permission %s still exists", rs.Primary.ID) } -} - -func testAccCheckAMILaunchPermissionDisappears(resourceName string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[resourceName] - if !ok { - return fmt.Errorf("Not found: %s", resourceName) - } - - if rs.Primary.ID == "" { - return fmt.Errorf("No resource ID is set") - } - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - accountID := rs.Primary.Attributes["account_id"] - imageID := rs.Primary.Attributes["image_id"] + return nil +} - input := &ec2.ModifyImageAttributeInput{ - ImageId: aws.String(imageID), - Attribute: aws.String(ec2.ImageAttributeNameLaunchPermission), - LaunchPermission: &ec2.LaunchPermissionModifications{ - Remove: []*ec2.LaunchPermission{ - {UserId: aws.String(accountID)}, - }, - }, - } +func testAccAMILaunchPermissionAccountIDConfig(rName string) string { + return acctest.ConfigCompose(acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), fmt.Sprintf(` +data "aws_caller_identity" "current" {} - _, err := conn.ModifyImageAttribute(input) +data "aws_region" "current" {} - return err - } +resource "aws_ami_copy" "test" { + description = %[1]q + name = %[1]q + source_ami_id = data.aws_ami.amzn-ami-minimal-hvm-ebs.id + source_ami_region = data.aws_region.current.name } -// testAccAMIDisappears is technically a "test check function" but really it -// exists to perform a side effect of deleting an AMI out from under a resource -// so we can test that Terraform will react properly -func testAccAMIDisappears(imageID *string) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - req := &ec2.DeregisterImageInput{ - ImageId: aws.String(*imageID), - } +resource "aws_ami_launch_permission" "test" { + account_id = data.aws_caller_identity.current.account_id + image_id = aws_ami_copy.test.id +} +`, rName)) +} - _, err := conn.DeregisterImage(req) - if err != nil { - return err - } +func testAccAMILaunchPermissionGroupConfig(rName string) string { + return acctest.ConfigCompose(acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), fmt.Sprintf(` +data "aws_region" "current" {} - if err := tfec2.AMIWaitForDestroy(tfec2.AMIDeleteRetryTimeout, *imageID, conn); err != nil { - return err - } - return nil - } +resource "aws_ami_copy" "test" { + description = %[1]q + name = %[1]q + source_ami_id = data.aws_ami.amzn-ami-minimal-hvm-ebs.id + source_ami_region = data.aws_region.current.name } -func testAccAMILaunchPermissionConfig(rName string) string { - return fmt.Sprintf(` -data "aws_ami" "amzn-ami-minimal-hvm" { - most_recent = true - owners = ["amazon"] - - filter { - name = "name" - values = ["amzn-ami-minimal-hvm-*"] - } - - filter { - name = "root-device-type" - values = ["ebs"] - } +resource "aws_ami_launch_permission" "test" { + group = "all" + image_id = aws_ami_copy.test.id +} +`, rName)) } -data "aws_caller_identity" "current" {} +func testAccAMILaunchPermissionOrganizationARNConfig(rName string) string { + return acctest.ConfigCompose(acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), fmt.Sprintf(` +data "aws_organizations_organization" "current" {} data "aws_region" "current" {} resource "aws_ami_copy" "test" { - description = %q - name = %q - source_ami_id = data.aws_ami.amzn-ami-minimal-hvm.id + description = %[1]q + name = %[1]q + source_ami_id = data.aws_ami.amzn-ami-minimal-hvm-ebs.id source_ami_region = data.aws_region.current.name } resource "aws_ami_launch_permission" "test" { - account_id = data.aws_caller_identity.current.account_id - image_id = aws_ami_copy.test.id + organization_arn = data.aws_organizations_organization.current.arn + image_id = aws_ami_copy.test.id } -`, rName, rName) +`, rName)) } -func testAccAMILaunchPermissionImportStateIdFunc(resourceName string) resource.ImportStateIdFunc { - return func(s *terraform.State) (string, error) { - rs, ok := s.RootModule().Resources[resourceName] - if !ok { - return "", fmt.Errorf("Not found: %s", resourceName) - } +func testAccAMILaunchPermissionOrganizationalUnitARNConfig(rName string) string { + return acctest.ConfigCompose(acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), fmt.Sprintf(` +resource "aws_organizations_organization" "test" {} - return fmt.Sprintf("%s/%s", rs.Primary.Attributes["account_id"], rs.Primary.Attributes["image_id"]), nil - } +resource "aws_organizations_organizational_unit" "test" { + name = %[1]q + parent_id = aws_organizations_organization.test.roots[0].id +} + +data "aws_region" "current" {} + +resource "aws_ami_copy" "test" { + description = %[1]q + name = %[1]q + source_ami_id = data.aws_ami.amzn-ami-minimal-hvm-ebs.id + source_ami_region = data.aws_region.current.name +} + +resource "aws_ami_launch_permission" "test" { + organizational_unit_arn = aws_organizations_organizational_unit.test.arn + image_id = aws_ami_copy.test.id +} +`, rName)) } diff --git a/internal/service/ec2/errors.go b/internal/service/ec2/errors.go index 06d900ae606e..964569e77cb5 100644 --- a/internal/service/ec2/errors.go +++ b/internal/service/ec2/errors.go @@ -16,6 +16,8 @@ const ( ErrCodeDependencyViolation = "DependencyViolation" ErrCodeGatewayNotAttached = "Gateway.NotAttached" ErrCodeIncorrectState = "IncorrectState" + ErrCodeInvalidAMIIDNotFound = "InvalidAMIID.NotFound" + ErrCodeInvalidAMIIDUnavailable = "InvalidAMIID.Unavailable" ErrCodeInvalidAddressNotFound = "InvalidAddress.NotFound" ErrCodeInvalidAllocationIDNotFound = "InvalidAllocationID.NotFound" ErrCodeInvalidAssociationIDNotFound = "InvalidAssociationID.NotFound" diff --git a/internal/service/ec2/find.go b/internal/service/ec2/find.go index d5c9b23c451a..93f935bbd1d9 100644 --- a/internal/service/ec2/find.go +++ b/internal/service/ec2/find.go @@ -1,6 +1,7 @@ package ec2 import ( + "context" "fmt" "strconv" @@ -590,6 +591,65 @@ func FindHost(conn *ec2.EC2, input *ec2.DescribeHostsInput) (*ec2.Host, error) { return host, nil } +func FindImageAttribute(ctx context.Context, conn *ec2.EC2, input *ec2.DescribeImageAttributeInput) (*ec2.DescribeImageAttributeOutput, error) { + output, err := conn.DescribeImageAttributeWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, ErrCodeInvalidAMIIDNotFound, ErrCodeInvalidAMIIDUnavailable) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} + +func FindImageLaunchPermissionsByID(ctx context.Context, conn *ec2.EC2, id string) ([]*ec2.LaunchPermission, error) { + input := &ec2.DescribeImageAttributeInput{ + Attribute: aws.String(ec2.ImageAttributeNameLaunchPermission), + ImageId: aws.String(id), + } + + output, err := FindImageAttribute(ctx, conn, input) + + if err != nil { + return nil, err + } + + if len(output.LaunchPermissions) == 0 { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.LaunchPermissions, nil +} + +func FindImageLaunchPermission(ctx context.Context, conn *ec2.EC2, imageID, accountID, group, organizationARN, organizationalUnitARN string) (*ec2.LaunchPermission, error) { + output, err := FindImageLaunchPermissionsByID(ctx, conn, imageID) + + if err != nil { + return nil, err + } + + for _, v := range output { + if (accountID != "" && aws.StringValue(v.UserId) == accountID) || + (group != "" && aws.StringValue(v.Group) == group) || + (organizationARN != "" && aws.StringValue(v.OrganizationArn) == organizationARN) || + (organizationalUnitARN != "" && aws.StringValue(v.OrganizationalUnitArn) == organizationalUnitARN) { + return v, nil + } + } + + return nil, &resource.NotFoundError{} +} + func FindInstances(conn *ec2.EC2, input *ec2.DescribeInstancesInput) ([]*ec2.Instance, error) { var output []*ec2.Instance diff --git a/internal/service/ec2/snapshot_create_volume_permission_test.go b/internal/service/ec2/snapshot_create_volume_permission_test.go index 8858c9cdec01..1e5c4554908e 100644 --- a/internal/service/ec2/snapshot_create_volume_permission_test.go +++ b/internal/service/ec2/snapshot_create_volume_permission_test.go @@ -65,6 +65,24 @@ func TestAccEC2SnapshotCreateVolumePermission_disappears(t *testing.T) { }) } +func testCheckResourceGetAttr(name, key string, value *string) resource.TestCheckFunc { + return func(s *terraform.State) error { + ms := s.RootModule() + rs, ok := ms.Resources[name] + if !ok { + return fmt.Errorf("Not found: %s", name) + } + + is := rs.Primary + if is == nil { + return fmt.Errorf("No primary instance: %s", name) + } + + *value = is.Attributes[key] + return nil + } +} + func testAccSnapshotCreateVolumePermissionDestroy(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn diff --git a/website/docs/r/ami_launch_permission.html.markdown b/website/docs/r/ami_launch_permission.html.markdown index ce9b422213c6..8bac7c2e1fa6 100644 --- a/website/docs/r/ami_launch_permission.html.markdown +++ b/website/docs/r/ami_launch_permission.html.markdown @@ -3,15 +3,17 @@ subcategory: "EC2 (Elastic Compute Cloud)" layout: "aws" page_title: "AWS: aws_ami_launch_permission" description: |- - Adds launch permission to Amazon Machine Image (AMI). + Adds a launch permission to an Amazon Machine Image (AMI). --- # Resource: aws_ami_launch_permission -Adds launch permission to Amazon Machine Image (AMI) from another AWS account. +Adds a launch permission to an Amazon Machine Image (AMI). ## Example Usage +### AWS Account ID + ```terraform resource "aws_ami_launch_permission" "example" { image_id = "ami-12345678" @@ -19,22 +21,45 @@ resource "aws_ami_launch_permission" "example" { } ``` +### Public Access + +```terraform +resource "aws_ami_launch_permission" "example" { + image_id = "ami-12345678" + group = "all" +} +``` + +### Organization Access + +```terraform +data "aws_organizations_organization" "current" {} + +resource "aws_ami_launch_permission" "example" { + image_id = "ami-12345678" + organization_arn = data.aws_organizations_organization.current.arn +} +``` + ## Argument Reference The following arguments are supported: -* `image_id` - (required) A region-unique name for the AMI. -* `account_id` - (required) An AWS Account ID to add launch permissions. +* `account_id` - (Optional) The AWS account ID for the launch permission. +* `group` - (Optional) The name of the group for the launch permission. Valid values: `"all"`. +* `image_id` - (Required) The ID of the AMI. +* `organization_arn` - (Optional) The ARN of an organization for the launch permission. +* `organizational_unit_arn` - (Optional) The ARN of an organizational unit for the launch permission. ## Attributes Reference In addition to all arguments above, the following attributes are exported: -* `id` - A combination of "`image_id`-`account_id`". +* `id` - Launch permission ID. ## Import -AWS AMI Launch Permission can be imported using the `ACCOUNT-ID/IMAGE-ID`, e.g., +AMI Launch Permissions can be imported using `[ACCOUNT-ID|GROUP-NAME|ORGANIZATION-ARN|ORGANIZATIONAL-UNIT-ARN]/IMAGE-ID`, e.g., ```sh $ terraform import aws_ami_launch_permission.example 123456789012/ami-12345678