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

Incorrect hostname when both useAccelerateEndpoint and useDualstackEndpoint are enabled #3001

Closed
trivikr opened this issue Nov 8, 2021 · 5 comments · Fixed by #3003
Closed
Labels
bug This issue is a bug.

Comments

@trivikr
Copy link
Member

trivikr commented Nov 8, 2021

Describe the bug

The SDK sets incorrect hostname when both useAccelerateEndpoint and useDualstackEndpoint are enabled

Your environment

SDK version number

@aws-sdk/client-s3@3.40.0

Is the issue in the browser/Node.js/ReactNative?

All

Details of the browser/Node.js/ReactNative version

$ node -v
v16.13.0

Steps to reproduce

Code
import { S3 } from "@aws-sdk/client-s3";

const region = "us-west-2";
const useAccelerateEndpoint = true;
const useDualstackEndpoint = true;

// Bucket with transfer acceleration enabled.
const Bucket = "trivikr-accelerate-testing";

const client = new S3({ region, useAccelerateEndpoint, useDualstackEndpoint });
await client.listObjects({ Bucket });

Observed behavior

throws getaddrinfo ENOTFOUND error.

Output
node:internal/process/esm_loader:94
    internalBinding('errors').triggerUncaughtException(
                              ^

Error: getaddrinfo ENOTFOUND trivikr-accelerate-testing.s3-accelerate.dualstack.us-west-2.amazonaws.com
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:71:26) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'trivikr-accelerate-testing.s3-accelerate.dualstack.us-west-2.amazonaws.com',
  '$metadata': { attempts: 1, totalRetryDelay: 0 }
}

Expected behavior

Sets correct hostname when both useAccelerateEndpoint and useDualstackEndpoint

@trivikr trivikr added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 8, 2021
@trivikr
Copy link
Member Author

trivikr commented Nov 8, 2021

For the example provided in the bug, the hostname is

trivikr-accelerate-testing.s3-accelerate.dualstack.us-west-2.amazonaws.com

It should be:

trivikr-accelerate-testing.s3-accelerate.dualstack.amazonaws.com

Equivalent AWS CLI code:

$ aws --version
aws-cli/2.2.12 Python/3.8.8 Darwin/20.6.0 exe/x86_64 prompt/off

$ aws configure set default.s3.use_dualstack_endpoint true

$ aws s3api list-objects --bucket trivikr-accelerate-testing --endpoint-url https://s3-accelerate.amazonaws.com --debug 2>&1 | grep "urllib3.connectionpool"
2021-11-08 09:49:35,075 - MainThread - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): trivikr-accelerate-testing.s3-accelerate.amazonaws.com:443
2021-11-08 09:49:35,387 - MainThread - urllib3.connectionpool - DEBUG - https://trivikr-accelerate-testing.s3-accelerate.amazonaws.com:443 "GET /?encoding-type=url HTTP/1.1" 200 None

@trivikr trivikr removed the needs-triage This issue or PR still needs to be triaged. label Nov 8, 2021
@trivikr
Copy link
Member Author

trivikr commented Nov 8, 2021

This issue was introduced in v3.40.0 release. It's not reproducible with v3.39.0

@trivikr
Copy link
Member Author

trivikr commented Nov 8, 2021

This issue happens as .dualstack is prefixed in build stage

Code
import { S3 } from "@aws-sdk/client-s3";

const logHostnameMiddleware = (next) => async (args) => {
  console.log({ hostname: args.request.hostname });
  return next(args);
};
const logHostnameMiddlewareOptions = { step: "build" };

const region = "us-west-2";
const useAccelerateEndpoint = true;
const useDualstackEndpoint = true;

// Bucket with transfer acceleration enabled.
const Bucket = "trivikr-accelerate-testing";

const client = new S3({ region, useAccelerateEndpoint, useDualstackEndpoint });
client.middlewareStack.add(logHostnameMiddleware, logHostnameMiddlewareOptions);
await client.listObjects({ Bucket });
Output
{ hostname: 's3.dualstack.us-west-2.amazonaws.com' }

In v3.39.0 and earlier, the .dualstack was added in bucketEndpointMiddleware

@trivikr
Copy link
Member Author

trivikr commented Nov 8, 2021

This happens as resolution of useDualstackEndpoint is done while resolving endpoint, as the configuration is now applicable to all clients.
Added in #2947

The .dualstack now comes from endpoint resolution:

"us-west-2": {
variants: [
{
hostname: "s3.us-west-2.amazonaws.com",
tags: [],
},
{
hostname: "s3-fips.dualstack.us-west-2.amazonaws.com",
tags: ["dualstack", "fips"],
},
{
hostname: "s3-fips.us-west-2.amazonaws.com",
tags: ["fips"],
},
{
hostname: "s3.dualstack.us-west-2.amazonaws.com",
tags: ["dualstack"],
},
],
},

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug.
Projects
None yet
1 participant