azurerm_api_management: Deploy APIM in a vnet #5494
Conversation
There was a problem hiding this comment.
Hi @pacon-vib,
Thank you for the PR! A couple things that need to be addressed before we merge:
- Could we support the other values?
- put them all into their own block?
- ensure we read all the values in on read
- add them to the complete & update tests
- update the docs
| @@ -514,6 +537,12 @@ func resourceArmApiManagementServiceCreateUpdate(d *schema.ResourceData, meta in | |||
| CustomProperties: customProperties, | |||
| Certificates: certificates, | |||
| HostnameConfigurations: hostnameConfigurations, | |||
| VirtualNetworkType: virtualNetworkTypeStringToType(utils.String(d.Get("vnet_type").(string))), | |||
There was a problem hiding this comment.
We should be able to go
| VirtualNetworkType: virtualNetworkTypeStringToType(utils.String(d.Get("vnet_type").(string))), | |
| VirtualNetworkType: apimanagement.VirtualNetworkType(d.Get("vnet_type").(string))), |
| @@ -62,6 +62,14 @@ func resourceArmApiManagementService() *schema.Resource { | |||
| }, | |||
| }, | |||
|
|
|||
| "private_ip_addresses": { | |||
There was a problem hiding this comment.
Could we put computed values at the end of the block?
| Vnetid: utils.String(""), | ||
| Subnetname: utils.String(""), |
There was a problem hiding this comment.
COuld we also support these two values?
| @@ -657,6 +686,7 @@ 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) | |||
There was a problem hiding this comment.
We should we reading in the other new values here too
| @@ -62,6 +62,14 @@ func resourceArmApiManagementService() *schema.Resource { | |||
| }, | |||
| }, | |||
|
|
|||
There was a problem hiding this comment.
Could we put these new values into a virtual_network block?
|
hey @pacon-vib Thanks for this PR :) Since we've not heard back from you here and another PR's been opened which also adds support for Virtual Networks to API Management (but which contains some of the changes mentioned above) - I hope you don't mind but I'm going to close this in favour of #5769. 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 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Long time user, first time contributor. This PR exposes the existing functionality in the Azure SDK to tell an API Management (APIM) service that it needs to live in a vnet, in either internal or external mode. In a vnet in internal mode, we need to export the service's private IP address so that we can create DNS records pointing to it, so this PR does that too.
I'm posting this just to get some early 👀 on it to check I'm on the right track, but I believe I have some more things to do before this is ready to merge. Here's my TODO list, please let me know if there's anything I've missed:
I'm pretty sure
virtualNetworkTypeStringToType()is less elegant than it could be, and may not accord with naming conventions, so I'd be grateful for your suggestions. I'm still finding my way around all the helper functions (and around Golang itself), so any pointers would be helpful.