added configuring of device mount permissions in udev discovery handler

#726 (#737) * added configuring of device mount permissions in udev discovery handler #726 Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * incremented version Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * formatting Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * fixed version Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * updated helm chart version Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * updated rust version Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * updated rust version to 1.84 Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * fixed default for permissions Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * added default func for permissions Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * updated rust version to 1.81 Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * Update discovery-handlers/udev/src/discovery_handler.rs Co-authored-by: Kate Goldenring <kate.goldenring@gmail.com> Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * added unit test and added dependencies Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * test for permissions Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * removed unused var Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * restructured test Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * fixed test Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * added default permissions for lint files Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * signoff Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> * removed panic Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> --------- Signed-off-by: Marcel Bindseil <marcelbindseil@gmail.com> Co-authored-by: Kate Goldenring <kate.goldenring@gmail.com>
project-akri · Jan 22, 2025 · 5e20ee4 · 5e20ee4
1 parent 8e5b3ab
commit 5e20ee4
Show file tree

Hide file tree

Showing 15 changed files with 79 additions and 27 deletions.
diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml
@@ -33,7 +33,7 @@ jobs:
     - name: Rust install
       uses: dtolnay/rust-toolchain@master
       with:
-        toolchain: 1.79.0
+        toolchain: 1.81.0
         components: clippy, rustfmt
     - name: Install Linux requirements
       # TODO: When ubuntu-latest gets updated to >= 23.04 replace the wget+unzip with just protobuf-compiler in apt

diff --git a/.github/workflows/run-tarpaulin.yml b/.github/workflows/run-tarpaulin.yml
@@ -16,7 +16,7 @@ on:
 
 env:
   CARGO_TERM_COLOR: always
-  CARGO_VERSION: 1.79.0
+  CARGO_VERSION: 1.81.0
 
 jobs:
   build:

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -23,10 +23,10 @@ members = [
 resolver = "2"
 
 [workspace.package]
-version = "0.13.9"
+version = "0.13.10"
 edition = "2021"
 license = "Apache-2.0"
 homepage = "https://docs.akri.sh/"
 repository = "https://github.com/project-akri/akri"
-rust-version = "1.79"
+rust-version = "1.81"
 authors = ["The Akri Team"]
diff --git a/README.md b/README.md
@@ -1,7 +1,7 @@
 <p align="center"><img src="https://github.com/project-akri/akri-docs/blob/main/art/logo-horizontal/akri-logo-horizontal-light.svg" alt="Akri Logo" width="300"></p>
 
 [![Slack channel #akri](https://img.shields.io/badge/slack-akri-blueviolet.svg?logo=slack)](https://kubernetes.slack.com/messages/akri)
-[![Rust Version](https://img.shields.io/badge/rustc-1.79.0-blue.svg)](https://blog.rust-lang.org/2023/03/31/Rust-1.79.0.html)
+[![Rust Version](https://img.shields.io/badge/rustc-1.81.0-blue.svg)](https://blog.rust-lang.org/2025/01/09/Rust-1.81.0.html)
 [![Kubernetes Version](https://img.shields.io/badge/kubernetes-≥%201.16-blue.svg)](https://kubernetes.io/)
 [![codecov](https://codecov.io/gh/project-akri/akri/branch/main/graph/badge.svg?token=V468HO7CDE)](https://codecov.io/gh/project-akri/akri)
 [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/5339/badge)](https://bestpractices.coreinfrastructure.org/projects/5339)

diff --git a/build/containers/Dockerfile.rust b/build/containers/Dockerfile.rust
@@ -1,6 +1,6 @@
 FROM --platform=$BUILDPLATFORM tonistiigi/xx:master AS xx
 
-FROM  --platform=$BUILDPLATFORM rust:1.79-slim-bookworm AS build
+FROM  --platform=$BUILDPLATFORM rust:1.81-slim-bookworm AS build
 RUN rustup component add rustfmt
 RUN apt-get update && apt-get install -y clang lld protobuf-compiler pkg-config mmdebstrap wget
 COPY --from=xx / /

diff --git a/build/setup.sh b/build/setup.sh
@@ -23,10 +23,10 @@ then
     if [ -x "$(command -v sudo)" ];
     then
         echo "Install rustup"
-        sudo curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain=1.79.0
+        sudo curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain=1.81.0
     else
         echo "Install rustup"
-        curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain=1.79.0
+        curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain=1.81.0
     fi
 else
     echo "Found rustup"

diff --git a/deployment/helm/Chart.yaml b/deployment/helm/Chart.yaml
@@ -16,9 +16,9 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.13.9
+version: 0.13.10
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
-appVersion: 0.13.9
+appVersion: 0.13.10
diff --git a/deployment/helm/templates/udev-configuration.yaml b/deployment/helm/templates/udev-configuration.yaml
@@ -10,6 +10,7 @@ spec:
       groupRecursive: {{ .Values.udev.configuration.discoveryDetails.groupRecursive }}
       udevRules:
       {{- required "Please set at least one udev rule with `--set udev.configuration.discoveryDetails.udevRules[0]==\"<udev rule>\"' to specify what you want discovered. See the udev Configuration document at https://docs.akri.sh/discovery-handlers/udev for more information." .Values.udev.configuration.discoveryDetails.udevRules | toYaml | nindent 6 }}
+      permissions: {{ .Values.udev.configuration.discoveryDetails.permissions }}
   {{- if or .Values.udev.configuration.brokerPod.image.repository .Values.udev.configuration.brokerJob.image.repository }}
   {{- /* Only add brokerSpec if a broker image is provided */}}
   brokerSpec:

diff --git a/deployment/helm/values.yaml b/deployment/helm/values.yaml
@@ -714,6 +714,9 @@ udev:
       # udevRules is the list of udev rules used to find instances created as a result of
       # applying this udev configuration
       udevRules:
+      # permissions is the list of Cgroups permissions of the device
+      # combination of r (read), w (write), m (modify)
+      permissions: rwm
     # capacity is the capacity for any instances created as a result of
     # applying this udev configuration
     capacity: 1

diff --git a/discovery-handlers/udev/src/discovery_handler.rs b/discovery-handlers/udev/src/discovery_handler.rs
@@ -12,6 +12,7 @@ use akri_discovery_utils::discovery::{
 };
 use async_trait::async_trait;
 use log::{error, info, trace};
+use serde::{de, Deserialize, Deserializer};
 use std::collections::{HashMap, HashSet};
 use std::time::Duration;
 use tokio::sync::mpsc;
@@ -30,6 +31,34 @@ pub struct UdevDiscoveryDetails {
 
     #[serde(default)]
     pub group_recursive: bool,
+
+    #[serde(default = "default_permissions")]
+    #[serde(deserialize_with = "validate_permissions")]
+    pub permissions: String,
+}
+
+// Validate the permissible set of cgroups `permissions`
+fn validate_permissions<'de, D>(deserializer: D) -> Result<String, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let value: String = Deserialize::deserialize(deserializer)?;
+
+    // Validating that the string only contains allowed combinations of 'r', 'w', 'm'
+    let valid_permissions = ["r", "w", "m", "rw", "rm", "rwm", "wm"];
+    if valid_permissions.contains(&value.as_str()) {
+        Ok(value)
+    } else {
+        Err(de::Error::invalid_value(
+            de::Unexpected::Str(&value),
+            &"a valid permission combination ('r', 'w', 'm', 'rw', 'rm', 'rwm', 'wm')",
+        ))
+    }
+}
+
+/// Default permissions for devices
+fn default_permissions() -> String {
+    "rwm".to_string()
 }
 
 /// `DiscoveryHandlerImpl` discovers udev instances by parsing the udev rules in `discovery_handler_config.udev_rules`.
@@ -105,7 +134,7 @@ impl DiscoveryHandler for DiscoveryHandlerImpl {
                                 device_specs.push(DeviceSpec {
                                     container_path: devnode.clone(),
                                     host_path: devnode,
-                                    permissions: "rwm".to_string(),
+                                    permissions: discovery_handler_config.permissions.clone(),
                                 })
                             }
                         }
@@ -178,7 +207,8 @@ mod tests {
         let udev_dh_config: UdevDiscoveryDetails = deserialize_discovery_details(yaml).unwrap();
         assert!(udev_dh_config.udev_rules.is_empty());
         let serialized = serde_json::to_string(&udev_dh_config).unwrap();
-        let expected_deserialized = r#"{"udevRules":[],"groupRecursive":false}"#;
+        let expected_deserialized =
+            r#"{"udevRules":[],"groupRecursive":false,"permissions":"rwm"}"#;
         assert_eq!(expected_deserialized, serialized);
     }
 
@@ -187,9 +217,24 @@ mod tests {
         let yaml = r#"
           udevRules:
           - 'KERNEL=="video[0-9]*"'
+          permissions: rwm
         "#;
         let udev_dh_config: UdevDiscoveryDetails = deserialize_discovery_details(yaml).unwrap();
         assert_eq!(udev_dh_config.udev_rules.len(), 1);
         assert_eq!(&udev_dh_config.udev_rules[0], "KERNEL==\"video[0-9]*\"");
+        assert_eq!(&udev_dh_config.permissions, "rwm");
+    }
+
+    #[test]
+    fn test_deserialize_discovery_details_permissions_invalid() {
+        let yaml = r#"
+          udevRules:
+          - 'KERNEL=="video[0-9]*"'
+          permissions: xyz
+        "#;
+        match deserialize_discovery_details::<UdevDiscoveryDetails>(yaml) {
+            Ok(_) => panic!("Expected error parsing invalid permissions"),
+            Err(e) => assert!(e.to_string().contains("a valid permission combination")),
+        }
     }
 }
diff --git a/test/helm-lint-values-jobs.yaml b/test/helm-lint-values-jobs.yaml
@@ -22,6 +22,7 @@ udev:
     discoveryDetails:
       udevRules:
         - 'KERNEL=="video[0-9]*"'
+      permissions: "rwm"
     brokerJob:
       image:
         repository: "busybox"

diff --git a/test/helm-lint-values.yaml b/test/helm-lint-values.yaml
@@ -22,6 +22,7 @@ udev:
       groupRecursive: true
       udevRules:
         - 'KERNEL=="video[0-9]*"'
+      permissions: "rwm"
     brokerPod:
       image:
         repository: "nginx"

diff --git a/test/yaml/akri-udev-video-configuration.yaml b/test/yaml/akri-udev-video-configuration.yaml
@@ -8,6 +8,7 @@ spec:
     discoveryDetails: |+
         udevRules:
         -  'KERNEL=="video[0-9]*"'
+        permissions: "rwm"
   brokerSpec:
     brokerPodSpec:
       containers:

diff --git a/version.txt b/version.txt
@@ -1 +1 @@
-0.13.9
+0.13.10
-Original file line number
+Diff line change
@@ Expand Up / @@ -16,7 +16,7 @@ on: @@
     env:
       CARGO_TERM_COLOR: always
-      CARGO_VERSION: 1.79.0
+      CARGO_VERSION: 1.81.0
     jobs:
       build:
@@ Expand Down @@