Skip to content

Commit

Permalink
feat: add jib provider and jib-container module type
Browse files Browse the repository at this point in the history
By popular demand, this adds a proper integration for
[Jib](https://github.com/GoogleContainerTools/jib), which makes building
Java container images much more efficient.

The user-facing implementation is described in the added docs, but here
are some technical points for the reviewer(s):

1. We followed Skaffold's example in most ways, in terms of how to
   interact with Jib.
2. The `jib-container` module extends the `container` module type using
   our handy inheritance mechanisms. It behaves the exact same way
   outside of the build flow, and adds just a handful of fields to
   control the image build.
3. The `kubernetes` provider has explicit support for the module type,
   in order to avoid requiring a local Docker instance when using remote
   building. This actually prompted the most complexity in the
   integration, but I figure that makes the feature much more usable for
   many users. Even then it just comes down to one plugin handler.

The included `jib-container` example project is a good place to start
in terms of testing and poking around. Also check out the added
reference docs.
  • Loading branch information
edvald authored and thsig committed Sep 13, 2021
1 parent 20babc2 commit e453d70
Show file tree
Hide file tree
Showing 189 changed files with 7,137 additions and 1,091 deletions.
6 changes: 6 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,12 @@ workflows:
name: e2e-hot-reload
project: hot-reload
requires: [build]
- e2e-project:
<<: *only-internal-prs
name: e2e-jib-container
project: jib-container
environment: remote
requires: [build]
- e2e-project:
<<: *only-internal-prs
name: e2e-openfaas
Expand Down
2 changes: 2 additions & 0 deletions cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
"@garden-io/garden-conftest": "*",
"@garden-io/garden-conftest-container": "*",
"@garden-io/garden-conftest-kubernetes": "*",
"@garden-io/garden-jib": "*",
"@garden-io/garden-maven-container": "*",
"chalk": "^4.1.0"
},
"devDependencies": {
Expand Down
2 changes: 2 additions & 0 deletions cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ export const getBundledPlugins = (): GardenPluginCallback[] =>
require("@garden-io/garden-conftest"),
require("@garden-io/garden-conftest-container"),
require("@garden-io/garden-conftest-kubernetes"),
require("@garden-io/garden-jib"),
require("@garden-io/garden-maven-container"),
].map((m) => () => m.gardenPlugin())

export async function runCli({
Expand Down
2 changes: 2 additions & 0 deletions cli/test/test-projects/bundled-projects/project.garden.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ providers:
- name: conftest
- name: conftest-container
- name: conftest-kubernetes
- name: jib
- name: maven-container
6 changes: 6 additions & 0 deletions cli/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
},
{
"path": "../plugins/conftest-kubernetes"
},
{
"path": "../plugins/jib"
},
{
"path": "../plugins/maven-container"
}
]
}
12 changes: 6 additions & 6 deletions core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -248,18 +248,18 @@
"typescript": "^4.3.5"
},
"scripts": {
"_integ": "mocha --opts test/mocha.integ.opts",
"build": "gulp pegjs",
"check-package-lock": "git diff-index --quiet HEAD -- yarn.lock || (echo 'yarn.lock is dirty!' && exit 1)",
"clean": "shx rm -rf build",
"dev": "gulp pegjs-watch",
"fix-format": "prettier --write \"{src,test}/**/*.ts\"",
"lint": "tslint -p .",
"migration:generate": "typeorm migration:generate --config ormconfig.js -n",
"integ-kind": "GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"cluster-docker cluster-buildkit cluster-buildkit-rootless kaniko remote-only\" yarn run _integ",
"integ-local": "GARDEN_LOGGER_TYPE=basic GARDEN_LOG_LEVEL=debug GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"cluster-docker\" yarn run _integ",
"integ-minikube": "GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"cluster-docker kaniko remote-only\" yarn run _integ",
"integ-remote": "GARDEN_INTEG_TEST_MODE=remote GARDEN_SKIP_TESTS=local-only yarn run _integ",
"_integ": "mocha --opts test/mocha.integ.opts",
"integ-kind": "GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"cluster-docker cluster-buildkit cluster-buildkit-rootless kaniko remote-only\" yarn _integ",
"integ-local": "GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"cluster-docker remote-only\" yarn _integ",
"integ-minikube": "GARDEN_INTEG_TEST_MODE=local GARDEN_SKIP_TESTS=\"cluster-docker kaniko remote-only\" yarn _integ",
"integ-remote": "GARDEN_INTEG_TEST_MODE=remote GARDEN_SKIP_TESTS=local-only yarn _integ",
"e2e": "cd test/e2e && ../../../bin/garden test",
"e2e-project": "node build/test/e2e/e2e-project.js",
"test": "mocha --opts test/mocha.opts"
Expand All @@ -271,4 +271,4 @@
]
},
"gitHead": "b0647221a4d2ff06952bae58000b104215aed922"
}
}
15 changes: 9 additions & 6 deletions core/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1191,7 +1191,7 @@ export class ActionRouter implements TypeGuard {
async (...args: any[]) => {
const result = await handler.apply(plugin, args)
if (result === undefined) {
throw new PluginError(`Got empty response from ${actionType} handler on ${pluginName}`, {
throw new PluginError(`Got empty response from ${actionType} handler on ${pluginName} provider`, {
args,
actionType,
pluginName,
Expand Down Expand Up @@ -1223,11 +1223,14 @@ export class ActionRouter implements TypeGuard {
<ModuleActionHandlers[T]>(async (...args: any[]) => {
const result = await handler.apply(plugin, args)
if (result === undefined) {
throw new PluginError(`Got empty response from ${moduleType}.${actionType} handler on ${pluginName}`, {
args,
actionType,
pluginName,
})
throw new PluginError(
`Got empty response from ${moduleType}.${actionType} handler on ${pluginName} provider`,
{
args,
actionType,
pluginName,
}
)
}
return validateSchema(result, schema, {
context: `${actionType} ${moduleType} output from provider ${pluginName}`,
Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/run/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import chalk from "chalk"
import { cloneDeep, flatten, last, merge, repeat, size } from "lodash"
import { printHeader, getTerminalWidth, formatGardenError, renderMessageWithDivider } from "../../logger/util"
import { printHeader, getTerminalWidth, formatGardenErrorWithDetail, renderMessageWithDivider } from "../../logger/util"
import { Command, CommandParams, CommandResult } from "../base"
import { dedent, wordWrap, deline } from "../../util/string"
import { Garden } from "../../garden"
Expand Down Expand Up @@ -435,7 +435,7 @@ export function logErrors(
} else {
// Error comes from a command step. We only log the detail here (and only for log.debug or higher), since
// the task graph's error logging takes care of the rest.
const taskDetailErrMsg = formatGardenError(error)
const taskDetailErrMsg = formatGardenErrorWithDetail(error)
log.debug(chalk.red(taskDetailErrMsg))
}
}
Expand Down
27 changes: 27 additions & 0 deletions core/src/config/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,30 @@ export const apiVersionSchema = () =>
.default(DEFAULT_API_VERSION)
.valid(DEFAULT_API_VERSION)
.description("The schema version of this config (currently not used).")

/**
* A little hack to allow unknown fields on the schema and recursively on all object schemas nested in it.
* Used when e.g. validating against the schema of a module type base (in which case we want to allow added fields
* in the inheriting schema).
*/
export function allowUnknown<T extends Joi.Schema>(schema: T) {
schema = cloneDeep(schema)

if (schema["type"] === "object") {
schema["_flags"].unknown = true

for (const key of schema["$_terms"].keys || []) {
key.schema = allowUnknown(key.schema)
}
} else if (schema["type"] === "array" || schema["type"] === "sparseArray") {
const terms = schema["$_terms"]
if (terms.items) {
terms.items = terms.items.map((item: Joi.Schema) => allowUnknown(item))
}
if (terms._inclusions) {
terms._inclusions = terms._inclusions.map((item: Joi.Schema) => allowUnknown(item))
}
}

return schema
}
10 changes: 9 additions & 1 deletion core/src/config/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { TaskConfig, taskConfigSchema } from "./task"
import { dedent, stableStringify } from "../util/string"
import { templateKind } from "./module-template"

export const defaultBuildTimeout = 1200

export interface BuildCopySpec {
source: string
target: string
Expand Down Expand Up @@ -63,6 +65,7 @@ export const buildDependencySchema = () =>

export interface BaseBuildSpec {
dependencies: BuildDependencyConfig[]
timeout?: number
}

export interface ModuleFileSpec {
Expand Down Expand Up @@ -137,6 +140,11 @@ export const baseBuildSpecSchema = () =>
dependencies: joiSparseArray(buildDependencySchema())
.description("A list of modules that must be built before this module is built.")
.example([{ name: "some-other-module-name" }]),
timeout: joi
.number()
.integer()
.default(defaultBuildTimeout)
.description("Maximum time in seconds to wait for build to finish."),
})
.default(() => ({ dependencies: [] }))
.description("Specify how to build the module. Note that plugins may define additional keys on this object.")
Expand Down Expand Up @@ -223,7 +231,7 @@ export interface ModuleConfig<M extends {} = any, S extends {} = any, T extends
path: string
configPath?: string
plugin?: string // used to identify modules that are bundled as part of a plugin
buildConfig?: object
buildConfig?: any
serviceConfigs: ServiceConfig<S>[]
testConfigs: TestConfig<T>[]
taskConfigs: TaskConfig<W>[]
Expand Down
2 changes: 1 addition & 1 deletion core/src/config/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export function providerFromConfig({
*/
export async function getAllProviderDependencyNames(plugin: GardenPlugin, config: GenericProviderConfig) {
return uniq([
...(plugin.dependencies || []),
...(plugin.dependencies || []).map((d) => d.name),
...(config.dependencies || []),
...(await getProviderTemplateReferences(config)),
]).sort()
Expand Down
1 change: 1 addition & 0 deletions core/src/docs/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export async function writeConfigReferenceDocs(docsRoot: string, plugins: Garden
{ name: "container" },
{ name: "exec" },
{ name: "hadolint" },
{ name: "jib" },
{ name: "kubernetes" },
{ name: "local-kubernetes" },
{ name: "maven-container" },
Expand Down
1 change: 1 addition & 0 deletions core/src/docs/module-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const moduleTypes = [
{ name: "conftest", pluginName: "conftest" },
{ name: "hadolint" },
{ name: "helm", pluginName: "local-kubernetes" },
{ name: "jib-container" },
{ name: "kubernetes", pluginName: "local-kubernetes" },
{ name: "maven-container" },
{ name: "openfaas" },
Expand Down
4 changes: 2 additions & 2 deletions core/src/logger/renderers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import hasAnsi = require("has-ansi")
import { LogEntry, LogEntryMessage } from "./log-entry"
import { JsonLogEntry } from "./writers/json-terminal-writer"
import { highlightYaml, PickFromUnion, safeDumpYaml } from "../util/util"
import { printEmoji, formatGardenError } from "./util"
import { printEmoji, formatGardenErrorWithDetail } from "./util"
import { LoggerType, Logger } from "./logger"

type RenderFn = (entry: LogEntry) => string
Expand Down Expand Up @@ -82,7 +82,7 @@ export function renderEmoji(entry: LogEntry): string {
export function renderError(entry: LogEntry) {
const { errorData: error } = entry
if (error) {
return formatGardenError(error)
return formatGardenErrorWithDetail(error)
}

const msg = chainMessages(entry.getMessages() || [])
Expand Down
16 changes: 11 additions & 5 deletions core/src/logger/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { LogLevel } from "./logger"
import { Logger } from "./logger"
import { LogEntry, LogEntryParams, EmojiName } from "./log-entry"
import { deepMap, deepFilter, safeDumpYaml } from "../util/util"
import { padEnd, isEmpty } from "lodash"
import { padEnd, isEmpty, isPlainObject } from "lodash"
import { dedent } from "../util/string"
import hasAnsi from "has-ansi"
import { GardenError } from "../exceptions"
Expand Down Expand Up @@ -230,9 +230,9 @@ export function renderMessageWithDivider(prefix: string, msg: string, isError: b
`
}

export function formatGardenError(error: GardenError) {
const { detail, message } = error
let out = message || ""
export function formatGardenErrorWithDetail(error: GardenError) {
const { detail, message, stack } = error
let out = stack || message || ""

// We recursively filter out internal fields (i.e. having names starting with _).
const filteredDetail = deepFilter(sanitizeObject(detail), (_val, key: string | number) => {
Expand Down Expand Up @@ -260,5 +260,11 @@ export function sanitizeObject(obj: any) {
obj = deepMap(obj, (value: any) => {
return Buffer.isBuffer(value) ? "<Buffer>" : value
})
return JSON.parse(CircularJSON.stringify(obj))

const cleanedJson = isPlainObject(obj)
? CircularJSON.stringify(obj)
: // Handle classes, e.g. Error objects
CircularJSON.stringify(obj, Object.getOwnPropertyNames(obj))

return JSON.parse(cleanedJson)
}
59 changes: 34 additions & 25 deletions core/src/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import {
} from "./types/plugin/plugin"
import { GenericProviderConfig } from "./config/provider"
import { ConfigurationError, PluginError, RuntimeError } from "./exceptions"
import { uniq, mapValues, fromPairs, flatten, keyBy, some, isString, isFunction } from "lodash"
import { findByName, pushToKey, getNames } from "./util/util"
import { uniq, mapValues, fromPairs, flatten, keyBy, some, isString, isFunction, sortBy } from "lodash"
import { findByName, pushToKey, getNames, isNotNull } from "./util/util"
import { deline } from "./util/string"
import { validateSchema } from "./config/validation"
import { LogEntry } from "./logger/log-entry"
Expand Down Expand Up @@ -78,9 +78,9 @@ export async function loadPlugins(
}

for (const dep of plugin.dependencies || []) {
const depPlugin = validatePlugin(dep)
const depPlugin = validatePlugin(dep.name)

if (!depPlugin) {
if (!depPlugin && !dep.optional) {
throw new PluginError(
`Plugin '${plugin.name}' lists plugin '${dep}' as a dependency, but that plugin has not been registered.`,
{ loadedPlugins: Object.keys(loadedPlugins), dependency: dep }
Expand Down Expand Up @@ -177,9 +177,9 @@ export function getDependencyOrder(loadedPlugins: PluginMap): string[] {
graph.addDependency(plugin.name, plugin.base)
}

for (const dependency of plugin.dependencies || []) {
graph.addNode(dependency)
graph.addDependency(plugin.name, dependency)
for (const dep of plugin.dependencies || []) {
graph.addNode(dep.name)
graph.addDependency(plugin.name, dep.name)
}
}

Expand Down Expand Up @@ -211,8 +211,21 @@ function resolvePlugin(plugin: GardenPlugin, loadedPlugins: PluginMap, configs:
...plugin,
}

// Merge dependencies with base
resolved.dependencies = uniq([...(plugin.dependencies || []), ...(base.dependencies || [])]).sort()
// Merge dependencies with base and sort
resolved.dependencies = []

for (const dep of [...(plugin.dependencies || []), ...(base.dependencies || [])]) {
const duplicate = resolved.dependencies.find((d) => d.name === dep.name)
if (duplicate) {
if (!dep.optional) {
duplicate.optional = false
}
} else {
resolved.dependencies.push(dep)
}
}

resolved.dependencies = sortBy(resolved.dependencies, "name")

// Merge plugin handlers
resolved.handlers = { ...(plugin.handlers || {}) }
Expand Down Expand Up @@ -342,12 +355,15 @@ export function getModuleTypeBases(
export function getPluginDependencies(plugin: GardenPlugin, loadedPlugins: PluginMap): GardenPlugin[] {
return uniq(
flatten(
(plugin.dependencies || []).map((depName) => {
const depPlugin = loadedPlugins[depName]
if (!depPlugin) {
throw new RuntimeError(`Unable to find dependency '${depName} for plugin '${plugin.name}'`, { plugin })
(plugin.dependencies || []).map((dep) => {
const depPlugin = loadedPlugins[dep.name]
if (depPlugin) {
return [depPlugin, ...getPluginDependencies(depPlugin, loadedPlugins)]
} else if (dep.optional) {
return []
} else {
throw new RuntimeError(`Unable to find dependency '${dep.name} for plugin '${plugin.name}'`, { plugin })
}
return [depPlugin, ...getPluginDependencies(depPlugin, loadedPlugins)]
})
)
)
Expand Down Expand Up @@ -448,16 +464,9 @@ function resolveModuleDefinitions(resolvedPlugins: PluginMap, configs: GenericPr
const moduleType = spec.name
const definition = moduleDefinitions[moduleType]

// Make sure plugins that extend module types correctly declare their dependencies
if (!definition) {
throw new PluginError(
deline`
Plugin '${plugin.name}' extends module type '${moduleType}' but the module type has not been declared.
The '${plugin.name}' plugin is likely missing a dependency declaration.
Please report an issue with the author.
`,
{ moduleType, pluginName: plugin.name }
)
// Ignore if the module type to extend cannot be found.
return null
}

// Attach base handlers (which are the corresponding declaration handlers, if any)
Expand Down Expand Up @@ -490,7 +499,7 @@ function resolveModuleDefinitions(resolvedPlugins: PluginMap, configs: GenericPr
return {
...plugin,
createModuleTypes: plugin.createModuleTypes.map((spec) => resolvedDefinitions[spec.name].spec),
extendModuleTypes,
extendModuleTypes: extendModuleTypes.filter(isNotNull),
}
})
}
Expand Down Expand Up @@ -535,7 +544,7 @@ function resolveModuleDefinition(
if (
declaredBy !== plugin.name &&
!pluginBases.includes(declaredBy) &&
!(plugin.dependencies && plugin.dependencies.includes(declaredBy))
!(plugin.dependencies && plugin.dependencies.find((d) => d.name === declaredBy))
) {
throw new PluginError(
deline`
Expand Down
Loading

0 comments on commit e453d70

Please sign in to comment.