From dd6f22820ffeb9de4cac20bb58911127372e3085 Mon Sep 17 00:00:00 2001
From: Ryan Parker <ryan.parker3@outlook.com>
Date: Thu, 7 Oct 2021 00:55:38 -0700
Subject: [PATCH] fix(aws-eks,sdk-provider): full proxy support (#16840)

## Summary

CDK was extracting the value of HTTPS?_PROXY and passing this to proxy-agent explicitly, which resulted in not honoring the NO_PROXY setting.

This removes that behavior and lets proxy-agent delegate to proxy-from-env, which will leverage values in HTTPS?_PROXY and NO_PROXY correctly.

Tested by deploying [this sample repo](https://github.com/ryparker/aws-cdk-sample-eks) and monitoring Squid proxy logs while triggering the "onEvent" Lambda.

Fixes #7121
Related PRs: https://github.com/aws/aws-cdk/pull/16751, https://github.com/aws/aws-cdk/pull/16751

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../lib/cluster-resource-handler/common.ts    | 26 ++++-----------
 .../aws-cdk/lib/api/aws-auth/sdk-provider.ts  | 33 ++++++-------------
 2 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts b/packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts
index 1adb2eb328564..8f563de833bf6 100644
--- a/packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts
+++ b/packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts
@@ -38,19 +38,15 @@ export abstract class ResourceHandler {
       RoleArn: roleToAssume,
       RoleSessionName: `AWSCDK.EKSCluster.${this.requestType}.${this.requestId}`,
     });
-
-    const proxyAddress = this.httpProxyFromEnvironment();
-    if (proxyAddress) {
-      this.log(`Using proxy server: ${proxyAddress}`);
-      // eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies
-      const ProxyAgent: any = require('proxy-agent');
-      aws.config.update({
-        httpOptions: { agent: new ProxyAgent(proxyAddress) },
-      });
-    }
   }
 
   public onEvent() {
+    // eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-extraneous-dependencies
+    const ProxyAgent: any = require('proxy-agent');
+    aws.config.update({
+      httpOptions: { agent: new ProxyAgent() },
+    });
+
     switch (this.requestType) {
       case 'Create': return this.onCreate();
       case 'Update': return this.onUpdate();
@@ -75,16 +71,6 @@ export abstract class ResourceHandler {
     console.log(JSON.stringify(x, undefined, 2));
   }
 
-  private httpProxyFromEnvironment(): string | undefined {
-    if (process.env.http_proxy) {
-      return process.env.http_proxy;
-    }
-    if (process.env.HTTP_PROXY) {
-      return process.env.HTTP_PROXY;
-    }
-    return undefined;
-  }
-
   protected abstract async onCreate(): Promise<OnEventResponse>;
   protected abstract async onDelete(): Promise<OnEventResponse | void>;
   protected abstract async onUpdate(): Promise<(OnEventResponse & EksUpdateId) | void>;
diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts b/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
index d06fba8a59529..4621d171bc357 100644
--- a/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
+++ b/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
@@ -374,48 +374,35 @@ function parseHttpOptions(options: SdkHttpOptions) {
   }
   config.customUserAgent = userAgent;
 
-  const proxyAddress = options.proxyAddress || httpsProxyFromEnvironment();
   const caBundlePath = options.caBundlePath || caBundlePathFromEnvironment();
 
-  if (proxyAddress && caBundlePath) {
-    throw new Error(`At the moment, cannot specify Proxy (${proxyAddress}) and CA Bundle (${caBundlePath}) at the same time. See https://github.com/aws/aws-cdk/issues/5804`);
+  if (options.proxyAddress && caBundlePath) {
+    throw new Error(`At the moment, cannot specify Proxy (${options.proxyAddress}) and CA Bundle (${caBundlePath}) at the same time. See https://github.com/aws/aws-cdk/issues/5804`);
     // Maybe it's possible after all, but I've been staring at
     // https://github.com/TooTallNate/node-proxy-agent/blob/master/index.js#L79
     // a while now trying to figure out what to pass in so that the underlying Agent
     // object will get the 'ca' argument. It's not trivial and I don't want to risk it.
   }
 
-  if (proxyAddress) { // Ignore empty string on purpose
-    // https://aws.amazon.com/blogs/developer/using-the-aws-sdk-for-javascript-from-behind-a-proxy/
-    debug('Using proxy server: %s', proxyAddress);
-    // eslint-disable-next-line @typescript-eslint/no-require-imports
-    const ProxyAgent: any = require('proxy-agent');
-    config.httpOptions.agent = new ProxyAgent(proxyAddress);
-  }
   if (caBundlePath) {
     debug('Using CA bundle path: %s', caBundlePath);
     config.httpOptions.agent = new https.Agent({
       ca: readIfPossible(caBundlePath),
       keepAlive: true,
     });
+  } else {
+    // Configure the proxy agent. By default, this will use HTTPS?_PROXY and
+    // NO_PROXY environment variables to determine which proxy to use for each
+    // request.
+    //
+    // eslint-disable-next-line @typescript-eslint/no-require-imports
+    const ProxyAgent: any = require('proxy-agent');
+    config.httpOptions.agent = new ProxyAgent();
   }
 
   return config;
 }
 
-/**
- * Find and return the configured HTTPS proxy address
- */
-function httpsProxyFromEnvironment(): string | undefined {
-  if (process.env.https_proxy) {
-    return process.env.https_proxy;
-  }
-  if (process.env.HTTPS_PROXY) {
-    return process.env.HTTPS_PROXY;
-  }
-  return undefined;
-}
-
 /**
  * Find and return a CA certificate bundle path to be passed into the SDK.
  */