Skip to content

Commit 0b6e562

Browse files
edwardfoyleakshbhu
edwardfoyle
authored and
akshbhu
committed
fix: fail init --forcePush fast if environment parameters or secrets are missing in the environment (#12279)
1 parent 08533c5 commit 0b6e562

File tree

19 files changed

+393
-93
lines changed

19 files changed

+393
-93
lines changed

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

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import { $TSContext, stateManager, pathManager } from 'amplify-cli-core';
1+
import { $TSContext, stateManager, pathManager, AmplifyError } from 'amplify-cli-core';
22
import { mocked } from 'ts-jest/utils';
33
import * as path from 'path';
44
import { getEnvParamManager } from '@aws-amplify/amplify-environment-parameters';
55
import { FunctionSecretsStateManager } from '../../../../provider-utils/awscloudformation/secrets/functionSecretsStateManager';
6-
import { getAppId } from '../../../../provider-utils/awscloudformation/secrets/secretName';
6+
import * as stateManagerModule from '../../../../provider-utils/awscloudformation/secrets/functionSecretsStateManager';
7+
import { getAppId, getFunctionSecretPrefix } from '../../../../provider-utils/awscloudformation/secrets/secretName';
78
import { SSMClientWrapper } from '../../../../provider-utils/awscloudformation/secrets/ssmClientWrapper';
89

910
jest.mock('amplify-cli-core');
@@ -17,6 +18,11 @@ const stateManagerMock = mocked(stateManager);
1718
const pathManagerMock = mocked(pathManager);
1819
const getAppIdMock = mocked(getAppId);
1920
const SSMClientWrapperMock = mocked(SSMClientWrapper);
21+
const getFunctionSecretPrefixMock = mocked(getFunctionSecretPrefix);
22+
const AmplifyErrorMock = AmplifyError as jest.MockedClass<typeof AmplifyError>;
23+
24+
const amplifyErrorMockImpl = { name: 'TestError' } as unknown as AmplifyError;
25+
AmplifyErrorMock.mockImplementation(() => amplifyErrorMockImpl);
2026

2127
stateManagerMock.getLocalEnvInfo.mockReturnValue({
2228
envName: 'testTest',
@@ -27,30 +33,56 @@ pathManagerMock.getBackendDirPath.mockReturnValue(path.join('test', 'path'));
2733

2834
getAppIdMock.mockReturnValue('testAppId');
2935

36+
const getSecretNamesByPathMock = jest.fn();
37+
3038
SSMClientWrapperMock.getInstance.mockResolvedValue({
3139
deleteSecret: jest.fn(),
3240
setSecret: jest.fn(),
41+
getSecretNamesByPath: getSecretNamesByPathMock,
3342
} as unknown as SSMClientWrapper);
3443

35-
describe('syncSecretDeltas', () => {
36-
const contextStub = {
37-
parameters: {
38-
command: 'update',
44+
const getLocalFunctionSecretNamesSpy = jest.spyOn(stateManagerModule, 'getLocalFunctionSecretNames');
45+
46+
const contextStub = {
47+
parameters: {
48+
command: 'update',
49+
},
50+
input: {
51+
options: {
52+
yes: true,
3953
},
40-
} as unknown as $TSContext;
41-
beforeEach(jest.clearAllMocks);
54+
},
55+
} as unknown as $TSContext;
56+
let functionSecretsStateManager: FunctionSecretsStateManager;
57+
58+
beforeAll(async () => {
59+
functionSecretsStateManager = await FunctionSecretsStateManager.getInstance(contextStub);
60+
});
4261

62+
beforeEach(() => jest.clearAllMocks());
63+
64+
describe('syncSecretDeltas', () => {
4365
it('sets Amplify AppID in team-provider-info if secrets are present', async () => {
44-
const functionSecretsStateManager = await FunctionSecretsStateManager.getInstance(contextStub);
4566
await functionSecretsStateManager.syncSecretDeltas({ TEST_SECRET: { operation: 'retain' } }, 'testFuncName');
4667
expect(getEnvParamManager('testTest').getResourceParamManager('function', 'testFuncName').getAllParams()).toEqual({
4768
secretsPathAmplifyAppId: 'testAppId',
4869
});
4970
});
5071

5172
it('removes Amplify AppId from team-provider-info if secrets are not present', async () => {
52-
const functionSecretsStateManager = await FunctionSecretsStateManager.getInstance(contextStub);
5373
await functionSecretsStateManager.syncSecretDeltas({ TEST_SECRET: { operation: 'remove' } }, 'testFuncName');
5474
expect(getEnvParamManager('testTest').getResourceParamManager('function', 'testFuncName').getAllParams()).toEqual({});
5575
});
5676
});
77+
78+
describe('ensureNewLocalSecretsSyncedToCloud', () => {
79+
it('throws AmplifyError if secrets are missing and not in interactive mode', async () => {
80+
const funcName = 'testFunc';
81+
const secretPrefix = 'testPrefix';
82+
getFunctionSecretPrefixMock.mockReturnValue(secretPrefix);
83+
getLocalFunctionSecretNamesSpy.mockReturnValue(['secret1', 'secret2']);
84+
getSecretNamesByPathMock.mockResolvedValue(['secret2'].map((sec) => `${secretPrefix}${sec}`));
85+
86+
await expect(functionSecretsStateManager.ensureNewLocalSecretsSyncedToCloud(funcName)).rejects.toStrictEqual(amplifyErrorMockImpl);
87+
});
88+
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@ const writeCFNTemplateMock = writeCFNTemplate as jest.MockedFunction<typeof writ
1010
describe('test ensureLambdaExecutionRoleOutputs function', () => {
1111
beforeEach(() => {
1212
pathManagerMock.getBackendDirPath = jest.fn().mockReturnValue('backend');
13-
stateManagerMock.getMeta = jest.fn();
13+
stateManagerMock.getBackendConfig = jest.fn();
1414
});
1515

1616
afterEach(() => jest.resetAllMocks());
1717

1818
it(' when no functions are present', async () => {
19-
stateManagerMock.getMeta.mockReturnValue({
19+
stateManagerMock.getBackendConfig.mockReturnValue({
2020
auth: {
2121
authtriggertestb3a9da62b3a9da62: {
2222
customAuth: false,
@@ -31,7 +31,7 @@ describe('test ensureLambdaExecutionRoleOutputs function', () => {
3131
});
3232

3333
it(' when functions have no role arns in outputs', async () => {
34-
stateManagerMock.getMeta.mockReturnValue({
34+
stateManagerMock.getBackendConfig.mockReturnValue({
3535
auth: {
3636
authtriggertestb3a9da62b3a9da62: {
3737
customAuth: false,

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,18 @@
1+
import { printer } from '@aws-amplify/amplify-prompts';
12
import { $TSContext } from 'amplify-cli-core';
23
import { FunctionSecretsStateManager } from '../provider-utils/awscloudformation/secrets/functionSecretsStateManager';
34

45
export const postEnvRemoveHandler = async (context: $TSContext, envName: string) => {
5-
await removeAllEnvSecrets(context, envName);
6+
try {
7+
await removeAllEnvSecrets(context, envName);
8+
} catch (err) {
9+
// catching this silently because that was the previous behavior
10+
// previously this error was caught by the platform event firing logic and silently ignored
11+
// now we are ignoring errors at the individual event handler
12+
printer.debug(`function category postEnvRemoveHandler failed to run.`);
13+
printer.debug(`You may need to manually clean up some function state.`);
14+
printer.debug(err);
15+
}
616
};
717

818
const removeAllEnvSecrets = async (context: $TSContext, envName: string) =>

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ export const prePushHandler = async (context: $TSContext): Promise<void> => {
1717
};
1818

1919
const ensureFunctionSecrets = async (context: $TSContext): Promise<void> => {
20-
const amplifyMeta = stateManager.getMeta();
21-
const functionNames = Object.keys(amplifyMeta?.[categoryName]);
20+
const backendConfig = stateManager.getBackendConfig();
21+
const functionNames = Object.keys(backendConfig?.[categoryName] || {});
2222
for (const funcName of functionNames) {
2323
if (getLocalFunctionSecretNames(funcName).length > 0) {
2424
const funcSecretsManager = await FunctionSecretsStateManager.getInstance(context);

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { $TSContext, JSONUtilities, pathManager, ResourceName } from 'amplify-cli-core';
1+
import { $TSContext, AmplifyError, JSONUtilities, pathManager, ResourceName } from 'amplify-cli-core';
22
import { removeSecret, retainSecret, SecretDeltas, SecretName, setSecret } from '@aws-amplify/amplify-function-plugin-interface';
33
import * as path from 'path';
44
import * as fs from 'fs-extra';
@@ -105,9 +105,16 @@ export class FunctionSecretsStateManager {
105105
return;
106106
}
107107
if (!this.isInteractive()) {
108-
throw new Error(
109-
`The following secrets in ${functionName} do not have values: [${addedSecrets}]\nRun 'amplify push' interactively to specify values.`,
110-
);
108+
const resolution =
109+
`Run 'amplify push' interactively to specify values.\n` +
110+
`Alternatively, manually add values in SSM ParameterStore for the following parameter names:\n\n` +
111+
`${addedSecrets.map((secretName) => getFullyQualifiedSecretName(secretName, functionName)).join('\n')}\n`;
112+
throw new AmplifyError('EnvironmentConfigurationError', {
113+
message: `Function ${functionName} is missing secret values in this environment.`,
114+
details: `[${addedSecrets}] ${addedSecrets.length > 1 ? 'does' : 'do'} not have values.`,
115+
resolution,
116+
link: 'https://docs.amplify.aws/cli/reference/ssm-parameter-store/#manually-creating-parameters',
117+
});
111118
}
112119
const delta = await prePushMissingSecretsWalkthrough(functionName, addedSecrets);
113120
await this.syncSecretDeltas(delta, functionName);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ import * as path from 'path';
55
* updates function cfn stack with lambda execution role arn parameter
66
*/
77
export const ensureLambdaExecutionRoleOutputs = async (): Promise<void> => {
8-
const amplifyMeta = stateManager.getMeta();
9-
const functionNames = Object.keys(amplifyMeta?.[AmplifyCategories.FUNCTION] ?? []);
8+
const backendConfig = stateManager.getBackendConfig();
9+
const functionNames = Object.keys(backendConfig?.[AmplifyCategories.FUNCTION] ?? []);
1010
// filter lambda layer from lambdas in function
1111
const lambdaFunctionNames = functionNames.filter((functionName) => {
12-
const functionObj = amplifyMeta?.[AmplifyCategories.FUNCTION]?.[functionName];
12+
const functionObj = backendConfig?.[AmplifyCategories.FUNCTION]?.[functionName];
1313
return functionObj.service === AmplifySupportedService.LAMBDA;
1414
});
1515
for (const functionName of lambdaFunctionNames) {

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,19 @@ const askForEnvironmentVariableValue = async (
170170
*/
171171
export const ensureEnvironmentVariableValues = async (context: $TSContext): Promise<void> => {
172172
const yesFlagSet = context?.exeInfo?.inputParams?.yes || context?.input?.options?.yes;
173-
const currentEnvName = stateManager.getLocalEnvInfo()?.envName;
173+
const currentEnvName = stateManager.localEnvInfoExists()
174+
? stateManager.getLocalEnvInfo()?.envName
175+
: context?.exeInfo?.inputParams?.amplify?.envName;
174176
await ensureEnvParamManager(currentEnvName);
175-
const functionNames = Object.keys(stateManager.getBackendConfig()?.function);
177+
const functionNames = Object.keys(stateManager.getBackendConfig()?.function || {});
176178
if (functionNames.length === 0) {
177179
return;
178180
}
179181

180182
const functionConfigMissingEnvVars = functionNames
181183
.map((funcName) => {
182184
const storedList = getStoredList(funcName);
183-
const keyValues = getStoredKeyValue(funcName);
185+
const keyValues = getStoredKeyValue(funcName, currentEnvName);
184186
return {
185187
funcName,
186188
existingKeyValues: keyValues,
@@ -218,7 +220,7 @@ export const ensureEnvironmentVariableValues = async (context: $TSContext): Prom
218220
});
219221
keyValues[cfnName] = newValue;
220222
}
221-
setStoredKeyValue(funcName, keyValues);
223+
setStoredKeyValue(funcName, keyValues, currentEnvName);
222224
}
223225
};
224226

@@ -317,6 +319,6 @@ const setStoredParameters = (resourceName: string, newParameters: $TSAny): void
317319
const getStoredKeyValue = (resourceName: string, envName?: string): Record<string, string> =>
318320
getEnvParamManager(envName).getResourceParamManager(categoryName, resourceName).getAllParams();
319321

320-
const setStoredKeyValue = (resourceName: string, newKeyValue: $TSAny): void => {
321-
getEnvParamManager().getResourceParamManager(categoryName, resourceName).setAllParams(newKeyValue);
322+
const setStoredKeyValue = (resourceName: string, newKeyValue: $TSAny, envName?: string): void => {
323+
getEnvParamManager(envName).getResourceParamManager(categoryName, resourceName).setAllParams(newKeyValue);
322324
};

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { getChangedResources } from '../../commands/build';
2+
import { prompter } from '@aws-amplify/amplify-prompts';
3+
import { ensureEnvParamManager, IEnvironmentParameterManager } from '@aws-amplify/amplify-environment-parameters';
4+
import { verifyExpectedEnvParams } from '../../utils/verify-expected-env-params';
5+
import { $TSContext } from 'amplify-cli-core';
6+
7+
jest.mock('../../commands/build');
8+
jest.mock('@aws-amplify/amplify-prompts');
9+
jest.mock('@aws-amplify/amplify-environment-parameters');
10+
11+
const getResourcesMock = getChangedResources as jest.MockedFunction<typeof getChangedResources>;
12+
const ensureEnvParamManagerMock = ensureEnvParamManager as jest.MockedFunction<typeof ensureEnvParamManager>;
13+
const prompterMock = prompter as jest.Mocked<typeof prompter>;
14+
15+
const verifyExpectedEnvParametersMock = jest.fn();
16+
const getMissingParametersMock = jest.fn();
17+
const saveMock = jest.fn();
18+
19+
ensureEnvParamManagerMock.mockResolvedValue({
20+
instance: {
21+
verifyExpectedEnvParameters: verifyExpectedEnvParametersMock,
22+
getMissingParameters: getMissingParametersMock,
23+
save: saveMock,
24+
getResourceParamManager: jest.fn().mockReturnValue({
25+
setParam: jest.fn(),
26+
}),
27+
downloadParameters: jest.fn(),
28+
} as unknown as IEnvironmentParameterManager,
29+
});
30+
31+
const resourceList = [
32+
{
33+
category: 'storage',
34+
resourceName: 'testStorage',
35+
service: 'S3',
36+
},
37+
{
38+
category: 'function',
39+
resourceName: 'testFunction',
40+
service: 'Lambda',
41+
},
42+
];
43+
44+
getResourcesMock.mockResolvedValue(resourceList);
45+
46+
const resetContext = {
47+
exeInfo: {
48+
inputParams: {
49+
yes: true,
50+
},
51+
},
52+
amplify: {
53+
invokePluginMethod: jest.fn(),
54+
},
55+
} as unknown as $TSContext;
56+
57+
describe('verifyExpectedEnvParams', () => {
58+
let contextStub = resetContext;
59+
beforeEach(() => {
60+
jest.clearAllMocks();
61+
contextStub = resetContext;
62+
});
63+
it('filters parameters based on category and resourceName if specified', async () => {
64+
await verifyExpectedEnvParams(contextStub, 'storage');
65+
expect(verifyExpectedEnvParametersMock).toHaveBeenCalledWith([resourceList[0]]);
66+
});
67+
68+
it('calls verify expected parameters if in non-interactive mode', async () => {
69+
await verifyExpectedEnvParams(contextStub, 'storage');
70+
expect(verifyExpectedEnvParametersMock).toHaveBeenCalled();
71+
expect(getMissingParametersMock).not.toHaveBeenCalled();
72+
});
73+
74+
it('prompts for missing parameters if in interactive mode', async () => {
75+
contextStub.exeInfo.inputParams.yes = false;
76+
getMissingParametersMock.mockResolvedValue([
77+
{
78+
categoryName: 'function',
79+
resourceName: 'testFunction',
80+
parameterName: 'something',
81+
},
82+
]);
83+
await verifyExpectedEnvParams(contextStub, 'storage');
84+
expect(verifyExpectedEnvParametersMock).not.toHaveBeenCalled();
85+
expect(prompterMock.input).toHaveBeenCalled();
86+
expect(saveMock).toHaveBeenCalled();
87+
});
88+
});

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export const init = (_context: Context): void => {
1818
* Handle exceptions
1919
*/
2020
export const handleException = async (exception: unknown): Promise<void> => {
21+
process.exitCode = 1;
2122
let amplifyException: AmplifyException;
2223

2324
if (exception instanceof AmplifyException) {

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export const run = async (context: $TSContext): Promise<void> => {
1414

1515
try {
1616
await generateDependentResourcesType();
17-
const resourcesToBuild: IAmplifyResource[] = await getResources(context);
17+
const resourcesToBuild: IAmplifyResource[] = await getChangedResources(context);
1818
let filteredResources: IAmplifyResource[] = resourcesToBuild;
1919
if (categoryName) {
2020
filteredResources = filteredResources.filter((resource) => resource.category === categoryName);
@@ -44,7 +44,7 @@ export const run = async (context: $TSContext): Promise<void> => {
4444
/**
4545
* Returns resources in create or update state
4646
*/
47-
export const getResources = async (context: $TSContext): Promise<IAmplifyResource[]> => {
47+
export const getChangedResources = async (context: $TSContext): Promise<IAmplifyResource[]> => {
4848
const resources: IAmplifyResource[] = [];
4949
const { resourcesToBeCreated, resourcesToBeUpdated } = await context.amplify.getResourceStatus();
5050
resourcesToBeCreated.forEach((resourceCreated: IAmplifyResource) => {
@@ -64,3 +64,16 @@ export const getResources = async (context: $TSContext): Promise<IAmplifyResourc
6464
});
6565
return resources;
6666
};
67+
68+
export const getAllResources = async (context: $TSContext): Promise<IAmplifyResource[]> => {
69+
const resources: IAmplifyResource[] = [];
70+
const { allResources } = await context.amplify.getResourceStatus();
71+
allResources.forEach((resource: IAmplifyResource) => {
72+
resources.push({
73+
service: resource.service,
74+
category: resource.category,
75+
resourceName: resource.resourceName,
76+
});
77+
});
78+
return resources;
79+
};

0 commit comments

Comments
 (0)