Address feedback from Tonis

Signed-off-by: John Howard <jhoward@microsoft.com>
This commit is contained in:
John Howard 2017-11-20 08:33:20 -08:00
parent afd305c4b5
commit 0cba7740d4
24 changed files with 103 additions and 21 deletions

View File

@ -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
}

View File

@ -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)

View File

@ -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

View File

@ -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
}

View File

@ -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

View File

@ -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

View File

@ -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 {

View File

@ -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.

View File

@ -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

View File

@ -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())

View File

@ -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{}

View File

@ -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 != "" {

View File

@ -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)

View File

@ -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
}

View File

@ -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 {

View File

@ -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 {

View File

@ -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)

View File

@ -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
}

View File

@ -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.

View File

@ -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

View File

@ -150,7 +150,7 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) {
return nil, fmt.Errorf("failed to get descriptor for %s: %s", layer, err)
}
os, err := ls.store.GetOS(layer)
os, err := ls.store.getOS(layer)
if err != nil {
return nil, fmt.Errorf("failed to get operating system for %s: %s", layer, err)
}

View File

@ -146,7 +146,7 @@ func storeLayer(tx MetadataTransaction, layer *roLayer) error {
return err
}
}
if err := tx.SetOS(layer.layerStore.os); err != nil {
if err := tx.setOS(layer.layerStore.os); err != nil {
return err
}

View File

@ -7,4 +7,7 @@ import (
var (
// ErrNotSupportedPlatform means the platform is not supported.
ErrNotSupportedPlatform = errors.New("platform and architecture is not supported")
// ErrNotSupportedOperatingSystem means the operating system is not supported.
ErrNotSupportedOperatingSystem = errors.New("operating system is not supported")
)

View File

@ -62,7 +62,7 @@ func IsOSSupported(os string) bool {
if runtime.GOOS == os {
return true
}
if LCOWSupported() && runtime.GOOS == "windows" && os == "linux" {
if LCOWSupported() && os == "linux" {
return true
}
return false