Skip to content

Improve test proxy sanitizers #4796

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

Merged
merged 1 commit into from
Jul 25, 2023
Merged

Conversation

Jinming-Hu
Copy link
Member

@Jinming-Hu Jinming-Hu commented Jul 17, 2023

In this PR we:

  1. Updated code logic to set sanitizers. Defined a function addSanitizer and call this function for each sanitizer, instead of writing duplicate code many times.
  2. Constructed json body with json library, instead of hard-coding
  3. Updated code logic for TestProxyManager::CheckSanitizers
  4. Defined constant that's used more than once as a global variable.
  5. Added two sanitizers to scrub storage sas tokens and user delegation keys
  6. Removed account UriRegexSanitizer as it's already covered by GeneralRegexSanitizer
  7. Removed account HeaderRegexSanitizer as it's already covered by GeneralRegexSanitizer. Plus this was not correctly set and never worked. According to the source code, HeaderRegexSanitizer requires a key which specifies which HTTP header this sanitizer applies to.

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

  • C++ Guidelines
  • Doxygen docs
  • Unit tests
  • No unwanted commits/changes
  • Descriptive title/description
    • PR is single purpose
    • Related issue listed
  • Comments in source
  • No typos
  • Update changelog
  • Not work-in-progress
  • External references or docs updated
  • Self review of PR done
  • Any breaking changes?

@Jinming-Hu Jinming-Hu added test-enhancement test-proxy Anything relating to test-proxy requests or issues. labels Jul 17, 2023
@Jinming-Hu Jinming-Hu self-assigned this Jul 17, 2023
@Jinming-Hu
Copy link
Member Author

@RickWinter @LarryOsterman @antkmsft @gearama @ahsonkhan can you help review this PR? Thanks

@Jinming-Hu Jinming-Hu merged commit ce85f6f into Azure:main Jul 25, 2023
@Jinming-Hu Jinming-Hu deleted the storage_cred branch July 25, 2023 04:52
microzchang added a commit that referenced this pull request Jul 27, 2023
* Set DOCKER_BUILDKIT to 1 in stress deploy image build (#4815)

Co-authored-by: Ben Broderick Phillips <[email protected]>

* Make x509 certificate script from azure-sdk-for-net common to repos (#4817)

Co-authored-by: Ben Broderick Phillips <[email protected]>

* improve test proxy sanitizers (#4796)

* Storage test cases improvements (#4819)

* soft delete test

* enable setTierCold

* IsConnectionReuse in record-playback mode

* DISABLED_UploadPagesFromUriCrc64AccessCondition

* update recording assets

* ClientSecretCredentialWorks works in record-playback mode

* ServiceContainerSasPermissions and ServiceBlobSasPermissions work in record-playback mode

* BlobServiceClientTest.UserDelegationKey works in record-playback mode

* update recordings

* f

---------

Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: Ben Broderick Phillips <[email protected]>
Co-authored-by: JinmingHu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test-enhancement test-proxy Anything relating to test-proxy requests or issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants