From 3fdbf98b600ca27d67bbc807dedad2f1bf16029b Mon Sep 17 00:00:00 2001
From: Blayne Chard <bchard@linz.govt.nz>
Date: Wed, 27 Mar 2024 11:49:50 +1300
Subject: [PATCH] feat: force fully qualified domains for s3 requests TDE-1084
 (#940)

#### 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
---
 src/__test__/fqdn.test.ts | 37 ++++++++++++++++++++++++++++++++++++
 src/fs.register.ts        | 40 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 75 insertions(+), 2 deletions(-)
 create mode 100644 src/__test__/fqdn.test.ts

diff --git a/src/__test__/fqdn.test.ts b/src/__test__/fqdn.test.ts
new file mode 100644
index 00000000..64e7f3ff
--- /dev/null
+++ b/src/__test__/fqdn.test.ts
@@ -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');
+  });
+});
diff --git a/src/fs.register.ts b/src/fs.register.ts
index 662f4555..7c4af6bd 100644
--- a/src/fs.register.ts
+++ b/src/fs.register.ts
@@ -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);
 };