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

[Test-Proxy] Preserve original format when returning recorded response body content #5968

Closed
mccoyp opened this issue Apr 13, 2023 · 2 comments
Assignees
Labels
feature-request This issue requires a new behavior in the product in order be resolved. Test-Proxy Anything relating to test-proxy requests or issues.
Milestone

Comments

@mccoyp
Copy link
Member

mccoyp commented Apr 13, 2023

Context

Python's azure-containerregistry library has tests that read a JSON manifest file and send this content to the service. The service creates a digest based on the stream content that was sent; this content is whitespace-dependent. Therefore, the digest we get from e.g. manifest_spaces.json:

{"foo": 2, "bar": "baz"}

will not match the digest we get from e.g. manifest_no_spaces.json:

{"foo":2,"bar":"baz"}

This is because the former digest is calculated with additional spaces in the stream.

In live mode, tests that validate the locally-calculated digest with the service-returned digest succeed when using manifest_spaces.json, since spaces are used in both digest calculations. The problem is that these tests fail in playback -- we calculate the local digest using spaces, but the recorded service response is returned without spaces by the test proxy.

For example, here's a recorded manifest from a response body (source from the assets repository):

"ResponseBody": {
  "config": {
    "digest": "sha256:feb5d9fea6a5e9606aa995e879d862b825965ba48de054caab5ef356dc6b3412",
    "mediaType": "application/vnd.docker.container.image.v1\u002Bjson",
    "size": 1469
  },
  "layers": [
    {
      "digest": "sha256:2db29710123e3e53a794f2694094b9b4338aa9ee5c40b930cb8063a1be392c54",
      "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
      "size": 2479
    }
  ],
  "mediaType": "application/vnd.docker.distribution.manifest.v2\u002Bjson",
  "schemaVersion": 2
}

When the test proxy sends the response body content in playback, the recorded JSON is written back without any whitespace -- no spaces between keys and values. Because of this, the digest that's calculated from the response body differs from the one calculated from the local manifest file.

Feature request

If we had a way to request a certain formatting of the response body, for example with a regular expression, we could preserve the actual response body format in playback. As things stand, the response body content in live mode doesn't exactly match the response body content in playback.

@scbedd I could probably help with this, but I would need assistance in figuring out where exactly the response body is fetched from the recording and sent back to the test.

@mccoyp mccoyp added feature-request This issue requires a new behavior in the product in order be resolved. Test-Proxy Anything relating to test-proxy requests or issues. labels Apr 13, 2023
@mccoyp mccoyp added this to the Backlog milestone Apr 13, 2023
@ghost ghost assigned mario-guerra Apr 13, 2023
@mario-guerra mario-guerra removed their assignment Apr 13, 2023
@scbedd
Copy link
Member

scbedd commented Apr 19, 2023

To properly do this, we need to maintain exactly the body that was sent. This will mean storing the Request/Response body as base64 encoded value.

We can either:

  1. Add a new attribute, store the body there. Keep existing RequestBody for visibility only.
  2. Replace RequestBody/ResponseBody w/ the base64 value.

In either case, we will need to update at the very least:

And any attachment points related to these.

@mccoyp mccoyp changed the title [Test-Proxy] Accept output format when returning recorded response body content [Test-Proxy] Preserve original format when returning recorded response body content Apr 19, 2023
@scbedd
Copy link
Member

scbedd commented Jun 27, 2024

@mccoyp this is now resolved. You will need to re-record the tests, but we now treat these as non-text bodies so they are stored byte-for-byte as they come in.

Resolved in #8430 #8325

@scbedd scbedd closed this as completed Jun 27, 2024
@scbedd scbedd moved this from 🤔 Triage to 🔬 Dev in PR in Azure SDK EngSys 🤖🧠 Jun 27, 2024
@scbedd scbedd moved this from 🔬 Dev in PR to 🎊 Closed in Azure SDK EngSys 🤖🧠 Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requires a new behavior in the product in order be resolved. Test-Proxy Anything relating to test-proxy requests or issues.
Projects
Archived in project
Development

No branches or pull requests

3 participants