-
Notifications
You must be signed in to change notification settings - Fork 60
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
Added optional headers to the AWS SigningDecorator. #253
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: dblock <[email protected]>
Thank you for the fix. I have confirmed that it works correctly in my environment. As stated in the following documentation:
The Since the Although the current OpenSearch does not support HTTP/2.0, it is planned to support it in version 3.0, so the following code conforms to that specification: public function sendRequest(RequestInterface $request): ResponseInterface
{
foreach ($this->headers as $name => $value) {
$request = $request->withHeader($name, $value);
}
if ($request->getProtocolVersion() === 'HTTP/1.0' || $request->getHeader('Host') === null) {
throw new \IRuntimeException('Header Host must be set');
}
if ($request->getProtocolVersion() === 'HTTP/2.0' || $request->getHeader(':authority') === null) {
throw new \RuntimeException('Header authority must be set');
}
$request = $request->withHeader('x-amz-content-sha256', hash('sha256', (string) $request->getBody()));
$request = $this->signer->signRequest($request, $this->credentials);
return $this->inner->sendRequest($request);
} |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
============================================
+ Coverage 24.02% 24.06% +0.03%
- Complexity 3398 3400 +2
============================================
Files 485 485
Lines 12984 12988 +4
============================================
+ Hits 3120 3126 +6
+ Misses 9864 9862 -2 ☔ View full report in Codecov by Sentry. |
@dmnlk Good point. I updated the code to raise an error when the HTTP2 support in OpenSearch was added in opensearch-project/OpenSearch#3847 and I am not sure whether AWS OpenSearch supports it today. For this PR I didn't include the protocol check, I'd prefer it if we made sure we can do HTTP/2 first, then add code like this (please do help!). |
ed6a44e
to
dc07f55
Compare
Good!! Thanks! @dblock |
Signed-off-by: dblock <[email protected]>
dc07f55
to
ad4ed40
Compare
Description
headers
to the signing decorator to passHost
.Issues Resolved
Closes #248.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.