Address feedback from Tonis

Signed-off-by: John Howard <jhoward@microsoft.com>

builder/dockerfile/builder.go

@@ -358,6 +358,9 @@ func addNodesForLabelOption(dockerfile *parser.Node, labels map[string]string) {
 //
 // TODO: Remove?
 func BuildFromConfig(config *container.Config, changes []string, os string) (*container.Config, error) {
+	if !system.IsOSSupported(os) {
+		return nil, errdefs.InvalidParameter(system.ErrNotSupportedOperatingSystem)
+	}
 	if len(changes) == 0 {
 		return config, nil
 	}

builder/dockerfile/dispatchers.go

@@ -156,7 +156,9 @@ func initializeStage(d dispatchRequest, cmd *instructions.Stage) error {
 		return err
 	}
 	state := d.state
-	state.beginStage(cmd.Name, image)
+	if err := state.beginStage(cmd.Name, image); err != nil {
+		return err
+	}
 	if len(state.runConfig.OnBuild) > 0 {
 		triggers := state.runConfig.OnBuild
 		state.runConfig.OnBuild = nil
@@ -309,7 +311,9 @@ func resolveCmdLine(cmd instructions.ShellDependantCmdLine, runConfig *container
 // RUN [ "echo", "hi" ] # echo hi
 //
 func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error {
-
+	if !system.IsOSSupported(d.state.operatingSystem) {
+		return system.ErrNotSupportedOperatingSystem
+	}
 	stateRunConfig := d.state.runConfig
 	cmdFromArgs := resolveCmdLine(c.ShellDependantCmdLine, stateRunConfig, d.state.operatingSystem)
 	buildArgs := d.state.buildArgs.FilterAllowed(stateRunConfig.Env)

builder/dockerfile/evaluator.go

@@ -21,6 +21,7 @@ package dockerfile
 
 import (
 	"reflect"
+	"runtime"
 	"strconv"
 	"strings"
 
@@ -211,10 +212,16 @@ func (s *dispatchState) hasFromImage() bool {
 	return s.imageID != "" || (s.baseImage != nil && s.baseImage.ImageID() == "")
 }
 
-func (s *dispatchState) beginStage(stageName string, image builder.Image) {
+func (s *dispatchState) beginStage(stageName string, image builder.Image) error {
 	s.stageName = stageName
 	s.imageID = image.ImageID()
 	s.operatingSystem = image.OperatingSystem()
+	if s.operatingSystem == "" { // In case it isn't set
+		s.operatingSystem = runtime.GOOS
+	}
+	if !system.IsOSSupported(s.operatingSystem) {
+		return system.ErrNotSupportedOperatingSystem
+	}
 
 	if image.RunConfig() != nil {
 		// copy avoids referencing the same instance when 2 stages have the same base
@@ -226,6 +233,7 @@ func (s *dispatchState) beginStage(stageName string, image builder.Image) {
 	s.setDefaultPath()
 	s.runConfig.OpenStdin = false
 	s.runConfig.StdinOnce = false
+	return nil
 }
 
 // Add the default PATH to runConfig.ENV if one exists for the operating system and there

daemon/build.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/docker/pkg/containerfs"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/stringid"
+	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/registry"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -70,7 +71,7 @@ func (rl *releaseableLayer) Commit() (builder.ReleaseableLayer, error) {
 	if err != nil {
 		return nil, err
 	}
-	// TODO: An optimization woudld be to handle empty layers before returning
+	// TODO: An optimization would be to handle empty layers before returning
 	return &releaseableLayer{layerStore: rl.layerStore, roLayer: newLayer}, nil
 }
 
@@ -171,6 +172,9 @@ func (daemon *Daemon) pullForBuilder(ctx context.Context, name string, authConfi
 // leaking of layers.
 func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) {
 	if refOrID == "" {
+		if !system.IsOSSupported(opts.OS) {
+			return nil, nil, system.ErrNotSupportedOperatingSystem
+		}
 		layer, err := newReleasableLayerForImage(nil, daemon.layerStores[opts.OS])
 		return nil, layer, err
 	}
@@ -182,6 +186,9 @@ func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID st
 		}
 		// TODO: shouldn't we error out if error is different from "not found" ?
 		if image != nil {
+			if !system.IsOSSupported(image.OperatingSystem()) {
+				return nil, nil, system.ErrNotSupportedOperatingSystem
+			}
 			layer, err := newReleasableLayerForImage(image, daemon.layerStores[image.OperatingSystem()])
 			return image, layer, err
 		}
@@ -191,6 +198,9 @@ func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID st
 	if err != nil {
 		return nil, nil, err
 	}
+	if !system.IsOSSupported(image.OperatingSystem()) {
+		return nil, nil, system.ErrNotSupportedOperatingSystem
+	}
 	layer, err := newReleasableLayerForImage(image, daemon.layerStores[image.OperatingSystem()])
 	return image, layer, err
 }

daemon/commit.go

@@ -17,6 +17,7 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/ioutils"
+	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 )
 
@@ -149,6 +150,9 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
 		daemon.containerPause(container)
 		defer daemon.containerUnpause(container)
 	}
+	if !system.IsOSSupported(container.OS) {
+		return "", system.ErrNotSupportedOperatingSystem
+	}
 
 	if c.MergeConfigs && c.Config == nil {
 		c.Config = container.Config
@@ -251,6 +255,7 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
 }
 
 func (daemon *Daemon) exportContainerRw(container *container.Container) (arch io.ReadCloser, err error) {
+	// Note: Indexing by OS is safe as only called from `Commit` which has already performed validation
 	rwlayer, err := daemon.layerStores[container.OS].GetRWLayer(container.ID)
 	if err != nil {
 		return nil, err

daemon/create.go

@@ -270,6 +270,8 @@ func (daemon *Daemon) setRWLayer(container *container.Container) error {
 		StorageOpt: container.HostConfig.StorageOpt,
 	}
 
+	// Indexing by OS is safe here as validation of OS has already been performed in create() (the only
+	// caller), and guaranteed non-nil
 	rwLayer, err := daemon.layerStores[container.OS].CreateRWLayer(container.ID, layerID, rwLayerOpts)
 	if err != nil {
 		return err

daemon/daemon.go

@@ -155,7 +155,10 @@ func (daemon *Daemon) restore() error {
 			logrus.Errorf("Failed to load container %v: %v", id, err)
 			continue
 		}
-
+		if !system.IsOSSupported(container.OS) {
+			logrus.Errorf("Failed to load container %v: %s (%q)", id, system.ErrNotSupportedOperatingSystem, container.OS)
+			continue
+		}
 		// Ignore the container if it does not support the current driver being used by the graph
 		currentDriverForContainerOS := daemon.graphDrivers[container.OS]
 		if (container.Driver == "" && currentDriverForContainerOS == "aufs") || container.Driver == currentDriverForContainerOS {

daemon/delete.go

@@ -94,6 +94,9 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
 			return fmt.Errorf("Could not kill running container %s, cannot remove - %v", container.ID, err)
 		}
 	}
+	if !system.IsOSSupported(container.OS) {
+		return fmt.Errorf("cannot remove %s: %s ", container.ID, system.ErrNotSupportedOperatingSystem)
+	}
 
 	// stop collection of stats for the container regardless
 	// if stats are currently getting collected.

daemon/export.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/ioutils"
+	"github.com/docker/docker/pkg/system"
 )
 
 // ContainerExport writes the contents of the container to the given
@@ -47,6 +48,9 @@ func (daemon *Daemon) ContainerExport(name string, out io.Writer) error {
 }
 
 func (daemon *Daemon) containerExport(container *container.Container) (arch io.ReadCloser, err error) {
+	if !system.IsOSSupported(container.OS) {
+		return nil, fmt.Errorf("cannot export %s: %s ", container.ID, system.ErrNotSupportedOperatingSystem)
+	}
 	rwlayer, err := daemon.layerStores[container.OS].GetRWLayer(container.ID)
 	if err != nil {
 		return nil, err

daemon/image_delete.go

@@ -11,6 +11,7 @@ import (
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/stringid"
+	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 )
 
@@ -66,10 +67,13 @@ func (daemon *Daemon) ImageDelete(imageRef string, force, prune bool) ([]types.I
 	start := time.Now()
 	records := []types.ImageDeleteResponseItem{}
 
-	imgID, _, err := daemon.GetImageIDAndOS(imageRef)
+	imgID, operatingSystem, err := daemon.GetImageIDAndOS(imageRef)
 	if err != nil {
 		return nil, err
 	}
+	if !system.IsOSSupported(operatingSystem) {
+		return nil, errors.Errorf("unable to delete image: %q", system.ErrNotSupportedOperatingSystem)
+	}
 
 	repoRefs := daemon.referenceStore.References(imgID.Digest())
 

daemon/image_inspect.go

@@ -6,6 +6,7 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/layer"
+	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 )
 
@@ -16,7 +17,9 @@ func (daemon *Daemon) LookupImage(name string) (*types.ImageInspect, error) {
 	if err != nil {
 		return nil, errors.Wrapf(err, "no such image: %s", name)
 	}
-
+	if !system.IsOSSupported(img.OperatingSystem()) {
+		return nil, system.ErrNotSupportedOperatingSystem
+	}
 	refs := daemon.referenceStore.References(img.ID().Digest())
 	repoTags := []string{}
 	repoDigests := []string{}

daemon/images.go

@@ -14,6 +14,7 @@ import (
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
+	"github.com/docker/docker/pkg/system"
 )
 
 var acceptedImageFilterTags = map[string]bool{
@@ -113,6 +114,13 @@ func (daemon *Daemon) Images(imageFilters filters.Args, all bool, withExtraAttrs
 			}
 		}
 
+		// Skip any images with an unsupported operating system to avoid a potential
+		// panic when indexing through the layerstore. Don't error as we want to list
+		// the other images. This should never happen, but here as a safety precaution.
+		if !system.IsOSSupported(img.OperatingSystem()) {
+			continue
+		}
+
 		layerID := img.RootFS.ChainID()
 		var size int64
 		if layerID != "" {

daemon/oci_windows.go

@@ -138,6 +138,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
 	max := len(img.RootFS.DiffIDs)
 	for i := 1; i <= max; i++ {
 		img.RootFS.DiffIDs = img.RootFS.DiffIDs[:i]
+		if !system.IsOSSupported(img.OperatingSystem()) {
+			return nil, fmt.Errorf("cannot get layerpath for ImageID %s: %s ", img.RootFS.ChainID(), system.ErrNotSupportedOperatingSystem)
+		}
 		layerPath, err := layer.GetLayerPath(daemon.layerStores[img.OperatingSystem()], img.RootFS.ChainID())
 		if err != nil {
 			return nil, fmt.Errorf("failed to get layer path from graphdriver %s for ImageID %s - %s", daemon.layerStores[img.OperatingSystem()], img.RootFS.ChainID(), err)

distribution/config.go

@@ -158,6 +158,9 @@ func (s *imageConfigStore) RootFSAndOSFromConfig(c []byte) (*image.RootFS, strin
 	if os == "" {
 		os = runtime.GOOS
 	}
+	if !system.IsOSSupported(os) {
+		return nil, "", system.ErrNotSupportedOperatingSystem
+	}
 	return unmarshalledConfig.RootFS, os, nil
 }
 

distribution/push_v1.go

@@ -14,6 +14,7 @@ import (
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/stringid"
+	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/registry"
 	"github.com/opencontainers/go-digest"
 	"github.com/sirupsen/logrus"
@@ -210,6 +211,9 @@ func (p *v1Pusher) imageListForTag(imgID image.ID, dependenciesSeen map[layer.Ch
 
 	topLayerID := img.RootFS.ChainID()
 
+	if !system.IsOSSupported(img.OperatingSystem()) {
+		return nil, system.ErrNotSupportedOperatingSystem
+	}
 	pl, err := p.config.LayerStores[img.OperatingSystem()].Get(topLayerID)
 	*referencedLayers = append(*referencedLayers, pl)
 	if err != nil {

distribution/xfer/download.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/progress"
+	"github.com/docker/docker/pkg/system"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 )
@@ -105,10 +106,14 @@ func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS ima
 		downloadsByKey = make(map[string]*downloadTransfer)
 	)
 
-	// Assume that the operating system is the host OS if blank
+	// Assume that the operating system is the host OS if blank, and validate it
+	// to ensure we don't cause a panic by an invalid index into the layerstores.
 	if os == "" {
 		os = runtime.GOOS
 	}
+	if !system.IsOSSupported(os) {
+		return image.RootFS{}, nil, system.ErrNotSupportedOperatingSystem
+	}
 
 	rootFS := initialRootFS
 	for _, descriptor := range layers {

image/store.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/docker/distribution/digestset"
 	"github.com/docker/docker/layer"
+	"github.com/docker/docker/pkg/system"
 	"github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -73,6 +74,9 @@ func (is *store) restore() error {
 		}
 		var l layer.Layer
 		if chainID := img.RootFS.ChainID(); chainID != "" {
+			if !system.IsOSSupported(img.OperatingSystem()) {
+				return system.ErrNotSupportedOperatingSystem
+			}
 			l, err = is.lss[img.OperatingSystem()].Get(chainID)
 			if err != nil {
 				return err
@@ -148,6 +152,9 @@ func (is *store) Create(config []byte) (ID, error) {
 
 	var l layer.Layer
 	if layerID != "" {
+		if !system.IsOSSupported(img.OperatingSystem()) {
+			return "", system.ErrNotSupportedOperatingSystem
+		}
 		l, err = is.lss[img.OperatingSystem()].Get(layerID)
 		if err != nil {
 			return "", errors.Wrapf(err, "failed to get layer %s", layerID)

layer/filestore_unix.go

@@ -4,12 +4,12 @@ package layer
 
 import "runtime"
 
-// SetOS writes the "os" file to the layer filestore
-func (fm *fileMetadataTransaction) SetOS(os string) error {
+// setOS writes the "os" file to the layer filestore
+func (fm *fileMetadataTransaction) setOS(os string) error {
 	return nil
 }
 
-// GetOS reads the "os" file from the layer filestore
-func (fms *fileMetadataStore) GetOS(layer ChainID) (string, error) {
+// getOS reads the "os" file from the layer filestore
+func (fms *fileMetadataStore) getOS(layer ChainID) (string, error) {
 	return runtime.GOOS, nil
 }

layer/filestore_windows.go

@@ -7,16 +7,16 @@ import (
 	"strings"
 )
 
-// SetOS writes the "os" file to the layer filestore
-func (fm *fileMetadataTransaction) SetOS(os string) error {
+// setOS writes the "os" file to the layer filestore
+func (fm *fileMetadataTransaction) setOS(os string) error {
 	if os == "" {
 		return nil
 	}
 	return fm.ws.WriteFile("os", []byte(os), 0644)
 }
 
-// GetOS reads the "os" file from the layer filestore
-func (fms *fileMetadataStore) GetOS(layer ChainID) (string, error) {
+// getOS reads the "os" file from the layer filestore
+func (fms *fileMetadataStore) getOS(layer ChainID) (string, error) {
 	contentBytes, err := ioutil.ReadFile(fms.getLayerFilename(layer, "os"))
 	if err != nil {
 		// For backwards compatibility, the os file may not exist. Default to "windows" if missing.

layer/layer.go

@@ -216,7 +216,7 @@ type MetadataTransaction interface {
 	SetDiffID(DiffID) error
 	SetCacheID(string) error
 	SetDescriptor(distribution.Descriptor) error
-	SetOS(string) error
+	setOS(string) error
 	TarSplitWriter(compressInput bool) (io.WriteCloser, error)
 
 	Commit(ChainID) error
@@ -237,7 +237,7 @@ type MetadataStore interface {
 	GetDiffID(ChainID) (DiffID, error)
 	GetCacheID(ChainID) (string, error)
 	GetDescriptor(ChainID) (distribution.Descriptor, error)
-	GetOS(ChainID) (string, error)
+	getOS(ChainID) (string, error)
 	TarSplitReader(ChainID) (io.ReadCloser, error)
 
 	SetMountID(string, string) error