Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions internal/extproc/backendauth/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ func (a *awsHandler) Do(ctx context.Context, requestHeaders map[string]string, h
if err != nil {
return fmt.Errorf("cannot create request: %w", err)
}
// By setting the content length to -1, we can avoid the inclusion of the `Content-Length` header in the signature.
// https://github.com/aws/aws-sdk-go-v2/blob/755839b2eebb246c7eec79b65404aee105196d5b/aws/signer/v4/v4.go#L427-L431
//
// The reason why we want to avoid this is that the ExtProc filter will remove the content-length header
// from the request currently. Envoy will instead do "transfer-encoding: chunked" for the request body,
// which should be acceptable for AWS Bedrock or any modern HTTP service.
// https://github.com/envoyproxy/envoy/blob/60b2b5187cf99db79ecfc54675354997af4765ea/source/extensions/filters/http/ext_proc/processor_state.cc#L180-L183
req.ContentLength = -1
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still no idea why AWS doesn't complain when the body is small but at least this makes all tests happy

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my best guess is that if there's only one chunk in the request (== small body), then AWS will fill in the content-length with the length of the only chunk and matches the signature. But we'll never know...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ye I think there is some special handling on the AWS side


err = a.signer.SignHTTP(ctx, a.credentials, req,
hex.EncodeToString(payloadHash[:]), "bedrock", a.region, time.Now())
Expand Down
51 changes: 30 additions & 21 deletions tests/extproc/real_providers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,34 +85,14 @@ func TestWithRealProviders(t *testing.T) {
requireExtProc(t, os.Stdout, extProcExecutablePath(), configPath)

t.Run("health-checking", func(t *testing.T) {
client := openai.NewClient(option.WithBaseURL(listenerAddress + "/v1/"))
for _, tc := range []realProvidersTestCase{
{name: "openai", modelName: "gpt-4o-mini", required: internaltesting.RequiredCredentialOpenAI},
{name: "aws-bedrock", modelName: "us.meta.llama3-2-1b-instruct-v1:0", required: internaltesting.RequiredCredentialAWS},
{name: "azure-openai", modelName: "o1", required: internaltesting.RequiredCredentialAzure},
} {
t.Run(tc.modelName, func(t *testing.T) {
cc.MaybeSkip(t, tc.required)
require.Eventually(t, func() bool {
chatCompletion, err := client.Chat.Completions.New(t.Context(), openai.ChatCompletionNewParams{
Messages: []openai.ChatCompletionMessageParamUnion{
openai.UserMessage("Say this is a test"),
},
Model: tc.modelName,
})
if err != nil {
t.Logf("error: %v", err)
return false
}
nonEmptyCompletion := false
for _, choice := range chatCompletion.Choices {
t.Logf("choice: %s", choice.Message.Content)
if choice.Message.Content != "" {
nonEmptyCompletion = true
}
}
return nonEmptyCompletion
}, eventuallyTimeout, eventuallyInterval)
requireEventuallyNonStreamingRequestOK(t, tc.modelName, "Say this is a test")
})
}
})
Expand Down Expand Up @@ -322,6 +302,11 @@ func TestWithRealProviders(t *testing.T) {
"o1",
}, models)
})
t.Run("aws-bedrock-large-body", func(t *testing.T) {
cc.MaybeSkip(t, internaltesting.RequiredCredentialAWS)
requireEventuallyNonStreamingRequestOK(t,
"us.meta.llama3-2-1b-instruct-v1:0", strings.Repeat("Say this is a test", 10000))
})
}

// realProvidersTestCase is a base test case for the real providers, which is mainly for the centralization of the
Expand All @@ -331,3 +316,27 @@ type realProvidersTestCase struct {
modelName string
required internaltesting.RequiredCredential
}

func requireEventuallyNonStreamingRequestOK(t *testing.T, modelName, msg string) {
client := openai.NewClient(option.WithBaseURL(listenerAddress + "/v1/"))
require.Eventually(t, func() bool {
chatCompletion, err := client.Chat.Completions.New(t.Context(), openai.ChatCompletionNewParams{
Messages: []openai.ChatCompletionMessageParamUnion{
openai.UserMessage(msg),
},
Model: modelName,
})
if err != nil {
t.Logf("error: %v", err)
return false
}
nonEmptyCompletion := false
for _, choice := range chatCompletion.Choices {
t.Logf("choice: %s", choice.Message.Content)
if choice.Message.Content != "" {
nonEmptyCompletion = true
}
}
return nonEmptyCompletion
}, eventuallyTimeout, eventuallyInterval)
}