Crash fixed when no tag is provided for the container image (#40)

Author: Siegfried Weber

.gitignore

@@ -0,0 +1 @@
+target/

Cargo.lock

@@ -30,3 +30,8 @@ thiserror = "1.0"
 url = "2.2"
 pnet = "0.26.0"
 stackable_config = { git = "https://github.com/stackabletech/common.git", branch = "main" }
+
+[dev-dependencies]
+indoc = "1.0"
+rstest = "0.6"
+serde_yaml = "0.8"

Cargo.toml

@@ -3,6 +3,7 @@ use std::fs;
 use std::path::PathBuf;
 use std::process::Child;
 
+use anyhow::anyhow;
 use k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::CustomResourceDefinition;
 use kube::{Api, Client};
 use kubelet::backoff::ExponentialBackoffStrategy;
@@ -87,23 +88,23 @@ impl StackableProvider {
     }
 
     fn get_package(pod: &Pod) -> Result<Package, StackableError> {
-        let containers = pod.containers();
-        return if containers.len().ne(&1) {
-            let e = PodValidationError {
-                msg: String::from("Size of containers list in PodSpec has to be exactly 1"),
-            };
-            Err(e)
+        if let Some((container, [])) = pod.containers().split_first() {
+            container
+                .image()
+                .and_then(|maybe_ref| maybe_ref.ok_or_else(|| anyhow!("Image is required.")))
+                .and_then(Package::try_from)
+                .map_err(|err| PodValidationError {
+                    msg: format!(
+                        "Unable to get package reference from pod [{}]: {}",
+                        &pod.name(),
+                        &err
+                    ),
+                })
         } else {
-            // List has exactly one value, try to parse this
-            if let Ok(Some(reference)) = containers[0].image() {
-                Package::try_from(reference)
-            } else {
-                let e = PodValidationError {
-                    msg: format!("Unable to get package reference from pod: {}", &pod.name()),
-                };
-                Err(e)
-            }
-        };
+            Err(PodValidationError {
+                msg: String::from("Only one container is supported in the PodSpec."),
+            })
+        }
     }
 
     async fn check_crds(&self) -> Result<Vec<String>, StackableError> {
@@ -206,3 +207,89 @@ impl Provider for StackableProvider {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use indoc::indoc;
+    use rstest::rstest;
+
+    #[test]
+    fn try_to_get_package_from_complete_configuration() {
+        let pod = parse_pod_from_yaml(indoc! {"
+            apiVersion: v1
+            kind: Pod
+            metadata:
+              name: test
+            spec:
+              containers:
+              - name: kafka
+                image: kafka:2.7
+        "});
+
+        let maybe_package = StackableProvider::get_package(&pod);
+
+        if let Ok(package) = maybe_package {
+            assert_eq!("kafka", package.product);
+            assert_eq!("2.7", package.version);
+        } else {
+            panic!("Package expected but got {:?}", maybe_package);
+        }
+    }
+
+    #[rstest(pod_config, expected_err,
+        case(indoc! {"
+                apiVersion: v1
+                kind: Pod
+                metadata:
+                  name: test
+                spec:
+                  containers:
+                  - name: kafka
+                    image: kafka:2.7
+                  - name: zookeeper
+                    image: zookeeper:3.6.2
+            "},
+            "Only one container is supported in the PodSpec."
+        ),
+        case(indoc! {"
+                apiVersion: v1
+                kind: Pod
+                metadata:
+                  name: test
+                spec:
+                  containers:
+                  - name: kafka
+            "},
+            "Unable to get package reference from pod [test]: Image is required."
+        ),
+        case(indoc! {"
+                apiVersion: v1
+                kind: Pod
+                metadata:
+                  name: test
+                spec:
+                  containers:
+                  - name: kafka
+                    image: kafka
+            "},
+            "Unable to get package reference from pod [test]: Tag is required."
+        ),
+    )]
+    fn try_to_get_package_from_insufficient_configuration(pod_config: &str, expected_err: &str) {
+        let pod = parse_pod_from_yaml(pod_config);
+
+        let maybe_package = StackableProvider::get_package(&pod);
+
+        if let Err(PodValidationError { msg }) = maybe_package {
+            assert_eq!(expected_err, msg);
+        } else {
+            panic!("PodValidationError expected but got {:?}", maybe_package);
+        }
+    }
+
+    fn parse_pod_from_yaml(pod_config: &str) -> Pod {
+        let kube_pod: k8s_openapi::api::core::v1::Pod = serde_yaml::from_str(pod_config).unwrap();
+        Pod::from(kube_pod)
+    }
+}

src/provider/mod.rs

@@ -1,11 +1,10 @@
 use std::convert::TryFrom;
 use std::fmt;
 
+use anyhow::{anyhow, Result};
 use oci_distribution::Reference;
 use serde::{Deserialize, Serialize};
 
-use crate::provider::error::StackableError;
-
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct Package {
     pub product: String,
@@ -31,15 +30,18 @@ impl Package {
 }
 
 impl TryFrom<Reference> for Package {
-    type Error = StackableError;
+    type Error = anyhow::Error;
 
     // Converts from an oci reference to a package representation
     // The oci tag (anything after the \":\" in the string) is used as
     // version by this code and needs to be present
-    fn try_from(value: Reference) -> Result<Self, Self::Error> {
+    fn try_from(value: Reference) -> Result<Self> {
+        let repository = value.repository();
+        let tag = value.tag().ok_or_else(|| anyhow!("Tag is required."))?;
+
         Ok(Package {
-            product: String::from(value.repository()),
-            version: String::from(value.tag().unwrap()),
+            product: String::from(repository),
+            version: String::from(tag),
         })
     }
 }
@@ -49,3 +51,35 @@ impl fmt::Display for Package {
         write!(f, "{}:{}", self.product, self.version)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn try_from_complete_reference() {
+        let reference = Reference::try_from("kafka:2.7").expect("Reference cannot be parsed.");
+
+        let maybe_package = Package::try_from(reference);
+
+        if let Ok(package) = maybe_package {
+            assert_eq!("kafka", package.product);
+            assert_eq!("2.7", package.version);
+        } else {
+            panic!("Package expected but got {:?}", maybe_package);
+        }
+    }
+
+    #[test]
+    fn try_from_reference_without_tag() {
+        let reference = Reference::try_from("kafka").expect("Reference cannot be parsed.");
+
+        let maybe_package = Package::try_from(reference);
+
+        if let Err(error) = maybe_package {
+            assert_eq!("Tag is required.", error.to_string());
+        } else {
+            panic!("Error expected but got {:?}", maybe_package);
+        }
+    }
+}