Merge pull request moby#29001 from darrenstahlmsft/WindowsOnLinux

Block pulling Windows images on non-Windows daemons

Author: vdemeester

distribution/config.go

@@ -142,9 +142,12 @@ func (s *imageConfigStore) RootFSFromConfig(c []byte) (*image.RootFS, error) {
 		return nil, err
 	}
 
-	// fail immediately on windows
+	// fail immediately on Windows when downloading a non-Windows image
+	// and vice versa
 	if runtime.GOOS == "windows" && unmarshalledConfig.OS == "linux" {
 		return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
+	} else if runtime.GOOS != "windows" && unmarshalledConfig.OS == "windows" {
+		return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
 	}
 
 	return unmarshalledConfig.RootFS, nil

distribution/pull_v2.go

@@ -534,15 +534,18 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 	}
 
 	configChan := make(chan []byte, 1)
-	errChan := make(chan error, 1)
+	configErrChan := make(chan error, 1)
+	layerErrChan := make(chan error, 1)
+	downloadsDone := make(chan struct{})
 	var cancel func()
 	ctx, cancel = context.WithCancel(ctx)
+	defer cancel()
 
 	// Pull the image config
 	go func() {
 		configJSON, err := p.pullSchema2Config(ctx, target.Digest)
 		if err != nil {
-			errChan <- ImageConfigPullError{Err: err}
+			configErrChan <- ImageConfigPullError{Err: err}
 			cancel()
 			return
 		}
@@ -553,6 +556,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 		configJSON       []byte        // raw serialized image config
 		downloadedRootFS *image.RootFS // rootFS from registered layers
 		configRootFS     *image.RootFS // rootFS from configuration
+		release          func()        // release resources from rootFS download
 	)
 
 	// https://github.com/docker/docker/issues/24766 - Err on the side of caution,
@@ -564,7 +568,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 	// check to block Windows images being pulled on Linux is implemented, it
 	// may be necessary to perform the same type of serialisation.
 	if runtime.GOOS == "windows" {
-		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan)
+		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
 		if err != nil {
 			return "", "", err
 		}
@@ -575,41 +579,52 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
 	}
 
 	if p.config.DownloadManager != nil {
-		downloadRootFS := *image.NewRootFS()
-		rootFS, release, err := p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput)
-		if err != nil {
-			if configJSON != nil {
-				// Already received the config
-				return "", "", err
-			}
-			select {
-			case err = <-errChan:
-				return "", "", err
-			default:
-				cancel()
-				select {
-				case <-configChan:
-				case <-errChan:
-				}
-				return "", "", err
+		go func() {
+			var (
+				err    error
+				rootFS image.RootFS
+			)
+			downloadRootFS := *image.NewRootFS()
+			rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput)
+			if err != nil {
+				// Intentionally do not cancel the config download here
+				// as the error from config download (if there is one)
+				// is more interesting than the layer download error
+				layerErrChan <- err
+				return
 			}
-		}
-		if release != nil {
-			defer release()
-		}
 
-		downloadedRootFS = &rootFS
+			downloadedRootFS = &rootFS
+			close(downloadsDone)
+		}()
+	} else {
+		// We have nothing to download
+		close(downloadsDone)
 	}
 
 	if configJSON == nil {
-		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan)
+		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
+		if err == nil && configRootFS == nil {
+			err = errRootFSInvalid
+		}
 		if err != nil {
+			cancel()
+			select {
+			case <-downloadsDone:
+			case <-layerErrChan:
+			}
 			return "", "", err
 		}
+	}
 
-		if configRootFS == nil {
-			return "", "", errRootFSInvalid
-		}
+	select {
+	case <-downloadsDone:
+	case err = <-layerErrChan:
+		return "", "", err
+	}
+
+	if release != nil {
+		defer release()
 	}
 
 	if downloadedRootFS != nil {

integration-cli/docker_cli_pull_test.go

@@ -272,3 +272,10 @@ func (s *DockerSuite) TestPullLinuxImageFailsOnWindows(c *check.C) {
 	_, _, err := dockerCmdWithError("pull", "ubuntu")
 	c.Assert(err.Error(), checker.Contains, "cannot be used on this platform")
 }
+
+// Regression test for https://github.com/docker/docker/issues/28892
+func (s *DockerSuite) TestPullWindowsImageFailsOnLinux(c *check.C) {
+	testRequires(c, DaemonIsLinux, Network)
+	_, _, err := dockerCmdWithError("pull", "microsoft/nanoserver")
+	c.Assert(err.Error(), checker.Contains, "cannot be used on this platform")
+}