diff --git a/api/server/router/image/backend.go b/api/server/router/image/backend.go index 624b6c7363..d54d2c0e6c 100644 --- a/api/server/router/image/backend.go +++ b/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 } diff --git a/api/server/router/image/image_routes.go b/api/server/router/image/image_routes.go index 9a4648bb49..71b0c14f64 100644 --- a/api/server/router/image/image_routes.go +++ b/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 diff --git a/api/swagger.yaml b/api/swagger.yaml index 3b1ab5e404..46505d06c0 100644 --- a/api/swagger.yaml +++ b/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"] diff --git a/daemon/images/image_import.go b/daemon/images/image_import.go index bd0f60c674..bec67f68a4 100644 --- a/daemon/images/image_import.go +++ b/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, }, diff --git a/docs/api/version-history.md b/docs/api/version-history.md index e782fa6782..d6e44c4bbc 100644 --- a/docs/api/version-history.md +++ b/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 diff --git a/integration-cli/docker_cli_import_test.go b/integration-cli/docker_cli_import_test.go index 03ce0b9fac..d8fbeb0fc3 100644 --- a/integration-cli/docker_cli_import_test.go +++ b/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") && diff --git a/integration/image/import_test.go b/integration/image/import_test.go index 2880df8d32..110ab87a5f 100644 --- a/integration/image/import_test.go +++ b/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) + } + }) + } +}