localstack / docs

The LocalStack documentation 📖
https://docs.localstack.cloud
Apache License 2.0
54 stars 97 forks source link

Go documentation refers to deprecated calls #1315

Open quinn opened 1 month ago

quinn commented 1 month ago

Here are the deprecation warnings:

aws.EndpointResolverWithOptionsFunc is deprecated: The global endpoint resolution interface is deprecated. See deprecation docs on [EndpointResolver].

aws.Endpoint is deprecated: This structure was used with the global [EndpointResolver] interface, which has been deprecated in favor of service-specific endpoint resolution. See the deprecation docs on that interface for more information.

This is the config I'm using, instead of the deprecated method in your doc:

        cfg, err := config.LoadDefaultConfig(context.Background(),
            config.WithCredentialsProvider(credentials.StaticCredentialsProvider{
                Value: aws.Credentials{
                    AccessKeyID:     "N/A",
                    SecretAccessKey: "N/A",
                },
            }),
        )
        if err != nil {
            return nil, fmt.Errorf("unable to load SDK config for local endpoint, %v", err)
        }

        svc = s3.NewFromConfig(cfg,
            s3.WithEndpointResolverV2(&S3LocalResolver{hostAndPort: "localhost:4566"}),
            func(o *s3.Options) { o.UsePathStyle = true },
        )

However, a putObject operation does not work with this config:

s3-1  | 2024-06-09T19:08:32.476 ERROR --- [et.reactor-0] l.aws.handlers.logging     : exception during call chain: Unable to parse request (syntax error: line 1, column 0), invalid XML received:
s3-1  | b'hello'
s3-1  | 2024-06-09T19:08:32.478  INFO --- [et.reactor-0] localstack.request.http    : PUT /upload-test => 500
s3-1  | 2024-06-09T19:08:33.995 ERROR --- [et.reactor-0] l.aws.handlers.logging     : exception during call chain: Unable to parse request (syntax error: line 1, column 0), invalid XML received:
s3-1  | b'hello'
s3-1  | 2024-06-09T19:08:33.996  INFO --- [et.reactor-0] localstack.request.http    : PUT /upload-test => 500
s3-1  | 2024-06-09T19:08:37.592 ERROR --- [et.reactor-0] l.aws.handlers.logging     : exception during call chain: Unable to parse request (syntax error: line 1, column 0), invalid XML received:
s3-1  | b'hello'
s3-1  | 2024-06-09T19:08:37.594  INFO --- [et.reactor-0] localstack.request.http    : PUT /upload-test => 500

this works using the aws cli:

aws s3api create-bucket --endpoint-url http://localhost:4566 --bucket test
aws s3 cp --endpoint-url http://localhost:4566 ./test-file s3://test

I'm not sure why the golang sdk is attempting to upload the file "directly" (or something?) whereas aws s3 cp uses the PutObject op.

bentsku commented 1 month ago

Hello!

From the LocalStack logs, it seems the UsePathStyle option is not being applied or taken into account. The logs you are seeing and what you're referring to with:

I'm not sure why the golang sdk is attempting to upload the file "directly" (or something?) whereas aws s3 cp uses the PutObject op.

Is your SDK sending the request to an host looking like http://<bucket-name>.localhost:4566/<your-object-key> which I suppose is upload-test.

LocalStack cannot parse this request, you'll need to prefix your endpoint, like the following : s3.localhost.localstack.cloud:4566. You can read more about it here: https://docs.localstack.cloud/user-guide/aws/s3/#path-style-and-virtual-hosted-style-requests

I'm trying to look for information about the Go SDK v2 and path style requests and found this documentation:

There's a note in the documentation here:

A note about Amazon S3

Amazon S3 is a complex service with many of its features modeled through complex endpoint customizations, such as bucket virtual hosting, S3 MRAP, and more. Because of this, we recommend that you don’t replace the EndpointResolverV2 implementation in your S3 client. If you need to extend its resolution behavior, perhaps by sending requests to a local development stack with additional endpoint considerations, we recommend wrapping the default implementation such that it delegates back to the default as a fallback (shown in examples below).

Even if deprecated, seeing this note, I believe it makes sense to keep the v1 endpoint resolver for now, we can however add a note about this and to the implementation in the v2 documentation. Or maybe one with BaseEndpoint, if this works with path style.

quinn commented 1 month ago

Hey, thanks for looking into this!

I tried BaseEndpoint, it did not work. I replaced my code with the deprecated code and it worked!

I think the code in the localstack docs is the only way to do this right now. I can make a PR to make a note of this in the doc if that would be helpful.