Skip to content

feat(acns): add transit encryption options for az create and update commands #8784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nddq
Copy link
Contributor

@nddq nddq commented May 22, 2025

Creates commands for specify transit encryption type (None, WireGuard) during cluster creation and update for aks-preview.

az aks create --enable-acns --acns-transit-encryption-type <None|WireGuard>
az aks update --enable-acns --acns-transit-encryption-type <None|WireGuard>


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

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)
  • My extension version conforms to the Extension version schema

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.json automatically.
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.

@Copilot Copilot AI review requested due to automatic review settings May 22, 2025 15:58
Copy link

azure-client-tools-bot-prd bot commented May 22, 2025

⚠️Azure CLI Extensions Breaking Change Test
⚠️aks-preview
rule cmd_name rule_message suggest_message
⚠️ 1006 - ParaAdd aks create cmd aks create added parameter acns_transit_encryption_type
⚠️ 1006 - ParaAdd aks update cmd aks update added parameter acns_transit_encryption_type

@yonzhan
Copy link
Collaborator

yonzhan commented May 22, 2025

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

Copy link

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
Contributor

@Copilot 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 adds support for specifying a transit encryption type parameter (None or WireGuard) for ACNS during both cluster creation and update.

  • Bump version in setup.py
  • Update linter exclusions for the new parameter
  • Introduce tests and enable handling in the managed cluster decorator, custom command code, and parameter definitions

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/aks-preview/setup.py Version bump to 18.0.0b3
src/aks-preview/linter_exclusions.yml Added linter rule exclusions for acns_transit_encryption_type
src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py Added tests for get_acns_transit_encryption_type functionality
src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py Added tests for cluster creation and update with transit encryption type
src/aks-preview/azext_aks_preview/managed_cluster_decorator.py Integrated handling for transit encryption type in cluster network profile setup and update
src/aks-preview/azext_aks_preview/custom.py Extended creation and update command functions with new parameter
src/aks-preview/azext_aks_preview/_params.py Defined argument and enum for acns_transit_encryption_type parameter
src/aks-preview/azext_aks_preview/_consts.py Added constants for transit encryption types
src/aks-preview/HISTORY.rst Updated history notes for new option

raise MutuallyExclusiveArgumentError(
"--disable-acns-security and --disable-acns cannot be used with acns_transit_encryption_type."
)
return self.raw_param.get("acns_transit_encryption_type")
Copy link
Preview

Copilot AI May 22, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider storing the retrieved value of 'acns_transit_encryption_type' in a local variable and returning that variable rather than calling self.raw_param.get() again. This would improve clarity and avoid potential inconsistencies.

Suggested change
return self.raw_param.get("acns_transit_encryption_type")
return acns_transit_encryption_type

Copilot uses AI. Check for mistakes.

Copy link

Copy link
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

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

Queued live test to validate the change.

  • test_aks_create_with_transit_encryption_type
  • test_aks_update_with_transit_encryption_type

@FumingZhang
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@FumingZhang
Copy link
Member

Please fix failed CI checks and resolve merge conflicts

  • FAIL - HIGH severity: missing_parameter_help
    Parameter: aks create, acns_transit_encryption_type - Missing help
    Parameter: aks update, acns_transit_encryption_type - Missing help

@nddq nddq force-pushed the acnsEncryptionCLI branch from d9099af to 1ab7818 Compare May 27, 2025 18:46
@nddq nddq requested a review from FumingZhang May 27, 2025 18:47
@FumingZhang
Copy link
Member

applied fix for failed test cases in PR #8798

@FumingZhang
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@FumingZhang
Copy link
Member

Queued live test to validate the change.

  • test_aks_create_with_transit_encryption_type
  • test_aks_update_with_transit_encryption_type

Requeued live test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AKS Auto-Assign Auto assign by bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants