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 committed Dec 1, 2022
1 parent 472adfa commit b733634
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 9 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 @@ -34,6 +34,7 @@ import {
Log,
NetworkingV1Api,
ApiextensionsV1Api,
V1ServiceAccount,
} from "@kubernetes/client-node"
import AsyncLock = require("async-lock")
import request = require("request-promise")
Expand Down Expand Up @@ -134,6 +135,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
5 changes: 5 additions & 0 deletions core/src/plugins/kubernetes/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ export interface KubernetesConfig extends BaseProviderConfig {
nodeSelector?: StringMap
tolerations?: V1Toleration[]
annotations?: StringMap
serviceAccountAnnotations?: StringMap
}
clusterDocker?: {
enableBuildKit?: boolean
Expand All @@ -229,6 +230,7 @@ export interface KubernetesConfig extends BaseProviderConfig {
nodeSelector?: StringMap
tolerations?: V1Toleration[]
annotations?: StringMap
serviceAccountAnnotations?: StringMap
util?: {
tolerations?: V1Toleration[]
annotations?: StringMap
Expand Down Expand Up @@ -649,6 +651,9 @@ 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."),
Expand Down
35 changes: 30 additions & 5 deletions core/src/plugins/kubernetes/container/build/buildkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ import {
utilRsyncPort,
ensureBuilderSecret,
builderToleration,
inClusterBuilderServiceAccount,
ensureServiceAccount,
cycleDeployment,
} from "./common"
import { getNamespaceStatus } from "../../namespace"
import { LogLevel } from "../../../../logger/logger"
Expand Down Expand Up @@ -179,6 +182,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.placeholder()

Expand All @@ -193,30 +204,43 @@ export async function ensureBuildkit({
const imagePullSecrets = await prepareSecrets({ api, namespace, secrets: provider.config.imagePullSecrets, log })

// Check status of the buildkit deployment
const manifest = getBuildkitDeployment(provider, authSecret.metadata.name, imagePullSecrets)
const status = await compareDeployedResources(ctx as KubernetesPluginContext, api, namespace, [manifest], deployLog)
const deployment = getBuildkitDeployment(provider, authSecret.metadata.name, imagePullSecrets)

const status = await compareDeployedResources(
ctx as KubernetesPluginContext,
api,
namespace,
[deployment],
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 buildkit daemon
deployLog.setState(
chalk.gray(`-> Deploying ${buildkitDeploymentName} daemon in ${namespace} namespace (was ${status.state})`)
)

await api.upsert({ kind: "Deployment", namespace, log: deployLog, obj: manifest })
await api.upsert({ kind: "Deployment", namespace, log: deployLog, obj: deployment })

await waitForResources({
namespace,
ctx,
provider,
serviceName: "garden-buildkit",
resources: [manifest],
resources: [deployment],
log: deployLog,
timeoutSec: 600,
})
Expand Down Expand Up @@ -377,6 +401,7 @@ export function getBuildkitDeployment(
annotations: provider.config.clusterBuildkit?.annotations,
},
spec: {
serviceAccountName: inClusterBuilderServiceAccount,
containers: [
{
name: buildkitContainerName,
Expand Down
128 changes: 126 additions & 2 deletions core/src/plugins/kubernetes/container/build/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ import { gardenEnv } from "../../../../constants"
import { ensureMutagenSync, flushMutagenSync, getKubectlExecDestination, terminateMutagenSync } from "../../mutagen"
import { randomString } from "../../../../util/string"
import { V1Container, V1Service } from "@kubernetes/client-node"
import { cloneDeep, isEmpty } from "lodash"
import _, { cloneDeep, isEmpty } from "lodash"
import { compareDeployedResources, waitForResources } from "../../status/status"
import { KubernetesDeployment, KubernetesResource } from "../../types"
import { KubernetesDeployment, KubernetesResource, KubernetesServiceAccount } from "../../types"
import { stringifyResources } from "../util"
import { StringMap } from "../../../../config/common"

const inClusterRegistryPort = 5000

export const inClusterBuilderServiceAccount = "garden-in-cluster-builder"
export const sharedBuildSyncDeploymentName = "garden-build-sync"
export const utilContainerName = "util"
export const utilRsyncPort = 8730
Expand Down Expand Up @@ -306,6 +308,59 @@ export async function getUtilDaemonPodRunner({
})
}

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

const serviceAccount = getBuilderServiceAccountSpec(annotations)

const status = await compareDeployedResources(
ctx as KubernetesPluginContext,
api,
namespace,
[serviceAccount],
deployLog
)

// 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: deployLog, 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 @@ -323,6 +378,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.placeholder()

Expand All @@ -346,6 +409,12 @@ export async function ensureUtilDeployment({
)

if (status.state === "ready") {
// 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 (serviceAccountChanged) {
await cycleDeployment({ ctx, provider, deployment, api, namespace, deployLog })
}

// Need to wait a little to ensure the secret is updated in the deployment
if (secretUpdated) {
await sleep(1000)
Expand Down Expand Up @@ -377,6 +446,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: LogEntry
}) {
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 function getSocatContainer(provider: KubernetesProvider) {
const registryHostname = getInClusterRegistryHostname(provider.config)

Expand Down Expand Up @@ -450,6 +559,20 @@ function isLocalHostname(hostname: string) {
return hostname === "localhost" || hostname.startsWith("127.")
}

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 @@ -549,6 +672,7 @@ export function getUtilManifests(
annotations: kanikoAnnotations,
},
spec: {
serviceAccountName: inClusterBuilderServiceAccount,
containers: [getUtilContainer(authSecretName, provider)],
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 @@ -107,7 +107,6 @@ async function buildAndPushViaRemote(params: BuildModuleParams<GardenModule>) {

// Make sure the sync target is up
if (buildMode === "kaniko") {
// Make sure the garden-util deployment is up
await ensureUtilDeployment({
ctx,
provider,
Expand All @@ -117,7 +116,6 @@ async function buildAndPushViaRemote(params: BuildModuleParams<GardenModule>) {
})
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 {
KubernetesObject,
V1DaemonSet,
V1Deployment,
V1ServiceAccount,
V1ObjectMeta,
V1ReplicaSet,
V1StatefulSet,
Expand Down Expand Up @@ -76,6 +77,7 @@ export type KubernetesDeployment = KubernetesResource<V1Deployment>
export type KubernetesReplicaSet = KubernetesResource<V1ReplicaSet>
export type KubernetesStatefulSet = KubernetesResource<V1StatefulSet>
export type KubernetesPod = KubernetesResource<V1Pod>
export type KubernetesServiceAccount = KubernetesResource<V1ServiceAccount>

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

0 comments on commit b733634

Please sign in to comment.