From fc0cdcc14f8b57216842a426c31bd9f46bf2e6df Mon Sep 17 00:00:00 2001
From: maastha <122359335+maastha@users.noreply.github.com>
Date: Thu, 6 Jun 2024 12:46:38 +0100
Subject: [PATCH] fix: Adds create error handling to push-based log export
 resource (#2324)

---
 .../service/pushbasedlogexport/resource.go    | 31 ++++++++++++++++-
 .../pushbasedlogexport/resource_test.go       | 33 +++++++++++++++++++
 .../pushbasedlogexport/state_transition.go    | 12 ++++---
 3 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/internal/service/pushbasedlogexport/resource.go b/internal/service/pushbasedlogexport/resource.go
index 81ad120399..5a20ffa19f 100644
--- a/internal/service/pushbasedlogexport/resource.go
+++ b/internal/service/pushbasedlogexport/resource.go
@@ -2,9 +2,13 @@ package pushbasedlogexport
 
 import (
 	"context"
+	"log"
 	"net/http"
+	"slices"
 	"time"
 
+	"go.mongodb.org/atlas-sdk/v20231115014/admin"
+
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 
@@ -51,7 +55,14 @@ func (r *pushBasedLogExportRS) Create(ctx context.Context, req resource.CreateRe
 	connV2 := r.Client.AtlasV2
 	projectID := tfPlan.ProjectID.ValueString()
 	if _, err := connV2.PushBasedLogExportApi.CreatePushBasedLogConfiguration(ctx, projectID, logExportConfigReq).Execute(); err != nil {
-		resp.Diagnostics.AddError("eError when creating push-based log export configuration", err.Error())
+		resp.Diagnostics.AddError("Error when creating push-based log export configuration", err.Error())
+
+		if err := unconfigureFailedPushBasedLog(ctx, connV2, projectID); err != nil {
+			resp.Diagnostics.AddError("Error when unconfiguring push-based log export configuration", err.Error())
+			return
+		}
+
+		resp.State.RemoveResource(ctx)
 		return
 	}
 
@@ -65,6 +76,13 @@ func (r *pushBasedLogExportRS) Create(ctx context.Context, req resource.CreateRe
 		retryTimeConfig(timeout, minTimeoutCreateUpdate))
 	if err != nil {
 		resp.Diagnostics.AddError("Error when creating push-based log export configuration", err.Error())
+
+		if err := unconfigureFailedPushBasedLog(ctx, connV2, projectID); err != nil {
+			resp.Diagnostics.AddError("Error when unconfiguring push-based log export configuration", err.Error())
+			return
+		}
+
+		resp.State.RemoveResource(ctx)
 		return
 	}
 
@@ -179,3 +197,14 @@ func retryTimeConfig(configuredTimeout, minTimeout time.Duration) retrystrategy.
 		Delay:      retryTimeDelay,
 	}
 }
+
+func unconfigureFailedPushBasedLog(ctx context.Context, connV2 *admin.APIClient, projectID string) error {
+	logConfig, _, _ := connV2.PushBasedLogExportApi.GetPushBasedLogConfiguration(ctx, projectID).Execute()
+	if logConfig != nil && slices.Contains(failureStates, *logConfig.State) {
+		log.Printf("[INFO] Unconfiguring push-based log export for project due to create failure: %s", projectID)
+		if _, err := connV2.PushBasedLogExportApi.DeletePushBasedLogConfiguration(ctx, projectID).Execute(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
diff --git a/internal/service/pushbasedlogexport/resource_test.go b/internal/service/pushbasedlogexport/resource_test.go
index caf4882f7d..3e25bbe489 100644
--- a/internal/service/pushbasedlogexport/resource_test.go
+++ b/internal/service/pushbasedlogexport/resource_test.go
@@ -3,6 +3,7 @@ package pushbasedlogexport_test
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -92,6 +93,38 @@ func noPrefixPathTestCase(tb testing.TB) *resource.TestCase {
 	}
 }
 
+func TestAccPushBasedLogExport_createFailure(t *testing.T) {
+	resource.Test(t, *createFailure(t))
+}
+
+func createFailure(tb testing.TB) *resource.TestCase {
+	tb.Helper()
+	var (
+		projectID = acc.ProjectIDExecution(tb)
+	)
+
+	return &resource.TestCase{
+		PreCheck:                 func() { acc.PreCheckBasic(tb) },
+		ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
+		CheckDestroy:             checkDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      pushBasedLogExportInvalidConfig(projectID),
+				ExpectError: regexp.MustCompile("CLOUD_PROVIDER_ACCESS_ROLE_NOT_FOUND"),
+			},
+		},
+	}
+}
+
+func pushBasedLogExportInvalidConfig(projectID string) string {
+	return fmt.Sprintf(`resource "mongodbatlas_push_based_log_export" "test" {
+		project_id  = %[1]q
+		bucket_name = "invalidBucket"
+		iam_role_id = "aaaaaaaaaa99999999999999"
+	}
+	`, projectID)
+}
+
 func commonChecks(s3BucketName, prefixPath string) []resource.TestCheckFunc {
 	attributes := map[string]string{
 		"bucket_name": s3BucketName,
diff --git a/internal/service/pushbasedlogexport/state_transition.go b/internal/service/pushbasedlogexport/state_transition.go
index 2bc3a50bc5..b04d8b43fd 100644
--- a/internal/service/pushbasedlogexport/state_transition.go
+++ b/internal/service/pushbasedlogexport/state_transition.go
@@ -15,12 +15,16 @@ import (
 )
 
 const (
-	ActiveState         = "ACTIVE"
-	UnconfiguredState   = "UNCONFIGURED"
-	InitiatingState     = "INITIATING"
-	BucketVerifiedState = "BUCKET_VERIFIED"
+	ActiveState                   = "ACTIVE"
+	UnconfiguredState             = "UNCONFIGURED"
+	InitiatingState               = "INITIATING"
+	BucketVerifiedState           = "BUCKET_VERIFIED"
+	BucketVerificationFailedState = "BUCKET_VERIFICATION_FAILED"
+	AssumeRoleFailedState         = "ASSUME_ROLE_FAILED"
 )
 
+var failureStates = []string{BucketVerificationFailedState, AssumeRoleFailedState}
+
 func WaitStateTransition(ctx context.Context, projectID string, client admin.PushBasedLogExportApi,
 	timeConfig retrystrategy.TimeConfig) (*admin.PushBasedLogExportProject, error) {
 	stateConf := &retry.StateChangeConf{