Upstream new restore policy (#4267) (#2750)

* 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 <[email protected]>
Signed-off-by: Modular Magician <[email protected]>

Co-authored-by: Thiago Carvalho <[email protected]>

Author: modular-magician

.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`
+```

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()

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()
 

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