Skip to content
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

Gen2 migrations fixes #14045

Merged
merged 7 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import assert from 'node:assert';
import { DataDefinition } from '@aws-amplify/amplify-gen2-codegen';
import { Stack } from '@aws-sdk/client-cloudformation';

export const tableMappingKey = 'TableMapping';
export const tableMappingKey = 'DataSourceMappingOutput';

export const getDataDefinition = (dataStack: Stack): DataDefinition => {
const rawTableMapping = dataStack.Outputs?.find((o) => o.OutputKey === tableMappingKey)?.OutputValue;
Expand Down
12 changes: 7 additions & 5 deletions packages/amplify-gen2-codegen/src/backend/synthesizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ describe('BackendRenderer', () => {
},
});
const output = printNodeArray(rendered);
assert(output.includes('s3Bucket.applyRemovalPolicy'));
assert(output.includes('// s3Bucket.applyRemovalPolicy'));
assert(output.includes('import { RemovalPolicy } from "aws-cdk-lib";'));
});
});
Expand Down Expand Up @@ -410,7 +410,7 @@ describe('BackendRenderer', () => {
CallbackURLs: ['XXXXXXXXXXXXXXXXXX', 'XXXXXXXXXXXXXXXXXXXXXX'],
LogoutURLs: ['XXXXXXXXXXXXXXXXXX', 'XXXXXXXXXXXXXXXXXXXXXX'],
DefaultRedirectURI: 'XXXXXXXXXXXXXXXXXX',
SupportedIdentityProviders: ['COGNITO'],
SupportedIdentityProviders: ['COGNITO', 'Facebook'],
AuthSessionValidity: 3,
EnableTokenRevocation: true,
EnablePropagateAdditionalUserContextData: true,
Expand Down Expand Up @@ -455,7 +455,9 @@ describe('BackendRenderer', () => {
expect(output).toContain('authFlows: { adminUserPassword: false, custom: false, userPassword: false, userSrp: false }');

// Additional settings
expect(output).toContain('supportedIdentityProviders');
expect(output).toContain(`supportedIdentityProviders`);
expect(output).toContain(`UserPoolClientIdentityProvider.COGNITO`);
expect(output).toContain(`UserPoolClientIdentityProvider.FACEBOOK`);
expect(output).toContain('authSessionValidity: Duration.minutes(3)');
expect(output).toContain('enableTokenRevocation: true');
expect(output).toContain('enablePropagateAdditionalUserContextData: true');
Expand All @@ -470,8 +472,8 @@ describe('BackendRenderer', () => {
},
});
const output = printNodeArray(rendered);
assert(output.includes('cfnUserPool.applyRemovalPolicy'));
assert(output.includes('cfnIdentityPool.applyRemovalPolicy'));
assert(output.includes('// cfnUserPool.applyRemovalPolicy'));
assert(output.includes('// cfnIdentityPool.applyRemovalPolicy'));
assert(output.includes('import { RemovalPolicy } from "aws-cdk-lib";'));
});
});
Expand Down
35 changes: 25 additions & 10 deletions packages/amplify-gen2-codegen/src/backend/synthesizer.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import ts, {
Node,
ExpressionStatement,
CallExpression,
Expression,
VariableDeclaration,
ExpressionStatement,
Identifier,
NodeArray,
ImportDeclaration,
Node,
NodeArray,
VariableDeclaration,
VariableStatement,
} from 'typescript';
import { PolicyOverrides, ReferenceAuth } from '../auth/source_builder.js';
import { BucketAccelerateStatus, BucketVersioningStatus } from '@aws-sdk/client-s3';
import { AccessPatterns, ServerSideEncryptionConfiguration } from '../storage/source_builder.js';
import { UserPoolClientType, OAuthFlowType, ExplicitAuthFlowsType } from '@aws-sdk/client-cognito-identity-provider';
import { ExplicitAuthFlowsType, OAuthFlowType, UserPoolClientType } from '@aws-sdk/client-cognito-identity-provider';
import assert from 'assert';

const factory = ts.factory;
export interface BackendRenderParameters {
data?: {
Expand Down Expand Up @@ -162,7 +163,7 @@ export class BackendSynthesizer {

private addRemovalPolicyAssignment(identifier: string) {
return factory.createCallExpression(
factory.createPropertyAccessExpression(factory.createIdentifier(identifier), factory.createIdentifier('applyRemovalPolicy')),
factory.createPropertyAccessExpression(factory.createIdentifier(`// ${identifier}`), factory.createIdentifier('applyRemovalPolicy')),
undefined,
[
factory.createIdentifier('RemovalPolicy.RETAIN'),
Expand Down Expand Up @@ -233,7 +234,12 @@ export class BackendSynthesizer {
const mappedProperty = gen2PropertyMap.get(key);
if (mappedProperty) {
if (typeof value == 'boolean') {
objectLiterals.push(this.createBooleanPropertyAssignment(mappedProperty, value));
if (key === 'AllowedOAuthFlowsUserPoolClient') {
// CDK equivalent is disableOAuth which is opposite of this prop
objectLiterals.push(this.createBooleanPropertyAssignment(mappedProperty, !value));
} else {
objectLiterals.push(this.createBooleanPropertyAssignment(mappedProperty, value));
}
} else if (typeof value == 'string') {
if (!this.oAuthFlag && key == 'DefaultRedirectURI') {
this.oAuthFlag = true;
Expand Down Expand Up @@ -276,7 +282,14 @@ export class BackendSynthesizer {
objectLiterals.push(this.createReadWriteAttributes(mappedProperty, value));
} else if (key == 'SupportedIdentityProviders') {
this.supportedIdentityProviderFlag = true;
objectLiterals.push(this.createEnumListPropertyAssignment(mappedProperty, 'UserPoolClientIdentityProvider', value));
// Providers are upper case in CDK
objectLiterals.push(
this.createEnumListPropertyAssignment(
mappedProperty,
'UserPoolClientIdentityProvider',
value.map((provider) => provider.toUpperCase()),
),
);
} else if (!this.oAuthFlag && key == 'AllowedOAuthFlows') {
this.oAuthFlag = true;
objectLiterals.push(this.createOAuthObjectExpression(object, gen2PropertyMap));
Expand Down Expand Up @@ -445,9 +458,12 @@ export class BackendSynthesizer {
TemporaryPasswordValidityDays: 'temporaryPasswordValidityDays',
};

if (renderArgs.auth || renderArgs.storage?.hasS3Bucket) {
imports.push(this.createImportStatement([factory.createIdentifier('RemovalPolicy')], 'aws-cdk-lib'));
}

if (renderArgs.auth) {
imports.push(this.createImportStatement([authFunctionIdentifier], renderArgs.auth.importFrom));
imports.push(this.createImportStatement([factory.createIdentifier('RemovalPolicy')], 'aws-cdk-lib'));
const auth = factory.createShorthandPropertyAssignment(authFunctionIdentifier);
defineBackendProperties.push(auth);
}
Expand All @@ -460,7 +476,6 @@ export class BackendSynthesizer {

if (renderArgs.storage?.hasS3Bucket) {
imports.push(this.createImportStatement([storageFunctionIdentifier], renderArgs.storage.importFrom));
imports.push(this.createImportStatement([factory.createIdentifier('RemovalPolicy')], 'aws-cdk-lib'));
const storage = factory.createShorthandPropertyAssignment(storageFunctionIdentifier);
defineBackendProperties.push(storage);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,23 @@ Describe stack refactor to check for execution status
`### STEP 2: REDEPLOY GEN2 APPLICATION
This step will remove the hardcoded references from the template and replace them with resource references (where applicable).

2.a) Only applicable to Storage category: Uncomment the following line in \`amplify/backend.ts\` file to instruct CDK to use the gen1 S3 bucket
2.a) Uncomment the following lines in \`amplify/backend.ts\` file to instruct CDK to use the gen1 S3 bucket (if storage is enabled) and apply retain removal policies for auth and/or storage resources
\`\`\`
s3Bucket.bucketName = YOUR_GEN1_BUCKET_NAME;
\`\`\`

\`\`\`
s3Bucket.applyRemovalPolicy(RemovalPolicy.RETAIN, { applyToUpdateReplacePolicy: true });
\`\`\`

\`\`\`
cfnUserPool.applyRemovalPolicy(RemovalPolicy.RETAIN, { applyToUpdateReplacePolicy: true });
\`\`\`

\`\`\`
cfnIdentityPool.applyRemovalPolicy(RemovalPolicy.RETAIN, { applyToUpdateReplacePolicy: true });
\`\`\`

2.b) Deploy sandbox using the below command or trigger a CI/CD build via hosting by committing this file to your Git repository
\`\`\`
npx ampx sandbox
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,23 @@ Describe stack refactor to check for execution status
`### STEP 2: REDEPLOY GEN2 APPLICATION
This step will remove the hardcoded references from the template and replace them with resource references (where applicable).

2.a) Only applicable to Storage category: Uncomment the following line in \`amplify/backend.ts\` file to instruct CDK to use the gen1 S3 bucket
2.a) Uncomment the following lines in \`amplify/backend.ts\` file to instruct CDK to use the gen1 S3 bucket (if storage is enabled) and apply retain removal policies for auth and/or storage resources
\`\`\`
s3Bucket.bucketName = YOUR_GEN1_BUCKET_NAME;
\`\`\`

\`\`\`
s3Bucket.applyRemovalPolicy(RemovalPolicy.RETAIN, { applyToUpdateReplacePolicy: true });
\`\`\`

\`\`\`
cfnUserPool.applyRemovalPolicy(RemovalPolicy.RETAIN, { applyToUpdateReplacePolicy: true });
\`\`\`

\`\`\`
cfnIdentityPool.applyRemovalPolicy(RemovalPolicy.RETAIN, { applyToUpdateReplacePolicy: true });
\`\`\`

2.b) Deploy sandbox using the below command or trigger a CI/CD build via hosting by committing this file to your Git repository
\`\`\`
npx ampx sandbox
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CognitoIdentityProviderClient, DescribeUserPoolCommand, ListGroupsCommand } from '@aws-sdk/client-cognito-identity-provider';
import { CognitoIdentityClient, GetIdentityPoolRolesCommand, ListIdentityPoolsCommand } from '@aws-sdk/client-cognito-identity';
import { CloudFormationClient } from '@aws-sdk/client-cloudformation';
import { CloudFormationClient, DescribeStackResourcesCommand } from '@aws-sdk/client-cloudformation';
import { AmplifyClient, ListBackendEnvironmentsCommand } from '@aws-sdk/client-amplify';
import { GetObjectCommand, S3Client } from '@aws-sdk/client-s3';

Expand Down Expand Up @@ -131,6 +131,7 @@ jest.mock('@aws-sdk/client-amplify', () => {
{
environmentName: 'dev',
deploymentArtifacts: 's3://deploymentArtifacts',
stackName: 'stackName',
},
],
});
Expand Down Expand Up @@ -160,6 +161,24 @@ jest.mock('@aws-sdk/client-s3', () => {
};
});

jest.mock('@aws-sdk/client-cloudformation', () => {
return {
...jest.requireActual('@aws-sdk/client-cloudformation'),
CloudFormationClient: function () {
return {
send: jest.fn().mockImplementation((command) => {
if (command instanceof DescribeStackResourcesCommand) {
return Promise.resolve({
StackResources: [],
});
}
return Promise.resolve();
}),
};
},
};
});

const cognitoIdentityProviderClient = new CognitoIdentityProviderClient();
const cognitoIdentityClient = new CognitoIdentityClient();
const cloudFormationClient = new CloudFormationClient();
Expand All @@ -179,6 +198,16 @@ describe('Auth definition Fetcher tests', () => {
() => Promise.resolve({}),
ccbFetcher,
);
it('should not fetch imported auth definitions when not present', async () => {
// arrange
mockReadFile.mockResolvedValueOnce(
JSON.stringify({
api: {},
}),
);
// act + assert
await expect(appAuthDefinitionFetcher.getDefinition()).resolves.toEqual(undefined);
});
it('should fetch imported auth definitions', async () => {
await expect(appAuthDefinitionFetcher.getDefinition()).resolves.toEqual({
referenceAuth: {
Expand All @@ -195,7 +224,7 @@ describe('Auth definition Fetcher tests', () => {
});

it('should not fetch imported auth definitions if there is no related cognito resource information', async () => {
mockReadFile.mockResolvedValue(
mockReadFile.mockResolvedValueOnce(
JSON.stringify({
auth: {
importedAuth: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ export class AppAuthDefinitionFetcher {
}

const amplifyMeta = (await this.readJsonFile(amplifyMetaPath)) ?? {};
const isImported = Object.keys(amplifyMeta.auth).map((key) => amplifyMeta.auth[key])[0].serviceType === 'imported';

const isImported =
'auth' in amplifyMeta && Object.keys(amplifyMeta.auth).map((key) => amplifyMeta.auth[key])[0].serviceType === 'imported';
if (!isImported) {
return undefined;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class AppFunctionsDefinitionFetcher {
const meta = this.stateManager.getMeta();
const functions = meta?.function ?? {};

const auth = meta?.auth;
const auth = meta?.auth ?? {};
const storageList = meta?.storage ?? {};

const functionCategoryMap = new Map<string, string>();
Expand Down
9 changes: 7 additions & 2 deletions packages/amplify-migration/src/command-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ const resolveAppId = (): string => {
const unsupportedCategories = (): Map<string, string> => {
const unsupportedCategories = new Map<string, string>();
const urlPrefix = 'https://docs.amplify.aws/react/build-a-backend/add-aws-services';
const restAPIKey = 'rest api';

unsupportedCategories.set('geo', `${urlPrefix}/geo/`);
unsupportedCategories.set('analytics', `${urlPrefix}/analytics/`);
Expand All @@ -179,13 +180,17 @@ const unsupportedCategories = (): Map<string, string> => {
Object.keys(apiList).forEach((api) => {
const apiObj = apiList[api];
if (apiObj.service == 'API Gateway') {
unsupportedCategoriesList.set('rest api', unsupportedCategories.get('rest api')!);
const restAPIDocsLink = unsupportedCategories.get(restAPIKey);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

part of lint fix

assert(restAPIDocsLink);
unsupportedCategoriesList.set(restAPIKey, restAPIDocsLink);
}
});
}
} else {
if (unsupportedCategories.has(category) && Object.entries(meta[category]).length > 0) {
unsupportedCategoriesList.set(category, unsupportedCategories.get(category)!);
const unsupportedCategoryDocLink = unsupportedCategories.get(category);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

part of lint fix

assert(unsupportedCategoryDocLink);
unsupportedCategoriesList.set(category, unsupportedCategoryDocLink);
}
}
});
Expand Down
Loading