Skip to content
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

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

Closed
1 task done
marshalexander99 opened this issue May 7, 2024 · 9 comments · Fixed by #1939 or #1953
Closed
1 task done

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

marshalexander99 opened this issue May 7, 2024 · 9 comments · Fixed by #1939 or #1953
Assignees
Labels
Class: Resource Module 📦 This is a resource module Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working

Comments

@marshalexander99
Copy link

marshalexander99 commented May 7, 2024

Check for previous/existing GitHub issues

  • I have checked 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

@marshalexander99 marshalexander99 added Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue labels May 7, 2024

Important

The "Needs: Triage 🔍" 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 microsoft-github-policy-service bot added the Type: Bug 🐛 Something isn't working label May 7, 2024

Note

The "Type: Bug 🐛" label was added as per ITA21.

@avm-team-linter avm-team-linter bot added the Class: Resource Module 📦 This is a resource module label May 7, 2024
Copy link

@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!

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 🔍" 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.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days label May 10, 2024
@hundredacres
Copy link
Contributor

Taking a look

@hundredacres hundredacres removed the Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days label May 10, 2024
@AlexanderSehr AlexanderSehr removed the Needs: Triage 🔍 Maintainers need to triage still label May 11, 2024
@hundredacres
Copy link
Contributor

@marshalexander99 I've got a PR that addresses the issues when specifying a Public IP Prefix for the Management IP here: #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 pushed a commit that referenced this issue May 16, 2024
…1939)

## Description

Resolved issue when specifying a public IP prefix for the management IP
address. Updated to use latest PublicIPAddress AVM module. Updated API
version of Microsoft.Network/publicIPAddresses used in tests. Updated
formatting of zone param default values. Also added new e2e tests for
Public IP Prefix usage.

Fixes #1867 
Closes #1867 

## Pipeline Reference

| Pipeline |
| -------- |
|
[![avm.res.network.azure-firewall](https://github.com/hundredacres/bicep-registry-modules/actions/workflows/avm.res.network.azure-firewall.yml/badge.svg?branch=fix%2Fissue%2F1867)](https://github.com/hundredacres/bicep-registry-modules/actions/workflows/avm.res.network.azure-firewall.yml)
|

## Type of Change

<!-- Use the check-boxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utlities (Non-module effecting
changes)
- [X] Azure Verified Module updates:
- [ ] Bugfix containing backwards compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [X] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [X] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [X] Update to documentation

## Checklist

- [X] I'm sure there are no other open Pull Requests for the same
update/change
- [X] I have run `Set-AVMModule` locally to generate the supporting
module files.
- [X] My corresponding pipelines / checks run clean and green without
any errors or warnings

---------

Co-authored-by: Máté Barabás <[email protected]>
Co-authored-by: Rainer Halanek <[email protected]>
Co-authored-by: JFolberth <[email protected]>
@github-project-automation github-project-automation bot moved this from Needs: Triage to Done in AVM - Module Issues May 16, 2024
@AlexanderSehr AlexanderSehr reopened this May 16, 2024
@AlexanderSehr
Copy link
Contributor

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

@hundredacres
Copy link
Contributor

@AlexanderSehr New PR: #1953

@AlexanderSehr AlexanderSehr linked a pull request May 17, 2024 that will close this issue
11 tasks
@AlexanderSehr
Copy link
Contributor

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

hundredacres added a commit to hundredacres/bicep-registry-modules that referenced this issue Jun 19, 2024
…zure#1939)

## Description

Resolved issue when specifying a public IP prefix for the management IP
address. Updated to use latest PublicIPAddress AVM module. Updated API
version of Microsoft.Network/publicIPAddresses used in tests. Updated
formatting of zone param default values. Also added new e2e tests for
Public IP Prefix usage.

Fixes Azure#1867 
Closes Azure#1867 

## Pipeline Reference

| Pipeline |
| -------- |
|
[![avm.res.network.azure-firewall](https://github.com/hundredacres/bicep-registry-modules/actions/workflows/avm.res.network.azure-firewall.yml/badge.svg?branch=fix%2Fissue%2F1867)](https://github.com/hundredacres/bicep-registry-modules/actions/workflows/avm.res.network.azure-firewall.yml)
|

## Type of Change

<!-- Use the check-boxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utlities (Non-module effecting
changes)
- [X] Azure Verified Module updates:
- [ ] Bugfix containing backwards compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [X] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [X] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [X] Update to documentation

## Checklist

- [X] I'm sure there are no other open Pull Requests for the same
update/change
- [X] I have run `Set-AVMModule` locally to generate the supporting
module files.
- [X] My corresponding pipelines / checks run clean and green without
any errors or warnings

---------

Co-authored-by: Máté Barabás <[email protected]>
Co-authored-by: Rainer Halanek <[email protected]>
Co-authored-by: JFolberth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Class: Resource Module 📦 This is a resource module Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working
Projects
3 participants