Browse code

Merge pull request #29001 from darrenstahlmsft/WindowsOnLinux

Block pulling Windows images on non-Windows daemons

Vincent Demeester authored on 2017/02/17 03:57:54
Showing 3 changed files
... ...
@@ -142,9 +142,12 @@ func (s *imageConfigStore) RootFSFromConfig(c []byte) (*image.RootFS, error) {
142 142
 		return nil, err
143 143
 	}
144 144
 
145
-	// fail immediately on windows
145
+	// fail immediately on Windows when downloading a non-Windows image
146
+	// and vice versa
146 147
 	if runtime.GOOS == "windows" && unmarshalledConfig.OS == "linux" {
147 148
 		return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
149
+	} else if runtime.GOOS != "windows" && unmarshalledConfig.OS == "windows" {
150
+		return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
148 151
 	}
149 152
 
150 153
 	return unmarshalledConfig.RootFS, nil
... ...
@@ -534,15 +534,18 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
534 534
 	}
535 535
 
536 536
 	configChan := make(chan []byte, 1)
537
-	errChan := make(chan error, 1)
537
+	configErrChan := make(chan error, 1)
538
+	layerErrChan := make(chan error, 1)
539
+	downloadsDone := make(chan struct{})
538 540
 	var cancel func()
539 541
 	ctx, cancel = context.WithCancel(ctx)
542
+	defer cancel()
540 543
 
541 544
 	// Pull the image config
542 545
 	go func() {
543 546
 		configJSON, err := p.pullSchema2Config(ctx, target.Digest)
544 547
 		if err != nil {
545
-			errChan <- ImageConfigPullError{Err: err}
548
+			configErrChan <- ImageConfigPullError{Err: err}
546 549
 			cancel()
547 550
 			return
548 551
 		}
... ...
@@ -553,6 +556,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
553 553
 		configJSON       []byte        // raw serialized image config
554 554
 		downloadedRootFS *image.RootFS // rootFS from registered layers
555 555
 		configRootFS     *image.RootFS // rootFS from configuration
556
+		release          func()        // release resources from rootFS download
556 557
 	)
557 558
 
558 559
 	// 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
564 564
 	// check to block Windows images being pulled on Linux is implemented, it
565 565
 	// may be necessary to perform the same type of serialisation.
566 566
 	if runtime.GOOS == "windows" {
567
-		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan)
567
+		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
568 568
 		if err != nil {
569 569
 			return "", "", err
570 570
 		}
... ...
@@ -575,41 +579,52 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s
575 575
 	}
576 576
 
577 577
 	if p.config.DownloadManager != nil {
578
-		downloadRootFS := *image.NewRootFS()
579
-		rootFS, release, err := p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput)
580
-		if err != nil {
581
-			if configJSON != nil {
582
-				// Already received the config
583
-				return "", "", err
584
-			}
585
-			select {
586
-			case err = <-errChan:
587
-				return "", "", err
588
-			default:
589
-				cancel()
590
-				select {
591
-				case <-configChan:
592
-				case <-errChan:
593
-				}
594
-				return "", "", err
578
+		go func() {
579
+			var (
580
+				err    error
581
+				rootFS image.RootFS
582
+			)
583
+			downloadRootFS := *image.NewRootFS()
584
+			rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput)
585
+			if err != nil {
586
+				// Intentionally do not cancel the config download here
587
+				// as the error from config download (if there is one)
588
+				// is more interesting than the layer download error
589
+				layerErrChan <- err
590
+				return
595 591
 			}
596
-		}
597
-		if release != nil {
598
-			defer release()
599
-		}
600 592
 
601
-		downloadedRootFS = &rootFS
593
+			downloadedRootFS = &rootFS
594
+			close(downloadsDone)
595
+		}()
596
+	} else {
597
+		// We have nothing to download
598
+		close(downloadsDone)
602 599
 	}
603 600
 
604 601
 	if configJSON == nil {
605
-		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan)
602
+		configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan)
603
+		if err == nil && configRootFS == nil {
604
+			err = errRootFSInvalid
605
+		}
606 606
 		if err != nil {
607
+			cancel()
608
+			select {
609
+			case <-downloadsDone:
610
+			case <-layerErrChan:
611
+			}
607 612
 			return "", "", err
608 613
 		}
614
+	}
609 615
 
610
-		if configRootFS == nil {
611
-			return "", "", errRootFSInvalid
612
-		}
616
+	select {
617
+	case <-downloadsDone:
618
+	case err = <-layerErrChan:
619
+		return "", "", err
620
+	}
621
+
622
+	if release != nil {
623
+		defer release()
613 624
 	}
614 625
 
615 626
 	if downloadedRootFS != nil {
... ...
@@ -272,3 +272,10 @@ func (s *DockerSuite) TestPullLinuxImageFailsOnWindows(c *check.C) {
272 272
 	_, _, err := dockerCmdWithError("pull", "ubuntu")
273 273
 	c.Assert(err.Error(), checker.Contains, "cannot be used on this platform")
274 274
 }
275
+
276
+// Regression test for https://github.com/docker/docker/issues/28892
277
+func (s *DockerSuite) TestPullWindowsImageFailsOnLinux(c *check.C) {
278
+	testRequires(c, DaemonIsLinux, Network)
279
+	_, _, err := dockerCmdWithError("pull", "microsoft/nanoserver")
280
+	c.Assert(err.Error(), checker.Contains, "cannot be used on this platform")
281
+}