seccomp: setupSeccomp(): update errors and remove redundant check

thaJeztah · thaJeztah · commit bfd4b64600ba · 2021-07-17T15:57:41.000+02:00
Make the error message slightly more informative, and remove the redundant
`len(config.ArchMap) != 0` check, as iterating over an empty, or 'nil' slice
is a no-op already. This allows to use a slightly more idiomatic "if ok := xx; ok"
condition.

Also move validation to the start of the loop (early return), and explicitly create
a new slice for "names" if the legacy "Name" field is used.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
diff --git a/integration-cli/docker_cli_run_unix_test.go b/integration-cli/docker_cli_run_unix_test.go
@@ -1473,7 +1473,7 @@ func (s *DockerDaemonSuite) TestRunSeccompJSONNoNameAndNames(c *testing.T) {
 
 	out, err := s.d.Cmd("run", "--security-opt", "seccomp="+tmpFile.Name(), "busybox", "chmod", "777", ".")
 	assert.ErrorContains(c, err, "")
-	assert.Assert(c, strings.Contains(out, "'name' and 'names' were specified in the seccomp profile, use either 'name' or 'names'"))
+	assert.Assert(c, strings.Contains(out, "use either 'name' or 'names'"))
 }
 
 func (s *DockerDaemonSuite) TestRunSeccompJSONNoArchAndArchMap(c *testing.T) {
@@ -1510,7 +1510,7 @@ func (s *DockerDaemonSuite) TestRunSeccompJSONNoArchAndArchMap(c *testing.T) {
 
 	out, err := s.d.Cmd("run", "--security-opt", "seccomp="+tmpFile.Name(), "busybox", "chmod", "777", ".")
 	assert.ErrorContains(c, err, "")
-	assert.Assert(c, strings.Contains(out, "'architectures' and 'archMap' were specified in the seccomp profile, use either 'architectures' or 'archMap'"))
+	assert.Assert(c, strings.Contains(out, "use either 'architectures' or 'archMap'"))
 }
 
 func (s *DockerDaemonSuite) TestRunWithDaemonDefaultSeccompProfile(c *testing.T) {
diff --git a/profiles/seccomp/seccomp_linux.go b/profiles/seccomp/seccomp_linux.go
@@ -85,7 +85,7 @@ func setupSeccomp(config *Seccomp, rs *specs.Spec) (*specs.LinuxSeccomp, error)
 	newConfig := &specs.LinuxSeccomp{}
 
 	if len(config.Architectures) != 0 && len(config.ArchMap) != 0 {
-		return nil, errors.New("'architectures' and 'archMap' were specified in the seccomp profile, use either 'architectures' or 'archMap'")
+		return nil, errors.New("both 'architectures' and 'archMap' are specified in the seccomp profile, use either 'architectures' or 'archMap'")
 	}
 
 	// if config.Architectures == 0 then libseccomp will figure out the architecture to use
@@ -94,9 +94,7 @@ func setupSeccomp(config *Seccomp, rs *specs.Spec) (*specs.LinuxSeccomp, error)
 	}
 
 	arch := goToNative[runtime.GOARCH]
-	seccompArch, archExists := nativeToSeccomp[arch]
-
-	if len(config.ArchMap) != 0 && archExists {
+	if seccompArch, ok := nativeToSeccomp[arch]; ok {
 		for _, a := range config.ArchMap {
 			if a.Arch == seccompArch {
 				newConfig.Architectures = append(newConfig.Architectures, a.Arch)
@@ -112,8 +110,14 @@ func setupSeccomp(config *Seccomp, rs *specs.Spec) (*specs.LinuxSeccomp, error)
 	newConfig.ListenerMetadata = config.ListenerMetadata
 
 Loop:
-	// Loop through all syscall blocks and convert them to libcontainer format after filtering them
+	// Convert Syscall to OCI runtimes-spec specs.LinuxSyscall after filtering them.
 	for _, call := range config.Syscalls {
+		if call.Name != "" {
+			if len(call.Names) != 0 {
+				return nil, errors.New("both 'name' and 'names' are specified in the seccomp profile, use either 'name' or 'names'")
+			}
+			call.Names = []string{call.Name}
+		}
 		if call.Excludes != nil {
 			if len(call.Excludes.Arches) > 0 {
 				if inSlice(call.Excludes.Arches, arch) {
@@ -156,14 +160,6 @@ Loop:
 				}
 			}
 		}
-
-		if call.Name != "" {
-			if len(call.Names) != 0 {
-				return nil, errors.New("'name' and 'names' were specified in the seccomp profile, use either 'name' or 'names'")
-			}
-			call.Names = append(call.Names, call.Name)
-		}
-
 		newConfig.Syscalls = append(newConfig.Syscalls, call.LinuxSyscall)
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -1473,7 +1473,7 @@ func (s DockerDaemonSuite) TestRunSeccompJSONNoNameAndNames(c testing.T) {`
`1473`	`1473`
`1474`	`1474`	`out, err := s.d.Cmd("run", "--security-opt", "seccomp="+tmpFile.Name(), "busybox", "chmod", "777", ".")`
`1475`	`1475`	`assert.ErrorContains(c, err, "")`
`1476`		`- assert.Assert(c, strings.Contains(out, "'name' and 'names' were specified in the seccomp profile, use either 'name' or 'names'"))`
	`1476`	`+ assert.Assert(c, strings.Contains(out, "use either 'name' or 'names'"))`
`1477`	`1477`	`}`
`1478`	`1478`
`1479`	`1479`	`func (s DockerDaemonSuite) TestRunSeccompJSONNoArchAndArchMap(c testing.T) {`
`@@ -1510,7 +1510,7 @@ func (s DockerDaemonSuite) TestRunSeccompJSONNoArchAndArchMap(c testing.T) {`
`1510`	`1510`
`1511`	`1511`	`out, err := s.d.Cmd("run", "--security-opt", "seccomp="+tmpFile.Name(), "busybox", "chmod", "777", ".")`
`1512`	`1512`	`assert.ErrorContains(c, err, "")`
`1513`		`- assert.Assert(c, strings.Contains(out, "'architectures' and 'archMap' were specified in the seccomp profile, use either 'architectures' or 'archMap'"))`
	`1513`	`+ assert.Assert(c, strings.Contains(out, "use either 'architectures' or 'archMap'"))`
`1514`	`1514`	`}`
`1515`	`1515`
`1516`	`1516`	`func (s DockerDaemonSuite) TestRunWithDaemonDefaultSeccompProfile(c testing.T) {`