From 58c13f989c7d8438ebd65489268786a674f667cb Mon Sep 17 00:00:00 2001 From: Charlie Briggs Date: Tue, 10 Jan 2023 12:29:39 +0000 Subject: [PATCH 1/5] Support name_prefix in aws_sagemaker_endpoint_configuration resource This allows a human readable name to be used when updating aws_sagemaker_endpoint_configuration in-place Fixes https://github.com/hashicorp/terraform-provider-aws/issues/21811 --- .../sagemaker/endpoint_configuration.go | 28 ++++--- .../sagemaker/endpoint_configuration_test.go | 84 +++++++++++++++++++ internal/service/sagemaker/validate.go | 21 +++++ internal/service/sagemaker/validate_test.go | 30 +++++++ ...maker_endpoint_configuration.html.markdown | 3 +- 5 files changed, 154 insertions(+), 12 deletions(-) diff --git a/internal/service/sagemaker/endpoint_configuration.go b/internal/service/sagemaker/endpoint_configuration.go index e11b4ef42846..65bd0f6dcc48 100644 --- a/internal/service/sagemaker/endpoint_configuration.go +++ b/internal/service/sagemaker/endpoint_configuration.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/flex" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" @@ -202,11 +203,20 @@ func ResourceEndpointConfiguration() *schema.Resource { }, }, "name": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, - ValidateFunc: validName, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"name_prefix"}, + ValidateFunc: validName, + }, + "name_prefix": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ConflictsWith: []string{"name"}, + ValidateFunc: validPrefix, }, "kms_key_arn": { Type: schema.TypeString, @@ -450,12 +460,7 @@ func resourceEndpointConfigurationCreate(d *schema.ResourceData, meta interface{ defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) - var name string - if v, ok := d.GetOk("name"); ok { - name = v.(string) - } else { - name = resource.UniqueId() - } + name := create.Name(d.Get("name").(string), d.Get("name_prefix").(string)) createOpts := &sagemaker.CreateEndpointConfigInput{ EndpointConfigName: aws.String(name), @@ -511,6 +516,7 @@ func resourceEndpointConfigurationRead(d *schema.ResourceData, meta interface{}) d.Set("arn", endpointConfig.EndpointConfigArn) d.Set("name", endpointConfig.EndpointConfigName) + d.Set("name_prefix", create.NamePrefixFromName(aws.StringValue(endpointConfig.EndpointConfigName))) d.Set("kms_key_arn", endpointConfig.KmsKeyId) if err := d.Set("production_variants", flattenProductionVariants(endpointConfig.ProductionVariants)); err != nil { diff --git a/internal/service/sagemaker/endpoint_configuration_test.go b/internal/service/sagemaker/endpoint_configuration_test.go index e520a760a37c..9f563c82c385 100644 --- a/internal/service/sagemaker/endpoint_configuration_test.go +++ b/internal/service/sagemaker/endpoint_configuration_test.go @@ -51,6 +51,60 @@ func TestAccSageMakerEndpointConfiguration_basic(t *testing.T) { }) } +func TestAccSageMakerEndpointConfiguration_nameGenerated(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_sagemaker_endpoint_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, sagemaker.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckEndpointConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccEndpointConfigurationConfig_nameGenerated(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckEndpointConfigurationExists(resourceName), + acctest.CheckResourceAttrNameGenerated(resourceName, "name"), + resource.TestCheckResourceAttr(resourceName, "name_prefix", "terraform-"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccSageMakerEndpointConfiguration_namePrefix(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_sagemaker_endpoint_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, sagemaker.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckEndpointConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccEndpointConfigurationConfig_namePrefix(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckEndpointConfigurationExists(resourceName), + acctest.CheckResourceAttrNameFromPrefix(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "name_prefix", rName), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccSageMakerEndpointConfiguration_shadowProductionVariants(t *testing.T) { rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_sagemaker_endpoint_configuration.test" @@ -519,6 +573,36 @@ resource "aws_sagemaker_endpoint_configuration" "test" { `, rName)) } +func testAccEndpointConfigurationConfig_nameGenerated(rName string) string { + return acctest.ConfigCompose(testAccEndpointConfigurationConfig_base(rName), ` +resource "aws_sagemaker_endpoint_configuration" "test" { + production_variants { + variant_name = "variant-1" + model_name = aws_sagemaker_model.test.name + initial_instance_count = 2 + instance_type = "ml.t2.medium" + initial_variant_weight = 1 + } +} +`) +} + +func testAccEndpointConfigurationConfig_namePrefix(rName string) string { + return acctest.ConfigCompose(testAccEndpointConfigurationConfig_base(rName), fmt.Sprintf(` +resource "aws_sagemaker_endpoint_configuration" "test" { + name_prefix = %[1]q + + production_variants { + variant_name = "variant-1" + model_name = aws_sagemaker_model.test.name + initial_instance_count = 2 + instance_type = "ml.t2.medium" + initial_variant_weight = 1 + } +} +`, rName)) +} + func testAccEndpointConfigurationConfig_shadowProductionVariants(rName string) string { return acctest.ConfigCompose(testAccEndpointConfigurationConfig_base(rName), fmt.Sprintf(` resource "aws_sagemaker_endpoint_configuration" "test" { diff --git a/internal/service/sagemaker/validate.go b/internal/service/sagemaker/validate.go index bfe6c3079298..86ea5b040375 100644 --- a/internal/service/sagemaker/validate.go +++ b/internal/service/sagemaker/validate.go @@ -3,6 +3,8 @@ package sagemaker import ( "fmt" "regexp" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) func validEnvironment(v interface{}, k string) (ws []string, errors []error) { @@ -78,3 +80,22 @@ func validName(v interface{}, k string) (ws []string, errors []error) { } return } + +func validPrefix(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + if !regexp.MustCompile(`^[0-9A-Za-z-]+$`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "only alphanumeric characters and hyphens allowed in %q: %q", + k, value)) + } + maxLength := 63 - resource.UniqueIDSuffixLength + if len(value) > maxLength { + errors = append(errors, fmt.Errorf( + "%q cannot be longer than %d characters: %q", k, maxLength, value)) + } + if regexp.MustCompile(`^-`).MatchString(value) { + errors = append(errors, fmt.Errorf( + "%q cannot begin with a hyphen: %q", k, value)) + } + return +} diff --git a/internal/service/sagemaker/validate_test.go b/internal/service/sagemaker/validate_test.go index b1351e707669..30a4473793a2 100644 --- a/internal/service/sagemaker/validate_test.go +++ b/internal/service/sagemaker/validate_test.go @@ -33,3 +33,33 @@ func TestValidName(t *testing.T) { } } } + +func TestValidPrefix(t *testing.T) { + maxLength := 37 + validPrefixes := []string{ + "ValidSageMakerName", + "Valid-5a63Mak3r-Name", + "123-456-789", + "1234", + strings.Repeat("W", maxLength), + } + for _, v := range validPrefixes { + _, errors := validPrefix(v, "name_prefix") + if len(errors) != 0 { + t.Fatalf("%q should be a valid SageMaker prefix with maximum length %d chars: %q", v, maxLength, errors) + } + } + + invalidPrefixes := []string{ + "Invalid prefix", // blanks are not allowed + "1#{}nook", // other non-alphanumeric chars + "-nook", // cannot start with hyphen + strings.Repeat("W", maxLength+1), // length > maxLength + } + for _, v := range invalidPrefixes { + _, errors := validPrefix(v, "name_prefix") + if len(errors) == 0 { + t.Fatalf("%q should be an invalid SageMaker prefix", v) + } + } +} diff --git a/website/docs/r/sagemaker_endpoint_configuration.html.markdown b/website/docs/r/sagemaker_endpoint_configuration.html.markdown index 33cb93562b06..f6b1b8f07ede 100644 --- a/website/docs/r/sagemaker_endpoint_configuration.html.markdown +++ b/website/docs/r/sagemaker_endpoint_configuration.html.markdown @@ -37,7 +37,8 @@ The following arguments are supported: * `production_variants` - (Required) An list of ProductionVariant objects, one for each model that you want to host at this endpoint. Fields are documented below. * `kms_key_arn` - (Optional) Amazon Resource Name (ARN) of a AWS Key Management Service key that Amazon SageMaker uses to encrypt data on the storage volume attached to the ML compute instance that hosts the endpoint. -* `name` - (Optional) The name of the endpoint configuration. If omitted, Terraform will assign a random, unique name. +* `name` - (Optional) The name of the endpoint configuration. If omitted, Terraform will assign a random, unique name. Conflicts with `name_prefix`. +- `name_prefix` - (Optional) Creates a unique endpoint configuration name beginning with the specified prefix. Conflicts with `name`. * `tags` - (Optional) A mapping of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. * `data_capture_config` - (Optional) Specifies the parameters to capture input/output of SageMaker models endpoints. Fields are documented below. * `async_inference_config` - (Optional) Specifies configuration for how an endpoint performs asynchronous inference. From d3e3b6c68539509f8f357faa6aecac75ebe5bf43 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 31 Mar 2023 10:31:48 -0400 Subject: [PATCH 2/5] Add CHANGELOG entry. --- .changelog/28785.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/28785.txt diff --git a/.changelog/28785.txt b/.changelog/28785.txt new file mode 100644 index 000000000000..cdb785e76a2b --- /dev/null +++ b/.changelog/28785.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_sagemaker_endpoint_configuration: Add `name_prefix` argument +``` \ No newline at end of file From a4a7010825bbd480f579f5265cf8c4b58d930fa7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 31 Mar 2023 10:42:08 -0400 Subject: [PATCH 3/5] Fix test compilation errors. --- .../sagemaker/endpoint_configuration_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/service/sagemaker/endpoint_configuration_test.go b/internal/service/sagemaker/endpoint_configuration_test.go index e932f9e26d47..139f245a0fd4 100644 --- a/internal/service/sagemaker/endpoint_configuration_test.go +++ b/internal/service/sagemaker/endpoint_configuration_test.go @@ -55,19 +55,20 @@ func TestAccSageMakerEndpointConfiguration_basic(t *testing.T) { } func TestAccSageMakerEndpointConfiguration_nameGenerated(t *testing.T) { + ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_sagemaker_endpoint_configuration.test" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, sagemaker.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckEndpointConfigurationDestroy, + CheckDestroy: testAccCheckEndpointConfigurationDestroy(ctx), Steps: []resource.TestStep{ { Config: testAccEndpointConfigurationConfig_nameGenerated(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckEndpointConfigurationExists(resourceName), + testAccCheckEndpointConfigurationExists(ctx, resourceName), acctest.CheckResourceAttrNameGenerated(resourceName, "name"), resource.TestCheckResourceAttr(resourceName, "name_prefix", "terraform-"), ), @@ -82,19 +83,20 @@ func TestAccSageMakerEndpointConfiguration_nameGenerated(t *testing.T) { } func TestAccSageMakerEndpointConfiguration_namePrefix(t *testing.T) { + ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_sagemaker_endpoint_configuration.test" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, + PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, sagemaker.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckEndpointConfigurationDestroy, + CheckDestroy: testAccCheckEndpointConfigurationDestroy(ctx), Steps: []resource.TestStep{ { Config: testAccEndpointConfigurationConfig_namePrefix(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckEndpointConfigurationExists(resourceName), + testAccCheckEndpointConfigurationExists(ctx, resourceName), acctest.CheckResourceAttrNameFromPrefix(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "name_prefix", rName), ), From 87a7d5ca702a37209148fc4697555f3d188f554f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 31 Mar 2023 10:44:13 -0400 Subject: [PATCH 4/5] Fix markdown-lint 'MD032/blanks-around-lists Lists should be surrounded by blank lines'. --- website/docs/r/sagemaker_endpoint_configuration.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/sagemaker_endpoint_configuration.html.markdown b/website/docs/r/sagemaker_endpoint_configuration.html.markdown index 8925875a7a42..864303453ac8 100644 --- a/website/docs/r/sagemaker_endpoint_configuration.html.markdown +++ b/website/docs/r/sagemaker_endpoint_configuration.html.markdown @@ -38,7 +38,7 @@ The following arguments are supported: * `production_variants` - (Required) An list of ProductionVariant objects, one for each model that you want to host at this endpoint. Fields are documented below. * `kms_key_arn` - (Optional) Amazon Resource Name (ARN) of a AWS Key Management Service key that Amazon SageMaker uses to encrypt data on the storage volume attached to the ML compute instance that hosts the endpoint. * `name` - (Optional) The name of the endpoint configuration. If omitted, Terraform will assign a random, unique name. Conflicts with `name_prefix`. -- `name_prefix` - (Optional) Creates a unique endpoint configuration name beginning with the specified prefix. Conflicts with `name`. +* `name_prefix` - (Optional) Creates a unique endpoint configuration name beginning with the specified prefix. Conflicts with `name`. * `tags` - (Optional) A mapping of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. * `data_capture_config` - (Optional) Specifies the parameters to capture input/output of SageMaker models endpoints. Fields are documented below. * `async_inference_config` - (Optional) Specifies configuration for how an endpoint performs asynchronous inference. From 8ed3e8d30776c353ce638bad0f200604aa148521 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 31 Mar 2023 11:08:27 -0400 Subject: [PATCH 5/5] Fix golangci-lint 'paralleltest'. --- internal/service/sagemaker/validate_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/service/sagemaker/validate_test.go b/internal/service/sagemaker/validate_test.go index c5cc6607a5f1..84270e8289e6 100644 --- a/internal/service/sagemaker/validate_test.go +++ b/internal/service/sagemaker/validate_test.go @@ -37,6 +37,8 @@ func TestValidName(t *testing.T) { } func TestValidPrefix(t *testing.T) { + t.Parallel() + maxLength := 37 validPrefixes := []string{ "ValidSageMakerName",