Skip to content

Commit

Permalink
fix: do not crash on missing deploymentRegistry for in-cluster buil…
Browse files Browse the repository at this point in the history
…ds (#6768)

* fix: do not crash when deploymentRegistry is not configured for in-cluster build mode

Throw a configuration error instead.

* fix: conditional validation for deploymentRegistry

* revert: remove conditional joi validation

It requires more changes in the docs generation process.
Now we do npt use `joi.alternatives` with conditional validation anywhere in the project,
and the docs generation does not support that option.
That can be done later in a separate PR if necessary.

* refactor: simplify type definition for `ContainerBuildMode`
  • Loading branch information
vvagaytsev authored Jan 21, 2025
1 parent aadc6ee commit c55486d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 23 deletions.
45 changes: 26 additions & 19 deletions core/src/plugins/kubernetes/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ interface KubernetesStorage {
builder: KubernetesStorageSpec
}

export type ContainerBuildMode = "local-docker" | "kaniko" | "cluster-buildkit"
const containerBuildModes = ["local-docker", "kaniko", "cluster-buildkit"] as const
export type ContainerBuildMode = (typeof containerBuildModes)[number]

/**
* To be removed in 0.14
Expand Down Expand Up @@ -418,21 +419,24 @@ export const utilImageRegistryDomainSpec = joi.string().default(defaultUtilImage
Otherwise the utility images are pulled directly from Docker Hub by default.
`)

const buildModeSchema = () =>
joi
.string()
.valid(...containerBuildModes)
.default("local-docker")
.description(
dedent`
Choose the mechanism for building container images before deploying. By default your local Docker daemon is used, but you can set it to \`cluster-buildkit\` or \`kaniko\` to sync files to the cluster, and build container images there. This removes the need to run Docker locally, and allows you to share layer and image caches between multiple developers, as well as between your development and CI workflows.
For more details on all the different options and what makes sense to use for your setup, please check out the [in-cluster building guide](${DOCS_BASE_URL}/kubernetes-plugins/guides/in-cluster-building).
`
)

export const kubernetesConfigBase = () =>
providerConfigBaseSchema()
.keys({
utilImageRegistryDomain: utilImageRegistryDomainSpec,
buildMode: joi
.string()
.valid("local-docker", "kaniko", "cluster-buildkit")
.default("local-docker")
.description(
dedent`
Choose the mechanism for building container images before deploying. By default your local Docker daemon is used, but you can set it to \`cluster-buildkit\` or \`kaniko\` to sync files to the cluster, and build container images there. This removes the need to run Docker locally, and allows you to share layer and image caches between multiple developers, as well as between your development and CI workflows.
For more details on all the different options and what makes sense to use for your setup, please check out the [in-cluster building guide](${DOCS_BASE_URL}/kubernetes-plugins/guides/in-cluster-building).
`
),
buildMode: buildModeSchema(),
clusterBuildkit: joi
.object()
.keys({
Expand Down Expand Up @@ -728,18 +732,21 @@ export const namespaceSchema = () =>

const kubectlPathExample = "${local.env.GARDEN_KUBECTL_PATH}?"

const deploymentRegistrySchema = () =>
containerRegistryConfigSchema().description(
dedent`
The registry where built containers should be pushed to, and then pulled to the cluster when deploying services.
Important: If you specify this in combination with in-cluster building, you must make sure \`imagePullSecrets\` includes authentication with the specified deployment registry, that has the appropriate write privileges (usually full write access to the configured \`deploymentRegistry.namespace\`).
`
)

export const configSchema = () =>
kubernetesConfigBase()
.keys({
name: joiProviderName("kubernetes"),
context: k8sContextSchema().required(),
deploymentRegistry: containerRegistryConfigSchema().description(
dedent`
The registry where built containers should be pushed to, and then pulled to the cluster when deploying services.
Important: If you specify this in combination with in-cluster building, you must make sure \`imagePullSecrets\` includes authentication with the specified deployment registry, that has the appropriate write privileges (usually full write access to the configured \`deploymentRegistry.namespace\`).
`
),
deploymentRegistry: deploymentRegistrySchema(),
ingressClass: joi.string().description(dedent`
The ingress class or ingressClassName to use on configured Ingresses (via the \`kubernetes.io/ingress.class\` annotation or \`spec.ingressClassName\` field depending on the kubernetes version)
when deploying \`container\` services. Use this if you have multiple ingress controllers in your cluster.
Expand Down
10 changes: 6 additions & 4 deletions core/src/plugins/kubernetes/container/build/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { KubernetesConfig, KubernetesPluginContext, KubernetesProvider } fr
import { PodRunner, PodRunnerError, PodRunnerTimeoutError } from "../../run.js"
import type { PluginContext } from "../../../../plugin-context.js"
import { hashString, sleep } from "../../../../util/util.js"
import { InternalError, RuntimeError } from "../../../../exceptions.js"
import { ConfigurationError, RuntimeError } from "../../../../exceptions.js"
import type { Log } from "../../../../logger/log-entry.js"
import { prepareDockerAuth } from "../../init.js"
import { prepareSecrets } from "../../secrets.js"
Expand Down Expand Up @@ -187,9 +187,11 @@ export async function skopeoBuildStatus({
const deploymentRegistry = provider.config.deploymentRegistry

if (!deploymentRegistry) {
// This is validated in the provider configure handler, so this is an internal error if it happens
throw new InternalError({
message: `Expected configured deploymentRegistry for remote build`,
// This was supposed to be validated in the provider configure handler
// with conditional joi validation, but that caused some troubles with docs generation.
// Throw a configuration error here instead of a crash.
throw new ConfigurationError({
message: `The deploymentRegistry must be configured in provider for remote builds`,
})
}

Expand Down

0 comments on commit c55486d

Please sign in to comment.