From 2539c12375f86ed81692edf4c66673e362cea84e Mon Sep 17 00:00:00 2001 From: Jarrod Overson Date: Wed, 20 Sep 2023 07:18:24 -0400 Subject: [PATCH 1/2] refactor: loosened trait signature --- crates/misc/asset-container/src/assets.rs | 6 +++--- crates/misc/asset-container/tests/assets.rs | 4 +--- crates/misc/derive-asset-container/tests/derive.rs | 4 +--- crates/wick/wick-asset-reference/src/asset_reference.rs | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/crates/misc/asset-container/src/assets.rs b/crates/misc/asset-container/src/assets.rs index d9bec9154..974b93a85 100644 --- a/crates/misc/asset-container/src/assets.rs +++ b/crates/misc/asset-container/src/assets.rs @@ -41,7 +41,7 @@ where self.asset.fetch_with_progress(options) } - fn fetch(&self, options: Self::Options) -> Pin, Error>> + Send + Sync + '_>> { + fn fetch(&self, options: Self::Options) -> Pin, Error>> + Send + '_>> { self.asset.fetch(options) } @@ -406,7 +406,7 @@ pub trait AssetManager { pub trait Asset: AssetManager { type Options: Clone; fn fetch_with_progress(&self, options: Self::Options) -> Pin + Send + '_>>; - fn fetch(&self, options: Self::Options) -> Pin, Error>> + Send + Sync + '_>>; + fn fetch(&self, options: Self::Options) -> Pin, Error>> + Send + '_>>; fn name(&self) -> &str; fn update_baseurl(&self, baseurl: &Path); } @@ -432,7 +432,7 @@ where ) } - fn fetch(&self, options: Self::Options) -> Pin, Error>> + Send + Sync + '_>> { + fn fetch(&self, options: Self::Options) -> Pin, Error>> + Send + '_>> { Box::pin(async move { match self { Some(a) => a.fetch(options).await, diff --git a/crates/misc/asset-container/tests/assets.rs b/crates/misc/asset-container/tests/assets.rs index c070ef251..6b147a710 100644 --- a/crates/misc/asset-container/tests/assets.rs +++ b/crates/misc/asset-container/tests/assets.rs @@ -135,9 +135,7 @@ impl Asset for LocationReference { fn fetch( &self, _options: Self::Options, - ) -> std::pin::Pin< - Box, asset_container::Error>> + Send + Sync>, - > { + ) -> std::pin::Pin, asset_container::Error>> + Send>> { let mut path = self .baseurl .lock() diff --git a/crates/misc/derive-asset-container/tests/derive.rs b/crates/misc/derive-asset-container/tests/derive.rs index 51d9bea2e..2b5b6d01a 100644 --- a/crates/misc/derive-asset-container/tests/derive.rs +++ b/crates/misc/derive-asset-container/tests/derive.rs @@ -146,9 +146,7 @@ impl Asset for TestAsset { fn fetch( &self, _options: Self::Options, - ) -> std::pin::Pin< - Box, asset_container::Error>> + Send + Sync>, - > { + ) -> std::pin::Pin, asset_container::Error>> + Send>> { let path = self.path.clone(); Box::pin(async move { let mut file = tokio::fs::File::open(&path) diff --git a/crates/wick/wick-asset-reference/src/asset_reference.rs b/crates/wick/wick-asset-reference/src/asset_reference.rs index 37120178e..42191e899 100644 --- a/crates/wick/wick-asset-reference/src/asset_reference.rs +++ b/crates/wick/wick-asset-reference/src/asset_reference.rs @@ -213,7 +213,7 @@ impl Asset for AssetReference { fn fetch( &self, options: OciOptions, - ) -> std::pin::Pin, assets::Error>> + Send + Sync>> { + ) -> std::pin::Pin, assets::Error>> + Send>> { let asset = self.clone(); Box::pin(async move { From 68ae03a2521c5e7e895f8d678125220d8649c775 Mon Sep 17 00:00:00 2001 From: Jarrod Overson Date: Wed, 20 Sep 2023 07:20:55 -0400 Subject: [PATCH 2/2] feat: improved tag push to avoid clone and immutable tag conflict --- crates/wick/wick-oci-utils/src/lib.rs | 13 +++ .../wick/wick-oci-utils/src/package/push.rs | 110 +++++++++++++----- crates/wick/wick-package/src/package.rs | 35 +++--- .../tests/wick_package_integration.rs | 2 +- .../tests/wick_package_types_integration.rs | 2 +- src/commands/registry/push.rs | 35 +++--- 6 files changed, 129 insertions(+), 68 deletions(-) diff --git a/crates/wick/wick-oci-utils/src/lib.rs b/crates/wick/wick-oci-utils/src/lib.rs index ac9a90663..723fa419b 100644 --- a/crates/wick/wick-oci-utils/src/lib.rs +++ b/crates/wick/wick-oci-utils/src/lib.rs @@ -183,6 +183,19 @@ pub enum WickPackageKind { TYPES, } +impl WickPackageKind { + /// Get the media type for the kind. + #[must_use] + pub const fn media_type(&self) -> &'static str { + use self::package::media_types; + match self { + Self::APPLICATION => media_types::APPLICATION, + Self::COMPONENT => media_types::COMPONENT, + Self::TYPES => media_types::TYPES, + } + } +} + /// Retrieve a manifest from an OCI url. pub async fn fetch_image_manifest(image: &str, options: &OciOptions) -> Result<(OciImageManifest, String), OciError> { if !options.allow_latest && image.ends_with(":latest") { diff --git a/crates/wick/wick-oci-utils/src/package/push.rs b/crates/wick/wick-oci-utils/src/package/push.rs index d8448759a..dcbff36d1 100644 --- a/crates/wick/wick-oci-utils/src/package/push.rs +++ b/crates/wick/wick-oci-utils/src/package/push.rs @@ -1,27 +1,38 @@ use std::collections::HashMap; -use oci_distribution::client::{Client, ClientConfig, ImageLayer, PushResponse}; +use oci_distribution::client::{Client, ClientConfig, ImageLayer}; use oci_distribution::manifest::{OciDescriptor, OciImageManifest}; +use oci_distribution::Reference; use sha256::digest; use super::annotations::Annotations; use super::{annotations, media_types, PackageFile}; -use crate::{Error, OciOptions}; +use crate::{Error, OciOptions, WickPackageKind}; + +#[derive(Debug, Clone)] +#[non_exhaustive] +pub struct PushResponse { + /// Pullable url for the config + pub config_url: String, + /// Pullable url for the manifest + pub manifest_url: String, + /// The OCI reference for the pushed image + pub reference: String, + /// Pullable url for any additional tags referencing this manifest + pub tags: Vec, +} + /// Push a Wick package to a registry. pub async fn push( reference: &str, + kind: WickPackageKind, config_json: String, files: Vec, annotations: Annotations, + tags: Vec, options: &OciOptions, ) -> Result { - let image_config = oci_distribution::client::Config { - data: config_json.as_bytes().to_vec(), - media_type: media_types::CONFIG.to_owned(), - annotations: None, - }; - - let mut image_layer_descriptors: Vec = Vec::new(); + let mut layers: Vec = Vec::new(); let mut image_layers: Vec = Vec::new(); for file in files { @@ -47,35 +58,29 @@ pub async fn push( urls: None, }; - image_layer_descriptors.push(image_layer_descriptor); + layers.push(image_layer_descriptor); image_layers.push(image_layer); } let image_annotations: HashMap = annotations.inner().clone(); - let image_manifest = OciImageManifest { - schema_version: 2, - config: OciDescriptor { - media_type: image_config.media_type.clone(), - digest: format!("sha256:{}", digest(config_json.clone())), - size: image_config.data.clone().len() as i64, - annotations: None, - urls: None, - }, - layers: image_layer_descriptors, - media_type: Some(media_types::MANIFEST.to_owned()), - annotations: Some(image_annotations), - }; + let (image_config, image_manifest) = gen_manifest( + &config_json, + layers, + Some(image_annotations), + Some(kind.media_type().to_owned()), + ); let (image_ref, protocol) = crate::utils::parse_reference_and_protocol(reference, &options.allow_insecure)?; let client_config = ClientConfig { protocol, ..Default::default() }; + let mut client = Client::new(client_config); let auth = options.get_auth(); - let result = client + let push_response = match client .push( &image_ref, &image_layers, @@ -83,13 +88,58 @@ pub async fn push( &auth, Some(image_manifest.clone()), ) - .await; - - match result { - Ok(push_response) => Ok(push_response), + .await + { + Ok(push_response) => push_response, Err(e) => { - tracing::error!(manifest = %image_manifest, error = %e, "Push failed"); - Err(Error::PushFailed(e.to_string())) + tracing::error!(manifest = %image_ref, error = %e, "Push failed"); + return Err(Error::PushFailed(e.to_string())); } + }; + + let mut pushed_tags = Vec::new(); + for tag in tags { + let image_ref = Reference::with_tag(image_ref.registry().to_owned(), image_ref.repository().to_owned(), tag); + + client + .push_manifest(&image_ref, &(image_manifest.clone().into())) + .await?; + pushed_tags.push(image_ref.to_string()); } + + Ok(PushResponse { + config_url: push_response.config_url, + manifest_url: push_response.manifest_url, + reference: image_ref.to_string(), + tags: pushed_tags, + }) +} + +fn gen_manifest( + config_json: &str, + layers: Vec, + annotations: Option>, + artifact_type: Option, +) -> (oci_distribution::client::Config, OciImageManifest) { + let image_config = oci_distribution::client::Config { + data: config_json.as_bytes().to_vec(), + media_type: media_types::CONFIG.to_owned(), + annotations: None, + }; + + let manifest = OciImageManifest { + schema_version: 2, + config: OciDescriptor { + media_type: image_config.media_type.clone(), + digest: format!("sha256:{}", digest(config_json.clone())), + size: image_config.data.len() as i64, + annotations: None, + urls: None, + }, + layers, + media_type: Some(media_types::MANIFEST.to_owned()), + annotations, + artifact_type, + }; + (image_config, manifest) } diff --git a/crates/wick/wick-package/src/package.rs b/crates/wick/wick-package/src/package.rs index f7fdcc94f..eeb7a9a76 100644 --- a/crates/wick/wick-package/src/package.rs +++ b/crates/wick/wick-package/src/package.rs @@ -10,7 +10,7 @@ use tracing::trace; use wick_config::config::RegistryConfig; use wick_config::{AssetReference, WickConfiguration}; use wick_oci_utils::package::annotations::Annotations; -use wick_oci_utils::package::{media_types, PackageFile}; +use wick_oci_utils::package::{media_types, PackageFile, PushResponse}; use wick_oci_utils::OciOptions; use crate::utils::{create_tar_gz, metadata_to_annotations}; @@ -298,32 +298,30 @@ impl WickPackage { self.registry.as_mut() } - /// Pushes the WickPackage to a specified registry using the provided reference, username, and password. - /// - /// The username and password are optional. If not provided, the function falls back to anonymous authentication. - pub async fn push(&mut self, reference: &str, options: &OciOptions) -> Result { - let kind = match self.kind { - wick_config::config::ConfigurationKind::App => wick_oci_utils::WickPackageKind::APPLICATION, - wick_config::config::ConfigurationKind::Component => wick_oci_utils::WickPackageKind::COMPONENT, - wick_config::config::ConfigurationKind::Types => wick_oci_utils::WickPackageKind::TYPES, - _ => { - return Err(Error::InvalidWickConfig(reference.to_owned())); - } - }; + /// Pushes the WickPackage to a specified registry using the provided OciOptions. + pub async fn push( + &mut self, + reference: &str, + tags: Vec, + options: &OciOptions, + ) -> Result { + let kind = convert_kind(self.kind).map_err(|_| Error::InvalidWickConfig(reference.to_owned()))?; let config = wick_oci_utils::WickOciConfig::new(kind, self.root.clone()); let image_config_contents = serde_json::to_string(&config).unwrap(); let files = self.files.drain(..).collect(); let push_response = wick_oci_utils::package::push( reference, + kind, image_config_contents, files, self.annotations.clone(), + tags, options, ) .await?; - Ok(push_response.manifest_url) + Ok(push_response) } /// This function pulls a WickPackage from a specified registry using the provided reference, username, and password. @@ -338,3 +336,12 @@ impl WickPackage { } } } + +const fn convert_kind(kind: wick_config::config::ConfigurationKind) -> Result { + Ok(match kind { + wick_config::config::ConfigurationKind::App => wick_oci_utils::WickPackageKind::APPLICATION, + wick_config::config::ConfigurationKind::Component => wick_oci_utils::WickPackageKind::COMPONENT, + wick_config::config::ConfigurationKind::Types => wick_oci_utils::WickPackageKind::TYPES, + _ => return Err(()), + }) +} diff --git a/crates/wick/wick-package/tests/wick_package_integration.rs b/crates/wick/wick-package/tests/wick_package_integration.rs index f71728cf2..cfd1edfe5 100644 --- a/crates/wick/wick-package/tests/wick_package_integration.rs +++ b/crates/wick/wick-package/tests/wick_package_integration.rs @@ -43,7 +43,7 @@ mod integration_test { let reference = expected .tagged_reference(&test_timestamp.as_secs().to_string()) .unwrap(); - let push_result = package.push(&reference, &options).await; + let push_result = package.push(&reference, Vec::new(), &options).await; if push_result.is_err() { panic!("Failed to push WickPackage: {:?}", push_result); }; diff --git a/crates/wick/wick-package/tests/wick_package_types_integration.rs b/crates/wick/wick-package/tests/wick_package_types_integration.rs index 29ed6ddfd..ea0b46942 100644 --- a/crates/wick/wick-package/tests/wick_package_types_integration.rs +++ b/crates/wick/wick-package/tests/wick_package_types_integration.rs @@ -43,7 +43,7 @@ mod integration_test { let reference = expected .tagged_reference(&test_timestamp.as_secs().to_string()) .unwrap(); - let push_result = package.push(&reference, &options).await; + let push_result = package.push(&reference, Vec::new(), &options).await; if push_result.is_err() { panic!("Failed to push WickPackage: {:?}", push_result); }; diff --git a/src/commands/registry/push.rs b/src/commands/registry/push.rs index 3ca4a7501..4d5f8f8c5 100644 --- a/src/commands/registry/push.rs +++ b/src/commands/registry/push.rs @@ -72,29 +72,20 @@ pub(crate) async fn handle( let mut lines = Vec::new(); - let url = if !opts.tags.is_empty() { - for tag in &opts.tags { - let mut pack = package.clone(); - let tagged_reference = pack.tagged_reference(tag).unwrap(); - span.in_scope(|| info!(reference = &tagged_reference, "pushing tag")); - pack.push(&tagged_reference, &oci_opts).await?; - lines.push(format!("Pushed tag: {}", reference)); - } - - // there must be a better way than cloning the package here, feel free to fix it. - let url = package.clone().push(&reference, &oci_opts).await?; - - span.in_scope(|| info!(%url, "artifact pushed")); - - package.push(&reference, &oci_opts).await?; - url - } else { - package.push(&reference, &oci_opts).await? - }; - let json = json!({"url":&url, "tags": opts.tags}); + let response = package.push(&reference, opts.tags, &oci_opts).await?; + + span.in_scope(|| info!(url=%response.reference, "manifest pushed")); + for tag in &response.tags { + span.in_scope(|| info!(url=%tag, "tag pushed")); + } - span.in_scope(|| info!(%url, "artifact pushed")); - lines.push(format!("Pushed artifact: {}", url)); + let json = json!({"manifest_url":&response.manifest_url, reference: &response.reference,"tags": &response.tags}); + + lines.push(format!("Manifest URL: {}", response.manifest_url)); + lines.push(format!("Pushed reference: {}", response.reference)); + if !response.tags.is_empty() { + lines.push(format!("Pushed tags:\n{}", response.tags.join("\n"))); + } Ok(StructuredOutput::new(lines.join("\n"), json)) }