Merge branch 'main' into fix/34785

Author: wxiaoguang

modules/packages/container/metadata.go

@@ -4,7 +4,6 @@
 package container
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"strings"
@@ -72,20 +71,39 @@ type Manifest struct {
 	Size     int64  `json:"size"`
 }
 
+func IsMediaTypeValid(mt string) bool {
+	return strings.HasPrefix(mt, "application/vnd.docker.") || strings.HasPrefix(mt, "application/vnd.oci.")
+}
+
+func IsMediaTypeImageManifest(mt string) bool {
+	return strings.EqualFold(mt, oci.MediaTypeImageManifest) || strings.EqualFold(mt, "application/vnd.docker.distribution.manifest.v2+json")
+}
+
+func IsMediaTypeImageIndex(mt string) bool {
+	return strings.EqualFold(mt, oci.MediaTypeImageIndex) || strings.EqualFold(mt, "application/vnd.docker.distribution.manifest.list.v2+json")
+}
+
 // ParseImageConfig parses the metadata of an image config
-func ParseImageConfig(mt string, r io.Reader) (*Metadata, error) {
-	if strings.EqualFold(mt, helm.ConfigMediaType) {
+func ParseImageConfig(mediaType string, r io.Reader) (*Metadata, error) {
+	if strings.EqualFold(mediaType, helm.ConfigMediaType) {
 		return parseHelmConfig(r)
 	}
 
 	// fallback to OCI Image Config
-	return parseOCIImageConfig(r)
+	// FIXME: this fallback is not right, we should strictly check the media type in the future
+	metadata, err := parseOCIImageConfig(r)
+	if err != nil {
+		if !IsMediaTypeImageManifest(mediaType) {
+			return &Metadata{Platform: "unknown/unknown"}, nil
+		}
+		return nil, err
+	}
+	return metadata, nil
 }
 
 func parseOCIImageConfig(r io.Reader) (*Metadata, error) {
 	var image oci.Image
-	// EOF means empty input, still use the default data
-	if err := json.NewDecoder(r).Decode(&image); err != nil && !errors.Is(err, io.EOF) {
+	if err := json.NewDecoder(r).Decode(&image); err != nil {
 		return nil, err
 	}
 

modules/packages/container/metadata_test.go

@@ -59,10 +59,8 @@ func TestParseImageConfig(t *testing.T) {
 	assert.ElementsMatch(t, []string{author}, metadata.Authors)
 	assert.Equal(t, projectURL, metadata.ProjectURL)
 	assert.Equal(t, repositoryURL, metadata.RepositoryURL)
-}
 
-func TestParseOCIImageConfig(t *testing.T) {
-	metadata, err := parseOCIImageConfig(strings.NewReader(""))
+	metadata, err = ParseImageConfig("anything-unknown", strings.NewReader(""))
 	require.NoError(t, err)
-	assert.Equal(t, &Metadata{Type: TypeOCI, Platform: DefaultPlatform, ImageLayers: []string{}}, metadata)
+	assert.Equal(t, &Metadata{Platform: "unknown/unknown"}, metadata)
 }

routers/api/packages/api.go

@@ -693,6 +693,8 @@ func ContainerRoutes() *web.Router {
 		&container.Auth{},
 	})
 
+	// TODO: Content Discovery / References (not implemented yet)
+
 	r.Get("", container.ReqContainerAccess, container.DetermineSupport)
 	r.Group("/token", func() {
 		r.Get("", container.Authenticate)

routers/api/packages/container/container.go

@@ -320,6 +320,7 @@ func PostBlobsUploads(ctx *context.Context) {
 
 // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-a-blob-in-chunks
 func GetBlobsUpload(ctx *context.Context) {
+	image := ctx.PathParam("image")
 	uuid := ctx.PathParam("uuid")
 
 	upload, err := packages_model.GetBlobUploadByID(ctx, uuid)
@@ -334,6 +335,7 @@ func GetBlobsUpload(ctx *context.Context) {
 
 	// FIXME: undefined behavior when the uploaded content is empty: https://github.com/opencontainers/distribution-spec/issues/578
 	respHeaders := &containerHeaders{
+		Location:   fmt.Sprintf("/v2/%s/%s/blobs/uploads/%s", ctx.Package.Owner.LowerName, image, upload.ID),
 		UploadUUID: upload.ID,
 		Status:     http.StatusNoContent,
 	}
@@ -386,7 +388,7 @@ func PatchBlobsUpload(ctx *context.Context) {
 		UploadUUID: uploader.ID,
 		Status:     http.StatusAccepted,
 	}
-	if contentRange != "" {
+	if uploader.Size() > 0 {
 		respHeaders.Range = fmt.Sprintf("0-%d", uploader.Size()-1)
 	}
 	setResponseHeaders(ctx.Resp, respHeaders)

routers/api/packages/container/manifest.go

@@ -29,18 +29,6 @@ import (
 	oci "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
-func isMediaTypeValid(mt string) bool {
-	return strings.HasPrefix(mt, "application/vnd.docker.") || strings.HasPrefix(mt, "application/vnd.oci.")
-}
-
-func isMediaTypeImageManifest(mt string) bool {
-	return strings.EqualFold(mt, oci.MediaTypeImageManifest) || strings.EqualFold(mt, "application/vnd.docker.distribution.manifest.v2+json")
-}
-
-func isMediaTypeImageIndex(mt string) bool {
-	return strings.EqualFold(mt, oci.MediaTypeImageIndex) || strings.EqualFold(mt, "application/vnd.docker.distribution.manifest.list.v2+json")
-}
-
 // manifestCreationInfo describes a manifest to create
 type manifestCreationInfo struct {
 	MediaType  string
@@ -66,16 +54,16 @@ func processManifest(ctx context.Context, mci *manifestCreationInfo, buf *packag
 		return "", err
 	}
 
-	if !isMediaTypeValid(mci.MediaType) {
+	if !container_module.IsMediaTypeValid(mci.MediaType) {
 		mci.MediaType = index.MediaType
-		if !isMediaTypeValid(mci.MediaType) {
+		if !container_module.IsMediaTypeValid(mci.MediaType) {
 			return "", errManifestInvalid.WithMessage("MediaType not recognized")
 		}
 	}
 
-	if isMediaTypeImageManifest(mci.MediaType) {
+	if container_module.IsMediaTypeImageManifest(mci.MediaType) {
 		return processOciImageManifest(ctx, mci, buf)
-	} else if isMediaTypeImageIndex(mci.MediaType) {
+	} else if container_module.IsMediaTypeImageIndex(mci.MediaType) {
 		return processOciImageIndex(ctx, mci, buf)
 	}
 	return "", errManifestInvalid
@@ -201,7 +189,7 @@ func processOciImageIndex(ctx context.Context, mci *manifestCreationInfo, buf *p
 		}
 
 		for _, manifest := range index.Manifests {
-			if !isMediaTypeImageManifest(manifest.MediaType) {
+			if !container_module.IsMediaTypeImageManifest(manifest.MediaType) {
 				return errManifestInvalid
 			}
 
@@ -336,7 +324,7 @@ func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, met
 			return nil, err
 		}
 
-		if isMediaTypeImageIndex(mci.MediaType) {
+		if container_module.IsMediaTypeImageIndex(mci.MediaType) {
 			if pv.CreatedUnix.AsTime().Before(time.Now().Add(-24 * time.Hour)) {
 				if err = packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil {
 					return nil, err

tests/integration/api_packages_container_test.go

@@ -297,11 +297,22 @@ func TestPackageContainer(t *testing.T) {
 					SetHeader("Content-Range", "1-10")
 				MakeRequest(t, req, http.StatusRequestedRangeNotSatisfiable)
 
-				contentRange := fmt.Sprintf("0-%d", len(blobContent)-1)
-				req.SetHeader("Content-Range", contentRange)
+				// first patch without Content-Range
+				req = NewRequestWithBody(t, "PATCH", setting.AppURL+uploadURL[1:], bytes.NewReader(blobContent[:1])).
+					AddTokenAuth(userToken)
+				resp = MakeRequest(t, req, http.StatusAccepted)
+				assert.NotEmpty(t, resp.Header().Get("Location"))
+				assert.Equal(t, "0-0", resp.Header().Get("Range"))
+
+				// then send remaining content with Content-Range
+				req = NewRequestWithBody(t, "PATCH", setting.AppURL+uploadURL[1:], bytes.NewReader(blobContent[1:])).
+					SetHeader("Content-Range", fmt.Sprintf("1-%d", len(blobContent)-1)).
+					AddTokenAuth(userToken)
 				resp = MakeRequest(t, req, http.StatusAccepted)
 
+				contentRange := fmt.Sprintf("0-%d", len(blobContent)-1)
 				assert.Equal(t, uuid, resp.Header().Get("Docker-Upload-Uuid"))
+				assert.NotEmpty(t, resp.Header().Get("Location"))
 				assert.Equal(t, contentRange, resp.Header().Get("Range"))
 
 				uploadURL = resp.Header().Get("Location")
@@ -311,6 +322,7 @@ func TestPackageContainer(t *testing.T) {
 				resp = MakeRequest(t, req, http.StatusNoContent)
 
 				assert.Equal(t, uuid, resp.Header().Get("Docker-Upload-Uuid"))
+				assert.Equal(t, uploadURL, resp.Header().Get("Location"))
 				assert.Equal(t, contentRange, resp.Header().Get("Range"))
 
 				pbu, err = packages_model.GetBlobUploadByID(db.DefaultContext, uuid)