From c22ac4f72c9025bed9ed8257ade65a0633efa022 Mon Sep 17 00:00:00 2001 From: Dan Mihai Date: Thu, 11 Jul 2024 20:08:54 +0000 Subject: [PATCH] genpolicy: add bind mounts for image volumes Add bind mounts for volumes defined by docker container images, unless those mounts have been defined in the input K8s YAML file too. For example, quay.io/opstree/redis defines two mounts: /data /node-conf Before these changes, if these mounts were not defined in the YAML file too, the auto-generated policy did not allow this container image to start. Signed-off-by: Dan Mihai --- src/tools/genpolicy/genpolicy-settings.json | 12 ++++ src/tools/genpolicy/src/cronjob.rs | 16 +++-- src/tools/genpolicy/src/daemon_set.rs | 16 +++-- src/tools/genpolicy/src/deployment.rs | 16 +++-- src/tools/genpolicy/src/job.rs | 16 +++-- src/tools/genpolicy/src/mount_and_storage.rs | 60 +++++++++++++++++++ src/tools/genpolicy/src/pod.rs | 16 +++-- src/tools/genpolicy/src/registry.rs | 29 +++++++-- .../genpolicy/src/registry_containerd.rs | 4 +- src/tools/genpolicy/src/replica_set.rs | 16 +++-- .../genpolicy/src/replication_controller.rs | 16 +++-- src/tools/genpolicy/src/settings.rs | 12 ++++ src/tools/genpolicy/src/stateful_set.rs | 18 +++--- src/tools/genpolicy/src/yaml.rs | 41 +++++++++---- .../kubernetes/k8s-policy-deployment.bats | 47 +++++++++++++++ .../kubernetes/run_kubernetes_tests.sh | 1 + .../k8s-policy-deployment.yaml | 36 +++++++++++ 17 files changed, 280 insertions(+), 92 deletions(-) create mode 100644 tests/integration/kubernetes/k8s-policy-deployment.bats create mode 100644 tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-deployment.yaml diff --git a/src/tools/genpolicy/genpolicy-settings.json b/src/tools/genpolicy/genpolicy-settings.json index aa22ec882c..95972de77c 100644 --- a/src/tools/genpolicy/genpolicy-settings.json +++ b/src/tools/genpolicy/genpolicy-settings.json @@ -189,6 +189,18 @@ "rprivate", "ro" ] + }, + "image_volume": { + "mount_type": "bind", + "mount_source": "$(sfprefix)", + "driver": "local", + "source": "local", + "fstype": "bind", + "options": [ + "rbind", + "rprivate", + "rw" + ] } }, "mount_destinations": [ diff --git a/src/tools/genpolicy/src/cronjob.rs b/src/tools/genpolicy/src/cronjob.rs index f920095b41..0a480abde6 100644 --- a/src/tools/genpolicy/src/cronjob.rs +++ b/src/tools/genpolicy/src/cronjob.rs @@ -94,15 +94,13 @@ impl yaml::K8sResource for CronJob { container: &pod::Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.jobTemplate.spec.template.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ); - } + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.jobTemplate.spec.template.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/daemon_set.rs b/src/tools/genpolicy/src/daemon_set.rs index 4616551d1a..3e0a6ee44b 100644 --- a/src/tools/genpolicy/src/daemon_set.rs +++ b/src/tools/genpolicy/src/daemon_set.rs @@ -96,15 +96,13 @@ impl yaml::K8sResource for DaemonSet { container: &pod::Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.template.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ) - } + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.template.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/deployment.rs b/src/tools/genpolicy/src/deployment.rs index 2296bc9eb2..adc7f9778d 100644 --- a/src/tools/genpolicy/src/deployment.rs +++ b/src/tools/genpolicy/src/deployment.rs @@ -94,15 +94,13 @@ impl yaml::K8sResource for Deployment { container: &pod::Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.template.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ); - } + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.template.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/job.rs b/src/tools/genpolicy/src/job.rs index 72d7efd2d4..e686d333c9 100644 --- a/src/tools/genpolicy/src/job.rs +++ b/src/tools/genpolicy/src/job.rs @@ -68,15 +68,13 @@ impl yaml::K8sResource for Job { container: &pod::Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.template.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ); - } + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.template.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/mount_and_storage.rs b/src/tools/genpolicy/src/mount_and_storage.rs index fd64b3d1df..0e5a5da193 100644 --- a/src/tools/genpolicy/src/mount_and_storage.rs +++ b/src/tools/genpolicy/src/mount_and_storage.rs @@ -106,6 +106,11 @@ pub fn get_mount_and_storage( yaml_volume: &volume::Volume, yaml_mount: &pod::VolumeMount, ) { + debug!( + "get_mount_and_storage: adding mount and storage for: {:?}", + &yaml_volume + ); + if let Some(emptyDir) = &yaml_volume.emptyDir { let settings_volumes = &settings.volumes; let mut volume: Option<&settings::EmptyDirVolume> = None; @@ -351,3 +356,58 @@ fn get_downward_api_mount(yaml_mount: &pod::VolumeMount, p_mounts: &mut Vec, + storages: &mut Vec, + destination: &str, +) { + // https://github.com/kubernetes/examples/blob/master/cassandra/image/Dockerfile + // has a volume mount starting with two '/' characters: + // + // CASSANDRA_DATA=/cassandra_data + // VOLUME ["/$CASSANDRA_DATA"] + let mut destination_string = destination.to_string(); + while destination_string.contains("//") { + destination_string = destination_string.replace("//", "/"); + } + debug!("get_image_mount_and_storage: image dest = {destination}, dest = {destination_string}"); + + for mount in &mut *p_mounts { + if mount.destination == destination_string { + debug!( + "get_image_mount_and_storage: mount {destination_string} already defined by YAML" + ); + return; + } + } + + let settings_image = &settings.volumes.image_volume; + debug!( + "get_image_mount_and_storage: settings for container image volumes: {:?}", + settings_image + ); + + storages.push(agent::Storage { + driver: settings_image.driver.clone(), + driver_options: Vec::new(), + source: settings_image.source.clone(), + fstype: settings_image.fstype.clone(), + options: settings_image.options.clone(), + mount_point: destination_string.clone(), + fs_group: protobuf::MessageField::none(), + special_fields: ::protobuf::SpecialFields::new(), + }); + + let file_name = Path::new(&destination_string).file_name().unwrap(); + let name = OsString::from(file_name).into_string().unwrap(); + let source = format!("{}{name}$", &settings_image.mount_source); + + p_mounts.push(policy::KataMount { + destination: destination_string, + type_: settings_image.fstype.clone(), + source, + options: settings_image.options.clone(), + }); +} diff --git a/src/tools/genpolicy/src/pod.rs b/src/tools/genpolicy/src/pod.rs index f91787e348..4fda029164 100644 --- a/src/tools/genpolicy/src/pod.rs +++ b/src/tools/genpolicy/src/pod.rs @@ -842,15 +842,13 @@ impl yaml::K8sResource for Pod { container: &Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ); - } + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/registry.rs b/src/tools/genpolicy/src/registry.rs index 1bee737e3c..af41737eaa 100644 --- a/src/tools/genpolicy/src/registry.rs +++ b/src/tools/genpolicy/src/registry.rs @@ -23,12 +23,15 @@ use oci_distribution::{ }; use serde::{Deserialize, Serialize}; use sha2::{digest::typenum::Unsigned, digest::OutputSizeUser, Sha256}; -use std::{fs::OpenOptions, io, io::BufWriter, io::Seek, io::Write, path::Path}; +use std::{ + collections::BTreeMap, fs::OpenOptions, io, io::BufWriter, io::Seek, io::Write, path::Path, +}; use tokio::io::AsyncWriteExt; /// Container image properties obtained from an OCI repository. #[derive(Clone, Debug, Default)] pub struct Container { + pub image: String, pub config_layer: DockerConfigLayer, pub image_layers: Vec, } @@ -37,19 +40,20 @@ pub struct Container { #[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct DockerConfigLayer { architecture: String, - config: DockerImageConfig, + pub config: DockerImageConfig, pub rootfs: DockerRootfs, } -/// Image config properties. +/// See: https://docs.docker.com/reference/dockerfile/. #[derive(Clone, Debug, Default, Deserialize, Serialize)] -struct DockerImageConfig { +pub struct DockerImageConfig { User: Option, Tty: Option, Env: Option>, Cmd: Option>, WorkingDir: Option, Entrypoint: Option>, + pub Volumes: Option>, } /// Container rootfs information. @@ -66,11 +70,21 @@ pub struct ImageLayer { pub verity_hash: String, } +/// See https://docs.docker.com/reference/dockerfile/#volume. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct DockerVolumeHostDirectory { + // This struct is empty because, according to the documentation: + // "The VOLUME instruction does not support specifying a host-dir + // parameter. You must specify the mountpoint when you create or + // run the container." +} + impl Container { pub async fn new(config: &Config, image: &str) -> Result { info!("============================================"); - info!("Pulling manifest and config for {:?}", image); - let reference: Reference = image.to_string().parse().unwrap(); + info!("Pulling manifest and config for {image}"); + let image_string = image.to_string(); + let reference: Reference = image_string.parse().unwrap(); let auth = build_auth(&reference); let mut client = Client::new(ClientConfig { @@ -96,6 +110,8 @@ impl Container { let config_layer: DockerConfigLayer = serde_json::from_str(&config_layer_str).unwrap(); + debug!("config_layer: {:?}", &config_layer); + let image_layers = get_image_layers( config.layers_cache_file_path.clone(), &mut client, @@ -107,6 +123,7 @@ impl Container { .unwrap(); Ok(Container { + image: image_string, config_layer, image_layers, }) diff --git a/src/tools/genpolicy/src/registry_containerd.rs b/src/tools/genpolicy/src/registry_containerd.rs index 60653d917e..6e0d6af3f0 100644 --- a/src/tools/genpolicy/src/registry_containerd.rs +++ b/src/tools/genpolicy/src/registry_containerd.rs @@ -46,7 +46,8 @@ impl Container { let ctrd_client = containerd_client::Client::from(containerd_channel.clone()); let k8_cri_image_client = ImageServiceClient::new(containerd_channel); - let image_ref: Reference = image.to_string().parse().unwrap(); + let image_str = image.to_string(); + let image_ref: Reference = image_str.parse().unwrap(); info!("Pulling image: {:?}", image_ref); @@ -67,6 +68,7 @@ impl Container { .await?; Ok(Container { + image: image_str, config_layer, image_layers, }) diff --git a/src/tools/genpolicy/src/replica_set.rs b/src/tools/genpolicy/src/replica_set.rs index f4a4e5f68e..04d46e1b81 100644 --- a/src/tools/genpolicy/src/replica_set.rs +++ b/src/tools/genpolicy/src/replica_set.rs @@ -66,15 +66,13 @@ impl yaml::K8sResource for ReplicaSet { container: &pod::Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.template.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ); - } + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.template.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/replication_controller.rs b/src/tools/genpolicy/src/replication_controller.rs index c23fd1fdc7..0c7583d816 100644 --- a/src/tools/genpolicy/src/replication_controller.rs +++ b/src/tools/genpolicy/src/replication_controller.rs @@ -68,15 +68,13 @@ impl yaml::K8sResource for ReplicationController { container: &pod::Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.template.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ); - } + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.template.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/settings.rs b/src/tools/genpolicy/src/settings.rs index 80d1ac40fc..949f6ad274 100644 --- a/src/tools/genpolicy/src/settings.rs +++ b/src/tools/genpolicy/src/settings.rs @@ -34,6 +34,7 @@ pub struct Volumes { pub emptyDir_memory: EmptyDirVolume, pub configMap: ConfigMapVolume, pub confidential_configMap: ConfigMapVolume, + pub image_volume: ImageVolume, } /// EmptyDir volume settings loaded from genpolicy-settings.json. @@ -59,6 +60,17 @@ pub struct ConfigMapVolume { pub options: Vec, } +/// Container image volume settings loaded from genpolicy-settings.json. +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct ImageVolume { + pub mount_type: String, + pub mount_source: String, + pub driver: String, + pub source: String, + pub fstype: String, + pub options: Vec, +} + /// Data corresponding to the kata runtime config file data, loaded from /// genpolicy-settings.json. #[derive(Clone, Debug, Serialize, Deserialize)] diff --git a/src/tools/genpolicy/src/stateful_set.rs b/src/tools/genpolicy/src/stateful_set.rs index 1c712002b7..e0c0325f42 100644 --- a/src/tools/genpolicy/src/stateful_set.rs +++ b/src/tools/genpolicy/src/stateful_set.rs @@ -116,16 +116,6 @@ impl yaml::K8sResource for StatefulSet { container: &pod::Container, settings: &settings::Settings, ) { - if let Some(volumes) = &self.spec.template.spec.volumes { - yaml::get_container_mounts_and_storages( - policy_mounts, - storages, - container, - settings, - volumes, - ); - } - // Example: // // containers: @@ -150,6 +140,14 @@ impl yaml::K8sResource for StatefulSet { StatefulSet::get_mounts_and_storages(policy_mounts, volume_mounts, claims); } } + + yaml::get_container_mounts_and_storages( + policy_mounts, + storages, + container, + settings, + &self.spec.template.spec.volumes, + ); } fn generate_policy(&self, agent_policy: &policy::AgentPolicy) -> String { diff --git a/src/tools/genpolicy/src/yaml.rs b/src/tools/genpolicy/src/yaml.rs index 8a6b1404a4..6597ee53e1 100644 --- a/src/tools/genpolicy/src/yaml.rs +++ b/src/tools/genpolicy/src/yaml.rs @@ -278,23 +278,40 @@ pub fn get_container_mounts_and_storages( storages: &mut Vec, container: &pod::Container, settings: &settings::Settings, - volumes: &Vec, + volumes_option: &Option>, ) { - if let Some(volume_mounts) = &container.volumeMounts { - for volume in volumes { - for volume_mount in volume_mounts { - if volume_mount.name.eq(&volume.name) { - mount_and_storage::get_mount_and_storage( - settings, - policy_mounts, - storages, - volume, - volume_mount, - ); + if let Some(volumes) = volumes_option { + if let Some(volume_mounts) = &container.volumeMounts { + for volume in volumes { + for volume_mount in volume_mounts { + if volume_mount.name.eq(&volume.name) { + mount_and_storage::get_mount_and_storage( + settings, + policy_mounts, + storages, + volume, + volume_mount, + ); + } } } } } + + // Add storage and mount for each volume defined in the docker container image + // configuration layer. + if let Some(volumes) = &container.registry.config_layer.config.Volumes { + for volume in volumes { + debug!("get_container_mounts_and_storages: {:?}", &volume); + + mount_and_storage::get_image_mount_and_storage( + settings, + policy_mounts, + storages, + volume.0, + ); + } + } } /// Add the "io.katacontainers.config.agent.policy" annotation into diff --git a/tests/integration/kubernetes/k8s-policy-deployment.bats b/tests/integration/kubernetes/k8s-policy-deployment.bats new file mode 100644 index 0000000000..8919c7dae1 --- /dev/null +++ b/tests/integration/kubernetes/k8s-policy-deployment.bats @@ -0,0 +1,47 @@ +#!/usr/bin/env bats +# +# Copyright (c) 2024 Microsoft. +# +# SPDX-License-Identifier: Apache-2.0 +# + +load "${BATS_TEST_DIRNAME}/../../common.bash" +load "${BATS_TEST_DIRNAME}/tests_common.sh" + +setup() { + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." + + get_pod_config_dir + + deployment_name="policy-redis-deployment" + deployment_yaml="${pod_config_dir}/k8s-policy-deployment.yaml" + + # Add an appropriate policy to the correct YAML file. + policy_settings_dir="$(create_tmp_policy_settings_dir "${pod_config_dir}")" + add_requests_to_policy_settings "${policy_settings_dir}" "ReadStreamRequest" + auto_generate_policy "${policy_settings_dir}" "${deployment_yaml}" +} + +@test "Successful deployment with auto-generated policy and container image volumes" { + # Initiate deployment + kubectl apply -f "${deployment_yaml}" + + # Wait for the deployment to be created + cmd="kubectl rollout status --timeout=1s deployment/${deployment_name} | grep 'successfully rolled out'" + info "Waiting for: ${cmd}" + waitForProcess "${wait_time}" "${sleep_time}" "${cmd}" +} + +teardown() { + auto_generate_policy_enabled || skip "Auto-generated policy tests are disabled." + + # Debugging information + info "Deployment ${deployment_name}:" + kubectl describe deployment "${deployment_name}" + kubectl rollout status deployment/${deployment_name} + + # Clean-up + kubectl delete deployment "${deployment_name}" + + delete_tmp_policy_settings_dir "${policy_settings_dir}" +} diff --git a/tests/integration/kubernetes/run_kubernetes_tests.sh b/tests/integration/kubernetes/run_kubernetes_tests.sh index 2cbd9a0efa..51a12dbe30 100755 --- a/tests/integration/kubernetes/run_kubernetes_tests.sh +++ b/tests/integration/kubernetes/run_kubernetes_tests.sh @@ -57,6 +57,7 @@ else "k8s-pid-ns.bats" \ "k8s-pod-quota.bats" \ "k8s-policy-hard-coded.bats" \ + "k8s-policy-deployment.bats" \ "k8s-policy-job.bats" \ "k8s-policy-pod.bats" \ "k8s-policy-pvc.bats" \ diff --git a/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-deployment.yaml b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-deployment.yaml new file mode 100644 index 0000000000..407b997290 --- /dev/null +++ b/tests/integration/kubernetes/runtimeclass_workloads/k8s-policy-deployment.yaml @@ -0,0 +1,36 @@ +# +# Copyright (c) 2024 Microsoft +# +# SPDX-License-Identifier: Apache-2.0 +# +apiVersion: apps/v1 +kind: Deployment +metadata: + name: policy-redis-deployment + labels: + app: policyredis +spec: + selector: + matchLabels: + app: policyredis + role: master + tier: backend + replicas: 1 + template: + metadata: + labels: + app: policyredis + role: master + tier: backend + spec: + terminationGracePeriodSeconds: 0 + runtimeClassName: kata + containers: + - name: master + image: quay.io/opstree/redis + resources: + requests: + cpu: 100m + memory: 100Mi + ports: + - containerPort: 6379