design 2 - volume api

enhancements/builds/volume-secrets.md

@@ -40,7 +40,7 @@ There are a few undesirable aspects to this:
 selected.
 2) Requires extra work by the user in the Dockerfile, so each Dockerfile must be customized
 
-This enhancement proposes to use buildah's capability to mount a volume at build time.  The content mounted into the build pod would be then mounted into the container processing the Dockerfile, making that content available within the container so Dockerfile commands could reference it.  No explicit `ADD` would be required, and since mounted content is not committed to the resulting image, no `RUN rm` equivalent is required to clean up the injected content.
+This enhancement proposes to introduce an option to use buildah's capability to mount a volume at build time.  The content mounted into the build pod would be then mounted into the container processing the Dockerfile, making that content available within the container so Dockerfile commands could reference it.  No explicit `ADD` would be required, and since mounted content is not committed to the resulting image, no `RUN rm` equivalent is required to clean up the injected content.
 
 
 ## Motivation
@@ -50,10 +50,11 @@ This enhancement proposes to use buildah's capability to mount a volume at build
 * Simplify how users consume secret + configmap content in builds
 * Increase the security of protected content being injected to images
 * Simplify use cases like mounting subscription entitlement credentials into a build so subscription content can be pulled down during a build while not including the credentials in the resulting image.
+* Eventually extend this api to allow the mounting of traditional volumes (such as those backed by persistent storage)
 
 ### Non-Goals
 
-* This enhancement is not related to the frequently requested "build volumes" feature which would make it possible to mount a volume during a build and read/write content on that volume during the build such as to cache dependencies.
+* This enhancement should not result in a change of behavior for users of the existing secret/configmap injection api.
 
 
 ## Proposal
@@ -62,14 +63,76 @@ This enhancement proposes to use buildah's capability to mount a volume at build
 
 The enabled use cases are essentially identical to what can be done with the configmap/secret input api in builds today, but with a better user experience and security as discussed above.  It does not enable a new use case that is not already possible today, except that layer squashing will not be required.
 
+Future extensions to this enhancement could enable the use case of providing build input content from a persistent volume and allowing the build to store/cache content for future builds on such a volume.  Those will be discussed in the future enhancement at that time.
+
 ### Implementation Details/Notes/Constraints [optional]
 
-We will need to introduce a new field in the build api which allows the user to indicate that they want to use this mechanism to inject their configmap/secret content, rather than the existing mechanism.
+We will need to introduce a new mechanism in the build api which allows the user to indicate that they want to inject "volume" content into the build.  Initially the only allowed volume types will be configmaps and secrets.  The api will otherwise be similar to the existing secret/configmap injection api in which users identify the configmap/secret and the target path for injection.  
+
+This will be done by adding a Volume[] field to the Source and Docker strategy structs.  The Volume[] field will allow 
+defining volumes to be mounted to the build pod in the same way that a normal pod allows for this.  Similarly a VolumeMount[]
+field will be added, but without the MountPropagation and SubPath fields.  MountPropagation and SubPath can be considered
+for support in the future.
+
+These fields will be wired, via the build controller, directly to the build pod that is constructed, modulo some validation logic to constrain the types of Volumes we want to support (initially just configmaps and secrets).  In addition all mounts
+into the pod will be done at a path of our choosing, not the VolumeMount path specified, to ensure the user cannot 
+overwrite critical function inside the build pod and use it as an escalation pathway.
+
+The logic that invokes buildah will then pass the mounted directories as volume mount arguments.  The mount path provided
+to buildah will be determined from the VolumeMount specification.
+
+Proposed api/structs:
+Note:  DockerBuildStrategy will be updated in the same way.
+```
+// SourceBuildStrategy defines input parameters specific to an Source build.
+type SourceBuildStrategy struct {
+  // From is reference to an DockerImage, ImageStream, ImageStreamTag, or ImageStreamImage from which
+  // the docker image should be pulled
+  From kapi.ObjectReference
+
+  // PullSecret is the name of a Secret that would be used for setting up
+  // the authentication for pulling the Docker images from the private Docker
+  // registries
+  PullSecret *kapi.LocalObjectReference
+
+  // Env contains additional environment variables you want to pass into a builder container.
+  Env []kapi.EnvVar
+
+  // Scripts is the location of Source scripts
+  Scripts string
+
+  // Incremental flag forces the Source build to do incremental builds if true.
+  Incremental *bool
 
-The build pod will continue to mount the content as it already does, however the logic that generates the dockerfile(for s2i) and invokes buildah(for both dockerfile + s2i builds) will need to change to not augment the dockerfile, but instead to pass volume mount paths to buildah.  It will mount the content from the path it was mounted into the build pod, and mount it to the destination path provided by the user (the same path where the logic previously injected it via the dockerfile `ADD` command).
+  // ForcePull describes if the builder should pull the images from registry prior to building.
+  ForcePull bool
 
-proposed api:
+  // Volumes is a list of volumes that can be mounted by the build
+  // More info: https://kubernetes.io/docs/concepts/storage/volumes
+  Volumes []kapi.Volume
 
+  // VolumeMounts are volumes to mount into the build
+  VolumeMounts []VolumeMount
+}
+```
+
+
+```
+// VolumeMount describes a mounting of a Volume within the build environment.
+type VolumeMount struct {
+  // This must match the Name of a Volume.
+  Name string
+  // Mounted read-only if true, read-write otherwise (false or unspecified).
+  // Defaults to false.
+  // +optional
+  ReadOnly bool
+  // Path within the build environment at which the volume should be mounted.  Must
+  // not contain ':'.
+  MountPath string
+}
+```
+
+Example usage (use of the existing secret/configmap injection api is included for comparison, it is not changing)
 ```
 apiVersion: v1
 items:
@@ -79,22 +142,52 @@ items:
     name: mybuild
     namespace: p1
   spec:
+    strategy:
+      sourceStrategy:
+        from:
+          kind: ImageStreamTag
+          name: nodejs:10-SCL
+          namespace: openshift
+        volumes:
+        - name: secret
+          secret:
+            secretName: somesecret
+        - name: config
+          configMap:
+            name: someconfigmap
+            items:
+            - key: somekey
+              path: volume/path/value.txt
+        volumeMounts:
+        - name: config
+          mountPath: /tmp/config
+        - name: secret
+          mountPath: /tmp/secret
+      type: Source
     source:
       secrets:
       - secret: 
-          name: mySecret
-        destinationDir: /etc/subscriptions
-        useVolume: true
-      - configMaps:
-        configMap:
-          name: myConfigMap
-        destinationDir: /tmp/config
-        useVolume: false # default
+          name: myOtherSecret
+        destinationDir: /tmp/othersecret
+      configMaps:
+      - configMap:
+          name: myOtherConfigMap
+        destinationDir: /tmp/otherconfig
 ```
 
 ### Risks and Mitigations
 
-NA
+Since the build pod is privileged, we need to ensure that users cannot abuse this api to trick the build controller
+into creating build pods that can exploit those privileges.  This means ensuring that any volume mount specifications
+the user provides, which are translated into volumemounts in the build pod, cannot be used to alter the build logic.
+To this end, we should explicitly control where the volumes are mounted within the build pod.  (We can mount them 
+anywhere the user specifies within the buildah container).  
+
+We also need to ensure that the user can't use this api to inject/mount content that they could not normally mount
+into a pod they created themselves.  For this reason we must explicitly disallow `HostPath` volume types, for example.
+We will mitigate this by whitelisting the volume types we support, starting with only allowing ConfigMaps and Secrets.
+As additional types are whitelisted, we will need to determine it is safe to add them.
+
 
 ## Design Details
 
@@ -126,4 +219,6 @@ Additional user complexity in choosing when to enable this behavior.  It is also
 
 ## Alternatives
 
-N/A
+Updating the existing injection apis to have a "asVolume" field was considered as it would be a simpler implementation (more code reuse) but it was rejected as there is a long term goal to allow builds to mount traditional volumes as well.  The existing injection api can't easily be extended to support such a thing, so the design proposed in this enhancement is a better stepping stone to that goal.  This also puts us on a better path to support using volumes for caching build content between build
+invocations which has been a longtime goal of the build api.
+