hashicorp / terraform-provider-azurerm

Terraform provider for Azure Resource Manager
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs
Mozilla Public License 2.0
4.6k stars 4.65k forks source link

azurerm_api_management zones requesting public_ip_address to be provided #16783

Open Radamonas opened 2 years ago

Radamonas commented 2 years ago

Is there an existing issue for this?

Community Note

Terraform Version

1.1.7

AzureRM Provider Version

3.5.0

Affected Resource(s)/Data Source(s)

azurerm_api_management

Terraform Configuration Files

resource "azurerm_api_management" "apim" {
  name                      = "name-${lower(var.environment)}"
  location                  = data.azurerm_resource_group.rg.location
  resource_group_name       = data.azurerm_resource_group.rg.name
  publisher_name            = var.publisher_name
  publisher_email           = var.publisher_email
  notification_sender_email = var.notification_sender_email
  tags                      = local.common_tags
  sku_name                  = var.sku
  zones                     = var.av_zones_enabled ? ["1", "2"] : null

  tenant_access {
    enabled = true
  }

  identity {
    type = "SystemAssigned"
  }

  sign_up {
    enabled = true
    terms_of_service {
      consent_required = true
      enabled          = true
      text             = "Text"
    }
  }
}

Debug Output/Panic Output

Terraform plan:
#azurerm_api_management.apim will be created
+ resource "azurerm_api_management" "apim" {
   + client_certificate_enabled    = false
   + developer_portal_url          = (known after apply)
   + gateway_disabled              = false
   + gateway_regional_url          = (known after apply)
   + gateway_url                   = (known after apply)
   + id                            = (known after apply)
   + location                      = "westeurope"
   + management_api_url            = (known after apply)
   + name                          = "apim-name"
   + notification_sender_email     = "EMAIL"
   + policy                        = (known after apply)
   + portal_url                    = (known after apply)
   + private_ip_addresses          = (known after apply)
   + public_ip_addresses           = (known after apply)
   + public_network_access_enabled = true
   + publisher_email               = "EMAIL"
   + publisher_name                = "COMPANY"
   + resource_group_name           = "RG_NAME
   + scm_url                       = (known after apply)
   + sku_name                      = "Premium_1"
   + tags                          = {
       + "product" = "productName"
       + "team"    = "teamName"
     }
   + virtual_network_type          = "None"
   + zones                         = [
       + "1",
       + "2",
     ]

   + hostname_configuration {
       + developer_portal {
           + certificate                     = (sensitive value)
           + certificate_password            = (sensitive value)
           + expiry                          = (known after apply)
           + host_name                       = (known after apply)
           + key_vault_id                    = (known after apply)
           + negotiate_client_certificate    = (known after apply)
           + ssl_keyvault_identity_client_id = (known after apply)
           + subject                         = (known after apply)
           + thumbprint                      = (known after apply)
         }

       + management {
           + certificate                     = (sensitive value)
           + certificate_password            = (sensitive value)
           + expiry                          = (known after apply)
           + host_name                       = (known after apply)
           + key_vault_id                    = (known after apply)
           + negotiate_client_certificate    = (known after apply)
           + ssl_keyvault_identity_client_id = (known after apply)
           + subject                         = (known after apply)
           + thumbprint                      = (known after apply)
         }

       + portal {
           + certificate                     = (sensitive value)
           + certificate_password            = (sensitive value)
           + expiry                          = (known after apply)
           + host_name                       = (known after apply)
           + key_vault_id                    = (known after apply)
           + negotiate_client_certificate    = (known after apply)
           + ssl_keyvault_identity_client_id = (known after apply)
           + subject                         = (known after apply)
           + thumbprint                      = (known after apply)
         }

       + proxy {
           + certificate                     = (sensitive value)
           + certificate_password            = (sensitive value)
           + default_ssl_binding             = (known after apply)
           + expiry                          = (known after apply)
           + host_name                       = (known after apply)
           + key_vault_id                    = (known after apply)
           + negotiate_client_certificate    = (known after apply)
           + ssl_keyvault_identity_client_id = (known after apply)
           + subject                         = (known after apply)
           + thumbprint                      = (known after apply)
         }

       + scm {
           + certificate                     = (sensitive value)
           + certificate_password            = (sensitive value)
           + expiry                          = (known after apply)
           + host_name                       = (known after apply)
           + key_vault_id                    = (known after apply)
           + negotiate_client_certificate    = (known after apply)
           + ssl_keyvault_identity_client_id = (known after apply)
           + subject                         = (known after apply)
           + thumbprint                      = (known after apply)
         }
     }

   + identity {
       + principal_id = (known after apply)
       + tenant_id    = (known after apply)
       + type         = "SystemAssigned"
     }

   + protocols {
       + enable_http2 = (known after apply)
     }

   + security {
       + enable_backend_ssl30                                = (known after apply)
       + enable_backend_tls10                                = (known after apply)
       + enable_backend_tls11                                = (known after apply)
       + enable_frontend_ssl30                               = (known after apply)
       + enable_frontend_tls10                               = (known after apply)
       + enable_frontend_tls11                               = (known after apply)
       + tls_ecdhe_ecdsa_with_aes128_cbc_sha_ciphers_enabled = (known after apply)
       + tls_ecdhe_ecdsa_with_aes256_cbc_sha_ciphers_enabled = (known after apply)
       + tls_ecdhe_rsa_with_aes128_cbc_sha_ciphers_enabled   = (known after apply)
       + tls_ecdhe_rsa_with_aes256_cbc_sha_ciphers_enabled   = (known after apply)
       + tls_rsa_with_aes128_cbc_sha256_ciphers_enabled      = (known after apply)
       + tls_rsa_with_aes128_cbc_sha_ciphers_enabled         = (known after apply)
       + tls_rsa_with_aes128_gcm_sha256_ciphers_enabled      = (known after apply)
       + tls_rsa_with_aes256_cbc_sha256_ciphers_enabled      = (known after apply)
       + tls_rsa_with_aes256_cbc_sha_ciphers_enabled         = (known after apply)
       + triple_des_ciphers_enabled                          = (known after apply)
     }

   + sign_in {
       + enabled = (known after apply)
     }

   + sign_up {
       + enabled = true

       + terms_of_service {
           + consent_required = true
           + enabled          = true
           + text             = "text"
         }
     }

   + tenant_access {
       + enabled       = true
       + primary_key   = (sensitive value)
       + secondary_key = (sensitive value)
       + tenant_id     = (known after apply)
     }
 }

Expected Behaviour

APIM created with two defined zones. There is no dependency described between zones and public_ip_address and how to handle such cases.

Actual Behaviour

Error during apply:

azurerm_api_management.apim: Creating...
╷
│ Error: `public_ip_address` must be specified when `zones` are provided
│ 
│   with azurerm_api_management.apim,
│   on main.tf line 45, in resource "azurerm_api_management" "apim":
│   45: resource "azurerm_api_management" "apim" {

Steps to Reproduce

No response

Important Factoids

No response

References

No response

IanBrown-OAG commented 2 years ago

I have also noticed this behaviour. Creating an APIM instance via the Azure portal, and adding extra av zones is possible and does not require a distinct public ip address or a VNET, but importing it into terraform shows that it has a public IP, but no ID associated with it. So you cannot even import the existing config to then manage with TF

"public_ip_address_id": "", "public_ip_addresses": [ "127.0.0.1" ], }, "virtual_network_configuration": [], "virtual_network_type": "None", "zones": [ "1", "3" ]
Radamonas commented 2 years ago

@jackofallops isn't that a bug if Azure portal UI is acting differently compared to terraform?

Radamonas commented 2 years ago

Checked the code, and it seems that this is just simple check. The Azure APIM service allows to change availability zones via UI without creating additional resources.

Also please note, that error message `public_ip_address` must be specified when `zones` are provided incorrect as there is no such setting in resource, there is public_ip_address_id

    if v := d.Get("zones").(*schema.Set).List(); len(v) > 0 {
        if sku.Name != apimanagement.SkuTypePremium {
            return fmt.Errorf("`zones` is only supported when sku type is `Premium`")
        }

        if publicIpAddressId == "" {
            return fmt.Errorf("`public_ip_address` must be specified when `zones` are provided")
        }
        zones := zones.Expand(v)
        properties.Zones = &zones
    }
tao-zhang commented 2 years ago

Yes, I hit the exact same issue, I try to create a Premium APIM service without vNet integration, but want to enable multiple zones support.

edwardccg commented 2 years ago

I was able to create with zones, we need to provide public_ip_address_id same snippet below (note: my instance is running in a VNET)

resource "azurerm_public_ip" "apim_zone_public_ip" {
  name                    = "${lower(local.apim_name)}-public-ip"
  resource_group_name     = azurerm_resource_group.example.name
  location                = azurerm_resource_group.example.location
  allocation_method       = "Static"
  sku                     = "Standard"
  sku_tier                = "Regional"
  zones                   = [1,2,3]
  idle_timeout_in_minutes = 4
  ip_version              = "IPv4"
  domain_name_label       = lower(local.apim_name)
}

resource "azurerm_api_management" "api_management" {
  name                = local.apim_name
  location            = azurerm_resource_group.example.location
  resource_group_name = azurerm_resource_group.example.name
  publisher_name      = "My Company"
  publisher_email     = "company@terraform.io"

  sku_name                  = "Premium_2"
  zones                     = [1,2]
  public_ip_address_id      = azurerm_public_ip.apim_zone_public_ip.id
}
IanBrown-OAG commented 2 years ago

Agree with EdwardCCG comment, but this is not the behaviour expected as you can create an APIM without specifying a public ip, Azure assigns one but it does not have an id you can pull back, so you cannot use it. Which makes this difficult for apims which are already live

scott-doyland-burrows commented 2 years ago

I hit this issue, or at least I think I did, as the entire APIM/publicIP/zones is really confusing.

I found that if I wanted to create an APIM that was zone redundant, I needed to supply an IP address created earlier azurerm_public_ip and be sure to specify the zones when creating the IP.

Then in the APIM resource I referenced the IP (and set the zones in the APIM resource as well).

The issue with allowing APIM to create its own public IP, is that it does not create a zone redundant IP - or at least it doesn't via the terraform resource, I didn't try via the portal.

Then stv1 and stv2 comes into play as well: https://docs.microsoft.com/en-us/azure/api-management/compute-infrastructure https://docs.microsoft.com/en-us/azure/api-management/api-management-using-with-vnet?tabs=stv2#prerequisites

Also - when stv2 is used, which it will be if using a vnet and redundancy, the subnet must not be delegated at all, not even to APIM, or the deploy will fail.

And you must have a NSG attached to the subnet before APIM starts to deploy to an stv2, so terraform needs a depends_on for the NSG resource if running it all via one terraform apply.

Also be sure that the NSG has the correct ports opened, or the deploy will fail. You will need port 6390 as referenced here if deploying a Premium in multi-zones: https://docs.microsoft.com/en-us/azure/api-management/api-management-using-with-vnet?tabs=stv2#configure-nsg-rules

Radamonas commented 2 years ago

@jackofallops so I am wondering what is the situation with this ticket? Is it planned to have a fix?

IanBrown-OAG commented 2 years ago

I agree, at the minute you can have an APIM deployed via an earlier version of TF which cannot be moved to a multi av zone config without adding a fixed ip and vnet/subnet config, which the azure gui does not require.

SeanGraySage commented 1 year ago

I'm also hitting issues with this. I don't have things set up for building and testing the provider myself at the moment, but if anyone does, I think this problem could be resolved simply by deleting lines 810-812 in /internal/services/apimanagement/api_management_resource.go

They look like this:

if publicIpAddressId == "" {
    return fmt.Errorf("`public_ip_address` must be specified when `zones` are provided")
}
scott-doyland-burrows commented 1 year ago

I'm also hitting issues with this. I don't have things set up for building and testing the provider myself at the moment, but if anyone does, I think this problem could be resolved simply by deleting lines 810-812 in /internal/services/apimanagement/api_management_resource.go

They look like this:

if publicIpAddressId == "" {
  return fmt.Errorf("`public_ip_address` must be specified when `zones` are provided")
}

If you are creating APIM in a vnet then you must specify a pre-created IP (it requests this via the portal as well as via terraform).

I haven't looked at /internal/services/apimanagement/api_management_resource.go as it wouldn't make much sense to me, but would removing those lines cause issues if an APIM creation attempt inside a vnet was requested if no pre-created IP had been provided.

Potentially there needs to be some logic to request a pre-created IP for a vnet'd APIM and to not request an IP for a non-vnet'd APIM.

SeanGraySage commented 1 year ago

Hello @scott-doyland-burrows,

That's not the issue here - this is checking that a public IP address is specified when availability zones are specified, regardless of whether APIM is in a VNET or not.

Azure does not require (or allow) a public IP address for an APIM instance with availability zones which is not in a VNET, but the provider isn't allowing us to deploy this scenario.

scott-doyland-burrows commented 1 year ago

Hello @scott-doyland-burrows,

That's not the issue here - this is checking that a public IP address is specified when availability zones are specified, regardless of whether APIM is in a VNET or not.

Azure does not require (or allow) a public IP address for an APIM instance with availability zones which is not in a VNET, but the provider isn't allowing us to deploy this scenario.

If those lines are removed and: we are using a vnet AND we do not supply an IP

then terraform will consider it a valid scenario and attempt to create APIM - and fail as it has no IP supplied.

Wouldn't it need some logic such as (not actual code):

if publicIpAddressId == "" and Vnet != "" {
    return fmt.Errorf("`public_ip_address` must be specified when `zones` and `vnet` are provided")
}

This would cause terraform to allow no IP when not in a vnet, and expect an IP when in a vnet.

SeanGraySage commented 1 year ago

Maybe; I think that it would important to test the various scenarios after making any change. I might not be the best person to confirm this, but I think what's really needed is for that check to only be about the public IP address and the VNET - I don't think zones needs to come into it at all, so it should probably be moved to a different part of the file.

imashishchawla commented 1 month ago

Hi Team, any update on this ?