diff --git a/daemon/commit.go b/daemon/commit.go index 7b7d4e4446..f7b69d13d0 100644 --- a/daemon/commit.go +++ b/daemon/commit.go @@ -38,16 +38,16 @@ func merge(userConf, imageConf *containertypes.Config) error { } else { for _, imageEnv := range imageConf.Env { found := false - imageEnvKey := strings.Split(imageEnv, "=")[0] + imageEnvKey, _, _ := strings.Cut(imageEnv, "=") for _, userEnv := range userConf.Env { - userEnvKey := strings.Split(userEnv, "=")[0] + userEnvKey, _, _ := strings.Cut(userEnv, "=") if isWindows { // Case insensitive environment variables on Windows - imageEnvKey = strings.ToUpper(imageEnvKey) - userEnvKey = strings.ToUpper(userEnvKey) + found = strings.EqualFold(imageEnvKey, userEnvKey) + } else { + found = imageEnvKey == userEnvKey } - if imageEnvKey == userEnvKey { - found = true + if found { break } } diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 85ad568cbc..b3ad66739a 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -107,18 +107,18 @@ func (daemon *Daemon) buildSandboxOptions(container *container.Container) ([]lib if _, err := opts.ValidateExtraHost(extraHost); err != nil { return nil, err } - parts := strings.SplitN(extraHost, ":", 2) + host, ip, _ := strings.Cut(extraHost, ":") // If the IP Address is a string called "host-gateway", replace this // value with the IP address stored in the daemon level HostGatewayIP // config variable - if parts[1] == opts.HostGatewayName { + if ip == opts.HostGatewayName { gateway := daemon.configStore.HostGatewayIP.String() if gateway == "" { return nil, fmt.Errorf("unable to derive the IP value for host-gateway") } - parts[1] = gateway + ip = gateway } - sboxOptions = append(sboxOptions, libnetwork.OptionExtraHost(parts[0], parts[1])) + sboxOptions = append(sboxOptions, libnetwork.OptionExtraHost(host, ip)) } if container.HostConfig.PortBindings != nil { diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 33e4eeecbf..5e7abf3239 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -215,26 +215,27 @@ func parseSecurityOpt(container *container.Container, config *containertypes.Hos continue } - var con []string + var k, v string + var ok bool if strings.Contains(opt, "=") { - con = strings.SplitN(opt, "=", 2) + k, v, ok = strings.Cut(opt, "=") } else if strings.Contains(opt, ":") { - con = strings.SplitN(opt, ":", 2) + k, v, ok = strings.Cut(opt, ":") logrus.Warn("Security options with `:` as a separator are deprecated and will be completely unsupported in 17.04, use `=` instead.") } - if len(con) != 2 { + if !ok { return fmt.Errorf("invalid --security-opt 1: %q", opt) } - switch con[0] { + switch k { case "label": - labelOpts = append(labelOpts, con[1]) + labelOpts = append(labelOpts, v) case "apparmor": - container.AppArmorProfile = con[1] + container.AppArmorProfile = v case "seccomp": - container.SeccompProfile = con[1] + container.SeccompProfile = v case "no-new-privileges": - noNewPrivileges, err := strconv.ParseBool(con[1]) + noNewPrivileges, err := strconv.ParseBool(v) if err != nil { return fmt.Errorf("invalid --security-opt 2: %q", opt) } @@ -1360,8 +1361,8 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig * return errors.Wrapf(err, "could not get container for %s", name) } for child.HostConfig.NetworkMode.IsContainer() { - parts := strings.SplitN(string(child.HostConfig.NetworkMode), ":", 2) - child, err = daemon.GetContainer(parts[1]) + cid := child.HostConfig.NetworkMode.ConnectedContainer() + child, err = daemon.GetContainer(cid) if err != nil { if errdefs.IsNotFound(err) { // Trying to link to a non-existing container is not valid, and @@ -1370,7 +1371,7 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig * // image could not be found (see moby/moby#39823) err = errdefs.InvalidParameter(err) } - return errors.Wrapf(err, "Could not get container for %s", parts[1]) + return errors.Wrapf(err, "could not get container for %s", cid) } } if child.HostConfig.NetworkMode.IsHost() { diff --git a/daemon/info.go b/daemon/info.go index 4162870042..44af037b5e 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -206,9 +206,7 @@ func (daemon *Daemon) fillAPIInfo(v *types.Info) { cfg := daemon.configStore for _, host := range cfg.Hosts { // cnf.Hosts is normalized during startup, so should always have a scheme/proto - h := strings.SplitN(host, "://", 2) - proto := h[0] - addr := h[1] + proto, addr, _ := strings.Cut(host, "://") if proto != "tcp" { continue } diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index 82b035f903..2806abfaae 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -241,8 +241,7 @@ func WithNamespaces(daemon *Daemon, c *container.Container) coci.SpecOpts { // network if !c.Config.NetworkDisabled { ns := specs.LinuxNamespace{Type: "network"} - parts := strings.SplitN(string(c.HostConfig.NetworkMode), ":", 2) - if parts[0] == "container" { + if c.HostConfig.NetworkMode.IsContainer() { nc, err := daemon.getNetworkedContainer(c.ID, c.HostConfig.NetworkMode.ConnectedContainer()) if err != nil { return err diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go index 38764c524a..9c55925ed2 100644 --- a/daemon/oci_windows.go +++ b/daemon/oci_windows.go @@ -279,29 +279,31 @@ func (daemon *Daemon) setWindowsCredentialSpec(c *container.Container, s *specs. // this doesn't seem like a great idea? credentialSpec := "" + // TODO(thaJeztah): extract validating and parsing SecurityOpt to a reusable function. for _, secOpt := range c.HostConfig.SecurityOpt { - optSplits := strings.SplitN(secOpt, "=", 2) - if len(optSplits) != 2 { + k, v, ok := strings.Cut(secOpt, "=") + if !ok { return errdefs.InvalidParameter(fmt.Errorf("invalid security option: no equals sign in supplied value %s", secOpt)) } - if !strings.EqualFold(optSplits[0], "credentialspec") { - return errdefs.InvalidParameter(fmt.Errorf("security option not supported: %s", optSplits[0])) + // FIXME(thaJeztah): options should not be case-insensitive + if !strings.EqualFold(k, "credentialspec") { + return errdefs.InvalidParameter(fmt.Errorf("security option not supported: %s", k)) } - credSpecSplits := strings.SplitN(optSplits[1], "://", 2) - if len(credSpecSplits) != 2 || credSpecSplits[1] == "" { + scheme, value, ok := strings.Cut(v, "://") + if !ok || value == "" { return errInvalidCredentialSpecSecOpt } - value := credSpecSplits[1] - var err error - switch strings.ToLower(credSpecSplits[0]) { + switch strings.ToLower(scheme) { case "file": - if credentialSpec, err = readCredentialSpecFile(c.ID, daemon.root, filepath.Clean(value)); err != nil { + credentialSpec, err = readCredentialSpecFile(c.ID, daemon.root, filepath.Clean(value)) + if err != nil { return errdefs.InvalidParameter(err) } case "registry": - if credentialSpec, err = readCredentialSpecRegistry(c.ID, value); err != nil { + credentialSpec, err = readCredentialSpecRegistry(c.ID, value) + if err != nil { return errdefs.InvalidParameter(err) } case "config": @@ -439,44 +441,41 @@ func readCredentialSpecRegistry(id, name string) (string, error) { // This allows for staging on machines which do not have the necessary components. func readCredentialSpecFile(id, root, location string) (string, error) { if filepath.IsAbs(location) { - return "", fmt.Errorf("invalid credential spec - file:// path cannot be absolute") + return "", fmt.Errorf("invalid credential spec: file:// path cannot be absolute") } base := filepath.Join(root, credentialSpecFileLocation) full := filepath.Join(base, location) if !strings.HasPrefix(full, base) { - return "", fmt.Errorf("invalid credential spec - file:// path must be under %s", base) + return "", fmt.Errorf("invalid credential spec: file:// path must be under %s", base) } bcontents, err := os.ReadFile(full) if err != nil { - return "", errors.Wrapf(err, "credential spec for container %s could not be read from file %q", id, full) + return "", errors.Wrapf(err, "failed to load credential spec for container %s", id) } return string(bcontents[:]), nil } func setupWindowsDevices(devices []containertypes.DeviceMapping) (specDevices []specs.WindowsDevice, err error) { - if len(devices) == 0 { - return - } - for _, deviceMapping := range devices { - devicePath := deviceMapping.PathOnHost - if strings.HasPrefix(devicePath, "class/") { - devicePath = strings.Replace(devicePath, "class/", "class://", 1) + if strings.HasPrefix(deviceMapping.PathOnHost, "class/") { + specDevices = append(specDevices, specs.WindowsDevice{ + ID: strings.TrimPrefix(deviceMapping.PathOnHost, "class/"), + IDType: "class", + }) + } else { + idType, id, ok := strings.Cut(deviceMapping.PathOnHost, "://") + if !ok { + return nil, errors.Errorf("invalid device assignment path: '%s', must be 'class/ID' or 'IDType://ID'", deviceMapping.PathOnHost) + } + if idType == "" { + return nil, errors.Errorf("invalid device assignment path: '%s', IDType cannot be empty", deviceMapping.PathOnHost) + } + specDevices = append(specDevices, specs.WindowsDevice{ + ID: id, + IDType: idType, + }) } - - srcParts := strings.SplitN(devicePath, "://", 2) - if len(srcParts) != 2 { - return nil, errors.Errorf("invalid device assignment path: '%s', must be 'class/ID' or 'IDType://ID'", deviceMapping.PathOnHost) - } - if srcParts[0] == "" { - return nil, errors.Errorf("invalid device assignment path: '%s', IDType cannot be empty", deviceMapping.PathOnHost) - } - wd := specs.WindowsDevice{ - ID: srcParts[1], - IDType: srcParts[0], - } - specDevices = append(specDevices, wd) } - return + return specDevices, nil } diff --git a/daemon/oci_windows_test.go b/daemon/oci_windows_test.go index ea0330b368..9237ddc12e 100644 --- a/daemon/oci_windows_test.go +++ b/daemon/oci_windows_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/fs" containertypes "github.com/docker/docker/api/types/container" @@ -87,7 +88,7 @@ func TestSetWindowsCredentialSpecInSpec(t *testing.T) { spec := &specs.Spec{} err := daemon.setWindowsCredentialSpec(containerFactory(`file://C:\path\to\my\credspec.json`), spec) - assert.ErrorContains(t, err, "invalid credential spec - file:// path cannot be absolute") + assert.ErrorContains(t, err, "invalid credential spec: file:// path cannot be absolute") assert.Check(t, spec.Windows == nil) }) @@ -96,7 +97,7 @@ func TestSetWindowsCredentialSpecInSpec(t *testing.T) { spec := &specs.Spec{} err := daemon.setWindowsCredentialSpec(containerFactory(`file://..\credspec.json`), spec) - assert.ErrorContains(t, err, fmt.Sprintf("invalid credential spec - file:// path must be under %s", credSpecsDir)) + assert.ErrorContains(t, err, fmt.Sprintf("invalid credential spec: file:// path must be under %s", credSpecsDir)) assert.Check(t, spec.Windows == nil) }) @@ -105,9 +106,8 @@ func TestSetWindowsCredentialSpecInSpec(t *testing.T) { spec := &specs.Spec{} err := daemon.setWindowsCredentialSpec(containerFactory("file://i-dont-exist.json"), spec) - assert.ErrorContains(t, err, fmt.Sprintf("credential spec for container %s could not be read from file", dummyContainerID)) - assert.ErrorContains(t, err, "The system cannot find") - + assert.Check(t, is.ErrorContains(err, fmt.Sprintf("failed to load credential spec for container %s", dummyContainerID))) + assert.Check(t, is.ErrorIs(err, os.ErrNotExist)) assert.Check(t, spec.Windows == nil) })