[aks-preview] secondary NIC public IP configuration - #10355
Chase Wilson (chasewilson) wants to merge 1 commit into
Conversation
|
Hi Chase Wilson (@chasewilson), |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
microsoft-github-policy-service
Bot
added
Auto-Assign
There was a problem hiding this comment.
🟡 Changes recommended
The parser does not enforce the documented mutual exclusion between ipTags and publicIPPrefixID.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds secondary NIC public IP configuration support for az aks nodepool add, including IP tags and public IP prefixes.
Changes:
- Parses and serializes nested public IP configuration.
- Updates command help and history.
- Adds unit coverage for supported and malformed inputs.
File summaries
| File | Description |
|---|---|
HISTORY.rst |
Documents the feature. |
test_agentpool_decorator.py |
Tests parsing and serialization. |
agentpool_decorator.py |
Adds nested configuration handling. |
_params.py |
Updates argument help. |
_help.py |
Updates generated command documentation. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ip_tags = public_ip_config.get("ipTags") | ||
| if ip_tags is not None: | ||
| if not isinstance(ip_tags, list): | ||
| raise InvalidArgumentValueError( | ||
| "--secondary-network-interfaces: ipTags in " | ||
| f"publicIPAddressConfiguration at index {idx} must be a JSON array." | ||
| ) |
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
| "--secondary-network-interfaces: publicIPAddressConfiguration " | ||
| f"at index {idx} must be a JSON object." | ||
| ) | ||
| ip_tags = public_ip_config.get("ipTags") |
There was a problem hiding this comment.
Non-blocking: could we add an explicit mutual-exclusion check for ipTags and publicIPPrefixID, along with a regression test? Both the help text and SDK contract say these properties are mutually exclusive, but the current validation only checks the JSON structure and still forwards both when supplied. Rejecting this combination locally would give users a clear CLI error instead of relying on resource-provider validation.
🤖 PR Validation — ️✔️ All clear
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az aks nodepool add --secondary-network-interfaces(alias:--secondary-nics)General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install azdevrequired)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.