Skip to content
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

Preserve the snapshot ID when testing for required auth on MD #2542

Merged
merged 13 commits into from
Mar 13, 2024

Conversation

adreed-msft
Copy link
Member

When supplying a managed disk SAS token, we would historically trim the snapshot (as expected with blob), but this is unnecessary and problematic with managed disk snapshots, where managed disks expect the snapshot as a part of the authorization. When we check for MD auth, we should preserve the snapshot ID, because it is a portion of auth.

This PR includes code changes and tests for both data access authentication mode ("OAuth + SAS") and non-authenticated data access (just a SAS).

gapra-msft
gapra-msft previously approved these changes Feb 8, 2024
Copy link
Member

@gapra-msft gapra-msft left a comment

Choose a reason for hiding this comment

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

just had a comment about a particular file

tasherif-msft
tasherif-msft previously approved these changes Feb 14, 2024
@adreed-msft adreed-msft force-pushed the adreed/md-snapshot-fix branch from 3cf43d5 to ef5cda7 Compare February 26, 2024 21:23
@adreed-msft adreed-msft changed the base branch from dev to main February 26, 2024 21:24
@adreed-msft adreed-msft dismissed stale reviews from tasherif-msft, siminsavani-msft, and gapra-msft February 26, 2024 21:24

The base branch was changed.

@adreed-msft adreed-msft force-pushed the adreed/md-snapshot-fix branch from bc64932 to cf2e7ce Compare March 5, 2024 22:15
@adreed-msft adreed-msft merged commit 3dd12c2 into main Mar 13, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants