diff --git a/.changelog/19749.txt b/.changelog/19749.txt new file mode 100644 index 00000000000..6eb11135d13 --- /dev/null +++ b/.changelog/19749.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_backup_vault_policy: Correctly handle reading policy of deleted vault +``` \ No newline at end of file diff --git a/.changelog/19854.txt b/.changelog/19854.txt new file mode 100644 index 00000000000..6e8f01e19ca --- /dev/null +++ b/.changelog/19854.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_backup_vault_policy: Correctly handle deleting policy of deleted vault +``` \ No newline at end of file diff --git a/aws/internal/service/backup/errors.go b/aws/internal/service/backup/errors.go new file mode 100644 index 00000000000..b07811ea247 --- /dev/null +++ b/aws/internal/service/backup/errors.go @@ -0,0 +1,5 @@ +package backup + +const ( + ErrCodeAccessDeniedException = "AccessDeniedException" +) diff --git a/aws/internal/service/backup/finder/finder.go b/aws/internal/service/backup/finder/finder.go new file mode 100644 index 00000000000..5541d757a30 --- /dev/null +++ b/aws/internal/service/backup/finder/finder.go @@ -0,0 +1,37 @@ +package finder + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/backup" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + tfbackup "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/backup" +) + +func BackupVaultAccessPolicyByName(conn *backup.Backup, name string) (*backup.GetBackupVaultAccessPolicyOutput, error) { + input := &backup.GetBackupVaultAccessPolicyInput{ + BackupVaultName: aws.String(name), + } + + output, err := conn.GetBackupVaultAccessPolicy(input) + + if tfawserr.ErrCodeEquals(err, backup.ErrCodeResourceNotFoundException) || tfawserr.ErrCodeEquals(err, tfbackup.ErrCodeAccessDeniedException) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, &resource.NotFoundError{ + Message: "Empty result", + LastRequest: input, + } + } + + return output, nil +} diff --git a/aws/resource_aws_backup_vault_policy.go b/aws/resource_aws_backup_vault_policy.go index b382caf340a..670e124fc44 100644 --- a/aws/resource_aws_backup_vault_policy.go +++ b/aws/resource_aws_backup_vault_policy.go @@ -6,8 +6,12 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/backup" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + tfbackup "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/backup" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/backup/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsBackupVaultPolicy() *schema.Resource { @@ -21,6 +25,10 @@ func resourceAwsBackupVaultPolicy() *schema.Resource { }, Schema: map[string]*schema.Schema{ + "backup_vault_arn": { + Type: schema.TypeString, + Computed: true, + }, "backup_vault_name": { Type: schema.TypeString, Required: true, @@ -32,10 +40,6 @@ func resourceAwsBackupVaultPolicy() *schema.Resource { ValidateFunc: validation.StringIsJSON, DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs, }, - "backup_vault_arn": { - Type: schema.TypeString, - Computed: true, - }, }, } } @@ -43,17 +47,19 @@ func resourceAwsBackupVaultPolicy() *schema.Resource { func resourceAwsBackupVaultPolicyPut(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).backupconn + name := d.Get("backup_vault_name").(string) input := &backup.PutBackupVaultAccessPolicyInput{ - BackupVaultName: aws.String(d.Get("backup_vault_name").(string)), + BackupVaultName: aws.String(name), Policy: aws.String(d.Get("policy").(string)), } _, err := conn.PutBackupVaultAccessPolicy(input) + if err != nil { - return fmt.Errorf("error creating Backup Vault Policy (%s): %w", d.Id(), err) + return fmt.Errorf("error creating Backup Vault Policy (%s): %w", name, err) } - d.SetId(d.Get("backup_vault_name").(string)) + d.SetId(name) return resourceAwsBackupVaultPolicyRead(d, meta) } @@ -61,13 +67,10 @@ func resourceAwsBackupVaultPolicyPut(d *schema.ResourceData, meta interface{}) e func resourceAwsBackupVaultPolicyRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).backupconn - input := &backup.GetBackupVaultAccessPolicyInput{ - BackupVaultName: aws.String(d.Id()), - } + output, err := finder.BackupVaultAccessPolicyByName(conn, d.Id()) - resp, err := conn.GetBackupVaultAccessPolicy(input) - if isAWSErr(err, backup.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] Backup Vault Policy %s not found, removing from state", d.Id()) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Backup Vault Policy (%s) not found, removing from state", d.Id()) d.SetId("") return nil } @@ -75,9 +78,10 @@ func resourceAwsBackupVaultPolicyRead(d *schema.ResourceData, meta interface{}) if err != nil { return fmt.Errorf("error reading Backup Vault Policy (%s): %w", d.Id(), err) } - d.Set("backup_vault_name", resp.BackupVaultName) - d.Set("policy", resp.Policy) - d.Set("backup_vault_arn", resp.BackupVaultArn) + + d.Set("backup_vault_arn", output.BackupVaultArn) + d.Set("backup_vault_name", output.BackupVaultName) + d.Set("policy", output.Policy) return nil } @@ -85,15 +89,16 @@ func resourceAwsBackupVaultPolicyRead(d *schema.ResourceData, meta interface{}) func resourceAwsBackupVaultPolicyDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).backupconn - input := &backup.DeleteBackupVaultAccessPolicyInput{ + log.Printf("[DEBUG] Deleting Backup Vault Policy (%s)", d.Id()) + _, err := conn.DeleteBackupVaultAccessPolicy(&backup.DeleteBackupVaultAccessPolicyInput{ BackupVaultName: aws.String(d.Id()), + }) + + if tfawserr.ErrCodeEquals(err, backup.ErrCodeResourceNotFoundException) || tfawserr.ErrCodeEquals(err, tfbackup.ErrCodeAccessDeniedException) { + return nil } - _, err := conn.DeleteBackupVaultAccessPolicy(input) if err != nil { - if isAWSErr(err, backup.ErrCodeResourceNotFoundException, "") { - return nil - } return fmt.Errorf("error deleting Backup Vault Policy (%s): %w", d.Id(), err) } diff --git a/aws/resource_aws_backup_vault_policy_test.go b/aws/resource_aws_backup_vault_policy_test.go index 4185e4834d7..b574690a616 100644 --- a/aws/resource_aws_backup_vault_policy_test.go +++ b/aws/resource_aws_backup_vault_policy_test.go @@ -8,10 +8,12 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/backup" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/backup/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func init() { @@ -27,45 +29,37 @@ func testSweepBackupVaultPolicies(region string) error { return fmt.Errorf("Error getting client: %w", err) } conn := client.(*AWSClient).backupconn - var sweeperErrs *multierror.Error - input := &backup.ListBackupVaultsInput{} + var sweeperErrs *multierror.Error + sweepResources := make([]*testSweepResource, 0) - for { - output, err := conn.ListBackupVaults(input) - if err != nil { - if testSweepSkipSweepError(err) { - log.Printf("[WARN] Skipping Backup Vault Policies sweep for %s: %s", region, err) - return nil - } - sweeperErrs = multierror.Append(sweeperErrs, fmt.Errorf("error retrieving Backup Vault Policies: %w", err)) - return sweeperErrs.ErrorOrNil() + err = conn.ListBackupVaultsPages(input, func(page *backup.ListBackupVaultsOutput, lastPage bool) bool { + if page == nil { + return !lastPage } - if len(output.BackupVaultList) == 0 { - log.Print("[DEBUG] No Backup Vault Policies to sweep") - return nil - } + for _, vault := range page.BackupVaultList { + r := resourceAwsBackupVaultPolicy() + d := r.Data(nil) + d.SetId(aws.StringValue(vault.BackupVaultName)) - for _, rule := range output.BackupVaultList { - name := aws.StringValue(rule.BackupVaultName) - - log.Printf("[INFO] Deleting Backup Vault Policies %s", name) - _, err := conn.DeleteBackupVaultAccessPolicy(&backup.DeleteBackupVaultAccessPolicyInput{ - BackupVaultName: aws.String(name), - }) - if err != nil { - sweeperErr := fmt.Errorf("error deleting Backup Vault Policies %s: %w", name, err) - log.Printf("[ERROR] %s", sweeperErr) - sweeperErrs = multierror.Append(sweeperErrs, sweeperErr) - continue - } + sweepResources = append(sweepResources, NewTestSweepResource(r, d, client)) } - if output.NextToken == nil { - break - } - input.NextToken = output.NextToken + return !lastPage + }) + + if testSweepSkipSweepError(err) { + log.Printf("[WARN] Skipping Backup Vault Policies sweep for %s: %s", region, err) + return sweeperErrs.ErrorOrNil() + } + + if err != nil { + sweeperErrs = multierror.Append(sweeperErrs, fmt.Errorf("error listing Backup Vaults for %s: %w", region, err)) + } + + if err := testSweepResourceOrchestrator(sweepResources); err != nil { + sweeperErrs = multierror.Append(sweeperErrs, fmt.Errorf("error sweeping Backup Vault Policies for %s: %w", region, err)) } return sweeperErrs.ErrorOrNil() @@ -73,9 +67,9 @@ func testSweepBackupVaultPolicies(region string) error { func TestAccAwsBackupVaultPolicy_basic(t *testing.T) { var vault backup.GetBackupVaultAccessPolicyOutput - rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_backup_vault_policy.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSBackup(t) }, ErrorCheck: testAccErrorCheck(t, backup.EndpointsID), @@ -107,9 +101,9 @@ func TestAccAwsBackupVaultPolicy_basic(t *testing.T) { func TestAccAwsBackupVaultPolicy_disappears(t *testing.T) { var vault backup.GetBackupVaultAccessPolicyOutput - rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_backup_vault_policy.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSBackup(t) }, ErrorCheck: testAccErrorCheck(t, backup.EndpointsID), @@ -128,24 +122,49 @@ func TestAccAwsBackupVaultPolicy_disappears(t *testing.T) { }) } +func TestAccAwsBackupVaultPolicy_disappears_vault(t *testing.T) { + var vault backup.GetBackupVaultAccessPolicyOutput + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_backup_vault_policy.test" + vaultResourceName := "aws_backup_vault.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSBackup(t) }, + ErrorCheck: testAccErrorCheck(t, backup.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsBackupVaultPolicyDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBackupVaultPolicyConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsBackupVaultPolicyExists(resourceName, &vault), + testAccCheckResourceDisappears(testAccProvider, resourceAwsBackupVault(), vaultResourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func testAccCheckAwsBackupVaultPolicyDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).backupconn + for _, rs := range s.RootModule().Resources { if rs.Type != "aws_backup_vault_policy" { continue } - input := &backup.GetBackupVaultAccessPolicyInput{ - BackupVaultName: aws.String(rs.Primary.ID), - } + _, err := finder.BackupVaultAccessPolicyByName(conn, rs.Primary.ID) - resp, err := conn.GetBackupVaultAccessPolicy(input) + if tfresource.NotFound(err) { + continue + } - if err == nil { - if aws.StringValue(resp.BackupVaultName) == rs.Primary.ID { - return fmt.Errorf("Backup Plan Policies '%s' was not deleted properly", rs.Primary.ID) - } + if err != nil { + return err } + + return fmt.Errorf("Backup Vault Policy %s still exists", rs.Primary.ID) } return nil @@ -158,16 +177,19 @@ func testAccCheckAwsBackupVaultPolicyExists(name string, vault *backup.GetBackup return fmt.Errorf("Not found: %s", name) } - conn := testAccProvider.Meta().(*AWSClient).backupconn - params := &backup.GetBackupVaultAccessPolicyInput{ - BackupVaultName: aws.String(rs.Primary.ID), + if rs.Primary.ID == "" { + return fmt.Errorf("No Backup Vault Policy ID is set") } - resp, err := conn.GetBackupVaultAccessPolicy(params) + + conn := testAccProvider.Meta().(*AWSClient).backupconn + + output, err := finder.BackupVaultAccessPolicyByName(conn, rs.Primary.ID) + if err != nil { return err } - *vault = *resp + *vault = *output return nil }