-
Notifications
You must be signed in to change notification settings - Fork 4.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
Firewall app rules #2532
Firewall app rules #2532
Conversation
…form-provider-azurerm into firewall_app_rules
here is an example of multiple |
@katbyte that example and all the others i've seen are for lists with |
It will work just fine. Because your saying that property |
@katbyte the requirement here is that properties of items in the list need to conflict with each other, but there could be multiple items in the list. resource "azurerm_firewall_application_rule_collection" "test" {
name = "acctestarc"
azure_firewall_name = "${azurerm_firewall.test.name}"
resource_group_name = "${azurerm_resource_group.test.name}"
priority = 100
action = "Allow"
rule {
name = "rule1"
source_addresses = [
"192.168.2.1",
]
target_fqdns = [
"*.microsoft.com",
]
protocol {
port = 8443
type = "Https"
}
protocol {
port = 8080
type = "Http"
}
}
rule {
name = "rule2"
source_addresses = [
"10.0.0.0/16",
]
fqdn_tags = [
"test",
]
}
} but using |
Ah sorry @hbuckle, i misunderstood. unfortunately i think they best way is to just check that in the create/update function and return an error if there is there is a conflict. |
OK I've added the validation into |
Thanks @hbuckle, its not ideal but its all we can do at the moment. |
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.
Thanks for the updates, I have left some mostly minor/stylistic comments inline. However it does appear some of your test checks are invalid. Other then that this is looking pretty good and once these issues are fixed up we should be good to merge 🙂
} | ||
|
||
err = future.WaitForCompletionRef(ctx, client.Client) | ||
if err != nil { |
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.
minor these lines could be combined:
if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
azurerm/resource_arm_firewall_application_rule_collection_test.go
Outdated
Show resolved
Hide resolved
azurerm/resource_arm_firewall_application_rule_collection_test.go
Outdated
Show resolved
Hide resolved
azurerm/resource_arm_firewall_application_rule_collection_test.go
Outdated
Show resolved
Hide resolved
azurerm/resource_arm_firewall_application_rule_collection_test.go
Outdated
Show resolved
Hide resolved
azurerm/resource_arm_firewall_application_rule_collection_test.go
Outdated
Show resolved
Hide resolved
I think I've fixed up all the test attributes, and also updated the utility functions |
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.
Thank you for all the updates @hbuckle LGTM now 🙂
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. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Adds support for Azure firewall application rules. I'll follow up with the docs shortly
I wasn't sure how to get the
ConflictsWith
to work with lists of more than one element - any pointers?