Skip to content

Commit

Permalink
fix(certificatemanager): remove bundled lambda devdependencies (#2186)
Browse files Browse the repository at this point in the history
Dev dependencies for the bundled Lambda were accidentally being
published to NPM because the node_modules directory is bootstrapped by
Lerna but not .npmignored.

Add ignore line and pkglint rule to prevent this from happening again.

(Also brings the package down from 16MB to 200kB)
  • Loading branch information
rix0rrr committed Apr 5, 2019
1 parent 66df1c8 commit 6728b41
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 10 deletions.
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-certificatemanager/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@ dist
# Include .jsii
!.jsii

*.snk
*.snk

# Don't pack node_modules directories of the lambdas in there. They should only be dev dependencies.
lambda-packages/dns_validated_certificate_handler/node_modules
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-certificatemanager/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,4 @@
"engines": {
"node": ">= 8.10.0"
}
}
}
4 changes: 4 additions & 0 deletions tools/pkglint/lib/packagejson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ export interface Report {
* Class representing a package.json file and the issues we found with it
*/
export class PackageJson {
public static fromDirectory(dir: string) {
return new PackageJson(path.join(dir, 'package.json'));
}

public readonly json: { [key: string]: any };
public readonly packageRoot: string;
public readonly packageName: string;
Expand Down
32 changes: 31 additions & 1 deletion tools/pkglint/lib/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path = require('path');
import semver = require('semver');
import { LICENSE, NOTICE } from './licensing';
import { PackageJson, ValidationRule } from './packagejson';
import { deepGet, deepSet, expectDevDependency, expectJSON, fileShouldBe, fileShouldContain, monoRepoVersion } from './util';
import { deepGet, deepSet, expectDevDependency, expectJSON, fileShouldBe, fileShouldContain, findInnerPackages, monoRepoVersion } from './util';

/**
* Verify that the package name matches the directory name
Expand Down Expand Up @@ -750,6 +750,36 @@ export class JestCoverageTarget extends ValidationRule {
}
}

/**
* Packages inside JSII packages (typically used for embedding Lambda handles)
* must only have dev dependencies and their node_modules must have been
* blacklisted for publishing
*
* We might loosen this at some point but we'll have to bundle all runtime dependencies
* and we don't have good transitive license checks.
*/
export class PackageInJsiiPackageNoRuntimeDeps extends ValidationRule {
public name = 'lambda-packages-no-runtime-deps';

public validate(pkg: PackageJson) {
if (!isJSII(pkg)) { return; }

for (const inner of findInnerPackages(pkg.packageRoot)) {
const innerPkg = PackageJson.fromDirectory(inner);

if (Object.keys(innerPkg.dependencies).length > 0) {
pkg.report({
ruleName: `${this.name}:1`,
message: `NPM Package '${innerPkg.packageName}' inside jsii package can only have devDepencencies`
});
}

const nodeModulesRelPath = path.relative(pkg.packageRoot, innerPkg.packageRoot) + '/node_modules';
fileShouldContain(`${this.name}:2`, pkg, '.npmignore', nodeModulesRelPath);
}
}
}

/**
* Determine whether this is a JSII package
*
Expand Down
32 changes: 25 additions & 7 deletions tools/pkglint/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,37 @@ export function monoRepoVersion() {
return lernaJson.version;
}

function findLernaJSON() {
let dir = process.cwd();
export function findUpward(dir: string, pred: (x: string) => boolean): string | undefined {
while (true) {
const fullPath = path.join(dir, 'lerna.json');
if (fs.existsSync(fullPath)) {
return fullPath;
}
if (pred(dir)) { return dir; }

const parent = path.dirname(dir);
if (parent === dir) {
throw new Error('Could not find lerna.json');
return undefined;
}

dir = parent;
}
}

function findLernaJSON() {
const ret = findUpward(process.cwd(), d => fs.existsSync(path.join(d, 'lerna.json')));
if (!ret) {
throw new Error('Could not find lerna.json');
}
return path.join(ret, 'lerna.json');
}

export function* findInnerPackages(dir: string): IterableIterator<string> {
for (const fname of fs.readdirSync(dir, { encoding: 'utf8' })) {
const stat = fs.statSync(path.join(dir, fname));
if (!stat.isDirectory()) { continue; }
if (fname === 'node_modules') { continue; }

if (fs.existsSync(path.join(dir, fname, 'package.json'))) {
yield path.join(dir, fname);
}

yield* findInnerPackages(path.join(dir, fname));
}
}

0 comments on commit 6728b41

Please sign in to comment.