Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(k8s): regression in globs in k8s manifest files #4903

Merged
merged 3 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
53 changes: 48 additions & 5 deletions core/src/plugins/kubernetes/kubernetes-type/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
*/

import { resolve } from "path"
import { readFile } from "fs-extra"
import { pathExists, readFile } from "fs-extra"
import Bluebird from "bluebird"
import { flatten, set } from "lodash"
import { loadAll } from "js-yaml"

import { KubernetesModule } from "./module-config"
import { KubernetesResource } from "../types"
import { KubeApi } from "../api"
import { gardenAnnotationKey } from "../../../util/string"
import { gardenAnnotationKey, naturalList } from "../../../util/string"
import { Log } from "../../../logger/log-entry"
import { PluginContext } from "../../../plugin-context"
import { ConfigurationError, PluginError } from "../../../exceptions"
Expand All @@ -24,10 +24,11 @@ import { HelmModule } from "../helm/module-config"
import { KubernetesDeployAction } from "./config"
import { CommonRunParams } from "../../../plugin/handlers/Run/run"
import { runAndCopy } from "../run"
import { getTargetResource, getResourcePodSpec, getResourceContainer, makePodName } from "../util"
import { getResourceContainer, getResourcePodSpec, getTargetResource, makePodName } from "../util"
import { Resolved } from "../../../actions/types"
import { KubernetesPodRunAction, KubernetesPodTestAction } from "./kubernetes-pod"
import { glob } from "glob"
import isGlob from "is-glob"

/**
* Reads the manifests and makes sure each has a namespace set (when applicable) and adds annotations.
Expand Down Expand Up @@ -128,11 +129,53 @@ export async function readManifests(
const manifestPath = readFromSrcDir ? action.basePath() : action.getBuildPath()

const spec = action.getSpec()
const specFiles = spec.files

const files = await glob(spec.files, { cwd: manifestPath })
const regularPaths = specFiles.filter((f) => !isGlob(f)).map((path) => resolve(manifestPath, path))
const missingPaths = await Bluebird.filter(regularPaths, async (regularPath) => {
return !(await pathExists(regularPath))
})
if (missingPaths.length) {
throw new ConfigurationError({
message: `Invalid manifest file path(s) in ${action.kind} action '${
Walther marked this conversation as resolved.
Show resolved Hide resolved
action.name
}'. Cannot find manifest file(s) at ${naturalList(missingPaths)}`,
detail: {
action: {
kind: action.kind,
name: action.name,
type: action.type,
spec: {
files: specFiles,
},
},
missingPaths,
},
})
}

const resolvedFiles = await glob(specFiles, { cwd: manifestPath })
if (specFiles.length > 0 && resolvedFiles.length === 0) {
throw new ConfigurationError({
message: `Invalid manifest file path(s) in ${action.kind} action '${
action.name
}'. Cannot find any manifest files for paths ${naturalList(specFiles)}`,
detail: {
action: {
kind: action.kind,
name: action.name,
type: action.type,
spec: {
files: specFiles,
},
},
resolvedFiles,
},
})
}

const fileManifests = flatten(
await Bluebird.map(files, async (path) => {
await Bluebird.map(resolvedFiles, async (path) => {
const absPath = resolve(manifestPath, path)
log.debug(`Reading manifest for ${action.longDescription()} from path ${absPath}`)
const str = (await readFile(absPath)).toString()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
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
args: [sh, -c, "while :; do sleep 2073600; done"]
env:
- name: FOO
value: banana
- name: BAR
value: ""
- name: BAZ
value: null
ports:
- containerPort: 80
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ type: kubernetes
name: with-build-action
build: exec-build
spec:
files: ["*.yaml"]
files: [ "deployment-action.yaml" ]
115 changes: 99 additions & 16 deletions core/test/integ/src/plugins/kubernetes/kubernetes-type/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { cloneDeep } from "lodash"
import { ConfigGraph } from "../../../../../../src/graph/config-graph"
import { PluginContext } from "../../../../../../src/plugin-context"
import { readManifests } from "../../../../../../src/plugins/kubernetes/kubernetes-type/common"
import { TestGarden, makeTestGarden, getExampleDir, expectError, getDataDir } from "../../../../../helpers"
import { expectError, getDataDir, getExampleDir, makeTestGarden, TestGarden } from "../../../../../helpers"
import { KubernetesDeployAction } from "../../../../../../src/plugins/kubernetes/kubernetes-type/config"
import { Resolved } from "../../../../../../src/actions/types"

Expand All @@ -33,27 +33,28 @@ export async function getKubernetesTestGarden() {
describe("readManifests", () => {
let garden: TestGarden
let ctx: PluginContext
let action: Resolved<KubernetesDeployAction>
let graph: ConfigGraph

const exampleDir = getExampleDir("kustomize")
context("kustomize", () => {
const exampleDir = getExampleDir("kustomize")

before(async () => {
garden = await makeTestGarden(exampleDir)
const provider = await garden.resolveProvider(garden.log, "local-kubernetes")
ctx = await garden.getPluginContext({ provider, templateContext: undefined, events: undefined })
})
let action: Resolved<KubernetesDeployAction>

beforeEach(async () => {
graph = await garden.getConfigGraph({ log: garden.log, emit: false })
action = await garden.resolveAction<KubernetesDeployAction>({
action: cloneDeep(graph.getDeploy("hello-world")),
log: garden.log,
graph,
before(async () => {
garden = await makeTestGarden(exampleDir)
const provider = await garden.resolveProvider(garden.log, "local-kubernetes")
ctx = await garden.getPluginContext({ provider, templateContext: undefined, events: undefined })
})

beforeEach(async () => {
graph = await garden.getConfigGraph({ log: garden.log, emit: false })
action = await garden.resolveAction<KubernetesDeployAction>({
action: cloneDeep(graph.getDeploy("hello-world")),
log: garden.log,
graph,
})
})
})

context("kustomize", () => {
const expectedErr = "kustomize.extraArgs must not include any of -o, --output, -h, --help"

it("throws if --output is set in extraArgs", async () => {
Expand Down Expand Up @@ -105,4 +106,86 @@ describe("readManifests", () => {
expect(kinds).to.eql(["Deployment", "Service", "ConfigMap"])
})
})

context("kubernetes manifest files resolution", () => {
before(async () => {
garden = await getKubernetesTestGarden()
const provider = await garden.resolveProvider(garden.log, "local-kubernetes")
ctx = await garden.getPluginContext({ provider, templateContext: undefined, events: undefined })
})

beforeEach(async () => {
graph = await garden.getConfigGraph({ log: garden.log, emit: false })
})

it("should support regular files paths", async () => {
const resolvedAction = await garden.resolveAction<KubernetesDeployAction>({
action: cloneDeep(graph.getDeploy("with-build-action")),
log: garden.log,
graph,
})
// Pre-check to ensure that the test filenames in the test data are correct.
expect(resolvedAction.getSpec().files).to.eql(["deployment-action.yaml"])

// We use readFromSrcDir = true here because we just resolve but do not execute any actions.
// It means that the build directory will not be created.
const manifests = await readManifests(ctx, resolvedAction, garden.log, true)
expect(manifests).to.exist
const names = manifests.map((m) => ({ kind: m.kind, name: m.metadata?.name }))
expect(names).to.eql([{ kind: "Deployment", name: "busybox-deployment" }])
})

it("should support both regular paths and glob patterns with deduplication", async () => {
const action = cloneDeep(graph.getDeploy("with-build-action"))
// Append a valid glob pattern that results to a non-empty list of files.
action["_config"]["spec"]["files"].push("*.yaml")
const resolvedAction = await garden.resolveAction<KubernetesDeployAction>({
action,
log: garden.log,
graph,
})
// Pre-check to ensure that the test filenames in the test data are correct.
expect(resolvedAction.getSpec().files).to.eql(["deployment-action.yaml", "*.yaml"])

// We use readFromSrcDir = true here because we just resolve but do not execute any actions.
// It means that the build directory will not be created.
const manifests = await readManifests(ctx, resolvedAction, garden.log, true)
expect(manifests).to.exist
const names = manifests.map((m) => ({ kind: m.kind, name: m.metadata?.name }))
expect(names).to.eql([{ kind: "Deployment", name: "busybox-deployment" }])
})

it("should throw on missing regular path", async () => {
const action = cloneDeep(graph.getDeploy("with-build-action"))
action["_config"]["spec"]["files"].push("missing-file.yaml")
const resolvedAction = await garden.resolveAction<KubernetesDeployAction>({
action,
log: garden.log,
graph,
})

// We use readFromSrcDir = true here because we just resolve but do not execute any actions.
// It means that the build directory will not be created.
await expectError(() => readManifests(ctx, resolvedAction, garden.log, true), {
contains: `Invalid manifest file path(s) in ${action.kind} action '${action.name}'`,
})
})

it("should throw when no files found from glob pattens", async () => {
const action = cloneDeep(graph.getDeploy("with-build-action"))
// Rewrite the whole files array to have a glob pattern that results to an empty list of files.
action["_config"]["spec"]["files"] = ["./**/manifests/*.yaml"]
const resolvedAction = await garden.resolveAction<KubernetesDeployAction>({
action,
log: garden.log,
graph,
})

// We use readFromSrcDir = true here because we just resolve but do not execute any actions.
// It means that the build directory will not be created.
await expectError(() => readManifests(ctx, resolvedAction, garden.log, true), {
contains: `Invalid manifest file path(s) in ${action.kind} action '${action.name}'`,
})
})
})
})