From 34d05c58c2f8662a06e99632c8cf71532db94325 Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Tue, 19 May 2020 13:04:06 -0700 Subject: [PATCH] PR fixes --- pkg/engine/mutation.go | 2 +- pkg/engine/utils.go | 24 ++++-------------------- pkg/engine/validation.go | 2 +- pkg/userinfo/roleRef.go | 7 ++++--- pkg/utils/util.go | 16 ++++++++++++++++ 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 5e66b4d0e3..0127e15b72 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -46,7 +46,7 @@ func Mutate(policyContext PolicyContext) (resp response.EngineResponse) { // check if the resource satisfies the filter conditions defined in the rule //TODO: this needs to be extracted, to filter the resource so that we can avoid passing resources that - // dont statisfy a policy rule resource description + // dont satisfy a policy rule resource description if err := MatchesResourceDescription(resource, rule, policyContext.AdmissionInfo); err != nil { logger.V(4).Info("resource fails the match description", "reason", err.Error()) continue diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 4a7cc23257..3824cd0b19 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -96,14 +96,14 @@ func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription, keys := append(admissionInfo.AdmissionUserInfo.Groups, admissionInfo.AdmissionUserInfo.Username) if len(userInfo.Roles) > 0 && - !DoesSliceContainsAnyOfTheseValues(keys, ExcludeUserInfo...) { - if !DoesSliceContainsAnyOfTheseValues(userInfo.Roles, admissionInfo.Roles...) { + !utils.SliceContains(keys, ExcludeUserInfo...) { + if !utils.SliceContains(userInfo.Roles, admissionInfo.Roles...) { errs = append(errs, fmt.Errorf("user info does not match roles for the given conditionBlock")) } } if len(userInfo.ClusterRoles) > 0 && - !DoesSliceContainsAnyOfTheseValues(keys, ExcludeUserInfo...) { - if !DoesSliceContainsAnyOfTheseValues(userInfo.ClusterRoles, admissionInfo.ClusterRoles...) { + !utils.SliceContains(keys, ExcludeUserInfo...) { + if !utils.SliceContains(userInfo.ClusterRoles, admissionInfo.ClusterRoles...) { errs = append(errs, fmt.Errorf("user info does not match clustersRoles for the given conditionBlock")) } } @@ -149,22 +149,6 @@ func matchSubjects(ruleSubjects []rbacv1.Subject, userInfo authenticationv1.User return false } -func DoesSliceContainsAnyOfTheseValues(slice []string, values ...string) bool { - - var sliceElementsMap = make(map[string]bool, len(slice)) - for _, sliceElement := range slice { - sliceElementsMap[sliceElement] = true - } - - for _, value := range values { - if sliceElementsMap[value] { - return true - } - } - - return false -} - //MatchesResourceDescription checks if the resource matches resource description of the rule or not func MatchesResourceDescription(resourceRef unstructured.Unstructured, ruleRef kyverno.Rule, admissionInfoRef kyverno.RequestInfo) error { rule := *ruleRef.DeepCopy() diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 0d28fcb32b..94f18a346b 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -148,7 +148,7 @@ func validateResource(log logr.Logger, ctx context.EvalInterface, policy kyverno // check if the resource satisfies the filter conditions defined in the rule // TODO: this needs to be extracted, to filter the resource so that we can avoid passing resources that - // dont statisfy a policy rule resource description + // dont satisfy a policy rule resource description if err := MatchesResourceDescription(resource, rule, admissionInfo); err != nil { log.V(4).Info("resource fails the match description", "reason", err.Error()) continue diff --git a/pkg/userinfo/roleRef.go b/pkg/userinfo/roleRef.go index 78e4d0a7dc..93517dd1c9 100644 --- a/pkg/userinfo/roleRef.go +++ b/pkg/userinfo/roleRef.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/nirmata/kyverno/pkg/engine" + "github.com/nirmata/kyverno/pkg/utils" v1beta1 "k8s.io/api/admission/v1beta1" authenticationv1 "k8s.io/api/authentication/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -22,7 +23,7 @@ const ( //GetRoleRef gets the list of roles and cluster roles for the incoming api-request func GetRoleRef(rbLister rbaclister.RoleBindingLister, crbLister rbaclister.ClusterRoleBindingLister, request *v1beta1.AdmissionRequest) (roles []string, clusterRoles []string, err error) { keys := append(request.UserInfo.Groups, request.UserInfo.Username) - if engine.DoesSliceContainsAnyOfTheseValues(keys, engine.ExcludeUserInfo...) { + if utils.SliceContains(keys, engine.ExcludeUserInfo...) { return } @@ -107,10 +108,10 @@ func matchSubjectsMap(subject rbacv1.Subject, userInfo authenticationv1.UserInfo func matchServiceAccount(subject rbacv1.Subject, userInfo authenticationv1.UserInfo) bool { subjectServiceAccount := subject.Namespace + ":" + subject.Name if userInfo.Username[len(SaPrefix):] != subjectServiceAccount { - log.Log.V(6).Info(fmt.Sprintf("service account not match, expect %s, got %s", subjectServiceAccount, userInfo.Username[len(SaPrefix):])) return false } + log.Log.V(3).Info(fmt.Sprintf("found a matched service account not match: %s", subjectServiceAccount)) return true } @@ -119,10 +120,10 @@ func matchUserOrGroup(subject rbacv1.Subject, userInfo authenticationv1.UserInfo keys := append(userInfo.Groups, userInfo.Username) for _, key := range keys { if subject.Name == key { + log.Log.V(3).Info(fmt.Sprintf("found a matched user/group '%v' in request userInfo: %v", subject.Name, keys)) return true } } - log.Log.V(6).Info(fmt.Sprintf("user/group '%v' info not found in request userInfo: %v", subject.Name, keys)) return false } diff --git a/pkg/utils/util.go b/pkg/utils/util.go index d5ff649d6e..13920a02c4 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -173,3 +173,19 @@ func HigherThanKubernetesVersion(client *client.Client, log logr.Logger, k8smajo } return true } + +func SliceContains(slice []string, values ...string) bool { + + var sliceElementsMap = make(map[string]bool, len(slice)) + for _, sliceElement := range slice { + sliceElementsMap[sliceElement] = true + } + + for _, value := range values { + if sliceElementsMap[value] { + return true + } + } + + return false +}