Switch aws_appautoscaling_target's role_arn to optional #2889
Conversation
There was a problem hiding this comment.
Hi @armanshan12
thanks for the PR.
I'm ok with the change in principle and with the new test. I'd prefer to keep existing tests untouched though (for backward compatibility).
Let me know what you think.
| @@ -192,58 +217,6 @@ func testAccCheckAWSAppautoscalingTargetExists(n string, target *applicationauto | |||
| func testAccAWSAppautoscalingTargetConfig( | |||
| randClusterName string) string { | |||
| return fmt.Sprintf(` | |||
| resource "aws_iam_role" "autoscale_role" { | |||
There was a problem hiding this comment.
It's great to have a test which tests the new behaviour (i.e. default role assigned by AWS), but we should keep existing tests as is - setting custom role should still be working and be tested the same way as before.
radeksimko
added
the
waiting-response
There was a problem hiding this comment.
Actually ignore my previous response. I was waiting for acceptance tests to finish and didn't realize the API already introduced a breaking change.
In such case it's ok to modify existing tests, because these are already failing. I noticed they started failing around 13th-14th December which is probably when AWS introduced the breaking change.
Sorry for the confusion and thanks again for the fix!
radeksimko
removed
the
waiting-response
|
Hi @radeksimko, Regarding your concern about testing custom |
|
This has been released in terraform-provider-aws version 1.7.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
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! |
ghost
locked and limited conversation to collaborators
fixes #2750