From a16e249619d2db7e2a109902f5e327e630b68233 Mon Sep 17 00:00:00 2001 From: Trent Bitterman Date: Thu, 14 Dec 2023 22:41:58 +0000 Subject: [PATCH 1/7] fix when importing a msk storage app autoscaling policy --- internal/service/appautoscaling/policy.go | 8 ++++++++ internal/service/appautoscaling/policy_test.go | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/internal/service/appautoscaling/policy.go b/internal/service/appautoscaling/policy.go index 1c5b52c79ea..16a0e4b49aa 100644 --- a/internal/service/appautoscaling/policy.go +++ b/internal/service/appautoscaling/policy.go @@ -510,6 +510,14 @@ func ValidPolicyImportInput(id string) ([]string, error) { dimensionIx = 5 } + resourceId = strings.Join(idParts[1:dimensionIx], "/") + scalableDimension = idParts[dimensionIx] + policyName = strings.Join(idParts[dimensionIx+1:], "/") + case "kafka": + serviceNamespace = idParts[0] + // MSK resource ID contains three sections, separated by '/', so scalableDimension is at index 4 + dimensionIx := 4 + resourceId = strings.Join(idParts[1:dimensionIx], "/") scalableDimension = idParts[dimensionIx] policyName = strings.Join(idParts[dimensionIx+1:], "/") diff --git a/internal/service/appautoscaling/policy_test.go b/internal/service/appautoscaling/policy_test.go index 06563cf43d1..0e8f7438509 100644 --- a/internal/service/appautoscaling/policy_test.go +++ b/internal/service/appautoscaling/policy_test.go @@ -68,6 +68,20 @@ func TestValidatePolicyImportInput(t *testing.T) { input: "dynamodb/missing/parts", errorExpected: true, }, + { + input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", + expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3"}, + errorExpected: false, + }, + { + input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/some-autoscaler-name", + expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "some-autoscaler-name"}, + errorExpected: false, + }, + { + input: "kafka/missing/parts", + errorExpected: true, + }, } for _, tc := range testCases { From 308c878b43a51e90def3c611040d738af90b1330 Mon Sep 17 00:00:00 2001 From: Trent Bitterman Date: Thu, 14 Dec 2023 23:16:18 +0000 Subject: [PATCH 2/7] add a new changelog file --- .changelog/34934.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/34934.txt diff --git a/.changelog/34934.txt b/.changelog/34934.txt new file mode 100644 index 00000000000..093de7092f8 --- /dev/null +++ b/.changelog/34934.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_appautoscaling_policy: Fix failing imports when importing a MSK storage autoscaling policy +``` From 244bcbbee7daaf70515322feed182785c7294c06 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 12:30:03 -0400 Subject: [PATCH 3/7] r/aws_appautoscaling_policy: Tidy up. --- .../service/appautoscaling/exports_test.go | 2 + internal/service/appautoscaling/policy.go | 321 +++++++----------- .../service/appautoscaling/policy_test.go | 97 +++--- .../appautoscaling/scheduled_action.go | 1 + 4 files changed, 168 insertions(+), 253 deletions(-) diff --git a/internal/service/appautoscaling/exports_test.go b/internal/service/appautoscaling/exports_test.go index e766811783c..2e6d1d5fb39 100644 --- a/internal/service/appautoscaling/exports_test.go +++ b/internal/service/appautoscaling/exports_test.go @@ -9,5 +9,7 @@ var ( ResourceScheduledAction = resourceScheduledAction ResourceTarget = resourceTarget + FindScalingPolicyByFourPartKey = findScalingPolicyByFourPartKey FindScheduledActionByFourPartKey = findScheduledActionByFourPartKey + ValidPolicyImportInput = validPolicyImportInput ) diff --git a/internal/service/appautoscaling/policy.go b/internal/service/appautoscaling/policy.go index 76421343f9e..c415a57ce9c 100644 --- a/internal/service/appautoscaling/policy.go +++ b/internal/service/appautoscaling/policy.go @@ -10,31 +10,25 @@ import ( "log" "strconv" "strings" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/applicationautoscaling" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "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/errs/sdkdiag" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" - "github.com/hashicorp/terraform-provider-aws/names" -) - -const ( - ResNamePolicy = "Policy" ) // @SDKResource("aws_appautoscaling_policy", name="Scaling Policy") func resourcePolicy() *schema.Resource { return &schema.Resource{ - CreateWithoutTimeout: resourcePolicyCreate, + CreateWithoutTimeout: resourcePolicyPut, ReadWithoutTimeout: resourcePolicyRead, - UpdateWithoutTimeout: resourcePolicyUpdate, + UpdateWithoutTimeout: resourcePolicyPut, DeleteWithoutTimeout: resourcePolicyDelete, Importer: &schema.ResourceImporter{ @@ -303,237 +297,198 @@ func resourcePolicy() *schema.Resource { } } -func resourcePolicyCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func resourcePolicyPut(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) - params := getPutScalingPolicyInput(d) + id := d.Get("name").(string) + input := expandPutScalingPolicyInput(d) + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.PutScalingPolicyWithContext(ctx, input) + }, applicationautoscaling.ErrCodeFailedResourceAccessException, applicationautoscaling.ErrCodeObjectNotFoundException) - log.Printf("[DEBUG] ApplicationAutoScaling PutScalingPolicy: %#v", params) - var resp *applicationautoscaling.PutScalingPolicyOutput - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - resp, err = conn.PutScalingPolicyWithContext(ctx, ¶ms) - if err != nil { - if tfawserr.ErrMessageContains(err, applicationautoscaling.ErrCodeFailedResourceAccessException, "Rate exceeded") { - return retry.RetryableError(err) - } - if tfawserr.ErrMessageContains(err, applicationautoscaling.ErrCodeFailedResourceAccessException, "is not authorized to perform") { - return retry.RetryableError(err) - } - if tfawserr.ErrMessageContains(err, applicationautoscaling.ErrCodeFailedResourceAccessException, "token included in the request is invalid") { - return retry.RetryableError(err) - } - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeObjectNotFoundException) { - return retry.RetryableError(err) - } - return retry.NonRetryableError(err) - } - return nil - }) - if tfresource.TimedOut(err) { - resp, err = conn.PutScalingPolicyWithContext(ctx, ¶ms) - } if err != nil { - return create.AppendDiagError(diags, names.AppAutoScaling, create.ErrActionCreating, ResNamePolicy, d.Get("name").(string), err) + return sdkdiag.AppendErrorf(diags, "putting Application Auto Scaling Scaling Policy (%s): %s", id, err) } - d.Set("arn", resp.PolicyARN) - d.SetId(d.Get("name").(string)) - log.Printf("[INFO] ApplicationAutoScaling scaling PolicyARN: %s", d.Get("arn").(string)) + if d.IsNewResource() { + d.SetId(id) + } return append(diags, resourcePolicyRead(ctx, d, meta)...) } func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics - var p *applicationautoscaling.ScalingPolicy - - err := retry.RetryContext(ctx, 2*time.Minute, func() *retry.RetryError { - var err error - p, err = getPolicy(ctx, d, meta) - if err != nil { - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeFailedResourceAccessException) { - return retry.RetryableError(err) - } - return retry.NonRetryableError(err) - } - if d.IsNewResource() && p == nil { - return retry.RetryableError(&retry.NotFoundError{}) - } - return nil - }) - if tfresource.TimedOut(err) { - p, err = getPolicy(ctx, d, meta) - } - if err != nil { - return create.AppendDiagError(diags, names.AppAutoScaling, create.ErrActionReading, ResNamePolicy, d.Id(), err) - } + conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) + + outputRaw, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return findScalingPolicyByFourPartKey(ctx, conn, d.Get("name").(string), d.Get("service_namespace").(string), d.Get("resource_id").(string), d.Get("scalable_dimension").(string)) + }, applicationautoscaling.ErrCodeFailedResourceAccessException) - if p == nil && !d.IsNewResource() { - log.Printf("[WARN] Application AutoScaling Policy (%s) not found, removing from state", d.Id()) + if tfresource.NotFound(err) && !d.IsNewResource() { + log.Printf("[WARN] Application Auto Scaling Scaling Policy (%s) not found, removing from state", d.Id()) d.SetId("") return diags } - log.Printf("[DEBUG] Read ApplicationAutoScaling policy: %s, SP: %s, Obj: %s", d.Get("name"), d.Get("name"), p) - - var alarmARNs = make([]string, 0, len(p.Alarms)) - for _, alarm := range p.Alarms { - alarmARNs = append(alarmARNs, aws.StringValue(alarm.AlarmARN)) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading Application Auto Scaling Scaling Policy (%s): %s", d.Id(), err) } - d.Set("alarm_arns", alarmARNs) - d.Set("arn", p.PolicyARN) - d.Set("name", p.PolicyName) - d.Set("policy_type", p.PolicyType) - d.Set("resource_id", p.ResourceId) - d.Set("scalable_dimension", p.ScalableDimension) - d.Set("service_namespace", p.ServiceNamespace) - if err := d.Set("step_scaling_policy_configuration", flattenStepScalingPolicyConfiguration(p.StepScalingPolicyConfiguration)); err != nil { - return create.AppendDiagSettingError(diags, names.AppAutoScaling, ResNamePolicy, d.Id(), "step_scaling_policy_configuration", err) + output := outputRaw.(*applicationautoscaling.ScalingPolicy) + d.Set("alarm_arns", tfslices.ApplyToAll(output.Alarms, func(v *applicationautoscaling.Alarm) string { + return aws.StringValue(v.AlarmARN) + })) + d.Set("arn", output.PolicyARN) + d.Set("name", output.PolicyName) + d.Set("policy_type", output.PolicyType) + d.Set("resource_id", output.ResourceId) + d.Set("scalable_dimension", output.ScalableDimension) + d.Set("service_namespace", output.ServiceNamespace) + if err := d.Set("step_scaling_policy_configuration", flattenStepScalingPolicyConfiguration(output.StepScalingPolicyConfiguration)); err != nil { + return sdkdiag.AppendErrorf(diags, "setting step_scaling_policy_configuration: %s", err) } - if err := d.Set("target_tracking_scaling_policy_configuration", flattenTargetTrackingScalingPolicyConfiguration(p.TargetTrackingScalingPolicyConfiguration)); err != nil { - return create.AppendDiagSettingError(diags, names.AppAutoScaling, ResNamePolicy, d.Id(), "target_tracking_scaling_policy_configuration", err) + if err := d.Set("target_tracking_scaling_policy_configuration", flattenTargetTrackingScalingPolicyConfiguration(output.TargetTrackingScalingPolicyConfiguration)); err != nil { + return sdkdiag.AppendErrorf(diags, "setting target_tracking_scaling_policy_configuration: %s", err) } return diags } -func resourcePolicyUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func resourcePolicyDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) - params := getPutScalingPolicyInput(d) + input := &applicationautoscaling.DeleteScalingPolicyInput{ + PolicyName: aws.String(d.Get("name").(string)), + ResourceId: aws.String(d.Get("resource_id").(string)), + ScalableDimension: aws.String(d.Get("scalable_dimension").(string)), + ServiceNamespace: aws.String(d.Get("service_namespace").(string)), + } - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - _, err := conn.PutScalingPolicyWithContext(ctx, ¶ms) - if err != nil { - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeFailedResourceAccessException) { - return retry.RetryableError(err) - } - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeObjectNotFoundException) { - return retry.RetryableError(err) - } - return retry.NonRetryableError(err) - } - return nil - }) - if tfresource.TimedOut(err) { - _, err = conn.PutScalingPolicyWithContext(ctx, ¶ms) + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.DeleteScalingPolicyWithContext(ctx, input) + }, applicationautoscaling.ErrCodeFailedResourceAccessException) + + if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeObjectNotFoundException) { + return diags } + if err != nil { - return create.AppendDiagError(diags, names.AppAutoScaling, create.ErrActionUpdating, ResNamePolicy, d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting Application Auto Scaling Scaling Policy (%s): %s", d.Id(), err) } - return append(diags, resourcePolicyRead(ctx, d, meta)...) + return diags } -func resourcePolicyDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) - p, err := getPolicy(ctx, d, meta) +func resourcePolicyImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + parts, err := validPolicyImportInput(d.Id()) if err != nil { - return create.AppendDiagError(diags, names.AppAutoScaling, create.ErrActionDeleting, ResNamePolicy, d.Id(), err) - } - if p == nil { - return diags + return nil, err } - params := applicationautoscaling.DeleteScalingPolicyInput{ - PolicyName: aws.String(d.Get("name").(string)), - ResourceId: aws.String(d.Get("resource_id").(string)), - ScalableDimension: aws.String(d.Get("scalable_dimension").(string)), - ServiceNamespace: aws.String(d.Get("service_namespace").(string)), - } - log.Printf("[DEBUG] Deleting Application AutoScaling Policy opts: %#v", params) - err = retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - _, err = conn.DeleteScalingPolicyWithContext(ctx, ¶ms) + serviceNamespace := parts[0] + resourceID := parts[1] + scalableDimension := parts[2] + name := parts[3] - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeFailedResourceAccessException) { - return retry.RetryableError(err) - } + d.SetId(name) + d.Set("name", name) + d.Set("resource_id", resourceID) + d.Set("scalable_dimension", scalableDimension) + d.Set("service_namespace", serviceNamespace) - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeObjectNotFoundException) { - return nil - } + return []*schema.ResourceData{d}, nil +} - if err != nil { - return retry.NonRetryableError(err) - } - return nil +func findScalingPolicyByFourPartKey(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, name, serviceNamespace, resourceID, scalableDimension string) (*applicationautoscaling.ScalingPolicy, error) { + input := &applicationautoscaling.DescribeScalingPoliciesInput{ + PolicyNames: aws.StringSlice([]string{name}), + ResourceId: aws.String(resourceID), + ScalableDimension: aws.String(scalableDimension), + ServiceNamespace: aws.String(serviceNamespace), + } + + return findScalingPolicy(ctx, conn, input, func(v *applicationautoscaling.ScalingPolicy) bool { + return aws.StringValue(v.PolicyName) == name && aws.StringValue(v.ScalableDimension) == scalableDimension }) +} - if tfresource.TimedOut(err) { - _, err = conn.DeleteScalingPolicyWithContext(ctx, ¶ms) - } +func findScalingPolicy(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, input *applicationautoscaling.DescribeScalingPoliciesInput, filter tfslices.Predicate[*applicationautoscaling.ScalingPolicy]) (*applicationautoscaling.ScalingPolicy, error) { + output, err := findScalingPolicies(ctx, conn, input, filter) if err != nil { - return create.AppendDiagError(diags, names.AppAutoScaling, create.ErrActionDeleting, ResNamePolicy, d.Id(), err) + return nil, err } - return diags + return tfresource.AssertSinglePtrResult(output) } -func resourcePolicyImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - idParts, err := ValidPolicyImportInput(d.Id()) +func findScalingPolicies(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, input *applicationautoscaling.DescribeScalingPoliciesInput, filter tfslices.Predicate[*applicationautoscaling.ScalingPolicy]) ([]*applicationautoscaling.ScalingPolicy, error) { + var output []*applicationautoscaling.ScalingPolicy + + err := conn.DescribeScalingPoliciesPagesWithContext(ctx, input, func(page *applicationautoscaling.DescribeScalingPoliciesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.ScalingPolicies { + if v != nil && filter(v) { + output = append(output, v) + } + } + + return !lastPage + }) + if err != nil { - return nil, create.Error(names.AppAutoScaling, create.ErrActionImporting, ResNamePolicy, d.Id(), err) + return nil, err } - serviceNamespace := idParts[0] - resourceId := idParts[1] - scalableDimension := idParts[2] - policyName := idParts[3] - - d.Set("service_namespace", serviceNamespace) - d.Set("resource_id", resourceId) - d.Set("scalable_dimension", scalableDimension) - d.Set("name", policyName) - d.SetId(policyName) - return []*schema.ResourceData{d}, nil + return output, nil } -func ValidPolicyImportInput(id string) ([]string, error) { +func validPolicyImportInput(id string) ([]string, error) { idParts := strings.Split(id, "/") if len(idParts) < 4 { return nil, fmt.Errorf("unexpected format (%q), expected ///", id) } - var serviceNamespace, resourceId, scalableDimension, policyName string + var serviceNamespace, resourceID, scalableDimension, name string switch idParts[0] { case "dynamodb": serviceNamespace = idParts[0] - dimensionIx := 3 + dimensionIdx := 3 // DynamoDB resource ID can be "/table/tableName" or "/table/tableName/index/indexName" - if idParts[dimensionIx] == "index" { - dimensionIx = 5 + if idParts[dimensionIdx] == "index" { + dimensionIdx = 5 } - resourceId = strings.Join(idParts[1:dimensionIx], "/") - scalableDimension = idParts[dimensionIx] - policyName = strings.Join(idParts[dimensionIx+1:], "/") + resourceID = strings.Join(idParts[1:dimensionIdx], "/") + scalableDimension = idParts[dimensionIdx] + name = strings.Join(idParts[dimensionIdx+1:], "/") case "kafka": serviceNamespace = idParts[0] // MSK resource ID contains three sections, separated by '/', so scalableDimension is at index 4 - dimensionIx := 4 + dimensionIdx := 4 - resourceId = strings.Join(idParts[1:dimensionIx], "/") - scalableDimension = idParts[dimensionIx] - policyName = strings.Join(idParts[dimensionIx+1:], "/") + resourceID = strings.Join(idParts[1:dimensionIdx], "/") + scalableDimension = idParts[dimensionIdx] + name = strings.Join(idParts[dimensionIdx+1:], "/") default: serviceNamespace = idParts[0] - resourceId = strings.Join(idParts[1:len(idParts)-2], "/") + resourceID = strings.Join(idParts[1:len(idParts)-2], "/") scalableDimension = idParts[len(idParts)-2] - policyName = idParts[len(idParts)-1] + name = idParts[len(idParts)-1] } - if serviceNamespace == "" || resourceId == "" || scalableDimension == "" || policyName == "" { + if serviceNamespace == "" || resourceID == "" || scalableDimension == "" || name == "" { return nil, fmt.Errorf("unexpected format (%q), expected ///", id) } - return []string{serviceNamespace, resourceId, scalableDimension, policyName}, nil + return []string{serviceNamespace, resourceID, scalableDimension, name}, nil } // Takes the result of flatmap.Expand for an array of step adjustments and @@ -695,26 +650,26 @@ func expandPredefinedMetricSpecification(configured []interface{}) *applicationa return spec } -func getPutScalingPolicyInput(d *schema.ResourceData) applicationautoscaling.PutScalingPolicyInput { - var params = applicationautoscaling.PutScalingPolicyInput{ +func expandPutScalingPolicyInput(d *schema.ResourceData) *applicationautoscaling.PutScalingPolicyInput { + apiObject := &applicationautoscaling.PutScalingPolicyInput{ PolicyName: aws.String(d.Get("name").(string)), ResourceId: aws.String(d.Get("resource_id").(string)), } if v, ok := d.GetOk("policy_type"); ok { - params.PolicyType = aws.String(v.(string)) + apiObject.PolicyType = aws.String(v.(string)) } - if v, ok := d.GetOk("service_namespace"); ok { - params.ServiceNamespace = aws.String(v.(string)) + if v, ok := d.GetOk("scalable_dimension"); ok { + apiObject.ScalableDimension = aws.String(v.(string)) } - if v, ok := d.GetOk("scalable_dimension"); ok { - params.ScalableDimension = aws.String(v.(string)) + if v, ok := d.GetOk("service_namespace"); ok { + apiObject.ServiceNamespace = aws.String(v.(string)) } if v, ok := d.GetOk("step_scaling_policy_configuration"); ok { - params.StepScalingPolicyConfiguration = expandStepScalingPolicyConfiguration(v.([]interface{})) + apiObject.StepScalingPolicyConfiguration = expandStepScalingPolicyConfiguration(v.([]interface{})) } if l, ok := d.GetOk("target_tracking_scaling_policy_configuration"); ok { @@ -745,33 +700,11 @@ func getPutScalingPolicyInput(d *schema.ResourceData) applicationautoscaling.Put cfg.PredefinedMetricSpecification = expandPredefinedMetricSpecification(v) } - params.TargetTrackingScalingPolicyConfiguration = cfg + apiObject.TargetTrackingScalingPolicyConfiguration = cfg } } - return params -} - -func getPolicy(ctx context.Context, d *schema.ResourceData, meta interface{}) (*applicationautoscaling.ScalingPolicy, error) { - conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) - - params := applicationautoscaling.DescribeScalingPoliciesInput{ - PolicyNames: []*string{aws.String(d.Get("name").(string))}, - ResourceId: aws.String(d.Get("resource_id").(string)), - ScalableDimension: aws.String(d.Get("scalable_dimension").(string)), - ServiceNamespace: aws.String(d.Get("service_namespace").(string)), - } - - log.Printf("[DEBUG] Application AutoScaling Policy Describe Params: %#v", params) - resp, err := conn.DescribeScalingPoliciesWithContext(ctx, ¶ms) - if err != nil { - return nil, fmt.Errorf("describing scaling policies: %s", err) - } - if len(resp.ScalingPolicies) == 0 { - return nil, nil - } - - return resp.ScalingPolicies[0], nil + return apiObject } func expandStepScalingPolicyConfiguration(cfg []interface{}) *applicationautoscaling.StepScalingPolicyConfiguration { diff --git a/internal/service/appautoscaling/policy_test.go b/internal/service/appautoscaling/policy_test.go index 1e1eb68228d..248bbc775ef 100644 --- a/internal/service/appautoscaling/policy_test.go +++ b/internal/service/appautoscaling/policy_test.go @@ -11,7 +11,6 @@ import ( "testing" "time" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/applicationautoscaling" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -19,6 +18,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfappautoscaling "github.com/hashicorp/terraform-provider-aws/internal/service/appautoscaling" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -559,7 +559,7 @@ resource "aws_appautoscaling_policy" "metric_math_test" { `, rName)) } -func testAccCheckPolicyExists(ctx context.Context, n string, policy *applicationautoscaling.ScalingPolicy) resource.TestCheckFunc { +func testAccCheckPolicyExists(ctx context.Context, n string, v *applicationautoscaling.ScalingPolicy) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -567,21 +567,14 @@ func testAccCheckPolicyExists(ctx context.Context, n string, policy *application } conn := acctest.Provider.Meta().(*conns.AWSClient).AppAutoScalingConn(ctx) - params := &applicationautoscaling.DescribeScalingPoliciesInput{ - PolicyNames: []*string{aws.String(rs.Primary.ID)}, - ResourceId: aws.String(rs.Primary.Attributes["resource_id"]), - ScalableDimension: aws.String(rs.Primary.Attributes["scalable_dimension"]), - ServiceNamespace: aws.String(rs.Primary.Attributes["service_namespace"]), - } - resp, err := conn.DescribeScalingPoliciesWithContext(ctx, params) + + output, err := tfappautoscaling.FindScalingPolicyByFourPartKey(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) + if err != nil { return err } - if len(resp.ScalingPolicies) == 0 { - return fmt.Errorf("ScalingPolicy %s not found", rs.Primary.ID) - } - *policy = *resp.ScalingPolicies[0] + *v = *output return nil } @@ -592,19 +585,21 @@ func testAccCheckPolicyDestroy(ctx context.Context) resource.TestCheckFunc { conn := acctest.Provider.Meta().(*conns.AWSClient).AppAutoScalingConn(ctx) for _, rs := range s.RootModule().Resources { - params := applicationautoscaling.DescribeScalingPoliciesInput{ - ServiceNamespace: aws.String(rs.Primary.Attributes["service_namespace"]), - PolicyNames: []*string{aws.String(rs.Primary.ID)}, + if rs.Type != "aws_appautoscaling_policy" { + continue } - resp, err := conn.DescribeScalingPoliciesWithContext(ctx, ¶ms) + _, err := tfappautoscaling.FindScalingPolicyByFourPartKey(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) + + if tfresource.NotFound(err) { + continue + } - if err == nil { - if len(resp.ScalingPolicies) != 0 && - *resp.ScalingPolicies[0].PolicyName == rs.Primary.ID { - return fmt.Errorf("Application autoscaling policy still exists: %s", rs.Primary.ID) - } + if err != nil { + return err } + + return fmt.Errorf("Application Auto Scaling Scaling Policy %s still exists", rs.Primary.ID) } return nil @@ -670,26 +665,11 @@ resource "aws_appautoscaling_policy" "test" { `, rName) } -func testAccPolicyConfig_spotFleetRequest(randPolicyName, validUntil string) string { - return fmt.Sprintf(` -data "aws_ami" "amzn-ami-minimal-hvm-ebs" { - most_recent = true - owners = ["amazon"] - - filter { - name = "name" - values = ["amzn-ami-minimal-hvm-*"] - } - - filter { - name = "root-device-type" - values = ["ebs"] - } -} - +func testAccPolicyConfig_spotFleetRequest(rName, validUntil string) string { + return acctest.ConfigCompose(acctest.ConfigLatestAmazonLinux2HVMEBSX8664AMI(), fmt.Sprintf(` data "aws_partition" "current" {} -resource "aws_iam_role" "fleet_role" { +resource "aws_iam_role" "test" { assume_role_policy = < Date: Mon, 18 Mar 2024 12:31:46 -0400 Subject: [PATCH 4/7] Tweak CHANGELOG entry. --- .changelog/34934.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/34934.txt b/.changelog/34934.txt index 093de7092f8..06a9319ced5 100644 --- a/.changelog/34934.txt +++ b/.changelog/34934.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_appautoscaling_policy: Fix failing imports when importing a MSK storage autoscaling policy +resource/aws_appautoscaling_policy: Fix errors when importing an MSK storage autoscaling policy ``` From 2eb44f58e72e61f19fb79c40130a19fde05b5c5c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 13:04:17 -0400 Subject: [PATCH 5/7] Fix providerlint 'AWSAT003' and 'AWSAT005'. --- internal/service/appautoscaling/policy_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/appautoscaling/policy_test.go b/internal/service/appautoscaling/policy_test.go index 248bbc775ef..e36c9ba4386 100644 --- a/internal/service/appautoscaling/policy_test.go +++ b/internal/service/appautoscaling/policy_test.go @@ -70,13 +70,13 @@ func TestValidatePolicyImportInput(t *testing.T) { errorExpected: true, }, { - input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", - expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3"}, + input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", //lintignore:AWSAT003,AWSAT005 + expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3"}, //lintignore:AWSAT003,AWSAT005 errorExpected: false, }, { - input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/some-autoscaler-name", - expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "some-autoscaler-name"}, + input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/some-autoscaler-name", //lintignore:AWSAT003,AWSAT005 + expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "some-autoscaler-name"}, //lintignore:AWSAT003,AWSAT005 errorExpected: false, }, { From 8a4837e29289982041297826b805d101b3bbbf8d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 13:05:51 -0400 Subject: [PATCH 6/7] Fix 'TestAccAppAutoScalingPolicy_DynamoDB_table'. --- .../service/appautoscaling/policy_test.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/service/appautoscaling/policy_test.go b/internal/service/appautoscaling/policy_test.go index e36c9ba4386..acc260f9b1f 100644 --- a/internal/service/appautoscaling/policy_test.go +++ b/internal/service/appautoscaling/policy_test.go @@ -287,8 +287,8 @@ func TestAccAppAutoScalingPolicy_spotFleetRequest(t *testing.T) { func TestAccAppAutoScalingPolicy_DynamoDB_table(t *testing.T) { ctx := acctest.Context(t) var policy applicationautoscaling.ScalingPolicy - - randPolicyName := fmt.Sprintf("test-appautoscaling-policy-%s", sdkacctest.RandString(5)) + resourceName := "aws_appautoscaling_policy.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -297,19 +297,19 @@ func TestAccAppAutoScalingPolicy_DynamoDB_table(t *testing.T) { CheckDestroy: testAccCheckPolicyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccPolicyConfig_dynamoDB(randPolicyName), + Config: testAccPolicyConfig_dynamoDB(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyExists(ctx, "aws_appautoscaling_policy.dynamo_test", &policy), - resource.TestCheckResourceAttr("aws_appautoscaling_policy.dynamo_test", "name", fmt.Sprintf("DynamoDBWriteCapacityUtilization:table/%s", randPolicyName)), - resource.TestCheckResourceAttr("aws_appautoscaling_policy.dynamo_test", "policy_type", "TargetTrackingScaling"), - resource.TestCheckResourceAttr("aws_appautoscaling_policy.dynamo_test", "service_namespace", "dynamodb"), - resource.TestCheckResourceAttr("aws_appautoscaling_policy.dynamo_test", "scalable_dimension", "dynamodb:table:WriteCapacityUnits"), + testAccCheckPolicyExists(ctx, resourceName, &policy), + resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("DynamoDBWriteCapacityUtilization:table/%s", rName)), + resource.TestCheckResourceAttr(resourceName, "policy_type", "TargetTrackingScaling"), + resource.TestCheckResourceAttr(resourceName, "service_namespace", "dynamodb"), + resource.TestCheckResourceAttr(resourceName, "scalable_dimension", "dynamodb:table:WriteCapacityUnits"), ), }, { - ResourceName: "aws_appautoscaling_policy.dynamo_test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccPolicyImportStateIdFunc("aws_appautoscaling_policy.dynamo_test"), + ImportStateIdFunc: testAccPolicyImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, From 793a816a9202d537950f574c08eca430d3f9c45d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 13:45:46 -0400 Subject: [PATCH 7/7] providerlint fixes. --- internal/service/appautoscaling/policy_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/service/appautoscaling/policy_test.go b/internal/service/appautoscaling/policy_test.go index acc260f9b1f..2f73b08e770 100644 --- a/internal/service/appautoscaling/policy_test.go +++ b/internal/service/appautoscaling/policy_test.go @@ -25,6 +25,7 @@ import ( func TestValidatePolicyImportInput(t *testing.T) { t.Parallel() + // lintignore:AWSAT003,AWSAT005 testCases := []struct { input string errorExpected bool @@ -70,13 +71,13 @@ func TestValidatePolicyImportInput(t *testing.T) { errorExpected: true, }, { - input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", //lintignore:AWSAT003,AWSAT005 - expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3"}, //lintignore:AWSAT003,AWSAT005 + input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", + expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "KafkaBrokerStorageUtilization-scaling-policy:arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3"}, errorExpected: false, }, { - input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/some-autoscaler-name", //lintignore:AWSAT003,AWSAT005 - expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "some-autoscaler-name"}, //lintignore:AWSAT003,AWSAT005 + input: "kafka/arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3/kafka:broker-storage:VolumeSize/some-autoscaler-name", + expected: []string{"kafka", "arn:aws:kafka:us-west-2:123456789012:cluster/example/279c0212-d057-4dba-9aa9-1c4e5a25bfc7-3", "kafka:broker-storage:VolumeSize", "some-autoscaler-name"}, errorExpected: false, }, {