Fix subject match selector issue in cli (#3887) (#3892)

Signed-off-by: Vyankatesh vyankateshkd@gmail.com

cmd/cli/kubectl-kyverno/apply/apply_command.go

@@ -247,12 +247,14 @@ func applyCommandHelper(resourcePaths []string, userInfoPath string, cluster boo
 
 	// get the user info as request info from a different file
 	var userInfo v1beta1.RequestInfo
+	var subjectInfo store.Subject
 	if userInfoPath != "" {
-		userInfo, err = common.GetUserInfoFromPath(fs, userInfoPath, false, "")
+		userInfo, subjectInfo, err = common.GetUserInfoFromPath(fs, userInfoPath, false, "")
 		if err != nil {
 			fmt.Printf("Error: failed to load request info\nCause: %s\n", err)
 			os.Exit(1)
 		}
+		store.SetSubjects(subjectInfo)
 	}
 
 	if variablesString != "" {

cmd/cli/kubectl-kyverno/test/test_command.go

@@ -752,12 +752,15 @@ func applyPoliciesFromPath(fs billy.Filesystem, policyBytes []byte, isGit bool,
 
 	// get the user info as request info from a different file
 	var userInfo v1beta1.RequestInfo
+	var subjectInfo store.Subject
+
 	if userInfoFile != "" {
-		userInfo, err = common.GetUserInfoFromPath(fs, userInfoFile, isGit, policyResourcePath)
+		userInfo, subjectInfo, err = common.GetUserInfoFromPath(fs, userInfoFile, isGit, policyResourcePath)
 		if err != nil {
 			fmt.Printf("Error: failed to load request info\nCause: %s\n", err)
 			os.Exit(1)
 		}
+		store.SetSubjects(subjectInfo)
 	}
 
 	policyFullPath := getFullPath(values.Policies, policyResourcePath, isGit)

cmd/cli/kubectl-kyverno/utils/common/common.go

@@ -1011,9 +1011,9 @@ func GetPatchedResourceFromPath(fs billy.Filesystem, path string, isGit bool, po
 }
 
 //GetUserInfoFromPath - get the request info as user info from a given path
-func GetUserInfoFromPath(fs billy.Filesystem, path string, isGit bool, policyResourcePath string) (v1beta1.RequestInfo, error) {
+func GetUserInfoFromPath(fs billy.Filesystem, path string, isGit bool, policyResourcePath string) (v1beta1.RequestInfo, store.Subject, error) {
 	userInfo := &v1beta1.RequestInfo{}
-
+	subjectInfo := &store.Subject{}
 	if isGit {
 		filep, err := fs.Open(filepath.Join(policyResourcePath, path))
 		if err != nil {
@@ -1031,6 +1031,14 @@ func GetUserInfoFromPath(fs billy.Filesystem, path string, isGit bool, policyRes
 		if err := json.Unmarshal(userInfoBytes, userInfo); err != nil {
 			fmt.Printf("failed to decode yaml: %v", err)
 		}
+		subjectBytes, err := yaml.ToJSON(bytes)
+		if err != nil {
+			fmt.Printf("failed to convert to JSON: %v", err)
+		}
+
+		if err := json.Unmarshal(subjectBytes, subjectInfo); err != nil {
+			fmt.Printf("failed to decode yaml: %v", err)
+		}
 	} else {
 		var errors []error
 		bytes, err := ioutil.ReadFile(filepath.Join(policyResourcePath, path))
@@ -1041,12 +1049,14 @@ func GetUserInfoFromPath(fs billy.Filesystem, path string, isGit bool, policyRes
 		if err != nil {
 			errors = append(errors, sanitizederror.NewWithError("failed to convert json", err))
 		}
-
 		if err := json.Unmarshal(userInfoBytes, userInfo); err != nil {
 			errors = append(errors, sanitizederror.NewWithError("failed to decode yaml", err))
 		}
+		if err := json.Unmarshal(userInfoBytes, subjectInfo); err != nil {
+			errors = append(errors, sanitizederror.NewWithError("failed to decode yaml", err))
+		}
 		if len(errors) > 0 {
-			return *userInfo, sanitizederror.NewWithErrors("failed to read file", errors)
+			return *userInfo, *subjectInfo, sanitizederror.NewWithErrors("failed to read file", errors)
 		}
 
 		if len(errors) > 0 && log.Log.V(1).Enabled() {
@@ -1056,5 +1066,5 @@ func GetUserInfoFromPath(fs billy.Filesystem, path string, isGit bool, policyRes
 			}
 		}
 	}
-	return *userInfo, nil
+	return *userInfo, *subjectInfo, nil
 }

cmd/cli/kubectl-kyverno/utils/store/store.go

@@ -1,10 +1,14 @@
 package store
 
-import "github.com/kyverno/kyverno/pkg/registryclient"
+import (
+	"github.com/kyverno/kyverno/pkg/registryclient"
+	rbacv1 "k8s.io/api/rbac/v1"
+)
 
 var Mock, RegistryAccess bool
 var ContextVar Context
 var ForeachElement int
+var Subjects Subject
 
 func SetMock(mock bool) {
 	Mock = mock
@@ -77,3 +81,15 @@ type Rule struct {
 	Values        map[string]interface{}   `json:"values"`
 	ForeachValues map[string][]interface{} `json:"foreachValues"`
 }
+
+func SetSubjects(subjects Subject) {
+	Subjects = subjects
+}
+
+func GetSubjects() Subject {
+	return Subjects
+}
+
+type Subject struct {
+	Subject rbacv1.Subject `json:"subject,omitempty" yaml:"subject,omitempty"`
+}

pkg/engine/utils.go

@@ -12,6 +12,7 @@ import (
 	wildcard "github.com/kyverno/go-wildcard"
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
 	urkyverno "github.com/kyverno/kyverno/api/kyverno/v1beta1"
+	"github.com/kyverno/kyverno/cmd/cli/kubectl-kyverno/utils/store"
 	"github.com/kyverno/kyverno/pkg/engine/common"
 	"github.com/kyverno/kyverno/pkg/engine/context"
 	"github.com/kyverno/kyverno/pkg/engine/response"
@@ -240,33 +241,50 @@ func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription,
 func matchSubjects(ruleSubjects []rbacv1.Subject, userInfo authenticationv1.UserInfo, dynamicConfig []string) bool {
 	const SaPrefix = "system:serviceaccount:"
 
-	userGroups := append(userInfo.Groups, userInfo.Username)
-
-	// TODO: see issue https://github.com/kyverno/kyverno/issues/861
-	for _, e := range dynamicConfig {
-		ruleSubjects = append(ruleSubjects,
-			rbacv1.Subject{Kind: "Group", Name: e},
-		)
-	}
-
-	for _, subject := range ruleSubjects {
-		switch subject.Kind {
-		case "ServiceAccount":
-			if len(userInfo.Username) <= len(SaPrefix) {
-				continue
-			}
-			subjectServiceAccount := subject.Namespace + ":" + subject.Name
-			if userInfo.Username[len(SaPrefix):] == subjectServiceAccount {
-				return true
-			}
-		case "User", "Group":
-			if utils.ContainsString(userGroups, subject.Name) {
-				return true
+	if store.GetMock() {
+		mockSubject := store.GetSubjects().Subject
+		for _, subject := range ruleSubjects {
+			switch subject.Kind {
+			case "ServiceAccount":
+				if subject.Name == mockSubject.Name && subject.Namespace == mockSubject.Namespace {
+					return true
+				}
+			case "User", "Group":
+				if mockSubject.Name == subject.Name {
+					return true
+				}
 			}
 		}
-	}
 
-	return false
+		return false
+	} else {
+		userGroups := append(userInfo.Groups, userInfo.Username)
+		// TODO: see issue https://github.com/kyverno/kyverno/issues/861
+		for _, e := range dynamicConfig {
+			ruleSubjects = append(ruleSubjects,
+				rbacv1.Subject{Kind: "Group", Name: e},
+			)
+		}
+
+		for _, subject := range ruleSubjects {
+			switch subject.Kind {
+			case "ServiceAccount":
+				if len(userInfo.Username) <= len(SaPrefix) {
+					continue
+				}
+				subjectServiceAccount := subject.Namespace + ":" + subject.Name
+				if userInfo.Username[len(SaPrefix):] == subjectServiceAccount {
+					return true
+				}
+			case "User", "Group":
+				if utils.ContainsString(userGroups, subject.Name) {
+					return true
+				}
+			}
+		}
+
+		return false
+	}
 }
 
 //MatchesResourceDescription checks if the resource matches resource description of the rule or not

test/cli/test/limit-configmap-for-sa/kyverno-test.yaml

@@ -4,6 +4,7 @@ policies:
 resources:
   - resource.yaml
 variables: variables.yaml
+userinfo: user_info.yaml
 results:
   - policy: limit-configmap-for-sa
     rule: limit-configmap-for-sa-developer

test/cli/test/limit-configmap-for-sa/limit_configmap_for_sa.yaml

@@ -12,17 +12,21 @@ metadata:
     policies.kyverno.io/description: This policy shows how to restrict certain operations on specific ConfigMaps by ServiceAccounts.
 spec:
   background: false
-  validationFailureAction: enforce
+  validationFailureAction: audit
   rules:
   - name: limit-configmap-for-sa-developer
     match:
       any:
       - resources:
           kinds:
-          - "ConfigMap"
+          - ConfigMap
+        subjects:
+        - kind: ServiceAccount
+          name: developer
+          namespace: kube-system
       - resources:
           kinds:
-          - "ConfigMap"
+          - ConfigMap
         subjects:
         - kind: ServiceAccount
           name: another-developer

test/cli/test/limit-configmap-for-sa/resource.yaml

@@ -5,7 +5,6 @@ metadata:
   namespace: any-namespace
 data:
   key: value
-
 ---
 apiVersion: v1
 kind: ConfigMap

test/cli/test/limit-configmap-for-sa/user_info.yaml

@@ -0,0 +1,4 @@
+subject:
+  kind: ServiceAccount
+  name: another-developer
+  namespace: another-namespace