-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Added Virtual Network Integration to the API Management service #2582
Added Virtual Network Integration to the API Management service #2582
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @joranm,
Hi @katbyte, I've found the issue why the tests were failing. Fixed the issue by adding additional checks if the virtual_network_configuration object is present. Additionally I've found that the Virtual Network Type will always return None from Azure. So I've added this as the default value of the property. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @joranm,
We are still seeing this error:
------- Stdout: -------
=== RUN TestAccAzureRMApiManagement_complete
=== PAUSE TestAccAzureRMApiManagement_complete
=== CONT TestAccAzureRMApiManagement_complete
--- FAIL: TestAccAzureRMApiManagement_complete (99.17s)
testing.go:538: Step 0 error: Error applying: 1 error(s) occurred:
* azurerm_api_management.test: 1 error(s) occurred:
* azurerm_api_management.test: Error creating/updating API Management Service "acctestAM-8760723735339727174" (Resource Group "acctestRG-8760723735339727174"): apimanagement.ServiceClient#CreateOrUpdate: Failure sending request: StatusCode=0 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="InvalidParameters" Message="Invalid parameter: Virtual Network configuration not found for location(s) eastus2"
FAIL
Is this only available in select regions at the moment?
Hi @katbyte, I've found that if the additional_location object is present, the virtual_network_configuration should also be present to make this work. Otherwise it reverses to the "default" virtual_network_configuration in the first location (which fails). This requires some rework and testing from my side which I will try to perform this week. |
Hi @joranm, this is great Could I ask that the private_ip_addresses attribute be added as part of this PR? Very useful when virtual_network_type is set to Internal If not then I can look to add it after |
@tombuildsstuff and @katbyte, I've
|
Thanks for the updates @joranm, I am still getting an error on our CI server, different this time however 😅
|
Hi @katbyte, it looks like the Azure platform has some long time lease on the subnet. When the API Management Service is deleted there is still a lock on the subnet for 15-30 minutes. Do you have any idea how to handle this? @steve-hawkins @tombuildsstuff |
Hi @joranm, at a guess this is probably due to the API Manager still reliant on classic parts, there is a note in the docs when connecting to a VNET:- This is further explained in the FAQ section @tombuildsstuff I'm pretty sure this has been addressed in other areas of this provider, I just can't remember where? |
hey @joranm Just to give an update here: since we've had to downgrade to API version Rather than leaving this PR open stagnant until that's available (which I've just asked for an update in the issue) - I'm going to assign this to the 'blocked' milestone and temporarily close this PR for the moment. When the other issue's closed (and the SDK is available) we'll re-open this PR and get support for this functionality merged. Thanks! |
Seems like the dependent ticket has been resolved. When will this PR be reopened again? |
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! |
👋 As @keero has mentioned the upstream issue's been fixed: Azure/azure-rest-api-specs#4409 - once the SDK's been updated #3335 we should be able to go to the new version of the API Management API and look to add support for this Thanks! |
hey @joranm Sorry for the bad news - it looks like the new SDK upgrade (in #3446) is still blocked, this time on: Azure/azure-rest-api-specs#6065 Once that's fixed, we'll circle back and fix this PR up - but for the moment as this is blocked on the external/upstream issue I'm going to temporarily re-close this PR, once that's been fixed we'll re-open this and fix this up / take another look 👍 Thanks! |
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! |
Added the virtual_network_type property which allows None, Internal or External
Added virtual_network_configuration object with property subnet_id which references the subnet to integrate with.
See https://docs.microsoft.com/en-us/azure/api-management/api-management-using-with-vnet for details regarding API Management with Vnet integration