From 4d47c58a3d229dfd7f98ddf5cf6528dd6252332c Mon Sep 17 00:00:00 2001 From: RahulThennarasu <97413501+RahulThennarasu@users.noreply.github.com> Date: Fri, 14 Nov 2025 19:32:15 -0800 Subject: [PATCH] Fix: Allow OCI archives without manifest annotations (#397) Fixes #369 Per the OCI Image Spec, manifest descriptor annotations are optional. Previously, archives without annotations would fail to import with "Failed to import image". **Changes:** - Modified `getImageReferencefromDescriptor` to return digest-based references (`untagged@sha256:...`) when annotations are missing - Removed guard that skipped manifests without annotations - Added test case with `scratch_no_annotations.tar` **Testing:** All 167 tests pass, including new test for images without annotations. --- Package.swift | 5 +++- .../ImageStore/ImageStore+OCILayout.swift | 4 +-- .../Client/LocalOCILayoutClient.swift | 28 ++++++++++-------- .../ImageTests/ImageStoreTests.swift | 22 ++++++++++++++ .../Resources/scratch_no_annotations.tar | Bin 0 -> 55296 bytes 5 files changed, 42 insertions(+), 17 deletions(-) create mode 100644 Tests/ContainerizationTests/ImageTests/Resources/scratch_no_annotations.tar diff --git a/Package.swift b/Package.swift index 4d070ff0..8cbe16ac 100644 --- a/Package.swift +++ b/Package.swift @@ -90,7 +90,10 @@ let package = Package( name: "ContainerizationUnitTests", dependencies: ["Containerization"], path: "Tests/ContainerizationTests", - resources: [.copy("ImageTests/Resources/scratch.tar")] + resources: [ + .copy("ImageTests/Resources/scratch.tar"), + .copy("ImageTests/Resources/scratch_no_annotations.tar"), + ] ), .target( name: "ContainerizationEXT4", diff --git a/Sources/Containerization/Image/ImageStore/ImageStore+OCILayout.swift b/Sources/Containerization/Image/ImageStore/ImageStore+OCILayout.swift index b51d0b00..cf89b121 100644 --- a/Sources/Containerization/Image/ImageStore/ImageStore+OCILayout.swift +++ b/Sources/Containerization/Image/ImageStore/ImageStore+OCILayout.swift @@ -80,9 +80,7 @@ extension ImageStore { let (id, tempDir) = try await self.contentStore.newIngestSession() do { for descriptor in index.manifests { - guard let reference = client.getImageReferencefromDescriptor(descriptor: descriptor) else { - continue - } + let reference = client.getImageReferencefromDescriptor(descriptor: descriptor) let ref = try Reference.parse(reference) let name = ref.path let operation = ImportOperation(name: name, contentStore: self.contentStore, client: client, ingestDir: tempDir, progress: progress) diff --git a/Sources/ContainerizationOCI/Client/LocalOCILayoutClient.swift b/Sources/ContainerizationOCI/Client/LocalOCILayoutClient.swift index 3e25b53e..299ec60e 100644 --- a/Sources/ContainerizationOCI/Client/LocalOCILayoutClient.swift +++ b/Sources/ContainerizationOCI/Client/LocalOCILayoutClient.swift @@ -205,11 +205,8 @@ extension LocalOCILayoutClient { descriptor.annotations = annotations } - package func getImageReferencefromDescriptor(descriptor: Descriptor) -> String? { + package func getImageReferencefromDescriptor(descriptor: Descriptor) -> String { let annotations = descriptor.annotations - guard let annotations else { - return nil - } // Annotations here do not conform to the OCI image specification. // The interpretation of the annotations "org.opencontainers.image.ref.name" and @@ -220,16 +217,21 @@ extension LocalOCILayoutClient { // https://github.com/moby/buildkit/issues/4615#issuecomment-2521810830 // Until a consensus is reached, the preference is given to "com.apple.containerization.image.name" and then to // using "io.containerd.image.name" as it is the next safest choice - if let name = annotations[AnnotationKeys.containerizationImageName] { - return name + if let annotations { + if let name = annotations[AnnotationKeys.containerizationImageName] { + return name + } + if let name = annotations[AnnotationKeys.containerdImageName] { + return name + } + if let name = annotations[AnnotationKeys.openContainersImageName] { + return name + } } - if let name = annotations[AnnotationKeys.containerdImageName] { - return name - } - if let name = annotations[AnnotationKeys.openContainersImageName] { - return name - } - return nil + + // Fallback: Generate digest-based reference for images without annotations + // This makes sure OCI spec compliance as annotations are optional + return "untagged@\(descriptor.digest)" } package enum Error: Swift.Error { diff --git a/Tests/ContainerizationTests/ImageTests/ImageStoreTests.swift b/Tests/ContainerizationTests/ImageTests/ImageStoreTests.swift index daab699b..622d1be8 100644 --- a/Tests/ContainerizationTests/ImageTests/ImageStoreTests.swift +++ b/Tests/ContainerizationTests/ImageTests/ImageStoreTests.swift @@ -87,4 +87,26 @@ public class ImageStoreTests: ContainsAuth { let _ = try await self.store.tag(existing: imageReference, new: upstreamTag) try await self.store.push(reference: upstreamTag, auth: authentication) } + + @Test func testLoadImageWithoutAnnotations() async throws { + let fileManager = FileManager.default + let tempDir = fileManager.uniqueTemporaryDirectory() + defer { + try? fileManager.removeItem(at: tempDir) + } + + let tarPath = Foundation.Bundle.module.url(forResource: "scratch_no_annotations", withExtension: "tar")! + let reader = try ArchiveReader(format: .pax, filter: .none, file: tarPath) + try reader.extractContents(to: tempDir) + + let loaded = try await self.store.load(from: tempDir) + + #expect(loaded.count == 1) + + let reference = loaded.first!.reference + #expect(reference.hasPrefix("untagged@sha256:")) + + let retrieved = try await self.store.get(reference: reference) + #expect(retrieved.reference == reference) + } } diff --git a/Tests/ContainerizationTests/ImageTests/Resources/scratch_no_annotations.tar b/Tests/ContainerizationTests/ImageTests/Resources/scratch_no_annotations.tar new file mode 100644 index 0000000000000000000000000000000000000000..37135a107156e381b4bfcc21d823383b4dd2c7f3 GIT binary patch literal 55296 zcmeHQ&5s;M+20Uhz(|M-=;g$y5fW1Dp8Bq?l_D@WAx1%j#C*NMT2)t9?{+-fv-Hev z?5tJ-#0gGt0FFqUIPxD5DF-AtSbz``hyy1sxgnwVl=tbLj~@55-8S1b*J~V)%>2P>Uk>M3wK<<5JHmk!A|)uX76xW-NQRVgwX-!7%@f};%I;{m6+}fkn4(- z`5#ZFx`3(``rvppJ+OJM3q3hrEBeg?Ye#FTbwbsWWupf52T%Cb`#=7Ho%ikRd|8{p zS6&-@b1;9EX*(Z+KLUT>fxq(pm+O}`NYrR@57D9o~hG6X2yr1J~|rN z@TeG{*j(qv?mY9pofm)n>!1A^egA8XNBwP|5_m=5dCBU;7DIDf6gHnGSs_|8N-YjZJoSr0+6l1LSS3gG$wCc0p&WrV{D^-it8UHFI3SNMO4e^~wj{CoSa`=L?t zzbsGeo$!sxICoPX^sjhi`){@W&!zD8UpJl6%+>S1rbM2q`&ItuY98ps|0@5h@Gm*` z_TP<+!zTZmt^W&H|5wV}e>c(vTP-9Ux z2lh}eO~2$yaA=cE-@JQd!4O<-%*%|Pf?0TYk|!ao7KD|>IGl}}6Z~9B90Pbt=UHke z(@C)V`ssg|N0RMB4p8jmKoc%@Ete=FNlJ-KjYK5Eu}KY0BiJWOwMHnV2$@Jkh_X0V zCXs2JDyp{dXh7qS6219>rMUL?y!u~(+|K)xtwevs9sFGND{^z~}wp>C3|2?9rRwjb;c$^srev)N=eFHV`tN8HWd0NVq?%S`ChHunQV0{(AM5S@pXbkmXmWQe$dI}u97Okoupq;-P0B`8h>$BNqWTT&-70%e>tlM0%q^J>=>kA#}F1d?=~kEfNP zzO4PC-tfV^a$zt;>E}xI;j0IP1pzAMTk{kw_U>L@_b7UM64Gj(+2)){jbM)pcDV=`X6E7 z|L^rb_oL_m`d<`LzyI64^tWF^1OGjuzHVCw{@e6FiU9v@T%yjkr>mzt{!ghVTs-~H zGT^5WtcU?cHHJ`brNIgr8>3Xlkwi7Y1D*J9(f|DV-)4qRyZ*a|HUW=7&Ef=Wfs7E~8AJruu}zqO{bCk#MqrO(OI5q^zsCPL z^YNcrBg;VJS;u|adEebFZCFY&A+-A!;`_a>(>Y2FwI7GFdTep z5FclwceMF4$@r4gB|r`np*i_y@z`8vh5kQqTWu3A(#F4ff_+cov@EEAHa2fj_w|Npa~4 zj)Czm{m19gVHeLl(0d%>uO7#5z~-WgwCDP;++uWedw+LEt%rr(2U1_$4QDzm@2IykHc|%7aZx!hKJ)~f4I;nK6g?|G0RUtgDOMnmKn|VRlPPo zE({3gGyriIcV8KUD4>ArOAVwf>PQzL!D$tr;50~!F^DTK>v6xLawVvwS?<9)4dyb3 zQl|FK6mnT6)enJc1G(G$B!etxO3-Pr5H)09341ux4A0=3PzUg^nd+rTZY*1L<2rml zBm87CJ|Po1x-Ac)H{N1nd<=^NZ$jfMQNX||KH)>+aajIbipBX}Z(|1Gq$u+WLgvB}~Q zbWN2CTIf7t!bAG_Q!4^GgO@w_o;&Bp{15Tp&WW0q8gxd-@DBlH zo9jQ4`QQIqfDW%NrvE?35T4jVST_XW-MC0Fg%DH*>-|ACQDn}TIUl)Rg^W6Sh*-`DYzfe(^QS3NH8Ukp+ z*?4f?g@!O&h;tEX%|yhJNTn7Or4Z;&Bg-HZ7vdzvDSTNa5t!%9dN??Ur17|6+8X$W z^`CQAs%5Np;eUPoN5Bct-u}}f%Kno)|L7bwRD{%ii<<2=xb|0e$@aQr9i|MvXf z{XlvE|BtweBI(zE+;_m1OK9M~N7UC1>%u=6`x^Y8`}ohb_?%vS@<$(><^SH7e)szu z_7@mB3n7Z1`QxQam;T9K`sv&E-uc__@4fS$Y{^zis~)7vBC~3((=!h2Q@*cUM^$ z*H`v`MUo_vA&F82_VkE?yM-W}z=`6e!97j6COE=8Np+kEIN{Jnu}%dXw}J$maA^Jh zuh~{RzyIs0+W&RlrL~s1s$>3d+5aUZ^6@`fBwgY}dU3LB@V{FB>v10F#Q(bf z$EZL5eItYD0s0@DPsf$;>pvUmgKZYlz<-aZuUplH{}%s$Uhe<&{{OWMU0tpF|IZz2 zmf?QA|Nm3v|9@Vsxn-bs;lE}52fR-`{#!)Zf0F0_{mb+M<=FWBulawE^FSy5*ZDv8 z{{Nd9MD6?^+;1c-0D1nunNHYZDGmJhi2AxwUHEVD{~*Z!{?`I@cy%%TKh7Jb&ij8X zhExOpaQ+`S&_pCjDUqp>5Pc2DCN(sTAUYX^Q#Daa5i*g85M^J$O(BY-${|<%wcfJWSZWMX+=#Y~<*1Yan_1P^9mvx0f* zS@mA|m=BhDIS|KrBRKRSj@c5@lOYa_6;4zF@d7mL`-4MEq*bulFE*N40zsH5SRW9x zjH^_u)GErDiBevM3@`-8YvSO7FGq*Y^D%_TgFtR&f3FOXaxl-rtA&M4ZDDg`Lw6$= zMqF!&v&eohKBr~yyjfVi1+tdoa@m?k8|%ySL7UyX_+Z219Rg>Q2+p6;%ES;Pfd~bG z6jY*wh~hL#wdNMW`YFsMMBk1f+(H?Hn-F9rN;DP7We4|$#V5Ffp}TKgev%{Hx%%bw P`FaU>33v%yYzh27%$&f- literal 0 HcmV?d00001