provider/aws: Fix reading auto scaling group termination policies #5101
Conversation
|
FYI. There is a subtle problem with this change. I'm in the process of fixing it. I will provide an update once the issue is fixed. |
catsby
added
the
waiting-response
|
Thanks for letting us know, just ping here when it's ready 😄 |
|
@catsby Should be good to go now. Let me know if you see anything that can be improved. |
phinze
removed
the
waiting-response
| // with the default AWS create API behavior. | ||
| _, ok := d.GetOk("termination_policies") | ||
| if !ok && len(g.TerminationPolicies) == 1 && *g.TerminationPolicies[0] == "Default" { | ||
| d.Set("termination_policies", []string{}) |
There was a problem hiding this comment.
I believe this should be []interface{}, same as the return type of flattenStringList.
There was a problem hiding this comment.
As a rule of thumb we check the error return on non-primitive d.Set() calls, which I believe would expose the above issue.
There was a problem hiding this comment.
Interesting. AFAICT, this doesn't seem like this is done much in practice. I'll fix up to use []interface{} though.
|
Thanks for this, @tpounds! See inline for a few comments. |
|
@catsby Updated to use []interface{}. LMK if I should address anything else. |
|
@tpounds Sorry didn't make it clear in the comment - |
|
@tamsky Once that is fixed - this looks good to go! Exercised locally and it seems to be working 👍 |
|
Whoops! Bad auto-complete there, sorry! Meant @tpounds 😝 |
|
Gah! 100% my fault. Should have built locally before pushing. Should be good now. 😉 |
…licies provider/aws: Fix reading auto scaling group termination policies
|
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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Similar fix to #5044 and #5045 that fixes reading an auto scaling groups' termination policies. I also added an acceptance test to make sure the default termination policy is applied as expected.