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

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

Closed
rafael6 opened this issue Apr 5, 2018 · 26 comments · Fixed by #6341
Closed

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

rafael6 opened this issue Apr 5, 2018 · 26 comments · Fixed by #6341

Comments

@rafael6
Copy link

rafael6 commented Apr 5, 2018

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Terraform Version

0.11.6

Affected Resource(s)

express_route_circuit_peering (peering type MicrosoftPeering)

Terraform Configuration Files

N/A

Debug Output

N/A

Panic Output

N/A

Expected Behavior

Support for route filters for Microsoft peering; see link below
https://docs.microsoft.com/en-us/azure/expressroute/how-to-routefilter-powershell

Actual Behavior

No support for route filters for Microsoft peering as stated in the Microsoft article below.
https://docs.microsoft.com/en-us/azure/expressroute/how-to-routefilter-powershell

Steps to Reproduce

N/A

Important Factoids

N/A

References

https://docs.microsoft.com/en-us/azure/expressroute/how-to-routefilter-powershell

Important
Microsoft peering of ExpressRoute circuits that were configured prior to August 1, 2017 will have all service prefixes advertised through the Microsoft peering, even if route filters are not defined. Microsoft peering of ExpressRoute circuits that are configured on or after August 1, 2017 will not have any prefixes advertised until a route filter is attached to the circuit. For more information, see Configure a route filter for Microsoft peering.

https://docs.microsoft.com/en-us/azure/expressroute/expressroute-howto-routing-portal-resource-manager

@tombuildsstuff tombuildsstuff changed the title Azurerm Feature Request: No support for route filters for Microsoft peering ExpressRoute: No support for route filters for Microsoft peering Nov 2, 2018
@leppeK
Copy link

leppeK commented Jun 21, 2019

Would really love to have this, it is impossible to use proper filtered peering with infrastructure as code without this feature.

@rapster83
Copy link

Route filter is needed for a customer project. Any updates?

@enorlando
Copy link

Do we have an update of when route filters will be available please?

@mharrison365
Copy link

Are there any updates for this? This has been open for two years and this is preventing us from configuring our ExpressRoute with MS Peering via Terraform. The only way we are going to be able to get this to work without this is to configure both the ExpressRoute Circuit, Peering, and Route Filter via ARM...as an azurerm_template_deployment.

@pearcec
Copy link
Contributor

pearcec commented Apr 1, 2020

An arm template is definitely an option if you need something immediately. I used them a few times in the distant past when azurerm was just getting off the ground. Perhaps we can sketchup what the terraform resources and arguments looks like.

@pearcec
Copy link
Contributor

pearcec commented Apr 1, 2020

How does this look? I think for testing adding and creating the route filter and rules should be low impact -- I have to check on the expressroute. I am not sure why this hasn't been addressed. @tombuildsstuff is there a particular challenge you saw? Or is it just the usually time and priority? I would be willing to draft up some initial code if the below looks good. Please advise.

resource "azurerm_resource_group" "example" {
  name     = "rtfltrTest"
  location = "East US"
}

resource "azurerm_route_filter" "example" {
  name                = "routeFilter1"
  resource_group_name = azurerm_resource_group.example.name
  location            = azurerm_resource_group.example.location

 rules = {
  access               = "Allow" # Must be Allow
  rule_type           = "Community" # Only accepted value, probably still code it for the future
  community_list = ["12076:53004"]
 }
}


resource "azurerm_express_route_circuit_peering" "example" {
  peering_type                  = "MicrosoftPeering"
  express_route_circuit_name    = azurerm_express_route_circuit.example.name
  resource_group_name           = azurerm_resource_group.example.name
  peer_asn                      = 100
  primary_peer_address_prefix   = "123.0.0.0/30"
  secondary_peer_address_prefix = "123.0.0.4/30"
  vlan_id                       = 300

  microsoft_peering_config {
    advertised_public_prefixes = ["123.1.0.0/24"]
  }
  
  route_filter                 = azurerm_route_filter.example.id
}

@mharrison365
Copy link

Wow, thanks for the quick response @pearcec ! I like what you have so far. Do you think it makes sense to do the route_filter parameter directly in the circuit_peering resource, or would this be a potential use case for a "express_route_peering_route_filter_attachment" sort of resource? (that long name probably isn't ideal either lol)

I think one thing to note, and I'm not sure if it has a huge impact on how the provider is written, is that we're wanting to use this in Azure China. The only real difference between China and Global for this service is that it only allows selection of the region communities, and does not allow granular selection of individual service families like in Global Azure.
image

@pearcec
Copy link
Contributor

pearcec commented Apr 1, 2020

I was not 100% sure if we do the peering direct or as an association. I would have to study some other situations to try and be consistent. You can't name the association, so I figured it would make more sense to include it as optional then track state and update the peering. I am hoping someone from the provider team can comment on the proposal. Then I will write some code.

I am not sure about the Azure China piece, I don't think we would check the communities for validity. I would let the REST API fail if you passed in garbage.

@tombuildsstuff
Copy link
Member

@pearcec albeit I’ve not checked that service in a while, but it looks reasonable to me. It may be worth calling it “express route filter” to avoid the duplication in the name, but that’s not a big issue

That said, I have a vague recollection that it wasn’t possible to create a Microsoft peering anymore (although I could be thinking of something else) - so I’m wondering how we’d test this if so?

For the association resource, I’d suggest we inline that for the moment - we mostly use them to work around API’s which can have circular references - but this seems fine to me without it (at least not having tried it)

@mharrison365 in practice Azure Public and China behave similarly enough I think we’d be fine here - likely the only difference is the association values - for which, it appears you can see in the portal, so I think that should be fine

@pearcec
Copy link
Contributor

pearcec commented Apr 1, 2020

@tombuildsstuff Thanks, I created Microsoft peering recently. Microsoft deprecated Public Peering for new circuits. They consolidated the it into Microsoft Peering.

https://docs.microsoft.com/en-us/azure/expressroute/expressroute-circuit-peerings#peeringcompare

They also are not allowing people peer O365 without authorization. See the note in the link below.

https://docs.microsoft.com/en-us/office365/enterprise/azure-expressroute

I will drop the route_route for route, I was wondering about that as well.

We also need to include a data resource.

@pearcec
Copy link
Contributor

pearcec commented Apr 1, 2020

I edited my example above. I dropped the express_route from the resources to match the API names.

pearcec pushed a commit to pearcec/terraform-provider-azurerm that referenced this issue Apr 1, 2020
@pearcec
Copy link
Contributor

pearcec commented Apr 1, 2020

As you can see I reference in my commit some early working code. I have the route_filter piece working. I want to find a descent example for the rules. I primarily used route_table for the an example on route_filter.

@mharrison365
Copy link

@pearcec - the selection process for the rules seems similar to subnet delegation or service endpoints from a UI perspective alone. not sure if that'd be a good starting point?

Just wanted to additionally say thanks for your expeditious efforts today in getting this going! I'll keep an eye on it so we can take advantage of it as soon as it's released (assuming all goes well).

@pearcec
Copy link
Contributor

pearcec commented Apr 1, 2020

I been using azurerm_route_table routes as an example. I am 99% of the way done with the azurerm_route_filter_rule part. I want to cycle back through and add the option for a rule {} in azurerm_route_filter. It is a bit awkward because I am not creating a data source for the rule. This matches the way route_table and routes are implemented, so it feels like a gap.

I felt like doing some code, I miss not be able to do it all the time. I also ran into the same issue you did. I could do everything for expressroute but this. We occasionally add ER ad-hoc and tear them down.

@pearcec
Copy link
Contributor

pearcec commented Apr 2, 2020

I need to put a constraint on this. I did notice in the portal it seemed to only support one rule. Need to modify my test to check for the error. I am also need to read up on the Allow vs Deny. There is no provision in the portal for this. Thinking it might be a Deny on one communities enables the rest. Not sure. Need to read.

Error: Error Creating/Updating Route Filter Rule "acctestroute_filter_rule2_200401224700407937" (Route Filter "acctestrf200401224700407937" / Resource Group "acctestRG-200401224700407937"): network.Rou
teFilterRulesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="RouteFilterRuleCountLimitReached" Message="A route filter cannot have more than 1 route filter rules." Detai
ls=[]

@pearcec
Copy link
Contributor

pearcec commented Apr 2, 2020

Thinking this through, if there is only one rule allowed there is no point in having a separate resource. I am not sure how to constrain it. It is old how they refer to it a "rules" even though you only get one. I would like to try an confirm it before I toss all the code I worked on for azurerm_route_filter_rule. Let me know if any of you find anything.

@pearcec
Copy link
Contributor

pearcec commented Apr 2, 2020

This link seems pretty conclusive. It says you can only have one rule and it must be type Allow. I suppose you could have multiple rules not attached. But they would just be dangling and that seems stupid. I can't see of a reason. I am going to push the code I have into a different branch so we can grab it if something changes.

I am going to edit the example above to go with one rule and constrain the access to Allow.

pearcec pushed a commit to pearcec/terraform-provider-azurerm that referenced this issue Apr 2, 2020
pearcec pushed a commit to pearcec/terraform-provider-azurerm that referenced this issue Apr 2, 2020
Other resource types use the singular rule. Update for hashicorp#1083
@pearcec
Copy link
Contributor

pearcec commented Apr 2, 2020

@mharrison365 It is most of the way there. The only piece is the association. You can build it and test it. Let me know if you find anything. I am working on the last piece now. I will submit a pull request then. I hope you are using azurerm 2.0 💃

pearcec pushed a commit to pearcec/terraform-provider-azurerm that referenced this issue Apr 2, 2020
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
@pearcec
Copy link
Contributor

pearcec commented Apr 2, 2020

I submitted a pull request. @mharrison365 I am done if you want to test it.

@pearcec
Copy link
Contributor

pearcec commented Apr 2, 2020

Adding @tombuildsstuff - I finished the code. Have a look.

@pearcec
Copy link
Contributor

pearcec commented Apr 2, 2020

Build is failing some lint checks, I am fixing them up now.

@mharrison365
Copy link

@pearcec Awesome!! Thanks for the update! I am using AzureRM2.0 :) I'll see what I can do to test it out

@mharrison365
Copy link

@tombuildsstuff - do you see any issues with adding this to the 2.5 milestone/release?

@pearcec
Copy link
Contributor

pearcec commented Apr 7, 2020

@mharrison365 did you test it 😃

manicminer pushed a commit to pearcec/terraform-provider-azurerm that referenced this issue Jul 14, 2020
manicminer pushed a commit to pearcec/terraform-provider-azurerm that referenced this issue Jul 14, 2020
Other resource types use the singular rule. Update for hashicorp#1083
manicminer pushed a commit to pearcec/terraform-provider-azurerm that referenced this issue Jul 14, 2020
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
@manicminer manicminer added this to the v2.19.0 milestone Jul 15, 2020
manicminer added a commit that referenced this issue Jul 15, 2020
…te_filter) (#6341)

* Add azurerm_route_filter for #1083
* Updated azurerm_route_filter to include rules
* Changed rules -> rule for consistency
* Adding route_filter_id for azurerm_express_route_circuit_peering
    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

Co-authored-by: Christian Pearce <chrpearce@hersheys.com>
Co-authored-by: Tom Bamford <tom@bamford.io>
@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 ...

@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
8 participants