Skip to content
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
15 changes: 9 additions & 6 deletions packages/cdktn/src/asset-staging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
IAssetPackaging,
} from "./assets";
import {
assetHashConflictingExcludeOptions,
assetHashConflictingHashType,
assetHashInvalid,
assetHashTypeCustomRequiresHash,
Expand All @@ -24,6 +23,7 @@ import {
assetStagingBundlerFileOutputNeedsFilePackaging,
assetStagingBundlerOutputNotDirectory,
assetStagingBundlerOutputNotFile,
assetStagingConflictingExcludeOptions,
} from "./errors";
import { CANONICAL_ASSET_HASHES } from "./features";
import { ExcludeIgnoreStrategy, IIgnoreStrategy } from "./ignore-strategy";
Expand Down Expand Up @@ -239,7 +239,7 @@ export class AssetStaging extends Construct implements IAsset {
this.bundler = props.bundler;

if (props.exclude?.length && props.ignoreStrategy) {
throw assetHashConflictingExcludeOptions();
throw assetStagingConflictingExcludeOptions(this.displayName);
}
this.ignoreStrategy =
props.ignoreStrategy ?? new ExcludeIgnoreStrategy(props.exclude ?? []);
Expand Down Expand Up @@ -469,13 +469,16 @@ export class AssetStaging extends Construct implements IAsset {
/**
* Absolute path to the source the bundler should read.
*
* With no exclusions the resolved source is handed over directly. Otherwise
* the excluded source tree is materialised into the scratch directory, so
* the bundler sees the same file set the hash was taken over.
* With no exclusions, or with a file source, the resolved source is handed
* over directly — a file has no children to filter, matching `hashSource`,
* which only ever applies the ignore strategy to a directory's entries and
* never to the root itself. Otherwise the excluded source tree is
* materialised into the scratch directory, so the bundler sees the same
* file set the hash was taken over.
*/
private filteredSource(scratch: string): string {
const absoluteSource = path.resolve(this.sourcePath);
if (!this.hasExclusions) {
if (!this.hasExclusions || fs.statSync(absoluteSource).isFile()) {
return absoluteSource;
}
const input = path.join(scratch, "input");
Expand Down
6 changes: 6 additions & 0 deletions packages/cdktn/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ export const assetHashConflictingExcludeOptions = () =>
`Both 'exclude' and 'ignoreStrategy' were passed to AssetHash.of(), but 'ignoreStrategy' replaces 'exclude' rather than combining with it. Pass only one.`,
);

export const assetStagingConflictingExcludeOptions = (id: string) =>
new Error(
`TerraformAsset ${id} was configured with both 'exclude' and 'ignoreStrategy', but 'ignoreStrategy' replaces 'exclude' rather than combining with it. Pass only one.
Learn more about TerraformAsset: https://cdktn.io/docs/concepts/assets`,
);

export const assetHashTypeCustomRequiresHash = (id: string) =>
new Error(
`TerraformAsset ${id} was configured with assetHashType 'CUSTOM' but no 'assetHash'. A custom hash type requires an explicit 'assetHash' value.
Expand Down
20 changes: 19 additions & 1 deletion packages/cdktn/src/terraform-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
IAssetPackaging,
} from "./assets";
import { AssetStaging } from "./asset-staging";
import { IIgnoreStrategy } from "./ignore-strategy";
import { findFileAboveCwd } from "./private/fs";
import { ISynthesisSession } from "./synthesize";
import { addCustomSynthesis } from "./synthesize/synthesizer";
Expand Down Expand Up @@ -47,12 +48,28 @@ export interface TerraformAssetConfig {
/**
* Paths to exclude from the asset, relative to `path`. See
* `AssetStagingOptions.exclude` for the accepted forms. Both the computed
* hash and the staged/packed content honor the exclusion.
* hash and the staged/packed content honor the exclusion. Cannot be
* combined with `ignoreStrategy`, which replaces this matcher rather than
* layering on top of it.
*
* @default - nothing is excluded
*/
readonly exclude?: string[];

/**
* Exclusion matching, for callers that need `.gitignore` / `.dockerignore`
* parity (including `!`-negation) rather than the built-in exact-path /
Comment thread
eduardomourar marked this conversation as resolved.
Resolved
* suffix / directory matcher. See `AssetStagingOptions.ignoreStrategy`.
*
* `!`-negation only takes effect if the strategy also sets
* `pruneExcludedDirectories: false`; otherwise an excluded directory is
* pruned before its contents are ever checked against a re-include
* pattern. See `IIgnoreStrategy.pruneExcludedDirectories`.
*
* @default - `exclude` is used with the built-in matcher
*/
readonly ignoreStrategy?: IIgnoreStrategy;

/**
* Extra information to fold into the hash (e.g. build instructions and
* other inputs).
Expand Down Expand Up @@ -180,6 +197,7 @@ export class TerraformAsset extends Construct implements IAsset {
assetHash: config.assetHash,
assetHashType: config.assetHashType,
exclude: config.exclude,
ignoreStrategy: config.ignoreStrategy,
Comment thread
eduardomourar marked this conversation as resolved.
Resolved
extraHash: config.extraHash,
bundler: config.bundler,
});
Expand Down
105 changes: 105 additions & 0 deletions packages/cdktn/test/canonical-asset-hash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,77 @@ describe("TerraformAsset with exclude/extraHash (AssetStaging integration)", ()

expect(asset.assetHash).toBe("my-custom-hash");
});

test("ignoreStrategy negation is honored by hashing and staged/packed output", () => {
// A plain `relativePath === "b.md"` exclusion is expressible with
// `exclude` already; the point of `ignoreStrategy` is `!`-negation,
// which only takes effect with `pruneExcludedDirectories: false` (see
// `IIgnoreStrategy.pruneExcludedDirectories`) -- without it the excluded
// directory is pruned and the re-include is never evaluated.
fs.mkdirSync(path.join(srcDir, "node_modules"));
fs.writeFileSync(path.join(srcDir, "node_modules", "junk.js"), "junk");
fs.writeFileSync(path.join(srcDir, "node_modules", "keep.js"), "keep");

const s = stack();
const asset = new TerraformAsset(s, "asset", {
path: srcDir,
type: AssetType.DIRECTORY,
ignoreStrategy: {
pruneExcludedDirectories: false,
ignores: ({ relativePath }) =>
(relativePath === "node_modules" ||
relativePath.startsWith("node_modules/")) &&
relativePath !== "node_modules/keep.js",
},
});
// Otherwise identical, but without the `keep.js` re-include: excludes
// all of `node_modules`. If the hash didn't actually account for
// `keep.js`, this would collide with `asset`'s hash.
const withoutReinclude = new TerraformAsset(s, "asset2", {
path: srcDir,
type: AssetType.DIRECTORY,
ignoreStrategy: {
pruneExcludedDirectories: false,
ignores: ({ relativePath }) =>
relativePath === "node_modules" ||
relativePath.startsWith("node_modules/"),
},
});

expect(asset.assetHash).not.toEqual(withoutReinclude.assetHash);

const outdir = Testing.fullSynth(s);
const stagedDir = path.join(outdir, "stacks", s.node.id, asset.path);
expect(fs.existsSync(path.join(stagedDir, "a.txt"))).toBe(true);
expect(fs.existsSync(path.join(stagedDir, "b.md"))).toBe(true);
expect(fs.existsSync(path.join(stagedDir, "node_modules", "junk.js"))).toBe(
false,
);
expect(fs.existsSync(path.join(stagedDir, "node_modules", "keep.js"))).toBe(
true,
);
});

test("exclude and ignoreStrategy cannot be combined", () => {
Comment thread
eduardomourar marked this conversation as resolved.
Resolved
let error: Error | undefined;
try {
new TerraformAsset(stack(), "asset", {
path: srcDir,
type: AssetType.DIRECTORY,
exclude: ["*.md"],
ignoreStrategy: { ignores: () => false },
});
} catch (e) {
error = e as Error;
}

// Names the TerraformAsset, not `AssetHash.of()`, which this path never
// calls into.
expect(error?.message).toMatch(
/TerraformAsset.*asset.*exclude.*ignoreStrategy/is,
);
expect(error?.message).not.toContain("AssetHash.of()");
});
});

describe("TerraformAsset artifact layout derives from the packaging", () => {
Expand Down Expand Up @@ -757,6 +828,40 @@ describe("TerraformAsset artifact layout derives from the packaging", () => {
}),
).toThrow(/AssetType\.DIRECTORY|AssetType\.ARCHIVE|single file/i);
});

// Regression: `filteredSource` materialised exclusions by `copySync`-ing
// the source into a scratch dir, which assumes a directory root and threw
// ENOTDIR against a file source. Hashing already special-cases a file
// root (there is nothing under it to filter); staging needs to agree, so
// a bundler for a file source still gets handed the file itself.
test.each([
// `hasExclusions` is true for any defined `ignoreStrategy`, unlike
// `exclude`, which only triggers it with at least one entry -- both
// reach `filteredSource` and must be covered.
["a no-op ignoreStrategy", { ignoreStrategy: { ignores: () => false } }],
["a non-empty exclude", { exclude: ["x"] }],
])(
"a file source with %s and a bundler stages without crashing",
(_label, extra) => {
const s = stack();
const asset = new TerraformAsset(s, "asset", {
path: srcFile,
type: AssetType.FILE,
...extra,
bundler: {
bundle: (opts) => {
const built = path.join(opts.outputDir, "built.bin");
fs.writeFileSync(built, "bundled");
return BundleResult.file(built);
},
},
});

const outdir = Testing.fullSynth(s);
const stagedFile = path.join(outdir, "stacks", s.node.id, asset.path);
expect(fs.readFileSync(stagedFile, "utf-8")).toBe("bundled");
},
);
});

describe("TerraformAsset stages inside the stack's own directory (#380)", () => {
Expand Down
Loading