fix: update OCI conformance workflow and improve blob upload handling (#4170)

Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>

Author: rchincha

.github/workflows/oci-conformance-action.yaml

@@ -20,7 +20,7 @@ jobs:
     runs-on: ubuntu-latest
     # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
-    - name: Install go 1.23
+    - name: Install go 1.26
       uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0
       with:
         cache: false
@@ -36,35 +36,24 @@ jobs:
           make binary
           RUNNER_TRACKING_ID="" && ./bin/zot-linux-amd64 serve examples/config-conformance.json &
           IP=`hostname -I | awk '{print $1}'`
-          echo "SERVER_URL=http://${IP}:8080" >> $GITHUB_ENV
-    - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
-      with:
-        repository: opencontainers/distribution-spec
-        ref: af32c8c0de81c7e2e7c74709fd98b1c11ac55659
-        path: distribution-spec
-        persist-credentials: false
-    - name: build conformance binary from main
-      run: |
-        (cd distribution-spec/ && make conformance-binary)
-        mv distribution-spec/output/conformance.test .
-        rm -rf distribution-spec/
+          echo "OCI_REGISTRY=${IP}:8080" >> $GITHUB_ENV
     - name: run conformance
+      # pinned to opencontainers/distribution-spec main@fcfba1ec55526073f48b2f6d4e3d7eef410ddcbc
+      uses: opencontainers/distribution-spec@fcfba1ec55526073f48b2f6d4e3d7eef410ddcbc
       env:
-        OCI_ROOT_URL: ${{ env.SERVER_URL }}
-        OCI_NAMESPACE: oci-conformance/distribution-test
-        OCI_TEST_PULL: 1
-        OCI_TEST_PUSH: 1
-        OCI_TEST_CONTENT_DISCOVERY: 1
-        OCI_TEST_CONTENT_MANAGEMENT: 1
-        OCI_REFERRERS: 1
-        OCI_CROSSMOUNT_NAMESPACE: oci-conformance/crossmount-test
-      run: |
-        ./conformance.test
-    - run: mkdir -p .out/ && mv {report.html,junit.xml} .out/
-      if: always()
-    - name: Upload test results zip as build artifact
+        OCI_REGISTRY: ${{ env.OCI_REGISTRY }}
+        OCI_TLS: "disabled"
+        OCI_REPO1: oci-conformance/distribution-test
+        OCI_REPO2: oci-conformance/crossmount-test
+        OCI_VERSION: "1.1"
+        OCI_RESULTS_DIR: "."
+    - name: upload conformance artifacts
       uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+      if: always()
       with:
-        name: oci-test-results-${{ github.sha }}
-        path: .out/
-      if: github.event_name == 'push'
+        name: oci-conformance-results
+        # Conformance output has used result.yaml/results.yaml across upstream versions.
+        path: |
+          junit.xml
+          report.html
+          result*.yaml

examples/config-conformance.json

@@ -3,7 +3,7 @@
   "storage": {
     "rootDirectory": "/tmp/zot",
     "gc": true,
-    "dedupe": true
+    "dedupe": false
   },
   "http": {
     "address": "0.0.0.0",

pkg/api/routes.go

@@ -1844,16 +1844,16 @@ func (rh *RouteHandler) CreateBlobUpload(response http.ResponseWriter, request *
 
 		var contentLength int64
 
-		contentLength, err = strconv.ParseInt(request.Header.Get("Content-Length"), 10, 64)
-		if err != nil || contentLength <= 0 {
-			rh.c.Log.Warn().Str("actual", request.Header.Get("Content-Length")).Msg("invalid content length")
-
-			details := map[string]string{"digest": digest.String()}
-
-			if err != nil {
-				details["conversion error"] = err.Error()
-			} else {
-				details["Content-Length"] = request.Header.Get("Content-Length")
+		// request.ContentLength is pre-parsed by net/http: 0 for an explicit empty
+		// body, -1 when unknown/chunked. Reject only the unknown case (< 0); a
+		// Content-Length of 0 is a valid empty-blob upload.
+		contentLength = request.ContentLength
+		if contentLength < 0 {
+			rh.c.Log.Warn().Int64("actual", contentLength).Msg("invalid content length")
+
+			details := map[string]string{
+				"digest":         digest.String(),
+				"Content-Length": strconv.FormatInt(contentLength, 10),
 			}
 
 			e := apiErr.NewError(apiErr.BLOB_UPLOAD_INVALID).AddDetail(details)
@@ -2134,39 +2134,44 @@ func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *
 		return
 	}
 
-	contentPresent := true
+	contentLenHeader := request.Header.Get("Content-Length")
+	contentRange := request.Header.Get("Content-Range")
+	// Header.Get cannot distinguish missing headers from present-but-empty headers.
+	contentLenHeaderPresent := len(request.Header.Values("Content-Length")) > 0
+	contentRangePresent := len(request.Header.Values("Content-Range")) > 0
 
-	contentLen, err := strconv.ParseInt(request.Header.Get("Content-Length"), 10, 64)
-	if err != nil {
-		contentPresent = false
-	}
+	shouldPutBlobChunk := contentLenHeaderPresent || contentRangePresent
 
-	contentRangePresent := true
+	var from, to int64
 
-	if request.Header.Get("Content-Range") == "" {
-		contentRangePresent = false
-	}
+	if shouldPutBlobChunk {
+		contentLen, err := strconv.ParseInt(contentLenHeader, 10, 64)
+		if err != nil || contentLen < 0 {
+			rh.c.Log.Warn().Str("actual", contentLenHeader).Msg("invalid content length")
 
-	// we expect at least one of "Content-Length" or "Content-Range" to be
-	// present
-	if !contentPresent && !contentRangePresent {
-		response.WriteHeader(http.StatusBadRequest)
+			details := map[string]string{"digest": digest.String()}
+			if err != nil {
+				details["conversion error"] = err.Error()
+			} else {
+				details["Content-Length"] = contentLenHeader
+			}
 
-		return
-	}
+			e := apiErr.NewError(apiErr.BLOB_UPLOAD_INVALID).AddDetail(details)
+			zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 
-	var from, to int64
+			return
+		}
 
-	if contentPresent {
-		contentRange := request.Header.Get("Content-Range")
 		if contentRange == "" { // monolithic upload
 			from = 0
 
-			if contentLen == 0 {
-				goto finish
+			if contentLen > 0 {
+				to = contentLen
+			} else {
+				// Zero-length monolithic upload (e.g. empty blob): skip
+				// PutBlobChunk and go straight to FinishBlobUpload below.
+				shouldPutBlobChunk = false
 			}
-
-			to = contentLen
 		} else if from, to, err = getContentRange(request); err != nil { // finish chunked upload
 			details := zerr.GetDetails(err)
 			details["session_id"] = sessionID
@@ -2175,7 +2180,9 @@ func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *
 
 			return
 		}
+	}
 
+	if shouldPutBlobChunk {
 		_, err = imgStore.PutBlobChunk(ctx, name, sessionID, from, to, request.Body)
 		if err != nil { //nolint:dupl
 			details := zerr.GetDetails(err)
@@ -2207,7 +2214,6 @@ func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *
 		}
 	}
 
-finish:
 	// blob chunks already transferred, just finish
 	if err := imgStore.FinishBlobUpload(name, sessionID, request.Body, digest); err != nil {
 		details := zerr.GetDetails(err)

pkg/api/routes_test.go

@@ -1353,6 +1353,23 @@ func TestRoutes(t *testing.T) {
 				})
 			So(statusCode, ShouldEqual, http.StatusInternalServerError)
 
+			// full blob upload with empty body and no Content-Length header
+			statusCode = testCreateBlobUpload(
+				[]struct{ k, v string }{
+					{"digest", "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
+				},
+				map[string]string{
+					"Content-Type": constants.BinaryMediaType,
+				},
+				&mocks.MockedImageStore{
+					FullBlobUploadFn: func(ctx context.Context, repo string, body io.Reader,
+						digest godigest.Digest,
+					) (string, int64, error) {
+						return sessionStr, 0, nil
+					},
+				})
+			So(statusCode, ShouldEqual, http.StatusCreated)
+
 			// newBlobUpload not found
 			statusCode = testCreateBlobUpload(
 				[]struct{ k, v string }{},

pkg/storage/common/common.go

@@ -186,7 +186,6 @@ func GetAndValidateRequestDigest(body []byte, reference string, log zlog.Logger)
 ) {
 	expectedDigest, err := godigest.Parse(reference)
 	if err != nil {
-		// This is a non-digest reference
 		return godigest.Canonical.FromBytes(body), err
 	}
 

pkg/storage/imagestore/imagestore.go

@@ -1444,7 +1444,34 @@ func (is *ImageStore) CheckBlob(ctx context.Context, repo string, digest godiges
 	}
 
 	binfo, err := is.storeDriver.Stat(blobPath)
-	if err == nil && binfo.Size() > 0 {
+	if err != nil {
+		dstRecord, err := is.checkCacheBlob(digest)
+		if err != nil {
+			if errors.Is(err, zerr.ErrCacheMiss) || errors.Is(err, zerr.ErrBlobNotFound) {
+				is.log.Debug().Err(err).Str("digest", digest.String()).Msg("cache miss for blob")
+			} else {
+				is.log.Warn().Err(err).Str("digest", digest.String()).Msg("failed to lookup blob in cache")
+			}
+
+			return false, -1, zerr.ErrBlobNotFound
+		}
+
+		blobSize, err := is.copyBlob(ctx, repo, blobPath, dstRecord)
+		if err != nil {
+			return false, -1, zerr.ErrBlobNotFound
+		}
+
+		// put deduped blob in cache
+		if err := is.cache.PutBlob(digest, blobPath); err != nil {
+			is.log.Error().Err(err).Str("blobPath", blobPath).Str("component", "dedupe").Msg("failed to insert blob record")
+
+			return false, -1, err
+		}
+
+		return true, blobSize, nil
+	}
+
+	if binfo.Size() > 0 {
 		// try to find blob size in blob descriptors, if blob can not be found
 		desc, err := common.GetBlobDescriptorFromRepo(is, repo, digest, is.log)
 		if err != nil || desc.Size == binfo.Size() {
@@ -1460,9 +1487,19 @@ func (is *ImageStore) CheckBlob(ctx context.Context, repo string, digest godiges
 			return false, -1, zerr.ErrBlobNotFound
 		}
 	}
-	// otherwise is a 'deduped' blob (empty file)
 
-	// Check blobs in cache
+	// Size == 0: either a genuine empty blob, or an S3-style deduped placeholder.
+	// Distinguish by comparing the digest against the hash of empty content for
+	// the same algorithm (cheap single-pass hash over 0 bytes).
+	emptyDigest := digest.Algorithm().FromBytes(nil)
+	if emptyDigest == digest {
+		// Genuine empty blob (e.g. sha256:e3b0c44... or sha512:cf83e13...).
+		is.log.Debug().Str("blob path", blobPath).Msg("empty blob found")
+
+		return true, 0, nil
+	}
+
+	// S3-style deduped placeholder: the real content lives elsewhere in the cache.
 	dstRecord, err := is.checkCacheBlob(digest)
 	if err != nil {
 		// Cache miss / not-found is a normal condition when the blob truly doesn't exist.
@@ -1633,6 +1670,16 @@ func (is *ImageStore) originalBlobInfo(repo string, digest godigest.Digest) (dri
 	}
 
 	if binfo.Size() == 0 {
+		// A zero-size file is either a genuine empty blob or an S3-style
+		// deduplication placeholder pointing into the cache.  Distinguish the
+		// two by checking whether the digest matches the hash of zero bytes for
+		// the same algorithm (cheap single-pass hash over 0 bytes).
+		emptyDigest := digest.Algorithm().FromBytes(nil)
+		if emptyDigest == digest {
+			// Genuine empty blob – return its FileInfo as-is.
+			return binfo, nil
+		}
+
 		dstRecord, err := is.checkCacheBlob(digest)
 		if err != nil {
 			is.log.Debug().Err(err).Str("digest", digest.String()).Msg("not found in cache")

pkg/storage/local/local_test.go

@@ -1677,6 +1677,28 @@ func TestDedupeRestoreCompleteMarker(t *testing.T) {
 	})
 }
 
+func TestCheckBlobTreatsRealEmptyBlobAsPresent(t *testing.T) {
+	Convey("CheckBlob returns present for genuine empty blob with dedupe/cache disabled", t, func() {
+		dir := t.TempDir()
+
+		logger := zlog.NewTestLogger()
+		metrics := monitoring.NewMetricsServer(false, logger)
+
+		imgStore := local.NewImageStore(dir, false, true, logger, metrics, nil, nil, nil, nil)
+
+		emptyDigest := godigest.Canonical.FromBytes(nil)
+
+		_, size, err := imgStore.FullBlobUpload(context.Background(), repoName, bytes.NewReader(nil), emptyDigest)
+		So(err, ShouldBeNil)
+		So(size, ShouldEqual, 0)
+
+		found, blobSize, err := imgStore.CheckBlob(context.Background(), repoName, emptyDigest)
+		So(err, ShouldBeNil)
+		So(found, ShouldBeTrue)
+		So(blobSize, ShouldEqual, int64(0))
+	})
+}
+
 // recheckDriver wraps local.Driver and, for a single target path, returns a
 // custom result on the second Stat call to simulate the state observed by
 // restoreDedupedBlobs' re-check under the write lock.