resource/lb_listener_rule: update maximum priority value to 50000

[loivis]

update according to the latest document:
https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_CreateRule.html

Valid Range: Minimum value of 1. Maximum value of 50000.

[bflad]

This is a great catch! 😄 Do you mind simplifying this even further by removing the custom function and switching it to ValidateFunc: validateIntegerInRange(1, 50000)? Thanks!

[loivis]

@bflad change applied and pushed.

[bflad]

🤔 In thinking about this a lot more, I think this will actually be a breaking change for anyone who managed to import a listener default rule (then set priority = 99999 in their configuration to match). We would need to migrate their state and note the "breaking" change.

That said, I just verified its technically possible to have a default priority rule and a 50000 priority rule, so changing default rules to 50000 could also be confusing as its also "incorrect". 🤷‍♂️

Maybe the most correct answer here is changing the attribute to type string and accepting default priority, but only for import. Or having a separate, special aws_lb_listener_default_rule resource (the API does error if you try to delete default rules). This is all turning into a lot of work though 😓 .

I'm not sure how you'd want to proceed but the quick way to fix the actual validation except default rules is to leave default rules set to 99999 and go back to the custom validation function where it allows 99999 along with the correct priority range of 1 through 50000. 🤕

[loivis]

Indeed it's a minor change in code but breaking change for user. I didn't realize the impact on imported resources as I never deal with default rule. I think the approach you mentioned is best for now. It's not necessary to change priority for default.

[bflad]

😅 From my comment earlier today, you'll also need to go back to the old custom validateAwsLbListenerRulePriority function and allow 99999 as a valid configuration. Sorry for the back and forth due to how this was implemented before we started trying to change it.

[loivis]

Ah, I think I get your point this time:smirk:. I wasn't thinking enough and lack of experiences on handling such scenarios.

[bflad]

LGTM! Thanks again for finding this. 🚀

[bflad]

This has been released in version 1.10.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!