-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[storage]STG100 preview #36746
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?
[storage]STG100 preview #36746
Conversation
there are more than one possible design, why was the one in this PR chosen? request PRs)_ - [ ] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)
…Azure#34803) there are more than one possible design, why was the one in this PR chosen? request PRs)_ - [ ] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)
|
@jeremymeng , this PR changed API file and pnpm lock file. Could you help to review? |
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 Azure Storage service version 2026-02-06 with User Delegation SAS functionality and Principal-Bound Identity User Delegation SAS across multiple storage packages (queue, file-share, and file-datalake). The changes include:
- Service API version upgrade from 2025-11-05 to 2026-02-06
- User delegation key support for generating SAS tokens using Azure AD tokens
- New SAS parameters for user delegation (skoid, sktid, skt, ske, sks, skv, sduoid)
- Principal-bound identity support via delegatedUserObjectId parameter
- Generated code from updated Swagger specifications
- Test coverage for new user delegation SAS functionality
Reviewed changes
Copilot reviewed 72 out of 110 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/storage/storage-queue/swagger/README.md | Updates Swagger input file to new API version and autorest version |
| sdk/storage/storage-queue/src/utils/constants.ts | Updates SDK and service version constants |
| sdk/storage/storage-queue/src/QueueServiceClient.ts | Adds getUserDelegationKey() method |
| sdk/storage/storage-queue/src/QueueClient.ts | Adds user delegation SAS generation methods |
| sdk/storage/storage-queue/src/SASQueryParameters.ts | Adds user delegation key fields and parameter serialization |
| sdk/storage/storage-queue/src/QueueSASSignatureValues.ts | Adds user delegation SAS generation logic with overloads |
| sdk/storage/storage-queue/test/node/sas.spec.ts | Adds test cases for user delegation SAS functionality |
| sdk/storage/storage-queue/test/utils/* | Adds test utility functions and new SAS parameters |
| sdk/storage/storage-file-share/* | Similar changes for file-share package |
| sdk/storage/storage-file-datalake/* | Version updates and test additions |
| sdk/storage/storage-queue/package.json | Updates version to 12.29.0-beta.1 and dependency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
jeremymeng
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.
Overall looks good! Minor question: are blob and file-datalake not using the UserDelegationKey from storage-common?
| import type { TokenCredential } from '@azure/core-auth'; | ||
| import type { TransferProgressEvent } from '@azure/core-rest-pipeline'; | ||
| import type { UserAgentPolicyOptions } from '@azure/core-rest-pipeline'; | ||
| import { UserDelegationKey as UserDelegationKey_2 } from '@azure/storage-common'; |
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.
Is this because we still have another different UserDelegationKey in file-share? Do we need both?
|
|
||
| // @public | ||
| export interface ShareProtocolSettings { | ||
| // Warning: (ae-forgotten-export) The symbol "ShareNfsSettings" needs to be exported by the entry point index.d.ts |
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.
Need to fix this warning.
|
|
||
| // @public | ||
| export interface ShareSmbSettings { | ||
| // Warning: (ae-forgotten-export) The symbol "ShareSmbSettingsEncryptionInTransit" needs to be exported by the entry point index.d.ts |
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.
Need to fix this warning.
| import { StorageSharedKeyCredentialPolicy } from '@azure/storage-common'; | ||
| import type { TokenCredential } from '@azure/core-auth'; | ||
| import type { UserAgentPolicyOptions } from '@azure/core-rest-pipeline'; | ||
| import { UserDelegationKey as UserDelegationKey_2 } from '@azure/storage-common'; |
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.
same question as the one for file-share.
API Change CheckAPIView identified API level changes in this PR and created the following API reviews @azure/storage-blob |
sdk/storage/storage-file-share/review/storage-file-share-node.api.md
Outdated
Show resolved
Hide resolved
sdk/storage/storage-file-share/review/storage-file-share-node.api.md
Outdated
Show resolved
Hide resolved
sdk/storage/storage-file-share/review/storage-file-share-node.api.md
Outdated
Show resolved
Hide resolved
sdk/storage/storage-file-share/review/storage-file-share-node.api.md
Outdated
Show resolved
Hide resolved
|
|
||
| // @public | ||
| export interface ShareNfsSettings { | ||
| // Warning: (ae-forgotten-export) The symbol "ShareNfsSettingsEncryptionInTransit" needs to be exported by the entry point index.d.ts |
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.
This warning still needs fix.
blueww
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.
LGTM
Packages impacted by this PR
Issues associated with this PR
Describe the problem that is addressed by this PR
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
Are there test cases added in this PR? (If not, why?)
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists