add unit test

pkg/engine/rbacValidation.go

@@ -10,7 +10,7 @@ import (
 )
 
 const (
-	SaPrefix = "system:serviceaccounts:"
+	SaPrefix = "system:serviceaccount:"
 )
 
 // matchAdmissionInfo return true if the rule can be applied to the request

pkg/webhooks/rbac.go

@@ -57,20 +57,20 @@ func getRoleRefByRoleBindings(roleBindings *unstructured.UnstructuredList, userI
 			return nil, nil, fmt.Errorf("%s/%s/%s has no subjects field", rolebinding.GetKind(), rolebinding.GetNamespace(), rolebinding.GetName())
 		}
 
+		roleRef, ok := rb["roleRef"]
+		if !ok {
+			return nil, nil, fmt.Errorf("%s/%s/%s has no roleRef", rolebinding.GetKind(), rolebinding.GetNamespace(), rolebinding.GetName())
+		}
+
 		for _, subject := range subjects.([]map[string]interface{}) {
 			if !matchSubjectsMap(subject, userInfo) {
 				continue
 			}
 
-			roleRef, ok := subject["roleRef"]
-			if !ok {
-				return nil, nil, fmt.Errorf("%s/%s/%s has no roleRef", rolebinding.GetKind(), rolebinding.GetNamespace(), rolebinding.GetName())
-			}
-
 			roleRefMap := roleRef.(map[string]interface{})
 			switch roleRefMap["kind"] {
 			case "role":
-				roles = append(roles, rolebinding.GetNamespace()+":"+roleRefMap["name"].(string))
+				roles = append(roles, roleRefMap["namespace"].(string)+":"+roleRefMap["name"].(string))
 			case "clusterRole":
 				clusterRoles = append(clusterRoles, roleRefMap["name"].(string))
 			}
@@ -89,16 +89,16 @@ func getRoleRefByClusterRoleBindings(clusterroleBindings *unstructured.Unstructu
 			return nil, fmt.Errorf("%s/%s has no subjects field", clusterRoleBinding.GetKind(), clusterRoleBinding.GetName())
 		}
 
+		roleRef, ok := crb["roleRef"]
+		if !ok {
+			return nil, fmt.Errorf("%s/%s has no roleRef", clusterRoleBinding.GetKind(), clusterRoleBinding.GetName())
+		}
+
 		for _, subject := range subjects.([]map[string]interface{}) {
 			if !matchSubjectsMap(subject, userInfo) {
 				continue
 			}
 
-			roleRef, ok := subject["roleRef"]
-			if !ok {
-				return nil, fmt.Errorf("%s/%s has no roleRef", clusterRoleBinding.GetKind(), clusterRoleBinding.GetName())
-			}
-
 			roleRefMap := roleRef.(map[string]interface{})
 			if roleRefMap["kind"] == "clusterRole" {
 				clusterRoles = append(clusterRoles, roleRefMap["name"].(string))
@@ -114,17 +114,11 @@ func getRoleRefByClusterRoleBindings(clusterroleBindings *unstructured.Unstructu
 func matchSubjectsMap(subject map[string]interface{}, userInfo authenticationv1.UserInfo) bool {
 	// ServiceAccount
 	if isServiceaccountUserInfo(userInfo.Username) {
-		if matchServiceAccount(subject, userInfo) {
-			return true
-		}
+		return matchServiceAccount(subject, userInfo)
 	}
 
 	// User or Group
-	if matchUserOrGroup(subject, userInfo) {
-		return true
-	}
-
-	return false
+	return matchUserOrGroup(subject, userInfo)
 }
 
 func isServiceaccountUserInfo(username string) bool {
@@ -138,13 +132,31 @@ func isServiceaccountUserInfo(username string) bool {
 // serviceaccount represents as saPrefix:namespace:name in userInfo
 func matchServiceAccount(subject map[string]interface{}, userInfo authenticationv1.UserInfo) bool {
 	// checks if subject contains the serviceaccount info
-	sa := subject["kind"]
-	if sa.(string) != "ServiceAccount" {
-		glog.V(3).Infof("subject %v has no service account info", subject)
+	sa, ok := subject["kind"].(string)
+	if !ok {
+		glog.V(3).Infof("subject %v has wrong kind field", subject)
 		return false
 	}
 
-	subjectServiceAccount := subject["namespace"].(string) + ":" + subject["name"].(string)
+	if sa != "ServiceAccount" {
+		glog.V(3).Infof("subject %v has no ServiceAccount info", subject)
+		return false
+	}
+
+	namespace, ok := subject["namespace"].(string)
+	if !ok {
+		glog.V(3).Infof("subject %v has wrong namespace field", subject)
+		return false
+	}
+
+	_ = subject["name"]
+	name, ok := subject["name"].(string)
+	if !ok {
+		glog.V(3).Infof("subject %v has wrong name field", subject)
+		return false
+	}
+
+	subjectServiceAccount := namespace + ":" + name
 	if userInfo.Username[len(engine.SaPrefix):] != subjectServiceAccount {
 		glog.V(3).Infof("service account not match, expect %s, got %s", subjectServiceAccount, userInfo.Username[len(engine.SaPrefix):])
 		return false

pkg/webhooks/rbac_test.go

@@ -0,0 +1,280 @@
+package webhooks
+
+import (
+	"flag"
+	"reflect"
+	"testing"
+
+	"gotest.tools/assert"
+	authenticationv1 "k8s.io/api/authentication/v1"
+	unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func Test_isServiceaccountUserInfo(t *testing.T) {
+	tests := []struct {
+		username string
+		expected bool
+	}{
+		{
+			username: "system:serviceaccount:default:saconfig",
+			expected: true,
+		},
+		{
+			username: "serviceaccount:default:saconfig",
+			expected: false,
+		},
+	}
+
+	for _, test := range tests {
+		res := isServiceaccountUserInfo(test.username)
+		assert.Assert(t, test.expected == res)
+	}
+}
+
+func Test_matchServiceAccount_subject_variants(t *testing.T) {
+	userInfo := authenticationv1.UserInfo{
+		Username: "system:serviceaccount:default:saconfig",
+	}
+
+	tests := []struct {
+		subject  map[string]interface{}
+		expected bool
+	}{
+		{
+			subject:  make(map[string]interface{}, 1),
+			expected: false,
+		},
+		{
+			subject: map[string]interface{}{
+				"kind": "serviceaccount",
+			},
+			expected: false,
+		},
+		{
+			subject: map[string]interface{}{
+				"kind":      "ServiceAccount",
+				"Namespace": "testnamespace",
+			},
+			expected: false,
+		},
+		{
+			subject: map[string]interface{}{
+				"kind":      "ServiceAccount",
+				"namespace": 1,
+			},
+			expected: false,
+		},
+		{
+			subject: map[string]interface{}{
+				"kind":      "ServiceAccount",
+				"namespace": "testnamespace",
+				"names":     "",
+			},
+			expected: false,
+		},
+		{
+			subject: map[string]interface{}{
+				"kind":      "ServiceAccount",
+				"namespace": "testnamespace",
+				"name":      "testname",
+			},
+			expected: false,
+		},
+	}
+
+	for _, test := range tests {
+		res := matchServiceAccount(test.subject, userInfo)
+		assert.Assert(t, test.expected == res)
+	}
+}
+
+func Test_matchUserOrGroup(t *testing.T) {
+	group := authenticationv1.UserInfo{
+		Username: "kubernetes-admin",
+		Groups:   []string{"system:masters", "system:authenticated"},
+	}
+
+	sa := authenticationv1.UserInfo{
+		Username: "system:serviceaccount:kube-system:deployment-controller",
+		Groups:   []string{"system:serviceaccounts", "system:serviceaccounts:kube-system", "system:authenticated"},
+	}
+
+	user := authenticationv1.UserInfo{
+		Username: "system:kube-scheduler",
+		Groups:   []string{"system:authenticated"},
+	}
+
+	userContext := map[string]interface{}{
+		"kind": "User",
+		"name": "system:kube-scheduler",
+	}
+
+	groupContext := map[string]interface{}{
+		"kind": "Group",
+		"name": "system:masters",
+	}
+
+	fakegroupContext := map[string]interface{}{
+		"kind": "Group",
+		"name": "fakeGroup",
+	}
+
+	res := matchUserOrGroup(userContext, user)
+	assert.Assert(t, res)
+
+	res = matchUserOrGroup(groupContext, group)
+	assert.Assert(t, res)
+
+	res = matchUserOrGroup(groupContext, sa)
+	assert.Assert(t, !res)
+
+	res = matchUserOrGroup(fakegroupContext, group)
+	assert.Assert(t, !res)
+}
+
+func Test_matchSubjectsMap(t *testing.T) {
+	sa := authenticationv1.UserInfo{
+		Username: "system:serviceaccount:default:saconfig",
+	}
+
+	group := authenticationv1.UserInfo{
+		Username: "kubernetes-admin",
+		Groups:   []string{"system:masters", "system:authenticated"},
+	}
+
+	sasubject := map[string]interface{}{
+		"kind":      "ServiceAccount",
+		"namespace": "default",
+		"name":      "saconfig",
+	}
+
+	groupsubject := map[string]interface{}{
+		"kind": "Group",
+		"name": "fakeGroup",
+	}
+
+	res := matchSubjectsMap(sasubject, sa)
+	assert.Assert(t, res)
+
+	res = matchSubjectsMap(groupsubject, group)
+	assert.Assert(t, !res)
+}
+
+func Test_getRoleRefByRoleBindings(t *testing.T) {
+	flag.Parse()
+	flag.Set("logtostderr", "true")
+	flag.Set("v", "3")
+	list := &unstructured.UnstructuredList{
+		Object: map[string]interface{}{"kind": "List", "apiVersion": "v1"},
+		Items: []unstructured.Unstructured{
+			{
+				Object: map[string]interface{}{
+					"kind":       "RoleBinding",
+					"apiVersion": "rbac.authorization.k8s.io/v1",
+					"metadata":   map[string]interface{}{"name": "test1"},
+					"roleRef": map[string]interface{}{
+						"kind":      "role",
+						"name":      "myrole",
+						"namespace": "mynamespace",
+					},
+					"subjects": []map[string]interface{}{
+						{
+							"kind":      "ServiceAccount",
+							"name":      "saconfig",
+							"namespace": "default",
+						},
+					},
+				},
+			},
+			{
+				Object: map[string]interface{}{
+					"kind":       "RoleBinding",
+					"apiVersion": "rbac.authorization.k8s.io/v1",
+					"metadata":   map[string]interface{}{"name": "test2"},
+					"roleRef": map[string]interface{}{
+						"kind": "clusterRole",
+						"name": "myclusterrole",
+					},
+					"subjects": []map[string]interface{}{
+						{
+							"kind":      "ServiceAccount",
+							"name":      "saconfig",
+							"namespace": "default",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	sa := authenticationv1.UserInfo{
+		Username: "system:serviceaccount:default:saconfig",
+	}
+
+	expectedrole := []string{"mynamespace:myrole"}
+	expectedClusterRole := []string{"myclusterrole"}
+	roles, clusterroles, err := getRoleRefByRoleBindings(list, sa)
+	assert.Assert(t, err == nil)
+	assert.Assert(t, reflect.DeepEqual(roles, expectedrole))
+	assert.Assert(t, reflect.DeepEqual(clusterroles, expectedClusterRole))
+}
+
+func Test_getRoleRefByClusterRoleBindings(t *testing.T) {
+	list := &unstructured.UnstructuredList{
+		Object: map[string]interface{}{"kind": "List", "apiVersion": "v1"},
+		Items: []unstructured.Unstructured{
+			{
+				Object: map[string]interface{}{
+					"kind":       "ClusterRoleBinding",
+					"apiVersion": "rbac.authorization.k8s.io/v1",
+					"metadata":   map[string]interface{}{"name": "test-1"},
+					"roleRef": map[string]interface{}{
+						"kind": "clusterRole",
+						"name": "fakeclusterrole",
+					},
+					"subjects": []map[string]interface{}{
+						{
+							"kind": "User",
+							"name": "kube-scheduler",
+						},
+					},
+				},
+			},
+			{
+				Object: map[string]interface{}{
+					"kind":       "ClusterRoleBinding",
+					"apiVersion": "rbac.authorization.k8s.io/v1",
+					"metadata":   map[string]interface{}{"name": "test-2"},
+					"roleRef": map[string]interface{}{
+						"kind": "clusterRole",
+						"name": "myclusterrole",
+					},
+					"subjects": []map[string]interface{}{
+						{
+							"kind": "Group",
+							"name": "system:masters",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	group := authenticationv1.UserInfo{
+		Username: "kubernetes-admin",
+		Groups:   []string{"system:masters", "system:authenticated"},
+	}
+
+	user := authenticationv1.UserInfo{
+		Username: "system:kube-scheduler",
+		Groups:   []string{"system:authenticated"},
+	}
+
+	clusterroles, err := getRoleRefByClusterRoleBindings(list, group)
+	assert.Assert(t, err == nil)
+	assert.Assert(t, reflect.DeepEqual(clusterroles, []string{"myclusterrole"}))
+
+	clusterroles, err = getRoleRefByClusterRoleBindings(list, user)
+	assert.Assert(t, err == nil)
+	assert.Assert(t, len(clusterroles) == 0)
+}