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

AWS credentials resolution for Sagemaker models #926

Open
nason opened this issue Mar 12, 2024 · 1 comment
Open

AWS credentials resolution for Sagemaker models #926

nason opened this issue Mar 12, 2024 · 1 comment

Comments

@nason
Copy link
Contributor

nason commented Mar 12, 2024

chat-ui is excellent, thanks for all your amazing work here!

I have been experimenting with a model in Sagemaker and am having some issues with the model endpoint configuration. It currently requires credentials to be provided explicitly. This does work, but the ergonomics are not great for our use cases:

  • in development, my team uses AWS SSO and it would be great to use our session credentials and not need to update our MODELS environment variable manually every time our sessions refresh
  • in deployments, we would want to use an instance or task execution role to sign requests

In my investigation I found this area of code

try {
AwsClient = (await import("aws4fetch")).AwsClient;
} catch (e) {
throw new Error("Failed to import aws4fetch");
}
const { url, accessKey, secretKey, sessionToken, model, region, service } =
endpointAwsParametersSchema.parse(input);
const aws = new AwsClient({
accessKeyId: accessKey,
secretAccessKey: secretKey,
sessionToken,
service,
region,
});
, which uses the aws4fetch library that only support signing with explicitly passed AWS credentials.

I was able to update this area of code locally and support AWS credential resolution by switching this to use a different library aws-sigv4-fetch like so:

try {
	createSignedFetcher = (await import("aws-sigv4-fetch")).createSignedFetcher;
} catch (e) {
	throw new Error("Failed to import aws-sigv4-fetch");
}

const { url, accessKey, secretKey, sessionToken, model, region, service } =
	endpointAwsParametersSchema.parse(input);

const signedFetch = createSignedFetcher({
	service,
	region,
	credentials:
		accessKey && secretKey
			? { accessKeyId: accessKey, secretAccessKey: secretKey, sessionToken }
			: undefined,
});

// Replacer `aws.fetch` with `signedFetch` below when passing `fetch` to `textGenerationStream#options`

My testing has found this supports passing credentials like today, or letting the AWS SDK resolve them through the default chain.

Would you be open to a PR with this change? Or is there a different/better/more suitable way to accomplish AWS credential resolution here?

@nsarrazin
Copy link
Collaborator

Yes I think that would be a great addition, especially since it's an added feature that doesn't require breaking changes 😄 Feel free to open a PR and I'll have a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants