Skip to content

Commit

Permalink
feat(k8s): add service account and irsa support for in-cluster-builder
Browse files Browse the repository at this point in the history
This commit introduces a new service account, and makes the annotations
configurable in kaniko and buildkit in-cluster-builders.

This change enables the use of IRSA for in-cluter-building which makes
it more secure.

Fixes #2931
  • Loading branch information
stefreak authored and twelvemo committed Nov 14, 2023
1 parent 71b3781 commit 16b8756
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 6 deletions.
10 changes: 10 additions & 0 deletions core/src/plugins/kubernetes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
V1Deployment,
V1Secret,
V1Service,
V1ServiceAccount,
ServerConfiguration,
createConfiguration,
} from "@kubernetes/client-node"
Expand Down Expand Up @@ -129,6 +130,15 @@ const crudMap = {
delete: "deleteNamespacedService",
patch: "patchNamespacedService",
},
ServiceAccount: {
cls: new V1ServiceAccount(),
group: "core",
read: "readNamespacedServiceAccount",
create: "createNamespacedServiceAccount",
replace: "replaceNamespacedServiceAccount",
delete: "deleteNamespacedServiceAccount",
patch: "patchNamespacedServiceAccount",
},
}

type CrudMap = typeof crudMap
Expand Down
28 changes: 27 additions & 1 deletion core/src/plugins/kubernetes/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ export interface KubernetesConfig extends BaseProviderConfig {
nodeSelector?: StringMap
tolerations?: V1Toleration[]
annotations?: StringMap
serviceAccountAnnotations?: StringMap
}
jib?: {
pushViaCluster?: boolean
Expand All @@ -143,6 +144,7 @@ export interface KubernetesConfig extends BaseProviderConfig {
nodeSelector?: StringMap
tolerations?: V1Toleration[]
annotations?: StringMap
serviceAccountAnnotations?: StringMap
util?: {
tolerations?: V1Toleration[]
annotations?: StringMap
Expand Down Expand Up @@ -517,9 +519,29 @@ export const kubernetesConfigBase = () =>
annotations: annotationsSchema().description(
"Specify annotations to apply to both the Pod and Deployment resources associated with cluster-buildkit. Annotations may have an effect on the behaviour of certain components, for example autoscalers."
),
serviceAccountAnnotations: annotationsSchema().description(
"Specify annotations to apply to the Kubernetes service account used by cluster-buildkit. This can be useful to set up IRSA with in-cluster building."
),
})
.default(() => ({}))
.description("Configuration options for the `cluster-buildkit` build mode."),
clusterDocker: joi
.object()
.keys({
enableBuildKit: joi
.boolean()
.default(false)
.description(
deline`
Enable [BuildKit](https://github.com/moby/buildkit) support. This should in most cases work well and be
more performant, but we're opting to keep it optional until it's enabled by default in Docker.
`
)
.meta({ deprecated: true }),
})
.default(() => ({}))
.description("Configuration options for the `cluster-docker` build mode.")
.meta({ deprecated: "The cluster-docker build mode has been deprecated." }),
jib: joi
.object()
.keys({
Expand All @@ -529,8 +551,12 @@ export const kubernetesConfigBase = () =>
.description(
"In some cases you may need to push images built with Jib to the remote registry via Kubernetes cluster, e.g. if you don't have connectivity or access from where Garden is being run. In that case, set this flag to true, but do note that the build will take considerably take longer to complete! Only applies when using in-cluster building."
),
annotations: annotationsSchema().description(
"Specify annotations to apply to both the Pod and Deployment resources associated with cluster-buildkit. Annotations may have an effect on the behaviour of certain components, for example autoscalers."
),
})
.description("Setting related to Jib image builds."),
.default(() => ({}))
.description("Configuration options for the `cluster-buildkit` build mode."),
kaniko: joi
.object()
.keys({
Expand Down
20 changes: 19 additions & 1 deletion core/src/plugins/kubernetes/container/build/buildkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import {
getUtilContainer,
ensureBuilderSecret,
builderToleration,
inClusterBuilderServiceAccount,
ensureServiceAccount,
cycleDeployment,
} from "./common.js"
import { getNamespaceStatus } from "../../namespace.js"
import { sleep } from "../../../../util/util.js"
Expand Down Expand Up @@ -187,6 +190,14 @@ export async function ensureBuildkit({
api: KubeApi
namespace: string
}) {
const serviceAccountChanged = await ensureServiceAccount({
ctx,
log,
api,
namespace,
annotations: provider.config.clusterBuildkit?.serviceAccountAnnotations,
})

return deployLock.acquire(namespace, async () => {
const deployLog = log.createLog()

Expand All @@ -210,12 +221,18 @@ export async function ensureBuildkit({
log: deployLog,
})

// if the service account changed, all pods part of the deployment must be restarted
// so that they receive new credentials (e.g. for IRSA)
if (status.remoteResources.length && serviceAccountChanged) {
await cycleDeployment({ ctx, provider, deployment: manifest, api, namespace, deployLog })
}

if (status.state === "ready") {
// Need to wait a little to ensure the secret is updated in the deployment
if (secretUpdated) {
await sleep(1000)
}
return { authSecret, updated: false }
return { authSecret, updated: serviceAccountChanged }
}

// Deploy the buildkit daemon
Expand Down Expand Up @@ -397,6 +414,7 @@ export function getBuildkitDeployment(
annotations: provider.config.clusterBuildkit?.annotations,
},
spec: {
serviceAccountName: inClusterBuilderServiceAccount,
containers: [
{
name: buildkitContainerName,
Expand Down
127 changes: 125 additions & 2 deletions core/src/plugins/kubernetes/container/build/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { randomString } from "../../../../util/string.js"
import type { V1Container, V1Service } from "@kubernetes/client-node"
import { cloneDeep, isEmpty } from "lodash-es"
import { compareDeployedResources, waitForResources } from "../../status/status.js"
import type { KubernetesDeployment, KubernetesResource } from "../../types.js"
import type { KubernetesDeployment, KubernetesResource, KubernetesServiceAccount } from "../../types.js"
import type { BuildActionHandler, BuildActionResults } from "../../../../plugin/action-types.js"
import { k8sGetContainerBuildActionOutputs } from "../handlers.js"
import type { Resolved } from "../../../../actions/types.js"
Expand All @@ -31,7 +31,10 @@ import { getKubectlExecDestination } from "../../sync.js"
import { getRunningDeploymentPod } from "../../util.js"
import { buildSyncVolumeName, dockerAuthSecretKey, k8sUtilImageName, rsyncPortName } from "../../constants.js"
import { styles } from "../../../../logger/styles.js"
import type { StringMap } from "../../../../config/common.js"

export const inClusterBuilderServiceAccount = "garden-in-cluster-builder"
export const sharedBuildSyncDeploymentName = "garden-build-sync"
export const utilContainerName = "util"
export const utilRsyncPort = 8730
export const utilDeploymentName = "garden-util"
Expand Down Expand Up @@ -262,6 +265,57 @@ export function skopeoManifestUnknown(errMsg: string | null | undefined): boolea
)
}

export async function ensureServiceAccount({
ctx,
log,
api,
namespace,
annotations,
}: {
ctx: PluginContext
log: Log
api: KubeApi
namespace: string
annotations?: StringMap
}): Promise<boolean> {
return deployLock.acquire(namespace, async () => {
const serviceAccount = getBuilderServiceAccountSpec(annotations)

const status = await compareDeployedResources({
ctx: ctx as KubernetesPluginContext,
api,
namespace,
manifests: [serviceAccount],
log,
})

// NOTE: This is here to make sure that we remove annotations in case they are removed in the garden config.
// `compareDeployedResources` as of today only checks wether the manifest is a subset of the deployed manifest.
// The manifest is still a subset of the deployed manifest, if an annotation has been removed. But we want the
// annotation to be actually removed.
// NOTE(steffen): I tried to change the behaviour of `compareDeployedResources` to return "outdated" when the
// annotations have changed. But a lot of code actually depends on the behaviour of it with missing annotations.
const annotationsNeedUpdate =
status.remoteResources.length > 0 && !isEqualAnnotations(serviceAccount, status.remoteResources[0])

const needUpsert = status.state !== "ready" || annotationsNeedUpdate

if (needUpsert) {
await api.upsert({ kind: "ServiceAccount", namespace, log, obj: serviceAccount })
return true
}

return false
})
}

function isEqualAnnotations(r1: KubernetesResource, r2: KubernetesResource): boolean {
// normalize annotations before comparison
const a1 = r1.metadata.annotations !== undefined ? r1.metadata.annotations : {}
const a2 = r2.metadata.annotations !== undefined ? r2.metadata.annotations : {}
return JSON.stringify(a1) === JSON.stringify(a2)
}

/**
* Ensures that a garden-util deployment exists in the specified namespace.
* Returns the docker auth secret that's generated and mounted in the deployment.
Expand All @@ -279,6 +333,14 @@ export async function ensureUtilDeployment({
api: KubeApi
namespace: string
}) {
const serviceAccountChanged = await ensureServiceAccount({
ctx,
log,
api,
namespace,
annotations: provider.config.kaniko?.serviceAccountAnnotations,
})

return deployLock.acquire(namespace, async () => {
const deployLog = log.createLog()

Expand All @@ -301,12 +363,18 @@ export async function ensureUtilDeployment({
log: deployLog,
})

// if the service account changed, all pods part of the deployment must be restarted
// so that they receive new credentials (e.g. for IRSA)
if (status.remoteResources.length && serviceAccountChanged) {
await cycleDeployment({ ctx, provider, deployment, api, namespace, deployLog })
}

if (status.state === "ready") {
// Need to wait a little to ensure the secret is updated in the deployment
if (secretUpdated) {
await sleep(1000)
}
return { authSecret, updated: false }
return { authSecret, updated: serviceAccountChanged }
}

// Deploy the service
Expand All @@ -333,6 +401,46 @@ export async function ensureUtilDeployment({
})
}

export async function cycleDeployment({
ctx,
provider,
deployment,
api,
namespace,
deployLog,
}: {
ctx: PluginContext
provider: KubernetesProvider
deployment: KubernetesDeployment
api: KubeApi
namespace: string
deployLog: Log
}) {
const originalReplicas = deployment.spec.replicas

deployment.spec.replicas = 0
await api.upsert({ kind: "Deployment", namespace, log: deployLog, obj: deployment })
await waitForResources({
namespace,
ctx,
provider,
resources: [deployment],
log: deployLog,
timeoutSec: 600,
})

deployment.spec.replicas = originalReplicas
await api.upsert({ kind: "Deployment", namespace, log: deployLog, obj: deployment })
await waitForResources({
namespace,
ctx,
provider,
resources: [deployment],
log: deployLog,
timeoutSec: 600,
})
}

export async function getManifestInspectArgs(remoteId: string, deploymentRegistry: ContainerRegistryConfig) {
const dockerArgs = ["manifest", "inspect", remoteId]
const { hostname } = deploymentRegistry
Expand Down Expand Up @@ -386,6 +494,20 @@ export async function ensureBuilderSecret({
return { authSecret, updated }
}

export function getBuilderServiceAccountSpec(annotations?: StringMap) {
const serviceAccount: KubernetesServiceAccount = {
apiVersion: "v1",
kind: "ServiceAccount",
metadata: {
name: inClusterBuilderServiceAccount,
// ensure we clear old annotations if config flags are removed
annotations: annotations || {},
},
}

return serviceAccount
}

export function getUtilContainer(authSecretName: string, provider: KubernetesProvider): V1Container {
return {
name: utilContainerName,
Expand Down Expand Up @@ -491,6 +613,7 @@ export function getUtilManifests(
annotations: kanikoAnnotations,
},
spec: {
serviceAccountName: inClusterBuilderServiceAccount,
containers: [utilContainer],
imagePullSecrets,
volumes: [
Expand Down
2 changes: 0 additions & 2 deletions core/src/plugins/kubernetes/jib-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ async function buildAndPushViaRemote(params: BuildActionParams<"build", Containe

// Make sure the sync target is up
if (buildMode === "kaniko") {
// Make sure the garden-util deployment is up
await ensureUtilDeployment({
ctx,
provider,
Expand All @@ -120,7 +119,6 @@ async function buildAndPushViaRemote(params: BuildActionParams<"build", Containe
})
deploymentName = utilDeploymentName
} else if (buildMode === "cluster-buildkit") {
// Make sure the buildkit deployment is up
await ensureBuildkit({
ctx,
provider,
Expand Down
2 changes: 2 additions & 0 deletions core/src/plugins/kubernetes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
KubernetesObject,
V1DaemonSet,
V1Deployment,
V1ServiceAccount,
V1ObjectMeta,
V1ReplicaSet,
V1StatefulSet,
Expand Down Expand Up @@ -92,6 +93,7 @@ export type KubernetesReplicaSet = KubernetesResource<V1ReplicaSet>
export type KubernetesStatefulSet = KubernetesResource<V1StatefulSet>
export type KubernetesPod = KubernetesResource<V1Pod>
export type KubernetesService = KubernetesResource<V1Service>
export type KubernetesServiceAccount = KubernetesResource<V1ServiceAccount>

export type KubernetesWorkload = KubernetesResource<V1DaemonSet | V1Deployment | V1ReplicaSet | V1StatefulSet>
export type KubernetesIngress = KubernetesResource<V1Ingress>
Expand Down
Loading

0 comments on commit 16b8756

Please sign in to comment.