-
Notifications
You must be signed in to change notification settings - Fork 4.1k
[Storage] Support encryption in transit #29083
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
base: main
Are you sure you want to change the base?
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
There was a problem hiding this 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 pull request adds support for encryption in transit configuration for both SMB and NFS protocols in Azure Storage file service properties.
Changes:
- Added new PowerShell parameters
SmbEncryptionInTransitRequiredandNfsEncryptionInTransitRequiredtoUpdate-AzStorageFileServicePropertycmdlet - Extended model classes to support NFS settings and encryption in transit properties
- Updated display format to show the new encryption in transit properties
- Added comprehensive test coverage for both SMB and NFS encryption in transit scenarios
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Storage.Management.format.ps1xml | Added display entries for SMB and NFS encryption in transit properties in the file service properties list view |
| PSFileServiceProperties.cs | Extended model classes with PSNfsSetting and PSEncryptionInTransit to represent the new encryption in transit settings |
| UpdateAzAzStorageFileServiceProperty.cs | Added two new boolean parameters for SMB and NFS encryption in transit, with implementation logic to set these properties on file service |
| ChangeLog.md | Documented the new encryption in transit support feature in the upcoming release section |
| StorageFileTests.ps1 | Added test scenarios validating both SMB and NFS encryption in transit can be enabled and disabled |
|
|
||
| [Parameter( | ||
| Mandatory = false, | ||
| HelpMessage = "Enable Multichannel by set to $true, disable Multichannel by set to $false. Applies to Premium FileStorage only.")] |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help message incorrectly describes this parameter as "Enable Multichannel" when it should describe SMB encryption in transit functionality. Update the help message to accurately describe this parameter's purpose, such as "Enable or disable encryption in transit for SMB protocol. Set to $true to require encryption, $false to disable."
| HelpMessage = "Enable Multichannel by set to $true, disable Multichannel by set to $false. Applies to Premium FileStorage only.")] | |
| HelpMessage = "Enable or disable encryption in transit for SMB protocol. Set to $true to require encryption, $false to disable. Applies to Premium FileStorage only.")] |
|
|
||
| [Parameter( | ||
| Mandatory = false, | ||
| HelpMessage = "Enable Multichannel by set to $true, disable Multichannel by set to $false. Applies to Premium FileStorage only.")] |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help message incorrectly describes this parameter as "Enable Multichannel" when it should describe NFS encryption in transit functionality. Update the help message to accurately describe this parameter's purpose, such as "Enable or disable encryption in transit for NFS protocol. Set to $true to require encryption, $false to disable."
| HelpMessage = "Enable Multichannel by set to $true, disable Multichannel by set to $false. Applies to Premium FileStorage only.")] | |
| HelpMessage = "Enable or disable encryption in transit for NFS protocol. Set to $true to require encryption, $false to disable.")] |
| <ListItem> | ||
| <Label>ProtocolSettings.Nfs.EncryptionInTransit.Required</Label> | ||
| <ScriptBlock>$_.ProtocolSettings.Nfs.EncryptionInTransit.Required</ScriptBlock> | ||
| </ListItem> |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is trailing whitespace at the end of this line. Remove the trailing whitespace to maintain code consistency.
| </ListItem> | |
| </ListItem> |
| if (protocolSettings.Smb.EncryptionInTransit == null) | ||
| { | ||
| protocolSettings.Smb.EncryptionInTransit = new EncryptionInTransit(this.smbEncryptionInTransitRequired); | ||
| } |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The null check for protocolSettings.Smb.EncryptionInTransit is unnecessary since a new SmbSetting instance is always created at line 266, which would have this property as null by default. For consistency with the similar Multichannel property handling (lines 283-287), remove the inner null check and directly instantiate the EncryptionInTransit object.
| if (protocolSettings.Smb.EncryptionInTransit == null) | |
| { | |
| protocolSettings.Smb.EncryptionInTransit = new EncryptionInTransit(this.smbEncryptionInTransitRequired); | |
| } | |
| protocolSettings.Smb.EncryptionInTransit = new EncryptionInTransit(this.smbEncryptionInTransitRequired); |
| this.SmbKerberosTicketEncryption != null || | ||
| this.SmbChannelEncryption != null || | ||
| this.enableSmbMultichannel != null) | ||
| this.enableSmbMultichannel != null || |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is trailing whitespace at the end of this line. Remove the trailing whitespace to maintain code consistency.
| } | ||
| } | ||
|
|
||
| public class PSEncryptionInTransit { |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is trailing whitespace at the end of this line after the opening brace. Remove the trailing whitespace to maintain code consistency.
| public class PSEncryptionInTransit { | |
| public class PSEncryptionInTransit { |
src/Storage/Storage.Management/File/UpdateAzAzStorageFileServiceProperty.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 5 out of 6 changed files in this pull request and generated no new comments.
vidai-msft
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the help doc since new params have been introduced.
Description
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.