New Data Source: `azurerm_express_route_circuit` #3158

romitgirdhar · 2019-04-03T04:55:01Z

This addresses Issue #3022
All tests passing

tombuildsstuff

Thanks for this PR :)

I've taken a look through and left some comments inline, but this is off to a good start - if we can fix up the comments then we should be able to kick off the tests and get this merged 👍

Thanks!

tombuildsstuff · 2019-04-03T07:13:08Z

azurerm/data_source_express_route_circuit.go

+			},
+
+			"service_provider_properties": {
+				Type:     schema.TypeSet,


due to a bug in Terraform Core this'll need to be a TypeList unfortunately - as such can we make this:

Suggested change

Type: schema.TypeSet,

Type: schema.TypeList,

tombuildsstuff · 2019-04-03T07:24:48Z

azurerm/data_source_express_route_circuit.go

+			},
+
+			"sku": {
+				Type:     schema.TypeSet,


(as above) since this is a Data Source there's a known bug with Sets, so can we update this to be a List:

Suggested change

Type: schema.TypeSet,

Type: schema.TypeList,

tombuildsstuff · 2019-04-03T10:35:39Z

azurerm/data_source_express_route_circuit_test.go

+	dataSourceName := "data.azurerm_express_route_circuit.test"
+	ri := tf.AccRandTimeInt()
+
+}*/


since this is unused, can we remove this?

tombuildsstuff · 2019-04-03T10:39:35Z

azurerm/data_source_express_route_circuit.go

+		if err := d.Set("sku", sku); err != nil {
+			return fmt.Errorf("Error setting `sku`: %+v", err)
+		}
+	}


(since this is the same in the other resource, it's fine to leave this as-is for now) in general we'd tend to ensure the sku block was always set, for example by making this:

sku := flattenExpressRouteCircuitSku(resp.Sku) if err := d.Set("sku", sku); err != nil { return fmt.Errorf("Error setting `sku`: %+v", err) }

(e.g. by removing the surrounding if)

and then updating the flattenExpressRouteCircuitSku method to be:

func flattenExpressRouteCircuitSku(sku *network.ExpressRouteCircuitSku) []interface{} { if sku == nil { return []interface{}{} } return []interface{}{ map[string]interface{}{ "tier": string(sku.Tier), "family": string(sku.Family), }, } }

(but as mentioned above, since this is the same in the resource this is fine for now, just for info 😄)

tombuildsstuff · 2019-04-03T10:41:12Z

azurerm/data_source_express_route_circuit.go

+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
+)
+
+func dataSourceArmExpressRouteCircuit() *schema.Resource {


could we add some documentation for this data source and a link to the sidebar?

tombuildsstuff · 2019-04-03T10:43:31Z

azurerm/data_source_express_route_circuit_test.go

+	"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
+)
+
+func TestAccDataSourceAzureRMExpressRoute_basic(t *testing.T) {


since there's a limit on the number of ExpressRoute's which can be provisioned at one time (and we run the tests in parallel) - we'll need to make this an internal method (by making it lower-case) and then ensure it's called from within this test function in the Express Route resource

(but to make this simpler to test, we probably want to do this when all the other changes are done 😄)

Since it only has the 1 test in there, do we need the function? Also, should we add this function to the test function (in the resource file) or should we create a similar function in this test file?

tombuildsstuff · 2019-04-03T10:44:14Z

azurerm/data_source_express_route_circuit_test.go

+}*/
+
+func testAccDataSourceAzureRMExpressRoute_basic(rInt int, location string) string {
+	config := testAccAzureRMExpressRouteCircuit_MeteredBasic(rInt, location)


rather than re-defining this we should be able to call the testAccAzureRMExpressRouteCircuit_basicMeteredConfig test method within the Express Route resource

That's what I meant to do. But, the subscription I used to test did not support the Express Route configuration that is hard-coded in the test you referenced. But, I checked another subscription and it looks like it does support those configurations. I'll make the change. thanks!

…the flag for ExpressRouteGatewayBypass to VNETGatewayConnection Resource

romitgirdhar · 2019-04-10T23:44:56Z

I made the requested changes. Additionally, I've also added the ExpressRouteGatewayBypass flag to the Virtual network gateway connection resource. This should address issue #3027

romitgirdhar · 2019-04-15T22:31:01Z

Hello @tombuildsstuff - Any updates on this PR? Thank you!

dismissing since changes have been pushed

tombuildsstuff · 2019-04-16T08:18:35Z

@romitgirdhar I hope you don't mind but I've merged master in/rebased this

tombuildsstuff

hey @romitgirdhar

Thanks for pushing those changes - this now LGTM 👍

Thanks!

azurerm/data_source_express_route_circuit_test.go

tombuildsstuff · 2019-04-16T19:07:40Z

Ignoring an existing test failure (due to an API change) the tests pass:

------- Stdout: -------
=== RUN   TestAccAzureRMExpressRouteCircuit
=== RUN   TestAccAzureRMExpressRouteCircuit/basic
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/data_basic
=== PAUSE TestAccAzureRMExpressRouteCircuit/basic/data_basic
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/metered
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/unlimited
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/update
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/allowClassicOperationsUpdate
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/requiresImport
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/tierUpdate
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/premiumMetered
=== RUN   TestAccAzureRMExpressRouteCircuit/basic/premiumUnlimited
=== CONT  TestAccAzureRMExpressRouteCircuit/basic/data_basic
=== RUN   TestAccAzureRMExpressRouteCircuit/PrivatePeering
=== RUN   TestAccAzureRMExpressRouteCircuit/PrivatePeering/azurePrivatePeering
=== RUN   TestAccAzureRMExpressRouteCircuit/PrivatePeering/requiresImport
=== RUN   TestAccAzureRMExpressRouteCircuit/MicrosoftPeering
=== RUN   TestAccAzureRMExpressRouteCircuit/MicrosoftPeering/microsoftPeering
=== RUN   TestAccAzureRMExpressRouteCircuit/authorization
=== RUN   TestAccAzureRMExpressRouteCircuit/authorization/basic
=== PAUSE TestAccAzureRMExpressRouteCircuit/authorization/basic
=== RUN   TestAccAzureRMExpressRouteCircuit/authorization/multiple
=== PAUSE TestAccAzureRMExpressRouteCircuit/authorization/multiple
=== RUN   TestAccAzureRMExpressRouteCircuit/authorization/requiresImport
=== CONT  TestAccAzureRMExpressRouteCircuit/authorization/basic
=== CONT  TestAccAzureRMExpressRouteCircuit/authorization/multiple
--- FAIL: TestAccAzureRMExpressRouteCircuit (1871.08s)
    --- PASS: TestAccAzureRMExpressRouteCircuit/basic (1038.06s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/metered (140.92s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/unlimited (139.06s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/update (170.05s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/allowClassicOperationsUpdate (159.91s)
        --- SKIP: TestAccAzureRMExpressRouteCircuit/basic/requiresImport (0.00s)
            resource_arm_express_route_circuit_test.go:84: Skipping since resources aren't required to be imported
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/tierUpdate (157.95s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/premiumMetered (128.26s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/premiumUnlimited (141.90s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/basic/data_basic (206.48s)
    --- FAIL: TestAccAzureRMExpressRouteCircuit/PrivatePeering (137.34s)
        --- FAIL: TestAccAzureRMExpressRouteCircuit/PrivatePeering/azurePrivatePeering (137.34s)
            testing.go:538: Step 0 error: Error applying: 1 error occurred:
                	* azurerm_express_route_circuit_peering.test: 1 error occurred:
                	* azurerm_express_route_circuit_peering.test: Code="InvalidCharactersInSharedKey" Message="The specified shared key contains invalid characters." Details=[]
                
                
                
                
        --- SKIP: TestAccAzureRMExpressRouteCircuit/PrivatePeering/requiresImport (0.00s)
            resource_arm_express_route_circuit_peering_test.go:44: Skipping since resources aren't required to be imported
    --- PASS: TestAccAzureRMExpressRouteCircuit/MicrosoftPeering (290.33s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/MicrosoftPeering/microsoftPeering (290.33s)
    --- PASS: TestAccAzureRMExpressRouteCircuit/authorization (0.00s)
        --- SKIP: TestAccAzureRMExpressRouteCircuit/authorization/requiresImport (0.00s)
            resource_arm_express_route_circuit_authorization_test.go:41: Skipping since resources aren't required to be imported
        --- PASS: TestAccAzureRMExpressRouteCircuit/authorization/multiple (193.70s)
        --- PASS: TestAccAzureRMExpressRouteCircuit/authorization/basic (198.88s)
FAIL

ghost · 2019-04-17T10:39:45Z

This has been released in version 1.25.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 = "~> 1.25.0"
}
# ... other configuration ...

ghost · 2019-05-17T13:50:27Z

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!

Added Data source for Express route Circuit

b7c2dbe

ghost added the size/L label Apr 3, 2019

romitgirdhar added 2 commits April 2, 2019 22:06

Updating provider.go with the express route circuit data source function

7153e5f

ran go fmt on the data source

4aa4381

tombuildsstuff added new-data-source service/express-route labels Apr 3, 2019

tombuildsstuff previously requested changes Apr 3, 2019

View reviewed changes

tombuildsstuff added the waiting-response label Apr 3, 2019

Updated the ER Data Source based on feedback. Additionally, added in …

f7f3f25

…the flag for ExpressRouteGatewayBypass to VNETGatewayConnection Resource

ghost added size/XL documentation and removed size/L labels Apr 10, 2019

ghost removed the waiting-response label Apr 10, 2019

Merge branch 'master' into master

794fe4d

tombuildsstuff approved these changes Apr 16, 2019

View reviewed changes

tombuildsstuff added this to the v1.25.0 milestone Apr 16, 2019

tombuildsstuff added 2 commits April 16, 2019 10:40

fixing the import so this builds

98c66ca

adding the data source test to the runable test

a79361a

tombuildsstuff reviewed Apr 16, 2019

View reviewed changes

azurerm/data_source_express_route_circuit_test.go Show resolved Hide resolved

tombuildsstuff changed the title ~~Added Data source for Express route Circuit~~ New Data Source: azurerm_express_route_circuit Apr 16, 2019

updating the test expectation to match the response

ca4fa3c

tombuildsstuff merged commit 25a6ea9 into hashicorp:master Apr 16, 2019

tombuildsstuff added a commit that referenced this pull request Apr 16, 2019

Updating to include #3158

11420a3

ghost locked and limited conversation to collaborators May 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Data Source: `azurerm_express_route_circuit` #3158

New Data Source: `azurerm_express_route_circuit` #3158

romitgirdhar commented Apr 3, 2019

tombuildsstuff left a comment

tombuildsstuff Apr 3, 2019

tombuildsstuff Apr 3, 2019

tombuildsstuff Apr 3, 2019

tombuildsstuff Apr 3, 2019

tombuildsstuff Apr 3, 2019

tombuildsstuff Apr 3, 2019

romitgirdhar Apr 9, 2019 •

edited

Loading

tombuildsstuff Apr 3, 2019

romitgirdhar Apr 6, 2019

romitgirdhar commented Apr 10, 2019

romitgirdhar commented Apr 15, 2019

tombuildsstuff commented Apr 16, 2019

tombuildsstuff left a comment

tombuildsstuff commented Apr 16, 2019

ghost commented Apr 17, 2019

ghost commented May 17, 2019

New Data Source: azurerm_express_route_circuit #3158

New Data Source: azurerm_express_route_circuit #3158

Conversation

romitgirdhar commented Apr 3, 2019

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romitgirdhar Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romitgirdhar commented Apr 10, 2019

romitgirdhar commented Apr 15, 2019

tombuildsstuff commented Apr 16, 2019

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff commented Apr 16, 2019

ghost commented Apr 17, 2019

ghost commented May 17, 2019

New Data Source: `azurerm_express_route_circuit` #3158

New Data Source: `azurerm_express_route_circuit` #3158

romitgirdhar Apr 9, 2019 •

edited

Loading