Merge pull request #44379 from thaJeztah/container_strings_cut

api/types/container: refactor to use strings.Cut, DRY, move tests and fix validation

api/types/container/host_config.go → api/types/container/hostconfig.go

@@ -147,17 +147,14 @@ func (n NetworkMode) IsPrivate() bool {
 
 // IsContainer indicates whether container uses a container network stack.
 func (n NetworkMode) IsContainer() bool {
-	parts := strings.SplitN(string(n), ":", 2)
-	return len(parts) > 1 && parts[0] == "container"
+	_, ok := containerID(string(n))
+	return ok
 }
 
 // ConnectedContainer is the id of the container which network this container is connected to.
-func (n NetworkMode) ConnectedContainer() string {
-	parts := strings.SplitN(string(n), ":", 2)
-	if len(parts) > 1 {
-		return parts[1]
-	}
-	return ""
+func (n NetworkMode) ConnectedContainer() (idOrName string) {
+	idOrName, _ = containerID(string(n))
+	return idOrName
 }
 
 // UserDefined indicates user-created network
@@ -178,18 +175,12 @@ func (n UsernsMode) IsHost() bool {
 
 // IsPrivate indicates whether the container uses the a private userns.
 func (n UsernsMode) IsPrivate() bool {
-	return !(n.IsHost())
+	return !n.IsHost()
 }
 
 // Valid indicates whether the userns is valid.
 func (n UsernsMode) Valid() bool {
-	parts := strings.Split(string(n), ":")
-	switch mode := parts[0]; mode {
-	case "", "host":
-	default:
-		return false
-	}
-	return true
+	return n == "" || n.IsHost()
 }
 
 // CgroupSpec represents the cgroup to use for the container.
@@ -197,22 +188,19 @@ type CgroupSpec string
 
 // IsContainer indicates whether the container is using another container cgroup
 func (c CgroupSpec) IsContainer() bool {
-	parts := strings.SplitN(string(c), ":", 2)
-	return len(parts) > 1 && parts[0] == "container"
+	_, ok := containerID(string(c))
+	return ok
 }
 
 // Valid indicates whether the cgroup spec is valid.
 func (c CgroupSpec) Valid() bool {
-	return c.IsContainer() || c == ""
+	return c == "" || c.IsContainer()
 }
 
-// Container returns the name of the container whose cgroup will be used.
-func (c CgroupSpec) Container() string {
-	parts := strings.SplitN(string(c), ":", 2)
-	if len(parts) > 1 {
-		return parts[1]
-	}
-	return ""
+// Container returns the ID or name of the container whose cgroup will be used.
+func (c CgroupSpec) Container() (idOrName string) {
+	idOrName, _ = containerID(string(c))
+	return idOrName
 }
 
 // UTSMode represents the UTS namespace of the container.
@@ -220,7 +208,7 @@ type UTSMode string
 
 // IsPrivate indicates whether the container uses its private UTS namespace.
 func (n UTSMode) IsPrivate() bool {
-	return !(n.IsHost())
+	return !n.IsHost()
 }
 
 // IsHost indicates whether the container uses the host's UTS namespace.
@@ -230,13 +218,7 @@ func (n UTSMode) IsHost() bool {
 
 // Valid indicates whether the UTS namespace is valid.
 func (n UTSMode) Valid() bool {
-	parts := strings.Split(string(n), ":")
-	switch mode := parts[0]; mode {
-	case "", "host":
-	default:
-		return false
-	}
-	return true
+	return n == "" || n.IsHost()
 }
 
 // PidMode represents the pid namespace of the container.
@@ -254,32 +236,19 @@ func (n PidMode) IsHost() bool {
 
 // IsContainer indicates whether the container uses a container's pid namespace.
 func (n PidMode) IsContainer() bool {
-	parts := strings.SplitN(string(n), ":", 2)
-	return len(parts) > 1 && parts[0] == "container"
+	_, ok := containerID(string(n))
+	return ok
 }
 
 // Valid indicates whether the pid namespace is valid.
 func (n PidMode) Valid() bool {
-	parts := strings.Split(string(n), ":")
-	switch mode := parts[0]; mode {
-	case "", "host":
-	case "container":
-		if len(parts) != 2 || parts[1] == "" {
-			return false
-		}
-	default:
-		return false
-	}
-	return true
+	return n == "" || n.IsHost() || n.IsContainer()
 }
 
 // Container returns the name of the container whose pid namespace is going to be used.
-func (n PidMode) Container() string {
-	parts := strings.SplitN(string(n), ":", 2)
-	if len(parts) > 1 {
-		return parts[1]
-	}
-	return ""
+func (n PidMode) Container() (idOrName string) {
+	idOrName, _ = containerID(string(n))
+	return idOrName
 }
 
 // DeviceRequest represents a request for devices from a device driver.
@@ -463,3 +432,13 @@ type HostConfig struct {
 	// Run a custom init inside the container, if null, use the daemon's configured settings
 	Init *bool `json:",omitempty"`
 }
+
+// containerID splits "container:<ID|name>" values. It returns the container
+// ID or name, and whether an ID/name was found. It returns an empty string and
+// a "false" if the value does not have a "container:" prefix. Further validation
+// of the returned, including checking if the value is empty, should be handled
+// by the caller.
+func containerID(val string) (idOrName string, ok bool) {
+	k, idOrName, ok := strings.Cut(val, ":")
+	return idOrName, ok && k == "container"
+}

api/types/container/hostconfig_unix_test.go

@@ -0,0 +1,172 @@
+//go:build !windows
+// +build !windows
+
+package container
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestCgroupnsMode(t *testing.T) {
+	modes := map[CgroupnsMode]struct{ private, host, empty, valid bool }{
+		"":                {private: false, host: false, empty: true, valid: true},
+		"something:weird": {private: false, host: false, empty: false, valid: false},
+		"host":            {private: false, host: true, empty: false, valid: true},
+		"host:":           {valid: false},
+		"host:name":       {valid: false},
+		":name":           {valid: false},
+		":":               {valid: false},
+		"private":         {private: true, host: false, empty: false, valid: true},
+		"private:name":    {private: false, host: false, empty: false, valid: false},
+	}
+	for mode, expected := range modes {
+		t.Run("mode="+string(mode), func(t *testing.T) {
+			assert.Check(t, is.Equal(mode.IsPrivate(), expected.private))
+			assert.Check(t, is.Equal(mode.IsHost(), expected.host))
+			assert.Check(t, is.Equal(mode.IsEmpty(), expected.empty))
+			assert.Check(t, is.Equal(mode.Valid(), expected.valid))
+		})
+	}
+}
+
+// TODO Windows: This will need addressing for a Windows daemon.
+func TestNetworkMode(t *testing.T) {
+	modes := map[NetworkMode]struct {
+		private, bridge, host, container, none, isDefault bool
+		name                                              string
+	}{
+		"":                {private: true, bridge: false, host: false, container: false, none: false, isDefault: false, name: ""},
+		"something:weird": {private: true, bridge: false, host: false, container: false, none: false, isDefault: false, name: "something:weird"},
+		"bridge":          {private: true, bridge: true, host: false, container: false, none: false, isDefault: false, name: "bridge"},
+		"host":            {private: false, bridge: false, host: true, container: false, none: false, isDefault: false, name: "host"},
+		"container:name":  {private: false, bridge: false, host: false, container: true, none: false, isDefault: false, name: "container"},
+		"none":            {private: true, bridge: false, host: false, container: false, none: true, isDefault: false, name: "none"},
+		"default":         {private: true, bridge: false, host: false, container: false, none: false, isDefault: true, name: "default"},
+	}
+	for mode, expected := range modes {
+		t.Run("mode="+string(mode), func(t *testing.T) {
+			assert.Check(t, is.Equal(mode.IsPrivate(), expected.private))
+			assert.Check(t, is.Equal(mode.IsBridge(), expected.bridge))
+			assert.Check(t, is.Equal(mode.IsHost(), expected.host))
+			assert.Check(t, is.Equal(mode.IsContainer(), expected.container))
+			assert.Check(t, is.Equal(mode.IsNone(), expected.none))
+			assert.Check(t, is.Equal(mode.IsDefault(), expected.isDefault))
+			assert.Check(t, is.Equal(mode.NetworkName(), expected.name))
+		})
+	}
+}
+
+func TestIpcMode(t *testing.T) {
+	ipcModes := map[IpcMode]struct {
+		private   bool
+		host      bool
+		container bool
+		shareable bool
+		valid     bool
+		ctrName   string
+	}{
+		"":                      {valid: true},
+		"private":               {private: true, valid: true},
+		"something:weird":       {},
+		":weird":                {},
+		"host":                  {host: true, valid: true},
+		"host:":                 {valid: false},
+		"host:name":             {valid: false},
+		":name":                 {valid: false},
+		":":                     {valid: false},
+		"container":             {},
+		"container:":            {container: true, valid: true, ctrName: ""},
+		"container:name":        {container: true, valid: true, ctrName: "name"},
+		"container:name1:name2": {container: true, valid: true, ctrName: "name1:name2"},
+		"shareable":             {shareable: true, valid: true},
+	}
+
+	for mode, expected := range ipcModes {
+		t.Run("mode="+string(mode), func(t *testing.T) {
+			assert.Check(t, is.Equal(mode.IsPrivate(), expected.private))
+			assert.Check(t, is.Equal(mode.IsHost(), expected.host))
+			assert.Check(t, is.Equal(mode.IsContainer(), expected.container))
+			assert.Check(t, is.Equal(mode.IsShareable(), expected.shareable))
+			assert.Check(t, is.Equal(mode.Valid(), expected.valid))
+			assert.Check(t, is.Equal(mode.Container(), expected.ctrName))
+		})
+	}
+}
+
+func TestUTSMode(t *testing.T) {
+	modes := map[UTSMode]struct{ private, host, valid bool }{
+		"":                {private: true, host: false, valid: true},
+		"something:weird": {private: true, host: false, valid: false},
+		"host":            {private: false, host: true, valid: true},
+		"host:":           {private: true, valid: false},
+		"host:name":       {private: true, valid: false},
+		":name":           {private: true, valid: false},
+		":":               {private: true, valid: false},
+	}
+	for mode, expected := range modes {
+		t.Run("mode="+string(mode), func(t *testing.T) {
+			assert.Check(t, is.Equal(mode.IsPrivate(), expected.private))
+			assert.Check(t, is.Equal(mode.IsHost(), expected.host))
+			assert.Check(t, is.Equal(mode.Valid(), expected.valid))
+		})
+
+	}
+}
+
+func TestUsernsMode(t *testing.T) {
+	modes := map[UsernsMode]struct{ private, host, valid bool }{
+		"":                {private: true, host: false, valid: true},
+		"something:weird": {private: true, host: false, valid: false},
+		"host":            {private: false, host: true, valid: true},
+		"host:":           {private: true, valid: false},
+		"host:name":       {private: true, valid: false},
+		":name":           {private: true, valid: false},
+		":":               {private: true, valid: false},
+	}
+	for mode, expected := range modes {
+		t.Run("mode="+string(mode), func(t *testing.T) {
+			assert.Check(t, is.Equal(mode.IsPrivate(), expected.private))
+			assert.Check(t, is.Equal(mode.IsHost(), expected.host))
+			assert.Check(t, is.Equal(mode.Valid(), expected.valid))
+		})
+	}
+}
+
+func TestPidMode(t *testing.T) {
+	modes := map[PidMode]struct{ private, host, valid bool }{
+		"":                {private: true, host: false, valid: true},
+		"something:weird": {private: true, host: false, valid: false},
+		"host":            {private: false, host: true, valid: true},
+		"host:":           {private: true, valid: false},
+		"host:name":       {private: true, valid: false},
+		":name":           {private: true, valid: false},
+		":":               {private: true, valid: false},
+	}
+	for mode, expected := range modes {
+		t.Run("mode="+string(mode), func(t *testing.T) {
+			assert.Check(t, is.Equal(mode.IsPrivate(), expected.private))
+			assert.Check(t, is.Equal(mode.IsHost(), expected.host))
+			assert.Check(t, is.Equal(mode.Valid(), expected.valid))
+		})
+	}
+}
+
+func TestRestartPolicy(t *testing.T) {
+	policies := map[RestartPolicy]struct{ none, always, onFailure bool }{
+		{Name: "", MaximumRetryCount: 0}:           {none: true, always: false, onFailure: false},
+		{Name: "something", MaximumRetryCount: 0}:  {none: false, always: false, onFailure: false},
+		{Name: "no", MaximumRetryCount: 0}:         {none: true, always: false, onFailure: false},
+		{Name: "always", MaximumRetryCount: 0}:     {none: false, always: true, onFailure: false},
+		{Name: "on-failure", MaximumRetryCount: 0}: {none: false, always: false, onFailure: true},
+	}
+	for policy, expected := range policies {
+		t.Run("policy="+policy.Name, func(t *testing.T) {
+			assert.Check(t, is.Equal(policy.IsNone(), expected.none))
+			assert.Check(t, is.Equal(policy.IsAlways(), expected.always))
+			assert.Check(t, is.Equal(policy.IsOnFailure(), expected.onFailure))
+		})
+	}
+}

runconfig/hostconfig_test.go

@@ -12,198 +12,8 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/pkg/sysinfo"
 	"gotest.tools/v3/assert"
-	is "gotest.tools/v3/assert/cmp"
 )
 
-func TestCgroupnsModeTest(t *testing.T) {
-	cgroupNsModes := map[container.CgroupnsMode][]bool{
-		// private, host, empty, valid
-		"":                {false, false, true, true},
-		"something:weird": {false, false, false, false},
-		"host":            {false, true, false, true},
-		"host:name":       {false, false, false, false},
-		"private":         {true, false, false, true},
-		"private:name":    {false, false, false, false},
-	}
-	for cgroupNsMode, state := range cgroupNsModes {
-		if cgroupNsMode.IsPrivate() != state[0] {
-			t.Fatalf("CgroupnsMode.IsPrivate for %v should have been %v but was %v", cgroupNsMode, state[0], cgroupNsMode.IsPrivate())
-		}
-		if cgroupNsMode.IsHost() != state[1] {
-			t.Fatalf("CgroupnsMode.IsHost for %v should have been %v but was %v", cgroupNsMode, state[1], cgroupNsMode.IsHost())
-		}
-		if cgroupNsMode.IsEmpty() != state[2] {
-			t.Fatalf("CgroupnsMode.Valid for %v should have been %v but was %v", cgroupNsMode, state[2], cgroupNsMode.Valid())
-		}
-		if cgroupNsMode.Valid() != state[3] {
-			t.Fatalf("CgroupnsMode.Valid for %v should have been %v but was %v", cgroupNsMode, state[2], cgroupNsMode.Valid())
-		}
-	}
-}
-
-// TODO Windows: This will need addressing for a Windows daemon.
-func TestNetworkModeTest(t *testing.T) {
-	networkModes := map[container.NetworkMode][]bool{
-		// private, bridge, host, container, none, default
-		"":                         {true, false, false, false, false, false},
-		"something:weird":          {true, false, false, false, false, false},
-		"bridge":                   {true, true, false, false, false, false},
-		DefaultDaemonNetworkMode(): {true, true, false, false, false, false},
-		"host":                     {false, false, true, false, false, false},
-		"container:name":           {false, false, false, true, false, false},
-		"none":                     {true, false, false, false, true, false},
-		"default":                  {true, false, false, false, false, true},
-	}
-	networkModeNames := map[container.NetworkMode]string{
-		"":                         "",
-		"something:weird":          "something:weird",
-		"bridge":                   "bridge",
-		DefaultDaemonNetworkMode(): "bridge",
-		"host":                     "host",
-		"container:name":           "container",
-		"none":                     "none",
-		"default":                  "default",
-	}
-	for networkMode, state := range networkModes {
-		if networkMode.IsPrivate() != state[0] {
-			t.Fatalf("NetworkMode.IsPrivate for %v should have been %v but was %v", networkMode, state[0], networkMode.IsPrivate())
-		}
-		if networkMode.IsBridge() != state[1] {
-			t.Fatalf("NetworkMode.IsBridge for %v should have been %v but was %v", networkMode, state[1], networkMode.IsBridge())
-		}
-		if networkMode.IsHost() != state[2] {
-			t.Fatalf("NetworkMode.IsHost for %v should have been %v but was %v", networkMode, state[2], networkMode.IsHost())
-		}
-		if networkMode.IsContainer() != state[3] {
-			t.Fatalf("NetworkMode.IsContainer for %v should have been %v but was %v", networkMode, state[3], networkMode.IsContainer())
-		}
-		if networkMode.IsNone() != state[4] {
-			t.Fatalf("NetworkMode.IsNone for %v should have been %v but was %v", networkMode, state[4], networkMode.IsNone())
-		}
-		if networkMode.IsDefault() != state[5] {
-			t.Fatalf("NetworkMode.IsDefault for %v should have been %v but was %v", networkMode, state[5], networkMode.IsDefault())
-		}
-		if networkMode.NetworkName() != networkModeNames[networkMode] {
-			t.Fatalf("Expected name %v, got %v", networkModeNames[networkMode], networkMode.NetworkName())
-		}
-	}
-}
-
-func TestIpcModeTest(t *testing.T) {
-	ipcModes := map[container.IpcMode]struct {
-		private   bool
-		host      bool
-		container bool
-		shareable bool
-		valid     bool
-		ctrName   string
-	}{
-		"":                      {valid: true},
-		"private":               {private: true, valid: true},
-		"something:weird":       {},
-		":weird":                {},
-		"host":                  {host: true, valid: true},
-		"container":             {},
-		"container:":            {container: true, valid: true, ctrName: ""},
-		"container:name":        {container: true, valid: true, ctrName: "name"},
-		"container:name1:name2": {container: true, valid: true, ctrName: "name1:name2"},
-		"shareable":             {shareable: true, valid: true},
-	}
-
-	for ipcMode, state := range ipcModes {
-		assert.Check(t, is.Equal(state.private, ipcMode.IsPrivate()), "IpcMode.IsPrivate() parsing failed for %q", ipcMode)
-		assert.Check(t, is.Equal(state.host, ipcMode.IsHost()), "IpcMode.IsHost()  parsing failed for %q", ipcMode)
-		assert.Check(t, is.Equal(state.container, ipcMode.IsContainer()), "IpcMode.IsContainer()  parsing failed for %q", ipcMode)
-		assert.Check(t, is.Equal(state.shareable, ipcMode.IsShareable()), "IpcMode.IsShareable()  parsing failed for %q", ipcMode)
-		assert.Check(t, is.Equal(state.valid, ipcMode.Valid()), "IpcMode.Valid()  parsing failed for %q", ipcMode)
-		assert.Check(t, is.Equal(state.ctrName, ipcMode.Container()), "IpcMode.Container() parsing failed for %q", ipcMode)
-	}
-}
-
-func TestUTSModeTest(t *testing.T) {
-	utsModes := map[container.UTSMode][]bool{
-		// private, host, valid
-		"":                {true, false, true},
-		"something:weird": {true, false, false},
-		"host":            {false, true, true},
-		"host:name":       {true, false, true},
-	}
-	for utsMode, state := range utsModes {
-		if utsMode.IsPrivate() != state[0] {
-			t.Fatalf("UtsMode.IsPrivate for %v should have been %v but was %v", utsMode, state[0], utsMode.IsPrivate())
-		}
-		if utsMode.IsHost() != state[1] {
-			t.Fatalf("UtsMode.IsHost for %v should have been %v but was %v", utsMode, state[1], utsMode.IsHost())
-		}
-		if utsMode.Valid() != state[2] {
-			t.Fatalf("UtsMode.Valid for %v should have been %v but was %v", utsMode, state[2], utsMode.Valid())
-		}
-	}
-}
-
-func TestUsernsModeTest(t *testing.T) {
-	usrensMode := map[container.UsernsMode][]bool{
-		// private, host, valid
-		"":                {true, false, true},
-		"something:weird": {true, false, false},
-		"host":            {false, true, true},
-		"host:name":       {true, false, true},
-	}
-	for usernsMode, state := range usrensMode {
-		if usernsMode.IsPrivate() != state[0] {
-			t.Fatalf("UsernsMode.IsPrivate for %v should have been %v but was %v", usernsMode, state[0], usernsMode.IsPrivate())
-		}
-		if usernsMode.IsHost() != state[1] {
-			t.Fatalf("UsernsMode.IsHost for %v should have been %v but was %v", usernsMode, state[1], usernsMode.IsHost())
-		}
-		if usernsMode.Valid() != state[2] {
-			t.Fatalf("UsernsMode.Valid for %v should have been %v but was %v", usernsMode, state[2], usernsMode.Valid())
-		}
-	}
-}
-
-func TestPidModeTest(t *testing.T) {
-	pidModes := map[container.PidMode][]bool{
-		// private, host, valid
-		"":                {true, false, true},
-		"something:weird": {true, false, false},
-		"host":            {false, true, true},
-		"host:name":       {true, false, true},
-	}
-	for pidMode, state := range pidModes {
-		if pidMode.IsPrivate() != state[0] {
-			t.Fatalf("PidMode.IsPrivate for %v should have been %v but was %v", pidMode, state[0], pidMode.IsPrivate())
-		}
-		if pidMode.IsHost() != state[1] {
-			t.Fatalf("PidMode.IsHost for %v should have been %v but was %v", pidMode, state[1], pidMode.IsHost())
-		}
-		if pidMode.Valid() != state[2] {
-			t.Fatalf("PidMode.Valid for %v should have been %v but was %v", pidMode, state[2], pidMode.Valid())
-		}
-	}
-}
-
-func TestRestartPolicy(t *testing.T) {
-	restartPolicies := map[container.RestartPolicy][]bool{
-		// none, always, failure
-		{}: {true, false, false},
-		{Name: "something", MaximumRetryCount: 0}:  {false, false, false},
-		{Name: "no", MaximumRetryCount: 0}:         {true, false, false},
-		{Name: "always", MaximumRetryCount: 0}:     {false, true, false},
-		{Name: "on-failure", MaximumRetryCount: 0}: {false, false, true},
-	}
-	for restartPolicy, state := range restartPolicies {
-		if restartPolicy.IsNone() != state[0] {
-			t.Fatalf("RestartPolicy.IsNone for %v should have been %v but was %v", restartPolicy, state[0], restartPolicy.IsNone())
-		}
-		if restartPolicy.IsAlways() != state[1] {
-			t.Fatalf("RestartPolicy.IsAlways for %v should have been %v but was %v", restartPolicy, state[1], restartPolicy.IsAlways())
-		}
-		if restartPolicy.IsOnFailure() != state[2] {
-			t.Fatalf("RestartPolicy.IsOnFailure for %v should have been %v but was %v", restartPolicy, state[2], restartPolicy.IsOnFailure())
-		}
-	}
-}
 func TestDecodeHostConfig(t *testing.T) {
 	fixtures := []struct {
 		file string