Merge pull request #29742 from miaoyq/rewrite-validate-privileges

Rewrite the function 'validatePrivileges' without checking order

api/types/plugin_responses.go

@@ -3,6 +3,7 @@ package types
 import (
 	"encoding/json"
 	"fmt"
+	"sort"
 )
 
 // PluginsListResponse contains the response for the Engine API
@@ -62,3 +63,17 @@ type PluginPrivilege struct {
 
 // PluginPrivileges is a list of PluginPrivilege
 type PluginPrivileges []PluginPrivilege
+
+func (s PluginPrivileges) Len() int {
+	return len(s)
+}
+
+func (s PluginPrivileges) Less(i, j int) bool {
+	return s[i].Name < s[j].Name
+}
+
+func (s PluginPrivileges) Swap(i, j int) {
+	sort.Strings(s[i].Value)
+	sort.Strings(s[j].Value)
+	s[i], s[j] = s[j], s[i]
+}

plugin/manager.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"regexp"
+	"sort"
 	"strings"
 	"sync"
 
@@ -314,13 +315,38 @@ func attachToLog(id string) func(libcontainerd.IOPipe) error {
 }
 
 func validatePrivileges(requiredPrivileges, privileges types.PluginPrivileges) error {
-	// todo: make a better function that doesn't check order
+	if !isEqual(requiredPrivileges, privileges, isEqualPrivilege) {
-	if !reflect.DeepEqual(privileges, requiredPrivileges) {
 		return errors.New("incorrect privileges")
 	}
+
 	return nil
 }
 
+func isEqual(arrOne, arrOther types.PluginPrivileges, compare func(x, y types.PluginPrivilege) bool) bool {
+	if len(arrOne) != len(arrOther) {
+		return false
+	}
+
+	sort.Sort(arrOne)
+	sort.Sort(arrOther)
+
+	for i := 1; i < arrOne.Len(); i++ {
+		if !compare(arrOne[i], arrOther[i]) {
+			return false
+		}
+	}
+
+	return true
+}
+
+func isEqualPrivilege(a, b types.PluginPrivilege) bool {
+	if a.Name != b.Name {
+		return false
+	}
+
+	return reflect.DeepEqual(a.Value, b.Value)
+}
+
 func configToRootFS(c []byte) (*image.RootFS, error) {
 	var pluginConfig types.PluginConfig
 	if err := json.Unmarshal(c, &pluginConfig); err != nil {

plugin/manager_test.go

@@ -0,0 +1,55 @@
+package plugin
+
+import (
+	"testing"
+
+	"github.com/docker/docker/api/types"
+)
+
+func TestValidatePrivileges(t *testing.T) {
+	testData := map[string]struct {
+		requiredPrivileges types.PluginPrivileges
+		privileges         types.PluginPrivileges
+		result             bool
+	}{
+		"diff-len": {
+			requiredPrivileges: []types.PluginPrivilege{
+				{"Privilege1", "Description", []string{"abc", "def", "ghi"}},
+			},
+			privileges: []types.PluginPrivilege{
+				{"Privilege1", "Description", []string{"abc", "def", "ghi"}},
+				{"Privilege2", "Description", []string{"123", "456", "789"}},
+			},
+			result: false,
+		},
+		"diff-value": {
+			requiredPrivileges: []types.PluginPrivilege{
+				{"Privilege1", "Description", []string{"abc", "def", "GHI"}},
+				{"Privilege2", "Description", []string{"123", "456", "***"}},
+			},
+			privileges: []types.PluginPrivilege{
+				{"Privilege1", "Description", []string{"abc", "def", "ghi"}},
+				{"Privilege2", "Description", []string{"123", "456", "789"}},
+			},
+			result: false,
+		},
+		"diff-order-but-same-value": {
+			requiredPrivileges: []types.PluginPrivilege{
+				{"Privilege1", "Description", []string{"abc", "def", "GHI"}},
+				{"Privilege2", "Description", []string{"123", "456", "789"}},
+			},
+			privileges: []types.PluginPrivilege{
+				{"Privilege2", "Description", []string{"123", "456", "789"}},
+				{"Privilege1", "Description", []string{"GHI", "abc", "def"}},
+			},
+			result: true,
+		},
+	}
+
+	for key, data := range testData {
+		err := validatePrivileges(data.requiredPrivileges, data.privileges)
+		if (err == nil) != data.result {
+			t.Fatalf("Test item %s expected result to be %t, got %t", key, data.result, (err == nil))
+		}
+	}
+}