From 73a7d7f51737e2be4513e3f6b15f9f2a203fe5f3 Mon Sep 17 00:00:00 2001 From: tabito Date: Thu, 11 Dec 2025 01:13:49 +0900 Subject: [PATCH 1/4] VpcId and SubnetIds are always passed to the AWS API when VpcOnly --- internal/service/sagemaker/domain.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/service/sagemaker/domain.go b/internal/service/sagemaker/domain.go index 98a14818780a..a233d623bc3b 100644 --- a/internal/service/sagemaker/domain.go +++ b/internal/service/sagemaker/domain.go @@ -1566,6 +1566,11 @@ func resourceDomainUpdate(ctx context.Context, d *schema.ResourceData, meta any) input.AppNetworkAccessType = awstypes.AppNetworkAccessType(v.(string)) } + if input.AppNetworkAccessType == awstypes.AppNetworkAccessTypeVpcOnly { + input.VpcId = aws.String(d.Get(names.AttrVPCID).(string)) + input.SubnetIds = flex.ExpandStringValueSet(d.Get(names.AttrSubnetIDs).(*schema.Set)) + } + if v, ok := d.GetOk("app_security_group_management"); ok && rstudioDomainEnabled(d.Get("domain_settings").([]any)) { input.AppSecurityGroupManagement = awstypes.AppSecurityGroupManagement(v.(string)) } From 795313ee5a25f34885cd441be0827f05bb52e053 Mon Sep 17 00:00:00 2001 From: tabito Date: Thu, 11 Dec 2025 01:15:58 +0900 Subject: [PATCH 2/4] Add a test case where the resource with VpcOnly is updated --- internal/service/sagemaker/domain_test.go | 63 ++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/internal/service/sagemaker/domain_test.go b/internal/service/sagemaker/domain_test.go index 8e720ecc1e15..a61034940977 100644 --- a/internal/service/sagemaker/domain_test.go +++ b/internal/service/sagemaker/domain_test.go @@ -13,6 +13,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/sagemaker" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -272,13 +273,18 @@ func testAccDomain_sharingSettings(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccDomainConfig_sharingSettings(rName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate), + }, + }, Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(ctx, resourceName, &domain), resource.TestCheckResourceAttr(resourceName, "default_user_settings.#", "1"), resource.TestCheckResourceAttr(resourceName, "default_user_settings.0.sharing_settings.#", "1"), resource.TestCheckResourceAttr(resourceName, "default_user_settings.0.sharing_settings.0.notebook_output_option", "Allowed"), resource.TestCheckResourceAttrPair(resourceName, "default_user_settings.0.sharing_settings.0.s3_kms_key_id", "aws_kms_key.test", names.AttrKeyID), - resource.TestCheckResourceAttrSet(resourceName, "default_user_settings.0.sharing_settings.0.s3_output_path"), + resource.TestMatchResourceAttr(resourceName, "default_user_settings.0.sharing_settings.0.s3_output_path", regexache.MustCompile(`^s3://tf-acc-.+/sharing$`)), ), }, { @@ -287,6 +293,23 @@ func testAccDomain_sharingSettings(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"retention_policy"}, }, + { + Config: testAccDomainConfig_sharingSettingsUpdated(rName), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, + Check: resource.ComposeTestCheckFunc( + testAccCheckDomainExists(ctx, resourceName, &domain), + resource.TestCheckResourceAttr(resourceName, "default_user_settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "default_user_settings.0.sharing_settings.#", "1"), + resource.TestCheckResourceAttr(resourceName, "default_user_settings.0.sharing_settings.0.notebook_output_option", "Allowed"), + resource.TestCheckResourceAttrPair(resourceName, "default_user_settings.0.sharing_settings.0.s3_kms_key_id", "aws_kms_key.test", names.AttrKeyID), + resource.TestCheckResourceAttrSet(resourceName, "default_user_settings.0.sharing_settings.0.s3_output_path"), + resource.TestMatchResourceAttr(resourceName, "default_user_settings.0.sharing_settings.0.s3_output_path", regexache.MustCompile(`^s3://tf-acc-.+/sharing-updated$`)), + ), + }, }, }) } @@ -2112,6 +2135,44 @@ resource "aws_sagemaker_domain" "test" { `, rName)) } +func testAccDomainConfig_sharingSettingsUpdated(rName string) string { + return acctest.ConfigCompose(testAccDomainConfig_base(rName), fmt.Sprintf(` +resource "aws_kms_key" "test" { + description = %[1]q + deletion_window_in_days = 7 + enable_key_rotation = true +} + + +resource "aws_s3_bucket" "test" { + bucket = %[1]q + force_destroy = true +} + +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 = "VpcOnly" + + default_user_settings { + execution_role = aws_iam_role.test.arn + + sharing_settings { + notebook_output_option = "Allowed" + s3_kms_key_id = aws_kms_key.test.key_id + s3_output_path = "s3://${aws_s3_bucket.test.bucket}/sharing-updated" + } + } + + retention_policy { + home_efs_file_system = "Delete" + } +} +`, rName)) +} + func testAccDomainConfig_canvasAppSettings(rName string) string { return acctest.ConfigCompose(testAccDomainConfig_base(rName), fmt.Sprintf(` resource "aws_sagemaker_domain" "test" { From 503a4d8b66c519bb06ab12fbf6bf82a299e180b9 Mon Sep 17 00:00:00 2001 From: tabito Date: Thu, 11 Dec 2025 01:27:54 +0900 Subject: [PATCH 3/4] add changelog --- .changelog/45530.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/45530.txt diff --git a/.changelog/45530.txt b/.changelog/45530.txt new file mode 100644 index 000000000000..4ce36693d815 --- /dev/null +++ b/.changelog/45530.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_sagemaker_domain: Fix update failure when `app_network_access_type` is `VpcOnly` +``` From 7b25cf5b418266ff4bf89989391250e806e4404d Mon Sep 17 00:00:00 2001 From: tabito Date: Thu, 11 Dec 2025 02:05:36 +0900 Subject: [PATCH 4/4] Refactor acctest --- internal/service/sagemaker/domain_test.go | 50 +++-------------------- 1 file changed, 6 insertions(+), 44 deletions(-) diff --git a/internal/service/sagemaker/domain_test.go b/internal/service/sagemaker/domain_test.go index a61034940977..d6f1c7fc21b1 100644 --- a/internal/service/sagemaker/domain_test.go +++ b/internal/service/sagemaker/domain_test.go @@ -272,7 +272,7 @@ func testAccDomain_sharingSettings(t *testing.T) { CheckDestroy: testAccCheckDomainDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccDomainConfig_sharingSettings(rName), + Config: testAccDomainConfig_sharingSettings(rName, "sharing"), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate), @@ -294,7 +294,7 @@ func testAccDomain_sharingSettings(t *testing.T) { ImportStateVerifyIgnore: []string{"retention_policy"}, }, { - Config: testAccDomainConfig_sharingSettingsUpdated(rName), + Config: testAccDomainConfig_sharingSettings(rName, "sharing-updated"), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), @@ -1308,7 +1308,7 @@ func testAccDomain_defaultUserSettingsUpdated(t *testing.T) { ImportStateVerifyIgnore: []string{"retention_policy"}, }, { - Config: testAccDomainConfig_sharingSettings(rName), + Config: testAccDomainConfig_sharingSettings(rName, "sharing"), Check: resource.ComposeTestCheckFunc( testAccCheckDomainExists(ctx, resourceName, &domain), resource.TestCheckResourceAttr(resourceName, "app_network_access_type", "VpcOnly"), @@ -2097,7 +2097,7 @@ resource "aws_sagemaker_domain" "test" { `, rName, tagKey1, tagValue1, tagKey2, tagValue2)) } -func testAccDomainConfig_sharingSettings(rName string) string { +func testAccDomainConfig_sharingSettings(rName, s3KeyPrefix string) string { return acctest.ConfigCompose(testAccDomainConfig_base(rName), fmt.Sprintf(` resource "aws_kms_key" "test" { description = %[1]q @@ -2124,7 +2124,7 @@ resource "aws_sagemaker_domain" "test" { sharing_settings { notebook_output_option = "Allowed" s3_kms_key_id = aws_kms_key.test.key_id - s3_output_path = "s3://${aws_s3_bucket.test.bucket}/sharing" + s3_output_path = "s3://${aws_s3_bucket.test.bucket}/%[2]s" } } @@ -2132,45 +2132,7 @@ resource "aws_sagemaker_domain" "test" { home_efs_file_system = "Delete" } } -`, rName)) -} - -func testAccDomainConfig_sharingSettingsUpdated(rName string) string { - return acctest.ConfigCompose(testAccDomainConfig_base(rName), fmt.Sprintf(` -resource "aws_kms_key" "test" { - description = %[1]q - deletion_window_in_days = 7 - enable_key_rotation = true -} - - -resource "aws_s3_bucket" "test" { - bucket = %[1]q - force_destroy = true -} - -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 = "VpcOnly" - - default_user_settings { - execution_role = aws_iam_role.test.arn - - sharing_settings { - notebook_output_option = "Allowed" - s3_kms_key_id = aws_kms_key.test.key_id - s3_output_path = "s3://${aws_s3_bucket.test.bucket}/sharing-updated" - } - } - - retention_policy { - home_efs_file_system = "Delete" - } -} -`, rName)) +`, rName, s3KeyPrefix)) } func testAccDomainConfig_canvasAppSettings(rName string) string {