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

feat: force fully qualified domains for s3 requests TDE-1084 #940

Merged
merged 2 commits into from
Mar 26, 2024
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
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')) {
l0b0 marked this conversation as resolved.
Show resolved Hide resolved
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
l0b0 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading