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: post-process cfn files for minification, consolidating concern of minification a bit #12072

Merged
merged 4 commits into from
Feb 22, 2023
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
13 changes: 11 additions & 2 deletions packages/amplify-e2e-core/src/init/amplifyPush.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,21 @@ export type LayerPushSettings = {
usePreviousPermissions?: boolean;
};

export type PushOpts = {
Copy link
Member

Choose a reason for hiding this comment

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

PushOptions would be much better.

minify?: boolean;
};

/**
* Function to test amplify push with verbose status
*/
export const amplifyPush = async (cwd: string, testingWithLatestCodebase = false): Promise<void> => {
export const amplifyPush = async (cwd: string, testingWithLatestCodebase = false, opts?: PushOpts): Promise<void> => {
// Test detailed status
await spawn(getCLIPath(testingWithLatestCodebase), ['status', '-v'], { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS })
.wait(/.*/)
.runAsync();
const pushArgs = ['push', ...(opts?.minify ? ['--minify'] : [])];
// Test amplify push
await spawn(getCLIPath(testingWithLatestCodebase), ['push'], { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS })
await spawn(getCLIPath(testingWithLatestCodebase), pushArgs, { cwd, stripColors: true, noOutputTimeout: pushTimeoutMS })
.wait('Are you sure you want to continue?')
.sendYes()
.wait('Do you want to generate code for your newly created GraphQL API')
Expand Down Expand Up @@ -155,11 +160,15 @@ export function amplifyPushUpdate(
testingWithLatestCodebase = false,
allowDestructiveUpdates = false,
overridePushTimeoutMS = 0,
minify?,
): Promise<void> {
const args = ['push'];
if (allowDestructiveUpdates) {
args.push('--allow-destructive-graphql-schema-updates');
}
if (minify) {
args.push('--minify');
}
return spawn(getCLIPath(testingWithLatestCodebase), args, {
cwd,
stripColors: true,
Expand Down
4 changes: 3 additions & 1 deletion packages/amplify-e2e-core/src/utils/projectMeta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ export const getAmplifyDirPath = (projectRoot: string): string => path.join(proj
// eslint-disable-next-line spellcheck/spell-checker
export const getAWSConfigIOSPath = (projectRoot: string): string => path.join(projectRoot, 'awsconfiguration.json');

export const getProjectMetaPath = (projectRoot: string) => path.join(projectRoot, 'amplify', '#current-cloud-backend', 'amplify-meta.json');

export const getProjectMeta = (projectRoot: string): $TSAny => {
const metaFilePath: string = path.join(projectRoot, 'amplify', '#current-cloud-backend', 'amplify-meta.json');
const metaFilePath: string = getProjectMetaPath(projectRoot);
return JSON.parse(fs.readFileSync(metaFilePath, 'utf8'));
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import {
amplifyPush,
deleteProject,
initJSProjectWithProfile,
createNewProjectDir,
deleteProjectDir,
generateRandomShortId,
getProjectMetaPath,
addApiWithBlankSchema,
updateApiSchema,
amplifyPushUpdate,
} from '@aws-amplify/amplify-e2e-core';
import fs from 'fs';
import path from 'path';

describe('minify behavior', () => {
let projRoot: string;
let projFolderName: string;

beforeEach(async () => {
projFolderName = `minify${generateRandomShortId()}`;
projRoot = await createNewProjectDir(projFolderName);
});

afterEach(async () => {
if (fs.existsSync(getProjectMetaPath(projRoot))) {
await deleteProject(projRoot);
}
deleteProjectDir(projRoot);
});

it('reduces file size when minify flag is provided', async () => {
const envName = 'devtest';
const projName = `minify${generateRandomShortId()}`;

// Configure and push app without minification
await initJSProjectWithProfile(projRoot, { name: projName, envName });
await addApiWithBlankSchema(projRoot);
updateApiSchema(projRoot, projName, 'simple_model.graphql', false);
await amplifyPush(projRoot, true, { minify: false });

// Read Cfn file sizes for both nested API stacks and top-level stacks
const currentCloudBackendPath = path.join(projRoot, 'amplify', '#current-cloud-backend');

const nestedApiStackPath = path.join(currentCloudBackendPath, 'api', projName, 'build', 'stacks', 'Todo.json');
const rootApiStackPath = path.join(
currentCloudBackendPath,
'awscloudformation',
'build',
'api',
projName,
'build',
'cloudformation-template.json',
);
const rootStackPath = path.join(currentCloudBackendPath, 'awscloudformation', 'build', 'root-cloudformation-stack.json');

const unminifiedNestedApiStackDefinition = fs.readFileSync(nestedApiStackPath, 'utf-8');
const unminifiedRootApiStackDefinition = fs.readFileSync(rootApiStackPath, 'utf-8');
const unminifiedRootStackDefinition = fs.readFileSync(rootStackPath, 'utf-8');

// Tweak schema file and push with minification
updateApiSchema(projRoot, projName, 'simple_model.graphql', true);
const pushParams = {
projRoot,
waitForText: undefined,
useLatestCodebase: true,
destructivePush: false,
overrideTimeout: 0,
minify: true,
};
await amplifyPushUpdate(
pushParams.projRoot,
pushParams.waitForText,
pushParams.waitForText,
pushParams.destructivePush,
pushParams.overrideTimeout,
pushParams.minify,
);

// Read Cfn file sizes for both nested API stacks and top-level stacks, verify files are smaller than initial push.
const minifiedNestedApiStackDefinition = fs.readFileSync(nestedApiStackPath, 'utf-8');
const minifiedRootApiStackDefinition = fs.readFileSync(rootApiStackPath, 'utf-8');
const minifiedRootStackDefinition = fs.readFileSync(rootStackPath, 'utf-8');

expect(minifiedNestedApiStackDefinition.length).toBeLessThan(unminifiedNestedApiStackDefinition.length);
expect(minifiedRootApiStackDefinition.length).toBeLessThan(unminifiedRootApiStackDefinition.length);
expect(minifiedRootStackDefinition.length).toBeLessThan(unminifiedRootStackDefinition.length);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { minifyJSONFile, minifyAllJSONInFolderRecursively } from '../../utils/minify-json';
import path from 'path';
import fs from 'fs-extra';
import os from 'os';

const JSON_STRING = `{
"city": "Seattle",
"state": "WA"
}`;

const NON_JSON_STRING = `CITY = Seattle
STATE = WA
`;

describe('minifyJSONFile', () => {
const writeFileMinifyAndReturnReadContents = (filePath: string, contents: string): string => {
const tmpFilePath = path.join(os.tmpdir(), filePath);
fs.writeFileSync(tmpFilePath, contents);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can this warning be fixed?

minifyJSONFile(tmpFilePath);
return fs.readFileSync(tmpFilePath, 'utf-8');
};

it('minifies JSON files', () => {
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.json', JSON_STRING);
expect(minifiedContents.length).toBeLessThan(JSON_STRING.length);
});

it('does not change a non-json file with json contents', () => {
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.dat', JSON_STRING);
expect(minifiedContents.length).toEqual(JSON_STRING.length);
});

it('does not change a non-json file with non-json contents', () => {
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.env', NON_JSON_STRING);
expect(minifiedContents.length).toEqual(NON_JSON_STRING.length);
});
});

describe('minifyAllJSONInFolderRecursively', () => {
it('will minify json files, but leave non-json files alone', () => {
// This test will set up a directory structure as follows
// .
// ├── file.json
// ├── config.env
// └── nested_stacks
// ├── nested.env
// ├── nested_1.json
// ├── nested_2.json
// └── sub_nested_stacks
// ├── sub_nested.env
// └── sub_nested.json
// We will then ensure that after a single invocation file.json, nested_1.json, nested_2.json, and sub_nested.json are all minified
// and that config.env is the same length.
const testDirectory = path.join(os.tmpdir(), 'minifyTestDir');
const nestedStacksDirectory = path.join(testDirectory, 'nested_stacks');
const subNestedStacksDirectory = path.join(nestedStacksDirectory, 'sub_nested_stacks');

// Clean and setup directory structure
fs.removeSync(testDirectory);
fs.mkdirSync(testDirectory);
fs.mkdirSync(nestedStacksDirectory);
fs.mkdirSync(subNestedStacksDirectory);

// Write files
fs.writeFileSync(path.join(testDirectory, 'file.json'), JSON_STRING);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).
fs.writeFileSync(path.join(testDirectory, 'config.env'), NON_JSON_STRING);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested_1.json'), JSON_STRING);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested_2.json'), JSON_STRING);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested.env'), NON_JSON_STRING);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).
fs.writeFileSync(path.join(subNestedStacksDirectory, 'sub_nested.json'), JSON_STRING);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).
fs.writeFileSync(path.join(subNestedStacksDirectory, 'sub_nested.env'), NON_JSON_STRING);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1).

// Apply recursive minification
minifyAllJSONInFolderRecursively(testDirectory);

// Verify all `.json` files are minified, and `.env` files are not.
expect(fs.readFileSync(path.join(testDirectory, 'file.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(testDirectory, 'config.env')).length).toEqual(NON_JSON_STRING.length);
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested_1.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested_2.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested.env')).length).toEqual(NON_JSON_STRING.length);
expect(fs.readFileSync(path.join(subNestedStacksDirectory, 'sub_nested.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(subNestedStacksDirectory, 'sub_nested.env')).length).toEqual(NON_JSON_STRING.length);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ import { prePushTemplateDescriptionHandler } from './template-description-utils'
import { buildOverridesEnabledResources } from './build-override-enabled-resources';

import { invokePostPushAnalyticsUpdate } from './plugin-client-api-analytics';
import { minifyJSONFile } from './utils/minify-json';

const logger = fileLogger('push-resources');

Expand Down Expand Up @@ -767,6 +768,9 @@ export const uploadTemplateToS3 = async (
amplifyMeta: $TSMeta,
): Promise<void> => {
const cfnFile = path.parse(filePath).base;
if (context.input.options?.minify) {
alharris-at marked this conversation as resolved.
Show resolved Hide resolved
minifyJSONFile(filePath);
}
const s3 = await S3.getInstance(context);

const s3Params = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const path = require('path');
const TransformPackage = require('graphql-transformer-core');
const { S3 } = require('./aws-utils/aws-s3');
const { fileLogger } = require('./utils/aws-logger');
const { minifyAllJSONInFolderRecursively } = require('./utils/minify-json');

const logger = fileLogger('upload-appsync-files');

const ROOT_APPSYNC_S3_KEY = 'amplify-appsync-files';
Expand Down Expand Up @@ -189,6 +191,9 @@ async function uploadAppSyncFiles(context, resourcesToUpdate, allResources, opti
if (!fs.existsSync(resourceBuildDir)) {
return;
}
if (context.input.options?.minify) {
minifyAllJSONInFolderRecursively(resourceBuildDir);
}
const spinner = new ora('Uploading files.');
spinner.start();
await TransformPackage.uploadAPIProject({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as fs from 'fs-extra';
import * as path from 'path';

/**
* Given a path to a json file, minify that file by reading in as JSON
* and writing back out without any whitespace.
* @param jsonFilePath the path to the JSON file we are going to minify.
* @returns when the file has been minified and written back to disk.
*/
export const minifyJSONFile = (jsonFilePath: string): void => {
if (!jsonFilePath.includes('.json')) return; // Don't convert files not ending in `.json`
const originalJSON = fs.readFileSync(jsonFilePath, 'utf-8');
const minifiedJSON = JSON.stringify(JSON.parse(originalJSON));
fs.writeFileSync(jsonFilePath, minifiedJSON);

Check failure

Code scanning / CodeQL

Insecure temporary file

Insecure creation of file in [the os temp dir](1). Insecure creation of file in [the os temp dir](2).
};

/**
* Recursively walk a folder, and minify (print without whitespace) all the json files you discover
* @param rootPath the top of the tree to walk
*/
export const minifyAllJSONInFolderRecursively = (rootPath: string): void => {
fs.readdirSync(rootPath).forEach((childHandle) => {
const childPath = path.join(rootPath, childHandle);
if (fs.lstatSync(childPath).isDirectory()) minifyAllJSONInFolderRecursively(childPath);
if (fs.lstatSync(childPath).isFile() && childPath.includes('.json')) minifyJSONFile(childPath);
});
};