Skip to content

[Test Proxy] Accept parameter-level SAS sanitization - #25898

Merged
laiapat merged 2 commits into
Azure:mainfrom
laiapat:generate-sas-sanitizer
Aug 29, 2022
Merged

[Test Proxy] Accept parameter-level SAS sanitization#25898
laiapat merged 2 commits into
Azure:mainfrom
laiapat:generate-sas-sanitizer

Conversation

@laiapat

@laiapat laiapat commented Aug 27, 2022

Copy link
Copy Markdown
Member

Description

Context: the Storage team discovered that there are cases where SAS tokens generated by tests can fail to match in playback because they contain tenant IDs. The problem is that AzureRecordedTestCase.generate_sas currently sanitizes only the sig, st, and se parameters of a generated token. In Storage's case, their tokens also contain a sktid field that includes a tenant ID. This ID doesn't get sanitized, and in playback the local value of the tenant ID ("00000000-0000-0000-0000-000000000000") doesn't match when a new SAS is generated.

This PR expands the functionality of the generate_sas method to resolve this issue. A fake_parameters kwarg can now be provided, specifying SAS token keys to sanitize. Since the change is purely additive, it's backwards-compatible for tests that rely on the existing generate_sas behavior. The Storage team has reviewed this solution offline and given a thumbs-up.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.
    • Tested with the test that motivated the change, as well as tests that currently use the generate_sas method.

@laiapat laiapat added EngSys This issue is impacting the engineering system. test-enhancement labels Aug 27, 2022
@laiapat
laiapat requested a review from scbedd August 27, 2022 00:48

@scbedd scbedd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I really like _create_fake_token's implementation!

@laiapat
laiapat merged commit bf4749b into Azure:main Aug 29, 2022
@laiapat
laiapat deleted the generate-sas-sanitizer branch August 29, 2022 21:04
laiapat added a commit to laiapat/azure-sdk-for-python that referenced this pull request Sep 22, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Oct 25, 2023
Add 2023-11-01-preview for appplatform (Azure#26231)

* Adds base for updating Microsoft.AppPlatform from version preview/2023-09-01-preview to version 2023-11-01-preview

* Updates readme

* Updates API version in new specs and examples

* Fix go readme for asa api (Azure#25897)

* Update language specific readme.md for 2023-11-01-preview api version (Azure#25898)

* add more build result information in build resource (Azure#25930)

Co-authored-by: 刘清仪 <liuqingyi@liuqingyideMacBook-Pro.local>

* Add apiTryOutEnabledState for api protal (Azure#25923)

* Add swagger for Spring Cloud Gateway response cache (Azure#25924)

* Update appplatform.json (Azure#26234)

* Update appplatform.json

* Update appplatform.json

---------

Co-authored-by: Sandy Cai <sandy.caiqing@gmail.com>
Co-authored-by: Qingyi Liu <34502364+smile37773@users.noreply.github.com>
Co-authored-by: 刘清仪 <liuqingyi@liuqingyideMacBook-Pro.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

EngSys This issue is impacting the engineering system. test-enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants