copacetic icon indicating copy to clipboard operation
copacetic copied to clipboard

fix: oci media type should be respected

Open robert-cronin opened this issue 9 months ago • 6 comments

Adds getExporterType to map media type to exporter type directly via a resolution of the descriptor using oras

Closes #842

robert-cronin avatar Mar 07 '25 02:03 robert-cronin

There might be a better way to get the media type maybe via build kit but I couldn't find any, if anyone has any suggestions or improvements, I would greatly appreciate them, thanks~

robert-cronin avatar Mar 07 '25 02:03 robert-cronin

Codecov Report

Attention: Patch coverage is 38.16794% with 81 lines in your changes missing coverage. Please review.

Project coverage is 48.29%. Comparing base (aeadfd4) to head (7483e97).

Files with missing lines Patch % Lines
pkg/patch/patch.go 22.72% 66 Missing and 2 partials :warning:
pkg/utils/mediatype.go 69.76% 11 Missing and 2 partials :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #949      +/-   ##
==========================================
- Coverage   48.60%   48.29%   -0.32%     
==========================================
  Files          18       19       +1     
  Lines        2471     2578     +107     
==========================================
+ Hits         1201     1245      +44     
- Misses       1189     1248      +59     
- Partials       81       85       +4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Mar 07 '25 02:03 codecov[bot]

please make sure to add an integration test for this

sozercan avatar Mar 07 '25 21:03 sozercan

validation for oci media types: https://oci.dag.dev/?image=ghcr.io%2Frobert-cronin%2Fubuntu%3Alatest-patch and for private remotes via docker config: image

robert-cronin avatar Mar 11 '25 03:03 robert-cronin

please make sure to add an integration test for this

okay, I will add some integration testing.

Update: I am encountering some issues with the integration testing, namely the patched images are unable to be resolved, setting this to draft until its resolved.

robert-cronin avatar Mar 11 '25 03:03 robert-cronin

@robert-cronin when testing with the nginx:1.27.0 image locally tagged as ashnam/nginx:1.27.0, the patched image does not seem to have oci type https://oci.dag.dev/?image=ashnam%2Fnginx%3A1.27.0-patched

ashnamehrotra avatar Apr 23 '25 21:04 ashnamehrotra

@robert-cronin when testing with the nginx:1.27.0 image locally tagged as ashnam/nginx:1.27.0, the patched image does not seem to have oci type https://oci.dag.dev/?image=ashnam%2Fnginx%3A1.27.0-patched

All fixed now! Originally, I was defaulting to docker for buildkit addresses because of an error that is described in this github issue: https://github.com/moby/buildkit/issues/4131 but I've managed to get it working now by using the suggested type=docker,oci-mediatypes=true configuration.

New nginx showing oci layout: https://oci.dag.dev/?image=ghcr.io%2Frobert-cronin%2Fnginx%3A1.27.0-patched

robert-cronin avatar Apr 30 '25 05:04 robert-cronin

https://oci.dag.dev/?image=ghcr.io%2Frobert-cronin%2Fnginx%3A1.27.0-patched

Looks like its still Content-Type: application/vnd.docker.distribution.manifest.list.v2+json rather than Content-Type: application/vnd.oci.image.index.v1+json"

ashnamehrotra avatar May 06 '25 21:05 ashnamehrotra

https://oci.dag.dev/?image=ghcr.io%2Frobert-cronin%2Fnginx%3A1.27.0-patched

Looks like its still Content-Type: application/vnd.docker.distribution.manifest.list.v2+json rather than Content-Type: application/vnd.oci.image.index.v1+json"

Should be fixed now!

robert-cronin avatar May 07 '25 00:05 robert-cronin

@robert-cronin its still showing docker for me, anything I could be doing wrong? https://oci.dag.dev/?image=ashnam%2Fnginx%3A1.27.0-patched. Im testing the pr via "gh pr checkout 949". Also do we need to change the integration test since it hasn't been catching this?

ashnamehrotra avatar May 07 '25 17:05 ashnamehrotra

@robert-cronin its still showing docker for me, anything I could be doing wrong? https://oci.dag.dev/?image=ashnam%2Fnginx%3A1.27.0-patched. Im testing the pr via "gh pr checkout 949". Also do we need to change the integration test since it hasn't been catching this?

It might be defaulting back to docker due to an error. I've added in some debug logging; can you please try again to check if anything shows up?

robert-cronin avatar May 12 '25 07:05 robert-cronin

@robert-cronin its still showing docker for me, anything I could be doing wrong? https://oci.dag.dev/?image=ashnam%2Fnginx%3A1.27.0-patched. Im testing the pr via "gh pr checkout 949". Also do we need to change the integration test since it hasn't been catching this?

It might be defaulting back to docker due to an error. I've added in some debug logging; can you please try again to check if anything shows up?

I dont see anything erroring out, here are the full logs

`ashnamehrotra@MacBookPro release % ./copa patch -i ashnam/nginx:1.27.0 --debug DEBU[0000] Handling platform specific errors with skip
DEBU[0000] Trying docker driver
DEBU[0000] serving grpc connection
DEBU[0000] stopping session
DEBU[0000] Image name has tag or digest, using docker.io/ashnam/nginx:1.27.0 as tag DEBU[0000] local media type not found for docker.io/ashnam/nginx:1.27.0: Error response from daemon: manifest unknown: manifest unknown DEBU[0001] failed to get remote media type for docker.io/ashnam/nginx:1.27.0: GET https://index.docker.io/v2/ashnam/nginx/manifests/1.27.0: MANIFEST_UNKNOWN: manifest unknown; unknown tag=1.27.0 WARN[0001] unable to determine media type, defaulting to docker, err: GET https://index.docker.io/v2/ashnam/nginx/manifests/1.27.0: MANIFEST_UNKNOWN: manifest unknown; unknown tag=1.27.0 DEBU[0001] Docker client initialized successfully
DEBU[0001] Loading image stream using Docker API client DEBU[0001] serving grpc connection
#1 resolve image config for docker-image://docker.io/ashnam/nginx:1.27.0 #1 DONE 0.3s Testing DiscoverPlatforms Utility - test: [], err: "error fetching descriptor for "docker.io/ashnam/nginx:1.27.0": GET https://index.docker.io/v2/ashnam/nginx/manifests/1.27.0: MANIFEST_UNKNOWN: manifest unknown; unknown tag=1.27.0" #2 resolve image config for docker-image://docker.io/ashnam/nginx:1.27.0 #2 DONE 0.1s DEBU[0001] Using debian:12-slim as basis for tooling image

#3 docker-image://docker.io/ashnam/nginx:1.27.0 #3 resolve docker.io/ashnam/nginx:1.27.0 0.0s done #3 CACHED

#4 docker-image://ghcr.io/project-copacetic/copacetic/debian:12-slim #4 resolve ghcr.io/project-copacetic/copacetic/debian:12-slim #4 resolve ghcr.io/project-copacetic/copacetic/debian:12-slim 0.6s done #4 ERROR: no match for platform in manifest: not found DEBU[0002] Using debian:12-slim as basis for tooling image

#5 docker-image://docker.io/library/debian:12-slim #5 resolve docker.io/library/debian:12-slim 0.1s done #5 DONE 0.1s

#5 docker-image://docker.io/library/debian:12-slim #5 CACHED

#6 apt-get update #6 0.153 Get:1 http://deb.debian.org/debian bookworm InRelease [151 kB] #6 0.218 Get:2 http://deb.debian.org/debian bookworm-updates InRelease [55.4 kB] #6 0.239 Get:3 http://deb.debian.org/debian-security bookworm-security InRelease [48.0 kB] #6 0.261 Get:4 http://deb.debian.org/debian bookworm/main arm64 Packages [8692 kB] #6 0.510 Get:5 http://deb.debian.org/debian bookworm-updates/main arm64 Packages [512 B] #6 0.513 Get:6 http://deb.debian.org/debian-security bookworm-security/main arm64 Packages [254 kB] #6 1.233 Fetched 9201 kB in 1s (8169 kB/s) #6 1.233 Reading package lists... #6 DONE 1.6s

#7 apt-get install busybox-static #7 0.117 Reading package lists... #7 0.454 Building dependency tree... #7 0.516 Reading state information... #7 0.606 The following NEW packages will be installed: #7 0.607 busybox-static #7 0.659 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. #7 0.659 Need to get 819 kB of archives. #7 0.659 After this operation, 1936 kB of additional disk space will be used. #7 0.659 Get:1 http://deb.debian.org/debian bookworm/main arm64 busybox-static arm64 1:1.35.0-4+b3 [819 kB] #7 0.824 debconf: delaying package configuration, since apt-utils is not installed #7 0.839 Fetched 819 kB in 0s (5976 kB/s) #7 0.849 Selecting previously unselected package busybox-static. (Reading database ... 6085 files and directories currently installed.) #7 0.851 Preparing to unpack .../busybox-static_1%3a1.35.0-4+b3_arm64.deb ... #7 0.853 Unpacking busybox-static (1:1.35.0-4+b3) ... #7 0.908 Setting up busybox-static (1:1.35.0-4+b3) ... #7 DONE 0.9s

#8 copy /bin/busybox /bin/busybox #8 CACHED

#9 mkdir /copa-out #9 CACHED

#10 /bin/busybox sh -c status="$DPKG_STATUS_IS_UNKNOWN" if [ -f "$DPKG_STATUS_PATH" ]; then status="$DPKG_STATUS_IS_FILE" cp "$DPKG_STATUS_PATH" "$RESULTS_PATH" elif [ -d "$DPKG_STATUS_FOLDER" ]; then status="$DPKG_STATUS_IS_DIRECTORY" ls -1 "$DPKG_STATUS_FOLDER" > "$RESULT_STATUSD_PATH" mv "$DPKG_STATUS_FOLDER"/* "$RESULTS_PATH" fi echo -n "$status" > "${RESULTS_PATH}/${STATUSD_OUTPUT_FILENAME}"

#10 CACHED

#11 apt-get update #11 0.162 Get:1 http://deb.debian.org/debian bookworm InRelease [151 kB] #11 0.223 Get:2 http://deb.debian.org/debian bookworm-updates InRelease [55.4 kB] #11 0.251 Get:3 http://deb.debian.org/debian-security bookworm-security InRelease [48.0 kB] #11 0.278 Get:4 http://deb.debian.org/debian bookworm/main arm64 Packages [8692 kB] #11 0.480 Get:5 http://deb.debian.org/debian bookworm-updates/main arm64 Packages [512 B] #11 0.480 Get:6 http://deb.debian.org/debian-security bookworm-security/main arm64 Packages [254 kB] #11 1.156 Fetched 9201 kB in 1s (8787 kB/s) #11 1.156 Reading package lists... #11 DONE 1.4s

#12 sh -c apt-get -s upgrade 2>/dev/null | grep -q ^Inst || exit 1 #12 DONE 0.9s

#13 sh -c apt-mark showhold | tee /held.txt #13 DONE 0.4s

#14 sh -c output=$(apt-get upgrade -y && apt-get clean -y && apt-get autoremove -y 2>&1); if [ $? -ne 0 ]; then echo $output >>error_log.txt; fi #14 1.345 debconf: delaying package configuration, since apt-utils is not installed #14 DONE 6.8s

#15 /bin/sh -c if [ -s error_log.txt ]; then cat error_log.txt; exit 1; fi #15 DONE 0.1s

#16 sh -c grep "^Package:|^Version:" "/var/lib/dpkg/status" >> "results.manifest" #16 DONE 0.1s

#17 diff (/bin/sh -c if [ -s error_log.txt ]; then cat error_log.txt; exit 1; fi) -> (sh -c grep "^Package:|^Version:" "/var/lib/dpkg/status" >> "results.manifest") #17 diffing 0.0s done #17 DONE 0.0s

#18 diff (sh -c apt-get -s upgrade 2>/dev/null | grep -q ^Inst || exit 1) -> (/bin/sh -c if [ -s error_log.txt ]; then cat error_log.txt; exit 1; fi) #18 DONE 0.0s

#19 merge (docker-image://docker.io/ashnam/nginx:1.27.0, diff (sh -c apt-get -s upgrade 2>/dev/null | grep -q ^Inst || exit 1) -> (/bin/sh -c if [ -s error_log.txt ]; then cat error_log.txt; exit 1; fi)) #19 DONE 0.0s

#20 exporting to docker image format #20 exporting layers #20 exporting layers 1.3s done

#20 exporting manifest sha256:235d9dee515114c41a5c05dfe19c9c0d87041429b9b905be74a493e16dcf7a99 done #20 exporting config sha256:43350ca17497aa9121dd2f6ef9c655e6eacd9a17473aa0639951b98a737451e4 done #20 sending tarball #20 sending tarball 1.4s done #20 DONE 2.7s DEBU[0017] stopping session

docker-image://ghcr.io/project-copacetic/copacetic/debian:12-slim:


DEBU[0017] ImageLoad response stream: {"stream":"Loaded image: ashnam/nginx:1.27.0-patched\n"} INFO[0017] Image loaded successfully via Docker API
WARN[0017] --debug specified, working folder at /var/folders/hg/8m1_79bn113dldgg978t9dlh0000gn/T/copa-2614986045 needs to be manually cleaned up `

ashnamehrotra avatar May 12 '25 17:05 ashnamehrotra

Thanks for the detailed logs @ashnamehrotra

it looks like both local and remote resolution fails early in the loop:

DEBU[0000] local media type not found for docker.io/ashnam/nginx:1.27.0: Error response from daemon: manifest unknown: manifest unknown DEBU[0001] failed to get remote media type for docker.io/ashnam/nginx:1.27.0: GET https://index.docker.io/v2/ashnam/nginx/manifests/1.27.0: MANIFEST_UNKNOWN: manifest unknown; unknown tag=1.27.0 WARN[0001] unable to determine media type, defaulting to docker, err: GET https://index.docker.io/v2/ashnam/nginx/manifests/1.27.0: MANIFEST_UNKNOWN: manifest unknown; unknown tag=1.27.0

This is because DistributionInspect is trying to contact the registry so it's not actually a local call as I understood it before. I'm guessing you retagged nginx:1.27.0 locally? I was able to repro this issue by doing this. I've changed the localMediaType function to use ImageInspect instead to make it actually stay local. Thanks for the help!

robert-cronin avatar May 13 '25 01:05 robert-cronin

@robert-cronin yup it works now, thank you!

ashnamehrotra avatar May 13 '25 15:05 ashnamehrotra