From b4a4e3a4f33a7c516c046f357448e363eb9b5cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Mon, 3 Apr 2023 06:57:48 +0200 Subject: [PATCH] refactor: don't process context/preconditions in invokeHandler (#6751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: engine handlers Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * refactor: don't process context/preconditions in invokeHandler Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché --- pkg/background/generate/generate.go | 2 +- pkg/engine/api/engine.go | 14 ++-- pkg/engine/engine.go | 67 ++++++------------ pkg/engine/generation.go | 12 ---- pkg/engine/handlers/handler.go | 9 +-- pkg/engine/handlers/mutation/common.go | 17 +++-- .../handlers/mutation/mutate_existing.go | 12 ++-- pkg/engine/handlers/mutation/mutate_image.go | 48 ++++++++----- .../handlers/mutation/mutate_resource.go | 17 ++--- .../handlers/validation/validate_image.go | 42 ++++++++---- .../handlers/validation/validate_manifest.go | 20 ++++++ .../handlers/validation/validate_pss.go | 21 +++++- .../handlers/validation/validate_resource.go | 21 ++---- .../{imageVerify.go => image_verify.go} | 68 +++++++++---------- ...ageVerify_test.go => image_verify_test.go} | 0 pkg/engine/mutation.go | 10 +-- .../{policyContext.go => policy_context.go} | 0 pkg/engine/validation.go | 56 +++++++-------- 18 files changed, 217 insertions(+), 219 deletions(-) rename pkg/engine/{imageVerify.go => image_verify.go} (50%) rename pkg/engine/{imageVerify_test.go => image_verify_test.go} (100%) rename pkg/engine/{policyContext.go => policy_context.go} (100%) diff --git a/pkg/background/generate/generate.go b/pkg/background/generate/generate.go index 0780e50a3e..f1e84e39c2 100644 --- a/pkg/background/generate/generate.go +++ b/pkg/background/generate/generate.go @@ -162,7 +162,7 @@ func (c *GenerateController) applyGenerate(resource unstructured.Unstructured, u } // check if the policy still applies to the resource - engineResponse := c.engine.GenerateResponse(context.Background(), policyContext, ur) + engineResponse := c.engine.Generate(context.Background(), policyContext) if len(engineResponse.PolicyResponse.Rules) == 0 { logger.V(4).Info(doesNotApply) return nil, errors.New(doesNotApply) diff --git a/pkg/engine/api/engine.go b/pkg/engine/api/engine.go index 157b050b32..2190981542 100644 --- a/pkg/engine/api/engine.go +++ b/pkg/engine/api/engine.go @@ -4,7 +4,6 @@ import ( "context" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" - kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" enginecontext "github.com/kyverno/kyverno/pkg/engine/context" ) @@ -28,6 +27,12 @@ type Engine interface { policyContext PolicyContext, ) EngineResponse + // Generate checks for validity of generate rule on the resource + Generate( + ctx context.Context, + policyContext PolicyContext, + ) EngineResponse + // VerifyAndPatchImages ... VerifyAndPatchImages( ctx context.Context, @@ -44,13 +49,6 @@ type Engine interface { policyContext PolicyContext, ) EngineResponse - // GenerateResponse checks for validity of generate rule on the resource - GenerateResponse( - ctx context.Context, - policyContext PolicyContext, - gr kyvernov1beta1.UpdateRequest, - ) EngineResponse - ContextLoader( policy kyvernov1.PolicyInterface, rule kyvernov1.Rule, diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 1f5099798b..d10578fcb1 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -6,9 +6,7 @@ import ( "time" "github.com/go-logr/logr" - gojmespath "github.com/jmespath/go-jmespath" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" - kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/clients/dclient" "github.com/kyverno/kyverno/pkg/config" engineapi "github.com/kyverno/kyverno/pkg/engine/api" @@ -34,6 +32,7 @@ type engine struct { validateResourceHandler handlers.Handler validateManifestHandler handlers.Handler validatePssHandler handlers.Handler + validateImageHandler handlers.Handler mutateResourceHandler handlers.Handler mutateExistingHandler handlers.Handler } @@ -63,11 +62,12 @@ func NewEngine( rclient: rclient, engineContextLoaderFactory: engineContextLoaderFactory, exceptionSelector: exceptionSelector, - validateResourceHandler: validation.NewValidateResourceHandler(engineContextLoaderFactory), + validateResourceHandler: validation.NewValidateResourceHandler(), validateManifestHandler: validation.NewValidateManifestHandler(client), validatePssHandler: validation.NewValidatePssHandler(), - mutateResourceHandler: mutation.NewMutateResourceHandler(engineContextLoaderFactory), - mutateExistingHandler: mutation.NewMutateExistingHandler(client, engineContextLoaderFactory), + validateImageHandler: validation.NewValidateImageHandler(configuration), + mutateResourceHandler: mutation.NewMutateResourceHandler(), + mutateExistingHandler: mutation.NewMutateExistingHandler(client), } } @@ -99,6 +99,19 @@ func (e *engine) Mutate( return response.Done(time.Now()) } +func (e *engine) Generate( + ctx context.Context, + policyContext engineapi.PolicyContext, +) engineapi.EngineResponse { + response := engineapi.NewEngineResponseFromPolicyContext(policyContext, time.Now()) + logger := internal.LoggerWithPolicyContext(logging.WithName("engine.generate"), policyContext) + if internal.MatchPolicyContext(logger, policyContext, e.configuration) { + policyResponse := e.generateResponse(ctx, logger, policyContext) + response = response.WithPolicyResponse(policyResponse) + } + return response.Done(time.Now()) +} + func (e *engine) VerifyAndPatchImages( ctx context.Context, policyContext engineapi.PolicyContext, @@ -126,20 +139,6 @@ func (e *engine) ApplyBackgroundChecks( return response.Done(time.Now()) } -func (e *engine) GenerateResponse( - ctx context.Context, - policyContext engineapi.PolicyContext, - gr kyvernov1beta1.UpdateRequest, -) engineapi.EngineResponse { - response := engineapi.NewEngineResponseFromPolicyContext(policyContext, time.Now()) - logger := internal.LoggerWithPolicyContext(logging.WithName("engine.generate"), policyContext) - if internal.MatchPolicyContext(logger, policyContext, e.configuration) { - policyResponse := e.generateResponse(ctx, logger, policyContext, gr) - response = response.WithPolicyResponse(policyResponse) - } - return response.Done(time.Now()) -} - func (e *engine) ContextLoader( policy kyvernov1.PolicyInterface, rule kyvernov1.Rule, @@ -189,7 +188,7 @@ func matches( func (e *engine) invokeRuleHandler( ctx context.Context, logger logr.Logger, - handlerFactory handlers.HandlerFactory, + handler handlers.Handler, policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, @@ -209,32 +208,8 @@ func (e *engine) invokeRuleHandler( if ruleResp := e.hasPolicyExceptions(logger, ruleType, policyContext, rule); ruleResp != nil { return resource, handlers.RuleResponses(ruleResp) } - if handlerFactory == nil { - return resource, handlers.RuleResponses(internal.RuleError(rule, ruleType, "failed to instantiate handler", nil)) - } else if handler, err := handlerFactory(); err != nil { - return resource, handlers.RuleResponses(internal.RuleError(rule, ruleType, "failed to instantiate handler", err)) - } else if handler != nil { - // load rule context - if err := internal.LoadContext(ctx, e, policyContext, rule); err != nil { - if _, ok := err.(gojmespath.NotFoundError); ok { - logger.V(3).Info("failed to load context", "reason", err.Error()) - } else { - logger.Error(err, "failed to load context") - } - return resource, handlers.RuleResponses(internal.RuleError(rule, ruleType, "failed to load context", err)) - } - // check preconditions - preconditionsPassed, err := internal.CheckPreconditions(logger, policyContext.JSONContext(), rule.GetAnyAllConditions()) - if err != nil { - return resource, handlers.RuleResponses(internal.RuleError(rule, ruleType, "failed to evaluate preconditions", err)) - } - if !preconditionsPassed { - return resource, handlers.RuleResponses(internal.RuleSkip(rule, ruleType, "preconditions not met")) - } - // process handler - return handler.Process(ctx, logger, policyContext, resource, rule) - } - return resource, nil + // process handler + return handler.Process(ctx, logger, policyContext, resource, rule, e.ContextLoader(policyContext.Policy(), rule)) }, ) } diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index cf09ea0ab1..b3109905e2 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -2,10 +2,8 @@ package engine import ( "context" - "time" "github.com/go-logr/logr" - kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/autogen" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/engine/internal" @@ -16,16 +14,6 @@ func (e *engine) generateResponse( ctx context.Context, logger logr.Logger, policyContext engineapi.PolicyContext, - gr kyvernov1beta1.UpdateRequest, -) engineapi.PolicyResponse { - return e.filterGenerateRules(policyContext, logger, gr.Spec.Policy, time.Now()) -} - -func (e *engine) filterGenerateRules( - policyContext engineapi.PolicyContext, - logger logr.Logger, - policyNameKey string, - startTime time.Time, ) engineapi.PolicyResponse { resp := engineapi.NewPolicyResponse() for _, rule := range autogen.ComputeRules(policyContext.Policy()) { diff --git a/pkg/engine/handlers/handler.go b/pkg/engine/handlers/handler.go index 0fc3be086a..e155201f75 100644 --- a/pkg/engine/handlers/handler.go +++ b/pkg/engine/handlers/handler.go @@ -16,17 +16,10 @@ type Handler interface { engineapi.PolicyContext, unstructured.Unstructured, kyvernov1.Rule, + engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) } -type HandlerFactory = func() (Handler, error) - -func WithHandler(handler Handler) HandlerFactory { - return func() (Handler, error) { - return handler, nil - } -} - func RuleResponses(rrs ...*engineapi.RuleResponse) []engineapi.RuleResponse { var out []engineapi.RuleResponse for _, rr := range rrs { diff --git a/pkg/engine/handlers/mutation/common.go b/pkg/engine/handlers/mutation/common.go index 78b3147243..4dc51c4a1d 100644 --- a/pkg/engine/handlers/mutation/common.go +++ b/pkg/engine/handlers/mutation/common.go @@ -14,17 +14,26 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func mutateResource(rule *kyvernov1.Rule, policyContext engineapi.PolicyContext, resource unstructured.Unstructured, logger logr.Logger) *mutate.Response { +func mutateResource( + ctx context.Context, + contextLoader engineapi.EngineContextLoader, + rule kyvernov1.Rule, + policyContext engineapi.PolicyContext, + resource unstructured.Unstructured, + logger logr.Logger, +) *mutate.Response { + if err := contextLoader(ctx, rule.Context, policyContext.JSONContext()); err != nil { + logger.Error(err, "failed to load context") + return mutate.NewErrorResponse("failed to load context", err) + } preconditionsPassed, err := internal.CheckPreconditions(logger, policyContext.JSONContext(), rule.GetAnyAllConditions()) if err != nil { return mutate.NewErrorResponse("failed to evaluate preconditions", err) } - if !preconditionsPassed { return mutate.NewResponse(engineapi.RuleStatusSkip, resource, nil, "preconditions not met") } - - return mutate.Mutate(rule, policyContext.JSONContext(), resource, logger) + return mutate.Mutate(&rule, policyContext.JSONContext(), resource, logger) } type forEachMutator struct { diff --git a/pkg/engine/handlers/mutation/mutate_existing.go b/pkg/engine/handlers/mutation/mutate_existing.go index 409afefb71..afcd4d2376 100644 --- a/pkg/engine/handlers/mutation/mutate_existing.go +++ b/pkg/engine/handlers/mutation/mutate_existing.go @@ -14,17 +14,14 @@ import ( ) type mutateExistingHandler struct { - client dclient.Interface - contextLoader engineapi.EngineContextLoaderFactory + client dclient.Interface } func NewMutateExistingHandler( client dclient.Interface, - contextLoader engineapi.EngineContextLoaderFactory, ) handlers.Handler { return mutateExistingHandler{ - client: client, - contextLoader: contextLoader, + client: client, } } @@ -34,9 +31,8 @@ func (h mutateExistingHandler) Process( policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, + contextLoader engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) { - policy := policyContext.Policy() - contextLoader := h.contextLoader(policy, rule) var responses []engineapi.RuleResponse logger.V(3).Info("processing mutate rule") var patchedResources []resourceInfo @@ -72,7 +68,7 @@ func (h mutateExistingHandler) Process( } mutateResp = m.mutateForEach(ctx) } else { - mutateResp = mutateResource(&rule, policyContext, patchedResource.unstructured, logger) + mutateResp = mutateResource(ctx, contextLoader, rule, policyContext, patchedResource.unstructured, logger) } if ruleResponse := buildRuleResponse(&rule, mutateResp, patchedResource); ruleResponse != nil { responses = append(responses, *ruleResponse) diff --git a/pkg/engine/handlers/mutation/mutate_image.go b/pkg/engine/handlers/mutation/mutate_image.go index 8befcc8510..24e4664ffd 100644 --- a/pkg/engine/handlers/mutation/mutate_image.go +++ b/pkg/engine/handlers/mutation/mutate_image.go @@ -4,6 +4,7 @@ import ( "context" "github.com/go-logr/logr" + gojmespath "github.com/jmespath/go-jmespath" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/config" engineapi "github.com/kyverno/kyverno/pkg/engine/api" @@ -13,7 +14,6 @@ import ( engineutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/engine/variables" "github.com/kyverno/kyverno/pkg/registryclient" - apiutils "github.com/kyverno/kyverno/pkg/utils/api" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -21,30 +21,18 @@ type mutateImageHandler struct { configuration config.Configuration rclient registryclient.Client ivm *engineapi.ImageVerificationMetadata - images []apiutils.ImageInfo } func NewMutateImageHandler( - policyContext engineapi.PolicyContext, - resource unstructured.Unstructured, - rule kyvernov1.Rule, configuration config.Configuration, rclient registryclient.Client, ivm *engineapi.ImageVerificationMetadata, -) (handlers.Handler, error) { - ruleImages, _, err := engineutils.ExtractMatchingImages(resource, policyContext.JSONContext(), rule, configuration) - if err != nil { - return nil, err - } - if len(ruleImages) == 0 { - return nil, nil - } +) handlers.Handler { return mutateImageHandler{ configuration: configuration, rclient: rclient, ivm: ivm, - images: ruleImages, - }, nil + } } func (h mutateImageHandler) Process( @@ -53,11 +41,39 @@ func (h mutateImageHandler) Process( policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, + contextLoader engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) { + if engineutils.IsDeleteRequest(policyContext) { + return resource, nil + } if len(rule.VerifyImages) == 0 { return resource, nil } + ruleImages, _, err := engineutils.ExtractMatchingImages(resource, policyContext.JSONContext(), rule, h.configuration) + if err != nil { + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.ImageVerify, "failed to extract images", err)) + } + if len(ruleImages) == 0 { + return resource, nil + } jsonContext := policyContext.JSONContext() + // load context + if err := contextLoader(ctx, rule.Context, jsonContext); err != nil { + if _, ok := err.(gojmespath.NotFoundError); ok { + logger.V(3).Info("failed to load context", "reason", err.Error()) + } else { + logger.Error(err, "failed to load context") + } + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.ImageVerify, "failed to load context", err)) + } + // check preconditions + preconditionsPassed, err := internal.CheckPreconditions(logger, jsonContext, rule.GetAnyAllConditions()) + if err != nil { + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.ImageVerify, "failed to evaluate preconditions", err)) + } + if !preconditionsPassed { + return resource, handlers.RuleResponses(internal.RuleSkip(rule, engineapi.ImageVerify, "preconditions not met")) + } ruleCopy, err := substituteVariables(rule, jsonContext, logger) if err != nil { return resource, handlers.RuleResponses( @@ -67,7 +83,7 @@ func (h mutateImageHandler) Process( iv := internal.NewImageVerifier(logger, h.rclient, policyContext, *ruleCopy, h.ivm) var engineResponses []*engineapi.RuleResponse for _, imageVerify := range ruleCopy.VerifyImages { - engineResponses = append(engineResponses, iv.Verify(ctx, imageVerify, h.images, h.configuration)...) + engineResponses = append(engineResponses, iv.Verify(ctx, imageVerify, ruleImages, h.configuration)...) } return resource, handlers.RuleResponses(engineResponses...) } diff --git a/pkg/engine/handlers/mutation/mutate_resource.go b/pkg/engine/handlers/mutation/mutate_resource.go index 98770f8931..11956673a6 100644 --- a/pkg/engine/handlers/mutation/mutate_resource.go +++ b/pkg/engine/handlers/mutation/mutate_resource.go @@ -12,16 +12,10 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -type mutateResourceHandler struct { - contextLoader engineapi.EngineContextLoaderFactory -} +type mutateResourceHandler struct{} -func NewMutateResourceHandler( - contextLoader engineapi.EngineContextLoaderFactory, -) handlers.Handler { - return mutateResourceHandler{ - contextLoader: contextLoader, - } +func NewMutateResourceHandler() handlers.Handler { + return mutateResourceHandler{} } func (h mutateResourceHandler) Process( @@ -30,9 +24,8 @@ func (h mutateResourceHandler) Process( policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, + contextLoader engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) { - policy := policyContext.Policy() - contextLoader := h.contextLoader(policy, rule) _, subresource := policyContext.ResourceKind() logger.V(3).Info("processing mutate rule") var parentResourceGVR metav1.GroupVersionResource @@ -58,7 +51,7 @@ func (h mutateResourceHandler) Process( } mutateResp = m.mutateForEach(ctx) } else { - mutateResp = mutateResource(&rule, policyContext, resourceInfo.unstructured, logger) + mutateResp = mutateResource(ctx, contextLoader, rule, policyContext, resourceInfo.unstructured, logger) } if mutateResp == nil { return resource, nil diff --git a/pkg/engine/handlers/validation/validate_image.go b/pkg/engine/handlers/validation/validate_image.go index d946fd4d97..92ab5fdcb0 100644 --- a/pkg/engine/handlers/validation/validate_image.go +++ b/pkg/engine/handlers/validation/validate_image.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/go-logr/logr" + gojmespath "github.com/jmespath/go-jmespath" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/config" engineapi "github.com/kyverno/kyverno/pkg/engine/api" @@ -15,22 +16,16 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -type validateImageHandler struct{} +type validateImageHandler struct { + configuration config.Configuration +} func NewValidateImageHandler( - policyContext engineapi.PolicyContext, - resource unstructured.Unstructured, - rule kyvernov1.Rule, configuration config.Configuration, -) (handlers.Handler, error) { - ruleImages, _, err := engineutils.ExtractMatchingImages(resource, policyContext.JSONContext(), rule, configuration) - if err != nil { - return nil, err +) handlers.Handler { + return validateImageHandler{ + configuration: configuration, } - if len(ruleImages) == 0 { - return nil, nil - } - return validateImageHandler{}, nil } func (h validateImageHandler) Process( @@ -39,11 +34,32 @@ func (h validateImageHandler) Process( policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, + contextLoader engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) { if engineutils.IsDeleteRequest(policyContext) { return resource, nil } - preconditionsPassed, err := internal.CheckPreconditions(logger, policyContext.JSONContext(), rule.RawAnyAllConditions) + if len(rule.VerifyImages) == 0 { + return resource, nil + } + ruleImages, _, err := engineutils.ExtractMatchingImages(resource, policyContext.JSONContext(), rule, h.configuration) + if err != nil { + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to extract images", err)) + } + if len(ruleImages) == 0 { + return resource, nil + } + // load context + if err := contextLoader(ctx, rule.Context, policyContext.JSONContext()); err != nil { + if _, ok := err.(gojmespath.NotFoundError); ok { + logger.V(3).Info("failed to load context", "reason", err.Error()) + } else { + logger.Error(err, "failed to load context") + } + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to load context", err)) + } + // check preconditions + preconditionsPassed, err := internal.CheckPreconditions(logger, policyContext.JSONContext(), rule.GetAnyAllConditions()) if err != nil { return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to evaluate preconditions", err)) } diff --git a/pkg/engine/handlers/validation/validate_manifest.go b/pkg/engine/handlers/validation/validate_manifest.go index f43087e73f..02e7eb42f8 100644 --- a/pkg/engine/handlers/validation/validate_manifest.go +++ b/pkg/engine/handlers/validation/validate_manifest.go @@ -14,6 +14,7 @@ import ( "github.com/ghodss/yaml" "github.com/go-logr/logr" + gojmespath "github.com/jmespath/go-jmespath" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/auth" "github.com/kyverno/kyverno/pkg/clients/dclient" @@ -50,10 +51,29 @@ func (h validateManifestHandler) Process( policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, + contextLoader engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) { if engineutils.IsDeleteRequest(policyContext) { return resource, nil } + // load context + if err := contextLoader(ctx, rule.Context, policyContext.JSONContext()); err != nil { + if _, ok := err.(gojmespath.NotFoundError); ok { + logger.V(3).Info("failed to load context", "reason", err.Error()) + } else { + logger.Error(err, "failed to load context") + } + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to load context", err)) + } + // check preconditions + preconditionsPassed, err := internal.CheckPreconditions(logger, policyContext.JSONContext(), rule.GetAnyAllConditions()) + if err != nil { + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to evaluate preconditions", err)) + } + if !preconditionsPassed { + return resource, handlers.RuleResponses(internal.RuleSkip(rule, engineapi.Validation, "preconditions not met")) + } + // verify manifest verified, reason, err := h.verifyManifest(ctx, logger, policyContext, *rule.Validation.Manifests) if err != nil { logger.V(3).Info("verifyManifest return err", "error", err.Error()) diff --git a/pkg/engine/handlers/validation/validate_pss.go b/pkg/engine/handlers/validation/validate_pss.go index 1765d53bde..55ab210b3c 100644 --- a/pkg/engine/handlers/validation/validate_pss.go +++ b/pkg/engine/handlers/validation/validate_pss.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/go-logr/logr" + gojmespath "github.com/jmespath/go-jmespath" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/engine/handlers" @@ -30,9 +31,27 @@ func (h validatePssHandler) Process( policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, + contextLoader engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) { - podSecurity := rule.Validation.PodSecurity + // load context + if err := contextLoader(ctx, rule.Context, policyContext.JSONContext()); err != nil { + if _, ok := err.(gojmespath.NotFoundError); ok { + logger.V(3).Info("failed to load context", "reason", err.Error()) + } else { + logger.Error(err, "failed to load context") + } + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to load context", err)) + } + // check preconditions + preconditionsPassed, err := internal.CheckPreconditions(logger, policyContext.JSONContext(), rule.GetAnyAllConditions()) + if err != nil { + return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to evaluate preconditions", err)) + } + if !preconditionsPassed { + return resource, handlers.RuleResponses(internal.RuleSkip(rule, engineapi.Validation, "preconditions not met")) + } // Marshal pod metadata and spec + podSecurity := rule.Validation.PodSecurity podSpec, metadata, err := getSpec(resource) if err != nil { return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "Error while getting new resource", err)) diff --git a/pkg/engine/handlers/validation/validate_resource.go b/pkg/engine/handlers/validation/validate_resource.go index cb4e4ed135..20fca3e5b3 100644 --- a/pkg/engine/handlers/validation/validate_resource.go +++ b/pkg/engine/handlers/validation/validate_resource.go @@ -21,16 +21,10 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -type validateResourceHandler struct { - contextLoader engineapi.EngineContextLoaderFactory -} +type validateResourceHandler struct{} -func NewValidateResourceHandler( - contextLoader engineapi.EngineContextLoaderFactory, -) handlers.Handler { - return validateResourceHandler{ - contextLoader: contextLoader, - } +func NewValidateResourceHandler() handlers.Handler { + return validateResourceHandler{} } func (h validateResourceHandler) Process( @@ -39,9 +33,8 @@ func (h validateResourceHandler) Process( policyContext engineapi.PolicyContext, resource unstructured.Unstructured, rule kyvernov1.Rule, + contextLoader engineapi.EngineContextLoader, ) (unstructured.Unstructured, []engineapi.RuleResponse) { - policy := policyContext.Policy() - contextLoader := h.contextLoader(policy, rule) v := newValidator(logger, contextLoader, policyContext, rule) return resource, handlers.RuleResponses(v.validate(ctx)) } @@ -87,12 +80,10 @@ func newForEachValidator( if err != nil { return nil, fmt.Errorf("failed to convert ruleCopy.Validation.ForEachValidation.AnyAllConditions: %w", err) } - nestedForEach, err := api.DeserializeJSONArray[kyvernov1.ForEachValidation](foreach.ForEachValidation) if err != nil { return nil, fmt.Errorf("failed to convert ruleCopy.Validation.ForEachValidation.AnyAllConditions: %w", err) } - return &validator{ log: log, policyContext: ctx, @@ -112,12 +103,10 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse { if err := v.loadContext(ctx); err != nil { return internal.RuleError(v.rule, engineapi.Validation, "failed to load context", err) } - preconditionsPassed, err := internal.CheckPreconditions(v.log, v.policyContext.JSONContext(), v.anyAllConditions) if err != nil { return internal.RuleError(v.rule, engineapi.Validation, "failed to evaluate preconditions", err) } - if !preconditionsPassed { return internal.RuleSkip(v.rule, engineapi.Validation, "preconditions not met") } @@ -222,10 +211,8 @@ func (v *validator) loadContext(ctx context.Context) error { } else { v.log.Error(err, "failed to load context") } - return err } - return nil } diff --git a/pkg/engine/imageVerify.go b/pkg/engine/image_verify.go similarity index 50% rename from pkg/engine/imageVerify.go rename to pkg/engine/image_verify.go index 804e6ee157..4cf6918558 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/image_verify.go @@ -8,7 +8,6 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/autogen" engineapi "github.com/kyverno/kyverno/pkg/engine/api" - "github.com/kyverno/kyverno/pkg/engine/handlers" "github.com/kyverno/kyverno/pkg/engine/handlers/mutation" "github.com/kyverno/kyverno/pkg/engine/internal" ) @@ -19,45 +18,42 @@ func (e *engine) verifyAndPatchImages( policyContext engineapi.PolicyContext, ) (engineapi.PolicyResponse, engineapi.ImageVerificationMetadata) { resp := engineapi.NewPolicyResponse() + policy := policyContext.Policy() + matchedResource := policyContext.NewResource() + applyRules := policy.GetSpec().GetApplyRules() + ivm := engineapi.ImageVerificationMetadata{} policyContext.JSONContext().Checkpoint() defer policyContext.JSONContext().Restore() - ivm := engineapi.ImageVerificationMetadata{} - policy := policyContext.Policy() - matchedResource := policyContext.NewResource() - applyRules := policy.GetSpec().GetApplyRules() - for _, rule := range autogen.ComputeRules(policyContext.Policy()) { - if rule.HasVerifyImages() { - startTime := time.Now() - handlerFactory := func() (handlers.Handler, error) { - return mutation.NewMutateImageHandler( - policyContext, - matchedResource, - rule, - e.configuration, - e.rclient, - &ivm, - ) - } - resource, ruleResp := e.invokeRuleHandler( - ctx, - logger, - handlerFactory, - policyContext, - matchedResource, - rule, - engineapi.ImageVerify, - ) - matchedResource = resource - for _, ruleResp := range ruleResp { - ruleResp := ruleResp - internal.AddRuleResponse(&resp, &ruleResp, startTime) - logger.V(4).Info("finished processing rule", "processingTime", ruleResp.Stats.ProcessingTime.String()) - } - if applyRules == kyvernov1.ApplyOne && resp.Stats.RulesAppliedCount > 0 { - break - } + for _, rule := range autogen.ComputeRules(policy) { + startTime := time.Now() + logger := internal.LoggerWithRule(logger, rule) + if !rule.HasVerifyImages() { + continue + } + handler := mutation.NewMutateImageHandler( + e.configuration, + e.rclient, + &ivm, + ) + resource, ruleResp := e.invokeRuleHandler( + ctx, + logger, + handler, + policyContext, + matchedResource, + rule, + engineapi.ImageVerify, + ) + matchedResource = resource + for _, ruleResp := range ruleResp { + ruleResp := ruleResp + internal.AddRuleResponse(&resp, &ruleResp, startTime) + logger.V(4).Info("finished processing rule", "processingTime", ruleResp.Stats.ProcessingTime.String()) + } + if applyRules == kyvernov1.ApplyOne && resp.Stats.RulesAppliedCount > 0 { + break } } // TODO: i doesn't make sense to not return the patched resource here diff --git a/pkg/engine/imageVerify_test.go b/pkg/engine/image_verify_test.go similarity index 100% rename from pkg/engine/imageVerify_test.go rename to pkg/engine/image_verify_test.go diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 62e45cbe32..11f5078917 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -22,26 +22,26 @@ func (e *engine) mutate( resp := engineapi.NewPolicyResponse() policy := policyContext.Policy() matchedResource := policyContext.NewResource() + applyRules := policy.GetSpec().GetApplyRules() policyContext.JSONContext().Checkpoint() defer policyContext.JSONContext().Restore() - applyRules := policy.GetSpec().GetApplyRules() - for _, rule := range autogen.ComputeRules(policy) { startTime := time.Now() logger := internal.LoggerWithRule(logger, rule) if !rule.HasMutate() { continue } - handlerFactory := handlers.WithHandler(e.mutateResourceHandler) + var handler handlers.Handler + handler = e.mutateResourceHandler if !policyContext.AdmissionOperation() && rule.IsMutateExisting() { - handlerFactory = handlers.WithHandler(e.mutateExistingHandler) + handler = e.mutateExistingHandler } resource, ruleResp := e.invokeRuleHandler( ctx, logger, - handlerFactory, + handler, policyContext, matchedResource, rule, diff --git a/pkg/engine/policyContext.go b/pkg/engine/policy_context.go similarity index 100% rename from pkg/engine/policyContext.go rename to pkg/engine/policy_context.go diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index cd7e88b393..3771329dcf 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -9,7 +9,6 @@ import ( "github.com/kyverno/kyverno/pkg/autogen" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/engine/handlers" - "github.com/kyverno/kyverno/pkg/engine/handlers/validation" "github.com/kyverno/kyverno/pkg/engine/internal" ) @@ -19,56 +18,49 @@ func (e *engine) validate( policyContext engineapi.PolicyContext, ) engineapi.PolicyResponse { resp := engineapi.NewPolicyResponse() + policy := policyContext.Policy() + matchedResource := policyContext.NewResource() + applyRules := policy.GetSpec().GetApplyRules() policyContext.JSONContext().Checkpoint() defer policyContext.JSONContext().Restore() - applyRules := policyContext.Policy().GetSpec().GetApplyRules() - - for _, rule := range autogen.ComputeRules(policyContext.Policy()) { - logger := internal.LoggerWithRule(logger, rule) + for _, rule := range autogen.ComputeRules(policy) { startTime := time.Now() + logger := internal.LoggerWithRule(logger, rule) hasValidate := rule.HasValidate() hasVerifyImageChecks := rule.HasVerifyImageChecks() if !hasValidate && !hasVerifyImageChecks { continue } - var handlerFactory handlers.HandlerFactory + var handler handlers.Handler if hasValidate { hasVerifyManifest := rule.HasVerifyManifests() hasValidatePss := rule.HasValidatePodSecurity() if hasVerifyManifest { - handlerFactory = handlers.WithHandler(e.validateManifestHandler) + handler = e.validateManifestHandler } else if hasValidatePss { - handlerFactory = handlers.WithHandler(e.validatePssHandler) + handler = e.validatePssHandler } else { - handlerFactory = handlers.WithHandler(e.validateResourceHandler) + handler = e.validateResourceHandler } } else if hasVerifyImageChecks { - handlerFactory = func() (handlers.Handler, error) { - return validation.NewValidateImageHandler( - policyContext, - policyContext.NewResource(), - rule, - e.configuration, - ) - } + handler = e.validateImageHandler } - if handlerFactory != nil { - _, ruleResp := e.invokeRuleHandler( - ctx, - logger, - handlerFactory, - policyContext, - policyContext.NewResource(), - rule, - engineapi.Validation, - ) - for _, ruleResp := range ruleResp { - ruleResp := ruleResp - internal.AddRuleResponse(&resp, &ruleResp, startTime) - logger.V(4).Info("finished processing rule", "processingTime", ruleResp.Stats.ProcessingTime.String()) - } + resource, ruleResp := e.invokeRuleHandler( + ctx, + logger, + handler, + policyContext, + matchedResource, + rule, + engineapi.Validation, + ) + matchedResource = resource + for _, ruleResp := range ruleResp { + ruleResp := ruleResp + internal.AddRuleResponse(&resp, &ruleResp, startTime) + logger.V(4).Info("finished processing rule", "processingTime", ruleResp.Stats.ProcessingTime.String()) } if applyRules == kyvernov1.ApplyOne && resp.Stats.RulesAppliedCount > 0 { break