Skip to content

Commit

Permalink
fix(config): do not set default include if exclude is set + add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
eysi09 authored and edvald committed Dec 19, 2019
1 parent 265696e commit 5f7dd18
Show file tree
Hide file tree
Showing 14 changed files with 299 additions and 11 deletions.
14 changes: 14 additions & 0 deletions docs/reference/module-types/container.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ description:
# for details.
#
# Also note that specifying an empty list here means _no sources_ should be included.
#
# If neither `include` nor `exclude` is set, and the module has a Dockerfile, Garden
# will parse the Dockerfile and automatically set `include` to match the files and
# folders added to the Docker image (via the `COPY` and `ADD` directives in the Dockerfile).
#
# If neither `include` nor `exclude` is set, and the module
# specifies a remote image, Garden automatically sets `include` to `[]`.
include:

# Specify a list of POSIX-style paths or glob patterns that should be excluded from the module.
Expand Down Expand Up @@ -384,6 +391,13 @@ source tree, which use the same format as `.gitignore` files. See the

Also note that specifying an empty list here means _no sources_ should be included.

If neither `include` nor `exclude` is set, and the module has a Dockerfile, Garden
will parse the Dockerfile and automatically set `include` to match the files and
folders added to the Docker image (via the `COPY` and `ADD` directives in the Dockerfile).

If neither `include` nor `exclude` is set, and the module
specifies a remote image, Garden automatically sets `include` to `[]`.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
12 changes: 12 additions & 0 deletions docs/reference/module-types/helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ description:
# for details.
#
# Also note that specifying an empty list here means _no sources_ should be included.
#
# If neither `include` nor `exclude` is set, and the module has local chart sources, Garden
# automatically set `include` to: `["*", "charts/**/*", "templates/**/*"]`.
#
# If neither `include` nor `exclude` is set and the module specifies a remote chart, Garden
# automatically sets `ìnclude` to `[]`.
include:

# Specify a list of POSIX-style paths or glob patterns that should be excluded from the module.
Expand Down Expand Up @@ -360,6 +366,12 @@ source tree, which use the same format as `.gitignore` files. See the

Also note that specifying an empty list here means _no sources_ should be included.

If neither `include` nor `exclude` is set, and the module has local chart sources, Garden
automatically set `include` to: `["*", "charts/**/*", "templates/**/*"]`.

If neither `include` nor `exclude` is set and the module specifies a remote chart, Garden
automatically sets `ìnclude` to `[]`.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
6 changes: 6 additions & 0 deletions docs/reference/module-types/kubernetes.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ description:
# for details.
#
# Also note that specifying an empty list here means _no sources_ should be included.
#
# If neither `include` nor `exclude` is set, Garden automatically sets `include` to equal the
# `files` directive so that only the Kubernetes manifests get included.
include:

# Specify a list of POSIX-style paths or glob patterns that should be excluded from the module.
Expand Down Expand Up @@ -183,6 +186,9 @@ source tree, which use the same format as `.gitignore` files. See the

Also note that specifying an empty list here means _no sources_ should be included.

If neither `include` nor `exclude` is set, Garden automatically sets `include` to equal the
`files` directive so that only the Kubernetes manifests get included.

| Type | Required |
| --------------- | -------- |
| `array[string]` | No |
Expand Down
26 changes: 25 additions & 1 deletion garden-service/src/config/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import chalk from "chalk"
import { relative } from "path"
import { splitLast } from "../util/util"
import isGitUrl from "is-git-url"
import { deline } from "../util/string"
import { deline, dedent } from "../util/string"

export type Primitive = string | number | boolean | null

Expand Down Expand Up @@ -235,6 +235,30 @@ export const joiIdentifierDescription =
"valid RFC1035/RFC1123 (DNS) label (may contain lowercase letters, numbers and dashes, must start with a letter, " +
"and cannot end with a dash) and must not be longer than 63 characters."

const moduleIncludeDescription = (extraDescription?: string) => {
const desc = dedent`
Specify a list of POSIX-style paths or globs that should be regarded as the source files for this
module. Files that do *not* match these paths or globs are excluded when computing the version of the module,
when responding to filesystem watch events, and when staging builds.
Note that you can also _exclude_ files using the \`exclude\` field or by placing \`.gardenignore\` files in your
source tree, which use the same format as \`.gitignore\` files. See the
[Configuration Files guide](${includeGuideLink}) for details.
Also note that specifying an empty list here means _no sources_ should be included.
`
if (extraDescription) {
return desc + "\n\n" + extraDescription
}
return desc
}

export const joiModuleIncludeDirective = (extraDescription?: string) =>
joi
.array()
.items(joi.string().posixPath({ allowGlobs: true, subPathOnly: true }))
.description(moduleIncludeDescription(extraDescription))

export const joiIdentifier = () =>
joi
.string()
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/config/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import dedent = require("dedent")
import stableStringify = require("json-stable-stringify")
import { ServiceConfig, ServiceSpec, serviceConfigSchema } from "./service"
import {
Expand All @@ -24,6 +23,7 @@ import { TestConfig, TestSpec, testConfigSchema } from "./test"
import { TaskConfig, TaskSpec, taskConfigSchema } from "./task"
import { DEFAULT_API_VERSION } from "../constants"
import { joiVariables } from "./common"
import { dedent } from "../util/string"

export interface BuildCopySpec {
source: string
Expand Down
9 changes: 9 additions & 0 deletions garden-service/src/plugins/container/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
envVarRegex,
Primitive,
ArtifactSpec,
joiModuleIncludeDirective,
} from "../../config/common"
import { Service, ingressHostnameSchema, linkUrlSchema } from "../../types/service"
import { DEFAULT_PORT_PROTOCOL } from "../../constants"
Expand Down Expand Up @@ -539,6 +540,14 @@ export const containerModuleSpecSchema = joi
Specify the image name for the container. Should be a valid Docker image identifier. If specified and
the module does not contain a Dockerfile, this image will be used to deploy services for this module.
If specified and the module does contain a Dockerfile, this identifier is used when pushing the built image.`),
include: joiModuleIncludeDirective(dedent`
If neither \`include\` nor \`exclude\` is set, and the module has a Dockerfile, Garden
will parse the Dockerfile and automatically set \`include\` to match the files and
folders added to the Docker image (via the \`COPY\` and \`ADD\` directives in the Dockerfile).
If neither \`include\` nor \`exclude\` is set, and the module
specifies a remote image, Garden automatically sets \`include\` to \`[]\`.
`),
hotReload: hotReloadConfigSchema,
dockerfile: joi
.string()
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/plugins/container/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export async function configureContainerModule({ ctx, log, moduleConfig }: Confi
}

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

Expand Down
10 changes: 9 additions & 1 deletion garden-service/src/plugins/kubernetes/helm/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
DeepPrimitiveMap,
joi,
ArtifactSpec,
joiModuleIncludeDirective,
} from "../../../config/common"
import { Module, FileCopySpec } from "../../../types/module"
import { containsSource, getReleaseName } from "./common"
Expand Down Expand Up @@ -238,6 +239,13 @@ export const helmModuleSpecSchema = joi.object().keys({
deline`Set this to true if the chart should only be built, but not deployed as a service.
Use this, for example, if the chart should only be used as a base for other modules.`
),
include: joiModuleIncludeDirective(dedent`
If neither \`include\` nor \`exclude\` is set, and the module has local chart sources, Garden
automatically set \`include\` to: \`["*", "charts/**/*", "templates/**/*"]\`.
If neither \`include\` nor \`exclude\` is set and the module specifies a remote chart, Garden
automatically sets \`ìnclude\` to \`[]\`.
`),
tasks: joiArray(taskSchema).description("The task definitions for this module."),
tests: joiArray(testSchema).description("The test suite definitions for this module."),
timeout: joi
Expand Down Expand Up @@ -353,7 +361,7 @@ export async function configureHelmModule({
}

// Automatically set the include if not explicitly set
if (!moduleConfig.include) {
if (!(moduleConfig.include || moduleConfig.exclude)) {
moduleConfig.include = containsSources ? ["*", "charts/**/*", "templates/**/*"] : []
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
*/

import { ServiceSpec, dependenciesSchema } from "../../../config/service"
import { joiArray, joi } from "../../../config/common"
import { joiArray, joi, joiModuleIncludeDirective } from "../../../config/common"
import { Module } from "../../../types/module"
import { ConfigureModuleParams, ConfigureModuleResult } from "../../../types/plugin/module/configure"
import { Service } from "../../../types/service"
import { ContainerModule } from "../../container/config"
import { baseBuildSpecSchema } from "../../../config/module"
import { KubernetesResource } from "../types"
import { deline } from "../../../util/string"
import { deline, dedent } from "../../../util/string"

// A Kubernetes Module always maps to a single Service
export type KubernetesModuleSpec = KubernetesServiceSpec
Expand Down Expand Up @@ -65,6 +65,10 @@ export const kubernetesModuleSpecSchema = joi.object().keys({
files: joiArray(joi.string().posixPath({ subPathOnly: true })).description(
"POSIX-style paths to YAML files to load manifests from. Each can contain multiple manifests."
),
include: joiModuleIncludeDirective(dedent`
If neither \`include\` nor \`exclude\` is set, Garden automatically sets \`include\` to equal the
\`files\` directive so that only the Kubernetes manifests get included.
`),
})

export async function configureKubernetesModule({
Expand All @@ -80,7 +84,7 @@ export async function configureKubernetesModule({
]

// Unless include is explicitly specified, we should just have it equal the `files` field
if (!moduleConfig.include) {
if (!(moduleConfig.include || moduleConfig.exclude)) {
moduleConfig.include = moduleConfig.spec.files
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
ContainerModuleConfig,
ContainerTaskSpec,
} from "../container/config"
import { joiArray, joiProviderName, joi } from "../../config/common"
import { joiArray, joiProviderName, joi, joiModuleIncludeDirective } from "../../config/common"
import { Module } from "../../types/module"
import { resolve } from "path"
import { RuntimeError, ConfigurationError } from "../../exceptions"
Expand Down Expand Up @@ -66,6 +66,7 @@ const mavenKeys = {
`
)
.example("11-jdk"),
include: joiModuleIncludeDirective(),
jarPath: joi
.string()
.required()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
kind: Project
name: kubernetes-module-test
providers:
- name: local-kubernetes
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
kind: Module
type: kubernetes
name: module-simple
description: Simple Kubernetes module with minimum config
manifests:
- apiVersion: apps/v1
kind: Deployment
metadata:
name: busybox-deployment
labels:
app: busybox
spec:
replicas: 1
selector:
matchLabels:
app: busybox
template:
metadata:
labels:
app: busybox
spec:
containers:
- name: busybox
image: busybox:1.31.1
ports:
- containerPort: 80
12 changes: 9 additions & 3 deletions garden-service/test/integ/src/plugins/kubernetes/helm/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,16 @@ describe("validateHelmModule", () => {
})
})

it("should not set default includes if they have already been explicitly set", async () => {
it("should not set default includes if include has already been explicitly set", async () => {
patchModuleConfig("api", { include: ["foo"] })
const config = await garden.resolveModuleConfig(garden.log, "api")
expect(config.include).to.eql(["foo"])
const configInclude = await garden.resolveModuleConfig(garden.log, "api")
expect(configInclude.include).to.eql(["foo"])
})

it("should not set default includes if exclude has already been explicitly set", async () => {
patchModuleConfig("api", { exclude: ["bar"] })
const configExclude = await garden.resolveModuleConfig(garden.log, "api")
expect(configExclude.include).to.be.undefined
})

it("should set include to empty if module does not have local chart sources", async () => {
Expand Down
Loading

0 comments on commit 5f7dd18

Please sign in to comment.