Azure / bicep-registry-modules

Bicep registry modules
MIT License
460 stars 305 forks source link

[AVM Module Issue]: Azure-Firewall PIP issue during deployment #1867

Closed marshalexander99 closed 3 months ago

marshalexander99 commented 4 months ago

Check for previous/existing GitHub issues

Issue Type?

Bug

Module Name

avm/res/network/azure-firewall

(Optional) Module Version

0.2.0

Description

With the following bicep config

module deployazurefw 'br/public:avm/res/network/azure-firewall:0.2.0' = {
  scope: resourceGroup('rg-${ResourcePrefix}-platform-01-connectivity-01')
  name: 'deployafw'
  params: {
    name: 'afw-${ResourcePrefix}-platform-01'
    location: deploymentLocation
    azureSkuTier: 'Basic'
    virtualNetworkResourceId: hubvnetresource
    publicIPAddressObject: {
      name: 'pip-platform-${ResourcePrefix}-afw-01'
      publicIPAllocationMethod: 'Static'
      publicIPPrefixResourceId: publicipprefix
      skuName: 'Standard'
      skuTier: 'Regional'
      zones: []
    }
    managementIPAddressObject: {
      name: 'pip-platform-${ResourcePrefix}-afw-mgmt-01'
      publicIPAllocationMethod: 'Static'
      publicIPPrefixResourceId: publicipprefix
      skuName: 'Standard'
      skuTier: 'Regional'
      zones: []
    }
    firewallPolicyId: deployafwp.outputs.resourceId
  }
}

When re-running a deployment which includes the above the following error is generated.

Microsoft.Network/publicIPAddresses/pip-platform-twr-afw-01 has an existing availability zone constraint NoZone and the request has availability zone constraint 1, 2, 3, which do not match. Zones cannot be added/updated/removed once the resource is created. The resource cannot be updated from regional to zonal or vice-versa. (Code: ResourceAvailabilityZonesCannotBeModified) Despite there being no changes made to the configuration when re-deploying. I'm assuming there is something in the template attempting to change the Public IP config? Also, the template ignores using the specified public IP prefix for the management IP address and creates it's own separate public IP (with correct naming as above).

(Optional) Correlation Id

No response

microsoft-github-policy-service[bot] commented 4 months ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

[!NOTE] This label was added as per ITA06.

microsoft-github-policy-service[bot] commented 4 months ago

[!NOTE] The "Type: Bug :bug:" label was added as per ITA21.

avm-team-linter[bot] commented 4 months ago

@marshalexander99, thanks for submitting this issue for the avm/res/network/azure-firewall module!

[!IMPORTANT] A member of the @Azure/avm-res-network-azurefirewall-module-owners-bicep or @Azure/avm-res-network-azurefirewall-module-contributors-bicep team will review it soon!

microsoft-github-policy-service[bot] commented 3 months ago

[!WARNING] Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

[!TIP]

  • To prevent further actions to take effect, the "Status: Response Overdue 🚩" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage :mag:" label must be removed as part of the triage process (when the issue is first responded to)!

[!NOTE] This message was posted as per ITA01BCP.

hundredacres commented 3 months ago

Taking a look

hundredacres commented 3 months ago

@marshalexander99 I've got a PR that addresses the issues when specifying a Public IP Prefix for the Management IP here: https://github.com/Azure/bicep-registry-modules/pull/1939

I also am able to get a repeat deployment without errors when using the following code with my updates in my PR:

module testDeployment '../../../main.bicep' = {
    scope: resourceGroup
    name: '${uniqueString(deployment().name, resourceLocation)}-test-${serviceShort}-${iteration}'
    params: {
      location: resourceLocation
      name: '${namePrefix}${serviceShort}001'
      virtualNetworkResourceId: nestedDependencies.outputs.virtualNetworkResourceId
      publicIPAddressObject: {
        name: 'publicIP01'
        publicIPAllocationMethod: 'Static'
        publicIPPrefixResourceId: nestedDependencies.outputs.publicIPPrefixResourceId
        skuName: 'Standard'
        skuTier: 'Regional'
      }
      azureSkuTier: 'Basic'
      managementIPAddressObject: {
        name: 'managementIP01'
        managementIPAllocationMethod: 'Static'
        managementIPPrefixResourceId: nestedDependencies.outputs.publicIPPrefixResourceId
        skuName: 'Standard'
        skuTier: 'Regional'
      }
      zones: []
      firewallPolicyId: nestedDependencies.outputs.firewallPolicyResourceId
    }
}

Declaring the zones param for the Azure Firewall resource and not individually for each IPAddressObject seems to do the trick, since we just refer to the main zones param and not an IPAddressObject key

module publicIPAddress 'br/public:avm/res/network/public-ip-address:0.4.0' = if (empty(publicIPResourceID) && azureSkuName == 'AZFW_VNet') {
  name: '${uniqueString(deployment().name, location)}-Firewall-PIP'
  params: {
    name: publicIPAddressObject.name
    publicIpPrefixResourceId: contains(publicIPAddressObject, 'publicIPPrefixResourceId')
      ? (!(empty(publicIPAddressObject.publicIPPrefixResourceId)) ? publicIPAddressObject.publicIPPrefixResourceId : '')
      : ''
    publicIPAllocationMethod: contains(publicIPAddressObject, 'publicIPAllocationMethod')
      ? (!(empty(publicIPAddressObject.publicIPAllocationMethod))
          ? publicIPAddressObject.publicIPAllocationMethod
          : 'Static')
      : 'Static'
    skuName: contains(publicIPAddressObject, 'skuName')
      ? (!(empty(publicIPAddressObject.skuName)) ? publicIPAddressObject.skuName : 'Standard')
      : 'Standard'
    skuTier: contains(publicIPAddressObject, 'skuTier')
      ? (!(empty(publicIPAddressObject.skuTier)) ? publicIPAddressObject.skuTier : 'Regional')
      : 'Regional'
    roleAssignments: contains(publicIPAddressObject, 'roleAssignments')
      ? (!empty(publicIPAddressObject.roleAssignments) ? publicIPAddressObject.roleAssignments : [])
      : []
    diagnosticSettings: publicIPAddressObject.?diagnosticSettings
    location: location
    lock: lock
    tags: publicIPAddressObject.?tags ?? tags
    zones: zones
    enableTelemetry: publicIPAddressObject.?enableTelemetry ?? enableTelemetry
  }
}
AlexanderSehr commented 3 months ago

Re-opened the issue as the new version was not published as a test failed. Needs a new PR

hundredacres commented 3 months ago

@AlexanderSehr New PR: https://github.com/Azure/bicep-registry-modules/pull/1953

AlexanderSehr commented 3 months ago

Thanks @hundredacres - the publishing went through and 0.3.0 was published. Closing the issue :)