feat: handling of "not found" resources in READ operations (#518)

SAP · Nov 8, 2023 · 376e3da · 376e3da
1 parent c88e25c
commit 376e3da
Show file tree

Hide file tree

Showing 16 changed files with 32 additions and 1,181 deletions.
diff --git a/internal/provider/fixtures/resource_subaccount_trust_configuration.error_notfound.yaml b/internal/provider/fixtures/resource_subaccount_trust_configuration.error_notfound.yaml
diff --git a/internal/provider/helper.go b/internal/provider/helper.go
@@ -26,7 +26,7 @@ func timeToValue(t time.Time) types.String {
 }
 
 func handleReadErrors(ctx context.Context, rawRes btpcli.CommandResponse, resp *resource.ReadResponse, err error, resLogName string) {
-	// Treat HTTP 404 Not Found status as a signal to recreate resource
+	// Treat HTTP 404 Not Found status as a signal to recreate resource see https://developer.hashicorp.com/terraform/plugin/framework/resources/read#recommendations
 	if rawRes.StatusCode == 404 {
 		resp.State.RemoveResource(ctx)
 	} else {

diff --git a/internal/provider/resource_directory.go b/internal/provider/resource_directory.go
@@ -184,9 +184,9 @@ func (rs *directoryResource) Read(ctx context.Context, req resource.ReadRequest,
 		return
 	}
 
-	cliRes, _, err := rs.cli.Accounts.Directory.Get(ctx, state.ID.ValueString())
+	cliRes, rawRes, err := rs.cli.Accounts.Directory.Get(ctx, state.ID.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Directory", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Directory")
 		return
 	}
 

diff --git a/internal/provider/resource_directory_entitlement.go b/internal/provider/resource_directory_entitlement.go
@@ -156,14 +156,16 @@ func (rs *directoryEntitlementResource) Read(ctx context.Context, req resource.R
 		return
 	}
 
-	entitlement, _, err := rs.cli.Accounts.Entitlement.GetEntitledByDirectory(ctx, state.DirectoryId.ValueString(), state.ServiceName.ValueString(), state.PlanName.ValueString())
+	entitlement, rawRes, err := rs.cli.Accounts.Entitlement.GetEntitledByDirectory(ctx, state.DirectoryId.ValueString(), state.ServiceName.ValueString(), state.PlanName.ValueString())
+
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Entitlement (Directory)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Entitlement (Directory)")
 		return
 	}
 
 	if entitlement == nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Entitlement (Directory)", "Resource not found")
+		// Treat "Not Found" as a signal to recreate resource see https://developer.hashicorp.com/terraform/plugin/framework/resources/read#recommendations
+		resp.State.RemoveResource(ctx)
 		return
 	}
 

diff --git a/internal/provider/resource_directory_role_collection.go b/internal/provider/resource_directory_role_collection.go
@@ -132,9 +132,9 @@ func (rs *directoryRoleCollectionType) Read(ctx context.Context, req resource.Re
 		return
 	}
 
-	cliRes, _, err := rs.cli.Security.RoleCollection.GetByDirectory(ctx, state.DirectoryId.ValueString(), state.Name.ValueString())
+	cliRes, rawRes, err := rs.cli.Security.RoleCollection.GetByDirectory(ctx, state.DirectoryId.ValueString(), state.Name.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Role Collection (Directory)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Role Collection (Directory)")
 		return
 	}
 

diff --git a/internal/provider/resource_globalaccount_resource_provider.go b/internal/provider/resource_globalaccount_resource_provider.go
@@ -111,9 +111,9 @@ func (rs *resourceGlobalaccountProviderResource) Read(ctx context.Context, req r
 		return
 	}
 
-	cliRes, _, err := rs.cli.Accounts.ResourceProvider.Get(ctx, state.Provider.ValueString(), state.TechnicalName.ValueString())
+	cliRes, rawRes, err := rs.cli.Accounts.ResourceProvider.Get(ctx, state.Provider.ValueString(), state.TechnicalName.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Resource Provider (Global Account)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Resource Provider (Global Account)")
 		return
 	}
 

diff --git a/internal/provider/resource_globalaccount_role_collection.go b/internal/provider/resource_globalaccount_role_collection.go
@@ -123,9 +123,9 @@ func (rs *globalaccountRoleCollectionResource) Read(ctx context.Context, req res
 		return
 	}
 
-	cliRes, _, err := rs.cli.Security.RoleCollection.GetByGlobalAccount(ctx, state.Name.ValueString())
+	cliRes, rawRes, err := rs.cli.Security.RoleCollection.GetByGlobalAccount(ctx, state.Name.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Role Collection (Global Account)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Role Collection (Global Account)")
 		return
 	}
 

diff --git a/internal/provider/resource_globalaccount_trust_configuration.go b/internal/provider/resource_globalaccount_trust_configuration.go
@@ -147,9 +147,9 @@ func (rs *globalaccountTrustConfigurationResource) Read(ctx context.Context, req
 		return
 	}
 
-	cliRes, _, err := rs.cli.Security.Trust.GetByGlobalAccount(ctx, state.Origin.ValueString())
+	cliRes, rawRes, err := rs.cli.Security.Trust.GetByGlobalAccount(ctx, state.Origin.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Trust Configuration (Global Account)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Trust Configuration (Global Account)")
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount.go b/internal/provider/resource_subaccount.go
@@ -207,9 +207,9 @@ func (rs *subaccountResource) Read(ctx context.Context, req resource.ReadRequest
 		return
 	}
 
-	cliRes, _, err := rs.cli.Accounts.Subaccount.Get(ctx, data.ID.ValueString())
+	cliRes, rawRes, err := rs.cli.Accounts.Subaccount.Get(ctx, data.ID.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Subaccount", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Subaccount")
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount_entitlement.go b/internal/provider/resource_subaccount_entitlement.go
@@ -159,15 +159,16 @@ func (rs *subaccountEntitlementResource) Read(ctx context.Context, req resource.
 	subaccountData, _, _ := rs.cli.Accounts.Subaccount.Get(ctx, state.SubaccountId.ValueString())
 	parentId, isParentGlobalAccount := determineParentId(rs.cli, ctx, subaccountData.ParentGUID)
 
-	entitlement, _, err := rs.cli.Accounts.Entitlement.GetAssignedBySubaccount(ctx, state.SubaccountId.ValueString(), state.ServiceName.ValueString(), state.PlanName.ValueString(), isParentGlobalAccount, parentId)
+	entitlement, rawRes, err := rs.cli.Accounts.Entitlement.GetAssignedBySubaccount(ctx, state.SubaccountId.ValueString(), state.ServiceName.ValueString(), state.PlanName.ValueString(), isParentGlobalAccount, parentId)
 
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Entitlement (Subaccount)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Entitlement (Subaccount)")
 		return
 	}
 
 	if entitlement == nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Entitlement (Subaccount)", "Resource not found")
+		// Treat "Not Found" as a signal to recreate resource see https://developer.hashicorp.com/terraform/plugin/framework/resources/read#recommendations
+		resp.State.RemoveResource(ctx)
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount_environment_instance.go b/internal/provider/resource_subaccount_environment_instance.go
@@ -206,9 +206,9 @@ func (rs *subaccountEnvironmentInstanceResource) Read(ctx context.Context, req r
 
 	timeoutsLocal := state.Timeouts
 
-	cliRes, _, err := rs.cli.Accounts.EnvironmentInstance.Get(ctx, state.SubaccountId.ValueString(), state.Id.ValueString())
+	cliRes, rawRes, err := rs.cli.Accounts.EnvironmentInstance.Get(ctx, state.SubaccountId.ValueString(), state.Id.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Environment Instance (Subaccount)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Environment Instance (Subaccount)")
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount_role_collection.go b/internal/provider/resource_subaccount_role_collection.go
@@ -132,9 +132,9 @@ func (rs *subaccountRoleCollectionResource) Read(ctx context.Context, req resour
 		return
 	}
 
-	cliRes, _, err := rs.cli.Security.RoleCollection.GetBySubaccount(ctx, state.SubaccountId.ValueString(), state.Name.ValueString())
+	cliRes, rawRes, err := rs.cli.Security.RoleCollection.GetBySubaccount(ctx, state.SubaccountId.ValueString(), state.Name.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Role Collection (Subaccount)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Role Collection (Subaccount)")
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount_service_binding.go b/internal/provider/resource_subaccount_service_binding.go
@@ -141,9 +141,9 @@ func (rs *subaccountServiceBindingResource) Read(ctx context.Context, req resour
 		return
 	}
 
-	cliRes, _, err := rs.cli.Services.Binding.GetById(ctx, state.SubaccountId.ValueString(), state.Id.ValueString())
+	cliRes, rawRes, err := rs.cli.Services.Binding.GetById(ctx, state.SubaccountId.ValueString(), state.Id.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Service Binding (Subaccount)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Service Binding (Subaccount)")
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount_service_instance.go b/internal/provider/resource_subaccount_service_instance.go
@@ -145,9 +145,9 @@ func (rs *subaccountServiceInstanceResource) Read(ctx context.Context, req resou
 	}
 	timeoutsLocal := state.Timeouts
 
-	cliRes, _, err := rs.cli.Services.Instance.GetById(ctx, state.SubaccountId.ValueString(), state.Id.ValueString())
+	cliRes, rawRes, err := rs.cli.Services.Instance.GetById(ctx, state.SubaccountId.ValueString(), state.Id.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Service Instance (Subaccount)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Service Instance (Subaccount)")
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount_subscription.go b/internal/provider/resource_subaccount_subscription.go
@@ -192,9 +192,9 @@ func (rs *subaccountSubscriptionResource) Read(ctx context.Context, req resource
 		return
 	}
 
-	cliRes, _, err := rs.cli.Accounts.Subscription.Get(ctx, state.SubaccountId.ValueString(), state.AppName.ValueString(), state.PlanName.ValueString())
+	cliRes, rawRes, err := rs.cli.Accounts.Subscription.Get(ctx, state.SubaccountId.ValueString(), state.AppName.ValueString(), state.PlanName.ValueString())
 	if err != nil {
-		resp.Diagnostics.AddError("API Error Reading Resource Subscription (Subaccount)", fmt.Sprintf("%s", err))
+		handleReadErrors(ctx, rawRes, resp, err, "Resource Subscription (Subaccount)")
 		return
 	}
 

diff --git a/internal/provider/resource_subaccount_trust_configuration_test.go b/internal/provider/resource_subaccount_trust_configuration_test.go
@@ -1,7 +1,6 @@
 package provider
 
 import (
-	"errors"
 	"fmt"
 	"regexp"
 	"testing"
@@ -125,42 +124,6 @@ func TestResourceSubaccountTrustConfiguration(t *testing.T) {
 		})
 	})
 
-	t.Run("happy path - plan recreate if resource doesn't exist anymore", func(t *testing.T) {
-		rec, user := setupVCR(t, "fixtures/resource_subaccount_trust_configuration.error_notfound")
-		defer stopQuietly(rec)
-
-		resource.Test(t, resource.TestCase{
-			IsUnitTest:               true,
-			ProtoV6ProviderFactories: getProviders(rec.GetDefaultClient()),
-			Steps: []resource.TestStep{
-				{
-					Config: hclProviderFor(user) + hclResourceSubaccountTrustConfigurationMinimal("uut", "ef23ace8-6ade-4d78-9c1f-8df729548bbf", "terraformint.accounts400.ondemand.com"),
-					Check: resource.ComposeAggregateTestCheckFunc(
-						resource.TestMatchResourceAttr("btp_subaccount_trust_configuration.uut", "subaccount_id", regexpValidUUID),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "identity_provider", "terraformint.accounts400.ondemand.com"),
-						resource.TestCheckNoResourceAttr("btp_subaccount_trust_configuration.uut", "domain"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "name", "Custom IAS tenant"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "description", "IAS tenant terraformint.accounts400.ondemand.com (OpenID Connect)"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "link_text", "terraformint.accounts400.ondemand.com"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "available_for_user_logon", "true"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "auto_create_shadow_users", "true"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "origin", "sap.custom"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "id", "sap.custom"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "type", "Application"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "protocol", "OpenID Connect"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "status", "active"),
-						resource.TestCheckResourceAttr("btp_subaccount_trust_configuration.uut", "read_only", "false"),
-					),
-				},
-				{
-					RefreshState:       true,
-					ExpectNonEmptyPlan: true,
-					Check:              verifyNoResourceState("btp_subaccount_trust_configuration.uut"),
-				},
-			},
-		})
-	})
-
 	t.Run("error path - import failure", func(t *testing.T) {
 		rec, user := setupVCR(t, "fixtures/resource_subaccount_trust_configuration.import_error")
 		defer stopQuietly(rec)
@@ -237,13 +200,3 @@ func getTrustConfigIdForImport(resourceName string) resource.ImportStateIdFunc {
 		return fmt.Sprintf("%s,%s", "ef23ace8-6ade-4d78-9c1f-8df729548bbf", rs.Primary.ID), nil
 	}
 }
-
-func verifyNoResourceState(resource string) resource.TestCheckFunc {
-	return func(state *terraform.State) error {
-		_, ok := state.RootModule().Resources[resource]
-		if ok {
-			return errors.New("expect State to be cleaned up")
-		}
-		return nil
-	}
-}