-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
add customize diff to prevent two rules with the same priority #3202
add customize diff to prevent two rules with the same priority #3202
Conversation
Mind asking someone else for a review? I've got a lot on my plate this week and next and trying to offload as many non-perf tasks as possible. |
|
||
nPriorities := map[int64]bool{} | ||
for _, rule := range nSet.List() { | ||
priority := int64(rule.(map[string]interface{})["priority"].(int)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will panic if the values aren't set correctly - is that okay? I'm not sure what invariants TF provides for types, unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Terraform should verify that priority
is an int
before we get here. Is that what you're asking? Very similar code is used in the update function and it seems we haven't hit any problems with that yet (https://github.com/terraform-providers/terraform-provider-google/blob/master/google/resource_compute_security_policy.go#L254), however if you feel more comfortable with double-checking it, I can certainly throw some checks around here.
@@ -97,6 +97,11 @@ func TestAccComputeSecurityPolicy_update(t *testing.T) { | |||
ImportStateVerify: true, | |||
}, | |||
|
|||
{ | |||
Config: testAccComputeSecurityPolicy_updateSamePriority(spName), | |||
ExpectError: regexp.MustCompile("Two rules have the same priority, please update one of the priorities to be different."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! thank you!!
f8b92b2
to
07597d8
Compare
Added CustomizeDiff to prevent users from adding/updating the security policy with two rules of the same priority.
Fixes hashicorp/terraform-provider-google#5804
Release Note Template for Downstream PRs (will be copied)