fix: Bug Updated delegatedSubnetResourceId to work as attended for db-for-my-sql flexible server module

[FallenHoot]

Description

When to Use mysqlFirewallRule
Public Network Access: If your MySQL flexible server has public network access enabled (publicNetworkAccess: 'Enabled'), you should use firewall rules to restrict access to specific IP addresses or ranges. This helps prevent unauthorized access from the internet.
Specific IP Restrictions: If you need to allow access only from certain IP addresses or ranges, even within a virtual network, firewall rules are necessary.

When You Might Not Need mysqlFirewallRule
Private Network Access Only: If your MySQL flexible server is configured to be accessible only within a virtual network (publicNetworkAccess: 'Disabled'), and you have other network security measures in place (e.g., Network Security Groups, Azure Firewall), you might not need additional firewall rules.
Controlled Environment: If the server is in a highly controlled environment where access is already restricted by other means, firewall rules might be redundant.

Before this fix, you didn't have the option to not use mysqlFirewallRule well still using delegatedSubnetResourceId.

The test in this module don't check for the above bug fix, because they don't have vnet/subnets/dns. I have added another dependency to include those missing test.

Fixes #3717

Pipeline Reference

I assume you need the following:

publicNetworkAccess: Enabled

publicNetworkAccess: Disabled

Pipeline

Type of Change

• Update to CI Environment or utilities (Non-module affecting changes)
• Azure Verified Module updates:
  • Bugfix containing backwards-compatible bug fixes, and I have NOT bumped the MAJOR or MINOR version in version.json:
    • 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.
  • 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.
  • Update to documentation

Checklist

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

[jtracey93]

Hey @FallenHoot ,

Firstly, thanks for your work on this PR!

We have made some changes to the AVM CI, detailed below, which means we need you to update your fork to pull in these latest changes and re-run your tests to show they still are passing prior to approving and merging this PR, as we don't and it fails once merged the publishing of your module will fail and will be blocked going forward until the test pass again via additional PRs.

Changes to CI That Have Been Made That You Need To Take Action On

• feat: Add WAF Security PS Rule Config #3745
  • Adds WAF Security PSRule Checks using this custom baseline of rules (/avm/utilities/pipelines/staticValidation/psrule/.ps-rule/cb-waf-security.Rule.yaml)
  • Action Required By You:
    • Ingest/pull-in these changes into your PR branch and re-run your tests to ensure they pass
      • If any issues with the rules and you think it should apply, please ping/reach out to the core team

Any questions reach out to the AVM Core Team by tagging us in your PR here or internally via Teams

Thanks

Jack (AVM Core Team)

[hundredacres]

@FallenHoot the pipeline badge you can get from your fork and the workflow. See the screenshot.

[FallenHoot]

@hundredacres

I can't complete the pipeline, because of how pester treats "Conditional". How did this get passed in previous code? What is the workaround, without changing how pester is configured?

During the Pester test for validating parameters in an Azure template. The error message suggests that a parameter (managedIdentities) is not labeled as "Required." in its description, even though it is conditionally required based on the value of another parameter (customerManagedKey).

@description('Conditional. The managed identity definition for this resource. Required if 'customerManagedKey' is not empty.')
param managedIdentities managedIdentitiesType

I don't really want to rewrite the logic behind this code, but I do think it is overcomplicated.

My suggestion:

@description('Category. Optional. Resource ID of the user-assigned managed identity.')
param userAssignedIdentityResourceId string = ''

identity: empty(userAssignedIdentityResourceId) ? null : {
  type: 'UserAssigned'
  userAssignedIdentities: {
    '${userAssignedIdentityResourceId}': {}
  }
}

What is in the code today with correct linters

@description('Conditional. The managed identity definition for this resource. Required if \'customerManagedKey\' is not empty.')
param managedIdentities managedIdentitiesType

type managedIdentitiesType = {
  @description('Optional. The resource ID(s) to assign to the resource.')
  userAssignedResourceIds: string[]
}?

var formattedUserAssignedIdentities = reduce(
  map((managedIdentities.?userAssignedResourceIds ?? []), (id) => { '${id}': {} }),
  {},
  (cur, next) => union(cur, next)
) // Converts the flat array to an object like { '${id1}': {}, '${id2}': {} }

var identity = !empty(managedIdentities)
  ? {
      type: !empty(managedIdentities.?userAssignedResourceIds ?? {}) ? 'UserAssigned' : null
      userAssignedIdentities: !empty(formattedUserAssignedIdentities) ? formattedUserAssignedIdentities : null
    }
  : null

The above code is doing 100 things and it only needs to do one thing.

Suggestions/Feedback

[FallenHoot]

I removed most of that "bloat" code and replaced it. Now I am only getting an issue when deploying the Max, because it runs out of time. I don't see this turning green because of that.

[FallenHoot]

@hundredacres

I can't complete the pipeline, because of how pester treats "Conditional". How did this get passed in previous code? What is the workaround, without changing how pester is configured?

During the Pester test for validating parameters in an Azure template. The error message suggests that a parameter (managedIdentities) is not labeled as "Required." in its description, even though it is conditionally required based on the value of another parameter (customerManagedKey).

@description('Conditional. The managed identity definition for this resource. Required if 'customerManagedKey' is not empty.') param managedIdentities managedIdentitiesType

I don't really want to rewrite the logic behind this code, but I do think it is overcomplicated.

My suggestion:

@description('Category. Optional. Resource ID of the user-assigned managed identity.')
param userAssignedIdentityResourceId string = ''

identity: empty(userAssignedIdentityResourceId) ? null : {
  type: 'UserAssigned'
  userAssignedIdentities: {
    '${userAssignedIdentityResourceId}': {}
  }
}

What is in the code today with correct linters

@description('Conditional. The managed identity definition for this resource. Required if \'customerManagedKey\' is not empty.')
param managedIdentities managedIdentitiesType

type managedIdentitiesType = {
  @description('Optional. The resource ID(s) to assign to the resource.')
  userAssignedResourceIds: string[]
}?

var formattedUserAssignedIdentities = reduce(
  map((managedIdentities.?userAssignedResourceIds ?? []), (id) => { '${id}': {} }),
  {},
  (cur, next) => union(cur, next)
) // Converts the flat array to an object like { '${id1}': {}, '${id2}': {} }

var identity = !empty(managedIdentities)
  ? {
      type: !empty(managedIdentities.?userAssignedResourceIds ?? {}) ? 'UserAssigned' : null
      userAssignedIdentities: !empty(formattedUserAssignedIdentities) ? formattedUserAssignedIdentities : null
    }
  : null

The above code is doing 100 things and it only needs to do one thing.

Suggestions/Feedback

Something wrong with this code that doesn't allow the Max to deploy anymore.

[FallenHoot]

Fixed the "Max" issue but now ran into a "Private" issue. It states that you can't run mysql/administrators without having an "Identity" assigned after the latest Bicep Update.

administrators: array | null
Optional. The Azure AD administrators when AAD authentication is enabled. Requires the assignment of userAssignedIdentities.

This was green three days ago:

After Bicep Update:

[FallenHoot]

Please review:

[AlexanderSehr]

Hey @FallenHoot,
be aware this is breaking with several AVM specs (most notably this one). The interface was defined in the way it is to allow or a consistent user experience accross all AVM modules.

I appreciate your comment that for this module it may be an overkill, but in that case I'd rather strongly advice to define a new spec, than to diverge completely.

On that note also one question:

• The original implementation allowed the user to define an array of user-assiged identities via their resource ID
• The new implementation only allows for exact one user assigned identity
  Does the resource only allow for one single identity?

Again, I'd rather like to keep the consistency high and if only one single identity is allowed, stick to the same user-defined type be it only with the change to allow for a single resource ID, and only adjust all the ther pieces.

Before I add even more to this comment, let me wait for your reply and then let's go from there 😄

cc: @Azure/avm-module-reviewers-bicep