fix: normalize AWS environment variables between CDK and SDK (#1215)

* fix: normalize AWS environment variables between CDK and SDK * Rewrite comment for spellcheck bug * Adds warning when both expected and legacy env vars are set * Cleans up warning messages to handle explicit cases * Injects printer so that display logic can be properly tested * Update packages/cli/src/command_middleware.ts Co-authored-by: Kamil Sobol <[email protected]> --------- Co-authored-by: ⛵️ Sean Quinn <[email protected]> Co-authored-by: Kamil Sobol <[email protected]>
aws-amplify · Apr 5, 2024 · ffb358e · ffb358e
1 parent 4123204
commit ffb358e
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 7 deletions.
diff --git a/.changeset/modern-fans-return.md b/.changeset/modern-fans-return.md
@@ -0,0 +1,5 @@
+---
+'@aws-amplify/backend-cli': patch
+---
+
+Added normalization logic for AWS environment variables
diff --git a/packages/cli/src/command_middleware.test.ts b/packages/cli/src/command_middleware.test.ts
@@ -1,11 +1,12 @@
 import assert from 'node:assert';
-import { after, before, beforeEach, describe, it } from 'node:test';
+import { after, before, beforeEach, describe, it, mock } from 'node:test';
 import { CommandMiddleware } from './command_middleware.js';
 import { EOL } from 'node:os';
 import { DEFAULT_PROFILE } from '@smithy/shared-ini-file-loader';
 import fs from 'fs/promises';
 import path from 'path';
 import { ArgumentsCamelCase } from 'yargs';
+import { Printer } from '@aws-amplify/cli-core';
 
 const restoreEnv = (restoreVal: string | undefined, envVar: string) => {
   if (restoreVal) {
@@ -17,7 +18,10 @@ const restoreEnv = (restoreVal: string | undefined, envVar: string) => {
 
 void describe('commandMiddleware', () => {
   void describe('ensureAwsCredentialAndRegion', () => {
-    const commandMiddleware = new CommandMiddleware();
+    const printerMock = { log: mock.fn() };
+    const commandMiddleware = new CommandMiddleware(
+      printerMock as unknown as Printer
+    );
     const testAccessKeyId = '124';
     const testSecretAccessKey = '667';
     const testProfile = 'profileA';
@@ -28,11 +32,13 @@ void describe('commandMiddleware', () => {
     let configFilePath: string;
 
     const currentProfile = process.env.AWS_PROFILE;
+    const currentDefaultProfile = process.env.AWS_DEFAULT_PROFILE;
     const currentConfigFile = process.env.AWS_CONFIG_FILE;
     const currentCredentialFile = process.env.AWS_SHARED_CREDENTIALS_FILE;
     const currentAccessKeyId = process.env.AWS_ACCESS_KEY_ID;
     const currentSecretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
     const currentRegion = process.env.AWS_REGION;
+    const currentDefaultRegion = process.env.AWS_DEFAULT_REGION;
 
     before(async () => {
       testDir = await fs.mkdtemp('profile_middleware_test');
@@ -45,11 +51,13 @@ void describe('commandMiddleware', () => {
     after(async () => {
       await fs.rm(testDir, { recursive: true, force: true });
       restoreEnv(currentProfile, 'AWS_PROFILE');
+      restoreEnv(currentDefaultProfile, 'AWS_DEFAULT_PROFILE');
       restoreEnv(currentConfigFile, 'AWS_CONFIG_FILE');
       restoreEnv(currentCredentialFile, 'AWS_SHARED_CREDENTIALS_FILE');
       restoreEnv(currentAccessKeyId, 'AWS_ACCESS_KEY_ID');
       restoreEnv(currentSecretAccessKey, 'AWS_SECRET_ACCESS_KEY');
       restoreEnv(currentRegion, 'AWS_REGION');
+      restoreEnv(currentDefaultRegion, 'AWS_DEFAULT_REGION');
     });
 
     void describe('from environment variables', () => {
@@ -58,6 +66,7 @@ void describe('commandMiddleware', () => {
         process.env.AWS_SECRET_ACCESS_KEY = testSecretAccessKey;
         process.env.AWS_REGION = testRegion;
         delete process.env.AWS_PROFILE;
+        printerMock.log.mock.resetCalls();
       });
 
       void it('loads credentials', async () => {
@@ -68,8 +77,38 @@ void describe('commandMiddleware', () => {
         );
       });
 
-      void it('throws error if absent region environment variable', async () => {
+      void it('maps AWS_DEFAULT_REGION to AWS_REGION', async () => {
         delete process.env.AWS_REGION;
+        process.env.AWS_DEFAULT_REGION = testRegion;
+        await assert.doesNotReject(() =>
+          commandMiddleware.ensureAwsCredentialAndRegion(
+            {} as ArgumentsCamelCase<{ profile: string | undefined }>
+          )
+        );
+        assert.equal(process.env.AWS_REGION, testRegion);
+        assert.match(
+          printerMock.log.mock.calls[0].arguments[0],
+          /Legacy environment variable/
+        );
+      });
+
+      void it('prefers AWS_REGION to AWS_DEFAULT_REGION', async () => {
+        process.env.AWS_DEFAULT_REGION = 'testDefaultRegion';
+        await assert.doesNotReject(() =>
+          commandMiddleware.ensureAwsCredentialAndRegion(
+            {} as ArgumentsCamelCase<{ profile: string | undefined }>
+          )
+        );
+        assert.equal(process.env.AWS_REGION, testRegion);
+        assert.match(
+          printerMock.log.mock.calls[0].arguments[0],
+          /Using 'AWS_REGION'/
+        );
+      });
+
+      void it('throws error if absent region environment variables', async () => {
+        delete process.env.AWS_REGION;
+        delete process.env.AWS_DEFAULT_REGION;
         try {
           await commandMiddleware.ensureAwsCredentialAndRegion(
             {} as ArgumentsCamelCase<{ profile: string | undefined }>
@@ -110,6 +149,7 @@ void describe('commandMiddleware', () => {
         await fs.writeFile(credFilePath, '', 'utf-8');
         await fs.writeFile(configFilePath, '', 'utf-8');
         delete process.env.AWS_PROFILE;
+        printerMock.log.mock.resetCalls();
       });
 
       const writeProfileCredential = async (
@@ -208,6 +248,41 @@ void describe('commandMiddleware', () => {
           } as ArgumentsCamelCase<{ profile: string | undefined }>)
         );
       });
+
+      void it('maps AWS_DEFAULT_PROFILE to AWS_PROFILE', async () => {
+        process.env.AWS_DEFAULT_PROFILE = testProfile;
+        await writeProfileRegion(testProfile);
+        await writeProfileCredential(testProfile);
+
+        await assert.doesNotReject(() =>
+          commandMiddleware.ensureAwsCredentialAndRegion(
+            {} as ArgumentsCamelCase<{ profile: string | undefined }>
+          )
+        );
+        assert.equal(process.env.AWS_PROFILE, testProfile);
+        assert.match(
+          printerMock.log.mock.calls[0].arguments[0],
+          /Legacy environment variable/
+        );
+      });
+
+      void it('prefers AWS_PROFILE over AWS_DEFAULT_PROFILE', async () => {
+        process.env.AWS_DEFAULT_PROFILE = 'testDefaultProfile';
+        process.env.AWS_PROFILE = testProfile;
+        await writeProfileRegion(testProfile);
+        await writeProfileCredential(testProfile);
+
+        await assert.doesNotReject(() =>
+          commandMiddleware.ensureAwsCredentialAndRegion(
+            {} as ArgumentsCamelCase<{ profile: string | undefined }>
+          )
+        );
+        assert.equal(process.env.AWS_PROFILE, testProfile);
+        assert.match(
+          printerMock.log.mock.calls[0].arguments[0],
+          /Using 'AWS_PROFILE'/
+        );
+      });
     });
   });
 });
diff --git a/packages/cli/src/command_middleware.ts b/packages/cli/src/command_middleware.ts
@@ -3,13 +3,19 @@ import { fromNodeProviderChain } from '@aws-sdk/credential-providers';
 import { loadConfig } from '@smithy/node-config-provider';
 import { NODE_REGION_CONFIG_OPTIONS } from '@aws-sdk/region-config-resolver';
 import { AmplifyUserError } from '@aws-amplify/platform-core';
+import { Printer } from '@aws-amplify/cli-core';
 
 export const profileSetupInstruction = `To configure a new Amplify profile, use "npx amplify configure profile".`;
 
 /**
  * Contains middleware functions.
  */
 export class CommandMiddleware {
+  /**
+   * Creates command middleware.
+   */
+  constructor(private readonly printer: Printer) {}
+
   /**
    * Ensure AWS credentials and region of the input profile (or 'default' if undefined) are available in the provider chain.
    * If the input profile is defined, the environment variable AWS_PROFILE will be set accordingly.
@@ -19,6 +25,15 @@ export class CommandMiddleware {
   >(
     argv: ArgumentsCamelCase<T>
   ) => {
+    /**
+     * The AWS CDK respects older CLI v1 variable names that are no longer supported in the
+     * latest AWS SDK. Developers that use the older variables and switch between Amplify
+     * and CDK tools will experience region mismatch failures when using Amplify tools. Variable
+     * names known to cause such failures are mapped here for a better developer experience.
+     */
+    this.mapEnvironmentVariables('AWS_DEFAULT_REGION', 'AWS_REGION');
+    this.mapEnvironmentVariables('AWS_DEFAULT_PROFILE', 'AWS_PROFILE');
+
     if (argv.profile) {
       process.env.AWS_PROFILE = argv.profile;
     }
@@ -61,4 +76,26 @@ export class CommandMiddleware {
       );
     }
   };
+
+  /**
+   * Maps one environment variable name to the other
+   */
+  private mapEnvironmentVariables(
+    legacyName: string,
+    preferredName: string
+  ): void {
+    if (!process.env[legacyName]) {
+      return;
+    }
+    if (process.env[preferredName]) {
+      this.printer.log(
+        `Both the legacy '${legacyName}' and preferred '${preferredName}' environment variables detected. Using '${preferredName}'`
+      );
+      return;
+    }
+    this.printer.log(
+      `Legacy environment variable '${legacyName}' detected. Mapping to '${preferredName}'`
+    );
+    process.env[preferredName] = process.env[legacyName];
+  }
 }
diff --git a/packages/cli/src/commands/generate/generate_command_factory.ts b/packages/cli/src/commands/generate/generate_command_factory.ts
@@ -17,6 +17,7 @@ import { AppBackendIdentifierResolver } from '../../backend-identifier/backend_i
 import { GenerateSchemaCommand } from './schema-from-database/generate_schema_command.js';
 import { getSecretClient } from '@aws-amplify/backend-secret';
 import { SchemaGenerator } from '@aws-amplify/schema-generator';
+import { printer } from '@aws-amplify/cli-core';
 
 /**
  * Creates wired generate command.
@@ -63,7 +64,7 @@ export const createGenerateCommand = (): CommandModule => {
     new SchemaGenerator()
   );
 
-  const commandMiddleware = new CommandMiddleware();
+  const commandMiddleware = new CommandMiddleware(printer);
 
   return new GenerateCommand(
     generateConfigCommand,

diff --git a/packages/cli/src/commands/sandbox/sandbox-delete/sandbox_delete_command.test.ts b/packages/cli/src/commands/sandbox/sandbox-delete/sandbox_delete_command.test.ts
@@ -14,7 +14,7 @@ void describe('sandbox delete command', () => {
   let commandRunner: TestCommandRunner;
   let sandboxDeleteMock = mock.fn();
 
-  const commandMiddleware = new CommandMiddleware();
+  const commandMiddleware = new CommandMiddleware(printer);
   const mockHandleProfile = mock.method(
     commandMiddleware,
     'ensureAwsCredentialAndRegion',

diff --git a/packages/cli/src/commands/sandbox/sandbox_command.test.ts b/packages/cli/src/commands/sandbox/sandbox_command.test.ts
@@ -37,7 +37,7 @@ void describe('sandbox command', () => {
     generateClientConfigToFile: clientConfigGenerationMock,
   } as unknown as ClientConfigGeneratorAdapter;
 
-  const commandMiddleware = new CommandMiddleware();
+  const commandMiddleware = new CommandMiddleware(printer);
   const mockHandleProfile = mock.method(
     commandMiddleware,
     'ensureAwsCredentialAndRegion',

diff --git a/packages/cli/src/commands/sandbox/sandbox_command_factory.ts b/packages/cli/src/commands/sandbox/sandbox_command_factory.ts
@@ -49,7 +49,7 @@ export const createSandboxCommand = (): CommandModule<
     async () => await new UsageDataEmitterFactory().getInstance(libraryVersion)
   );
 
-  const commandMiddleWare = new CommandMiddleware();
+  const commandMiddleWare = new CommandMiddleware(printer);
   return new SandboxCommand(
     sandboxFactory,
     [new SandboxDeleteCommand(sandboxFactory), createSandboxSecretCommand()],