Merge pull request #30786 from dod38fr/b-sagemaker-segv

fix segv in sagemaker app resource

Author: ewbankkit

.changelog/30786.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_sagemaker_app: Fix crash when app is not found
+```

internal/service/sagemaker/app_test.go

@@ -6,7 +6,6 @@ import (
 	"regexp"
 	"testing"
 
-	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/sagemaker"
 	sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -248,39 +247,36 @@ func testAccCheckAppDestroy(ctx context.Context) resource.TestCheckFunc {
 				continue
 			}
 
-			userProfileOrSpaceName := ""
 			domainID := rs.Primary.Attributes["domain_id"]
 			appType := rs.Primary.Attributes["app_type"]
 			appName := rs.Primary.Attributes["app_name"]
 
+			var userProfileOrSpaceName string
 			if v, ok := rs.Primary.Attributes["user_profile_name"]; ok {
 				userProfileOrSpaceName = v
 			}
-
 			if v, ok := rs.Primary.Attributes["space_name"]; ok {
 				userProfileOrSpaceName = v
 			}
 
-			app, err := tfsagemaker.FindAppByName(ctx, conn, domainID, userProfileOrSpaceName, appType, appName)
+			_, err := tfsagemaker.FindAppByName(ctx, conn, domainID, userProfileOrSpaceName, appType, appName)
 
 			if tfresource.NotFound(err) {
 				continue
 			}
 
 			if err != nil {
-				return fmt.Errorf("reading SageMaker App (%s): %w", rs.Primary.ID, err)
+				return err
 			}
 
-			if aws.StringValue(app.AppArn) == rs.Primary.ID {
-				return fmt.Errorf("sagemaker App %q still exists", rs.Primary.ID)
-			}
+			return fmt.Errorf("SageMaker App (%s) still exists", rs.Primary.ID)
 		}
 
 		return nil
 	}
 }
 
-func testAccCheckAppExists(ctx context.Context, n string, app *sagemaker.DescribeAppOutput) resource.TestCheckFunc {
+func testAccCheckAppExists(ctx context.Context, n string, v *sagemaker.DescribeAppOutput) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
 		if !ok {
@@ -292,25 +288,26 @@ func testAccCheckAppExists(ctx context.Context, n string, app *sagemaker.Describ
 		}
 
 		conn := acctest.Provider.Meta().(*conns.AWSClient).SageMakerConn()
-		userProfileOrSpaceName := ""
+
 		domainID := rs.Primary.Attributes["domain_id"]
 		appType := rs.Primary.Attributes["app_type"]
 		appName := rs.Primary.Attributes["app_name"]
 
+		var userProfileOrSpaceName string
 		if v, ok := rs.Primary.Attributes["user_profile_name"]; ok {
 			userProfileOrSpaceName = v
 		}
-
 		if v, ok := rs.Primary.Attributes["space_name"]; ok {
 			userProfileOrSpaceName = v
 		}
 
-		resp, err := tfsagemaker.FindAppByName(ctx, conn, domainID, userProfileOrSpaceName, appType, appName)
+		output, err := tfsagemaker.FindAppByName(ctx, conn, domainID, userProfileOrSpaceName, appType, appName)
+
 		if err != nil {
 			return err
 		}
 
-		*app = *resp
+		*v = *output
 
 		return nil
 	}

internal/service/sagemaker/find.go

@@ -271,23 +271,22 @@ func FindAppImageConfigByName(ctx context.Context, conn *sagemaker.SageMaker, ap
 }
 
 func listAppsByName(ctx context.Context, conn *sagemaker.SageMaker, domainID, userProfileOrSpaceName, appType, appName string) (*sagemaker.AppDetails, error) {
-	var apps []*sagemaker.AppDetails
-
 	input := &sagemaker.ListAppsInput{
 		DomainIdEquals: aws.String(domainID),
 	}
+	var output []*sagemaker.AppDetails
 
 	err := conn.ListAppsPagesWithContext(ctx, input, func(page *sagemaker.ListAppsOutput, lastPage bool) bool {
 		if page == nil {
 			return !lastPage
 		}
 
-		for _, app := range page.Apps {
-			if app == nil {
+		for _, v := range page.Apps {
+			if v == nil {
 				continue
 			}
 
-			apps = append(apps, app)
+			output = append(output, v)
 		}
 
 		return !lastPage
@@ -297,29 +296,15 @@ func listAppsByName(ctx context.Context, conn *sagemaker.SageMaker, domainID, us
 		return nil, err
 	}
 
-	if len(apps) == 0 {
-		return nil, nil
-	}
-
-	var foundApp *sagemaker.AppDetails
-	for _, app := range apps {
-		if aws.StringValue(app.AppName) == appName &&
-			aws.StringValue(app.AppType) == appType &&
-			(aws.StringValue(app.SpaceName) == userProfileOrSpaceName ||
-				aws.StringValue(app.UserProfileName) == userProfileOrSpaceName) {
-			foundApp = app
+	for _, v := range output {
+		if aws.StringValue(v.AppName) == appName && aws.StringValue(v.AppType) == appType && (aws.StringValue(v.SpaceName) == userProfileOrSpaceName || aws.StringValue(v.UserProfileName) == userProfileOrSpaceName) {
+			return v, nil
 		}
 	}
 
-	if foundApp == nil {
-		return nil, tfresource.NewEmptyResultError(input)
-	}
-
-	return foundApp, nil
+	return nil, &retry.NotFoundError{}
 }
 
-// FindAppByName returns the domain corresponding to the specified domain id.
-// Returns nil if no domain is found.
 func FindAppByName(ctx context.Context, conn *sagemaker.SageMaker, domainID, userProfileOrSpaceName, appType, appName string) (*sagemaker.DescribeAppOutput, error) {
 	foundApp, err := listAppsByName(ctx, conn, domainID, userProfileOrSpaceName, appType, appName)
 
@@ -328,15 +313,13 @@ func FindAppByName(ctx context.Context, conn *sagemaker.SageMaker, domainID, use
 	}
 
 	input := &sagemaker.DescribeAppInput{
-		DomainId: aws.String(domainID),
-		AppType:  aws.String(appType),
 		AppName:  aws.String(appName),
+		AppType:  aws.String(appType),
+		DomainId: aws.String(domainID),
 	}
-
 	if foundApp.SpaceName != nil {
 		input.SpaceName = foundApp.SpaceName
 	}
-
 	if foundApp.UserProfileName != nil {
 		input.UserProfileName = foundApp.UserProfileName
 	}