build: Support Mounted Resource Volumes

[adambkaplan]

Proposal to support mounts for Secrets and ConfigMaps in builds. This
augments prior work which let Secrets and ConfigMaps be used as sources
in builds. Unlike the current approach, this enhancement leverages
buildah's volume mount feature to let content be available only at build
time, and not at runtime. This is useful for builds that need to access
private artifact repositories or RHEL subscription content.

This current proposal builds on top of Ben Parees's original volume
mount proposal. Supported volume types will be gated at the API level to
ensure the volume type does not pose a security risk and is correctly
lifecycled. To distinguish Kubernetes volumes+mounts from buildah's
volume mount mechanism, the terms "volume content," "input volume,"
"buildah volume mount," and "buildah runtime environment" were
introduced. The proposed API uses these terms to distinguish build
volume mounts from pod volume mounts. Documentation requirements for
this feature were added in the "Drawbacks" section of the proposal.

Because the build controller uses the privileged security context
constraint, build pods to bypass most security features in OpenShift.
Therefore, future volume mounts in build pods need to be tightly
controlled. Open-ended volume types like CSI volumes could be an attack
vector if a developer uses an insecure CSI driver implementation. This
proposal establishes graduation criteria for adding new volume source
types to builds, including security, testing, lifecycle concerns,
failure modes, and feature gating.

[adambkaplan]

/assign @bparees

/cc @gabemontero @coreydaley @alicerum @otaviof @smarterclayton @derekwaynecarr

[bparees]

one nit, otherwise looks as good as the day i wrote it :)

[bparees]

/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [bparees]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[gabemontero]

Took a first pass at this @bparees @adambkaplan

Ranges from some clarification on verbiage in godoc and proposal doc ... though I won't argue too too much if you all think I'm being overly precise.

The read only bit and some of the internal details of how the projected resource driver currently work probably will engender more discussion and possibly more changes to the driver at some point.

[sttts]

OpenAPI to restrict to exactly one:

oneOf:
- required: ["secret"]
- required: ["configMap"]
- required: ["csi"]

This can be added to the CRD OpenAPI schema using a yaml patch (kubebuilder has no marker for that).

[deads2k]

I like the way this API mirrors volumes. I want to be sure

1. we don't accidentally escalate users by adding this feature - this is the dev-ex team's responsibility
2. that SCC can actually handle CSI volumes in some reasonable way - this the apiserver or storage team's responsibility

[adambkaplan]

/cc @nalind

For mount options supported by buildah.

[nalind]

The library accepts options for controlling propagation ("shared", "rshared", "private", "rprivate", "slave", "rslave"), regular options ("rw, "ro", "dev"/"nodev", "suid"/"nosuid", "exec"/"noexec"), and SELinux labeling controls that I think end up being ignored since we're already in a container ("Z", "z").
There are also a couple of custom ones: "U" to have the ownership of the volume's contents changed to match the current USER value, and "O" which causes the volume source to be used as the lower layer for an overlay mount at the mount point, using a per-run temporary directory as the upper (write) layer. I don't know that either of those is particularly useful in builds, and of course their functionality willl be limited in the future by the permissions that the builder binary has.

[adambkaplan]

Good to know - we will start with ReadOnly ro as the only supported option.

[adambkaplan]

I want to be sure

1. we don't accidentally escalate users by adding this feature - this is the dev-ex team's responsibility
2. that SCC can actually handle CSI volumes in some reasonable way - this the apiserver or storage team's responsibility

Based on our conversation in slack and my own investigation, we need additional build controller configuration to gate CSI volume mounts. I think this is worth pursuing in a separate enhancement proposal, since ephemeral CSI drivers will be a net-new thing for OpenShift.

Enhancing SCCs probably needs to be added to the original EP for the projected resource CSI driver. We can only allow an SCC to allow all or no CSI volume mounts, which is not sufficient protection.

[nalind]

Currently, when multiple volumes are set to be mounted at a given location, we choose one of them and proceed. This will require some patching.

[adambkaplan]

I think this is something we can do in openshift/builder or in the build controller. Buildah itself doesn't have to fail here (unless you believe this is buggy behavior).

[adambkaplan]

@bparees @deads2k @sttts @mrunalp all comments have been addressed.

Final call for review - otherwise I think this is good to merge.

[bparees]

we should be able to generate our own volume names(both on the pod and in buildah) such that we avoid collisions. We don't have to use the volume name the user provides, when setting up volumes.

[adambkaplan]

True about creating our own volume names, though we get into fun situations where we need to validate/truncate superficially long volume names.

[adambkaplan]

@bparees I made this a "we can change this in a future enhancement" given the short window we have to land this in 4.8

[bparees]

that's reasonable.

[bparees]

btw i wasn't suggesting random suffixes. you don't have to use any part of the user given name, you can just call it "vol1", "vol2", etc. So no issue w/ truncation. Again we already do this when mounting the secrets/configmaps into the build pod. (generate our own volume names)

[bparees]

is there a risk that a user can create a build that mounts a secret or configmap that they themselves don't have permission to view?

i think we mitigate that in the existing secret injection logic, though i don't recall how. We need to ensure it is mitigated here as well.

[adambkaplan]

I don't see any such logic in the build controller. Secrets and ConfigMap volume mounts are local object references in the pod spec. My understanding is that if a Secret exists in a namespace, it can be used by any service account in said namespace.

[bparees]

i guess it is constrained by "you can't create builds if you can't create pods"

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[bparees]

/lgtm