From 7d9e29b76145ec70e045ae8505b99b85021c39a4 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 5 Apr 2019 16:15:12 +0200 Subject: [PATCH] fix(certificatemanager): remove bundled lambda devdependencies Dev dependencies for the bundled Lambda were accidentally being published to NPM because the `node_modules` directory is bootstrapped by Lerna but not `.npmignore`d. Add ignore line and pkglint rule to prevent this from happening again. --- .../aws-certificatemanager/.npmignore | 5 ++- .../aws-certificatemanager/package.json | 2 +- tools/pkglint/lib/packagejson.ts | 4 +++ tools/pkglint/lib/rules.ts | 32 ++++++++++++++++++- tools/pkglint/lib/util.ts | 32 +++++++++++++++---- 5 files changed, 65 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/aws-certificatemanager/.npmignore b/packages/@aws-cdk/aws-certificatemanager/.npmignore index b757d55c46996..41a9c0cf03dfa 100644 --- a/packages/@aws-cdk/aws-certificatemanager/.npmignore +++ b/packages/@aws-cdk/aws-certificatemanager/.npmignore @@ -13,4 +13,7 @@ dist # Include .jsii !.jsii -*.snk \ No newline at end of file +*.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 diff --git a/packages/@aws-cdk/aws-certificatemanager/package.json b/packages/@aws-cdk/aws-certificatemanager/package.json index cd3b56b7aaca4..7663a0428fb72 100644 --- a/packages/@aws-cdk/aws-certificatemanager/package.json +++ b/packages/@aws-cdk/aws-certificatemanager/package.json @@ -81,4 +81,4 @@ "engines": { "node": ">= 8.10.0" } -} \ No newline at end of file +} diff --git a/tools/pkglint/lib/packagejson.ts b/tools/pkglint/lib/packagejson.ts index 98cdb3da06a10..8dfa4561a9b5d 100644 --- a/tools/pkglint/lib/packagejson.ts +++ b/tools/pkglint/lib/packagejson.ts @@ -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; diff --git a/tools/pkglint/lib/rules.ts b/tools/pkglint/lib/rules.ts index b251a10d1ac9c..63d7502c20d00 100644 --- a/tools/pkglint/lib/rules.ts +++ b/tools/pkglint/lib/rules.ts @@ -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 @@ -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 * diff --git a/tools/pkglint/lib/util.ts b/tools/pkglint/lib/util.ts index b2cd1715c3464..fe96d8903a046 100644 --- a/tools/pkglint/lib/util.ts +++ b/tools/pkglint/lib/util.ts @@ -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 { + 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)); + } +}