Support mounts field on docker_environment

src/python/pants/core/util_rules/environments.py

@@ -201,6 +201,34 @@ class DockerImageField(StringField):
         """
     )
 
+class DockerBindMountsField(StringSequenceField):
+    alias = "bind_mounts"
+    help = help_text(
+        """
+        Read-only bind mounts to use when starting the container.
+
+        The values should be in the form "<path on host>:<path on container>"
+        """
+    )
+
+    @classmethod
+    def compute_value(
+        cls, raw_value: Optional[Iterable[str]], address: Address
+    ) -> Optional[Tuple[str, ...]]:
+        values = super().compute_value(raw_value, address)
+        if values and (bad_values := [val for val in values if len(val.split(":")) != 2]):
+            raise ValueError(
+                softwrap(
+                    f"""
+                    `bind_mounts` values must be of the form '<val>:<val>', but the following
+                    values for target '{address.target_name}' were not:
+
+                    {bullet_list(bad_values)}
+                    """
+                )
+            )
+
+        return values
 
 class DockerPlatformField(StringField):
     alias = "platform"
@@ -263,6 +291,7 @@ class DockerEnvironmentTarget(Target):
     core_fields = (
         *COMMON_TARGET_FIELDS,
         DockerImageField,
+        DockerBindMountsField,
         DockerPlatformField,
         DockerFallbackEnvironmentField,
     )
@@ -896,6 +925,7 @@ async def extract_process_config_from_environment(
     environments_subsystem: EnvironmentsSubsystem,
 ) -> ProcessExecutionEnvironment:
     docker_image = None
+    docker_bind_mounts = None
     remote_execution = False
     raw_remote_execution_extra_platform_properties: tuple[str, ...] = ()
 
@@ -909,6 +939,7 @@ async def extract_process_config_from_environment(
         docker_image = (
             tgt.val[DockerImageField].value if tgt.val.has_field(DockerImageField) else None
         )
+        docker_bind_mounts = tgt.val[DockerBindMountsField].value if tgt.val.has_field(DockerBindMountsField) else None
 
         # If a docker image name is provided, convert to an image ID so caching works properly.
         # TODO(17104): Append image ID instead to the image name.
@@ -937,6 +968,7 @@ async def extract_process_config_from_environment(
         environment_name=tgt.name,
         platform=platform.value,
         docker_image=docker_image,
+        docker_bind_mounts=docker_bind_mounts,
         remote_execution=remote_execution,
         remote_execution_extra_platform_properties=[
             tuple(pair.split("=", maxsplit=1))  # type: ignore[misc]

src/python/pants/engine/internals/native_engine.pyi

@@ -493,6 +493,7 @@ class ProcessExecutionEnvironment:
         environment_name: str | None,
         platform: str,
         docker_image: str | None,
+        docker_bind_mounts: Sequence[str] | None,
         remote_execution: bool,
         remote_execution_extra_platform_properties: Sequence[tuple[str, str]],
     ) -> None: ...

src/rust/engine/process_execution/docker/src/docker.rs

@@ -410,7 +410,7 @@ impl<'a> process_execution::CommandRunner for CommandRunner<'a> {
 
                 // Obtain ID of the base container in which to run the execution for this process.
                 let (container_id, named_caches) = {
-                    let ProcessExecutionStrategy::Docker(image) =
+                    let ProcessExecutionStrategy::Docker(image, bind_mounts) =
                         &req.execution_environment.strategy
                     else {
                         return Err(ProcessError::Unclassified(
@@ -423,6 +423,7 @@ impl<'a> process_execution::CommandRunner for CommandRunner<'a> {
                     self.container_cache
                         .container_for_image(
                             image,
+                            bind_mounts,
                             &req.execution_environment.platform,
                             &context.build_id,
                         )
@@ -740,6 +741,8 @@ impl Command {
 /// multiple tasks trying to access an initializing container do not try to start multiple
 /// containers.
 type CachedContainer = Arc<OnceCell<(String, NamedCaches)>>;
+/// Cache that maps (image name / bind mounts / platform) to a cached container.
+type ContainerMap = BTreeMap<(String, Option<Vec<String>>, Platform), CachedContainer>;
 
 /// Caches running containers so that build actions can be invoked by running "executions"
 /// within those cached containers.
@@ -749,8 +752,7 @@ pub(crate) struct ContainerCache<'a> {
     executor: Executor,
     work_dir_base: String,
     immutable_inputs_base_dir: String,
-    /// Cache that maps image name / platform to a cached container.
-    containers: Mutex<BTreeMap<(String, Platform), CachedContainer>>,
+    containers: Mutex<ContainerMap>,
 }
 
 impl<'a> ContainerCache<'a> {
@@ -794,6 +796,7 @@ impl<'a> ContainerCache<'a> {
         docker: Docker,
         executor: Executor,
         image_name: String,
+        mut bind_mounts: Vec<String>,
         platform: Platform,
         image_pull_scope: ImagePullScope,
         image_pull_cache: ImagePullCache,
@@ -816,17 +819,19 @@ impl<'a> ContainerCache<'a> {
             .await
             .map_err(|e| format!("Failed to create named cache volume for {image_name}: {e}"))?;
 
+        for mount in bind_mounts.iter_mut() {
+            mount.push_str(":ro");
+        }
+        bind_mounts.extend(vec![
+            format!("{work_dir_base}:{SANDBOX_BASE_PATH_IN_CONTAINER}"),
+            format!("{named_cache_volume_name}:{NAMED_CACHES_BASE_PATH_IN_CONTAINER}",),
+            // DOCKER-TODO: Consider making this bind mount read-only.
+            format!("{immutable_inputs_base_dir}:{IMMUTABLE_INPUTS_BASE_PATH_IN_CONTAINER}"),
+        ]);
         let config = bollard::container::Config {
             entrypoint: Some(vec!["/bin/sh".to_string()]),
             host_config: Some(bollard::service::HostConfig {
-                binds: Some(vec![
-                    format!("{work_dir_base}:{SANDBOX_BASE_PATH_IN_CONTAINER}"),
-                    format!("{named_cache_volume_name}:{NAMED_CACHES_BASE_PATH_IN_CONTAINER}",),
-                    // DOCKER-TODO: Consider making this bind mount read-only.
-                    format!(
-                        "{immutable_inputs_base_dir}:{IMMUTABLE_INPUTS_BASE_PATH_IN_CONTAINER}"
-                    ),
-                ]),
+                binds: Some(bind_mounts),
                 // The init process ensures that child processes are properly reaped.
                 init: Some(true),
                 ..bollard::service::HostConfig::default()
@@ -942,6 +947,7 @@ impl<'a> ContainerCache<'a> {
     pub async fn container_for_image(
         &self,
         image_name: &str,
+        bind_mounts: &Option<Vec<String>>,
         platform: &Platform,
         build_generation: &str,
     ) -> Result<(String, NamedCaches), String> {
@@ -951,7 +957,7 @@ impl<'a> ContainerCache<'a> {
         let container_id_cell = {
             let mut containers = self.containers.lock();
             let cell = containers
-                .entry((image_name.to_string(), *platform))
+                .entry((image_name.to_string(), bind_mounts.clone(), *platform))
                 .or_insert_with(|| Arc::new(OnceCell::new()));
             cell.clone()
         };
@@ -966,6 +972,7 @@ impl<'a> ContainerCache<'a> {
                     docker.clone(),
                     executor,
                     image_name.to_string(),
+                    bind_mounts.clone().unwrap_or_default(),
                     *platform,
                     image_pull_scope,
                     self.image_pull_cache.clone(),

src/rust/engine/process_execution/src/lib.rs

@@ -431,8 +431,8 @@ pub enum ProcessExecutionStrategy {
     Local,
     /// Stores the platform_properties.
     RemoteExecution(Vec<(String, String)>),
-    /// Stores the image name.
-    Docker(String),
+    /// Stores the image name and mount info.
+    Docker(String, Option<Vec<String>>),
 }
 
 impl ProcessExecutionStrategy {
@@ -444,15 +444,16 @@ impl ProcessExecutionStrategy {
             Self::RemoteExecution(_) => "remote_execution".to_string(),
             // NB: this image will include the container ID, thanks to
             // https://github.com/pantsbuild/pants/pull/17101.
-            Self::Docker(image) => format!("docker_execution: {image}"),
+            // NB: We don't include the mount info in the cache.
+            Self::Docker(image, _) => format!("docker_execution: {image}"),
         }
     }
 
     pub fn strategy_type(&self) -> &'static str {
         match self {
             Self::Local => "local",
             Self::RemoteExecution(_) => "remote",
-            Self::Docker(_) => "docker",
+            Self::Docker(_, _) => "docker",
         }
     }
 }
@@ -654,11 +655,11 @@ impl Process {
     ///
     /// Set the execution environment to Docker, with the specified image.
     ///
-    pub fn docker(mut self, image: String) -> Process {
+    pub fn docker(mut self, image: String, mount_info: Option<Vec<String>>) -> Process {
         self.execution_environment = ProcessExecutionEnvironment {
             name: None,
             platform: Platform::current().unwrap(),
-            strategy: ProcessExecutionStrategy::Docker(image),
+            strategy: ProcessExecutionStrategy::Docker(image, mount_info),
         };
         self
     }

src/rust/engine/src/context.rs

@@ -263,7 +263,7 @@ impl Core {
         let runner = Box::new(SwitchedCommandRunner::new(docker_runner, runner, |req| {
             matches!(
                 req.execution_environment.strategy,
-                ProcessExecutionStrategy::Docker(_)
+                ProcessExecutionStrategy::Docker(_, _)
             )
         }));
 

src/rust/engine/src/externs/process.rs

@@ -31,17 +31,21 @@ impl PyProcessExecutionEnvironment {
         remote_execution_extra_platform_properties: Vec<(String, String)>,
         environment_name: Option<String>,
         docker_image: Option<String>,
+        docker_bind_mounts: Option<Vec<String>>,
     ) -> PyResult<Self> {
         let platform = Platform::try_from(platform).map_err(PyValueError::new_err)?;
-        let strategy = match (docker_image, remote_execution) {
-            (None, true) => Ok(ProcessExecutionStrategy::RemoteExecution(
+        let strategy = match (docker_image, docker_bind_mounts, remote_execution) {
+            (None, None, true) => Ok(ProcessExecutionStrategy::RemoteExecution(
                 remote_execution_extra_platform_properties,
             )),
-            (None, false) => Ok(ProcessExecutionStrategy::Local),
-            (Some(image), false) => Ok(ProcessExecutionStrategy::Docker(image)),
-            (Some(_), true) => Err(PyAssertionError::new_err(
+            (None, None, false) => Ok(ProcessExecutionStrategy::Local),
+            (Some(image), mounts, false) => Ok(ProcessExecutionStrategy::Docker(image, mounts)),
+            (Some(_), _, true) => Err(PyAssertionError::new_err(
                 "docker_image cannot be set at the same time as remote_execution",
             )),
+            (None, Some(_), _) => Err(PyAssertionError::new_err(
+                "docker_bind_mounts cannot be set without docker_image",
+            )),
         }?;
         Ok(Self {
             environment: ProcessExecutionEnvironment {
@@ -104,7 +108,15 @@ impl PyProcessExecutionEnvironment {
     #[getter]
     fn docker_image(&self) -> Option<&str> {
         match &self.environment.strategy {
-            ProcessExecutionStrategy::Docker(image) => Some(image),
+            ProcessExecutionStrategy::Docker(image, _) => Some(image),
+            _ => None,
+        }
+    }
+
+    #[getter]
+    fn docker_bind_mounts(&self) -> Option<Vec<String>> {
+        match &self.environment.strategy {
+            ProcessExecutionStrategy::Docker(_, Some(bind_mounts)) => Some(bind_mounts.to_vec()),
             _ => None,
         }
     }

src/rust/engine/src/intrinsics.rs

@@ -641,7 +641,7 @@ fn interactive_process(
         (py_interactive_process.extract().unwrap(), py_process, process_config)
       });
       match process_config.environment.strategy {
-        ProcessExecutionStrategy::Docker(_) | ProcessExecutionStrategy::RemoteExecution(_) => {
+        ProcessExecutionStrategy::Docker(_, _) | ProcessExecutionStrategy::RemoteExecution(_) => {
           // TODO: #17182 covers adding support for running processes interactively in Docker.
           Err(
             format!(