fix: fail init --forcePush fast if environment parameters or secret…

…s are missing in the environment (aws-amplify#12279)

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

@@ -1,9 +1,10 @@
-import { $TSContext, stateManager, pathManager } from 'amplify-cli-core';
+import { $TSContext, stateManager, pathManager, AmplifyError } from 'amplify-cli-core';
 import { mocked } from 'ts-jest/utils';
 import * as path from 'path';
 import { getEnvParamManager } from '@aws-amplify/amplify-environment-parameters';
 import { FunctionSecretsStateManager } from '../../../../provider-utils/awscloudformation/secrets/functionSecretsStateManager';
-import { getAppId } from '../../../../provider-utils/awscloudformation/secrets/secretName';
+import * as stateManagerModule from '../../../../provider-utils/awscloudformation/secrets/functionSecretsStateManager';
+import { getAppId, getFunctionSecretPrefix } from '../../../../provider-utils/awscloudformation/secrets/secretName';
 import { SSMClientWrapper } from '../../../../provider-utils/awscloudformation/secrets/ssmClientWrapper';
 
 jest.mock('amplify-cli-core');
@@ -17,6 +18,11 @@ const stateManagerMock = mocked(stateManager);
 const pathManagerMock = mocked(pathManager);
 const getAppIdMock = mocked(getAppId);
 const SSMClientWrapperMock = mocked(SSMClientWrapper);
+const getFunctionSecretPrefixMock = mocked(getFunctionSecretPrefix);
+const AmplifyErrorMock = AmplifyError as jest.MockedClass<typeof AmplifyError>;
+
+const amplifyErrorMockImpl = { name: 'TestError' } as unknown as AmplifyError;
+AmplifyErrorMock.mockImplementation(() => amplifyErrorMockImpl);
 
 stateManagerMock.getLocalEnvInfo.mockReturnValue({
   envName: 'testTest',
@@ -27,30 +33,56 @@ pathManagerMock.getBackendDirPath.mockReturnValue(path.join('test', 'path'));
 
 getAppIdMock.mockReturnValue('testAppId');
 
+const getSecretNamesByPathMock = jest.fn();
+
 SSMClientWrapperMock.getInstance.mockResolvedValue({
   deleteSecret: jest.fn(),
   setSecret: jest.fn(),
+  getSecretNamesByPath: getSecretNamesByPathMock,
 } as unknown as SSMClientWrapper);
 
-describe('syncSecretDeltas', () => {
-  const contextStub = {
-    parameters: {
-      command: 'update',
+const getLocalFunctionSecretNamesSpy = jest.spyOn(stateManagerModule, 'getLocalFunctionSecretNames');
+
+const contextStub = {
+  parameters: {
+    command: 'update',
+  },
+  input: {
+    options: {
+      yes: true,
     },
-  } as unknown as $TSContext;
-  beforeEach(jest.clearAllMocks);
+  },
+} as unknown as $TSContext;
+let functionSecretsStateManager: FunctionSecretsStateManager;
+
+beforeAll(async () => {
+  functionSecretsStateManager = await FunctionSecretsStateManager.getInstance(contextStub);
+});
 
+beforeEach(() => jest.clearAllMocks());
+
+describe('syncSecretDeltas', () => {
   it('sets Amplify AppID in team-provider-info if secrets are present', async () => {
-    const functionSecretsStateManager = await FunctionSecretsStateManager.getInstance(contextStub);
     await functionSecretsStateManager.syncSecretDeltas({ TEST_SECRET: { operation: 'retain' } }, 'testFuncName');
     expect(getEnvParamManager('testTest').getResourceParamManager('function', 'testFuncName').getAllParams()).toEqual({
       secretsPathAmplifyAppId: 'testAppId',
     });
   });
 
   it('removes Amplify AppId from team-provider-info if secrets are not present', async () => {
-    const functionSecretsStateManager = await FunctionSecretsStateManager.getInstance(contextStub);
     await functionSecretsStateManager.syncSecretDeltas({ TEST_SECRET: { operation: 'remove' } }, 'testFuncName');
     expect(getEnvParamManager('testTest').getResourceParamManager('function', 'testFuncName').getAllParams()).toEqual({});
   });
 });
+
+describe('ensureNewLocalSecretsSyncedToCloud', () => {
+  it('throws AmplifyError if secrets are missing and not in interactive mode', async () => {
+    const funcName = 'testFunc';
+    const secretPrefix = 'testPrefix';
+    getFunctionSecretPrefixMock.mockReturnValue(secretPrefix);
+    getLocalFunctionSecretNamesSpy.mockReturnValue(['secret1', 'secret2']);
+    getSecretNamesByPathMock.mockResolvedValue(['secret2'].map((sec) => `${secretPrefix}${sec}`));
+
+    await expect(functionSecretsStateManager.ensureNewLocalSecretsSyncedToCloud(funcName)).rejects.toStrictEqual(amplifyErrorMockImpl);
+  });
+});

packages/amplify-category-function/src/__tests__/provider-utils/awscloudformation/utils/ensure-lambda-arn-outputs.test.ts

@@ -10,13 +10,13 @@ const writeCFNTemplateMock = writeCFNTemplate as jest.MockedFunction<typeof writ
 describe('test ensureLambdaExecutionRoleOutputs function', () => {
   beforeEach(() => {
     pathManagerMock.getBackendDirPath = jest.fn().mockReturnValue('backend');
-    stateManagerMock.getMeta = jest.fn();
+    stateManagerMock.getBackendConfig = jest.fn();
   });
 
   afterEach(() => jest.resetAllMocks());
 
   it(' when no functions are present', async () => {
-    stateManagerMock.getMeta.mockReturnValue({
+    stateManagerMock.getBackendConfig.mockReturnValue({
       auth: {
         authtriggertestb3a9da62b3a9da62: {
           customAuth: false,
@@ -31,7 +31,7 @@ describe('test ensureLambdaExecutionRoleOutputs function', () => {
   });
 
   it(' when functions have no role arns in outputs', async () => {
-    stateManagerMock.getMeta.mockReturnValue({
+    stateManagerMock.getBackendConfig.mockReturnValue({
       auth: {
         authtriggertestb3a9da62b3a9da62: {
           customAuth: false,

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

@@ -1,8 +1,18 @@
+import { printer } from '@aws-amplify/amplify-prompts';
 import { $TSContext } from 'amplify-cli-core';
 import { FunctionSecretsStateManager } from '../provider-utils/awscloudformation/secrets/functionSecretsStateManager';
 
 export const postEnvRemoveHandler = async (context: $TSContext, envName: string) => {
-  await removeAllEnvSecrets(context, envName);
+  try {
+    await removeAllEnvSecrets(context, envName);
+  } catch (err) {
+    // catching this silently because that was the previous behavior
+    // previously this error was caught by the platform event firing logic and silently ignored
+    // now we are ignoring errors at the individual event handler
+    printer.debug(`function category postEnvRemoveHandler failed to run.`);
+    printer.debug(`You may need to manually clean up some function state.`);
+    printer.debug(err);
+  }
 };
 
 const removeAllEnvSecrets = async (context: $TSContext, envName: string) =>

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

@@ -17,8 +17,8 @@ export const prePushHandler = async (context: $TSContext): Promise<void> => {
 };
 
 const ensureFunctionSecrets = async (context: $TSContext): Promise<void> => {
-  const amplifyMeta = stateManager.getMeta();
-  const functionNames = Object.keys(amplifyMeta?.[categoryName]);
+  const backendConfig = stateManager.getBackendConfig();
+  const functionNames = Object.keys(backendConfig?.[categoryName] || {});
   for (const funcName of functionNames) {
     if (getLocalFunctionSecretNames(funcName).length > 0) {
       const funcSecretsManager = await FunctionSecretsStateManager.getInstance(context);

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

@@ -1,4 +1,4 @@
-import { $TSContext, JSONUtilities, pathManager, ResourceName } from 'amplify-cli-core';
+import { $TSContext, AmplifyError, JSONUtilities, pathManager, ResourceName } 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,9 +105,16 @@ export class FunctionSecretsStateManager {
       return;
     }
     if (!this.isInteractive()) {
-      throw new Error(
-        `The following secrets in ${functionName} do not have values: [${addedSecrets}]\nRun 'amplify push' interactively to specify values.`,
-      );
+      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`;
+      throw new AmplifyError('EnvironmentConfigurationError', {
+        message: `Function ${functionName} is missing secret values in this environment.`,
+        details: `[${addedSecrets}] ${addedSecrets.length > 1 ? 'does' : 'do'} not have values.`,
+        resolution,
+        link: 'https://docs.amplify.aws/cli/reference/ssm-parameter-store/#manually-creating-parameters',
+      });
     }
     const delta = await prePushMissingSecretsWalkthrough(functionName, addedSecrets);
     await this.syncSecretDeltas(delta, functionName);

packages/amplify-category-function/src/provider-utils/awscloudformation/utils/ensure-lambda-arn-outputs.ts

@@ -5,11 +5,11 @@ import * as path from 'path';
  * updates function cfn stack with lambda execution role arn parameter
  */
 export const ensureLambdaExecutionRoleOutputs = async (): Promise<void> => {
-  const amplifyMeta = stateManager.getMeta();
-  const functionNames = Object.keys(amplifyMeta?.[AmplifyCategories.FUNCTION] ?? []);
+  const backendConfig = stateManager.getBackendConfig();
+  const functionNames = Object.keys(backendConfig?.[AmplifyCategories.FUNCTION] ?? []);
   // filter lambda layer from lambdas in function
   const lambdaFunctionNames = functionNames.filter((functionName) => {
-    const functionObj = amplifyMeta?.[AmplifyCategories.FUNCTION]?.[functionName];
+    const functionObj = backendConfig?.[AmplifyCategories.FUNCTION]?.[functionName];
     return functionObj.service === AmplifySupportedService.LAMBDA;
   });
   for (const functionName of lambdaFunctionNames) {

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

@@ -170,17 +170,19 @@ const askForEnvironmentVariableValue = async (
  */
 export const ensureEnvironmentVariableValues = async (context: $TSContext): Promise<void> => {
   const yesFlagSet = context?.exeInfo?.inputParams?.yes || context?.input?.options?.yes;
-  const currentEnvName = stateManager.getLocalEnvInfo()?.envName;
+  const currentEnvName = stateManager.localEnvInfoExists()
+    ? stateManager.getLocalEnvInfo()?.envName
+    : context?.exeInfo?.inputParams?.amplify?.envName;
   await ensureEnvParamManager(currentEnvName);
-  const functionNames = Object.keys(stateManager.getBackendConfig()?.function);
+  const functionNames = Object.keys(stateManager.getBackendConfig()?.function || {});
   if (functionNames.length === 0) {
     return;
   }
 
   const functionConfigMissingEnvVars = functionNames
     .map((funcName) => {
       const storedList = getStoredList(funcName);
-      const keyValues = getStoredKeyValue(funcName);
+      const keyValues = getStoredKeyValue(funcName, currentEnvName);
       return {
         funcName,
         existingKeyValues: keyValues,
@@ -218,7 +220,7 @@ export const ensureEnvironmentVariableValues = async (context: $TSContext): Prom
       });
       keyValues[cfnName] = newValue;
     }
-    setStoredKeyValue(funcName, keyValues);
+    setStoredKeyValue(funcName, keyValues, currentEnvName);
   }
 };
 
@@ -317,6 +319,6 @@ const setStoredParameters = (resourceName: string, newParameters: $TSAny): void
 const getStoredKeyValue = (resourceName: string, envName?: string): Record<string, string> =>
   getEnvParamManager(envName).getResourceParamManager(categoryName, resourceName).getAllParams();
 
-const setStoredKeyValue = (resourceName: string, newKeyValue: $TSAny): void => {
-  getEnvParamManager().getResourceParamManager(categoryName, resourceName).setAllParams(newKeyValue);
+const setStoredKeyValue = (resourceName: string, newKeyValue: $TSAny, envName?: string): void => {
+  getEnvParamManager(envName).getResourceParamManager(categoryName, resourceName).setAllParams(newKeyValue);
 };

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

@@ -0,0 +1,88 @@
+import { getChangedResources } from '../../commands/build';
+import { prompter } from '@aws-amplify/amplify-prompts';
+import { ensureEnvParamManager, IEnvironmentParameterManager } from '@aws-amplify/amplify-environment-parameters';
+import { verifyExpectedEnvParams } from '../../utils/verify-expected-env-params';
+import { $TSContext } from 'amplify-cli-core';
+
+jest.mock('../../commands/build');
+jest.mock('@aws-amplify/amplify-prompts');
+jest.mock('@aws-amplify/amplify-environment-parameters');
+
+const getResourcesMock = getChangedResources as jest.MockedFunction<typeof getChangedResources>;
+const ensureEnvParamManagerMock = ensureEnvParamManager as jest.MockedFunction<typeof ensureEnvParamManager>;
+const prompterMock = prompter as jest.Mocked<typeof prompter>;
+
+const verifyExpectedEnvParametersMock = jest.fn();
+const getMissingParametersMock = jest.fn();
+const saveMock = jest.fn();
+
+ensureEnvParamManagerMock.mockResolvedValue({
+  instance: {
+    verifyExpectedEnvParameters: verifyExpectedEnvParametersMock,
+    getMissingParameters: getMissingParametersMock,
+    save: saveMock,
+    getResourceParamManager: jest.fn().mockReturnValue({
+      setParam: jest.fn(),
+    }),
+    downloadParameters: jest.fn(),
+  } as unknown as IEnvironmentParameterManager,
+});
+
+const resourceList = [
+  {
+    category: 'storage',
+    resourceName: 'testStorage',
+    service: 'S3',
+  },
+  {
+    category: 'function',
+    resourceName: 'testFunction',
+    service: 'Lambda',
+  },
+];
+
+getResourcesMock.mockResolvedValue(resourceList);
+
+const resetContext = {
+  exeInfo: {
+    inputParams: {
+      yes: true,
+    },
+  },
+  amplify: {
+    invokePluginMethod: jest.fn(),
+  },
+} as unknown as $TSContext;
+
+describe('verifyExpectedEnvParams', () => {
+  let contextStub = resetContext;
+  beforeEach(() => {
+    jest.clearAllMocks();
+    contextStub = resetContext;
+  });
+  it('filters parameters based on category and resourceName if specified', async () => {
+    await verifyExpectedEnvParams(contextStub, 'storage');
+    expect(verifyExpectedEnvParametersMock).toHaveBeenCalledWith([resourceList[0]]);
+  });
+
+  it('calls verify expected parameters if in non-interactive mode', async () => {
+    await verifyExpectedEnvParams(contextStub, 'storage');
+    expect(verifyExpectedEnvParametersMock).toHaveBeenCalled();
+    expect(getMissingParametersMock).not.toHaveBeenCalled();
+  });
+
+  it('prompts for missing parameters if in interactive mode', async () => {
+    contextStub.exeInfo.inputParams.yes = false;
+    getMissingParametersMock.mockResolvedValue([
+      {
+        categoryName: 'function',
+        resourceName: 'testFunction',
+        parameterName: 'something',
+      },
+    ]);
+    await verifyExpectedEnvParams(contextStub, 'storage');
+    expect(verifyExpectedEnvParametersMock).not.toHaveBeenCalled();
+    expect(prompterMock.input).toHaveBeenCalled();
+    expect(saveMock).toHaveBeenCalled();
+  });
+});

packages/amplify-cli/src/amplify-exception-handler.ts

@@ -18,6 +18,7 @@ export const init = (_context: Context): void => {
  * Handle exceptions
  */
 export const handleException = async (exception: unknown): Promise<void> => {
+  process.exitCode = 1;
   let amplifyException: AmplifyException;
 
   if (exception instanceof AmplifyException) {

packages/amplify-cli/src/commands/build.ts

@@ -14,7 +14,7 @@ export const run = async (context: $TSContext): Promise<void> => {
 
   try {
     await generateDependentResourcesType();
-    const resourcesToBuild: IAmplifyResource[] = await getResources(context);
+    const resourcesToBuild: IAmplifyResource[] = await getChangedResources(context);
     let filteredResources: IAmplifyResource[] = resourcesToBuild;
     if (categoryName) {
       filteredResources = filteredResources.filter((resource) => resource.category === categoryName);
@@ -44,7 +44,7 @@ export const run = async (context: $TSContext): Promise<void> => {
 /**
  * Returns resources in create or update state
  */
-export const getResources = async (context: $TSContext): Promise<IAmplifyResource[]> => {
+export const getChangedResources = async (context: $TSContext): Promise<IAmplifyResource[]> => {
   const resources: IAmplifyResource[] = [];
   const { resourcesToBeCreated, resourcesToBeUpdated } = await context.amplify.getResourceStatus();
   resourcesToBeCreated.forEach((resourceCreated: IAmplifyResource) => {
@@ -64,3 +64,16 @@ export const getResources = async (context: $TSContext): Promise<IAmplifyResourc
   });
   return resources;
 };
+
+export const getAllResources = async (context: $TSContext): Promise<IAmplifyResource[]> => {
+  const resources: IAmplifyResource[] = [];
+  const { allResources } = await context.amplify.getResourceStatus();
+  allResources.forEach((resource: IAmplifyResource) => {
+    resources.push({
+      service: resource.service,
+      category: resource.category,
+      resourceName: resource.resourceName,
+    });
+  });
+  return resources;
+};

packages/amplify-cli/src/commands/export.ts

@@ -10,7 +10,7 @@ import { printer } from '@aws-amplify/amplify-prompts';
 import chalk from 'chalk';
 import { getResourceOutputs } from '../extensions/amplify-helpers/get-resource-outputs';
 import Ora from 'ora';
-import { getResources } from './build';
+import { getChangedResources } from './build';
 import * as _ from 'lodash';
 
 export const run = async (context: $TSContext) => {
@@ -77,7 +77,7 @@ async function exportBackend(context: $TSContext, exportPath: string) {
 }
 
 async function buildAllResources(context: $TSContext) {
-  const resourcesToBuild: IAmplifyResource[] = await getResources(context);
+  const resourcesToBuild: IAmplifyResource[] = await getChangedResources(context);
   await context.amplify.executeProviderUtils(context, 'awscloudformation', 'buildOverrides', { resourcesToBuild, forceCompile: true });
 }