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

fix: package json name and deps, always set generateSecrets prop #14070

Merged
merged 3 commits into from
Jan 16, 2025
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
25 changes: 24 additions & 1 deletion packages/amplify-gen2-codegen/src/backend/synthesizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ describe('BackendRenderer', () => {
const output = printNodeArray(rendered);

// Basic configuration
expect(output).toContain('"us-west-2_aaaaaaaaa"');
expect(output).toContain('NativeAppClient');
expect(output).toContain('userPoolClientName: "MyApp"');

// Token validity settings
Expand Down Expand Up @@ -461,6 +461,7 @@ describe('BackendRenderer', () => {
expect(output).toContain('authSessionValidity: Duration.minutes(3)');
expect(output).toContain('enableTokenRevocation: true');
expect(output).toContain('enablePropagateAdditionalUserContextData: true');
expect(output).toContain('generateSecret: true');
});
it('renders userpool and identitypool deletion policy', () => {
const renderer = new BackendSynthesizer();
Expand All @@ -476,5 +477,27 @@ describe('BackendRenderer', () => {
assert(output.includes('// cfnIdentityPool.applyRemovalPolicy'));
assert(output.includes('import { RemovalPolicy } from "aws-cdk-lib";'));
});
it('renders user pool client configuration with default value for generateSecrets', () => {
const renderer = new BackendSynthesizer();
const rendered = renderer.render({
auth: {
importFrom: './auth/resource.ts',
userPoolClient: {
UserPoolId: 'us-west-2_aaaaaaaaa',
ClientName: 'MyApp',
ClientId: '38fjsnc484p94kpqsnet7mpld0',
},
},
});

const output = printNodeArray(rendered);

// Basic configuration
expect(output).toContain('NativeAppClient');
expect(output).toContain('userPoolClientName: "MyApp"');

// Additional settings
expect(output).toContain('generateSecret: false');
});
});
});
16 changes: 11 additions & 5 deletions packages/amplify-gen2-codegen/src/backend/synthesizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,7 @@ export class BackendSynthesizer {
const test = factory.createCallExpression(
factory.createPropertyAccessExpression(factory.createIdentifier('userPool'), factory.createIdentifier('addClient')),
undefined,
[
factory.createStringLiteral(userPoolClient.UserPoolId!),
this.createNestedObjectExpression(userPoolClient, userPoolClientAttributesMap),
],
[factory.createStringLiteral('NativeAppClient'), this.createNestedObjectExpression(userPoolClient, userPoolClientAttributesMap)],
);

if (this.importDurationFlag) {
Expand All @@ -229,6 +226,7 @@ export class BackendSynthesizer {

private createNestedObjectExpression(object: Record<string, any>, gen2PropertyMap: Map<string, string>): ts.ObjectLiteralExpression {
const objectLiterals = [];
const clientSecretKey = 'ClientSecret';

for (const [key, value] of Object.entries(object)) {
const mappedProperty = gen2PropertyMap.get(key);
Expand All @@ -244,7 +242,7 @@ export class BackendSynthesizer {
if (!this.oAuthFlag && key == 'DefaultRedirectURI') {
this.oAuthFlag = true;
objectLiterals.push(this.createOAuthObjectExpression(object, gen2PropertyMap));
} else if (key == 'ClientSecret') {
} else if (key === clientSecretKey) {
objectLiterals.push(this.createBooleanPropertyAssignment(mappedProperty, true));
} else if (key != 'DefaultRedirectURI') {
objectLiterals.push(this.createStringPropertyAssignment(mappedProperty, value));
Expand Down Expand Up @@ -315,6 +313,14 @@ export class BackendSynthesizer {
}
}
}
// We need to set generateSecret to false explicitly when not defined.
// If it's set as undefined and current value in CFN template is false (moved from gen1 after refactor), CFN thinks the property has changed
// and requests for creation of a new resource (user pool client) instead of an update.
if (object[clientSecretKey] === undefined && gen2PropertyMap.has(clientSecretKey)) {
const mappedClientSecretKey = gen2PropertyMap.get(clientSecretKey);
assert(mappedClientSecretKey);
objectLiterals.push(this.createBooleanPropertyAssignment(mappedClientSecretKey, false));
}
return factory.createObjectLiteralExpression(objectLiterals, true);
}

Expand Down
26 changes: 22 additions & 4 deletions packages/amplify-gen2-codegen/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import path from 'path';
import fs from 'node:fs/promises';
import { patchNpmPackageJson } from './npm_package/renderer';
import { PackageJson, patchNpmPackageJson } from './npm_package/renderer';
import { RenderPipeline, Renderer } from './render_pipeline';
import { JsonRenderer } from './renderers/package_json';
import { TypescriptNodeArrayRenderer } from './renderers/typescript_block_node';
Expand Down Expand Up @@ -74,15 +74,33 @@ export const createGen2Renderer = ({
const ensureOutputDir = new EnsureDirectory(outputDir);
const ensureAmplifyDirectory = new EnsureDirectory(path.join(outputDir, 'amplify'));
const amplifyPackageJson = new JsonRenderer(
() => ({ type: 'module' }),
async () => ({ type: 'module' }),
(content) => fileWriter(content, path.join(outputDir, 'amplify', 'package.json')),
);
const jsonRenderer = new JsonRenderer(
() => patchNpmPackageJson({}),
async () => {
let packageJson: PackageJson = {
name: 'my-gen2-app',
};
try {
const packageJsonContents = await fs.readFile(`./package.json`, { encoding: 'utf-8' });
packageJson = JSON.parse(packageJsonContents);
} catch (e) {
// File doesn't exist or is inaccessible. Ignore.
}
// Restrict dev dependencies to specific versions based on create-amplify gen2 flow:
// https://github.com/aws-amplify/amplify-backend/blob/2dab201cb9a222c3b8c396a46c17d661411839ab/packages/create-amplify/src/amplify_project_creator.ts#L15-L24
return patchNpmPackageJson(packageJson, {
'aws-cdk': '^2',
'aws-cdk-lib': '^2',
constructs: '^10.0.0',
typescript: '^5.0.0',
});
},
(content) => fileWriter(content, path.join(outputDir, 'package.json')),
);
const amplifyTsConfigJson = new JsonRenderer(
() => ({
async () => ({
compilerOptions: {
target: 'es2022',
module: 'es2022',
Expand Down
17 changes: 6 additions & 11 deletions packages/amplify-gen2-codegen/src/npm_package/renderer.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import { AmplifyDependencies, AmplifyDevDependencies, AmplifyPackageVersions, patchNpmPackageJson } from './renderer';
import { AmplifyDependencies, AmplifyDevDependencies, AmplifyPackageVersions, PackageJson, patchNpmPackageJson } from './renderer';
import assert from 'node:assert';
interface PackageJSON {
name?: string;
scripts?: Record<string, string>;
dependencies?: Record<string, string>;
devDependencies?: Record<string, string>;
}
const createPackageJson = (): PackageJSON => ({
const createPackageJson = (): PackageJson => ({
name: 'my-package',
scripts: {
test: 'echo "hello, world"',
Expand Down Expand Up @@ -78,9 +72,10 @@ describe('package.json renderer', () => {
});
});
describe('package name', () => {
it('is set to my-gen2-app when not defined', async () => {
const examplePackageJson = createPackageJson();
delete examplePackageJson.name;
it('is not overwritten ', async () => {
const examplePackageJson = {
name: 'my-gen2-app',
};
const packageJson = patchNpmPackageJson(examplePackageJson, {});
assert.equal(packageJson.name, 'my-gen2-app');
});
Expand Down
7 changes: 2 additions & 5 deletions packages/amplify-gen2-codegen/src/npm_package/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,13 @@ export type PackageJsonDependencies = {

export type PackageJson = {
name: string;
scripts?: Record<string, string>;
} & PackageJsonDependencies;

const withDefault = (version?: string) => version ?? '*';

export const patchNpmPackageJson = (
packageJson: PackageJsonDependencies,
packageVersions: Partial<AmplifyPackageVersions> = {},
): PackageJson => {
export const patchNpmPackageJson = (packageJson: PackageJson, packageVersions: Partial<AmplifyPackageVersions> = {}): PackageJson => {
return {
name: 'my-gen2-app',
...packageJson,
devDependencies: {
...(packageJson.devDependencies ?? {}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { JsonRenderer } from './package_json';
describe('PackageJsonRenderer', () => {
it('renders the json object to a string', async () => {
const json = { name: 'my-package', version: 'my-version' };
const jsonCreator = () => json;
const jsonCreator = async () => json;
const testWriter = jest.fn(async () => {});
const jsonRenderer = new JsonRenderer(jsonCreator, testWriter);
await jsonRenderer.render();
Expand Down
4 changes: 2 additions & 2 deletions packages/amplify-gen2-codegen/src/renderers/package_json.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Renderer } from '../render_pipeline.js';

export class JsonRenderer implements Renderer {
constructor(private createJson: () => Record<string, unknown>, private writeFile: (content: string) => Promise<void>) {}
constructor(private createJson: () => Promise<Record<string, unknown>>, private writeFile: (content: string) => Promise<void>) {}

render = async (): Promise<void> => {
const packageJson = this.createJson();
const packageJson = await this.createJson();
await this.writeFile(JSON.stringify(packageJson, null, 2));
};
}
Loading