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

[FEATURE] Add support for OpenSearch Serverless #356

Closed
Tracked by #44
dblock opened this issue Jan 3, 2023 · 17 comments · Fixed by #366 or #377
Closed
Tracked by #44

[FEATURE] Add support for OpenSearch Serverless #356

dblock opened this issue Jan 3, 2023 · 17 comments · Fixed by #366 or #377

Comments

@dblock
Copy link
Member

dblock commented Jan 3, 2023

Is your feature request related to a problem?

With #333 one can supply aoss. I believe client.info() will be broken because that doesn't exist in serverless. What else?

What solution would you like?

Document support for OpenSearch Serverless in the compatibility matrix, add integration tests.

Do you have any additional context?

Coming from opensearch-project/opensearch-clients#44

@magoz
Copy link
Contributor

magoz commented Jan 5, 2023

I swapped es with aoss manually in 2.1.0:

  function buildSignedRequestObject(request = {}) {
    request.service = 'aoss';   // <-------------------------
    request.region = opts.region;
    request.headers = request.headers || {};
    request.headers['host'] = request.hostname;
    return aws4.sign(request, credentialsState.credentials);
  }

So far, it allows me to create and query indices via client.indices.create().
But I get access denied when trying to use client.index().

@bwg
Copy link
Contributor

bwg commented Jan 5, 2023

Request signatures continue to be the biggest pain point for us. The decision to disallow content-length as a signed header is particularly frustrating in concert with the aws4 signing library the opensearch-js sdk currently uses.

aws4 will add a missing content-length header prior to signing, so delete request.headers['content-length'] wont work, and setting request.headers['content-length'] = '0'; as suggested in the developer guide no longer works as it appears something has changed with how the signature is validated on the server (setting content-length = '0' did work previously).

aws4 does not have functionality to provide a set of unsignable headers, and previous requests for that functionality have been denied by the maintainer, instead opting to add service-specific logic internally.

The AWS Javascript v3 sdk has a standalone signature v4 package (@aws-sdk/signature-v4) that has builtin credential support and support for unsignable headers, which would solve the content-length problem as well as simplify a lot of the credential handling code in AwsSigv4Signer. Unfortunately the SignatureV4::sign() method returns a promise, which doesn't work with how the opensearch-js library makes requests.

Perhaps a PR to aws4 that adds aoss specific functionality to ignore the content-length header is the easiest approach, with supporting an async/promise based signing library (@aws-sdk/signature-v4) a follow on.

@dblock
Copy link
Member Author

dblock commented Jan 6, 2023

cc: @shwetathareja ^ I'll bring this up with the serverless team, the same problem with this zero content length came up in acm19/aws-request-signing-apache-interceptor#90.

@richdredge
Copy link

We're having the same experience, unable to get a connection to work. Aside from a fix on the aws4 signer, is there any other way to possibly use the serverless API ?

@dblock
Copy link
Member Author

dblock commented Jan 7, 2023

Same. I tried to use the example in https://docs.aws.amazon.com/opensearch-service/latest/developerguide/serverless-clients.html#serverless-javascript as is and it didn't work. I'll sort it out, stay tuned.

@dblock
Copy link
Member Author

dblock commented Jan 7, 2023

@mhart
Copy link

mhart commented Jan 7, 2023

Thanks for bringing this to my attention. This is now supported in v1.12.0 of aws4:

Eg:

aws4.sign({
  host: '07tjusf2h91cunochc.us-east-1.aoss.amazonaws.com',
  method: 'PUT',
  path: '/my-index',
  body: '{"mappings":{}}',
  headers: {
    'Content-Type': 'application/json',
    'X-Amz-Content-Sha256': 'UNSIGNED-PAYLOAD'
  },
  extraHeadersToIgnore: {
    'content-length': true
  }
})

@dblock
Copy link
Member Author

dblock commented Jan 8, 2023

Thanks @mhart. Doesn't seem like that was necessary after all (although it's a good feature). In dblock/opensearch-node-client-demo@9290669 I deleted the body before singing it and putting it back and everything seems to work.

@bwg I think that's the workaround for preview. We need to modify opensearch-js to do that for now I imagine (want to try it?).

I'm aking the serverless folks to make Sigv4 work the same way as for the managed service, so I expect the above to change before it's officially launched out of preview.

@bwg
Copy link
Contributor

bwg commented Jan 9, 2023

@dblock I prefer the explicitness of the extraHeadersToIgnoreapproach vs. removing and re-adding request properties. I'm happy to put a PR together for that approach.

hopefully you'll have a better chance convincing the serverless team to change the signature requirements than I did.

@bwg
Copy link
Contributor

bwg commented Jan 13, 2023

After way to much hacking about with every combination I can think of, I am unable to get serverless to accept a request signed with extraHeadersToIgnore = {'content-length': true};

The only way I am able to get the sdk/aws4 to generate a signature that serverless will accept is to delete both the request.body and request.headers['content-length']; prior to signing, then putting those values back afterward ... like @dblock showed in his example.

Serverless does not provide an expected "string to sign" on failure, so its like punching in the dark to figure out what it doesn't like about the signature.

As luck would have it, the serverless team reached out the other day and they said they would update their guidance on how to use serverless with the opensearch-js sdk. We'll see what they come back with.

@mhart
Copy link

mhart commented Jan 14, 2023

@bwg that's super annoying. The fact that you have to delete the body is even more annoying. Just to double check, you are providing the 'X-Amz-Content-Sha256': 'UNSIGNED-PAYLOAD' header, right?

@dblock
Copy link
Member Author

dblock commented Jan 14, 2023

I'm actively working with the serverless team on this. We have the goal that no changes in clients is required other than the signing service name, and that serverless behaves exactly as the Amazon Managed OpenSearch service. I expect the whole X-Amz-Content-Sha256 to go away soon. Stay tuned!

Btw, if you find any other API change vs. the managed service, please speak up and we'll take a look. Easy to fix before it launches while it's in preview.

@bwg
Copy link
Contributor

bwg commented Jan 15, 2023

serverless behaves exactly as the Amazon Managed OpenSearch

Thanks for advocating for this @dblock

Just to double check, you are providing the 'X-Amz-Content-Sha256': 'UNSIGNED-PAYLOAD' header, right?

@mhart yep. tried it with a legit Sha256 hash too. 🤷‍♂️

@magoz
Copy link
Contributor

magoz commented Jan 20, 2023

Really appreciate your work @dblock! Following this closely.

@harshavamsi
Copy link
Collaborator

#367 -- I have a sample PR here that computes the hash in the header but there is an invalid signature mismatch. Does anyone have a clue why this is failing?

@dblock
Copy link
Member Author

dblock commented Jan 20, 2023

OpenSearch serverless aligned itself to the managed service, but signing the body is required. Works the same on both managed and serverless with that. Ruby client from HEAD working here.

@magoz
Copy link
Contributor

magoz commented Jan 28, 2023

The types weren't updated, so I've added a new PR #374.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment