resource/autoscaling_policy: fix conditional logic based on policy type

[loivis]

fix #3715
fix #3689

Issue with current approach is that even user doesn't set the argument, it will be set to zero in terraform state from aws api response as placeholder. Later on during update, zero value is used thus trigger the error. One way to fix is to add conditional logic in read function as well. I'm not feeling comfortable with this because multiple conditions are introduced to handle the same situation.

Instead, I'm changing the logic in the same place to set arguments only for valid policy types. With checking policy type before setting input parameter, it's also easily compare each parameter with api specification.

I didn't figure out how to distinguish zero value between user input and terraform state value in current logic but that's not the focus here.

[loivis]

⎇  make fmt; and echo > aws/debug.log ; and make testacc TEST=./aws TESTARGS='-run=TestAccAWSAutoscalingPolicy_'
gofmt -w $(find . -name '*.go' |grep -v vendor)
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAutoscalingPolicy_ -timeout 120m
=== RUN   TestAccAWSAutoscalingPolicy_basic
--- PASS: TestAccAWSAutoscalingPolicy_basic (139.01s)
=== RUN   TestAccAWSAutoscalingPolicy_disappears
--- PASS: TestAccAWSAutoscalingPolicy_disappears (104.02s)
=== RUN   TestAccAWSAutoscalingPolicy_upgrade
--- PASS: TestAccAWSAutoscalingPolicy_upgrade (132.35s)
=== RUN   TestAccAWSAutoscalingPolicy_SimpleScalingStepAdjustment
--- PASS: TestAccAWSAutoscalingPolicy_SimpleScalingStepAdjustment (99.10s)
=== RUN   TestAccAWSAutoscalingPolicy_TargetTrack_Predefined
--- PASS: TestAccAWSAutoscalingPolicy_TargetTrack_Predefined (103.59s)
=== RUN   TestAccAWSAutoscalingPolicy_TargetTrack_Custom
--- PASS: TestAccAWSAutoscalingPolicy_TargetTrack_Custom (97.17s)
=== RUN   TestAccAWSAutoscalingPolicy_zerovalue
--- PASS: TestAccAWSAutoscalingPolicy_zerovalue (99.81s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	775.102s

[tilgovi]

I expect this will fix #3689 also

[tyjonesAncestry]

Can we get this merged? This is actively breaking us in Prod as well.

[tilgovi]

We're only a few days out from the initial report and there is a complete PR. Thank you, @loivis!

[mathematician]

When could this be merged? I am unsure if the documentation for autoscaling_policy works in its current state at all.

[bflad]

LGTM, thanks for fixing this. Sorry this took some extra time for review since it was quite large.

 7 tests passed (all tests)
=== RUN   TestAccAWSAutoscalingPolicy_TargetTrack_Predefined
--- PASS: TestAccAWSAutoscalingPolicy_TargetTrack_Predefined (56.48s)
=== RUN   TestAccAWSAutoscalingPolicy_TargetTrack_Custom
--- PASS: TestAccAWSAutoscalingPolicy_TargetTrack_Custom (59.59s)
=== RUN   TestAccAWSAutoscalingPolicy_zerovalue
--- PASS: TestAccAWSAutoscalingPolicy_zerovalue (61.08s)
=== RUN   TestAccAWSAutoscalingPolicy_SimpleScalingStepAdjustment
--- PASS: TestAccAWSAutoscalingPolicy_SimpleScalingStepAdjustment (74.00s)
=== RUN   TestAccAWSAutoscalingPolicy_disappears
--- PASS: TestAccAWSAutoscalingPolicy_disappears (98.28s)
=== RUN   TestAccAWSAutoscalingPolicy_upgrade
--- PASS: TestAccAWSAutoscalingPolicy_upgrade (121.46s)
=== RUN   TestAccAWSAutoscalingPolicy_basic
--- PASS: TestAccAWSAutoscalingPolicy_basic (123.73s)

[bflad]

This has been released in version 1.13.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!