Merge pull request #866 from kubewarden/manifest-should-pull-on-missi…

…ng-policy

feat: allow `manifest` to pull a remote policy

Cargo.toml

@@ -36,6 +36,7 @@ syntect = { version = "5.2", default-features = false, features = [
   "parsing",
 ] }
 tar = "0.4.40"
+thiserror = "1.0"
 tiny-bench = "0.3"
 tokio = { version = "^1.39.0", features = ["full"] }
 tracing = "0.1"

src/cli.rs

@@ -16,8 +16,9 @@ Use the `info` command to display system information.
     };
 }
 
-fn subcommand_pull() -> Command {
-    let mut args = vec![
+// Minimum set of flags required to pull a policy from a registry
+fn pull_shared_flags() -> Vec<Arg> {
+    vec![
         Arg::new("docker-config-json-path")
             .long("docker-config-json-path")
             .value_name("DOCKER_CONFIG")
@@ -74,12 +75,16 @@ fn subcommand_pull() -> Command {
             .number_of_values(1)
             .value_name("VALUE")
             .help("GitHub repository expected in the certificates generated in CD pipelines"),
-        Arg::new("output-path")
-            .short('o')
-            .long("output-path")
-            .value_name("PATH")
-            .help("Output file. If not provided will be downloaded to the Kubewarden store"),
-    ];
+    ]
+}
+
+fn subcommand_pull() -> Command {
+    let mut args = pull_shared_flags();
+    args.extend_from_slice(&[Arg::new("output-path")
+        .short('o')
+        .long("output-path")
+        .value_name("PATH")
+        .help("Output file. If not provided will be downloaded to the Kubewarden store")]);
     args.sort_by(|a, b| a.get_id().cmp(b.get_id()));
     args.push(
         Arg::new("uri")
@@ -457,6 +462,8 @@ fn subcommand_scaffold() -> Command {
             .num_args(0)
             .help("Uses the policy metadata to define which Kubernetes resources can be accessed by the policy. Warning: review the list of resources carefully to avoid abuses. Disabled by default"),
     ];
+    // When scaffolding the manifest of a missing policy, we can pull it from a registry
+    manifest_args.extend_from_slice(&pull_shared_flags());
     manifest_args.sort_by(|a, b| a.get_id().cmp(b.get_id()));
     manifest_args.push(
         Arg::new("uri_or_sha_prefix")

src/main.rs

@@ -22,6 +22,7 @@ use std::{
 };
 use verify::VerificationAnnotations;
 
+use crate::utils::LookupError;
 use tracing::{debug, info, warn};
 use tracing_subscriber::prelude::*;
 use tracing_subscriber::{
@@ -147,39 +148,7 @@ async fn main() -> Result<()> {
                     Some(destination) => PullDestination::LocalFile(destination),
                     None => PullDestination::MainStore,
                 };
-
-                let sources = remote_server_options(matches)?;
-
-                let verification_options = verification_options(matches)?;
-                let mut verified_manifest_digest: Option<String> = None;
-                if verification_options.is_some() {
-                    let sigstore_trust_root = build_sigstore_trust_root(matches.to_owned()).await?;
-                    // verify policy prior to pulling if keys listed, and keep the
-                    // verified manifest digest:
-                    verified_manifest_digest = Some(
-                        verify::verify(
-                            uri,
-                            sources.as_ref(),
-                            verification_options.as_ref().unwrap(),
-                            sigstore_trust_root.clone(),
-                        )
-                        .await
-                        .map_err(|e| anyhow!("Policy {} cannot be validated\n{:?}", uri, e))?,
-                    );
-                }
-
-                let policy = pull::pull(uri, sources.as_ref(), destination).await?;
-
-                if verification_options.is_some() {
-                    let sigstore_trust_root = build_sigstore_trust_root(matches.to_owned()).await?;
-                    verify::verify_local_checksum(
-                        &policy,
-                        sources.as_ref(),
-                        &verified_manifest_digest.unwrap(),
-                        sigstore_trust_root.clone(),
-                    )
-                    .await?
-                }
+                pull_command(uri, destination, matches).await?
             };
             Ok(())
         }
@@ -368,42 +337,7 @@ async fn main() -> Result<()> {
             }
             if let Some(matches) = matches.subcommand_matches("scaffold") {
                 if let Some(matches) = matches.subcommand_matches("manifest") {
-                    let uri_or_sha_prefix = matches.get_one::<String>("uri_or_sha_prefix").unwrap();
-                    let resource_type = matches.get_one::<String>("type").unwrap();
-                    if matches.contains_id("settings-path") && matches.contains_id("settings-json")
-                    {
-                        return Err(anyhow!(
-                            "'settings-path' and 'settings-json' cannot be used at the same time"
-                        ));
-                    }
-                    let settings = if matches.contains_id("settings-path") {
-                        matches
-                            .get_one::<String>("settings-path")
-                            .map(|settings| -> Result<String> {
-                                fs::read_to_string(settings).map_err(|e| {
-                                    anyhow!("Error reading settings from {}: {}", settings, e)
-                                })
-                            })
-                            .transpose()?
-                    } else if matches.contains_id("settings-json") {
-                        Some(matches.get_one::<String>("settings-json").unwrap().clone())
-                    } else {
-                        None
-                    };
-                    let policy_title = matches.get_one::<String>("title").cloned();
-
-                    let allow_context_aware_resources = matches
-                        .get_one::<bool>("allow-context-aware")
-                        .unwrap_or(&false)
-                        .to_owned();
-
-                    scaffold::manifest(
-                        uri_or_sha_prefix,
-                        resource_type.parse()?,
-                        settings.as_deref(),
-                        policy_title.as_deref(),
-                        allow_context_aware_resources,
-                    )?;
+                    scaffold_manifest_command(matches).await?;
                 };
             }
             if let Some(matches) = matches.subcommand_matches("scaffold") {
@@ -836,3 +770,103 @@ async fn build_sigstore_trust_root(
         Ok(Some(Arc::new(manual_root)))
     }
 }
+
+// Check if the policy is already present in the local store, and if not, pull it from the remote server.
+async fn pull_if_needed(uri_or_sha_prefix: &str, matches: &ArgMatches) -> Result<()> {
+    match crate::utils::get_wasm_path(uri_or_sha_prefix) {
+        Err(LookupError::PolicyMissing(uri)) => {
+            info!(
+                "cannot find policy with uri: {}, trying to pull it from remote registry",
+                uri
+            );
+            pull_command(&uri, PullDestination::MainStore, matches).await
+        }
+        Err(e) => Err(anyhow!("{}", e)),
+        Ok(_path) => Ok(()),
+    }
+}
+
+// Pulls a policy from a remote server and verifies it if verification options are provided.
+async fn pull_command(
+    uri: &String,
+    destination: PullDestination,
+    matches: &ArgMatches,
+) -> Result<()> {
+    let sources = remote_server_options(matches)?;
+
+    let verification_options = verification_options(matches)?;
+    let mut verified_manifest_digest: Option<String> = None;
+    if verification_options.is_some() {
+        let sigstore_trust_root = build_sigstore_trust_root(matches.to_owned()).await?;
+        // verify policy prior to pulling if keys listed, and keep the
+        // verified manifest digest:
+        verified_manifest_digest = Some(
+            verify::verify(
+                uri,
+                sources.as_ref(),
+                verification_options.as_ref().unwrap(),
+                sigstore_trust_root.clone(),
+            )
+            .await
+            .map_err(|e| anyhow!("Policy {} cannot be validated\n{:?}", uri, e))?,
+        );
+    }
+
+    let policy = pull::pull(uri, sources.as_ref(), destination).await?;
+
+    if verification_options.is_some() {
+        let sigstore_trust_root = build_sigstore_trust_root(matches.to_owned()).await?;
+        return verify::verify_local_checksum(
+            &policy,
+            sources.as_ref(),
+            &verified_manifest_digest.unwrap(),
+            sigstore_trust_root.clone(),
+        )
+        .await;
+    }
+    Ok(())
+}
+
+/*
+ * Scaffold a manifest from a policy.
+ * 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();
+
+    pull_if_needed(uri_or_sha_prefix, matches).await?;
+
+    let resource_type = matches.get_one::<String>("type").unwrap();
+    if matches.contains_id("settings-path") && matches.contains_id("settings-json") {
+        return Err(anyhow!(
+            "'settings-path' and 'settings-json' cannot be used at the same time"
+        ));
+    }
+    let settings = if matches.contains_id("settings-path") {
+        matches
+            .get_one::<String>("settings-path")
+            .map(|settings| -> Result<String> {
+                fs::read_to_string(settings)
+                    .map_err(|e| anyhow!("Error reading settings from {}: {}", settings, e))
+            })
+            .transpose()?
+    } else if matches.contains_id("settings-json") {
+        Some(matches.get_one::<String>("settings-json").unwrap().clone())
+    } else {
+        None
+    };
+    let policy_title = matches.get_one::<String>("title").cloned();
+
+    let allow_context_aware_resources = matches
+        .get_one::<bool>("allow-context-aware")
+        .unwrap_or(&false)
+        .to_owned();
+
+    scaffold::manifest(
+        uri_or_sha_prefix,
+        resource_type.parse()?,
+        settings.as_deref(),
+        policy_title.as_deref(),
+        allow_context_aware_resources,
+    )
+}

src/scaffold/manifest.rs

@@ -122,7 +122,7 @@ pub(crate) fn manifest(
     policy_title: Option<&str>,
     allow_context_aware_resources: bool,
 ) -> Result<()> {
-    let uri = crate::utils::map_path_to_uri(uri_or_sha_prefix)?;
+    let uri = crate::utils::get_uri(&uri_or_sha_prefix.to_owned())?;
     let wasm_path = crate::utils::wasm_path(&uri)?;
 
     let metadata = Metadata::from_path(&wasm_path)?

src/utils.rs

@@ -1,12 +1,30 @@
 use anyhow::{anyhow, Result};
 use policy_evaluator::policy_evaluator::PolicyExecutionMode;
-use policy_evaluator::policy_fetcher::store::Store;
+use policy_evaluator::policy_fetcher::oci_distribution::Reference;
+use policy_evaluator::policy_fetcher::store::{errors::StoreError, Store};
 use regex::Regex;
 use serde_json::json;
 use std::path::PathBuf;
+use std::str::FromStr;
 use url::Url;
 
-pub(crate) fn map_path_to_uri(uri_or_sha_prefix: &str) -> Result<String> {
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum LookupError {
+    #[error("Cannot find policy with uri: {0}")]
+    PolicyMissing(String),
+    #[error("{0}")]
+    StoreError(#[from] StoreError),
+    #[error("Unknown scheme: {0}")]
+    UnknownScheme(String),
+    #[error("{0}")]
+    UrlParserError(#[from] url::ParseError),
+    #[error("Error while converting URL to string")]
+    UrlToStringConversionError(),
+    #[error("{0}")]
+    IoError(#[from] std::io::Error),
+}
+
+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));
@@ -22,31 +40,41 @@ pub(crate) fn map_path_to_uri(uri_or_sha_prefix: &str) -> Result<String> {
         if let Some(policy) = store.get_policy_by_sha_prefix(uri_or_sha_prefix)? {
             Ok(policy.uri.clone())
         } else {
-            Err(anyhow!(
-                "Cannot find policy with prefix: {}",
-                uri_or_sha_prefix
-            ))
+            Err(LookupError::PolicyMissing(uri_or_sha_prefix.to_string()))
         }
     }
 }
 
-pub(crate) fn wasm_path(uri: &str) -> Result<PathBuf> {
+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)
+            .map(|oci_reference| format!("registry://{}", oci_reference.whole()))
+            .map_err(|_| LookupError::PolicyMissing(uri_or_sha_prefix.to_string()))
+    })
+}
+
+pub(crate) fn get_wasm_path(uri_or_sha_prefix: &str) -> std::result::Result<PathBuf, LookupError> {
+    let uri = get_uri(&uri_or_sha_prefix.to_owned())?;
+    wasm_path(&uri)
+}
+
+pub(crate) fn wasm_path(uri: &str) -> std::result::Result<PathBuf, LookupError> {
     let url = Url::parse(uri)?;
     match url.scheme() {
         "file" => url
             .to_file_path()
-            .map_err(|err| anyhow!("cannot retrieve path from uri {}: {:?}", url, err)),
+            .map_err(|_| LookupError::UrlToStringConversionError()),
         "http" | "https" | "registry" => {
             let store = Store::default();
             let policy = store.get_policy_by_uri(uri)?;
 
             if let Some(policy) = policy {
                 Ok(policy.local_path)
             } else {
-                Err(anyhow!("Cannot find policy '{uri}' inside of the local store.\nTry executing `kwctl pull {uri}`", uri = uri))
+                Err(LookupError::PolicyMissing(uri.to_string()))
             }
         }
-        _ => Err(anyhow!("unknown scheme: {}", url.scheme())),
+        _ => Err(LookupError::UnknownScheme(url.scheme().to_string())),
     }
 }
 

tests/e2e.rs

@@ -377,10 +377,14 @@ fn test_push() {
         .stdout(contains("my-pod-priviliged-policy:v0.1.10"));
 }
 
-#[test]
-fn test_scaffold_manifest() {
+#[rstest]
+#[case::pull_policies_before_scaffold(true)]
+#[case::pull_policies_on_demand(false)]
+fn test_scaffold_manifest(#[case] pull_policies_before: bool) {
     let tempdir = tempdir().unwrap();
-    pull_policies(tempdir.path(), POLICIES);
+    if pull_policies_before {
+        pull_policies(tempdir.path(), POLICIES);
+    }
 
     let mut cmd = setup_command(tempdir.path());
     cmd.arg("scaffold")