Merge pull request #40049 from hashicorp/b-sagemaker-appsecuritygroup…

…mgmt sagemaker/domain: Fix ValidationException with AppSecurityGroupMangement
hashicorp · Nov 8, 2024 · e7577bb · e7577bb
2 parents ab85962 + 02f3e41
commit e7577bb
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 2 deletions.
diff --git a/.changelog/40049.txt b/.changelog/40049.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_sagemaker_domain: Fix issue causing a `ValidationException` on updates when RStudio is disabled on the domain
+```
diff --git a/internal/service/sagemaker/domain.go b/internal/service/sagemaker/domain.go
@@ -1474,7 +1474,7 @@ func resourceDomainCreate(ctx context.Context, d *schema.ResourceData, meta inte
 		Tags:                 getTagsIn(ctx),
 	}
 
-	if v, ok := d.GetOk("app_security_group_management"); ok {
+	if v, ok := d.GetOk("app_security_group_management"); ok && rstudioDomainEnabled(d.Get("domain_settings").([]interface{})) {
 		input.AppSecurityGroupManagement = awstypes.AppSecurityGroupManagement(v.(string))
 	}
 
@@ -1575,7 +1575,7 @@ func resourceDomainUpdate(ctx context.Context, d *schema.ResourceData, meta inte
 			input.AppNetworkAccessType = awstypes.AppNetworkAccessType(v.(string))
 		}
 
-		if v, ok := d.GetOk("app_security_group_management"); ok {
+		if v, ok := d.GetOk("app_security_group_management"); ok && rstudioDomainEnabled(d.Get("domain_settings").([]interface{})) {
 			input.AppSecurityGroupManagement = awstypes.AppSecurityGroupManagement(v.(string))
 		}
 
@@ -1763,6 +1763,32 @@ func expandDomainSettingsUpdate(l []interface{}) *awstypes.DomainSettingsForUpda
 	return config
 }
 
+// rstudioDomainEnabled takes domain_settings and returns true if rstudio is enabled
+func rstudioDomainEnabled(domainSettings []interface{}) bool {
+	if len(domainSettings) == 0 || domainSettings[0] == nil {
+		return false
+	}
+
+	m := domainSettings[0].(map[string]interface{})
+
+	v, ok := m["r_studio_server_pro_domain_settings"].([]interface{})
+	if !ok || len(v) < 1 {
+		return false
+	}
+
+	rsspds, ok := v[0].(map[string]interface{})
+	if !ok || len(rsspds) == 0 {
+		return false
+	}
+
+	domainExecutionRoleArn, ok := rsspds["domain_execution_role_arn"].(string)
+	if !ok || domainExecutionRoleArn == "" {
+		return false
+	}
+
+	return true
+}
+
 func expandRStudioServerProDomainSettingsUpdate(l []interface{}) *awstypes.RStudioServerProDomainSettingsForUpdate {
 	if len(l) == 0 || l[0] == nil {
 		return nil

diff --git a/internal/service/sagemaker/domain_test.go b/internal/service/sagemaker/domain_test.go
@@ -730,6 +730,37 @@ func testAccDomain_rStudioServerProDomainSettings(t *testing.T) {
 	})
 }
 
+func testAccDomain_rStudioDomainDisabledNetworkUpdate(t *testing.T) {
+	ctx := acctest.Context(t)
+	var domain sagemaker.DescribeDomainOutput
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	resourceName := "aws_sagemaker_domain.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.SageMakerServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckDomainDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccDomainConfig_rStudioDomainDisabledNetworkUpdate(rName, "PublicInternetOnly"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDomainExists(ctx, resourceName, &domain),
+					resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "0"),
+					resource.TestCheckResourceAttr(resourceName, "app_network_access_type", "PublicInternetOnly"),
+				),
+			},
+			{
+				Config: testAccDomainConfig_rStudioDomainDisabledNetworkUpdate(rName, "VpcOnly"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDomainExists(ctx, resourceName, &domain),
+					resource.TestCheckResourceAttr(resourceName, "domain_settings.#", "0"),
+					resource.TestCheckResourceAttr(resourceName, "app_network_access_type", "VpcOnly")),
+			},
+		},
+	})
+}
+
 func testAccDomain_kernelGatewayAppSettings(t *testing.T) {
 	ctx := acctest.Context(t)
 	var domain sagemaker.DescribeDomainOutput
@@ -2487,6 +2518,26 @@ resource "aws_sagemaker_domain" "test" {
 `, rName, connectURL, packageURL))
 }
 
+func testAccDomainConfig_rStudioDomainDisabledNetworkUpdate(rName, networkAccess string) string {
+	return acctest.ConfigCompose(testAccDomainConfig_baseWithLicense(rName), fmt.Sprintf(`
+resource "aws_sagemaker_domain" "test" {
+  domain_name             = %[1]q
+  auth_mode               = "IAM"
+  vpc_id                  = aws_vpc.test.id
+  subnet_ids              = aws_subnet.test[*].id
+  app_network_access_type = %[2]q
+
+  default_user_settings {
+    execution_role = aws_iam_role.test.arn
+  }
+
+  retention_policy {
+    home_efs_file_system = "Delete"
+  }
+}
+`, rName, networkAccess))
+}
+
 func testAccDomainConfig_baseWithLicense(rName string) string {
 	return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 1), fmt.Sprintf(`
 data "aws_partition" "current" {}

diff --git a/internal/service/sagemaker/sagemaker_test.go b/internal/service/sagemaker/sagemaker_test.go
@@ -74,6 +74,7 @@ func TestAccSageMaker_serial(t *testing.T) {
 			"rSessionAppSettings":                                     testAccDomain_rSessionAppSettings,
 			"rStudioServerProAppSettings":                             testAccDomain_rStudioServerProAppSettings,
 			"rStudioServerProDomainSettings":                          testAccDomain_rStudioServerProDomainSettings,
+			"rStudioDomainDisabledNetworkUpdate":                      testAccDomain_rStudioDomainDisabledNetworkUpdate,
 			"spaceSettingsKernelGatewayAppSettings":                   testAccDomain_spaceSettingsKernelGatewayAppSettings,
 			"spaceSettingsJupyterLabAppSettings":                      testAccDomain_spaceSettingsJupyterLabAppSettings,
 			"spaceSettingsSpaceStorageSettings":                       testAccDomain_spaceSettingsSpaceStorageSettings,