Skip to content

[Az.ServiceFabric] Update New and Set commands for Managed Clusters with new parameters #29263

Draft
iliu816 wants to merge 5 commits intoAzure:mainfrom
iliu816:user/iliu/addParameters
Draft

[Az.ServiceFabric] Update New and Set commands for Managed Clusters with new parameters #29263
iliu816 wants to merge 5 commits intoAzure:mainfrom
iliu816:user/iliu/addParameters

Conversation

@iliu816
Copy link
Copy Markdown
Member

@iliu816 iliu816 commented Mar 11, 2026

Description

  • Fixed Set-AzServiceFabricManagedClusterApplication to use correct resource completer for managed clusters instead of classic clusters.
  • Added parameters -IdentityType, -UserAssignedIdentityId, and -ApplicationManagedIdentity to New-AzServiceFabricManagedClusterApplication to support managed identity configuration on applications.
  • Added parameters -IdentityType, -UserAssignedIdentityId, and -ApplicationManagedIdentity to Set-AzServiceFabricManagedClusterApplication to support managed identity configuration on applications.
  • Added parameters -AzureActiveDirectoryClientApplication, -AzureActiveDirectoryClusterApplication, -AzureActiveDirectoryTenantId, -EnableHttpGatewayExclusiveAuthMode, -HttpGatewayTokenAuthConnectionPort, -MaxPercentUnhealthyApplications, -MaxPercentUnhealthyNodes, -MaxUnusedVersionsToKeep, -AddonFeature, -DdosProtectionPlanId, -EnableIpv6, -EnableServicePublicIP, -PublicIPPrefixId, -PublicIPv6PrefixId, -SubnetId, -UseCustomVnet, -VMImage, -AllocatedOutboundPort, -EnableOutboundOnlyNodeTypes, and -SkipManagedNsgAssignment to New-AzServiceFabricManagedCluster.
  • Added parameters -AzureActiveDirectoryClientApplication, -AzureActiveDirectoryClusterApplication, -AzureActiveDirectoryTenantId, -EnableHttpGatewayExclusiveAuthMode, -HttpGatewayTokenAuthConnectionPort, -MaxPercentUnhealthyApplications, -MaxPercentUnhealthyNodes, -MaxUnusedVersionsToKeep, -AddonFeature, -DdosProtectionPlanId, -PublicIPPrefixId, -PublicIPv6PrefixId, -VMImage, -AllocatedOutboundPort, -EnableOutboundOnlyNodeTypes, and -SkipManagedNsgAssignment to Set-AzServiceFabricManagedCluster.
  • Enabled Manual option for the -UpgradeMode parameter in Set-AzServiceFabricManagedCluster.
  • Added parameters -IsOutboundOnly, -EnableResilientEphemeralOSDisk, -EnableAcceleratedNetworking, -EnableEncryptionAtHost, -EnableNodePublicIP, -EnableNodePublicIPv6, -SecureBootEnabled, and -UseEphemeralOSDisk to New-AzServiceFabricManagedNodeType.
  • Added parameters -IsOutboundOnly, -EnableResilientEphemeralOSDisk, -EnableAcceleratedNetworking, -EnableEncryptionAtHost, -EnableNodePublicIP, -EnableNodePublicIPv6, -SecureBootEnabled, and -UseEphemeralOSDisk to Set-AzServiceFabricManagedNodeType.
  • Added parameter -ServiceDnsName to New-AzServiceFabricManagedClusterService for DNS-based service discovery.

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Copilot AI review requested due to automatic review settings March 11, 2026 16:57
@azure-client-tools-bot-prd
Copy link
Copy Markdown

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Az.ServiceFabric managed cluster cmdlets and PowerShell model wrappers to surface newly supported Service Fabric Managed Cluster, Node Type, Service, and Application properties from the underlying management SDK models.

Changes:

  • Expanded New-/Set-AzServiceFabricManagedCluster to support additional networking, infrastructure, security/auth, and health policy parameters.
  • Added new Node Type parameters (e.g., outbound-only, resilient ephemeral OS disk, secure boot, etc.) to New-/Set-AzServiceFabricManagedNodeType.
  • Added managed application identity parameters and service DNS name support for managed cluster applications/services, plus corresponding model mapping updates.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
src/ServiceFabric/ServiceFabric/Models/PSCluster.cs Updates wrapper constructor to pass through newer SDK Cluster properties.
src/ServiceFabric/ServiceFabric/Models/ManagedClusters/PSManagedService.cs Adjusts managed service projection/mapping; removes SystemData from output model.
src/ServiceFabric/ServiceFabric/Models/ManagedClusters/PSManagedNodeType.cs Maps newly added NodeType SDK properties into PS wrapper.
src/ServiceFabric/ServiceFabric/Models/ManagedClusters/PSManagedCluster.cs Maps newly added ManagedCluster SDK properties into PS wrapper.
src/ServiceFabric/ServiceFabric/Models/ManagedClusters/PSManagedApplicationTypeVersion.cs Reorders base ctor args to match updated SDK signatures.
src/ServiceFabric/ServiceFabric/Models/ManagedClusters/PSManagedApplicationType.cs Reorders base ctor args to match updated SDK signatures.
src/ServiceFabric/ServiceFabric/Models/ManagedClusters/PSManagedApplication.cs Updates mapping to include identity/managed identities in base ctor.
src/ServiceFabric/ServiceFabric/Commands/ManagedClusters/Services/NewAzServiceFabricManagedClusterService.cs Adds -ServiceDnsName and maps it into service properties.
src/ServiceFabric/ServiceFabric/Commands/ManagedClusters/NodeTypes/SetAzServiceFabricManagedNodeType.cs Adds multiple new updatable node type parameters and applies them on update.
src/ServiceFabric/ServiceFabric/Commands/ManagedClusters/NodeTypes/NewAzServiceFabricManagedNodeType.cs Adds multiple new create-time node type parameters and applies them on create.
src/ServiceFabric/ServiceFabric/Commands/ManagedClusters/ManagedClusters/SetAzServiceFabricManagedCluster.cs Adds many new update parameters and applies them during cluster update.
src/ServiceFabric/ServiceFabric/Commands/ManagedClusters/ManagedClusters/NewAzServiceFabricManagedCluster.cs Adds many new create parameters and applies them during cluster create.
src/ServiceFabric/ServiceFabric/Commands/ManagedClusters/Applications/SetAzServiceFabricManagedClusterApplication.cs Adds application identity parameters and applies them during update.
src/ServiceFabric/ServiceFabric/Commands/ManagedClusters/Applications/NewAzServiceFabricManagedClusterApplication.cs Adds application identity parameters and applies them during create.

You can also share your feedback on Copilot code review. Take the survey.

{
currentApp.Identity = new ManagedIdentity();
}

Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

If -UserAssignedIdentityId is specified without also setting IdentityType to a value that includes UserAssigned, the cmdlet will set UserAssignedIdentities while leaving Identity.Type unchanged/unspecified, which can lead to an invalid request or ignored identities. Validate the dependency (require IdentityType include UserAssigned) or set Identity.Type automatically when UserAssignedIdentityId is provided.

Suggested change
// Ensure the identity type includes UserAssigned when user-assigned identities are specified
if (string.IsNullOrEmpty(currentApp.Identity.Type))
{
currentApp.Identity.Type = "UserAssigned";
}
else if (!currentApp.Identity.Type.Contains("UserAssigned", StringComparison.OrdinalIgnoreCase))
{
currentApp.Identity.Type = currentApp.Identity.Type + ", UserAssigned";
}

Copilot uses AI. Check for mistakes.
Comment on lines +249 to +253
[Parameter(Mandatory = false, ParameterSetName = ByResourceGroup,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByResourceId,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByInputObject,
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

New identity parameters introduce new behavior for managed cluster applications, but existing scenario tests for Set-AzServiceFabricManagedClusterApplication don't cover identity updates. Add/update tests to exercise setting identity type and user-assigned identities and validate the resulting properties.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 25 to 28
public string Type { get; set; }
public string Location { get; set; }
public IDictionary<string, string> Tags { get; set; }
public SystemData SystemData { get; }
public string ProvisioningState { get; set; }
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

PSManagedService previously exposed SystemData, but this PR removes the property entirely even though the underlying ServiceResource model still contains SystemData. Removing it is a breaking change for users who rely on that output shape; consider keeping SystemData (possibly nullable) for backward compatibility and populating it from service.SystemData.

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +105
[Parameter(Mandatory = false, ParameterSetName = WithParamsByName, HelpMessage = "Indicates if this node type can only be used for outbound connections. Outbound-only node types will not have load balancing rules associated with them.")]
public bool? IsOutboundOnly { get; set; }

[Parameter(Mandatory = false, ParameterSetName = WithParamsByName, HelpMessage = "Enable resilient ephemeral OS disk for the node type. This provides better performance and resilience for ephemeral OS disks.")]
public bool? EnableResilientEphemeralOSDisk { get; set; }
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

New node type parameters (e.g., IsOutboundOnly/EnableResilientEphemeralOSDisk/SecureBootEnabled/UseEphemeralOSDisk, etc.) change Set-AzServiceFabricManagedNodeType behavior, but scenario tests currently only cover a subset of updates. Add coverage that sets these flags and asserts they round-trip via Get-AzServiceFabricManagedNodeType.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +329 to +333
if (!string.IsNullOrEmpty(clientApp) && !string.IsNullOrEmpty(clusterApp) && !string.IsNullOrEmpty(tenantId))
{
currentCluster.AzureActiveDirectory = new AzureActiveDirectory(
clientApplication: clientApp,
clusterApplication: clusterApp,
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

If any AzureActiveDirectory* parameter is bound but the effective (clientApp/clusterApp/tenantId) set is incomplete and the cluster has no existing AAD config, this code currently does nothing silently. Prefer validating the effective values and throwing a PSArgumentException when the configuration is incomplete.

Copilot uses AI. Check for mistakes.
Comment on lines +316 to +320
[Parameter(Mandatory = false, ParameterSetName = StatelessSingleton,
HelpMessage = "Specify the DNS name for the service. This enables service discovery via DNS.")]
[Parameter(Mandatory = false, ParameterSetName = StatelessUniformInt64,
HelpMessage = "Specify the DNS name for the service. This enables service discovery via DNS.")]
[Parameter(Mandatory = false, ParameterSetName = StatelessNamed,
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

ServiceDnsName adds new service creation behavior for New-AzServiceFabricManagedClusterService, but existing scenario tests create services without exercising DNS-based service discovery. Add a test case that sets -ServiceDnsName and validates it is returned by Get-AzServiceFabricManagedClusterService.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 29 to +33
: base(
location: cluster.Location,
id: cluster.Id,
name: cluster.Name,
type: cluster.Type,
tags: cluster.Tags,
addOnFeatures: cluster.AddOnFeatures,
applicationTypeVersionsCleanupPolicy: cluster.ApplicationTypeVersionsCleanupPolicy,
availableClusterVersions: cluster.AvailableClusterVersions,
clusterId: cluster.ClusterId,
clusterState: cluster.ClusterState,
clusterEndpoint: cluster.ClusterEndpoint,
clusterCodeVersion: cluster.ClusterCodeVersion,
azureActiveDirectory: cluster.AzureActiveDirectory,
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

PSCluster is intended to be a faithful wrapper of an existing SDK Cluster instance, but the base constructor call does not pass through Cluster.SystemData. Because SystemData has a private setter on the SDK type, omitting it here will drop that metadata from the output object. Consider passing systemData: cluster.SystemData into the base constructor.

Copilot uses AI. Check for mistakes.
@NoriZC
Copy link
Copy Markdown
Contributor

NoriZC commented Mar 11, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@NoriZC
Copy link
Copy Markdown
Contributor

NoriZC commented Mar 11, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

Copilot AI review requested due to automatic review settings March 27, 2026 23:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 78 out of 78 changed files in this pull request and generated 8 comments.

Comment on lines +94 to +112
[Parameter(Mandatory = false, ParameterSetName = SkipAppTypeVersion,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = CreateAppTypeVersion,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
public ManagedIdentityType IdentityType { get; set; }

[Parameter(Mandatory = false, ParameterSetName = SkipAppTypeVersion,
HelpMessage = "Specify the list of user assigned identity ARM resource IDs for the application.")]
[Parameter(Mandatory = false, ParameterSetName = CreateAppTypeVersion,
HelpMessage = "Specify the list of user assigned identity ARM resource IDs for the application.")]
[ValidateNotNullOrEmpty]
public string[] UserAssignedIdentityId { get; set; }

[Parameter(Mandatory = false, ParameterSetName = SkipAppTypeVersion,
HelpMessage = "Specify the application managed identities as key/value pairs. The key is the friendly identity name, and the value is the principal id.")]
[Parameter(Mandatory = false, ParameterSetName = CreateAppTypeVersion,
HelpMessage = "Specify the application managed identities as key/value pairs. The key is the friendly identity name, and the value is the principal id.")]
[ValidateNotNullOrEmpty]
public Hashtable ApplicationManagedIdentity { get; set; }
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

The IdentityType help text lists SystemAssigned,UserAssigned, but the enum value users can pass is SystemAssignedUserAssigned (the comma form is only the serialized value). Update the help message to reflect the actual accepted PowerShell value(s) to avoid binding errors.

Copilot uses AI. Check for mistakes.
Comment on lines +227 to +231
return this.ApplicationManagedIdentity.Cast<DictionaryEntry>()
.Select(entry => new ApplicationUserAssignedIdentity(
name: entry.Key as string,
principalId: entry.Value as string))
.ToList();
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

ApplicationManagedIdentity entries are cast with entry.Key as string / entry.Value as string. Non-string keys/values will become null and will trigger ApplicationUserAssignedIdentity.Validate() failures (Name/PrincipalId cannot be null). Add validation/conversion to ensure all hashtable keys/values are non-empty strings before constructing ApplicationUserAssignedIdentity objects.

Suggested change
return this.ApplicationManagedIdentity.Cast<DictionaryEntry>()
.Select(entry => new ApplicationUserAssignedIdentity(
name: entry.Key as string,
principalId: entry.Value as string))
.ToList();
var identities = new List<ApplicationUserAssignedIdentity>();
foreach (DictionaryEntry entry in this.ApplicationManagedIdentity)
{
var name = entry.Key?.ToString();
var principalId = entry.Value?.ToString();
if (string.IsNullOrWhiteSpace(name) || string.IsNullOrWhiteSpace(principalId))
{
throw new PSArgumentException("ApplicationManagedIdentity must be a hashtable with non-empty string keys (identity names) and non-empty string values (principal identifiers).");
}
identities.Add(new ApplicationUserAssignedIdentity(
name: name,
principalId: principalId));
}
return identities;

Copilot uses AI. Check for mistakes.
}

currentApp.Identity.UserAssignedIdentities = this.UserAssignedIdentityId
?.ToDictionary(id => id, id => new UserAssignedIdentity());
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

When -UserAssignedIdentityId is bound, the cmdlet sets currentApp.Identity.UserAssignedIdentities but does not ensure currentApp.Identity.Type includes UserAssigned (and Identity may be newly created with Type == null). This can produce an invalid identity payload. Consider setting Identity.Type automatically when user-assigned IDs are provided (or enforcing that -IdentityType must be supplied alongside -UserAssignedIdentityId).

Suggested change
?.ToDictionary(id => id, id => new UserAssignedIdentity());
?.ToDictionary(id => id, id => new UserAssignedIdentity());
// Ensure identity type includes UserAssigned when user-assigned identities are provided,
// unless the caller explicitly specified -IdentityType.
if (!this.IsParameterBound(c => c.IdentityType))
{
var currentType = currentApp.Identity.Type;
if (currentType == null || currentType == ManagedIdentityType.None)
{
currentApp.Identity.Type = ManagedIdentityType.UserAssigned;
}
else if (currentType == ManagedIdentityType.SystemAssigned)
{
currentApp.Identity.Type = ManagedIdentityType.SystemAssignedUserAssigned;
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +250 to +254
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByResourceId,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByInputObject,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

The IdentityType help text lists SystemAssigned,UserAssigned, but the enum value users can pass is SystemAssignedUserAssigned (the comma form is only the serialized value). Update the help message to reflect the actual accepted PowerShell value(s) to avoid binding errors.

Suggested change
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByResourceId,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByInputObject,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.")]
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssignedUserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByResourceId,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssignedUserAssigned.")]
[Parameter(Mandatory = false, ParameterSetName = ByInputObject,
HelpMessage = "Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssignedUserAssigned.")]

Copilot uses AI. Check for mistakes.
Comment on lines +393 to +397
currentApp.ManagedIdentities = this.ApplicationManagedIdentity?.Cast<DictionaryEntry>()
.Select(entry => new ApplicationUserAssignedIdentity(
name: entry.Key as string,
principalId: entry.Value as string))
.ToList();
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

ApplicationManagedIdentity entries are cast with entry.Key as string / entry.Value as string. Non-string keys/values will become null and will trigger ApplicationUserAssignedIdentity.Validate() failures (Name/PrincipalId cannot be null). Add validation/conversion to ensure all hashtable keys/values are non-empty strings before constructing ApplicationUserAssignedIdentity objects.

Suggested change
currentApp.ManagedIdentities = this.ApplicationManagedIdentity?.Cast<DictionaryEntry>()
.Select(entry => new ApplicationUserAssignedIdentity(
name: entry.Key as string,
principalId: entry.Value as string))
.ToList();
if (this.ApplicationManagedIdentity != null)
{
var managedIdentities = new List<ApplicationUserAssignedIdentity>();
foreach (DictionaryEntry entry in this.ApplicationManagedIdentity)
{
if (entry.Key == null || entry.Value == null)
{
continue;
}
var nameString = entry.Key as string ?? entry.Key.ToString();
var principalIdString = entry.Value as string ?? entry.Value.ToString();
if (string.IsNullOrWhiteSpace(nameString) || string.IsNullOrWhiteSpace(principalIdString))
{
continue;
}
managedIdentities.Add(new ApplicationUserAssignedIdentity(
name: nameString,
principalId: principalIdString));
}
currentApp.ManagedIdentities = managedIdentities;
}
else
{
currentApp.ManagedIdentities = null;
}

Copilot uses AI. Check for mistakes.
Comment on lines +190 to +194
### -IdentityType
Specify the type of managed identity for the application. Options are None, SystemAssigned, UserAssigned, and SystemAssigned,UserAssigned.

```yaml
Type: ManagedIdentityType
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

-IdentityType docs mention SystemAssigned,UserAssigned, but the actual PowerShell enum value name is SystemAssignedUserAssigned (comma form is the serialized value). Update the parameter description so users know what value to pass.

Copilot uses AI. Check for mistakes.
Comment on lines 21 to +26
* Updated SFMC to latest api general version "2026-02-01"
* Fixed `Set-AzServiceFabricManagedClusterApplication` to use correct resource completer for managed clusters instead of classic clusters.
* Added parameters `-IdentityType`, `-UserAssignedIdentityId`, and `-ApplicationManagedIdentity` to `New-AzServiceFabricManagedClusterApplication` to support managed identity configuration on applications.
* Added parameters `-IdentityType`, `-UserAssignedIdentityId`, and `-ApplicationManagedIdentity` to `Set-AzServiceFabricManagedClusterApplication` to support managed identity configuration on applications.
* Added parameters `-AzureActiveDirectoryClientApplication`, `-AzureActiveDirectoryClusterApplication`, `-AzureActiveDirectoryTenantId`, `-EnableHttpGatewayExclusiveAuthMode`, `-HttpGatewayTokenAuthConnectionPort`, `-MaxPercentUnhealthyApplication`, `-MaxPercentUnhealthyNode`, `-MaxUnusedVersionsToKeep`, `-AddonFeature`, `-DdosProtectionPlanId`, `-EnableIpv6`, `-EnableServicePublicIP`, `-PublicIPPrefixId`, `-PublicIPv6PrefixId`, `-SubnetId`, `-UseCustomVnet`, `-VMImage`, `-AllocatedOutboundPort`, `-EnableOutboundOnlyNodeType`, and `-SkipManagedNsgAssignment` to `New-AzServiceFabricManagedCluster`.
* Added parameters `-AzureActiveDirectoryClientApplication`, `-AzureActiveDirectoryClusterApplication`, `-AzureActiveDirectoryTenantId`, `-EnableHttpGatewayExclusiveAuthMode`, `-HttpGatewayTokenAuthConnectionPort`, `-MaxPercentUnhealthyApplication`, `-MaxPercentUnhealthyNode`, `-MaxUnusedVersionsToKeep`, `-AddonFeature`, `-DdosProtectionPlanId`, `-PublicIPPrefixId`, `-PublicIPv6PrefixId`, `-VMImage`, `-AllocatedOutboundPort`, `-EnableOutboundOnlyNodeType`, and `-SkipManagedNsgAssignment` to `Set-AzServiceFabricManagedCluster`.
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

ChangeLog entry uses the acronym "SFMC" without expansion and includes double quotes around the API version. Per repo ChangeLog guidelines, expand less-obvious acronyms on first use and avoid problematic special characters like unescaped quotes (consider writing the version without quotes).

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 92 to +218
@@ -168,9 +194,43 @@ private ApplicationResource GetNewAppParameters(string location)
version: this.GetAppTypeArmResourceId(this.DefaultContext.Subscription.Id, this.ResourceGroupName, this.ClusterName, this.ApplicationTypeName, this.ApplicationTypeVersion),
parameters: this.ApplicationParameter?.Cast<DictionaryEntry>().ToDictionary(d => d.Key as string, d => d.Value as string),
location: location,
identity: this.GetManagedIdentity(),
managedIdentities: this.GetApplicationManagedIdentities(),
tags: this.Tag?.Cast<DictionaryEntry>().ToDictionary(d => d.Key as string, d => d.Value as string));
}

private ManagedIdentity GetManagedIdentity()
{
if (!this.IsParameterBound(c => c.IdentityType))
{
return null;
}

var identity = new ManagedIdentity(type: this.IdentityType);

if (this.UserAssignedIdentityId != null && this.UserAssignedIdentityId.Length > 0)
{
identity.UserAssignedIdentities = this.UserAssignedIdentityId
.ToDictionary(id => id, id => new UserAssignedIdentity());
}

return identity;
}
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

UserAssignedIdentityId can be specified without IdentityType, but GetManagedIdentity() returns null unless IdentityType is bound, so user-assigned identities are silently ignored. Consider either making -IdentityType mandatory when -UserAssignedIdentityId is provided, or inferring/setting IdentityType to UserAssigned (or SystemAssignedUserAssigned) when user-assigned IDs are supplied.

Copilot uses AI. Check for mistakes.
@NoriZC
Copy link
Copy Markdown
Contributor

NoriZC commented Mar 28, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants