-
Notifications
You must be signed in to change notification settings - Fork 825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Defer root stack creation to first amplify push
#7174
Changes from all commits
1c23274
18fd3f0
fe5904a
814b1ae
d890f86
83d81f8
c60fe33
6792dda
c12f99a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,12 @@ export function saveEnvResourceParameters(context: $TSContext, category: string, | |
stateManager.setTeamProviderInfo(undefined, teamProviderInfo); | ||
// write hostedUIProviderCreds to deploymentSecrets | ||
const deploymentSecrets = stateManager.getDeploymentSecrets(); | ||
const rootStackId = getRootStackId(); | ||
let rootStackId; | ||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on usage of |
||
rootStackId = getRootStackId(); | ||
} catch (err) { | ||
return; | ||
} | ||
if (hostedUIProviderCreds) { | ||
stateManager.setDeploymentSecrets( | ||
mergeDeploymentSecrets({ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,8 +4,8 @@ export function getRootStackId(): string { | |
const teamProviderInfo = stateManager.getTeamProviderInfo(); | ||
const { envName } = stateManager.getLocalEnvInfo(); | ||
const envTeamProviderInfo = teamProviderInfo[envName]; | ||
if (envTeamProviderInfo && envTeamProviderInfo.awscloudformation) { | ||
const stackId = envTeamProviderInfo.awscloudformation.StackId; | ||
const stackId = envTeamProviderInfo?.awscloudformation?.StackId; | ||
if (typeof stackId === 'string') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What else could it be if not undefined? why checking for type here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should only ever be a string or undefined but checking for string is a stronger assertion than checking for not undefined |
||
return stackId.split('/')[2]; | ||
} | ||
throw new Error('Root stack Id not found'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we enhance the error to show what was the value of |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import { getProviderPlugins } from '../extensions/amplify-helpers/get-provider-p | |
import { insertAmplifyIgnore } from '../extensions/amplify-helpers/git-manager'; | ||
import { writeReadMeFile } from '../extensions/amplify-helpers/docs-manager'; | ||
import { initializeEnv } from '../initialize-env'; | ||
import _ from 'lodash'; | ||
|
||
export async function onHeadlessSuccess(context: $TSContext) { | ||
const frontendPlugins = getFrontendPlugins(context); | ||
|
@@ -108,12 +109,19 @@ function generateLocalTagsFile(context: $TSContext) { | |
} | ||
|
||
export function generateAmplifyMetaFile(context: $TSContext) { | ||
if (context.exeInfo.isNewEnv) { | ||
const { projectPath } = context.exeInfo.localEnvInfo; | ||
const { projectPath } = context.exeInfo.localEnvInfo; | ||
|
||
stateManager.setCurrentMeta(projectPath, context.exeInfo.amplifyMeta); | ||
stateManager.setMeta(projectPath, context.exeInfo.amplifyMeta); | ||
} | ||
const { isNewEnv } = context.exeInfo; | ||
|
||
// store amplifyMeta | ||
const meta = isNewEnv ? {} : stateManager.getMeta(projectPath, { throwIfNotExist: false }) || {}; | ||
_.merge(meta, context.exeInfo.amplifyMeta); | ||
stateManager.setMeta(projectPath, meta); | ||
|
||
// store current cloud meta | ||
const currMeta = isNewEnv ? {} : stateManager.getCurrentMeta(projectPath, { throwIfNotExist: false }) || {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: The previous code was setting the same meta object into both files, why do we need to change the logic to read in the file, merge, and store it individually? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. previously, this code was only called during env initialization when there was nothing preexisting in the meta files. But now this could be called after adding resources to the env so we need to make sure we don't erase what's there |
||
_.merge(currMeta, context.exeInfo.amplifyMeta); | ||
stateManager.setCurrentMeta(projectPath, currMeta); | ||
} | ||
|
||
function generateNonRuntimeFiles(context: $TSContext) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ import { nspawn as spawn, retry, getCLIPath, describeCloudFormationStack, getPro | |
|
||
export const deleteProject = async (cwd: string, profileConfig?: any): Promise<void> => { | ||
const { StackName: stackName, Region: region } = getProjectMeta(cwd).providers.awscloudformation; | ||
if (!stackName || !region) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was it a missing check causing test failures or is it due to changes in init? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have some tests that just do an init and then check some local state but then the afterEach step tries to delete the project and fails here because the stack doesn't exist yet |
||
return; | ||
} | ||
await retry( | ||
() => describeCloudFormationStack(stackName, region, profileConfig), | ||
stack => stack.StackStatus.endsWith('_COMPLETE'), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
import { | ||
addFunction, | ||
amplifyInitYes, | ||
amplifyPushAuth, | ||
createNewProjectDir, | ||
deleteProject, | ||
deleteProjectDir, | ||
getLocalEnvInfo, | ||
getTeamProviderInfo, | ||
initJSProjectWithProfile, | ||
} from 'amplify-e2e-core'; | ||
import { addEnvironment, addEnvironmentYes, checkoutEnvironment, removeEnvironment } from '../environment/env'; | ||
|
||
describe('defer root stack push', () => { | ||
let projRoot: string; | ||
beforeEach(async () => { | ||
projRoot = await createNewProjectDir('defer-init-push'); | ||
}); | ||
|
||
afterEach(async () => { | ||
await deleteProject(projRoot); | ||
deleteProjectDir(projRoot); | ||
}); | ||
|
||
it('does not create a root stack on interactive init', async () => { | ||
await initJSProjectWithProfile(projRoot, { name: 'deferInitPush', envName: 'dev' }); | ||
const tpi_before = getTeamProviderInfo(projRoot); | ||
expect(tpi_before?.dev?.awscloudformation?.DeploymentBucketName).toBeUndefined(); | ||
await addFunction(projRoot, { functionTemplate: 'Hello World' }, 'nodejs'); | ||
await amplifyPushAuth(projRoot); | ||
const tpi_after = getTeamProviderInfo(projRoot); | ||
expect(tpi_after?.dev?.awscloudformation?.DeploymentBucketName).toBeDefined(); | ||
}); | ||
|
||
it('does not create a root stack on interactive env add', async () => { | ||
await initJSProjectWithProfile(projRoot, { name: 'deferInitPush', envName: 'dev' }); | ||
const tpi_dev = getTeamProviderInfo(projRoot); | ||
expect(tpi_dev?.dev?.awscloudformation?.DeploymentBucketName).toBeUndefined(); | ||
await addEnvironment(projRoot, { envName: 'test' }); | ||
const tpi_test = getTeamProviderInfo(projRoot); | ||
expect(tpi_test?.test?.awscloudformation?.DeploymentBucketName).toBeUndefined(); | ||
}); | ||
|
||
it('creates a root stack on headless init', async () => { | ||
await amplifyInitYes(projRoot); | ||
const tpi_after = getTeamProviderInfo(projRoot); | ||
expect(tpi_after?.dev?.awscloudformation?.DeploymentBucketName).toBeDefined(); | ||
}); | ||
|
||
it('creates a root stack on headless env add', async () => { | ||
await initJSProjectWithProfile(projRoot, { name: 'deferInitPush', envName: 'dev' }); | ||
const tpi_dev = getTeamProviderInfo(projRoot); | ||
expect(tpi_dev?.dev?.awscloudformation?.DeploymentBucketName).toBeUndefined(); | ||
await addEnvironmentYes(projRoot, { envName: 'test' }); | ||
const tpi_test = getTeamProviderInfo(projRoot); | ||
expect(tpi_test?.test?.awscloudformation?.DeploymentBucketName).toBeDefined(); | ||
}); | ||
|
||
it('can checkout unpushed environment', async () => { | ||
await initJSProjectWithProfile(projRoot, { name: 'deferInitPush', envName: 'dev' }); | ||
const tpi_dev = getTeamProviderInfo(projRoot); | ||
expect(tpi_dev?.dev?.awscloudformation?.DeploymentBucketName).toBeUndefined(); | ||
await addEnvironment(projRoot, { envName: 'test' }); | ||
const tpi_test = getTeamProviderInfo(projRoot); | ||
expect(tpi_test?.test?.awscloudformation?.DeploymentBucketName).toBeUndefined(); | ||
await checkoutEnvironment(projRoot, { envName: 'dev' }); | ||
const localEnvInfo = getLocalEnvInfo(projRoot); | ||
expect(localEnvInfo?.envName).toEqual('dev'); | ||
}); | ||
|
||
it('can remove unpushed environment', async () => { | ||
await initJSProjectWithProfile(projRoot, { name: 'deferInitPush', envName: 'dev' }); | ||
await addEnvironment(projRoot, { envName: 'test' }); | ||
await checkoutEnvironment(projRoot, { envName: 'dev' }); | ||
const tpi_before = getTeamProviderInfo(projRoot); | ||
expect(Object.keys(tpi_before).sort()).toEqual(['dev', 'test']); | ||
await removeEnvironment(projRoot, { envName: 'test' }); | ||
const tpi_after = getTeamProviderInfo(projRoot); | ||
expect(Object.keys(tpi_after)).toEqual(['dev']); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to add these to
split-e2e-tests.ts
that will generate this file based onconfig-base.yml
otherwise these will be lost.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is generated by the split script. The script will automatically add any files in the tests dir