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

[FallenHoot]

It works with multiple objects as well. I never changed how the test works, but I understand the need to keep the AVM specs.

I’ve spent considerable time attempting to implement a simple fix for this module. However, due to the numerous linter errors in the current version, it wasn't feasible to proceed.

I will simply go back to not using this module going forward until someone fixes the main linter issues, that caused me to change all of this in the first place. As it sits now, it can't deploy with the latest linters.

@AlexanderSehr, can we get the team to fix the below error?
Expected $null or empty, because required parameters in the template file should have a description that starts with "Required.". Found incorrect items: [managedIdentities.userAssignedResourceIds]., but got 'managedIdentities.userAssignedResourceIds'.

This is the main reason why I had to change how this worked.

[AlexanderSehr]

Hey @FallenHoot,
please excuse the late reply. Let me look into this in the next couple days - I just returned from some time off.

[AlexanderSehr]

Hey @FallenHoot,
I can't find the reason for the Expected $null or empty, because required parameters in the template file should have a description that starts with "Required.". Found incorrect items: [managedIdentities.userAssignedResourceIds]., but got 'managedIdentities.userAssignedResourceIds'. error as the userAssignedResourceIds property of the corresponding type looks absolutely fine to me.
Anyways - considering the latest state of the module in main this should not be an issue anymore anyways as the local UDT was replaced with a reference from the avm-common-types utility module:

bicep-registry-modules/avm/res/db-for-my-sql/flexible-server/main.bicep

Lines 72 to 74 in 701814f

	import { managedIdentityOnlyUserAssignedType } from 'br/public:avm/utl/types/avm-common-types:0.4.0'
	@description('Conditional. The managed identity definition for this resource. Required if \'customerManagedKey\' is not empty.')
	param managedIdentities managedIdentityOnlyUserAssignedType?

I'd recommend pulling in the latest changes from main as at least the reason for this comment should be gone now :)

[hundredacres]

@FallenHoot see if this #3959 does what you're needing here.