Skip to content

fix: Add Active Directory site support to NetApp account module AD configuration#7032

Open
fbinotto with Copilot wants to merge 8 commits into
mainfrom
copilot/add-site-property-active-directory
Open

fix: Add Active Directory site support to NetApp account module AD configuration#7032
fbinotto with Copilot wants to merge 8 commits into
mainfrom
copilot/add-site-property-active-directory

Conversation

Copilot AI commented May 12, 2026

Copy link
Copy Markdown
Contributor

Description

The NetApp account module did not expose the Active Directory site field, which blocked parity with existing portal-configured AD integrations during migration to Bicep. This change adds an explicit module input and maps it to the ARM activeDirectories[].site property.

  • Module behavior

    • Added new optional parameter: activeDirectorySite.
    • Wired activeDirectorySite into Microsoft.NetApp/netAppAccounts.properties.activeDirectories[].site.
    • Emits site only when both domainName and activeDirectorySite are non-empty (avoids sending empty string values).
  • Generated artifacts

    • Updated main.json to include the new parameter and compiled mapping.
    • Updated README.md to document activeDirectorySite, keep parameter ordering consistent, and clarify the site description.
    • Updated CHANGELOG.md with a new entry (0.12.2) for this enhancement.
@description('Optional. The Active Directory site (as configured in Active Directory Sites and Services) the service will limit Domain Controller discovery to.')
param activeDirectorySite string = ''

var activeDirectoryConnectionProperties = [
  {
    // ...
    site: !empty(domainName) && !empty(activeDirectorySite) ? activeDirectorySite : null
  }
]

Pipeline Reference

Pipeline
TBD

Type of Change

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

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
  • I have updated the module's CHANGELOG.md file with an entry for the next version

@microsoft-github-policy-service

Copy link
Copy Markdown

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.

@microsoft-github-policy-service microsoft-github-policy-service Bot added the Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue label May 12, 2026
@microsoft-github-policy-service

Copy link
Copy Markdown

Important

If this is a module-related PR, being submitted by the sole owner of the module, the AVM core team must review and approve it (as module owners can't approve their own PRs).

To indicate this PR needs the core team''s attention, apply the "Needs: Core Team 🧞" label!

The core team will only review and approve PRs that have this label applied!

@fbinotto fbinotto requested a review from Copilot May 12, 2026 04:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

@fbinotto

Copy link
Copy Markdown
Contributor

@copilot make the changes based on the plan.

Copilot AI changed the title [WIP] Add site property for activeDirectory configuration in NetApp module Add Active Directory site support to NetApp account module AD configuration May 12, 2026
Copilot AI requested a review from fbinotto May 12, 2026 05:03

Copilot AI commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

@copilot make the changes based on the plan.

Implemented the follow-up changes from the plan, including the module CHANGELOG update for activeDirectorySite support. Addressed in commit d5c7098.

@fbinotto fbinotto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot it failed validation. Fix it.

Error: No release type found in pull request title "Add Active Directory site support to NetApp account module AD configuration". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:

  • feat: A new feature
  • fix: A bug fix
  • docs: Documentation only changes
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • refactor: A code change that neither fixes a bug nor adds a feature
  • perf: A code change that improves performance
  • test: Adding missing tests or correcting existing tests
  • build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
  • ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
  • chore: Other changes that don't modify src or test files
  • revert: Reverts a previous commit

Copilot AI commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

@copilot it failed validation. Fix it.

Error: No release type found in pull request title "Add Active Directory site support to NetApp account module AD configuration". Add a prefix to indicate what ...

Updated the PR title to use a conventional-commit release type prefix (fix: Add Active Directory site support to NetApp account module AD configuration). No code changes were required; current head is d5c7098.

Copilot AI changed the title Add Active Directory site support to NetApp account module AD configuration fix: Add Active Directory site support to NetApp account module AD configuration May 12, 2026
Copilot AI requested a review from fbinotto May 12, 2026 05:32

@fbinotto fbinotto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fbinotto fbinotto marked this pull request as ready for review May 12, 2026 05:34
@fbinotto fbinotto requested review from a team as code owners May 12, 2026 05:34
param serverRootCACertificate string = ''

@description('Optional. The Active Directory site (as configured in Active Directory Sites and Services) the service will limit Domain Controller discovery to.')
param activeDirectorySite string = ''

@eriqua eriqua May 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to make it nullable instead of adding an empty string default value?

Another comment would be that ideally, all paramrters should be validated via at least one e2e test. However I don't think it's applicable in this case

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @fbinotto I see you raised this via copilot. Is this experimental or ready for review?
If ready, could you please add a pipeline badge proving the successful run in your fork?
If not ready, could you please set this as draft?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[AVM Module Issue]: Netapp account module missing site property for activeDirectory configuration

4 participants