From d6062fdd47b26561211bad02a32fe1e4edd2a2cf Mon Sep 17 00:00:00 2001 From: shuting Date: Fri, 14 Aug 2020 12:21:06 -0700 Subject: [PATCH] Add go fmt (#1055) * remove empty flag * format code * revert change in install.yaml --- Makefile | 12 +++++++++-- cmd/kyverno/main.go | 6 +++--- definitions/install.yaml | 1 - definitions/manifest/deployment.yaml | 1 - pkg/config/dynamicconfig.go | 31 ++++++++++++++-------------- pkg/engine/generation.go | 10 ++++----- pkg/engine/mutation.go | 4 ++-- pkg/engine/utils.go | 14 ++++++------- pkg/engine/utils_test.go | 14 ++++++------- pkg/engine/validation.go | 22 ++++++++++---------- pkg/generate/cleanup/controller.go | 2 +- pkg/generate/controller.go | 2 +- pkg/generate/generate.go | 10 ++++----- pkg/policy/apply.go | 4 ++-- pkg/policy/existing.go | 2 +- pkg/policystatus/main.go | 3 ++- pkg/testrunner/scenario.go | 10 ++++----- pkg/userinfo/roleRef.go | 28 ++++++++++++------------- pkg/webhooks/generation.go | 10 ++++----- pkg/webhooks/mutation.go | 6 +++--- pkg/webhooks/policyvalidation.go | 6 ++++++ pkg/webhooks/server.go | 16 +++++++------- pkg/webhooks/validate_audit.go | 8 +++---- pkg/webhooks/validation.go | 10 ++++----- 24 files changed, 121 insertions(+), 111 deletions(-) diff --git a/Makefile b/Makefile index 3de092ebb6..2e8ac81506 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,7 @@ PWD := $(CURDIR) ################################## INITC_PATH := cmd/initContainer INITC_IMAGE := kyvernopre -initContainer: +initContainer: fmt vet GOOS=$(GOOS) go build -o $(PWD)/$(INITC_PATH)/kyvernopre -ldflags=$(LD_FLAGS) $(PWD)/$(INITC_PATH)/main.go .PHONY: docker-build-initContainer docker-tag-repo-initContainer docker-push-initContainer @@ -58,7 +58,7 @@ local: go build -ldflags=$(LD_FLAGS) $(PWD)/$(KYVERNO_PATH) go build -ldflags=$(LD_FLAGS) $(PWD)/$(CLI_PATH) -kyverno: +kyverno: fmt vet GOOS=$(GOOS) go build -o $(PWD)/$(KYVERNO_PATH)/kyverno -ldflags=$(LD_FLAGS) $(PWD)/$(KYVERNO_PATH)/main.go docker-publish-kyverno: docker-build-kyverno docker-tag-repo-kyverno docker-push-kyverno @@ -164,3 +164,11 @@ release: kustomize build ./definitions > ./definitions/install.yaml kustomize build ./definitions > ./definitions/release/install.yaml + +# Run go fmt against code +fmt: + go fmt ./... + +vet: + go vet ./... + diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 40bdce0a6a..e452884261 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -48,7 +48,7 @@ var ( filterK8Resources string excludeGroupRole string - excludeUsername string + excludeUsername string // User FQDN as CSR CN fqdncn bool setupLog = log.Log.WithName("setup") @@ -58,8 +58,8 @@ func main() { klog.InitFlags(nil) log.SetLogger(klogr.New()) flag.StringVar(&filterK8Resources, "filterK8Resources", "", "k8 resource in format [kind,namespace,name] where policy is not evaluated by the admission webhook. example --filterKind \"[Deployment, kyverno, kyverno]\" --filterKind \"[Deployment, kyverno, kyverno],[Events, *, *]\"") - flag.StringVar(&excludeGroupRole, "excludeGroupRole","","") - flag.StringVar(&excludeUsername, "excludeUsername","","") + flag.StringVar(&excludeGroupRole, "excludeGroupRole", "", "") + flag.StringVar(&excludeUsername, "excludeUsername", "", "") flag.IntVar(&webhookTimeout, "webhooktimeout", 3, "timeout for webhook configurations") flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.") flag.StringVar(&serverIP, "serverIP", "", "IP address where Kyverno controller runs. Only required if out-of-cluster.") diff --git a/definitions/install.yaml b/definitions/install.yaml index 3985976273..c90a8c0e58 100644 --- a/definitions/install.yaml +++ b/definitions/install.yaml @@ -766,7 +766,6 @@ spec: containers: - args: - --filterK8Resources=[Event,*,*][*,kube-system,*][*,kube-public,*][*,kube-node-lease,*][Node,*,*][APIService,*,*][TokenReview,*,*][SubjectAccessReview,*,*][*,kyverno,*][Binding,*,*][ReplicaSet,*,*] - - --excludeGroupRole="" - -v=2 env: - name: INIT_CONFIG diff --git a/definitions/manifest/deployment.yaml b/definitions/manifest/deployment.yaml index 7a224434fd..0be4f21d29 100644 --- a/definitions/manifest/deployment.yaml +++ b/definitions/manifest/deployment.yaml @@ -31,7 +31,6 @@ spec: #- "--webhooktimeout=4" # enable profiling # - "--profile" - - --excludeGroupRole="" - "-v=2" ports: - containerPort: 443 diff --git a/pkg/config/dynamicconfig.go b/pkg/config/dynamicconfig.go index 963d6a7a28..a9ca525915 100644 --- a/pkg/config/dynamicconfig.go +++ b/pkg/config/dynamicconfig.go @@ -19,7 +19,7 @@ import ( // this configmap stores the resources that are to be filtered const cmNameEnv string = "INIT_CONFIG" -var defaultExcludeGroupRole []string = []string{"system:serviceaccounts:kube-system", "system:nodes","system:kube-scheduler"} +var defaultExcludeGroupRole []string = []string{"system:serviceaccounts:kube-system", "system:nodes", "system:kube-scheduler"} // ConfigData stores the configuration type ConfigData struct { @@ -86,7 +86,7 @@ type Interface interface { } // NewConfigData ... -func NewConfigData(rclient kubernetes.Interface, cmInformer informers.ConfigMapInformer, filterK8Resources,excludeGroupRole,excludeUsername string, log logr.Logger) *ConfigData { +func NewConfigData(rclient kubernetes.Interface, cmInformer informers.ConfigMapInformer, filterK8Resources, excludeGroupRole, excludeUsername string, log logr.Logger) *ConfigData { // environment var is read at start only if cmNameEnv == "" { log.Info("ConfigMap name not defined in env:INIT_CONFIG: loading no default configuration") @@ -108,12 +108,12 @@ func NewConfigData(rclient kubernetes.Interface, cmInformer informers.ConfigMapI if excludeGroupRole != "" { cd.log.Info("init configuration from commandline arguments for excludeGroupRole") - cd.initRbac("excludeRoles",excludeGroupRole) + cd.initRbac("excludeRoles", excludeGroupRole) } if excludeUsername != "" { cd.log.Info("init configuration from commandline arguments for excludeUsername") - cd.initRbac("excludeUsername",excludeUsername) + cd.initRbac("excludeUsername", excludeUsername) } cmInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -220,28 +220,28 @@ func (cd *ConfigData) load(cm v1.ConfigMap) { newFilters := parseKinds(filters) if reflect.DeepEqual(newFilters, cd.filters) { logger.V(4).Info("resourceFilters did not change") - }else{ + } else { logger.V(2).Info("Updated resource filters", "oldFilters", cd.filters, "newFilters", newFilters) // update filters cd.filters = newFilters } newExcludeGroupRoles := parseRbac(excludeGroupRole) - newExcludeGroupRoles = append(newExcludeGroupRoles,defaultExcludeGroupRole...) + newExcludeGroupRoles = append(newExcludeGroupRoles, defaultExcludeGroupRole...) if reflect.DeepEqual(newExcludeGroupRoles, cd.excludeGroupRole) { logger.V(4).Info("excludeGroupRole did not change") - }else{ + } else { logger.V(2).Info("Updated resource excludeGroupRoles", "oldExcludeGroupRole", cd.excludeGroupRole, "newExcludeGroupRole", newExcludeGroupRoles) // update filters - cd.excludeGroupRole = newExcludeGroupRoles + cd.excludeGroupRole = newExcludeGroupRoles } excludeUsernames := parseRbac(excludeUsername) if reflect.DeepEqual(excludeUsernames, cd.excludeUsername) { logger.V(4).Info("excludeGroupRole did not change") - }else{ + } else { logger.V(2).Info("Updated resource excludeUsernames", "oldExcludeUsername", cd.excludeUsername, "newExcludeUsername", excludeUsernames) // update filters - cd.excludeUsername = excludeUsernames + cd.excludeUsername = excludeUsernames } } @@ -260,7 +260,7 @@ func (cd *ConfigData) initFilters(filters string) { cd.filters = newFilters } -func (cd *ConfigData) initRbac(action,exclude string) { +func (cd *ConfigData) initRbac(action, exclude string) { logger := cd.log // parse and load the configuration cd.mux.Lock() @@ -270,14 +270,13 @@ func (cd *ConfigData) initRbac(action,exclude string) { // update filters if action == "excludeRoles" { cd.excludeGroupRole = rbac - cd.excludeGroupRole = append(cd.excludeGroupRole,defaultExcludeGroupRole...) - }else{ + cd.excludeGroupRole = append(cd.excludeGroupRole, defaultExcludeGroupRole...) + } else { cd.excludeUsername = rbac } } - func (cd *ConfigData) unload(cm v1.ConfigMap) { logger := cd.log logger.Info("ConfigMap deleted, removing configuration filters", "name", cm.Name, "namespace", cm.Namespace) @@ -285,7 +284,7 @@ func (cd *ConfigData) unload(cm v1.ConfigMap) { defer cd.mux.Unlock() cd.filters = []k8Resource{} cd.excludeGroupRole = []string{} - cd.excludeGroupRole = append(cd.excludeGroupRole,defaultExcludeGroupRole...) + cd.excludeGroupRole = append(cd.excludeGroupRole, defaultExcludeGroupRole...) cd.excludeUsername = []string{} } @@ -326,4 +325,4 @@ func parseKinds(list string) []k8Resource { func parseRbac(list string) []string { return strings.Split(list, ",") -} \ No newline at end of file +} diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index a149f6824c..f280cd3a41 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -25,16 +25,16 @@ func Generate(policyContext PolicyContext) (resp response.EngineResponse) { logger := log.Log.WithName("Generate").WithValues("policy", policy.Name, "kind", resource.GetKind(), "namespace", resource.GetNamespace(), "name", resource.GetName()) - return filterRules(policy, resource, admissionInfo, ctx, logger,policyContext.ExcludeGroupRole) + return filterRules(policy, resource, admissionInfo, ctx, logger, policyContext.ExcludeGroupRole) } -func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger,excludeGroupRole []string) *response.RuleResponse { +func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger, excludeGroupRole []string) *response.RuleResponse { if !rule.HasGenerate() { return nil } startTime := time.Now() - if err := MatchesResourceDescription(resource, rule, admissionInfo,excludeGroupRole); err != nil { + if err := MatchesResourceDescription(resource, rule, admissionInfo, excludeGroupRole); err != nil { return nil } // operate on the copy of the conditions, as we perform variable substitution @@ -56,7 +56,7 @@ func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admission } } -func filterRules(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger,excludeGroupRole []string) response.EngineResponse { +func filterRules(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger, excludeGroupRole []string) response.EngineResponse { resp := response.EngineResponse{ PolicyResponse: response.PolicyResponse{ Policy: policy.Name, @@ -68,7 +68,7 @@ func filterRules(policy kyverno.ClusterPolicy, resource unstructured.Unstructure }, } for _, rule := range policy.Spec.Rules { - if ruleResp := filterRule(rule, resource, admissionInfo, ctx, log,excludeGroupRole); ruleResp != nil { + if ruleResp := filterRule(rule, resource, admissionInfo, ctx, log, excludeGroupRole); ruleResp != nil { resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, *ruleResp) } } diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 6835113750..713b144a83 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -51,11 +51,11 @@ 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 satisfy a policy rule resource description - excludeResource := []string{} + excludeResource := []string{} if len(policyContext.ExcludeGroupRole) > 0 { excludeResource = policyContext.ExcludeGroupRole } - if err := MatchesResourceDescription(patchedResource, rule, policyContext.AdmissionInfo,excludeResource); err != nil { + if err := MatchesResourceDescription(patchedResource, rule, policyContext.AdmissionInfo, excludeResource); err != nil { logger.V(3).Info("resource not matched", "reason", err.Error()) continue } diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go index 6a5e0363a9..e4db466a68 100644 --- a/pkg/engine/utils.go +++ b/pkg/engine/utils.go @@ -78,7 +78,7 @@ func checkSelector(labelSelector *metav1.LabelSelector, resourceLabels map[strin // should be: AND across attibutes but an OR inside attributes that of type list // To filter out the targeted resources with UserInfo, the check // should be: OR (accross & inside) attributes -func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription, userInfo kyverno.UserInfo, admissionInfo kyverno.RequestInfo, resource unstructured.Unstructured,dynamicConfig []string) []error { +func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription, userInfo kyverno.UserInfo, admissionInfo kyverno.RequestInfo, resource unstructured.Unstructured, dynamicConfig []string) []error { var errs []error if len(conditionBlock.Kinds) > 0 { if !checkKind(conditionBlock.Kinds, resource.GetKind()) { @@ -132,7 +132,7 @@ func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription, if len(userInfo.Subjects) > 0 { checkedItem++ - if !matchSubjects(userInfo.Subjects, admissionInfo.AdmissionUserInfo,dynamicConfig) { + if !matchSubjects(userInfo.Subjects, admissionInfo.AdmissionUserInfo, dynamicConfig) { userInfoErrors = append(userInfoErrors, fmt.Errorf("user info does not match subject for the given conditionBlock")) } else { return errs @@ -147,13 +147,13 @@ func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription, } // matchSubjects return true if one of ruleSubjects exist in userInfo -func matchSubjects(ruleSubjects []rbacv1.Subject, userInfo authenticationv1.UserInfo,dynamicConfig []string) bool { +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/nirmata/kyverno/issues/861 - for _,e := range dynamicConfig { + for _, e := range dynamicConfig { ruleSubjects = append(ruleSubjects, rbacv1.Subject{Kind: "Group", Name: e}, ) @@ -180,7 +180,7 @@ func matchSubjects(ruleSubjects []rbacv1.Subject, userInfo authenticationv1.User } //MatchesResourceDescription checks if the resource matches resource description of the rule or not -func MatchesResourceDescription(resourceRef unstructured.Unstructured, ruleRef kyverno.Rule, admissionInfoRef kyverno.RequestInfo,dynamicConfig []string) error { +func MatchesResourceDescription(resourceRef unstructured.Unstructured, ruleRef kyverno.Rule, admissionInfoRef kyverno.RequestInfo, dynamicConfig []string) error { rule := *ruleRef.DeepCopy() resource := *resourceRef.DeepCopy() @@ -195,7 +195,7 @@ func MatchesResourceDescription(resourceRef unstructured.Unstructured, ruleRef k // checking if resource matches the rule if !reflect.DeepEqual(rule.MatchResources.ResourceDescription, kyverno.ResourceDescription{}) || !reflect.DeepEqual(rule.MatchResources.UserInfo, kyverno.UserInfo{}) { - matchErrs := doesResourceMatchConditionBlock(rule.MatchResources.ResourceDescription, rule.MatchResources.UserInfo, admissionInfo, resource,dynamicConfig) + matchErrs := doesResourceMatchConditionBlock(rule.MatchResources.ResourceDescription, rule.MatchResources.UserInfo, admissionInfo, resource, dynamicConfig) reasonsForFailure = append(reasonsForFailure, matchErrs...) } else { reasonsForFailure = append(reasonsForFailure, fmt.Errorf("match cannot be empty")) @@ -204,7 +204,7 @@ func MatchesResourceDescription(resourceRef unstructured.Unstructured, ruleRef k // checking if resource has been excluded if !reflect.DeepEqual(rule.ExcludeResources.ResourceDescription, kyverno.ResourceDescription{}) || !reflect.DeepEqual(rule.ExcludeResources.UserInfo, kyverno.UserInfo{}) { - excludeErrs := doesResourceMatchConditionBlock(rule.ExcludeResources.ResourceDescription, rule.ExcludeResources.UserInfo, admissionInfo, resource,dynamicConfig) + excludeErrs := doesResourceMatchConditionBlock(rule.ExcludeResources.ResourceDescription, rule.ExcludeResources.UserInfo, admissionInfo, resource, dynamicConfig) if excludeErrs == nil { reasonsForFailure = append(reasonsForFailure, fmt.Errorf("resource excluded")) } diff --git a/pkg/engine/utils_test.go b/pkg/engine/utils_test.go index ef2881f996..ed924365e4 100644 --- a/pkg/engine/utils_test.go +++ b/pkg/engine/utils_test.go @@ -70,7 +70,7 @@ func TestMatchesResourceDescription(t *testing.T) { resource, _ := utils.ConvertToUnstructured(tc.Resource) for _, rule := range policy.Spec.Rules { - err := MatchesResourceDescription(*resource, rule, tc.AdmissionInfo,[]string{}) + err := MatchesResourceDescription(*resource, rule, tc.AdmissionInfo, []string{}) if err != nil { if !tc.areErrorsExpected { t.Errorf("Testcase %d Unexpected error: %v", i+1, err) @@ -138,7 +138,7 @@ func TestResourceDescriptionMatch_MultipleKind(t *testing.T) { } rule := kyverno.Rule{MatchResources: kyverno.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{},[]string{}); err != nil { + if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{}, []string{}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } @@ -199,7 +199,7 @@ func TestResourceDescriptionMatch_Name(t *testing.T) { } rule := kyverno.Rule{MatchResources: kyverno.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{},[]string{}); err != nil { + if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{}, []string{}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -259,7 +259,7 @@ func TestResourceDescriptionMatch_Name_Regex(t *testing.T) { } rule := kyverno.Rule{MatchResources: kyverno.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{},[]string{}); err != nil { + if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{}, []string{}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -327,7 +327,7 @@ func TestResourceDescriptionMatch_Label_Expression_NotMatch(t *testing.T) { } rule := kyverno.Rule{MatchResources: kyverno.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{},[]string{}); err != nil { + if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{}, []string{}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -396,7 +396,7 @@ func TestResourceDescriptionMatch_Label_Expression_Match(t *testing.T) { } rule := kyverno.Rule{MatchResources: kyverno.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{},[]string{}); err != nil { + if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{}, []string{}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -476,7 +476,7 @@ func TestResourceDescriptionExclude_Label_Expression_Match(t *testing.T) { rule := kyverno.Rule{MatchResources: kyverno.MatchResources{ResourceDescription: resourceDescription}, ExcludeResources: kyverno.ExcludeResources{ResourceDescription: resourceDescriptionExclude}} - if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{},[]string{}); err == nil { + if err := MatchesResourceDescription(*resource, rule, kyverno.RequestInfo{}, []string{}); err == nil { t.Errorf("Testcase has failed due to the following:\n Function has returned no error, even though it was suposed to fail") } } diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 58e36dcd79..91e2ca59c6 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -62,19 +62,19 @@ func Validate(policyContext PolicyContext) (resp response.EngineResponse) { // If request is delete, newR will be empty if reflect.DeepEqual(newR, unstructured.Unstructured{}) { - return *isRequestDenied(logger, ctx, policy, oldR, admissionInfo,policyContext.ExcludeGroupRole) + return *isRequestDenied(logger, ctx, policy, oldR, admissionInfo, policyContext.ExcludeGroupRole) } - if denyResp := isRequestDenied(logger, ctx, policy, newR, admissionInfo,policyContext.ExcludeGroupRole); !denyResp.IsSuccessful() { + if denyResp := isRequestDenied(logger, ctx, policy, newR, admissionInfo, policyContext.ExcludeGroupRole); !denyResp.IsSuccessful() { return *denyResp } if reflect.DeepEqual(oldR, unstructured.Unstructured{}) { - return *validateResource(logger, ctx, policy, newR, admissionInfo,policyContext.ExcludeGroupRole) + return *validateResource(logger, ctx, policy, newR, admissionInfo, policyContext.ExcludeGroupRole) } - oldResponse := validateResource(logger, ctx, policy, oldR, admissionInfo,policyContext.ExcludeGroupRole) - newResponse := validateResource(logger, ctx, policy, newR, admissionInfo,policyContext.ExcludeGroupRole) + oldResponse := validateResource(logger, ctx, policy, oldR, admissionInfo, policyContext.ExcludeGroupRole) + newResponse := validateResource(logger, ctx, policy, newR, admissionInfo, policyContext.ExcludeGroupRole) if !isSameResponse(oldResponse, newResponse) { return *newResponse } @@ -102,7 +102,7 @@ func incrementAppliedCount(resp *response.EngineResponse) { resp.PolicyResponse.RulesAppliedCount++ } -func isRequestDenied(log logr.Logger, ctx context.EvalInterface, policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo,excludeGroupRole []string) *response.EngineResponse { +func isRequestDenied(log logr.Logger, ctx context.EvalInterface, policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, excludeGroupRole []string) *response.EngineResponse { resp := &response.EngineResponse{} if policy.HasAutoGenAnnotation() && excludePod(resource) { log.V(5).Info("Skip applying policy, Pod has ownerRef set", "policy", policy.GetName()) @@ -117,7 +117,7 @@ func isRequestDenied(log logr.Logger, ctx context.EvalInterface, policy kyverno. continue } - if err := MatchesResourceDescription(resource, rule, admissionInfo,excludeResource); err != nil { + if err := MatchesResourceDescription(resource, rule, admissionInfo, excludeResource); err != nil { log.V(4).Info("resource fails the match description", "reason", err.Error()) continue } @@ -147,7 +147,7 @@ func isRequestDenied(log logr.Logger, ctx context.EvalInterface, policy kyverno. return resp } -func validateResource(log logr.Logger, ctx context.EvalInterface, policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo,excludeGroupRole []string) *response.EngineResponse { +func validateResource(log logr.Logger, ctx context.EvalInterface, policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, excludeGroupRole []string) *response.EngineResponse { resp := &response.EngineResponse{} if policy.HasAutoGenAnnotation() && excludePod(resource) { @@ -155,8 +155,8 @@ func validateResource(log logr.Logger, ctx context.EvalInterface, policy kyverno return resp } - excludeResource := []string{} - if len(excludeGroupRole)>0 { + excludeResource := []string{} + if len(excludeGroupRole) > 0 { excludeResource = excludeGroupRole } @@ -168,7 +168,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 satisfy a policy rule resource description - if err := MatchesResourceDescription(resource, rule, admissionInfo,excludeResource); err != nil { + if err := MatchesResourceDescription(resource, rule, admissionInfo, excludeResource); err != nil { log.V(4).Info("resource fails the match description", "reason", err.Error()) continue } diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go index ec7267eec2..ed71f85d3b 100644 --- a/pkg/generate/cleanup/controller.go +++ b/pkg/generate/cleanup/controller.go @@ -262,7 +262,7 @@ func (c *Controller) syncGenerateRequest(key string) error { if err != nil { return err } - _, err = c.pLister.Get(gr.Spec.Policy); + _, err = c.pLister.Get(gr.Spec.Policy) if err != nil { if !errors.IsNotFound(err) { return err diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go index f7a710d1bd..ceafc4068a 100644 --- a/pkg/generate/controller.go +++ b/pkg/generate/controller.go @@ -84,7 +84,7 @@ func NewController( dynamicInformer: dynamicInformer, log: log, policyStatusListener: policyStatus, - Config: dynamicConfig, + Config: dynamicConfig, } c.statusControl = StatusControl{client: kyvernoclient} diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 0bb22faeae..e3e00e0380 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -93,11 +93,11 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern } policyContext := engine.PolicyContext{ - NewResource: resource, - Policy: *policy, - Context: ctx, - AdmissionInfo: gr.Spec.Context.UserRequestInfo, - ExcludeGroupRole : c.Config.GetExcludeGroupRole(), + NewResource: resource, + Policy: *policy, + Context: ctx, + AdmissionInfo: gr.Spec.Context.UserRequestInfo, + ExcludeGroupRole: c.Config.GetExcludeGroupRole(), } // check if the policy still applies to the resource diff --git a/pkg/policy/apply.go b/pkg/policy/apply.go index 37ad7851ef..59a77ea318 100644 --- a/pkg/policy/apply.go +++ b/pkg/policy/apply.go @@ -19,7 +19,7 @@ import ( // applyPolicy applies policy on a resource //TODO: generation rules -func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, logger logr.Logger,excludeGroupRole []string) (responses []response.EngineResponse) { +func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, logger logr.Logger, excludeGroupRole []string) (responses []response.EngineResponse) { startTime := time.Now() defer func() { name := resource.GetKind() + "/" + resource.GetName() @@ -47,7 +47,7 @@ func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructure } //VALIDATION - engineResponseValidation = engine.Validate(engine.PolicyContext{Policy: policy, Context: ctx, NewResource: resource,ExcludeGroupRole: excludeGroupRole}) + engineResponseValidation = engine.Validate(engine.PolicyContext{Policy: policy, Context: ctx, NewResource: resource, ExcludeGroupRole: excludeGroupRole}) engineResponses = append(engineResponses, mergeRuleRespose(engineResponseMutation, engineResponseValidation)) //TODO: GENERATION diff --git a/pkg/policy/existing.go b/pkg/policy/existing.go index 12328dce84..11298d1879 100644 --- a/pkg/policy/existing.go +++ b/pkg/policy/existing.go @@ -47,7 +47,7 @@ func (pc *PolicyController) processExistingResources(policy *kyverno.ClusterPoli } // apply the policy on each - engineResponse := applyPolicy(*policy, resource, logger,pc.configHandler.GetExcludeGroupRole()) + engineResponse := applyPolicy(*policy, resource, logger, pc.configHandler.GetExcludeGroupRole()) // get engine response for mutation & validation independently engineResponses = append(engineResponses, engineResponse...) // post-processing, register the resource as processed diff --git a/pkg/policystatus/main.go b/pkg/policystatus/main.go index 451e17ac4b..e18d2aab70 100644 --- a/pkg/policystatus/main.go +++ b/pkg/policystatus/main.go @@ -3,10 +3,11 @@ package policystatus import ( "encoding/json" "fmt" - kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" "sync" "time" + kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" + "k8s.io/apimachinery/pkg/util/wait" "github.com/nirmata/kyverno/pkg/client/clientset/versioned" diff --git a/pkg/testrunner/scenario.go b/pkg/testrunner/scenario.go index f9640bbf41..a2c13b6b88 100644 --- a/pkg/testrunner/scenario.go +++ b/pkg/testrunner/scenario.go @@ -128,7 +128,7 @@ func runTestCase(t *testing.T, tc scaseT) bool { var er response.EngineResponse - er = engine.Mutate(engine.PolicyContext{Policy: *policy, NewResource: *resource,ExcludeGroupRole: []string{}}) + er = engine.Mutate(engine.PolicyContext{Policy: *policy, NewResource: *resource, ExcludeGroupRole: []string{}}) t.Log("---Mutation---") validateResource(t, er.PatchedResource, tc.Expected.Mutation.PatchedResource) validateResponse(t, er.PolicyResponse, tc.Expected.Mutation.PolicyResponse) @@ -138,7 +138,7 @@ func runTestCase(t *testing.T, tc scaseT) bool { resource = &er.PatchedResource } - er = engine.Validate(engine.PolicyContext{Policy: *policy, NewResource: *resource,ExcludeGroupRole: []string{}}) + er = engine.Validate(engine.PolicyContext{Policy: *policy, NewResource: *resource, ExcludeGroupRole: []string{}}) t.Log("---Validation---") validateResponse(t, er.PolicyResponse, tc.Expected.Validation.PolicyResponse) @@ -153,9 +153,9 @@ func runTestCase(t *testing.T, tc scaseT) bool { t.Error(err) } else { policyContext := engine.PolicyContext{ - NewResource: *resource, - Policy: *policy, - Client: client, + NewResource: *resource, + Policy: *policy, + Client: client, ExcludeGroupRole: []string{}, } diff --git a/pkg/userinfo/roleRef.go b/pkg/userinfo/roleRef.go index 73259f36ac..aa92ed72ac 100644 --- a/pkg/userinfo/roleRef.go +++ b/pkg/userinfo/roleRef.go @@ -18,18 +18,18 @@ const ( clusterrolekind = "ClusterRole" rolekind = "Role" SaPrefix = "system:serviceaccount:" - KyvernoSuffix = "kyverno:" + KyvernoSuffix = "kyverno:" ) type allRolesStruct struct { RoleType string - Role []string + Role []string } + var allRoles []allRolesStruct - //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,dynamicConfig config.Interface) (roles []string, clusterRoles []string, err error) { +func GetRoleRef(rbLister rbaclister.RoleBindingLister, crbLister rbaclister.ClusterRoleBindingLister, request *v1beta1.AdmissionRequest, dynamicConfig config.Interface) (roles []string, clusterRoles []string, err error) { keys := append(request.UserInfo.Groups, request.UserInfo.Username) if utils.SliceContains(keys, dynamicConfig.GetExcludeGroupRole()...) { return @@ -137,33 +137,33 @@ func matchUserOrGroup(subject rbacv1.Subject, userInfo authenticationv1.UserInfo } //IsRoleAuthorize is role authorize or not -func IsRoleAuthorize(rbLister rbaclister.RoleBindingLister, crbLister rbaclister.ClusterRoleBindingLister, rLister rbaclister.RoleLister, crLister rbaclister.ClusterRoleLister, request *v1beta1.AdmissionRequest,dynamicConfig config.Interface) (bool, error) { +func IsRoleAuthorize(rbLister rbaclister.RoleBindingLister, crbLister rbaclister.ClusterRoleBindingLister, rLister rbaclister.RoleLister, crLister rbaclister.ClusterRoleLister, request *v1beta1.AdmissionRequest, dynamicConfig config.Interface) (bool, error) { if strings.Contains(request.UserInfo.Username, SaPrefix) { - roles, clusterRoles, err := GetRoleRef(rbLister, crbLister, request,dynamicConfig) + roles, clusterRoles, err := GetRoleRef(rbLister, crbLister, request, dynamicConfig) if err != nil { return false, err } - allRoles := append(allRoles,allRolesStruct{ + allRoles := append(allRoles, allRolesStruct{ RoleType: "ClusterRole", - Role : clusterRoles, - },allRolesStruct{ + Role: clusterRoles, + }, allRolesStruct{ RoleType: "Role", - Role : roles, + Role: roles, }) for _, r := range allRoles { - for _,e := range r.Role { + for _, e := range r.Role { if strings.Contains(e, KyvernoSuffix) { return true, nil } var labels map[string]string if r.RoleType == "Role" { roleData := strings.Split(e, ":") - role, err := rLister.Roles(roleData[0]).Get(strings.Join(roleData[1:],":")) + role, err := rLister.Roles(roleData[0]).Get(strings.Join(roleData[1:], ":")) if err != nil { return false, err } labels = role.GetLabels() - }else{ + } else { role, err := crLister.Get(e) if err != nil { return false, err @@ -194,7 +194,7 @@ func IsRoleAuthorize(rbLister rbaclister.RoleBindingLister, crbLister rbaclister } var matchedRoles []bool - excludeGroupRule := append(dynamicConfig.GetExcludeGroupRole(),KyvernoSuffix) + excludeGroupRule := append(dynamicConfig.GetExcludeGroupRole(), KyvernoSuffix) for _, e := range request.UserInfo.Groups { for _, defaultSuffix := range excludeGroupRule { diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index bf2915420b..4c8c391925 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -21,7 +21,7 @@ import ( ) //HandleGenerate handles admission-requests for policies with generate rules -func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, policies []*kyverno.ClusterPolicy, ctx *context.Context, userRequestInfo kyverno.RequestInfo,dynamicConfig config.Interface) { +func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, policies []*kyverno.ClusterPolicy, ctx *context.Context, userRequestInfo kyverno.RequestInfo, dynamicConfig config.Interface) { logger := ws.log.WithValues("action", "generation", "uid", request.UID, "kind", request.Kind, "namespace", request.Namespace, "name", request.Name, "operation", request.Operation) logger.V(4).Info("incoming request") var engineResponses []response.EngineResponse @@ -40,10 +40,10 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic // CREATE resources, do not have name, assigned in admission-request policyContext := engine.PolicyContext{ - NewResource: *resource, - AdmissionInfo: userRequestInfo, - Context: ctx, - ExcludeGroupRole : dynamicConfig.GetExcludeGroupRole(), + NewResource: *resource, + AdmissionInfo: userRequestInfo, + Context: ctx, + ExcludeGroupRole: dynamicConfig.GetExcludeGroupRole(), } // engine.Generate returns a list of rules that are applicable on this resource diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index 9746f02766..9a6763d914 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -39,9 +39,9 @@ func (ws *WebhookServer) HandleMutation( var patches [][]byte var engineResponses []response.EngineResponse policyContext := engine.PolicyContext{ - NewResource: resource, - AdmissionInfo: userRequestInfo, - Context: ctx, + NewResource: resource, + AdmissionInfo: userRequestInfo, + Context: ctx, ExcludeGroupRole: ws.configHandler.GetExcludeGroupRole(), } diff --git a/pkg/webhooks/policyvalidation.go b/pkg/webhooks/policyvalidation.go index a9bbf81579..0e3dbcb018 100644 --- a/pkg/webhooks/policyvalidation.go +++ b/pkg/webhooks/policyvalidation.go @@ -1,6 +1,8 @@ package webhooks import ( + "time" + policyvalidate "github.com/nirmata/kyverno/pkg/policy" v1beta1 "k8s.io/api/admission/v1beta1" @@ -12,6 +14,10 @@ import ( func (ws *WebhookServer) policyValidation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse { logger := ws.log.WithValues("action", "policyvalidation", "uid", request.UID, "kind", request.Kind, "namespace", request.Namespace, "name", request.Name, "operation", request.Operation) + startTime := time.Now() + logger.V(3).Info("start validating policy") + defer logger.V(3).Info("finished validating policy", "time", time.Since(startTime).String()) + //TODO: can this happen? wont this be picked by OpenAPI spec schema ? if err := policyvalidate.Validate(request.Object.Raw, ws.client, false, ws.openAPIController); err != nil { logger.Error(err, "faield to validate policy") diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 71b60fa902..36d1d58863 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -280,7 +280,7 @@ func (ws *WebhookServer) resourceMutation(request *v1beta1.AdmissionRequest) *v1 var roles, clusterRoles []string var err error if containRBACinfo(mutatePolicies, validatePolicies, generatePolicies) { - roles, clusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request,ws.configHandler) + roles, clusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request, ws.configHandler) if err != nil { // TODO(shuting): continue apply policy if error getting roleRef? logger.Error(err, "failed to get RBAC infromation for request") @@ -342,7 +342,7 @@ func (ws *WebhookServer) resourceMutation(request *v1beta1.AdmissionRequest) *v1 ws.auditHandler.Add(request.DeepCopy()) // VALIDATION - ok, msg := HandleValidation(request, validatePolicies, nil, ctx, userRequestInfo, ws.statusListener, ws.eventGen, ws.pvGenerator, ws.log,ws.configHandler) + ok, msg := HandleValidation(request, validatePolicies, nil, ctx, userRequestInfo, ws.statusListener, ws.eventGen, ws.pvGenerator, ws.log, ws.configHandler) if !ok { logger.Info("admission request denied") return &v1beta1.AdmissionResponse{ @@ -363,9 +363,7 @@ func (ws *WebhookServer) resourceMutation(request *v1beta1.AdmissionRequest) *v1 // Success -> Generate Request CR created successfully // Failed -> Failed to create Generate Request CR - if request.Operation == v1beta1.Create || request.Operation == v1beta1.Update { - go ws.HandleGenerate(request.DeepCopy(), generatePolicies, ctx, userRequestInfo,ws.configHandler) - } + go ws.HandleGenerate(request.DeepCopy(), generatePolicies, ctx, userRequestInfo, ws.configHandler) // Succesful processing of mutation & validation rules in policy patchType := v1beta1.PatchTypeJSONPatch @@ -427,7 +425,7 @@ func (ws *WebhookServer) resourceValidation(request *v1beta1.AdmissionRequest) * var err error // getRoleRef only if policy has roles/clusterroles defined if containRBACinfo(policies) { - roles, clusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request,ws.configHandler) + roles, clusterRoles, err = userinfo.GetRoleRef(ws.rbLister, ws.crbLister, request, ws.configHandler) if err != nil { logger.Error(err, "failed to get RBAC information for request") return &v1beta1.AdmissionResponse{ @@ -463,7 +461,7 @@ func (ws *WebhookServer) resourceValidation(request *v1beta1.AdmissionRequest) * logger.Error(err, "failed to load service account in context") } - ok, msg := HandleValidation(request, policies, nil, ctx, userRequestInfo, ws.statusListener, ws.eventGen, ws.pvGenerator, ws.log,ws.configHandler) + ok, msg := HandleValidation(request, policies, nil, ctx, userRequestInfo, ws.statusListener, ws.eventGen, ws.pvGenerator, ws.log, ws.configHandler) if !ok { logger.Info("admission request denied") return &v1beta1.AdmissionResponse{ @@ -577,13 +575,13 @@ func (ws *WebhookServer) excludeKyvernoResources(request *v1beta1.AdmissionReque labels := resource.GetLabels() if labels != nil { if labels["app.kubernetes.io/managed-by"] == "kyverno" && labels["policy.kyverno.io/synchronize"] == "enable" { - isAuthorized, err := userinfo.IsRoleAuthorize(ws.rbLister, ws.crbLister, ws.rLister, ws.crLister, request,ws.configHandler) + isAuthorized, err := userinfo.IsRoleAuthorize(ws.rbLister, ws.crbLister, ws.rLister, ws.crLister, request, ws.configHandler) if err != nil { return fmt.Errorf("failed to get RBAC infromation for request %v", err) } if !isAuthorized { // convert RAW to unstructured - return fmt.Errorf("Resource is managed by a Kyverno policy and cannot be update manually. You can edit the policy %s to update this resource.", labels["policy.kyverno.io/policy-name"]) + return fmt.Errorf("resource is managed by a Kyverno policy and cannot be update manually. You can edit the policy %s to update this resource.", labels["policy.kyverno.io/policy-name"]) } } } diff --git a/pkg/webhooks/validate_audit.go b/pkg/webhooks/validate_audit.go index 0d9d5f6815..d60b276f99 100644 --- a/pkg/webhooks/validate_audit.go +++ b/pkg/webhooks/validate_audit.go @@ -50,7 +50,7 @@ type auditHandler struct { crbLister rbaclister.ClusterRoleBindingLister crbSynced cache.InformerSynced - log logr.Logger + log logr.Logger configHandler config.Interface } @@ -75,7 +75,7 @@ func NewValidateAuditHandler(pCache policycache.Interface, crbLister: crbInformer.Lister(), crbSynced: crbInformer.Informer().HasSynced, log: log, - configHandler : dynamicConfig, + configHandler: dynamicConfig, } } @@ -138,7 +138,7 @@ func (h *auditHandler) process(request *v1beta1.AdmissionRequest) error { // getRoleRef only if policy has roles/clusterroles defined if containRBACinfo(policies) { - roles, clusterRoles, err = userinfo.GetRoleRef(h.rbLister, h.crbLister, request,h.configHandler) + roles, clusterRoles, err = userinfo.GetRoleRef(h.rbLister, h.crbLister, request, h.configHandler) if err != nil { logger.Error(err, "failed to get RBAC information for request") } @@ -165,7 +165,7 @@ func (h *auditHandler) process(request *v1beta1.AdmissionRequest) error { return errors.Wrap(err, "failed to load service account in context") } - HandleValidation(request, policies, nil, ctx, userRequestInfo, h.statusListener, h.eventGen, h.pvGenerator, logger,h.configHandler) + HandleValidation(request, policies, nil, ctx, userRequestInfo, h.statusListener, h.eventGen, h.pvGenerator, logger, h.configHandler) return nil } diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index a62fce5bb6..d03b8d9db8 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -68,11 +68,11 @@ func HandleValidation( } policyContext := engine.PolicyContext{ - NewResource: newR, - OldResource: oldR, - Context: ctx, - AdmissionInfo: userRequestInfo, - ExcludeGroupRole : dynamicConfig.GetExcludeGroupRole(), + NewResource: newR, + OldResource: oldR, + Context: ctx, + AdmissionInfo: userRequestInfo, + ExcludeGroupRole: dynamicConfig.GetExcludeGroupRole(), } var engineResponses []response.EngineResponse