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.

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 azurerm_firewall management IP configuration #8235

Merged
merged 14 commits into from
Sep 14, 2020

Conversation

draggeta
Copy link
Contributor

@draggeta draggeta commented Aug 24, 2020

This PR adds the management IP configuration for the Azure Firewall. This allows for forced tunneling to be configured by configuring a UDR on the AzureFirewallSubnet.

To do:

  • update read function
  • update create function
  • update delete function
  • clean up code
  • update tests
  • update docs

Closes #7152

Acceptance test

❯  go test -timeout 180m -v -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc" `
>> github.com\terraform-providers\terraform-provider-azurerm\azurerm\internal\services\network\tests `
>> -run "^TestAccAzureRMFirewall_withManagementIp" -count=1

=== RUN   TestAccAzureRMFirewall_withManagementIp
=== PAUSE TestAccAzureRMFirewall_withManagementIp
=== CONT  TestAccAzureRMFirewall_withManagementIp
--- PASS: TestAccAzureRMFirewall_withManagementIp (793.90s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/tests                                                       796.110s

@ghost ghost added the size/M label Aug 24, 2020
@ghost ghost added size/L and removed size/M labels Aug 26, 2020
@ghost ghost added the documentation label Aug 26, 2020
@draggeta draggeta changed the title WIP: Add azurerm_firewall management IP configuration Add azurerm_firewall management IP configuration Aug 26, 2020
…vider-azurerm into add-azure-firewall-management-ip
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @caius

Thanks for this PR :)

Taking a look through this is off to a good start - if we can fix up the comments left inline (and the tests pass) then this otherwise LGTM 👍

Thanks!

azurerm/internal/services/network/firewall_resource.go Outdated Show resolved Hide resolved
azurerm/internal/services/network/firewall_resource.go Outdated Show resolved Hide resolved
website/docs/r/firewall.html.markdown Outdated Show resolved Hide resolved
website/docs/r/firewall.html.markdown Outdated Show resolved Hide resolved
* `ip_configuration` - (Required) A `ip_configuration` block as documented below.
* `ip_configuration` - (Required) An `ip_configuration` block as documented below.

* `management_ip_configuration` - (Optional) A `management_ip_configuration` block as documented below, which allows force-tunnelling of traffic to be performed by the firewall. Changing this forces a new resource to be created.
Copy link
Contributor Author

@draggeta draggeta Aug 31, 2020

Choose a reason for hiding this comment

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

I'm still not sure about the wording. You can change the management_ip_configuration block without recreating the firewall. It only forces a new resource when you change the subnet_id. Adding or removing the block does force a new resource of course.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dragetta I think you pretty much got it there! How about:

Suggested change
* `management_ip_configuration` - (Optional) A `management_ip_configuration` block as documented below, which allows force-tunnelling of traffic to be performed by the firewall. Changing this forces a new resource to be created.
* `management_ip_configuration` - (Optional) A `management_ip_configuration` block as documented below, which allows force-tunnelling of traffic to be performed by the firewall. You can add and remove this block, however changing `subnet_id` in an existing block forces a new resource to be created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manicminer I've adjusted your wording a bit. Let me know if this is ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

@draggeta briiliant 👍

website/docs/r/firewall.html.markdown Outdated Show resolved Hide resolved
draggeta and others added 2 commits September 14, 2020 11:04
Update description of the management_ip_config
@manicminer
Copy link
Contributor

Test results:

Screenshot 2020-09-14 at 11 29 42

@manicminer manicminer added this to the v2.28.0 milestone Sep 14, 2020
@manicminer manicminer merged commit 385178d into hashicorp:master Sep 14, 2020
manicminer added a commit that referenced this pull request Sep 14, 2020
@ghost
Copy link

ghost commented Sep 17, 2020

This has been released in version 2.28.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.28.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Oct 14, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Forced Tunneling / azurerm_firewall
3 participants