Merge pull request #2618 from itowlson/registry-infer-annotations

Infer predefined annotations when pushing to registry

Author: itowlson

Cargo.lock

@@ -10,6 +10,7 @@ async-compression = "0.4.3"
 # Fork with nested async-std dependency bumped to satisfy Windows build; branch/revision is protected
 async-tar = { git = "https://github.com/vdice/async-tar", rev = "71e037f9652971e7a55b412a8e47a37b06f9c29d" }
 base64 = "0.21"
+chrono = "0.4"
 # Fork with updated auth to support ACR login
 # Ref https://github.com/camallo/dkregistry-rs/pull/263
 dkregistry = { git = "https://github.com/fermyon/dkregistry-rs", rev = "161cf2b66996ed97c7abaf046e38244484814de3" }

crates/oci/Cargo.toml

@@ -84,6 +84,17 @@ pub struct ClientOpts {
     pub content_ref_inline_max_size: usize,
 }
 
+/// Controls whether predefined annotations are generated when pushing an application.
+/// If an explicit annotation has the same name as a predefined one, the explicit
+/// one takes precedence.
+#[derive(Debug, PartialEq)]
+pub enum InferPredefinedAnnotations {
+    /// Infer annotations for created, authors, version, name and description.
+    All,
+    /// Do not generate any annotations; use only explicitly supplied annotations.
+    None,
+}
+
 impl Client {
     /// Create a new instance of an OCI client for distributing Spin applications.
     pub async fn new(insecure: bool, cache_root: Option<PathBuf>) -> Result<Self> {
@@ -107,6 +118,7 @@ impl Client {
         manifest_path: &Path,
         reference: impl AsRef<str>,
         annotations: Option<BTreeMap<String, String>>,
+        infer_annotations: InferPredefinedAnnotations,
     ) -> Result<Option<String>> {
         let reference: Reference = reference
             .as_ref()
@@ -125,7 +137,7 @@ impl Client {
         )
         .await?;
 
-        self.push_locked_core(locked, auth, reference, annotations)
+        self.push_locked_core(locked, auth, reference, annotations, infer_annotations)
             .await
     }
 
@@ -136,14 +148,15 @@ impl Client {
         locked: LockedApp,
         reference: impl AsRef<str>,
         annotations: Option<BTreeMap<String, String>>,
+        infer_annotations: InferPredefinedAnnotations,
     ) -> Result<Option<String>> {
         let reference: Reference = reference
             .as_ref()
             .parse()
             .with_context(|| format!("cannot parse reference {}", reference.as_ref()))?;
         let auth = Self::auth(&reference).await?;
 
-        self.push_locked_core(locked, auth, reference, annotations)
+        self.push_locked_core(locked, auth, reference, annotations, infer_annotations)
             .await
     }
 
@@ -155,6 +168,7 @@ impl Client {
         auth: RegistryAuth,
         reference: Reference,
         annotations: Option<BTreeMap<String, String>>,
+        infer_annotations: InferPredefinedAnnotations,
     ) -> Result<Option<String>> {
         let mut locked_app = locked.clone();
         let mut layers = self
@@ -174,6 +188,8 @@ impl Client {
                 .context("could not assemble archive layers for locked application")?;
         }
 
+        let annotations = all_annotations(&locked_app, annotations, infer_annotations);
+
         // Push layer for locked spin application config
         let locked_config_layer = ImageLayer::new(
             serde_json::to_vec(&locked_app).context("could not serialize locked config")?,
@@ -688,6 +704,76 @@ fn registry_from_input(server: impl AsRef<str>) -> String {
     }
 }
 
+fn all_annotations(
+    locked_app: &LockedApp,
+    explicit: Option<BTreeMap<String, String>>,
+    predefined: InferPredefinedAnnotations,
+) -> Option<BTreeMap<String, String>> {
+    use spin_locked_app::{MetadataKey, APP_DESCRIPTION_KEY, APP_NAME_KEY, APP_VERSION_KEY};
+    const APP_AUTHORS_KEY: MetadataKey<Vec<String>> = MetadataKey::new("authors");
+
+    if predefined == InferPredefinedAnnotations::None {
+        return explicit;
+    }
+
+    // We will always, at minimum, have a `created` annotation, so if we don't already have an
+    // anootations collection then we may as well create one now...
+    let mut current = explicit.unwrap_or_default();
+
+    let authors = locked_app
+        .get_metadata(APP_AUTHORS_KEY)
+        .unwrap_or_default()
+        .unwrap_or_default();
+    if !authors.is_empty() {
+        let authors = authors.join(", ");
+        add_inferred(
+            &mut current,
+            oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_AUTHORS,
+            Some(authors),
+        );
+    }
+
+    let name = locked_app.get_metadata(APP_NAME_KEY).unwrap_or_default();
+    add_inferred(
+        &mut current,
+        oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_TITLE,
+        name,
+    );
+
+    let description = locked_app
+        .get_metadata(APP_DESCRIPTION_KEY)
+        .unwrap_or_default();
+    add_inferred(
+        &mut current,
+        oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_DESCRIPTION,
+        description,
+    );
+
+    let version = locked_app.get_metadata(APP_VERSION_KEY).unwrap_or_default();
+    add_inferred(
+        &mut current,
+        oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_VERSION,
+        version,
+    );
+
+    let created = chrono::Utc::now().to_rfc3339();
+    add_inferred(
+        &mut current,
+        oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_CREATED,
+        Some(created),
+    );
+
+    Some(current)
+}
+
+fn add_inferred(map: &mut BTreeMap<String, String>, key: &str, value: Option<String>) {
+    if let Some(value) = value {
+        if let std::collections::btree_map::Entry::Vacant(e) = map.entry(key.to_string()) {
+            e.insert(value);
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -976,4 +1062,152 @@ mod test {
             }
         }
     }
+
+    fn annotatable_app() -> LockedApp {
+        let mut meta_builder = spin_locked_app::values::ValuesMapBuilder::new();
+        meta_builder
+            .string("name", "this-is-spinal-tap")
+            .string("version", "11.11.11")
+            .string("description", "")
+            .string_array("authors", vec!["Marty DiBergi", "Artie Fufkin"]);
+        let metadata = meta_builder.build();
+        LockedApp {
+            spin_lock_version: Default::default(),
+            must_understand: vec![],
+            metadata,
+            host_requirements: Default::default(),
+            variables: Default::default(),
+            triggers: Default::default(),
+            components: Default::default(),
+        }
+    }
+
+    fn as_annotations(annotations: &[(&str, &str)]) -> Option<BTreeMap<String, String>> {
+        Some(
+            annotations
+                .iter()
+                .map(|(k, v)| (k.to_string(), v.to_string()))
+                .collect(),
+        )
+    }
+
+    #[test]
+    fn no_annotations_no_infer_result_is_no_annotations() {
+        let locked_app = annotatable_app();
+        let explicit = None;
+        let infer = InferPredefinedAnnotations::None;
+
+        assert!(all_annotations(&locked_app, explicit, infer).is_none());
+    }
+
+    #[test]
+    fn explicit_annotations_no_infer_result_is_explicit_annotations() {
+        let locked_app = annotatable_app();
+        let explicit = as_annotations(&[("volume", "11"), ("dimensions", "feet")]);
+        let infer = InferPredefinedAnnotations::None;
+
+        let annotations =
+            all_annotations(&locked_app, explicit, infer).expect("should still have annotations");
+        assert_eq!(2, annotations.len());
+        assert_eq!("11", annotations.get("volume").unwrap());
+        assert_eq!("feet", annotations.get("dimensions").unwrap());
+    }
+
+    #[test]
+    fn no_annotations_infer_all_result_is_auto_annotations() {
+        let locked_app = annotatable_app();
+        let explicit = None;
+        let infer = InferPredefinedAnnotations::All;
+
+        let annotations =
+            all_annotations(&locked_app, explicit, infer).expect("should now have annotations");
+        assert_eq!(4, annotations.len());
+        assert_eq!(
+            "Marty DiBergi, Artie Fufkin",
+            annotations
+                .get(oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_AUTHORS)
+                .expect("should have authors annotation")
+        );
+        assert_eq!(
+            "this-is-spinal-tap",
+            annotations
+                .get(oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_TITLE)
+                .expect("should have title annotation")
+        );
+        assert_eq!(
+            "11.11.11",
+            annotations
+                .get(oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_VERSION)
+                .expect("should have version annotation")
+        );
+        assert!(
+            annotations
+                .get(oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_DESCRIPTION)
+                .is_none(),
+            "empty description should not have generated annotation"
+        );
+        assert!(
+            annotations
+                .get(oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_CREATED)
+                .is_some(),
+            "creation annotation should have been generated"
+        );
+    }
+
+    #[test]
+    fn explicit_annotations_infer_all_gets_both_sets() {
+        let locked_app = annotatable_app();
+        let explicit = as_annotations(&[("volume", "11"), ("dimensions", "feet")]);
+        let infer = InferPredefinedAnnotations::All;
+
+        let annotations =
+            all_annotations(&locked_app, explicit, infer).expect("should still have annotations");
+        assert_eq!(6, annotations.len());
+        assert_eq!(
+            "11",
+            annotations
+                .get("volume")
+                .expect("should have retained explicit annotation")
+        );
+        assert_eq!(
+            "Marty DiBergi, Artie Fufkin",
+            annotations
+                .get(oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_AUTHORS)
+                .expect("should have authors annotation")
+        );
+    }
+
+    #[test]
+    fn explicit_annotations_take_precedence_over_inferred() {
+        let locked_app = annotatable_app();
+        let explicit = as_annotations(&[
+            ("volume", "11"),
+            (
+                oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_AUTHORS,
+                "David St Hubbins, Nigel Tufnel",
+            ),
+        ]);
+        let infer = InferPredefinedAnnotations::All;
+
+        let annotations =
+            all_annotations(&locked_app, explicit, infer).expect("should still have annotations");
+        assert_eq!(
+            5,
+            annotations.len(),
+            "should have one custom, one predefined explicit, and three inferred"
+        );
+        assert_eq!(
+            "11",
+            annotations
+                .get("volume")
+                .expect("should have retained explicit annotation")
+        );
+        assert_eq!(
+            "David St Hubbins, Nigel Tufnel",
+            annotations
+                .get(oci_distribution::annotations::ORG_OPENCONTAINERS_IMAGE_AUTHORS)
+                .expect("should have authors annotation"),
+            "explicit authors should have taken precedence"
+        );
+    }
 }

crates/oci/src/client.rs

@@ -3,7 +3,7 @@ use anyhow::{Context, Result};
 use clap::{Parser, Subcommand};
 use indicatif::{ProgressBar, ProgressStyle};
 use spin_common::arg_parser::parse_kv;
-use spin_oci::Client;
+use spin_oci::{client::InferPredefinedAnnotations, Client};
 use std::{io::Read, path::PathBuf, time::Duration};
 
 /// Commands for working with OCI registries to distribute applications.
@@ -86,7 +86,14 @@ impl Push {
 
         let _spinner = create_dotted_spinner(2000, "Pushing app to the Registry".to_owned());
 
-        let digest = client.push(&app_file, &self.reference, annotations).await?;
+        let digest = client
+            .push(
+                &app_file,
+                &self.reference,
+                annotations,
+                InferPredefinedAnnotations::All,
+            )
+            .await?;
         match digest {
             Some(digest) => println!("Pushed with digest {digest}"),
             None => println!("Pushed; the registry did not return the digest"),