Merge pull request #43103 from thaJeztah/image_import_platform

Add support for platform (os and architecture) on image import

api/server/router/image/backend.go

@@ -30,7 +30,7 @@ type imageBackend interface {
 
 type importExportBackend interface {
 	LoadImage(inTar io.ReadCloser, outStream io.Writer, quiet bool) error
-	ImportImage(src string, repository, platform string, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error
+	ImportImage(src string, repository string, platform *specs.Platform, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error
 	ExportImage(names []string, outStream io.Writer) error
 }
 

api/server/router/image/image_routes.go

@@ -29,13 +29,13 @@ func (s *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrite
 	}
 
 	var (
-		image    = r.Form.Get("fromImage")
-		repo     = r.Form.Get("repo")
-		tag      = r.Form.Get("tag")
-		message  = r.Form.Get("message")
-		err      error
-		output   = ioutils.NewWriteFlusher(w)
-		platform *specs.Platform
+		image       = r.Form.Get("fromImage")
+		repo        = r.Form.Get("repo")
+		tag         = r.Form.Get("tag")
+		message     = r.Form.Get("message")
+		progressErr error
+		output      = ioutils.NewWriteFlusher(w)
+		platform    *specs.Platform
 	)
 	defer output.Close()
 
@@ -43,9 +43,8 @@ func (s *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrite
 
 	version := httputils.VersionFromContext(ctx)
 	if versions.GreaterThanOrEqualTo(version, "1.32") {
-		apiPlatform := r.FormValue("platform")
-		if apiPlatform != "" {
-			sp, err := platforms.Parse(apiPlatform)
+		if p := r.FormValue("platform"); p != "" {
+			sp, err := platforms.Parse(p)
 			if err != nil {
 				return err
 			}
@@ -71,23 +70,16 @@ func (s *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrite
 				authConfig = &types.AuthConfig{}
 			}
 		}
-		err = s.backend.PullImage(ctx, image, tag, platform, metaHeaders, authConfig, output)
+		progressErr = s.backend.PullImage(ctx, image, tag, platform, metaHeaders, authConfig, output)
 	} else { // import
 		src := r.Form.Get("fromSrc")
-		// 'err' MUST NOT be defined within this block, we need any error
-		// generated from the download to be available to the output
-		// stream processing below
-		os := ""
-		if platform != nil {
-			os = platform.OS
-		}
-		err = s.backend.ImportImage(src, repo, os, tag, message, r.Body, output, r.Form["changes"])
+		progressErr = s.backend.ImportImage(src, repo, platform, tag, message, r.Body, output, r.Form["changes"])
 	}
-	if err != nil {
+	if progressErr != nil {
 		if !output.Flushed() {
-			return err
+			return progressErr
 		}
-		_, _ = output.Write(streamformatter.FormatError(err))
+		_, _ = output.Write(streamformatter.FormatError(progressErr))
 	}
 
 	return nil

api/swagger.yaml

@@ -7678,7 +7678,22 @@ paths:
             type: "string"
         - name: "platform"
           in: "query"
-          description: "Platform in the format os[/arch[/variant]]"
+          description: |
+            Platform in the format os[/arch[/variant]].
+
+            When used in combination with the `fromImage` option, the daemon checks
+            if the given image is present in the local image cache with the given
+            OS and Architecture, and otherwise attempts to pull the image. If the
+            option is not set, the host's native OS and Architecture are used.
+            If the given image does not exist in the local image cache, the daemon
+            attempts to pull the image with the host's native OS and Architecture.
+            If the given image does exists in the local image cache, but its OS or
+            architecture does not match, a warning is produced.
+
+            When used with the `fromSrc` option to import an image from an archive,
+            this option sets the platform information for the imported image. If
+            the option is not set, the host's native OS and Architecture are used
+            for the imported image.
           type: "string"
           default: ""
       tags: ["Image"]

daemon/images/image_import.go

@@ -5,10 +5,10 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"runtime"
 	"strings"
 	"time"
 
+	"github.com/containerd/containerd/platforms"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/dockerfile"
@@ -20,6 +20,7 @@ import (
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
 
@@ -27,18 +28,13 @@ import (
 // inConfig (if src is "-"), or from a URI specified in src. Progress output is
 // written to outStream. Repository and tag names can optionally be given in
 // the repo and tag arguments, respectively.
-func (i *ImageService) ImportImage(src string, repository, os string, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error {
+func (i *ImageService) ImportImage(src string, repository string, platform *specs.Platform, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error {
 	var (
 		rc     io.ReadCloser
 		resp   *http.Response
 		newRef reference.Named
 	)
 
-	// Default the operating system if not supplied.
-	if os == "" {
-		os = runtime.GOOS
-	}
-
 	if repository != "" {
 		var err error
 		newRef, err = reference.ParseNormalizedNamed(repository)
@@ -57,7 +53,13 @@ func (i *ImageService) ImportImage(src string, repository, os string, tag string
 		}
 	}
 
-	config, err := dockerfile.BuildFromConfig(&container.Config{}, changes, os)
+	// Normalize platform - default to the operating system and architecture if not supplied.
+	if platform == nil {
+		p := platforms.DefaultSpec()
+		platform = &p
+	}
+
+	config, err := dockerfile.BuildFromConfig(&container.Config{}, changes, platform.OS)
 	if err != nil {
 		return err
 	}
@@ -102,8 +104,9 @@ func (i *ImageService) ImportImage(src string, repository, os string, tag string
 		V1Image: image.V1Image{
 			DockerVersion: dockerversion.Version,
 			Config:        config,
-			Architecture:  runtime.GOARCH,
-			OS:            os,
+			Architecture:  platform.Architecture,
+			Variant:       platform.Variant,
+			OS:            platform.OS,
 			Created:       created,
 			Comment:       msg,
 		},

docs/api/version-history.md

@@ -34,6 +34,12 @@ keywords: "API, Docker, rcli, REST, documentation"
   (`a disk usage operation is already running`) would be returned in this
   situation. This change is not versioned, and affects all API versions if the
   daemon has this patch.
+* The `POST /images/create` now supports both the operating system and architecture
+  that is passed through the `platform` query parameter when using the `fromSrc`
+  option to import an image from an archive. Previously, only the operating system
+  was used and the architecture was ignored. If no `platform` option is set, the
+  host's operating system and architecture as used as default. This change is not
+  versioned, and affects all API versions if the daemon has this patch.
 
 ## v1.41 API changes
 

integration-cli/docker_cli_import_test.go

@@ -33,7 +33,7 @@ func (s *DockerSuite) TestImportDisplay(c *testing.T) {
 }
 
 func (s *DockerSuite) TestImportBadURL(c *testing.T) {
-	out, _, err := dockerCmdWithError("import", "http://nourl/bad")
+	out, _, err := dockerCmdWithError("import", "https://nosuchdomain.invalid/bad")
 	assert.Assert(c, err != nil, "import was supposed to fail but didn't")
 	// Depending on your system you can get either of these errors
 	if !strings.Contains(out, "dial tcp") &&

integration/image/import_test.go

@@ -6,10 +6,12 @@ import (
 	"context"
 	"io"
 	"runtime"
+	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/image"
 	"github.com/docker/docker/testutil"
 	"github.com/docker/docker/testutil/daemon"
 	"gotest.tools/v3/assert"
@@ -47,3 +49,99 @@ func TestImportExtremelyLargeImageWorks(t *testing.T) {
 		types.ImageImportOptions{})
 	assert.NilError(t, err)
 }
+
+func TestImportWithCustomPlatform(t *testing.T) {
+	skip.If(t, testEnv.OSType == "windows", "TODO enable on windows")
+
+	defer setupTest(t)()
+	client := testEnv.APIClient()
+	ctx := context.Background()
+
+	// Construct an empty tar archive.
+	var tarBuffer bytes.Buffer
+
+	tw := tar.NewWriter(&tarBuffer)
+	err := tw.Close()
+	assert.NilError(t, err)
+	imageRdr := io.MultiReader(&tarBuffer, io.LimitReader(testutil.DevZero, 0))
+
+	tests := []struct {
+		name        string
+		platform    string
+		expected    image.V1Image
+		expectedErr string
+	}{
+		{
+			platform: "",
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: runtime.GOARCH, // this may fail on armhf due to normalization?
+			},
+		},
+		{
+			platform:    "       ",
+			expectedErr: "is an invalid component",
+		},
+		{
+			platform:    "/",
+			expectedErr: "is an invalid component",
+		},
+		{
+			platform: runtime.GOOS,
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: runtime.GOARCH, // this may fail on armhf due to normalization?
+			},
+		},
+		{
+			platform: strings.ToUpper(runtime.GOOS),
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: runtime.GOARCH, // this may fail on armhf due to normalization?
+			},
+		},
+		{
+			platform: runtime.GOOS + "/sparc64",
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: "sparc64",
+			},
+		},
+		{
+			platform:    "macos",
+			expectedErr: "operating system is not supported",
+		},
+		{
+			platform:    "macos/arm64",
+			expectedErr: "operating system is not supported",
+		},
+		{
+			// TODO: platforms.Normalize() only validates os or arch if a single component is passed,
+			//       but ignores unknown os/arch in other cases. See:
+			//       https://github.com/containerd/containerd/blob/7d4891783aac5adf6cd83f657852574a71875631/platforms/platforms.go#L183-L209
+			platform:    "nintendo64",
+			expectedErr: "unknown operating system or architecture",
+		},
+	}
+
+	for i, tc := range tests {
+		tc := tc
+		t.Run(tc.platform, func(t *testing.T) {
+			reference := "import-with-platform:tc-" + strconv.Itoa(i)
+			_, err = client.ImageImport(context.Background(),
+				types.ImageImportSource{Source: imageRdr, SourceName: "-"},
+				reference,
+				types.ImageImportOptions{Platform: tc.platform})
+			if tc.expectedErr != "" {
+				assert.ErrorContains(t, err, tc.expectedErr)
+			} else {
+				assert.NilError(t, err)
+
+				inspect, _, err := client.ImageInspectWithRaw(ctx, reference)
+				assert.NilError(t, err)
+				assert.Equal(t, inspect.Os, tc.expected.OS)
+				assert.Equal(t, inspect.Architecture, tc.expected.Architecture)
+			}
+		})
+	}
+}