Merge pull request #50825 from austinvazquez/move-decode-security-opts-from-types-to-pkg

api/types/system: move `SecurityOpt` and `DecodeSecurityOptions` to client mod

client/pkg/security/security_opts.go

@@ -1,4 +1,4 @@
-package system
+package security
 
 import (
 	"errors"
@@ -6,23 +6,28 @@ import (
 	"strings"
 )
 
-// SecurityOpt contains the name and options of a security option
-type SecurityOpt struct {
+// Option contains the name and options of a security option
+type Option struct {
 	Name    string
 	Options []KeyValue
 }
 
-// DecodeSecurityOptions decodes a security options string slice to a
-// type-safe [SecurityOpt].
-func DecodeSecurityOptions(opts []string) ([]SecurityOpt, error) {
-	so := []SecurityOpt{}
+// KeyValue holds a key/value pair.
+type KeyValue struct {
+	Key, Value string
+}
+
+// DecodeOptions decodes a security options string slice to a
+// type-safe [Option].
+func DecodeOptions(opts []string) ([]Option, error) {
+	so := []Option{}
 	for _, opt := range opts {
 		// support output from a < 1.13 docker daemon
 		if !strings.Contains(opt, "=") {
-			so = append(so, SecurityOpt{Name: opt})
+			so = append(so, Option{Name: opt})
 			continue
 		}
-		secopt := SecurityOpt{}
+		secopt := Option{}
 		for _, s := range strings.Split(opt, ",") {
 			k, v, ok := strings.Cut(s, "=")
 			if !ok {
@@ -41,8 +46,3 @@ func DecodeSecurityOptions(opts []string) ([]SecurityOpt, error) {
 	}
 	return so, nil
 }
-
-// KeyValue holds a key/value pair.
-type KeyValue struct {
-	Key, Value string
-}

client/pkg/security/security_opts_test.go

@@ -0,0 +1,240 @@
+package security
+
+import (
+	"fmt"
+	"testing"
+
+	"gotest.tools/v3/assert"
+	"gotest.tools/v3/assert/cmp"
+)
+
+func TestDecode(t *testing.T) {
+	tests := []struct {
+		name    string
+		opts    []string
+		want    []Option
+		wantErr string
+	}{
+		{
+			name: "empty options",
+			opts: []string{},
+			want: []Option{},
+		},
+		{
+			name: "nil options",
+			opts: nil,
+			want: []Option{},
+		},
+		{
+			name: "legacy format without equals",
+			opts: []string{"apparmor:unconfined"},
+			want: []Option{
+				{Name: "apparmor:unconfined"},
+			},
+		},
+		{
+			name: "single option with name only",
+			opts: []string{"name=apparmor"},
+			want: []Option{
+				{Name: "apparmor"},
+			},
+		},
+		{
+			name: "single option with name and additional options",
+			opts: []string{"name=selinux,type=container_t,level=s0:c1.c2"},
+			want: []Option{
+				{
+					Name: "selinux",
+					Options: []KeyValue{
+						{Key: "type", Value: "container_t"},
+						{Key: "level", Value: "s0:c1.c2"},
+					},
+				},
+			},
+		},
+		{
+			name: "multiple options",
+			opts: []string{
+				"name=apparmor,profile=docker-default",
+				"name=seccomp,profile=unconfined",
+			},
+			want: []Option{
+				{
+					Name: "apparmor",
+					Options: []KeyValue{
+						{Key: "profile", Value: "docker-default"},
+					},
+				},
+				{
+					Name: "seccomp",
+					Options: []KeyValue{
+						{Key: "profile", Value: "unconfined"},
+					},
+				},
+			},
+		},
+		{
+			name: "mixed legacy and new format",
+			opts: []string{
+				"label:disable",
+				"name=apparmor,profile=custom",
+			},
+			want: []Option{
+				{Name: "label:disable"},
+				{
+					Name: "apparmor",
+					Options: []KeyValue{
+						{Key: "profile", Value: "custom"},
+					},
+				},
+			},
+		},
+		{
+			name: "option without name key",
+			opts: []string{"profile=custom,type=container_t"},
+			want: []Option{
+				{
+					Options: []KeyValue{
+						{Key: "profile", Value: "custom"},
+						{Key: "type", Value: "container_t"},
+					},
+				},
+			},
+		},
+		{
+			name: "option with equals in value",
+			opts: []string{"name=selinux,level=s0:c1=c2"},
+			want: []Option{
+				{
+					Name: "selinux",
+					Options: []KeyValue{
+						{Key: "level", Value: "s0:c1=c2"},
+					},
+				},
+			},
+		},
+		{
+			name:    "invalid option without equals in comma-separated list",
+			opts:    []string{"name=apparmor,invalid"},
+			wantErr: `invalid security option "invalid"`,
+		},
+		{
+			name:    "empty key",
+			opts:    []string{"=value"},
+			wantErr: "invalid empty security option",
+		},
+		{
+			name:    "empty value",
+			opts:    []string{"key="},
+			wantErr: "invalid empty security option",
+		},
+		{
+			name:    "empty key and value",
+			opts:    []string{"="},
+			wantErr: "invalid empty security option",
+		},
+		{
+			name:    "empty key in middle",
+			opts:    []string{"name=apparmor,=value"},
+			wantErr: "invalid empty security option",
+		},
+		{
+			name:    "empty value in middle",
+			opts:    []string{"name=apparmor,key="},
+			wantErr: "invalid empty security option",
+		},
+		{
+			name: "complex real-world example",
+			opts: []string{
+				"name=selinux,user=system_u,role=system_r,type=container_t,level=s0:c1.c2",
+				"name=apparmor,profile=/usr/bin/docker",
+				"name=seccomp,profile=builtin",
+			},
+			want: []Option{
+				{
+					Name: "selinux",
+					Options: []KeyValue{
+						{Key: "user", Value: "system_u"},
+						{Key: "role", Value: "system_r"},
+						{Key: "type", Value: "container_t"},
+						{Key: "level", Value: "s0:c1.c2"},
+					},
+				},
+				{
+					Name: "apparmor",
+					Options: []KeyValue{
+						{Key: "profile", Value: "/usr/bin/docker"},
+					},
+				},
+				{
+					Name: "seccomp",
+					Options: []KeyValue{
+						{Key: "profile", Value: "builtin"},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := DecodeOptions(tc.opts)
+
+			if tc.wantErr == "" {
+				assert.NilError(t, err)
+				assert.Check(t, cmp.DeepEqual(got, tc.want))
+			} else {
+				assert.Check(t, err != nil, "expected error but got none")
+				assert.ErrorContains(t, err, tc.wantErr)
+			}
+		})
+	}
+}
+
+func BenchmarkDecode(b *testing.B) {
+	opts := []string{
+		"name=selinux,user=system_u,role=system_r,type=container_t,level=s0:c1.c2",
+		"name=apparmor,profile=/usr/bin/docker",
+		"name=seccomp,profile=builtin",
+		"legacy:format",
+	}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, err := DecodeOptions(opts)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkDecodeLegacy(b *testing.B) {
+	opts := []string{
+		"apparmor:unconfined",
+		"label:disable",
+		"seccomp:unconfined",
+	}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, err := DecodeOptions(opts)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkDecodeComplex(b *testing.B) {
+	opts := make([]string, 100)
+	for i := range opts {
+		opts[i] = fmt.Sprintf("name=test%d,key1=value1,key2=value2,key3=value3", i)
+	}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, err := DecodeOptions(opts)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}

daemon/server/router/system/system_routes.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/containerd/log"
@@ -20,6 +21,7 @@ import (
 	"github.com/moby/moby/v2/daemon/server/backend"
 	"github.com/moby/moby/v2/daemon/server/httputils"
 	"github.com/moby/moby/v2/daemon/server/router/build"
+	"github.com/moby/moby/v2/daemon/server/systembackend"
 	"github.com/moby/moby/v2/pkg/ioutils"
 	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
@@ -74,7 +76,7 @@ func (s *systemRouter) getInfo(ctx context.Context, w http.ResponseWriter, r *ht
 
 		if versions.LessThan(version, "1.25") {
 			// TODO: handle this conversion in engine-api
-			kvSecOpts, err := system.DecodeSecurityOptions(info.SecurityOptions)
+			kvSecOpts, err := decodeSecurityOptions(info.SecurityOptions)
 			if err != nil {
 				info.Warnings = append(info.Warnings, err.Error())
 			}
@@ -142,6 +144,36 @@ func (s *systemRouter) getInfo(ctx context.Context, w http.ResponseWriter, r *ht
 	return httputils.WriteJSON(w, http.StatusOK, info)
 }
 
+// decodeSecurityOptions decodes a security options string slice to a
+// type-safe [systembackend.SecurityOption].
+func decodeSecurityOptions(opts []string) ([]systembackend.SecurityOption, error) {
+	so := []systembackend.SecurityOption{}
+	for _, opt := range opts {
+		// support output from a < 1.13 docker daemon
+		if !strings.Contains(opt, "=") {
+			so = append(so, systembackend.SecurityOption{Name: opt})
+			continue
+		}
+		secopt := systembackend.SecurityOption{}
+		for _, s := range strings.Split(opt, ",") {
+			k, v, ok := strings.Cut(s, "=")
+			if !ok {
+				return nil, fmt.Errorf("invalid security option %q", s)
+			}
+			if k == "" || v == "" {
+				return nil, errors.New("invalid empty security option")
+			}
+			if k == "name" {
+				secopt.Name = v
+				continue
+			}
+			secopt.Options = append(secopt.Options, systembackend.KeyValue{Key: k, Value: v})
+		}
+		so = append(so, secopt)
+	}
+	return so, nil
+}
+
 func (s *systemRouter) getVersion(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	info, err := s.backend.SystemVersion(ctx)
 	if err != nil {

daemon/server/systembackend/security_opts.go

@@ -0,0 +1,12 @@
+package systembackend
+
+// SecurityOption contains the name and options of a security option
+type SecurityOption struct {
+	Name    string
+	Options []KeyValue
+}
+
+// KeyValue holds a key/value pair.
+type KeyValue struct {
+	Key, Value string
+}

vendor/github.com/moby/moby/api/types/system/security_opts.go

@@ -1,48 +0,0 @@
-package system
-
-import (
-	"errors"
-	"fmt"
-	"strings"
-)
-
-// SecurityOpt contains the name and options of a security option
-type SecurityOpt struct {
-	Name    string
-	Options []KeyValue
-}
-
-// DecodeSecurityOptions decodes a security options string slice to a
-// type-safe [SecurityOpt].
-func DecodeSecurityOptions(opts []string) ([]SecurityOpt, error) {
-	so := []SecurityOpt{}
-	for _, opt := range opts {
-		// support output from a < 1.13 docker daemon
-		if !strings.Contains(opt, "=") {
-			so = append(so, SecurityOpt{Name: opt})
-			continue
-		}
-		secopt := SecurityOpt{}
-		for _, s := range strings.Split(opt, ",") {
-			k, v, ok := strings.Cut(s, "=")
-			if !ok {
-				return nil, fmt.Errorf("invalid security option %q", s)
-			}
-			if k == "" || v == "" {
-				return nil, errors.New("invalid empty security option")
-			}
-			if k == "name" {
-				secopt.Name = v
-				continue
-			}
-			secopt.Options = append(secopt.Options, KeyValue{Key: k, Value: v})
-		}
-		so = append(so, secopt)
-	}
-	return so, nil
-}
-
-// KeyValue holds a key/value pair.
-type KeyValue struct {
-	Key, Value string
-}