Storage - STG102 Directory-Level SAS on Blob FNS#48477
Storage - STG102 Directory-Level SAS on Blob FNS#48477ibrandes wants to merge 4 commits intoAzure:feature/storage/stg102basefrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support and tests for directory-level Service SAS on Blob Flat Namespace (FNS), including emitting the directory resource type (sr=d) and directory depth (sdd) and validating behavior in unit/integration tests.
Changes:
- Introduces
setDirectory(...)/isDirectory()onBlobServiceSasSignatureValuesto indicate directory-level SAS intent. - Updates
BlobSasImplUtilto select directory resource (sr=d) and compute directory depth for SAS. - Adds/extends tests to cover directory canonicalization/resource selection and end-to-end directory SAS usage (shared key + user delegation).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java | Adds directory SAS resource selection and emits directory depth query parameter. |
| sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/sas/BlobServiceSasSignatureValues.java | Adds public API surface to mark SAS as directory-level. |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasClientTests.java | Adds new directory SAS integration tests and updates canonicalized resource test cases. |
| sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/BlobServiceSasModelsTests.java | Extends ensureState() tests to validate directory resource/depth handling. |
sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasClientTests.java
Outdated
Show resolved
Hide resolved
...e-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java
Outdated
Show resolved
Hide resolved
sdk/storage/azure-storage-blob/src/test/java/com/azure/storage/blob/SasClientTests.java
Outdated
Show resolved
Hide resolved
...e-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java
Outdated
Show resolved
Hide resolved
...e-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java
Show resolved
Hide resolved
...e-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java
Show resolved
Hide resolved
...orage/azure-storage-blob/src/test/java/com/azure/storage/blob/BlobServiceSasModelsTests.java
Outdated
Show resolved
Hide resolved
...ure-storage-blob/src/main/java/com/azure/storage/blob/sas/BlobServiceSasSignatureValues.java
Outdated
Show resolved
Hide resolved
...e-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java
Outdated
Show resolved
Hide resolved
...e-storage-blob/src/main/java/com/azure/storage/blob/implementation/util/BlobSasImplUtil.java
Outdated
Show resolved
Hide resolved
...ure-storage-blob/src/main/java/com/azure/storage/blob/sas/BlobServiceSasSignatureValues.java
Show resolved
Hide resolved
...orage/azure-storage-blob/src/test/java/com/azure/storage/blob/BlobServiceSasModelsTests.java
Outdated
Show resolved
Hide resolved
| allPermissions.setImmutabilityPolicyPermission(true); | ||
| } | ||
| BlobSasPermission allPermissions = getAllBlobSasPermissions(); | ||
|
|
There was a problem hiding this comment.
Just for my understanding, we're consolidating everything because the version will be the latest version (since we don't have a ServiceVersion annotation at the top?
|
|
||
| StepVerifier.create(appendBlobClient1.create().flatMap(ignored -> appendBlobClient2.create()).then()) | ||
| .verifyComplete(); | ||
| } |
There was a problem hiding this comment.
Just a question, ghcp says its oaky (and there are no asserts in dotnet), but should we assert something? I know StepVerifier will catch any errors, but should we check that a read permission or something is set?
| BlobSasImplUtil implUtil = new BlobSasImplUtil(sasValues, containerName, blobName, null, null, null); | ||
|
|
||
| List<String> stringToSignHolder = new ArrayList<>(); | ||
| String sasToken = implUtil.generateSas(ENVIRONMENT.getPrimaryAccount().getCredential(), stringToSignHolder::add, |
There was a problem hiding this comment.
good use of the stringToSignHolderHandler. I remember seeing this when I was debugging some tests a while back and it was quite useful for seeing the actual values that go in instead of a bunch of trying to count newlines 😅
|
It looks like you need to run all of the tests at once to capture them at once. E.g. when I ran |
No description provided.