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

Fix for ExpressRoute: No support for route filters for Microsoft peering #1083 #6341

Merged
merged 8 commits into from
Jul 15, 2020

Conversation

pearcec
Copy link
Contributor

@pearcec pearcec commented Apr 2, 2020

Added azurerm_route_filter resource. Updated azurerm_express_route_circuit_peering to include route_filter_id. Updated all the documentation. Tests included. Please review. I did set the test for the circuit to lowercase to match the existing tests. But I did run the TestAccAzureRMExpressRouteCircuitPeering_microsoftPeeringWithRouteFilter and it was successful.

make acctests SERVICE='network' TESTARGS='-run=TestAccAzureRMExpressRouteCircuitPeering' TESTTIMEOUT='60m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test -v ./azurerm/internal/services/network/tests/ -run=TestAccAzureRMExpressRouteCircuitPeering -timeout 60m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.Provider
Version=acc"
=== RUN   TestAccAzureRMExpressRouteCircuitPeering_microsoftPeeringWithRouteFilter
--- PASS: TestAccAzureRMExpressRouteCircuitPeering_microsoftPeeringWithRouteFilter (372.94s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/tests       375.315s

Fixes: #1083

@pearcec
Copy link
Contributor Author

pearcec commented Apr 2, 2020

I added a commit how do I get it to retest or update the pull?

@pearcec
Copy link
Contributor Author

pearcec commented Apr 23, 2020

@tombuildsstuff Reviewing another expressroute issue, I see why we set test to lower case now in resource_arm_express_route_circuit_test.go func TestAccAzureRMExpressRouteCircuit(t *testing.T) { combines them all.

This needs reworked. Feel free to flag it as such. I will work on it today.

Christian Pearce added 7 commits July 14, 2020 10:17
Other resource types use the singular rule. Update for hashicorp#1083
Last update for hashicorp#1083.  The tests are all set to lowercase.  Assuming it is an expense issue.
The test was run successfully for TestAccAzureRMExpressRouteCircuitPeering_microsoftPeeringWithRouteFilter.

=== RUN   TestAccAzureRMExpressRouteCircuitPeering_microsoftPeeringWithRouteFilter
--- PASS: TestAccAzureRMExpressRouteCircuitPeering_microsoftPeeringWithRouteFilter (372.94s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/network/tests       375.315
The testAccAzureRMExpressRouteCircuitPeering_microsoftPeeringWithRouteFilter failed lint as dead code
it was moved to a upper case T for testing.
@manicminer manicminer force-pushed the azurerm_route_filter_fix_1083 branch from 922a5a5 to 8f0e73e Compare July 14, 2020 09:23
@manicminer manicminer force-pushed the azurerm_route_filter_fix_1083 branch from e23c24e to aec652c Compare July 14, 2020 11:11
Copy link
Member

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

Thanks for this @pearcec, it looks great. I rebased onto master and tests are passing.

Screenshot 2020-07-15 23 10 13
Screenshot 2020-07-15 23 10 20

@manicminer manicminer added this to the v2.19.0 milestone Jul 15, 2020
@manicminer manicminer merged commit 322f363 into hashicorp:master Jul 15, 2020
manicminer added a commit that referenced this pull request Jul 15, 2020
@ghost
Copy link

ghost commented Jul 16, 2020

This has been released in version 2.19.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.19.0"
}
# ... other configuration ...

@azrael-lu
Copy link

This has been released in version 2.19.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.19.0"
}
# ... other configuration ...

It is great to have azurerm_route_filter, finally, I could replace the azurerm_template_deployment for route filter in my TF template.
But I still can not find the document update for azurerm_route_filter.
Could you please let me know when we will have the document for resource"azurerm_route_filter"?

Thanks.

@manicminer
Copy link
Member

Hi @azrael-lu, the resource doc is published here (data source doc here)

@ghost
Copy link

ghost commented Aug 15, 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 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Aug 15, 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.

ExpressRoute: No support for route filters for Microsoft peering
5 participants