Skip to content

[Service Fabric] az sf managed-cluster network-security-rule add: Add new parameters for port ranges and address prefixes#31714

Merged
zhoxing-ms merged 1 commit intoAzure:devfrom
iliu816:user/iliu/nsg-test
Jul 2, 2025
Merged

[Service Fabric] az sf managed-cluster network-security-rule add: Add new parameters for port ranges and address prefixes#31714
zhoxing-ms merged 1 commit intoAzure:devfrom
iliu816:user/iliu/nsg-test

Conversation

@iliu816
Copy link
Copy Markdown
Member

@iliu816 iliu816 commented Jun 25, 2025

Related command
az sf managed-cluster network-security-rule add
Description

SF management ports 19000 and 19080 are considered high risk when exposed to Internet traffic. A recent failure to cleanup test resources left the resource with ports exposed and resulted in an incident (https://portal.microsofticm.com/imp/v5/incidents/details/647032599/summary).

This PR modifies the test_network_security_rule test to add a network security rule to deny inbound traffic to ports 19000 and 19080 from the internet. This should prevent the created resource from being flagged as high risk in the event another cleanup failure occurs. In order to add this rule, the az sf managed-cluster network-security-rule add command was updated to take in parameters for source_addr_prefix, dest_addr_prefix, source_port_range, and dest_port_range to allow the required input tags and port ranges.

History Notes

[Service Fabric] az sf managed-cluster network-security-rule: Add new parameter --source-addr-prefix to specify the CIDR or source IP range
[Service Fabric] az sf managed-cluster network-security-rule: Add new parameter --dest-addr-prefix to specify the destination port or range
[Service Fabric] az sf managed-cluster network-security-rule: Add new parameter --source-port-range to specify the CIDR or source IP range
[Service Fabric] az sf managed-cluster network-security-rule: Add new parameter --dest-port-range to specify the destination address prefix


This checklist is used to make sure that common guidelines for a pull request are followed.

Copilot AI review requested due to automatic review settings June 25, 2025 20:43
@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented Jun 25, 2025

️✔️AzureCLI-FullTest
️✔️acr
️✔️latest
️✔️3.12
️✔️3.9
️✔️acs
️✔️latest
️✔️3.12
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.12
️✔️3.9
️✔️ams
️✔️latest
️✔️3.12
️✔️3.9
️✔️apim
️✔️latest
️✔️3.12
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.12
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️aro
️✔️latest
️✔️3.12
️✔️3.9
️✔️backup
️✔️latest
️✔️3.12
️✔️3.9
️✔️batch
️✔️latest
️✔️3.12
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.12
️✔️3.9
️✔️billing
️✔️latest
️✔️3.12
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.12
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.12
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️compute_recommender
️✔️latest
️✔️3.12
️✔️3.9
️✔️computefleet
️✔️latest
️✔️3.12
️✔️3.9
️✔️config
️✔️latest
️✔️3.12
️✔️3.9
️✔️configure
️✔️latest
️✔️3.12
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.12
️✔️3.9
️✔️container
️✔️latest
️✔️3.12
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.12
️✔️3.9
️✔️core
️✔️latest
️✔️3.12
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.12
️✔️3.9
️✔️databoxedge
️✔️latest
️✔️3.12
️✔️3.9
️✔️dls
️✔️latest
️✔️3.12
️✔️3.9
️✔️dms
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.12
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.12
️✔️3.9
️✔️find
️✔️latest
️✔️3.12
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.12
️✔️3.9
️✔️identity
️✔️latest
️✔️3.12
️✔️3.9
️✔️iot
️✔️latest
️✔️3.12
️✔️3.9
️✔️keyvault
️✔️latest
️✔️3.12
️✔️3.9
️✔️lab
️✔️latest
️✔️3.12
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️maps
️✔️latest
️✔️3.12
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.12
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.12
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.12
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.12
️✔️3.9
️✔️network
️✔️latest
️✔️3.12
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.12
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.12
️✔️3.9
️✔️profile
️✔️latest
️✔️3.12
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.12
️✔️3.9
️✔️redis
️✔️latest
️✔️3.12
️✔️3.9
️✔️relay
️✔️latest
️✔️3.12
️✔️3.9
️✔️resource
️✔️latest
️✔️3.12
️✔️3.9
️✔️role
️✔️latest
️✔️3.12
️✔️3.9
️✔️search
️✔️latest
️✔️3.12
️✔️3.9
️✔️security
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.12
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.12
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.12
️✔️3.9
️✔️sql
️✔️latest
️✔️3.12
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.12
️✔️3.9
️✔️storage
️✔️latest
️✔️3.12
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.12
️✔️3.9
️✔️telemetry
️✔️latest
️✔️3.12
️✔️3.9
️✔️util
️✔️latest
️✔️3.12
️✔️3.9
️✔️vm
️✔️latest
️✔️3.12
️✔️3.9

@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented Jun 25, 2025

⚠️AzureCLI-BreakingChangeTest
⚠️servicefabric
rule cmd_name rule_message suggest_message
⚠️ 1006 - ParaAdd sf managed-cluster network-security-rule add cmd sf managed-cluster network-security-rule add added parameter dest_addr_prefix
⚠️ 1006 - ParaAdd sf managed-cluster network-security-rule add cmd sf managed-cluster network-security-rule add added parameter dest_port_range
⚠️ 1006 - ParaAdd sf managed-cluster network-security-rule add cmd sf managed-cluster network-security-rule add added parameter source_addr_prefix
⚠️ 1006 - ParaAdd sf managed-cluster network-security-rule add cmd sf managed-cluster network-security-rule add added parameter source_port_range

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Jun 25, 2025

Thank you for your contribution! We will review the pull request and get back to you soon.

@github-actions
Copy link
Copy Markdown

The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR.

Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

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 network security rule functionality for SF managed clusters by fixing test coverage issues and adding support for singular port range and address prefix parameters to better control inbound traffic.

  • Updated test cases to validate both allow and deny network security rules.
  • Added new parameters (source_port_range, dest_port_range, source_addr_prefix, dest_addr_prefix) in operations, _params, and _help modules.
  • Revised help examples to demonstrate the usage of these new parameters.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

File Description
src/azure-cli/azure/cli/command_modules/servicefabric/tests/latest/test_sf_managed_cluster.py Updated tests to include deny rule parameters and adjusted security rule checks.
src/azure-cli/azure/cli/command_modules/servicefabric/operations/managed_clusters.py Integrated new singular port and address prefix parameters into the API call.
src/azure-cli/azure/cli/command_modules/servicefabric/_params.py Added new argument definitions for singular port and address prefix parameters with corresponding help messages.
src/azure-cli/azure/cli/command_modules/servicefabric/_help.py Updated help examples to include scenarios for using singular parameters.
Comments suppressed due to low confidence (1)

src/azure-cli/azure/cli/command_modules/servicefabric/_params.py:296

  • The help message for 'dest_addr_prefix' incorrectly states that '*' matches all source IPs; it should indicate that it matches all destination IPs for clarity.
        c.argument('dest_addr_prefix', help='The destination address prefix. CIDR or destination IP range. Asterisk \'*\' can also be used to match all source IPs. Default tags such as \'VirtualNetwork\', \'AzureLoadBalancer\' and \'Internet\' can also be used.')

@wangzelin007
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@zhoxing-ms zhoxing-ms changed the title [Service Fabric] az sf managed-cluster network-security-rule add: Fix test_network_security_rule and add new parameter options for port ranges and address prefixes [Service Fabric] az sf managed-cluster network-security-rule add: Add new parameter options for port ranges and address prefixes Jun 26, 2025
@zhoxing-ms zhoxing-ms changed the title [Service Fabric] az sf managed-cluster network-security-rule add: Add new parameter options for port ranges and address prefixes [Service Fabric] az sf managed-cluster network-security-rule add: Add new parameters for port ranges and address prefixes Jun 26, 2025
@zhoxing-ms
Copy link
Copy Markdown
Contributor

zhoxing-ms commented Jun 26, 2025

Please refer to this doc
https://github.com/Azure/azure-cli/tree/dev/doc/authoring_command_modules#format-pr-title to add history notes for the newly added parameters. Please note that these history notes will be included in the release notes for customers to see
image

@iliu816
Copy link
Copy Markdown
Member Author

iliu816 commented Jun 26, 2025

@zhoxing-ms Added history notes, please let me know if they are insufficient

@zhoxing-ms zhoxing-ms merged commit 5058e03 into Azure:dev Jul 2, 2025
59 checks passed
@iliu816 iliu816 deleted the user/iliu/nsg-test branch July 14, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot Portal az portal Service Fabric az sf

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants