From 9e5f19b8998be21fe89a6e86e00ae5b231588807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 22 Mar 2023 16:29:42 +0100 Subject: [PATCH] fix: do not create UR for dryrun admission requests (#6649) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- cmd/cli/kubectl-kyverno/main.go | 1 - pkg/utils/admission/dryrun.go | 9 ++++ pkg/utils/admission/dryrun_test.go | 46 +++++++++++++++++++ pkg/webhooks/handlers/trace.go | 3 +- pkg/webhooks/resource/handlers.go | 5 +- .../resource/imageverification/handler.go | 25 +++++----- .../resource/validation/validation.go | 2 +- 7 files changed, 74 insertions(+), 17 deletions(-) create mode 100644 pkg/utils/admission/dryrun.go create mode 100644 pkg/utils/admission/dryrun_test.go diff --git a/cmd/cli/kubectl-kyverno/main.go b/cmd/cli/kubectl-kyverno/main.go index 2ac62f6895..5a067eb04a 100644 --- a/cmd/cli/kubectl-kyverno/main.go +++ b/cmd/cli/kubectl-kyverno/main.go @@ -18,7 +18,6 @@ import ( const EnableExperimentalEnv = "KYVERNO_EXPERIMENTAL" -// CLI ... func main() { cli := &cobra.Command{ Use: "kyverno", diff --git a/pkg/utils/admission/dryrun.go b/pkg/utils/admission/dryrun.go new file mode 100644 index 0000000000..a5b95320c2 --- /dev/null +++ b/pkg/utils/admission/dryrun.go @@ -0,0 +1,9 @@ +package admission + +import ( + admissionv1 "k8s.io/api/admission/v1" +) + +func IsDryRun(request *admissionv1.AdmissionRequest) bool { + return request.DryRun != nil && *request.DryRun +} diff --git a/pkg/utils/admission/dryrun_test.go b/pkg/utils/admission/dryrun_test.go new file mode 100644 index 0000000000..ff68aeb06c --- /dev/null +++ b/pkg/utils/admission/dryrun_test.go @@ -0,0 +1,46 @@ +package admission + +import ( + "testing" + + admissionv1 "k8s.io/api/admission/v1" +) + +func TestIsDryRun(t *testing.T) { + true := true + false := false + type args struct { + request *admissionv1.AdmissionRequest + } + tests := []struct { + name string + args args + want bool + }{{ + args: args{ + request: &admissionv1.AdmissionRequest{}, + }, + want: false, + }, { + args: args{ + request: &admissionv1.AdmissionRequest{ + DryRun: &true, + }, + }, + want: true, + }, { + args: args{ + request: &admissionv1.AdmissionRequest{ + DryRun: &false, + }, + }, + want: false, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsDryRun(tt.args.request); got != tt.want { + t.Errorf("IsDryRun() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/webhooks/handlers/trace.go b/pkg/webhooks/handlers/trace.go index a178425a99..f2afc54818 100644 --- a/pkg/webhooks/handlers/trace.go +++ b/pkg/webhooks/handlers/trace.go @@ -8,6 +8,7 @@ import ( "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/tracing" + admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" semconv "go.opentelemetry.io/otel/semconv/v1.17.0" "go.opentelemetry.io/otel/trace" admissionv1 "k8s.io/api/admission/v1" @@ -68,7 +69,7 @@ func (inner AdmissionHandler) WithTrace(name string) AdmissionHandler { tracing.RequestNamespaceKey.String(tracing.StringValue(request.Namespace)), tracing.RequestUidKey.String(tracing.StringValue(string(request.UID))), tracing.RequestOperationKey.String(tracing.StringValue(string(request.Operation))), - tracing.RequestDryRunKey.Bool(request.DryRun != nil && *request.DryRun), + tracing.RequestDryRunKey.Bool(admissionutils.IsDryRun(request)), tracing.RequestKindGroupKey.String(tracing.StringValue(request.Kind.Group)), tracing.RequestKindVersionKey.String(tracing.StringValue(request.Kind.Version)), tracing.RequestKindKindKey.String(tracing.StringValue(request.Kind.Kind)), diff --git a/pkg/webhooks/resource/handlers.go b/pkg/webhooks/resource/handlers.go index 3c514ca035..a81e73da4c 100644 --- a/pkg/webhooks/resource/handlers.go +++ b/pkg/webhooks/resource/handlers.go @@ -137,8 +137,9 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request *ad logger.Info("admission request denied") return admissionutils.Response(request.UID, errors.New(msg), warnings...) } - - go h.handleBackgroundApplies(ctx, logger, request, policyContext, generatePolicies, mutatePolicies, startTime) + if !admissionutils.IsDryRun(request) { + go h.handleBackgroundApplies(ctx, logger, request, policyContext, generatePolicies, mutatePolicies, startTime) + } return admissionutils.ResponseSuccess(request.UID, warnings...) } diff --git a/pkg/webhooks/resource/imageverification/handler.go b/pkg/webhooks/resource/imageverification/handler.go index 43f4a99b3d..dc477f5ee3 100644 --- a/pkg/webhooks/resource/imageverification/handler.go +++ b/pkg/webhooks/resource/imageverification/handler.go @@ -13,6 +13,7 @@ import ( engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/event" "github.com/kyverno/kyverno/pkg/tracing" + admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" jsonutils "github.com/kyverno/kyverno/pkg/utils/json" reportutils "github.com/kyverno/kyverno/pkg/utils/report" webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils" @@ -149,30 +150,30 @@ func (v *imageVerificationHandler) handleAudit( namespaceLabels map[string]string, engineResponses ...*engineapi.EngineResponse, ) { - if !v.admissionReports { - return - } - if request.DryRun != nil && *request.DryRun { - return + createReport := v.admissionReports + if admissionutils.IsDryRun(request) { + createReport = false } // we don't need reports for deletions and when it's about sub resources if request.Operation == admissionv1.Delete || request.SubResource != "" { - return + createReport = false } // check if the resource supports reporting if !reportutils.IsGvkSupported(schema.GroupVersionKind(request.Kind)) { - return + createReport = false } tracing.Span( context.Background(), "", fmt.Sprintf("AUDIT %s %s", request.Operation, request.Kind), func(ctx context.Context, span trace.Span) { - report := reportutils.BuildAdmissionReport(resource, request, engineResponses...) - if len(report.GetResults()) > 0 { - _, err := reportutils.CreateReport(context.Background(), report, v.kyvernoClient) - if err != nil { - v.log.Error(err, "failed to create report") + if createReport { + report := reportutils.BuildAdmissionReport(resource, request, engineResponses...) + if len(report.GetResults()) > 0 { + _, err := reportutils.CreateReport(context.Background(), report, v.kyvernoClient) + if err != nil { + v.log.Error(err, "failed to create report") + } } } }, diff --git a/pkg/webhooks/resource/validation/validation.go b/pkg/webhooks/resource/validation/validation.go index 02a5e8b337..1bf55b2318 100644 --- a/pkg/webhooks/resource/validation/validation.go +++ b/pkg/webhooks/resource/validation/validation.go @@ -180,7 +180,7 @@ func (v *validationHandler) handleAudit( engineResponses ...*engineapi.EngineResponse, ) { createReport := v.admissionReports - if request.DryRun != nil && *request.DryRun { + if admissionutils.IsDryRun(request) { createReport = false } // we don't need reports for deletions