Merge pull request #1595 from jvanz/issue1547

fix(kwctl): normalize registry:// URIs to include explicit tag

Author: jvanz

crates/kwctl/src/main.rs

@@ -22,7 +22,7 @@ use crate::{
     },
     load::load,
     save::save,
-    utils::{LookupError, find_file_matching_file},
+    utils::{LookupError, find_file_matching_file, normalize_uri},
 };
 
 mod annotate;
@@ -119,7 +119,8 @@ async fn main() -> Result<()> {
         Some("info") => info::info(),
         Some("pull") => {
             if let Some(matches) = matches.subcommand_matches("pull") {
-                let uri = matches.get_one::<String>("uri").unwrap();
+                let uri = normalize_uri(matches.get_one::<String>("uri").unwrap());
+                let uri = &uri;
                 let destination = matches
                     .get_one::<String>("output-path")
                     .map(|output| PathBuf::from_str(output).unwrap());
@@ -133,7 +134,8 @@ async fn main() -> Result<()> {
         }
         Some("verify") => {
             if let Some(matches) = matches.subcommand_matches("verify") {
-                let uri = matches.get_one::<String>("uri").unwrap();
+                let uri = normalize_uri(matches.get_one::<String>("uri").unwrap());
+                let uri = uri.as_str();
                 let sources = remote_server_options(matches)?;
                 let verification_options = build_verification_options(matches)?
                     .ok_or_else(|| anyhow!("could not retrieve sigstore options"))?;
@@ -193,8 +195,9 @@ async fn main() -> Result<()> {
         }
         Some("rm") => {
             if let Some(matches) = matches.subcommand_matches("rm") {
-                let uri_or_sha_prefix = matches.get_one::<String>("uri_or_sha_prefix").unwrap();
-                rm::rm(uri_or_sha_prefix)?;
+                let uri_or_sha_prefix =
+                    normalize_uri(matches.get_one::<String>("uri_or_sha_prefix").unwrap());
+                rm::rm(&uri_or_sha_prefix)?;
             }
             Ok(())
         }
@@ -233,7 +236,8 @@ async fn main() -> Result<()> {
         }
         Some("inspect") => {
             if let Some(matches) = matches.subcommand_matches("inspect") {
-                let uri_or_sha_prefix = matches.get_one::<String>("uri_or_sha_prefix").unwrap();
+                let uri_or_sha_prefix =
+                    normalize_uri(matches.get_one::<String>("uri_or_sha_prefix").unwrap());
                 let output = inspect::OutputType::try_from(
                     matches.get_one::<String>("output").map(|s| s.as_str()),
                 )?;
@@ -242,7 +246,7 @@ async fn main() -> Result<()> {
                     .get_one::<bool>("show-signatures")
                     .unwrap_or(&false)
                     .to_owned();
-                inspect::inspect(uri_or_sha_prefix, output, sources, no_color, no_signatures)
+                inspect::inspect(&uri_or_sha_prefix, output, sources, no_color, no_signatures)
                     .await?;
             };
             Ok(())
@@ -446,7 +450,8 @@ async fn pull_command(
  * This function will pull the policy if it is not already present in the local store.
  */
 async fn scaffold_manifest_command(matches: &ArgMatches) -> Result<()> {
-    let uri_or_sha_prefix = matches.get_one::<String>("uri_or_sha_prefix").unwrap();
+    let uri_or_sha_prefix = normalize_uri(matches.get_one::<String>("uri_or_sha_prefix").unwrap());
+    let uri_or_sha_prefix = uri_or_sha_prefix.as_str();
 
     pull_if_needed(uri_or_sha_prefix, matches).await?;
 

crates/kwctl/src/utils.rs

@@ -24,10 +24,23 @@ pub(crate) enum LookupError {
     IoError(#[from] std::io::Error),
 }
 
+/// Normalizes a `registry://` URI to include an explicit tag.
+/// If no tag is specified, defaults to `:latest`, making the URI consistent
+/// with how the local store paths are keyed.
+/// Non-registry URIs (e.g. `file://`, `http://`) are returned unchanged.
+pub(crate) fn normalize_uri(uri: &str) -> String {
+    if let Some(image) = uri.strip_prefix("registry://")
+        && let Ok(reference) = Reference::from_str(image)
+    {
+        return format!("registry://{}", reference.whole());
+    }
+    uri.to_string()
+}
+
 pub(crate) fn map_path_to_uri(uri_or_sha_prefix: &str) -> std::result::Result<String, LookupError> {
     let uri_has_schema = Regex::new(r"^\w+://").unwrap();
     if uri_has_schema.is_match(uri_or_sha_prefix) {
-        return Ok(String::from(uri_or_sha_prefix));
+        return Ok(normalize_uri(uri_or_sha_prefix));
     }
 
     let path = PathBuf::from(uri_or_sha_prefix);
@@ -46,8 +59,9 @@ pub(crate) fn map_path_to_uri(uri_or_sha_prefix: &str) -> std::result::Result<St
 }
 
 pub(crate) fn get_uri(uri_or_sha_prefix: &String) -> std::result::Result<String, LookupError> {
-    map_path_to_uri(uri_or_sha_prefix).or_else(|_| {
-        Reference::from_str(uri_or_sha_prefix)
+    let normalized = normalize_uri(uri_or_sha_prefix);
+    map_path_to_uri(&normalized).or_else(|_| {
+        Reference::from_str(&normalized)
             .map(|oci_reference| format!("registry://{}", oci_reference.whole()))
             .map_err(|_| LookupError::PolicyMissing(uri_or_sha_prefix.to_string()))
     })
@@ -103,8 +117,29 @@ pub(crate) fn find_file_matching_file(possible_names: &[&str]) -> Option<PathBuf
 mod tests {
     use std::collections::HashMap;
 
+    use rstest::rstest;
+
     use super::*;
 
+    #[rstest]
+    #[case(
+        "registry://ghcr.io/kubewarden/policies/pod-privileged",
+        "registry://ghcr.io/kubewarden/policies/pod-privileged:latest"
+    )]
+    #[case(
+        "registry://ghcr.io/kubewarden/policies/pod-privileged:v0.2.2",
+        "registry://ghcr.io/kubewarden/policies/pod-privileged:v0.2.2"
+    )]
+    #[case(
+        "registry://ghcr.io/kubewarden/policies/pod-privileged@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+        "registry://ghcr.io/kubewarden/policies/pod-privileged@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+    )]
+    #[case("file:///some/path/policy.wasm", "file:///some/path/policy.wasm")]
+    #[case("https://example.com/policy.wasm", "https://example.com/policy.wasm")]
+    fn test_normalize_uri(#[case] input: &str, #[case] expected: &str) {
+        assert_eq!(normalize_uri(input), expected);
+    }
+
     #[test]
     fn test_map_path_to_uri_remote_scheme() -> Result<()> {
         assert_eq!(