Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/lb_listener_rule: update maximum priority value to 50000 #3379

Merged
merged 3 commits into from
Feb 16, 2018
Merged

resource/lb_listener_rule: update maximum priority value to 50000 #3379

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04b2681
resource/lb_listener_rule: update maximum priority value to 50000
Feb 14, 2018
1e73a7f
resource/lb_listener_rule: remove custom validation function
Feb 15, 2018
8e33671
resource/lb_listener_rule: avoid breaking change for default rule pri…
Feb 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions aws/resource_aws_lb_listener_rule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func resourceAwsLbbListenerRule() *schema.Resource {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
ValidateFunc: validateAwsLbListenerRulePriority,
ValidateFunc: validateIntegerInRange(1, 50000),
},
"action": {
Type: schema.TypeList,
Expand Down Expand Up @@ -154,7 +154,7 @@ func resourceAwsLbListenerRuleRead(d *schema.ResourceData, meta interface{}) err

// Rules are evaluated in priority order, from the lowest value to the highest value. The default rule has the lowest priority.
if *rule.Priority == "default" {
d.Set("priority", 99999)
d.Set("priority", 50000)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 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. 🤕

} else {
if priority, err := strconv.Atoi(*rule.Priority); err != nil {
return errwrap.Wrapf("Cannot convert rule priority %q to int: {{err}}", err)
Expand Down Expand Up @@ -276,14 +276,6 @@ func resourceAwsLbListenerRuleDelete(d *schema.ResourceData, meta interface{}) e
return nil
}

func validateAwsLbListenerRulePriority(v interface{}, k string) (ws []string, errors []error) {
value := v.(int)
if value < 1 || value > 99999 {
errors = append(errors, fmt.Errorf("%q must be in the range 1-99999", k))
}
return
}

func validateAwsListenerRuleField(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)
if len(value) > 64 {
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/lb_listener_rule.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ resource "aws_lb_listener_rule" "host_based_routing" {
The following arguments are supported:

* `listener_arn` - (Required, Forces New Resource) The ARN of the listener to which to attach the rule.
* `priority` - (Required) The priority for the rule. A listener can't have multiple rules with the same priority.
* `priority` - (Required) The priority for the rule between `1` and `50000`. A listener can't have multiple rules with the same priority.
* `action` - (Required) An Action block. Action blocks are documented below.
* `condition` - (Required) A Condition block. Condition blocks are documented below.

Expand Down