Skip to content

Commit

Permalink
improvement(container): automatically set include field based on config
Browse files Browse the repository at this point in the history
This avoids common issues where multiple module sources overlap, or
Garden build contexts accidentally include a bunch of unnecessary files.

We parse the Dockerfile if the module has one and extract the paths
referenced in the ADD and COPY fields there. If there is no Dockerfile,
we automatically set include to an empty list.
  • Loading branch information
edvald committed Dec 10, 2019
1 parent 9acda7d commit 7cef10e
Show file tree
Hide file tree
Showing 8 changed files with 297 additions and 4 deletions.
22 changes: 22 additions & 0 deletions garden-service/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions garden-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"deline": "^1.0.4",
"dependency-graph": "^0.8.0",
"directory-tree": "^2.2.4",
"docker-file-parser": "^1.0.4",
"dockerode": "^3.0.2",
"dotenv": "^8.2.0",
"elegant-spinner": "^2.0.0",
Expand All @@ -68,6 +69,8 @@
"indent-string": "^4.0.0",
"inquirer": "^7.0.0",
"is-git-url": "^1.0.0",
"is-glob": "^4.0.1",
"is-url": "^1.2.4",
"js-yaml": "^3.13.1",
"json-diff": "^0.5.4",
"json-merge-patch": "^0.2.3",
Expand Down Expand Up @@ -140,6 +143,8 @@
"@types/has-ansi": "^3.0.0",
"@types/inquirer": "6.5.0",
"@types/is-git-url": "^1.0.0",
"@types/is-glob": "^4.0.1",
"@types/is-url": "^1.2.28",
"@types/js-yaml": "^3.12.1",
"@types/json-merge-patch": "0.0.4",
"@types/json-stringify-safe": "^5.0.0",
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,7 @@ export class ActionRouter implements TypeGuard {
pluginName,
})
}
return validate(result, schema, { context: `${actionType} output from plugin ${pluginName}` })
return validate(result, schema, { context: `${actionType} ${moduleType} output from provider ${pluginName}` })
}),
{ actionType, pluginName, moduleType }
)
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/config/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export const joi: Joi.Root = Joi.extend({

if (result.error) {
// tslint:disable-next-line:no-invalid-this
return this.createError("posixPath", { v: value }, state, prefs)
return this.createError("string.posixPath", { v: value }, state, prefs)
}

const options = params.options || {}
Expand Down
7 changes: 6 additions & 1 deletion garden-service/src/plugins/container/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const taskOutputsSchema = joi.object().keys({
),
})

export async function configureContainerModule({ ctx, moduleConfig }: ConfigureModuleParams<ContainerModule>) {
export async function configureContainerModule({ ctx, log, moduleConfig }: ConfigureModuleParams<ContainerModule>) {
// validate hot reload configuration
// TODO: validate this when validating this action's output
const hotReloadConfig = moduleConfig.spec.hotReload
Expand Down Expand Up @@ -150,6 +150,11 @@ export async function configureContainerModule({ ctx, moduleConfig }: ConfigureM
"deployment-image-name": deploymentImageName,
}

// Automatically set the include field based on the Dockerfile and config, if not explicitly set
if (!moduleConfig.include) {
moduleConfig.include = await containerHelpers.autoResolveIncludes(moduleConfig, log)
}

return { moduleConfig }
}

Expand Down
93 changes: 92 additions & 1 deletion garden-service/src/plugins/container/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,21 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { join } from "path"
import { join, posix } from "path"
import { readFile, pathExists, lstat } from "fs-extra"
import semver from "semver"
import { parse, CommandEntry } from "docker-file-parser"
import isGlob from "is-glob"
import { ConfigurationError, RuntimeError } from "../../exceptions"
import { splitFirst, spawn, splitLast } from "../../util/util"
import { ModuleConfig } from "../../config/module"
import { ContainerModule, ContainerRegistryConfig, defaultTag, defaultNamespace, ContainerModuleConfig } from "./config"
import { Writable } from "stream"
import Bluebird from "bluebird"
import { flatten, uniq } from "lodash"
import { LogEntry } from "../../logger/log-entry"
import chalk from "chalk"
import isUrl from "is-url"

export const DEFAULT_BUILD_TIMEOUT = 600
export const minDockerVersion = "17.07.0"
Expand Down Expand Up @@ -287,6 +295,89 @@ const helpers = {
getDockerfileSourcePath(config: ModuleConfig) {
return getDockerfilePath(config.path, config.spec.dockerfile)
},

/**
* Parses the Dockerfile in the module (if any) and returns a list of include patterns to apply to the module.
* Returns undefined if the whole module directory should be included, or if the Dockerfile cannot be parsed.
* Returns an empty list if there is no Dockerfile, and an `image` is set.
*/
async autoResolveIncludes(config: ContainerModuleConfig, log: LogEntry) {
const dockerfilePath = helpers.getDockerfileSourcePath(config)

if (!(await pathExists(dockerfilePath))) {
// No Dockerfile, nothing to build, return empty list
return []
}

const dockerfile = await readFile(dockerfilePath)
let commands: CommandEntry[] = []

try {
commands = parse(dockerfile.toString()).filter(
(cmd) => (cmd.name === "ADD" || cmd.name === "COPY") && cmd.args && cmd.args.length > 0
)
} catch (err) {
log.warn(chalk.yellow(`Unable to parse Dockerfile ${dockerfilePath}: ${err.message}`))
return undefined
}

const paths: string[] = uniq(
flatten(
commands.map((cmd) => {
const args = cmd.args as string[]
if (args[0].startsWith("--chown")) {
// Ignore --chown args
return args.slice(1, -1)
} else if (args[0].startsWith("--from")) {
// Skip statements copying from another build stage
return []
} else {
return args.slice(0, -1)
}
})
// Ignore URLs
).filter((path) => !isUrl(path))
)

for (const path of paths) {
if (path === ".") {
// If any path is "." we need the full build context
return undefined
} else if (path.match(/(?<!\\)(?:\\\\)*\$[{\w]/)) {
// If the path contains a template string we can't currently reason about it
// TODO: interpolate args into paths
return undefined
}
}

// Make sure to include the Dockerfile
paths.push(config.spec.dockerfile || "Dockerfile")

return Bluebird.map(paths, async (path) => {
const absPath = join(config.path, path)

// Unescape escaped template strings
path = path.replace(/\\\$/g, "$")

if (isGlob(path, { strict: false })) {
// Pass globs through directly
return path
} else if (await pathExists(absPath)) {
const stat = await lstat(absPath)

if (stat.isDirectory()) {
// If it's a directory, we want to match everything in the directory
return posix.join(path, "**", "*")
} else {
// If it's a file, pass it through as-is
return path
}
} else {
// Pass the file through directly if it can't be found (an error will occur in the build later)
return path
}
})
},
}

export const containerHelpers = helpers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ describe("plugins.container", () => {
build: { dependencies: [] },
apiVersion: "garden.io/v0",
name: "module-a",
include: ["Dockerfile"],
outputs: {
"local-image-name": "module-a",
"deployment-image-name": "module-a",
Expand Down

0 comments on commit 7cef10e

Please sign in to comment.