fix: env param fail-fast checks don't fatal on missing appId or envName (aws-amplify#12373)

Author: edwardfoyle

packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/utils/environmentVariablesHelper.test.ts

@@ -82,7 +82,7 @@ describe('ensureEnvironmentVariableValues', () => {
 
     prompterMock.input.mockResolvedValueOnce('testVal2').mockResolvedValueOnce('testVal3');
 
-    await envVarHelper.ensureEnvironmentVariableValues({ usageData: { emitError: jest.fn() } } as unknown as $TSContext);
+    await envVarHelper.ensureEnvironmentVariableValues({ usageData: { emitError: jest.fn() } } as unknown as $TSContext, 'testAppId');
     expect(getEnvParamManager().getResourceParamManager('function', 'testFunc').getAllParams()).toEqual({
       envVarOne: 'testVal1',
       envVarTwo: 'testVal2',

packages/amplify-category-function/src/events/prePushHandler.ts

@@ -1,3 +1,4 @@
+import { printer } from '@aws-amplify/amplify-prompts';
 import { $TSContext, stateManager } from 'amplify-cli-core';
 import { categoryName } from '../constants';
 import {
@@ -11,8 +12,25 @@ import { ensureEnvironmentVariableValues } from '../provider-utils/awscloudforma
  * prePush Handler event for function category
  */
 export const prePushHandler = async (context: $TSContext): Promise<void> => {
-  await ensureEnvironmentVariableValues(context);
-  await ensureFunctionSecrets(context);
+  // if appId and envName can be resolved, proceed with checking env vars and secrets
+  const envName = stateManager.getCurrentEnvName() || context?.exeInfo?.inputParams?.amplify?.envName;
+  // get appId from amplify-meta or fallback to input params
+  const appId: string | undefined =
+    (stateManager.getMeta(undefined, { throwIfNotExist: false }) || {})?.providers?.awscloudformation?.AmplifyAppId ||
+    context?.exeInfo?.inputParams?.amplify?.appId;
+
+  // this handler is executed during `init --forcePush` which does an init, then a pull, then a push all in one
+  // These parameters should always be present but it is possible they are not on init.
+  // Hence this check will skip these checks if we can't resolve the prerequisite information
+  if (envName && appId) {
+    await ensureEnvironmentVariableValues(context, appId);
+    await ensureFunctionSecrets(context);
+  } else {
+    printer.warn(
+      'Could not resolve either appId, environment name or both. Skipping environment check for function secrets and environment variables',
+    );
+  }
+
   await ensureLambdaExecutionRoleOutputs();
 };
 

packages/amplify-category-function/src/provider-utils/awscloudformation/secrets/functionSecretsStateManager.ts

@@ -1,4 +1,4 @@
-import { $TSContext, AmplifyError, JSONUtilities, pathManager, ResourceName } from 'amplify-cli-core';
+import { $TSContext, AmplifyError, JSONUtilities, pathManager, ResourceName, stateManager } from 'amplify-cli-core';
 import { removeSecret, retainSecret, SecretDeltas, SecretName, setSecret } from '@aws-amplify/amplify-function-plugin-interface';
 import * as path from 'path';
 import * as fs from 'fs-extra';
@@ -105,13 +105,15 @@ export class FunctionSecretsStateManager {
       return;
     }
     if (!this.isInteractive()) {
+      const inputEnvName = this.context?.exeInfo?.inputParams?.amplify?.envName;
+      const inputAppId = this.context?.exeInfo?.inputParams?.amplify?.appId;
       const resolution =
         `Run 'amplify push' interactively to specify values.\n` +
-        `Alternatively, manually add values in SSM ParameterStore for the following parameter names:\n\n` +
-        `${addedSecrets.map((secretName) => getFullyQualifiedSecretName(secretName, functionName)).join('\n')}\n`;
+        `Alternatively, manually add SecureString values in SSM Parameter Store for the following parameter names:\n\n` +
+        `${addedSecrets.map((secretName) => getFullyQualifiedSecretName(secretName, functionName, inputEnvName, inputAppId)).join('\n')}\n`;
       throw new AmplifyError('EnvironmentConfigurationError', {
         message: `Function ${functionName} is missing secret values in this environment.`,
-        details: `[${addedSecrets}] ${addedSecrets.length > 1 ? 'does' : 'do'} not have values.`,
+        details: `[${addedSecrets}] ${addedSecrets.length > 1 ? 'do' : 'does'} not have values.`,
         resolution,
         link: 'https://docs.amplify.aws/cli/reference/ssm-parameter-store/#manually-creating-parameters',
       });
@@ -184,7 +186,10 @@ export class FunctionSecretsStateManager {
    * @returns string[] of all secret names for the function
    */
   private getCloudFunctionSecretNames = async (functionName: string, envName?: string): Promise<string[]> => {
-    const prefix = getFunctionSecretPrefix(functionName, envName);
+    if (envName === undefined) {
+      envName = stateManager.getCurrentEnvName() || this.context?.exeInfo?.inputParams?.amplify?.envName;
+    }
+    const prefix = getFunctionSecretPrefix(functionName, envName, this.context?.exeInfo?.inputParams?.amplify?.appId);
     const parts = path.parse(prefix);
     const unfilteredSecrets = await this.ssmClientWrapper.getSecretNamesByPath(parts.dir);
     return unfilteredSecrets.filter((secretName) => secretName.startsWith(prefix)).map((secretName) => secretName.slice(prefix.length));

packages/amplify-category-function/src/provider-utils/awscloudformation/secrets/secretName.ts

@@ -16,27 +16,27 @@ export const secretsPathAmplifyAppIdKey = 'secretsPathAmplifyAppId';
  *
  * If envName is not specified, the current env is assumed
  */
-export const getFullyQualifiedSecretName = (secretName: string, functionName: string, envName?: string) =>
-  `${getFunctionSecretPrefix(functionName, envName)}${secretName}`;
+export const getFullyQualifiedSecretName = (secretName: string, functionName: string, envName?: string, appId?: string) =>
+  `${getFunctionSecretPrefix(functionName, envName, appId)}${secretName}`;
 
 /**
  * Returns the SSM parameter name prefix for all secrets for the given function in the given env
  *
  * If envName is not specified, the current env is assumed
  */
-export const getFunctionSecretPrefix = (functionName: string, envName?: string) =>
-  path.posix.join(getEnvSecretPrefix(envName), `AMPLIFY_${functionName}_`);
+export const getFunctionSecretPrefix = (functionName: string, envName?: string, appId?: string) =>
+  path.posix.join(getEnvSecretPrefix(envName, appId), `AMPLIFY_${functionName}_`);
 
 /**
  * Returns the SSM parameter name prefix for all secrets in the given env.
  *
  * If envName is not specified, the current env is assumed
  */
-export const getEnvSecretPrefix = (envName: string = stateManager.getLocalEnvInfo()?.envName) => {
+export const getEnvSecretPrefix = (envName: string = stateManager.getCurrentEnvName(), appId: string = getAppId()) => {
   if (!envName) {
     throw new Error('Could not determine the current Amplify environment name. Try running `amplify env checkout`.');
   }
-  return path.posix.join('/amplify', getAppId(), envName);
+  return path.posix.join('/amplify', appId, envName);
 };
 
 // NOTE: Even though the following 2 functions are CFN specific, I'm putting them here to colocate all of the secret naming logic

packages/amplify-category-function/src/provider-utils/awscloudformation/utils/environmentVariablesHelper.ts

@@ -1,8 +1,8 @@
 import path from 'path';
 import _ from 'lodash';
 import * as uuid from 'uuid';
-import { $TSContext, stateManager, pathManager, JSONUtilities, exitOnNextTick, $TSAny, $TSObject } from 'amplify-cli-core';
-import { byValue, formatter, maxLength, printer, prompter } from '@aws-amplify/amplify-prompts';
+import { $TSContext, stateManager, pathManager, JSONUtilities, $TSAny, $TSObject, AmplifyError } from 'amplify-cli-core';
+import { byValue, maxLength, printer, prompter } from '@aws-amplify/amplify-prompts';
 import { getEnvParamManager, ensureEnvParamManager } from '@aws-amplify/amplify-environment-parameters';
 import { functionParametersFileName } from './constants';
 import { categoryName } from '../../../constants';
@@ -168,7 +168,7 @@ const askForEnvironmentVariableValue = async (
 /**
  * Ensure that values are provided for all env vars in the current environment
  */
-export const ensureEnvironmentVariableValues = async (context: $TSContext): Promise<void> => {
+export const ensureEnvironmentVariableValues = async (context: $TSContext, appId: string): Promise<void> => {
   const yesFlagSet = context?.exeInfo?.inputParams?.yes || context?.input?.options?.yes;
   const currentEnvName = stateManager.localEnvInfoExists()
     ? stateManager.getLocalEnvInfo()?.envName
@@ -198,16 +198,7 @@ export const ensureEnvironmentVariableValues = async (context: $TSContext): Prom
   // there are some missing env vars
 
   if (yesFlagSet) {
-    // in this case, we can't prompt for missing values, so fail gracefully
-    const errMessage = `Cannot push Amplify environment "${currentEnvName}" due to missing Lambda function environment variable values. Rerun 'amplify push' without '--yes' to fix.`;
-    printer.error(errMessage);
-    const missingEnvVarsMessage = functionConfigMissingEnvVars.map(({ missingEnvVars, funcName }) => {
-      const missingEnvVarsString = missingEnvVars.map((missing) => missing.environmentVariableName).join(', ');
-      return `Function ${funcName} is missing values for environment variables: ${missingEnvVarsString}`;
-    });
-    formatter.list(missingEnvVarsMessage);
-    await context.usageData.emitError(new Error(errMessage));
-    exitOnNextTick(1);
+    throw createMissingEnvVarsError(functionConfigMissingEnvVars, appId, currentEnvName);
   }
 
   printer.info('Some Lambda function environment variables are missing values in this Amplify environment.');
@@ -322,3 +313,62 @@ const getStoredKeyValue = (resourceName: string, envName?: string): Record<strin
 const setStoredKeyValue = (resourceName: string, newKeyValue: $TSAny, envName?: string): void => {
   getEnvParamManager(envName).getResourceParamManager(categoryName, resourceName).setAllParams(newKeyValue);
 };
+
+type MissingEnvVarsConfig = {
+  funcName: string;
+  missingEnvVars: {
+    environmentVariableName: string;
+    cloudFormationParameterName: string;
+  }[];
+}[];
+
+const createMissingEnvVarsError = (
+  missingVars: MissingEnvVarsConfig,
+  appId: string | undefined,
+  envName: string | undefined,
+): AmplifyError => {
+  const message = `This environment is missing some function environment variable values.`;
+  const missingEnvVarsDetails = missingVars
+    .map(({ missingEnvVars, funcName }) => {
+      const missingEnvVarsString = missingEnvVars.map((missing) => missing.environmentVariableName).join(', ');
+      return `Function ${funcName} is missing values for environment variables: ${missingEnvVarsString}`;
+    })
+    .join('\n');
+  if (appId === undefined) {
+    return new AmplifyError('EnvironmentConfigurationError', {
+      message: `${message} An AppId could not be determined for fetching missing parameters.`,
+      details: missingEnvVarsDetails,
+      resolution: `Make sure your project is initialized and rerun 'amplify push' without '--yes' to fix.`,
+    });
+  }
+
+  if (envName === undefined) {
+    return new AmplifyError('EnvironmentConfigurationError', {
+      message: `${message} A current environment name could not be determined for fetching missing parameters.`,
+      details: missingEnvVarsDetails,
+      resolution: `Make sure your project is initialized using "amplify init"`,
+    });
+  }
+
+  // appId and envName are specified so we can provide a specific error message
+
+  const missingFullPaths = missingVars
+    .map(({ missingEnvVars, funcName }) =>
+      missingEnvVars.map((missing) => getParamKey(appId, envName, funcName, missing.cloudFormationParameterName)),
+    )
+    .flat();
+
+  const resolution =
+    `Run 'amplify push' interactively to specify values.\n` +
+    `Alternatively, manually add values in SSM ParameterStore for the following parameter names:\n\n` +
+    `${missingFullPaths.join('\n')}\n`;
+  return new AmplifyError('EnvironmentConfigurationError', {
+    message,
+    details: missingEnvVarsDetails,
+    resolution,
+    link: 'https://docs.amplify.aws/cli/reference/ssm-parameter-store/#manually-creating-parameters',
+  });
+};
+
+const getParamKey = (appId: string, envName: string, funcName: string, paramName: string) =>
+  `/amplify/${appId}/${envName}/AMPLIFY_function_${funcName}_${paramName}`;

packages/amplify-cli/src/__tests__/commands/env.test.ts

@@ -1,15 +1,15 @@
-describe('amplify env: ', () => {
-  const mockExit = jest.fn();
-  jest.mock('amplify-cli-core', () => ({
-    exitOnNextTick: mockExit,
-    pathManager: { getAmplifyMetaFilePath: jest.fn().mockReturnValue('test_file_does_not_exist') },
-    constants: jest.requireActual('amplify-cli-core').constants,
-  }));
-  const { run: runEnvCmd } = require('../../commands/env');
-  const { run: runAddEnvCmd } = require('../../commands/env/add');
-  const envList = require('../../commands/env/list');
-  jest.mock('../../commands/env/list');
+import { $TSContext, AmplifyError } from 'amplify-cli-core';
+jest.mock('../../commands/init');
+jest.mock('amplify-cli-core');
+import { run as runEnvCmd } from '../../commands/env';
+import { run as runAddEnvCmd } from '../../commands/env/add';
+import * as envList from '../../commands/env/list';
+
+const AmplifyErrorMock = AmplifyError as jest.MockedClass<typeof AmplifyError>;
+
+AmplifyErrorMock.mockImplementation(() => new Error('test error') as AmplifyError);
 
+describe('amplify env: ', () => {
   it('env run method should exist', () => {
     expect(runEnvCmd).toBeDefined();
   });
@@ -18,24 +18,28 @@ describe('amplify env: ', () => {
     expect(runAddEnvCmd).toBeDefined();
   });
 
-  it('env add method should throw if meta file does not exist', () => {
-    expect(async () => await runAddEnvCmd()).rejects.toThrow();
+  it('env add method should throw if meta file does not exist', async () => {
+    await expect(runAddEnvCmd({} as $TSContext)).rejects.toThrow();
   });
 
   it('env ls is an alias for env list', async () => {
     const mockEnvListRun = jest.spyOn(envList, 'run');
-    await runEnvCmd({
-      input: {
-        subCommands: ['list'],
+    const mockContext = {
+      print: {
+        table: jest.fn(),
+      },
+      amplify: {
+        getAllEnvs: jest.fn().mockReturnValue(['testa', 'testb']),
+        getEnvInfo: jest.fn().mockReturnValue({ envName: 'testa' }),
       },
-      parameters: {},
-    });
-    await runEnvCmd({
       input: {
-        subCommands: ['ls'],
+        subCommands: ['list'],
       },
       parameters: {},
-    });
+    };
+    await runEnvCmd(mockContext);
+    mockContext.input.subCommands = ['ls'];
+    await runEnvCmd(mockContext);
     expect(mockEnvListRun).toHaveBeenCalledTimes(2);
   });
 

packages/amplify-cli/src/__tests__/utils/verify-expected-env-params.test.ts

@@ -7,6 +7,8 @@ import { $TSContext } from 'amplify-cli-core';
 jest.mock('../../commands/build');
 jest.mock('@aws-amplify/amplify-prompts');
 jest.mock('@aws-amplify/amplify-environment-parameters');
+jest.mock('@aws-amplify/amplify-provider-awscloudformation');
+jest.mock('amplify-cli-core');
 
 const getResourcesMock = getChangedResources as jest.MockedFunction<typeof getChangedResources>;
 const ensureEnvParamManagerMock = ensureEnvParamManager as jest.MockedFunction<typeof ensureEnvParamManager>;
@@ -62,7 +64,7 @@ describe('verifyExpectedEnvParams', () => {
   });
   it('filters parameters based on category and resourceName if specified', async () => {
     await verifyExpectedEnvParams(contextStub, 'storage');
-    expect(verifyExpectedEnvParametersMock).toHaveBeenCalledWith([resourceList[0]]);
+    expect(verifyExpectedEnvParametersMock).toHaveBeenCalledWith([resourceList[0]], undefined, undefined);
   });
 
   it('calls verify expected parameters if in non-interactive mode', async () => {

packages/amplify-cli/src/commands/env/list.ts

@@ -8,7 +8,7 @@ import { printEnvInfo } from '../helpers/envUtils';
 export const run = async (context): Promise<void> => {
   const { envName } = context.amplify.getEnvInfo();
 
-  if (context.parameters.options.details) {
+  if (context?.parameters?.options?.details) {
     const allEnvs = context.amplify.getEnvDetails();
     if (context.parameters.options.json) {
       printer.info(JSONUtilities.stringify(allEnvs) as string);
@@ -25,7 +25,7 @@ export const run = async (context): Promise<void> => {
     });
   } else {
     const allEnvs = context.amplify.getAllEnvs();
-    if (context.parameters.options.json) {
+    if (context?.parameters?.options?.json) {
       printer.info(JSONUtilities.stringify({ envs: allEnvs }) as string);
       return;
     }

packages/amplify-cli/src/utils/verify-expected-env-params.ts

@@ -2,6 +2,7 @@ import { $TSContext, IAmplifyResource, stateManager, constants } from 'amplify-c
 import { ensureEnvParamManager, IEnvironmentParameterManager, ServiceDownloadHandler } from '@aws-amplify/amplify-environment-parameters';
 import { printer, prompter } from '@aws-amplify/amplify-prompts';
 import { getChangedResources, getAllResources } from '../commands/build';
+import { resolveAppId } from '@aws-amplify/amplify-provider-awscloudformation';
 
 export const verifyExpectedEnvParams = async (context: $TSContext, category?: string, resourceName?: string) => {
   const envParamManager = stateManager.localEnvInfoExists()
@@ -27,7 +28,16 @@ export const verifyExpectedEnvParams = async (context: $TSContext, category?: st
   });
 
   if (context?.exeInfo?.inputParams?.yes || context?.exeInfo?.inputParams?.headless) {
-    await envParamManager.verifyExpectedEnvParameters(parametersToCheck);
+    let appId: string | undefined = undefined;
+    try {
+      appId = resolveAppId(context);
+    } catch {
+      // If AppId can't be resolved, this only affects the error message of verifyExpectedEnvParameters in the case that parameters are
+      // actually missing. So we let appId be undefined here and verifyExpectedEnvParameters will print a different error message based
+      // on the information available to it
+    }
+    const envName = stateManager.getCurrentEnvName() || context?.exeInfo?.inputParams?.amplify?.envName;
+    await envParamManager.verifyExpectedEnvParameters(parametersToCheck, appId, envName);
   } else {
     const missingParameters = await envParamManager.getMissingParameters(parametersToCheck);
     if (missingParameters.length > 0) {

packages/amplify-e2e-core/src/init/amplifyPush.ts

@@ -335,7 +335,7 @@ export const amplifyPushIterativeRollback = (cwd: string, testingWithLatestCodeb
  */
 export const amplifyPushMissingEnvVar = (cwd: string, newEnvVarValue: string) =>
   spawn(getCLIPath(), ['push'], { cwd, stripColors: true })
-    .wait('Enter the missing environment variable value of')
+    .wait('Enter a value for')
     .sendLine(newEnvVarValue)
     .wait('Are you sure you want to continue?')
     .sendYes()