From 7b73103810b8c1f57becbd416e9cf2f76713b4bc Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Mon, 30 Nov 2020 21:09:09 +0000 Subject: [PATCH] Upstream new restore policy (#4267) * Adds REVERT_AND_IGNORE description and prefix on log lines with printf * Fixes typo * Adds new restore policy REVERT_AND_IGNORE_FAILURE in order to ignore errors returned on the revert during the resource destroy * Adds new test for new restore policy. This function should not check revert because the policy says to ignore. I have injected an error on the undelete function to simulate API error and it worked as expected. Worth to mention that on day to day it is just testing it overall behavior when the restore_policy is REVERT_AND_IGNORE_FAILURE * Reordering error handling code, no functional changes Co-authored-by: Thiago Carvalho Signed-off-by: Modular Magician --- .changelog/4267.txt | 3 ++ ...google_project_default_service_accounts.go | 27 +++++++++++++----- ...e_project_default_service_accounts_test.go | 28 +++++++++++++++++++ ...ect_default_service_accounts.html.markdown | 5 +++- 4 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 .changelog/4267.txt diff --git a/.changelog/4267.txt b/.changelog/4267.txt new file mode 100644 index 0000000000..ec68d78b7c --- /dev/null +++ b/.changelog/4267.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +project: added new restore_policy `REVERT_AND_IGNORE_FAILURE` to `google_project_default_service_accounts` +``` diff --git a/google-beta/resource_google_project_default_service_accounts.go b/google-beta/resource_google_project_default_service_accounts.go index 618fa47144..69ff5e2f74 100644 --- a/google-beta/resource_google_project_default_service_accounts.go +++ b/google-beta/resource_google_project_default_service_accounts.go @@ -2,6 +2,7 @@ package google import ( "fmt" + "log" "strings" "time" @@ -48,9 +49,9 @@ func resourceGoogleProjectDefaultServiceAccounts() *schema.Resource { Type: schema.TypeString, Optional: true, Default: "REVERT", - ValidateFunc: validation.StringInSlice([]string{"NONE", "REVERT"}, false), + ValidateFunc: validation.StringInSlice([]string{"NONE", "REVERT", "REVERT_AND_IGNORE_FAILURE"}, false), Description: `The action to be performed in the default service accounts on the resource destroy. - Valid values are NONE and REVERT. If set to REVERT it will attempt to restore all default SAs but in the DEPRIVILEGE action.`, + Valid values are NONE, REVERT and REVERT_AND_IGNORE_FAILURE. It is applied for any action but in the DEPRIVILEGE.`, }, "service_accounts": { Type: schema.TypeMap, @@ -67,7 +68,7 @@ func resourceGoogleProjectDefaultServiceAccountsDoAction(d *schema.ResourceData, if err != nil { return err } - + restorePolicy := d.Get("restore_policy").(string) serviceAccountSelfLink := fmt.Sprintf("projects/%s/serviceAccounts/%s", project, uniqueID) switch action { case "DELETE": @@ -77,8 +78,14 @@ func resourceGoogleProjectDefaultServiceAccountsDoAction(d *schema.ResourceData, } case "UNDELETE": _, err := config.NewIamClient(userAgent).Projects.ServiceAccounts.Undelete(serviceAccountSelfLink, &iam.UndeleteServiceAccountRequest{}).Do() - if err != nil { - return fmt.Errorf("cannot undelete service account %s: %v", serviceAccountSelfLink, err) + errExpected := restorePolicy == "REVERT_AND_IGNORE_FAILURE" + errReceived := err != nil + if errReceived { + if !errExpected { + return fmt.Errorf("cannot undelete service account %s: %v", serviceAccountSelfLink, err) + } + log.Printf("cannot undelete service account %s: %v", serviceAccountSelfLink, err) + log.Printf("restore policy is %s... ignoring error", restorePolicy) } case "DISABLE": _, err := config.NewIamClient(userAgent).Projects.ServiceAccounts.Disable(serviceAccountSelfLink, &iam.DisableServiceAccountRequest{}).Do() @@ -87,8 +94,14 @@ func resourceGoogleProjectDefaultServiceAccountsDoAction(d *schema.ResourceData, } case "ENABLE": _, err := config.NewIamClient(userAgent).Projects.ServiceAccounts.Enable(serviceAccountSelfLink, &iam.EnableServiceAccountRequest{}).Do() - if err != nil { - return fmt.Errorf("cannot enable service account %s: %v", serviceAccountSelfLink, err) + errReceived := err != nil + errExpected := restorePolicy == "REVERT_AND_IGNORE_FAILURE" + if errReceived { + if !errExpected { + return fmt.Errorf("cannot enable service account %s: %v", serviceAccountSelfLink, err) + } + log.Printf("cannot enable service account %s: %v", serviceAccountSelfLink, err) + log.Printf("restore policy is %s... ignoring error", restorePolicy) } case "DEPRIVILEGE": iamPolicy, err := config.NewResourceManagerClient(userAgent).Projects.GetIamPolicy(project, &cloudresourcemanager.GetIamPolicyRequest{}).Do() diff --git a/google-beta/resource_google_project_default_service_accounts_test.go b/google-beta/resource_google_project_default_service_accounts_test.go index 10ae316a88..122f6ab7ea 100644 --- a/google-beta/resource_google_project_default_service_accounts_test.go +++ b/google-beta/resource_google_project_default_service_accounts_test.go @@ -109,6 +109,34 @@ func TestAccResourceGoogleProjectDefaultServiceAccountsDelete(t *testing.T) { }) } +func TestAccResourceGoogleProjectDefaultServiceAccountsDeleteRevertIgnoreFailure(t *testing.T) { + t.Parallel() + + org := getTestOrgFromEnv(t) + project := fmt.Sprintf("tf-project-%d", randInt(t)) + billingAccount := getTestBillingAccountFromEnv(t) + action := "DELETE" + restorePolicy := "REVERT_AND_IGNORE_FAILURE" + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCheckGoogleProjectDefaultServiceAccountsAdvanced(org, project, billingAccount, action, restorePolicy), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("google_project_default_service_accounts.acceptance", "id", "projects/"+project), + resource.TestCheckResourceAttrSet("google_project_default_service_accounts.acceptance", "project"), + resource.TestCheckResourceAttr("google_project_default_service_accounts.acceptance", "action", action), + resource.TestCheckResourceAttrSet("google_project_default_service_accounts.acceptance", "project"), + sleepInSecondsForTest(10), + testAccCheckGoogleProjectDefaultServiceAccountsChanges(t, project, action), + ), + }, + }, + }) +} + func TestAccResourceGoogleProjectDefaultServiceAccountsDeprivilege(t *testing.T) { t.Parallel() diff --git a/website/docs/r/google_project_default_service_accounts.html.markdown b/website/docs/r/google_project_default_service_accounts.html.markdown index 5cec6f1a06..41a6972531 100644 --- a/website/docs/r/google_project_default_service_accounts.html.markdown +++ b/website/docs/r/google_project_default_service_accounts.html.markdown @@ -44,7 +44,10 @@ The following arguments are supported: - `action` - (Required) The action to be performed in the default service accounts. Valid values are: `DEPRIVILEGE`, `DELETE`, `DISABLE`. Note that `DEPRIVILEGE` action will ignore the REVERT configuration in the restore_policy -- `restore_policy` - (Optional) The action to be performed in the default service accounts on the resource destroy. Valid values are `NONE` and `REVERT`. If set to `REVERT` it will attempt to restore all default SAs but in the `DEPRIVILEGE` action. +- `restore_policy` - (Optional) The action to be performed in the default service accounts on the resource destroy. + Valid values are NONE, REVERT and REVERT_AND_IGNORE_FAILURE. It is applied for any action but in the DEPRIVILEGE. + If set to REVERT it attempts to restore all default SAs but the DEPRIVILEGE action. + If set to REVERT_AND_IGNORE_FAILURE it is the same behavior as REVERT but ignores errors returned by the API. ## Attributes Reference