Skip to content

Commit

Permalink
feat: force fully qualified domains for s3 requests TDE-1084 (#940)
Browse files Browse the repository at this point in the history
#### Motivation

Inside kubernetes dns resolution uses `ndots: 5` this means any dns
requests that has less than 5 dots will go through all the search
domains before looking up the actual domain. AWS s3 has 4 dots.

We are seeing 10-20 DNS requests per s3 DNS request due to the number of
search domains as well as IPV6 requests.

#### Modification

Any DNS requests to S3 are modified to have a trailing "." to force the
DNS lookup to be fully qualified.

#### Checklist

_If not applicable, provide explanation of why._

- [x] Tests updated
- [ ] Docs updated
- [x] Issue linked in Title
  • Loading branch information
blacha authored Mar 26, 2024
1 parent df30792 commit 3fdbf98
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 2 deletions.
37 changes: 37 additions & 0 deletions src/__test__/fqdn.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import assert from 'node:assert';
import { describe, it } from 'node:test';

import { FinalizeHandler, MetadataBearer } from '@smithy/types';

import { fqdn } from '../fs.register.js';

describe('fqdnMiddleware', () => {
const fakeNext: FinalizeHandler<object, MetadataBearer> = () => {
return Promise.resolve({ output: { $metadata: {} }, response: {} });
};
const fakeRequest = { input: {}, request: { hostname: 'nz-imagery.s3.ap-southeast-2.amazonaws.com' } };

it('should add FQDN to s3 requests', () => {
fakeRequest.request.hostname = 'nz-imagery.s3.ap-southeast-2.amazonaws.com';
fqdn(fakeNext, {})(fakeRequest);
assert.equal(fakeRequest.request.hostname, 'nz-imagery.s3.ap-southeast-2.amazonaws.com.');
});

it('should not add for other services', () => {
fakeRequest.request.hostname = 'logs.ap-southeast-2.amazonaws.com';
fqdn(fakeNext, {})(fakeRequest);
assert.equal(fakeRequest.request.hostname, 'logs.ap-southeast-2.amazonaws.com');
});

it('should not add for other regions', () => {
fakeRequest.request.hostname = 'nz-imagery.s3.us-east-1.amazonaws.com';
fqdn(fakeNext, {})(fakeRequest);
assert.equal(fakeRequest.request.hostname, 'nz-imagery.s3.us-east-1.amazonaws.com');
});

it('should not add for unknown hosts', () => {
fakeRequest.request.hostname = 'google.com';
fqdn(fakeNext, {})(fakeRequest);
assert.equal(fakeRequest.request.hostname, 'google.com');
});
});
40 changes: 38 additions & 2 deletions src/fs.register.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,51 @@
import { S3Client } from '@aws-sdk/client-s3';
import { FileSystem } from '@chunkd/core';
import { fsa } from '@chunkd/fs';
import { AwsCredentialConfig } from '@chunkd/source-aws';
import { FsAwsS3 } from '@chunkd/source-aws';
import { FsAwsS3V3 } from '@chunkd/source-aws-v3';
import { FsAwsS3V3, S3LikeV3 } from '@chunkd/source-aws-v3';
import { FinalizeRequestMiddleware, MetadataBearer } from '@smithy/types';

import { logger } from './log.js';

export const s3Fs = new FsAwsS3V3();
/** Check to see if hostname exists inside of a object */
function hasHostName(x: unknown): x is { hostname: string } {
if (x == null) return false;
if (typeof x === 'object' && 'hostname' in x && typeof x.hostname === 'string') return true;
return false;
}

/**
* AWS SDK middleware function to force fully qualified domain name on s3 requests
*
* AWS S3 inside of kubernetes triggers a lot of DNS requests
* by forcing a fully qualified domain name lookup (trailing ".")
* it greatly reduces the number of DNS requests we make
*/
export const fqdn: FinalizeRequestMiddleware<object, MetadataBearer> = (next) => {
return (args) => {
if (hasHostName(args.request) && args.request.hostname.endsWith('.s3.ap-southeast-2.amazonaws.com')) {
args.request.hostname += '.';
}
return next(args);
};
};

const client = new S3Client();
export const s3Fs = new FsAwsS3V3(client);
client.middlewareStack.add(fqdn, { name: 'FQDN', step: 'finalizeRequest' });

FsAwsS3.MaxListCount = 1000;
s3Fs.credentials.onFileSystemCreated = (acc: AwsCredentialConfig, fs: FileSystem): void => {
logger.debug({ prefix: acc.prefix, roleArn: acc.roleArn }, 'FileSystem:Register');

if (fs.protocol === 's3') {
// TODO this cast can be removed once chunkd is upgraded
const fsS3 = fs as FsAwsS3V3;
const fsClient = fsS3.s3 as S3LikeV3;
fsClient.client.middlewareStack.add(fqdn, { name: 'FQDN', step: 'finalizeRequest' });
}

fsa.register(acc.prefix, fs);
};

Expand Down

0 comments on commit 3fdbf98

Please sign in to comment.