Fix multi-platform image support

MULTIPLATFORM_FIX_PLAN.md

@@ -26,7 +26,7 @@ The branch has made progress:
 
 ## Issues to Fix
 
-### 1. Hardcoded Package Name ✅ **HIGH PRIORITY**
+### 1. Hardcoded Package Name ✅ **HIGH PRIORITY** - **COMPLETED**
 
 **Location:** [src/client/client.rs:490](src/client/client.rs#L490)
 
@@ -51,6 +51,41 @@ let url = format!("https://ghcr.io/v2/snok%2Fcontainer-retention-policy/manifest
 - Only need to support GitHub Container Registry (ghcr.io)
 - Must support multiple owners (different users/organizations)
 
+**Implementation Summary:**
+
+1. **Added `Owner` struct to Package model** ([models.rs:33-36](src/client/models.rs#L33-L36))
+   - Added `Owner` struct with `login` field to capture owner information from GitHub API
+   - Updated `Package` struct to include the `owner` field
+
+2. **Updated PackagesClient to store Account** ([client.rs:15,32](src/client/client.rs#L15,L32))
+   - Added `Account` import and field to `PackagesClient` struct
+
+3. **Updated PackagesClientBuilder** ([builder.rs:19-84,147-171](src/client/builder.rs#L19-L84,L147-L171))
+   - Added `account` field to builder
+   - Updated `generate_urls` to store the account
+   - Updated `build` method to require and pass the account
+
+4. **Updated select_packages flow** ([select_packages.rs:13-62](src/core/select_packages.rs#L13-L62))
+   - Changed `filter_by_matchers` to return `Vec<(String, String)>` (package_name, owner_login)
+   - Updated `select_packages` to return tuples with owner information
+   - Updated tests to include owner information
+
+5. **Updated select_package_versions flow** ([select_package_versions.rs:238-317](src/core/select_package_versions.rs#L238-L317))
+   - Changed function signature to accept `Vec<(String, String)>` instead of `Vec<String>`
+   - Created `package_owners` HashMap for lookup
+   - Updated manifest fetching to pass owner information
+
+6. **Fixed fetch_image_manifest method** ([client.rs:484-519](src/client/client.rs#L484-L519))
+   - Updated signature to accept `owner` parameter
+   - **Fixed hardcoded URL**: Now constructs URL dynamically as `https://ghcr.io/v2/{owner}%2F{package_name}/manifests/{tag}`
+   - Properly URL-encodes the package path
+
+7. **Fixed missing imports**
+   - Added `eyre!` macro import to select_package_versions.rs
+   - Added `info!` macro import to main.rs
+
+**Result:** ✅ Code compiles successfully. The manifest URL is now dynamically constructed using the owner from the Package API response, supporting multiple owners.
+
 ---
 
 ### 2. Improve Manifest Fetching ✅ **HIGH PRIORITY**
@@ -315,7 +350,7 @@ fn test_keep_n_most_recent_after_digest_filtering() {
 
 ## Implementation Order
 
-1. ✅ **Fix hardcoded package name** (blocks everything else)
+1. ✅ **Fix hardcoded package name** (blocks everything else) - **COMPLETED**
 2. ✅ **Improve manifest type handling** (critical for correctness)
 3. ✅ **Fix keep-n-most-recent logic** (potential bug)
 4. ✅ **Enhanced logging** (improves user experience)
@@ -332,7 +367,7 @@ None currently - all clarifications received:
 
 ## Progress Tracking
 
-- [ ] Issue #1: Fix hardcoded package name
+- [x] Issue #1: Fix hardcoded package name - **COMPLETED**
 - [ ] Issue #2: Improve manifest fetching
 - [ ] Issue #3: Enhanced logging
 - [ ] Issue #4: Fix keep-n-most-recent logic

src/client/builder.rs

@@ -22,6 +22,7 @@ pub struct PackagesClientBuilder {
     pub oci_headers: Option<HeaderMap>,
     pub urls: Option<Urls>,
     pub token: Option<Token>,
+    pub account: Option<Account>,
     pub fetch_package_service: Option<RateLimitedService>,
     pub list_packages_service: Option<RateLimitedService>,
     pub list_package_versions_service: Option<RateLimitedService>,
@@ -40,6 +41,7 @@ impl PackagesClientBuilder {
             list_package_versions_service: None,
             delete_package_versions_service: None,
             token: None,
+            account: None,
         }
     }
 
@@ -78,6 +80,7 @@ impl PackagesClientBuilder {
     pub fn generate_urls(mut self, github_server_url: &Url, github_api_url: &Url, account: &Account) -> Self {
         debug!("Constructing base urls");
         self.urls = Some(Urls::new(github_server_url, github_api_url, account));
+        self.account = Some(account.clone());
         self
     }
 
@@ -149,6 +152,7 @@ impl PackagesClientBuilder {
             || self.list_package_versions_service.is_none()
             || self.delete_package_versions_service.is_none()
             || self.token.is_none()
+            || self.account.is_none()
         {
             return Err("All required fields are not set".into());
         }
@@ -163,6 +167,7 @@ impl PackagesClientBuilder {
             list_package_versions_service: self.list_package_versions_service.unwrap(),
             delete_package_versions_service: self.delete_package_versions_service.unwrap(),
             token: self.token.unwrap(),
+            account: self.account.unwrap(),
         };
 
         Ok(client)

src/client/client.rs

@@ -12,7 +12,7 @@ use tracing::{debug, error, info, Span};
 use tracing_indicatif::span_ext::IndicatifSpanExt;
 use url::Url;
 
-use crate::cli::models::Token;
+use crate::cli::models::{Account, Token};
 use crate::client::builder::RateLimitedService;
 use crate::client::headers::GithubHeaders;
 use crate::client::models::{Package, PackageVersion};
@@ -29,6 +29,7 @@ pub struct PackagesClient {
     pub list_package_versions_service: RateLimitedService,
     pub delete_package_versions_service: RateLimitedService,
     pub token: Token,
+    pub account: Account,
 }
 
 impl PackagesClient {
@@ -482,12 +483,15 @@ impl PackagesClient {
 
     pub async fn fetch_image_manifest(
         &self,
+        owner: String,
         package_name: String,
         tag: String,
     ) -> Result<(String, String, Vec<String>)> {
         debug!(tag = tag, "Retrieving image manifest");
 
-        let url = format!("https://ghcr.io/v2/snok%2Fcontainer-retention-policy/manifests/{tag}");
+        // URL-encode the package path (owner/package_name)
+        let package_path = format!("{}%2F{}", owner, package_name);
+        let url = format!("https://ghcr.io/v2/{}/manifests/{}", package_path, tag);
 
         // Construct initial request
         let response = Client::new().get(url).headers(self.oci_headers.clone()).send().await?;

src/client/models.rs

@@ -30,10 +30,16 @@ impl PackageVersion {
     }
 }
 
+#[derive(Debug, Clone, Deserialize)]
+pub struct Owner {
+    pub login: String,
+}
+
 #[derive(Debug, Clone, Deserialize)]
 pub struct Package {
     pub id: u32,
     pub name: String,
+    pub owner: Owner,
     pub created_at: DateTime<Utc>,
     pub updated_at: Option<DateTime<Utc>>,
 }

src/core/select_package_versions.rs

@@ -5,6 +5,7 @@ use crate::client::urls::Urls;
 use crate::matchers::Matchers;
 use crate::{Counts, PackageVersions};
 use chrono::Utc;
+use color_eyre::eyre::eyre;
 use color_eyre::Result;
 use humantime::Duration as HumantimeDuration;
 use indicatif::ProgressStyle;
@@ -236,7 +237,7 @@ pub fn filter_package_versions(
 /// Fetches and filters package versions by account type, image-tag filters, cut-off,
 /// tag-selection, and a bunch of other things. Fetches versions concurrently.
 pub async fn select_package_versions(
-    package_names: Vec<String>,
+    packages: Vec<(String, String)>,
     client: &'static PackagesClient,
     image_tags: Vec<String>,
     shas_to_skip: Vec<String>,
@@ -249,9 +250,12 @@ pub async fn select_package_versions(
     // Create matchers for the image tags
     let matchers = Matchers::from(&image_tags);
 
+    // Create a map from package name to owner for later use
+    let package_owners: HashMap<String, String> = packages.iter().cloned().collect();
+
     // Create async tasks to fetch everything concurrently
     let mut set = JoinSet::new();
-    for package_name in package_names {
+    for (package_name, owner) in packages {
         let span = info_span!("fetch package versions", package_name = %package_name);
         span.pb_set_style(
             &ProgressStyle::default_spinner()
@@ -297,9 +301,16 @@ pub async fn select_package_versions(
         let (package_name, mut package_versions) = r??;
 
         // Queue fetching of digests for each tag
+        let owner = package_owners.get(&package_name).ok_or_else(|| {
+            eyre!("Could not find owner for package {}", package_name)
+        })?;
         for package_version in &package_versions.tagged {
             for tag in &package_version.metadata.container.tags {
-                fetch_digest_set.spawn(client.fetch_image_manifest(package_name.clone(), tag.clone()));
+                fetch_digest_set.spawn(client.fetch_image_manifest(
+                    owner.clone(),
+                    package_name.clone(),
+                    tag.clone()
+                ));
             }
         }
 

src/core/select_packages.rs

@@ -10,18 +10,18 @@ use tracing::{debug, info};
 /// Filter packages by package name-matchers.
 ///
 /// See the [`Matchers`] definition for details on what matchers are.
-fn filter_by_matchers(packages: &[Package], matchers: &Matchers) -> Vec<String> {
+fn filter_by_matchers(packages: &[Package], matchers: &Matchers) -> Vec<(String, String)> {
     packages
         .iter()
         .filter_map(|p| {
             if matchers.negative_match(&p.name) {
                 return None;
             };
             if matchers.positive.is_empty() {
-                return Some(p.name.to_string());
+                return Some((p.name.to_string(), p.owner.login.to_string()));
             };
             if matchers.positive_match(&p.name) {
-                return Some(p.name.to_string());
+                return Some((p.name.to_string(), p.owner.login.to_string()));
             };
             debug!("No match for package {} in {:?}", p.name, matchers.positive);
             None
@@ -30,13 +30,14 @@ fn filter_by_matchers(packages: &[Package], matchers: &Matchers) -> Vec<String>
 }
 
 /// Fetch and filters packages based on token type, account type, and image name filters.
+/// Returns a vector of tuples (package_name, owner_login).
 pub async fn select_packages(
     client: &mut PackagesClient,
     image_names: &Vec<String>,
     token: &Token,
     account: &Account,
     counts: Arc<Counts>,
-) -> Vec<String> {
+) -> Vec<(String, String)> {
     // Fetch all packages that the account owns
     let packages = client.fetch_packages(token, image_names, counts.clone()).await;
 
@@ -51,31 +52,34 @@ pub async fn select_packages(
 
     // Filter image names
     let image_name_matchers = Matchers::from(image_names);
-    let selected_package_names = filter_by_matchers(&packages, &image_name_matchers);
+    let selected_packages = filter_by_matchers(&packages, &image_name_matchers);
     info!(
         "{}/{} package names matched the `package-name` filters",
-        selected_package_names.len(),
+        selected_packages.len(),
         packages.len()
     );
 
-    selected_package_names
+    selected_packages
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::client::models::Package;
+    use crate::client::models::{Owner, Package};
 
     #[test]
     fn test_filter_by_matchers() {
         let packages = vec![Package {
             id: 0,
             name: "foo".to_string(),
+            owner: Owner {
+                login: "test-owner".to_string(),
+            },
             created_at: Default::default(),
             updated_at: None,
         }];
         // Negative matches
-        let empty_vec: Vec<String> = vec![];
+        let empty_vec: Vec<(String, String)> = vec![];
         assert_eq!(
             filter_by_matchers(&packages, &Matchers::from(&vec![String::from("!foo")])),
             empty_vec
@@ -95,11 +99,11 @@ mod tests {
                 &packages,
                 &Matchers::from(&vec![String::from("!bar"), String::from("!baz")])
             ),
-            vec!["foo".to_string()]
+            vec![("foo".to_string(), "test-owner".to_string())]
         );
         assert_eq!(
             filter_by_matchers(&packages, &Matchers::from(&vec![String::from("!")])),
-            vec!["foo".to_string()]
+            vec![("foo".to_string(), "test-owner".to_string())]
         );
 
         // No positive matches
@@ -114,15 +118,15 @@ mod tests {
         // Positive matches
         assert_eq!(
             filter_by_matchers(&packages, &Matchers::from(&vec![String::from("foo")])),
-            vec!["foo".to_string()]
+            vec![("foo".to_string(), "test-owner".to_string())]
         );
         assert_eq!(
             filter_by_matchers(&packages, &Matchers::from(&vec![String::from("*")])),
-            vec!["foo".to_string()]
+            vec![("foo".to_string(), "test-owner".to_string())]
         );
         assert_eq!(
             filter_by_matchers(&packages, &Matchers::from(&vec![String::from("f*")])),
-            vec!["foo".to_string()]
+            vec![("foo".to_string(), "test-owner".to_string())]
         );
     }
 }

src/main.rs

@@ -4,7 +4,7 @@ use std::sync::Arc;
 
 use color_eyre::eyre::Result;
 use tokio::sync::RwLock;
-use tracing::{debug, error, info_span, trace, Instrument};
+use tracing::{debug, error, info, info_span, trace, Instrument};
 use tracing_indicatif::IndicatifLayer;
 use tracing_subscriber::layer::SubscriberExt;
 use tracing_subscriber::util::SubscriberInitExt;