Skip to content

fix docker image validation on macos#75

Merged
Panaetius merged 1 commit intomainfrom
fix-macos-docker
Mar 26, 2026
Merged

fix docker image validation on macos#75
Panaetius merged 1 commit intomainfrom
fix-macos-docker

Conversation

@Panaetius
Copy link
Copy Markdown
Member

No description provided.

@Panaetius Panaetius requested a review from a team as a code owner March 26, 2026 13:10
@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Add fallback handling for manifest platform resolution

The platform resolver function does not handle cases where the manifest entries do
not have platform information. This could cause failures when inspecting images that
lack detailed platform metadata. Add a fallback mechanism to handle such cases
gracefully.

coman/src/util/types.rs [82-106]

 pub async fn inspect(&self, arch: &str) -> Result<DockerImageMeta> {
     let arch = match arch {
         "arm64" => Arch::ARM64,
         "amd64" => Arch::Amd64,
         _ => {
             return Err(eyre!("unsupported architecture {}", arch));
         }
     };
     let client = Client::new(ClientConfig {
         protocol: ClientProtocol::Https,
         platform_resolver: Some(Box::new(move |manifests: &[ImageIndexEntry]| {
             manifests
                 .iter()
                 .find(|entry| {
                     entry
                         .platform
                         .as_ref()
                         .is_some_and(|platform| platform.os == Os::Linux && platform.architecture == arch)
                 })
+                .or_else(|| manifests.first()) // Fallback to first manifest if no matching platform found
                 .map(|entry| entry.digest.clone())
         })),
         ..Default::default()
     });
     let reference = self.to_string().parse()?;
     let auth = docker_auth(&reference)?;
Suggestion importance[1-10]: 8

__

Why: The suggestion identifies a potential runtime failure scenario when manifest entries lack platform information. Adding a fallback to the first manifest ensures robustness in image inspection, which is critical for the functionality of the application.

Medium
Improve architecture selection logic for image inspection

The current implementation tries multiple architectures but only uses the last
successful inspection result. This can lead to using an incorrect architecture for
image metadata. Instead, it should prioritize architectures based on system
compatibility or use the first valid result.

coman/src/cscs/handlers.rs [862-878]

 if let Some(system_info) = config.values.cscs.systems.get(current_system) {
     let mut meta = None;
     for arch in system_info.architecture.iter() {
         if let Ok(img_meta) = docker_image.inspect(&arch).await {
             meta = Some(img_meta);
+            break; // Use first successful inspection
         }
     }
     if meta.is_none() {
         println!("couldn't get image information, skipping checks");
     }
     meta
 } else {
     None
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion addresses a logical issue in trying multiple architectures but only using the last result, proposing to break on the first success. This improves correctness by ensuring the first valid architecture is used, which is more reliable than potentially using an outdated or irrelevant architecture.

Medium
Remove unused Os import

The Os import is added but not used in this file. Removing unused imports helps
maintain cleaner code and avoids potential confusion about its usage.

coman/src/util/types.rs [22]

-use oci_spec::image::{Arch, Os};
+use oci_spec::image::Arch;
Suggestion importance[1-10]: 3

__

Why: The suggestion points out an unused import, which is a minor code hygiene issue. While good practice, it doesn't significantly impact functionality or correctness, hence a low score.

Low

@Panaetius Panaetius merged commit dc2709b into main Mar 26, 2026
2 checks passed
@Panaetius Panaetius deleted the fix-macos-docker branch March 26, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant