azurerm_api_management - support for virtual network integration #5769

francescopersico · 2020-02-15T08:24:11Z

The PR adds API Management virtual network integration both internal and external.

nsmeef · 2020-03-06T04:29:33Z

Whats the status of this? Would love to get this in soon :)

dansali · 2020-03-06T04:46:28Z

This will be incredibly helpful for me, looking forward to seeing this put in :)

katbyte

Thanks for the PR @francescopersico, overall this looks great and i've left a few comments inline that should be addressed. Looks like you forgot to add private_ip_addresses to the schema

katbyte · 2020-03-10T00:15:37Z

azurerm/internal/services/apimanagement/resource_arm_api_management.go

+							Type:     schema.TypeString,
+							Required: true,
+							ForceNew: true,
+						},


Could we validate this is a proper id with azure.ValidateResourceID

katbyte · 2020-03-10T00:17:43Z

azurerm/internal/services/apimanagement/tests/resource_arm_api_management_test.go

+
+func testAccAzureRMApiManagement_virtualNetworkInternal(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+resource "azurerm_resource_group" "test" {


we'll need a provider block here

katbyte · 2020-03-10T00:17:54Z

azurerm/internal/services/apimanagement/tests/resource_arm_api_management_test.go

+
+resource "azurerm_virtual_network" "test" {
+  name                = "acctestVNET-%d"
+  location            = "${azurerm_resource_group.test.location}"


could we switch the test to .12 syntax?

katbyte · 2020-03-10T00:51:45Z

azurerm/internal/services/apimanagement/resource_arm_api_management.go

@@ -674,6 +713,8 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{
 		d.Set("management_api_url", props.ManagementAPIURL)
 		d.Set("scm_url", props.ScmURL)
 		d.Set("public_ip_addresses", props.PublicIPAddresses)
+		d.Set("private_ip_addresses", props.PrivateIPAddresses)


This property doesn't exist?

could we add it to the schema

katbyte

Hey @francescopersico,

thank you for this PR. In addition to the comments i've left and the missing schema for private IP addresses i am noticing the following failure:

FAIL: TestAccAzureRMApiManagement_virtualNetworkInternal (1994.83s)
    testing.go:701: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: errors during apply: Error deleting Subnet "acctestSNET-200309190719588572" (Virtual Network "acctestVNET-200309190719588572" / Resource Group "acctestRG-200309190719588572"): network.SubnetsClient#Delete: Failure sending request: StatusCode=400 -- Original Error: Code="InUseSubnetCannotBeDeleted" Message="Subnet acctestSNET-200309190719588572 is in use by /subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/providers/Microsoft.ApiManagement/service?vnetResourceGuid=f0075bd1-4258-45c9-b336-1ebc09e42a70&subnet=acctestSNET-200309190719588572&api-version=2016-07-07 and cannot be deleted. In order to delete the subnet, delete all the resources within the subnet. See aka.ms/deletesubnet." Details=[]

katbyte · 2020-03-10T03:07:40Z

azurerm/internal/services/apimanagement/resource_arm_api_management.go

@@ -674,6 +713,8 @@ func resourceArmApiManagementServiceRead(d *schema.ResourceData, meta interface{
 		d.Set("management_api_url", props.ManagementAPIURL)
 		d.Set("scm_url", props.ScmURL)
 		d.Set("public_ip_addresses", props.PublicIPAddresses)
+		d.Set("private_ip_addresses", props.PrivateIPAddresses)


could we add it to the schema

tombuildsstuff · 2020-03-10T10:53:14Z

@francescopersico unfortunately there's an issue in this version of the API which is fixed in the latest release (but not the intermediate 2019-01-01) release - this is available in the new version of the SDK and as such I'm going to mark this as waiting on that for the moment. Once the SDK been updated we should be able to switch over to this new API version, update the code to match the changes in that version and then update this PR

francescopersico · 2020-03-18T08:31:56Z

@katbyte @tombuildsstuff i have tested and yes there is a problem deleting the resource, the subnet can't be deleted because the APIM deletion process is really long and in background.

This PR supersedes #6049 as nested go modules and merge conflicts do not spark joy - but fundamentally this: - updates github.com/Azure/azure-sdk-for-go to v40.3.0 - updates github.com/Azure/go-autorest to our fork containing Azure/go-autorest#512 - updates github.com/terraform-providers/terraform-provider-azuread to v0.8.0 - code changes needed for v40.3.0 of the Azure SDK - including opting into the old count 429's as requests which should be retried without adding to the total failure count Enables #5769 Enables #5696

cs-marek-ruszczyk · 2020-03-24T11:27:33Z

Hi and thanks for your work, I'd like to ask if you're going to release this in next version with resource deletion problem or more likely you would like to way for improvements on Azure side?

katbyte · 2020-03-24T23:28:50Z

@francescopersico & @cs-marek-ruszczyk,

We have updated the SDK so we should be able to update the API version used now

francescopersico · 2020-04-03T12:27:09Z

@katbyte as @tombuildsstuff said the fix is not included in the API v2019-01-01. So i think we must wait for the update of the sdk to v40.6.0 which includes v2019-12-01.
Am i wrong?

unblocks #2613 #5769

katbyte · 2020-04-22T17:15:17Z

@francescopersico - API update has been merged!

jonatasbaldin · 2020-04-22T17:44:36Z

Excited to see this coming! Just did a PowerShell script inside Terraform to enable vNet in APIM today heheh thanks for allllll the hard work :)

francescopersico · 2020-04-23T10:24:44Z

@francescopersico - API update has been merged!

Going to update the PR. Thanks.

francescopersico · 2020-04-23T11:27:42Z

@katbyte rebase and change requests done.

katbyte

Thanks @francescopersico! LGTM and running tests now 🚀

katbyte · 2020-04-23T15:07:36Z

azurerm/internal/services/apimanagement/resource_arm_api_management.go

@@ -64,6 +64,14 @@ func resourceArmApiManagementService() *schema.Resource {
 				},
 			},

+			"private_ip_addresses": {


We typically group computed properties at the bottom of the schema, ie required -> optional -> computes

…vnet-integration

katbyte · 2020-04-23T19:52:37Z

@francescopersico - i had to add a default to get the tests to pass, but pushed the changes and will merge as soon as travis finishes 🙂

ghost · 2020-04-23T23:31:01Z

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

dansali · 2020-04-25T09:22:33Z

Thank you guys!!!

dansali · 2020-04-25T09:25:11Z

@katbyte

This change also includes a new resource variable "private_ip_addresses" which was missed in the documentation. I think it should also be added in? :)

website/docs/r/api_management.html.markdown

private_ip_addresses - Private Internal IP addresses of the API Management service in the virtual network. Available only for Basic, Standard and Premium SKU after VNet integration.

unblocks hashicorp#2613 hashicorp#5769

ghost · 2020-05-24T14:50:31Z

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 added the size/M label Feb 15, 2020

francescopersico requested review from tombuildsstuff and katbyte February 15, 2020 08:24

ghost added the documentation label Feb 15, 2020

tombuildsstuff mentioned this pull request Feb 18, 2020

azurerm_api_management: Deploy APIM in a vnet #5494

Closed

2 tasks

tombuildsstuff added enhancement service/api-management labels Feb 18, 2020

tombuildsstuff added this to the v2.1.0 milestone Feb 18, 2020

katbyte requested changes Mar 10, 2020

View reviewed changes

katbyte added the waiting-response label Mar 10, 2020

tombuildsstuff added sdk/requires-newer-api-version This requires upgrading the version of the API being used sdk/requires-upgrade This is dependent upon upgrading an SDK labels Mar 10, 2020

katbyte mentioned this pull request Mar 10, 2020

dependencies: upgrade azure-sdk-for-go to v40.3.0 #6049

Closed

tombuildsstuff modified the milestones: v2.1.0, v2.2.0 Mar 10, 2020

tombuildsstuff mentioned this pull request Mar 17, 2020

Updating the Azure SDK to v40.3.0 / AzureAD to v0.8.0 #6134

Merged

ghost removed the waiting-response label Mar 18, 2020

katbyte modified the milestones: v2.2.0, v2.3.0 Mar 18, 2020

tombuildsstuff modified the milestones: v2.3.0, v2.5.0 Mar 25, 2020

katbyte added a commit that referenced this pull request Apr 22, 2020

provider: upgrade API Management to 2019-12-01 (#6479)

7253b6e

unblocks #2613 #5769

francescopersico added 5 commits April 23, 2020 12:58

Add API Management vnet integration

131e088

Add tests

a664951

Add website documentation

560f383

Add private_ip_addresses in schema

66e6d13

Add provider to tests

06db1d3

francescopersico force-pushed the add-api-management-vnet-integration branch from c96a325 to 06db1d3 Compare April 23, 2020 11:09

ghost added size/L and removed size/M labels Apr 23, 2020

francescopersico added 3 commits April 23, 2020 13:12

Add subnet_id validation

76de8e1

Change error message to adhere conventions

8100760

Use .12 syntax for tests

900f945

katbyte approved these changes Apr 23, 2020

View reviewed changes

katbyte added 2 commits April 23, 2020 09:26

Merge remote-tracking branch 'origin/master' into add-api-management-…

dc3fe40

…vnet-integration

add default to apim network type

9f165ba

katbyte changed the title ~~Add API Management vnet integration~~ azurerm_api_management - support for virtual network integration Apr 23, 2020

katbyte merged commit 8b8adca into hashicorp:master Apr 23, 2020

katbyte added a commit that referenced this pull request Apr 23, 2020

update CHANGEOG.md to include #5769

b6e889d

jonatasbaldin mentioned this pull request Apr 24, 2020

azurerm_api_management_api_version_set fails to update or craeate after 2.7.0 #6609

Closed

neil-yechenwei pushed a commit to neil-yechenwei/terraform-provider-azurerm that referenced this pull request Apr 26, 2020

provider: upgrade API Management to 2019-12-01 (hashicorp#6479)

8786915

unblocks hashicorp#2613 hashicorp#5769

ghost locked and limited conversation to collaborators May 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azurerm_api_management - support for virtual network integration #5769

azurerm_api_management - support for virtual network integration #5769

francescopersico commented Feb 15, 2020 •

edited by tombuildsstuff

Loading

nsmeef commented Mar 6, 2020

dansali commented Mar 6, 2020

katbyte left a comment

katbyte Mar 10, 2020

francescopersico Apr 23, 2020

katbyte Mar 10, 2020

francescopersico Apr 23, 2020

katbyte Mar 10, 2020

katbyte Mar 10, 2020

katbyte Mar 10, 2020

francescopersico Apr 23, 2020

katbyte left a comment

katbyte Mar 10, 2020

tombuildsstuff commented Mar 10, 2020 •

edited

Loading

francescopersico commented Mar 18, 2020

cs-marek-ruszczyk commented Mar 24, 2020 •

edited

Loading

katbyte commented Mar 24, 2020

francescopersico commented Apr 3, 2020

katbyte commented Apr 22, 2020

jonatasbaldin commented Apr 22, 2020

francescopersico commented Apr 23, 2020

francescopersico commented Apr 23, 2020

katbyte left a comment

katbyte Apr 23, 2020 •

edited

Loading

katbyte commented Apr 23, 2020

ghost commented Apr 23, 2020

dansali commented Apr 25, 2020

dansali commented Apr 25, 2020 •

edited

Loading

ghost commented May 24, 2020

azurerm_api_management - support for virtual network integration #5769

azurerm_api_management - support for virtual network integration #5769

Conversation

francescopersico commented Feb 15, 2020 • edited by tombuildsstuff Loading

nsmeef commented Mar 6, 2020

dansali commented Mar 6, 2020

katbyte 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff commented Mar 10, 2020 • edited Loading

francescopersico commented Mar 18, 2020

cs-marek-ruszczyk commented Mar 24, 2020 • edited Loading

katbyte commented Mar 24, 2020

francescopersico commented Apr 3, 2020

katbyte commented Apr 22, 2020

jonatasbaldin commented Apr 22, 2020

francescopersico commented Apr 23, 2020

francescopersico commented Apr 23, 2020

katbyte left a comment

Choose a reason for hiding this comment

katbyte Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

katbyte commented Apr 23, 2020

ghost commented Apr 23, 2020

dansali commented Apr 25, 2020

dansali commented Apr 25, 2020 • edited Loading

ghost commented May 24, 2020

francescopersico commented Feb 15, 2020 •

edited by tombuildsstuff

Loading

tombuildsstuff commented Mar 10, 2020 •

edited

Loading

cs-marek-ruszczyk commented Mar 24, 2020 •

edited

Loading

katbyte Apr 23, 2020 •

edited

Loading

dansali commented Apr 25, 2020 •

edited

Loading