From c9bbf3819140469f8548fb83cefb88663558dbee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 4 Apr 2023 07:11:18 +0200 Subject: [PATCH] refactor: remove more admission request pointers (#6774) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- .../handlers/admission/handlers.go | 12 +-- cmd/cleanup-controller/server.go | 3 +- pkg/background/common/context.go | 4 +- pkg/background/generate/generate.go | 2 +- pkg/engine/context/context.go | 4 +- pkg/engine/context/evaluate_test.go | 8 +- .../validation/validate_manifest_test.go | 12 +-- pkg/engine/policycontext/policy_context.go | 4 +- pkg/engine/validation_test.go | 2 +- pkg/utils/admission/cleanup.go | 2 +- pkg/utils/admission/dryrun.go | 2 +- pkg/utils/admission/dryrun_test.go | 8 +- pkg/utils/admission/exception.go | 2 +- pkg/utils/admission/policy.go | 4 +- pkg/utils/admission/resource.go | 4 +- pkg/utils/admission/resource_test.go | 6 +- pkg/utils/report/new.go | 2 +- pkg/webhooks/exception/validate.go | 10 +-- pkg/webhooks/handlers/admission.go | 9 +- pkg/webhooks/handlers/dump.go | 16 ++-- pkg/webhooks/handlers/dump_test.go | 4 +- pkg/webhooks/handlers/filter.go | 6 +- pkg/webhooks/handlers/metrics.go | 3 +- pkg/webhooks/handlers/protect.go | 4 +- pkg/webhooks/handlers/trace.go | 7 +- pkg/webhooks/handlers/types.go | 9 +- pkg/webhooks/handlers/verify.go | 3 +- pkg/webhooks/policy/handlers.go | 12 +-- pkg/webhooks/resource/fake.go | 2 +- pkg/webhooks/resource/generation/handler.go | 14 +-- pkg/webhooks/resource/generation/utils.go | 4 +- pkg/webhooks/resource/handlers.go | 28 +++--- pkg/webhooks/resource/handlers_test.go | 89 ++++++++++--------- .../resource/imageverification/handler.go | 8 +- pkg/webhooks/resource/mutation/mutation.go | 8 +- pkg/webhooks/resource/updaterequest.go | 6 +- pkg/webhooks/resource/utils.go | 4 +- .../resource/validation/validation.go | 8 +- pkg/webhooks/server.go | 18 ++-- pkg/webhooks/utils/policy_context_builder.go | 4 +- 40 files changed, 185 insertions(+), 172 deletions(-) diff --git a/cmd/cleanup-controller/handlers/admission/handlers.go b/cmd/cleanup-controller/handlers/admission/handlers.go index d09a8aca45..e1fed09eb5 100644 --- a/cmd/cleanup-controller/handlers/admission/handlers.go +++ b/cmd/cleanup-controller/handlers/admission/handlers.go @@ -8,21 +8,21 @@ import ( "github.com/kyverno/kyverno/pkg/clients/dclient" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" validation "github.com/kyverno/kyverno/pkg/validation/cleanuppolicy" - admissionv1 "k8s.io/api/admission/v1" + "github.com/kyverno/kyverno/pkg/webhooks/handlers" ) -type handlers struct { +type clenaupHandlers struct { client dclient.Interface } -func New(client dclient.Interface) *handlers { - return &handlers{ +func New(client dclient.Interface) *clenaupHandlers { + return &clenaupHandlers{ client: client, } } -func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, _ time.Time) admissionv1.AdmissionResponse { - policy, _, err := admissionutils.GetCleanupPolicies(&request) +func (h *clenaupHandlers) Validate(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, _ time.Time) handlers.AdmissionResponse { + policy, _, err := admissionutils.GetCleanupPolicies(request.AdmissionRequest) if err != nil { logger.Error(err, "failed to unmarshal policies from admission request") return admissionutils.Response(request.UID, err) diff --git a/cmd/cleanup-controller/server.go b/cmd/cleanup-controller/server.go index 7e7ed0de08..a3b9e6f58a 100644 --- a/cmd/cleanup-controller/server.go +++ b/cmd/cleanup-controller/server.go @@ -14,7 +14,6 @@ import ( "github.com/kyverno/kyverno/pkg/metrics" "github.com/kyverno/kyverno/pkg/webhooks" "github.com/kyverno/kyverno/pkg/webhooks/handlers" - admissionv1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" ) @@ -31,7 +30,7 @@ type server struct { type ( TlsProvider = func() ([]byte, []byte, error) - ValidationHandler = func(context.Context, logr.Logger, admissionv1.AdmissionRequest, time.Time) admissionv1.AdmissionResponse + ValidationHandler = func(context.Context, logr.Logger, handlers.AdmissionRequest, time.Time) handlers.AdmissionResponse CleanupHandler = func(context.Context, logr.Logger, string, time.Time, config.Configuration) error ) diff --git a/pkg/background/common/context.go b/pkg/background/common/context.go index 6b0008f8ff..ded48ce225 100644 --- a/pkg/background/common/context.go +++ b/pkg/background/common/context.go @@ -26,11 +26,11 @@ func NewBackgroundContext(dclient dclient.Interface, ur *kyvernov1beta1.UpdateRe var err error if ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest != nil { - if err := ctx.AddRequest(ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest); err != nil { + if err := ctx.AddRequest(*ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest); err != nil { return nil, fmt.Errorf("failed to load request in context: %w", err) } - new, old, err = admissionutils.ExtractResources(nil, ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest) + new, old, err = admissionutils.ExtractResources(nil, *ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest) if err != nil { return nil, fmt.Errorf("failed to load request in context: %w", err) } diff --git a/pkg/background/generate/generate.go b/pkg/background/generate/generate.go index f1e84e39c2..905222523b 100644 --- a/pkg/background/generate/generate.go +++ b/pkg/background/generate/generate.go @@ -125,7 +125,7 @@ const doesNotApply = "policy does not apply to resource" func (c *GenerateController) getTrigger(spec kyvernov1beta1.UpdateRequestSpec) (*unstructured.Unstructured, error) { if spec.Context.AdmissionRequestInfo.Operation == admissionv1.Delete { request := spec.Context.AdmissionRequestInfo.AdmissionRequest - _, oldResource, err := admissionutils.ExtractResources(nil, request) + _, oldResource, err := admissionutils.ExtractResources(nil, *request) if err != nil { return nil, fmt.Errorf("failed to load resource from context: %w", err) } diff --git a/pkg/engine/context/context.go b/pkg/engine/context/context.go index 3fa1b49664..35ae3850f9 100644 --- a/pkg/engine/context/context.go +++ b/pkg/engine/context/context.go @@ -33,7 +33,7 @@ type EvalInterface interface { // Interface to manage context operations type Interface interface { // AddRequest marshals and adds the admission request to the context - AddRequest(request *admissionv1.AdmissionRequest) error + AddRequest(request admissionv1.AdmissionRequest) error // AddVariable adds a variable to the context AddVariable(key string, value interface{}) error @@ -131,7 +131,7 @@ func (ctx *context) addJSON(dataRaw []byte) error { } // AddRequest adds an admission request to context -func (ctx *context) AddRequest(request *admissionv1.AdmissionRequest) error { +func (ctx *context) AddRequest(request admissionv1.AdmissionRequest) error { return addToContext(ctx, request, "request") } diff --git a/pkg/engine/context/evaluate_test.go b/pkg/engine/context/evaluate_test.go index 9816cf5ae2..278238e6de 100644 --- a/pkg/engine/context/evaluate_test.go +++ b/pkg/engine/context/evaluate_test.go @@ -27,7 +27,7 @@ func TestHasChanged(t *testing.T) { } func TestRequestNotInitialize(t *testing.T) { - request := &admissionv1.AdmissionRequest{} + request := admissionv1.AdmissionRequest{} ctx := NewContext() ctx.AddRequest(request) @@ -36,7 +36,7 @@ func TestRequestNotInitialize(t *testing.T) { } func TestMissingOldObject(t *testing.T) { - request := &admissionv1.AdmissionRequest{} + request := admissionv1.AdmissionRequest{} ctx := NewContext() ctx.AddRequest(request) request.Object.Raw = []byte(`{"a": {"b": 1, "c": 2}, "d": 3}`) @@ -46,7 +46,7 @@ func TestMissingOldObject(t *testing.T) { } func TestMissingObject(t *testing.T) { - request := &admissionv1.AdmissionRequest{} + request := admissionv1.AdmissionRequest{} ctx := NewContext() ctx.AddRequest(request) request.OldObject.Raw = []byte(`{"a": {"b": 1, "c": 2}, "d": 3}`) @@ -56,7 +56,7 @@ func TestMissingObject(t *testing.T) { } func createTestContext(obj, oldObj string) Interface { - request := &admissionv1.AdmissionRequest{} + request := admissionv1.AdmissionRequest{} request.Operation = "UPDATE" request.Object.Raw = []byte(obj) request.OldObject.Raw = []byte(oldObj) diff --git a/pkg/engine/handlers/validation/validate_manifest_test.go b/pkg/engine/handlers/validation/validate_manifest_test.go index 778ae811ef..eb9ba3c155 100644 --- a/pkg/engine/handlers/validation/validate_manifest_test.go +++ b/pkg/engine/handlers/validation/validate_manifest_test.go @@ -624,7 +624,7 @@ var ( func Test_VerifyManifest_SignedYAML(t *testing.T) { policyContext := buildContext(t, test_policy, signed_resource, "") - var request *v1.AdmissionRequest + var request v1.AdmissionRequest _ = json.Unmarshal([]byte(signed_adreq), &request) policyContext.JSONContext().AddRequest(request) policyContext.Policy().SetName("test-policy") @@ -646,7 +646,7 @@ func Test_VerifyManifest_SignedYAML(t *testing.T) { func Test_VerifyManifest_UnsignedYAML(t *testing.T) { policyContext := buildContext(t, test_policy, unsigned_resource, "") - var request *v1.AdmissionRequest + var request v1.AdmissionRequest _ = json.Unmarshal([]byte(unsigned_adreq), &request) policyContext.JSONContext().AddRequest(request) policyContext.Policy().SetName("test-policy") @@ -668,7 +668,7 @@ func Test_VerifyManifest_UnsignedYAML(t *testing.T) { func Test_VerifyManifest_InvalidYAML(t *testing.T) { policyContext := buildContext(t, test_policy, invalid_resource, "") - var request *v1.AdmissionRequest + var request v1.AdmissionRequest _ = json.Unmarshal([]byte(invalid_adreq), &request) policyContext.JSONContext().AddRequest(request) policyContext.Policy().SetName("test-policy") @@ -690,7 +690,7 @@ func Test_VerifyManifest_InvalidYAML(t *testing.T) { func Test_VerifyManifest_MustAll_InvalidYAML(t *testing.T) { policyContext := buildContext(t, test_policy, multi_sig_resource, "") - var request *v1.AdmissionRequest + var request v1.AdmissionRequest _ = json.Unmarshal([]byte(multi_sig_adreq), &request) policyContext.JSONContext().AddRequest(request) policyContext.Policy().SetName("test-policy") @@ -718,7 +718,7 @@ func Test_VerifyManifest_MustAll_InvalidYAML(t *testing.T) { func Test_VerifyManifest_MustAll_ValidYAML(t *testing.T) { policyContext := buildContext(t, test_policy, multi_sig2_resource, "") - var request *v1.AdmissionRequest + var request v1.AdmissionRequest _ = json.Unmarshal([]byte(multi_sig2_adreq), &request) policyContext.JSONContext().AddRequest(request) policyContext.Policy().SetName("test-policy") @@ -750,7 +750,7 @@ func Test_VerifyManifest_MustAll_ValidYAML(t *testing.T) { func Test_VerifyManifest_AtLeastOne(t *testing.T) { policyContext := buildContext(t, test_policy, multi_sig_resource, "") - var request *v1.AdmissionRequest + var request v1.AdmissionRequest _ = json.Unmarshal([]byte(multi_sig_adreq), &request) policyContext.JSONContext().AddRequest(request) policyContext.Policy().SetName("test-policy") diff --git a/pkg/engine/policycontext/policy_context.go b/pkg/engine/policycontext/policy_context.go index fda257ea66..2fbb5ba628 100644 --- a/pkg/engine/policycontext/policy_context.go +++ b/pkg/engine/policycontext/policy_context.go @@ -191,7 +191,7 @@ func NewPolicyContext(operation kyvernov1.AdmissionOperation) *PolicyContext { func NewPolicyContextFromAdmissionRequest( client dclient.IDiscovery, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, admissionInfo kyvernov1beta1.RequestInfo, configuration config.Configuration, ) (*PolicyContext, error) { @@ -220,7 +220,7 @@ func NewPolicyContextFromAdmissionRequest( return policyContext, nil } -func newVariablesContext(request *admissionv1.AdmissionRequest, userRequestInfo *kyvernov1beta1.RequestInfo) (enginectx.Interface, error) { +func newVariablesContext(request admissionv1.AdmissionRequest, userRequestInfo *kyvernov1beta1.RequestInfo) (enginectx.Interface, error) { ctx := enginectx.NewContext() if err := ctx.AddRequest(request); err != nil { return nil, fmt.Errorf("failed to load incoming request in context: %w", err) diff --git a/pkg/engine/validation_test.go b/pkg/engine/validation_test.go index 9f0d26f430..a7fd7954a8 100644 --- a/pkg/engine/validation_test.go +++ b/pkg/engine/validation_test.go @@ -2082,7 +2082,7 @@ func executeTest(t *testing.T, test testCase) { t.Fatal(err) } - var request *admissionv1.AdmissionRequest + var request admissionv1.AdmissionRequest err = json.Unmarshal(test.request, &request) if err != nil { t.Fatal(err) diff --git a/pkg/utils/admission/cleanup.go b/pkg/utils/admission/cleanup.go index c532f162d4..6985787bdb 100644 --- a/pkg/utils/admission/cleanup.go +++ b/pkg/utils/admission/cleanup.go @@ -25,7 +25,7 @@ func UnmarshalCleanupPolicy(kind string, raw []byte) (kyvernov2alpha1.CleanupPol return nil, fmt.Errorf("admission request does not contain a cleanuppolicy") } -func GetCleanupPolicies(request *admissionv1.AdmissionRequest) (kyvernov2alpha1.CleanupPolicyInterface, kyvernov2alpha1.CleanupPolicyInterface, error) { +func GetCleanupPolicies(request admissionv1.AdmissionRequest) (kyvernov2alpha1.CleanupPolicyInterface, kyvernov2alpha1.CleanupPolicyInterface, error) { var emptypolicy kyvernov2alpha1.CleanupPolicyInterface policy, err := UnmarshalCleanupPolicy(request.Kind.Kind, request.Object.Raw) if err != nil { diff --git a/pkg/utils/admission/dryrun.go b/pkg/utils/admission/dryrun.go index a5b95320c2..aff3bbc0f4 100644 --- a/pkg/utils/admission/dryrun.go +++ b/pkg/utils/admission/dryrun.go @@ -4,6 +4,6 @@ import ( admissionv1 "k8s.io/api/admission/v1" ) -func IsDryRun(request *admissionv1.AdmissionRequest) bool { +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 index ff68aeb06c..2218bd0fee 100644 --- a/pkg/utils/admission/dryrun_test.go +++ b/pkg/utils/admission/dryrun_test.go @@ -10,7 +10,7 @@ func TestIsDryRun(t *testing.T) { true := true false := false type args struct { - request *admissionv1.AdmissionRequest + request admissionv1.AdmissionRequest } tests := []struct { name string @@ -18,19 +18,19 @@ func TestIsDryRun(t *testing.T) { want bool }{{ args: args{ - request: &admissionv1.AdmissionRequest{}, + request: admissionv1.AdmissionRequest{}, }, want: false, }, { args: args{ - request: &admissionv1.AdmissionRequest{ + request: admissionv1.AdmissionRequest{ DryRun: &true, }, }, want: true, }, { args: args{ - request: &admissionv1.AdmissionRequest{ + request: admissionv1.AdmissionRequest{ DryRun: &false, }, }, diff --git a/pkg/utils/admission/exception.go b/pkg/utils/admission/exception.go index 197019d4d1..6cff5ed662 100644 --- a/pkg/utils/admission/exception.go +++ b/pkg/utils/admission/exception.go @@ -15,7 +15,7 @@ func UnmarshalPolicyException(raw []byte) (*kyvernov2alpha1.PolicyException, err return exception, nil } -func GetPolicyExceptions(request *admissionv1.AdmissionRequest) (*kyvernov2alpha1.PolicyException, *kyvernov2alpha1.PolicyException, error) { +func GetPolicyExceptions(request admissionv1.AdmissionRequest) (*kyvernov2alpha1.PolicyException, *kyvernov2alpha1.PolicyException, error) { var empty *kyvernov2alpha1.PolicyException exception, err := UnmarshalPolicyException(request.Object.Raw) if err != nil { diff --git a/pkg/utils/admission/policy.go b/pkg/utils/admission/policy.go index 6327d78869..1f0ccf94e2 100644 --- a/pkg/utils/admission/policy.go +++ b/pkg/utils/admission/policy.go @@ -25,11 +25,11 @@ func UnmarshalPolicy(kind string, raw []byte) (kyvernov1.PolicyInterface, error) return nil, fmt.Errorf("admission request does not contain a policy") } -func GetPolicy(request *admissionv1.AdmissionRequest) (kyvernov1.PolicyInterface, error) { +func GetPolicy(request admissionv1.AdmissionRequest) (kyvernov1.PolicyInterface, error) { return UnmarshalPolicy(request.Kind.Kind, request.Object.Raw) } -func GetPolicies(request *admissionv1.AdmissionRequest) (kyvernov1.PolicyInterface, kyvernov1.PolicyInterface, error) { +func GetPolicies(request admissionv1.AdmissionRequest) (kyvernov1.PolicyInterface, kyvernov1.PolicyInterface, error) { policy, err := UnmarshalPolicy(request.Kind.Kind, request.Object.Raw) if err != nil { return policy, nil, err diff --git a/pkg/utils/admission/resource.go b/pkg/utils/admission/resource.go index 1630d57e6a..b93316fd86 100644 --- a/pkg/utils/admission/resource.go +++ b/pkg/utils/admission/resource.go @@ -9,7 +9,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -func GetResourceName(request *admissionv1.AdmissionRequest) string { +func GetResourceName(request admissionv1.AdmissionRequest) string { resourceName := request.Kind.Kind + "/" + request.Name if request.Namespace != "" { resourceName = request.Namespace + "/" + resourceName @@ -18,7 +18,7 @@ func GetResourceName(request *admissionv1.AdmissionRequest) string { } // ExtractResources extracts the new and old resource as unstructured -func ExtractResources(newRaw []byte, request *admissionv1.AdmissionRequest) (unstructured.Unstructured, unstructured.Unstructured, error) { +func ExtractResources(newRaw []byte, request admissionv1.AdmissionRequest) (unstructured.Unstructured, unstructured.Unstructured, error) { var emptyResource unstructured.Unstructured var newResource unstructured.Unstructured var oldResource unstructured.Unstructured diff --git a/pkg/utils/admission/resource_test.go b/pkg/utils/admission/resource_test.go index 05ee4cb76d..6c7eae3282 100644 --- a/pkg/utils/admission/resource_test.go +++ b/pkg/utils/admission/resource_test.go @@ -10,7 +10,7 @@ import ( func TestGetResourceName(t *testing.T) { type args struct { - request *admissionv1.AdmissionRequest + request admissionv1.AdmissionRequest } tests := []struct { name string @@ -19,7 +19,7 @@ func TestGetResourceName(t *testing.T) { }{{ name: "with namespace", args: args{ - request: &admissionv1.AdmissionRequest{ + request: admissionv1.AdmissionRequest{ Kind: v1.GroupVersionKind{ Kind: "Pod", }, @@ -31,7 +31,7 @@ func TestGetResourceName(t *testing.T) { }, { name: "without namespace", args: args{ - request: &admissionv1.AdmissionRequest{ + request: admissionv1.AdmissionRequest{ Kind: v1.GroupVersionKind{ Kind: "Namespace", }, diff --git a/pkg/utils/report/new.go b/pkg/utils/report/new.go index b5bd2859e6..8bfb1a246a 100644 --- a/pkg/utils/report/new.go +++ b/pkg/utils/report/new.go @@ -27,7 +27,7 @@ func NewAdmissionReport(namespace, name string, gvr schema.GroupVersionResource, return report } -func BuildAdmissionReport(resource unstructured.Unstructured, request *admissionv1.AdmissionRequest, responses ...engineapi.EngineResponse) kyvernov1alpha2.ReportInterface { +func BuildAdmissionReport(resource unstructured.Unstructured, request admissionv1.AdmissionRequest, responses ...engineapi.EngineResponse) kyvernov1alpha2.ReportInterface { report := NewAdmissionReport(resource.GetNamespace(), string(request.UID), schema.GroupVersionResource(request.Resource), resource) SetResponses(report, responses...) return report diff --git a/pkg/webhooks/exception/validate.go b/pkg/webhooks/exception/validate.go index 70e1a60756..3a1d3a40fa 100644 --- a/pkg/webhooks/exception/validate.go +++ b/pkg/webhooks/exception/validate.go @@ -8,22 +8,22 @@ import ( admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" validation "github.com/kyverno/kyverno/pkg/validation/exception" "github.com/kyverno/kyverno/pkg/webhooks" - admissionv1 "k8s.io/api/admission/v1" + "github.com/kyverno/kyverno/pkg/webhooks/handlers" ) -type handlers struct { +type exceptionHandlers struct { validationOptions validation.ValidationOptions } func NewHandlers(validationOptions validation.ValidationOptions) webhooks.ExceptionHandlers { - return &handlers{ + return &exceptionHandlers{ validationOptions: validationOptions, } } // Validate performs the validation check on policy exception resources -func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { - polex, _, err := admissionutils.GetPolicyExceptions(&request) +func (h *exceptionHandlers) Validate(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, startTime time.Time) handlers.AdmissionResponse { + polex, _, err := admissionutils.GetPolicyExceptions(request.AdmissionRequest) if err != nil { logger.Error(err, "failed to unmarshal policy exceptions from admission request") return admissionutils.Response(request.UID, err) diff --git a/pkg/webhooks/handlers/admission.go b/pkg/webhooks/handlers/admission.go index 937f0ab1bc..b466667f74 100644 --- a/pkg/webhooks/handlers/admission.go +++ b/pkg/webhooks/handlers/admission.go @@ -33,7 +33,7 @@ func (inner AdmissionHandler) withAdmission(logger logr.Logger) HttpHandler { HttpError(request.Context(), writer, request, logger, errors.New("invalid Content-Type"), http.StatusUnsupportedMediaType) return } - admissionReview := &admissionv1.AdmissionReview{} + var admissionReview admissionv1.AdmissionReview if err := json.Unmarshal(body, &admissionReview); err != nil { HttpError(request.Context(), writer, request, logger, err, http.StatusExpectationFailed) return @@ -51,8 +51,11 @@ func (inner AdmissionHandler) withAdmission(logger logr.Logger) HttpHandler { Allowed: true, UID: admissionReview.Request.UID, } - // TODO: check request is not nil ? - admissionResponse := inner(request.Context(), logger, *admissionReview.Request, startTime) + admissionRequest := AdmissionRequest{ + AdmissionRequest: *admissionReview.Request, + // TODO: roles/clusterroles + } + admissionResponse := inner(request.Context(), logger, admissionRequest, startTime) admissionReview.Response = &admissionResponse responseJSON, err := json.Marshal(admissionReview) if err != nil { diff --git a/pkg/webhooks/handlers/dump.go b/pkg/webhooks/handlers/dump.go index b2e5d57e64..cec42ff568 100644 --- a/pkg/webhooks/handlers/dump.go +++ b/pkg/webhooks/handlers/dump.go @@ -32,9 +32,9 @@ func (inner AdmissionHandler) withDump( rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, ) AdmissionHandler { - return func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + return func(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { response := inner(ctx, logger, request, startTime) - dumpPayload(logger, rbLister, crbLister, &request, &response) + dumpPayload(logger, rbLister, crbLister, request.AdmissionRequest, response) return response } } @@ -43,17 +43,15 @@ func dumpPayload( logger logr.Logger, rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, - request *admissionv1.AdmissionRequest, - response *admissionv1.AdmissionResponse, + request admissionv1.AdmissionRequest, + response AdmissionResponse, ) { reqPayload, err := newAdmissionRequestPayload(request, rbLister, crbLister) if err != nil { logger.Error(err, "Failed to extract resources") } else { - if response != nil { - logger = logger.WithValues("AdmissionResponse", *response) - } - logger.Info("Logging admission request and response payload ", "AdmissionRequest", reqPayload) + logger = logger.WithValues("AdmissionResponse", response, "AdmissionRequest", reqPayload) + logger.Info("Logging admission request and response payload ") } } @@ -79,7 +77,7 @@ type admissionRequestPayload struct { } func newAdmissionRequestPayload( - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, ) (*admissionRequestPayload, error) { diff --git a/pkg/webhooks/handlers/dump_test.go b/pkg/webhooks/handlers/dump_test.go index bc1d998414..58e2cb09e0 100644 --- a/pkg/webhooks/handlers/dump_test.go +++ b/pkg/webhooks/handlers/dump_test.go @@ -138,8 +138,8 @@ func Test_RedactPayload(t *testing.T) { for _, c := range tc { t.Run(c.name, func(t *testing.T) { - req := new(admissionv1.AdmissionRequest) - err := json.Unmarshal(c.requestPayload, req) + var req admissionv1.AdmissionRequest + err := json.Unmarshal(c.requestPayload, &req) assert.NilError(t, err) payload, err := newAdmissionRequestPayload(req, nil, nil) assert.NilError(t, err) diff --git a/pkg/webhooks/handlers/filter.go b/pkg/webhooks/handlers/filter.go index 7a20513aeb..80e167880e 100644 --- a/pkg/webhooks/handlers/filter.go +++ b/pkg/webhooks/handlers/filter.go @@ -26,7 +26,7 @@ func (inner AdmissionHandler) WithSubResourceFilter(subresources ...string) Admi } func (inner AdmissionHandler) withFilter(c config.Configuration) AdmissionHandler { - return func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + return func(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { // filter by username for _, username := range c.GetExcludedUsernames() { if wildcard.Match(username, request.UserInfo.Username) { @@ -58,7 +58,7 @@ func (inner AdmissionHandler) withOperationFilter(operations ...admissionv1.Oper for _, operation := range operations { allowed.Insert(string(operation)) } - return func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + return func(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { if allowed.Has(string(request.Operation)) { return inner(ctx, logger, request, startTime) } @@ -68,7 +68,7 @@ func (inner AdmissionHandler) withOperationFilter(operations ...admissionv1.Oper func (inner AdmissionHandler) withSubResourceFilter(subresources ...string) AdmissionHandler { allowed := sets.New(subresources...) - return func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + return func(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { if request.SubResource == "" || allowed.Has(request.SubResource) { return inner(ctx, logger, request, startTime) } diff --git a/pkg/webhooks/handlers/metrics.go b/pkg/webhooks/handlers/metrics.go index 7977ce2a70..52a9f3fdc4 100644 --- a/pkg/webhooks/handlers/metrics.go +++ b/pkg/webhooks/handlers/metrics.go @@ -13,7 +13,6 @@ import ( "go.opentelemetry.io/otel/metric/global" "go.opentelemetry.io/otel/metric/instrument" semconv "go.opentelemetry.io/otel/semconv/v1.17.0" - admissionv1 "k8s.io/api/admission/v1" ) func (inner AdmissionHandler) WithMetrics(logger logr.Logger, metricsConfig config.MetricsConfiguration, attrs ...attribute.KeyValue) AdmissionHandler { @@ -36,7 +35,7 @@ func (inner AdmissionHandler) withMetrics(logger logr.Logger, metricsConfig conf if err != nil { logger.Error(err, "Failed to create instrument, kyverno_admission_review_duration_seconds") } - return func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + return func(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { response := inner(ctx, logger, request, startTime) namespace := request.Namespace if metricsConfig.CheckNamespace(namespace) { diff --git a/pkg/webhooks/handlers/protect.go b/pkg/webhooks/handlers/protect.go index eff8c30cd9..75866fa1a4 100644 --- a/pkg/webhooks/handlers/protect.go +++ b/pkg/webhooks/handlers/protect.go @@ -24,12 +24,12 @@ func (inner AdmissionHandler) WithProtection(enabled bool) AdmissionHandler { } func (inner AdmissionHandler) withProtection() AdmissionHandler { - return func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + return func(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { // Allows deletion of namespace containing managed resources if request.Operation == admissionv1.Delete && request.UserInfo.Username == namespaceControllerUsername { return inner(ctx, logger, request, startTime) } - newResource, oldResource, err := admissionutils.ExtractResources(nil, &request) + newResource, oldResource, err := admissionutils.ExtractResources(nil, request.AdmissionRequest) if err != nil { logger.Error(err, "Failed to extract resources") return admissionutils.Response(request.UID, err) diff --git a/pkg/webhooks/handlers/trace.go b/pkg/webhooks/handlers/trace.go index 316dd3912f..a9b8c53b5a 100644 --- a/pkg/webhooks/handlers/trace.go +++ b/pkg/webhooks/handlers/trace.go @@ -11,7 +11,6 @@ import ( 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" ) func (inner HttpHandler) WithTrace(name string) HttpHandler { @@ -35,12 +34,12 @@ func (inner HttpHandler) WithTrace(name string) HttpHandler { } func (inner AdmissionHandler) WithTrace(name string) AdmissionHandler { - return func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + return func(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { return tracing.Span1( ctx, "webhooks/handlers", fmt.Sprintf("%s %s %s", name, request.Operation, request.Kind), - func(ctx context.Context, span trace.Span) admissionv1.AdmissionResponse { + func(ctx context.Context, span trace.Span) AdmissionResponse { response := inner(ctx, logger, request, startTime) span.SetAttributes( tracing.ResponseUidKey.String(tracing.StringValue(string(response.UID))), @@ -67,7 +66,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(admissionutils.IsDryRun(&request)), + tracing.RequestDryRunKey.Bool(admissionutils.IsDryRun(request.AdmissionRequest)), 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/handlers/types.go b/pkg/webhooks/handlers/types.go index f1665ca3de..584515542b 100644 --- a/pkg/webhooks/handlers/types.go +++ b/pkg/webhooks/handlers/types.go @@ -9,8 +9,15 @@ import ( admissionv1 "k8s.io/api/admission/v1" ) +type AdmissionRequest struct { + // AdmissionRequest is the original admission request. + admissionv1.AdmissionRequest +} + +type AdmissionResponse = admissionv1.AdmissionResponse + type ( - AdmissionHandler func(context.Context, logr.Logger, admissionv1.AdmissionRequest, time.Time) admissionv1.AdmissionResponse + AdmissionHandler func(context.Context, logr.Logger, AdmissionRequest, time.Time) AdmissionResponse HttpHandler func(http.ResponseWriter, *http.Request) ) diff --git a/pkg/webhooks/handlers/verify.go b/pkg/webhooks/handlers/verify.go index 63909d949f..a77ce7fd07 100644 --- a/pkg/webhooks/handlers/verify.go +++ b/pkg/webhooks/handlers/verify.go @@ -8,10 +8,9 @@ import ( "github.com/kyverno/kyverno/pkg/config" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" jsonutils "github.com/kyverno/kyverno/pkg/utils/json" - admissionv1 "k8s.io/api/admission/v1" ) -func Verify(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { +func Verify(ctx context.Context, logger logr.Logger, request AdmissionRequest, startTime time.Time) AdmissionResponse { if request.Name != "kyverno-health" || request.Namespace != config.KyvernoNamespace() { return admissionutils.ResponseSuccess(request.UID) } diff --git a/pkg/webhooks/policy/handlers.go b/pkg/webhooks/policy/handlers.go index b8060e83b2..4ab8d9106d 100644 --- a/pkg/webhooks/policy/handlers.go +++ b/pkg/webhooks/policy/handlers.go @@ -10,23 +10,23 @@ import ( policyvalidate "github.com/kyverno/kyverno/pkg/policy" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" "github.com/kyverno/kyverno/pkg/webhooks" - admissionv1 "k8s.io/api/admission/v1" + "github.com/kyverno/kyverno/pkg/webhooks/handlers" ) -type handlers struct { +type policyHandlers struct { client dclient.Interface openApiManager openapi.Manager } func NewHandlers(client dclient.Interface, openApiManager openapi.Manager) webhooks.PolicyHandlers { - return &handlers{ + return &policyHandlers{ client: client, openApiManager: openApiManager, } } -func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, _ time.Time) admissionv1.AdmissionResponse { - policy, oldPolicy, err := admissionutils.GetPolicies(&request) +func (h *policyHandlers) Validate(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, _ time.Time) handlers.AdmissionResponse { + policy, oldPolicy, err := admissionutils.GetPolicies(request.AdmissionRequest) if err != nil { logger.Error(err, "failed to unmarshal policies from admission request") return admissionutils.Response(request.UID, err) @@ -38,6 +38,6 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request adm return admissionutils.Response(request.UID, err, warnings...) } -func (h *handlers) Mutate(_ context.Context, _ logr.Logger, request admissionv1.AdmissionRequest, _ time.Time) admissionv1.AdmissionResponse { +func (h *policyHandlers) Mutate(_ context.Context, _ logr.Logger, request handlers.AdmissionRequest, _ time.Time) handlers.AdmissionResponse { return admissionutils.ResponseSuccess(request.UID) } diff --git a/pkg/webhooks/resource/fake.go b/pkg/webhooks/resource/fake.go index fe4984e1b6..882f2ffeff 100644 --- a/pkg/webhooks/resource/fake.go +++ b/pkg/webhooks/resource/fake.go @@ -42,7 +42,7 @@ func NewFakeHandlers(ctx context.Context, policyCache policycache.Cache) webhook peLister := kyvernoInformers.Kyverno().V2alpha1().PolicyExceptions().Lister() rclient := registryclient.NewOrDie() - return &handlers{ + return &resourceHandlers{ client: dclient, rclient: rclient, configuration: configuration, diff --git a/pkg/webhooks/resource/generation/handler.go b/pkg/webhooks/resource/generation/handler.go index 46025a3fc3..355c1645f0 100644 --- a/pkg/webhooks/resource/generation/handler.go +++ b/pkg/webhooks/resource/generation/handler.go @@ -25,7 +25,7 @@ import ( ) type GenerationHandler interface { - Handle(context.Context, *admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext) + Handle(context.Context, admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext) } func NewGenerationHandler( @@ -72,7 +72,7 @@ type generationHandler struct { func (h *generationHandler) Handle( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, ) { @@ -101,7 +101,7 @@ func getAppliedRules(policy kyvernov1.PolicyInterface, applied []engineapi.RuleR func (h *generationHandler) handleTrigger( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, ) { @@ -132,7 +132,7 @@ func (h *generationHandler) handleTrigger( func (h *generationHandler) handleNonTrigger( ctx context.Context, policyContext *engine.PolicyContext, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, ) { resource := policyContext.OldResource() labels := resource.GetLabels() @@ -146,7 +146,7 @@ func (h *generationHandler) handleNonTrigger( func (h *generationHandler) applyGeneration( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policy kyvernov1.PolicyInterface, appliedRules []engineapi.RuleResponse, policyContext *engine.PolicyContext, @@ -182,7 +182,7 @@ func (h *generationHandler) applyGeneration( // it can be 1. trigger deletion; 2. trigger no longer matches, when a rule fails func (h *generationHandler) syncTriggerAction( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policy kyvernov1.PolicyInterface, failedRules []engineapi.RuleResponse, policyContext *engine.PolicyContext, @@ -231,7 +231,7 @@ func (h *generationHandler) syncTriggerAction( } } -func (h *generationHandler) createUR(ctx context.Context, policyContext *engine.PolicyContext, request *admissionv1.AdmissionRequest) (err error) { +func (h *generationHandler) createUR(ctx context.Context, policyContext *engine.PolicyContext, request admissionv1.AdmissionRequest) (err error) { var policy kyvernov1.PolicyInterface new := policyContext.NewResource() labels := new.GetLabels() diff --git a/pkg/webhooks/resource/generation/utils.go b/pkg/webhooks/resource/generation/utils.go index e31c72a85b..3dad0a1985 100644 --- a/pkg/webhooks/resource/generation/utils.go +++ b/pkg/webhooks/resource/generation/utils.go @@ -19,11 +19,11 @@ func buildURSpec(requestType kyvernov1beta1.RequestType, policyKey, ruleName str } } -func buildURContext(request *admissionv1.AdmissionRequest, policyContext *engine.PolicyContext) kyvernov1beta1.UpdateRequestSpecContext { +func buildURContext(request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext) kyvernov1beta1.UpdateRequestSpecContext { return kyvernov1beta1.UpdateRequestSpecContext{ UserRequestInfo: policyContext.AdmissionInfo(), AdmissionRequestInfo: kyvernov1beta1.AdmissionRequestInfoObject{ - AdmissionRequest: request, + AdmissionRequest: &request, Operation: request.Operation, }, } diff --git a/pkg/webhooks/resource/handlers.go b/pkg/webhooks/resource/handlers.go index f1502c8d66..77a6daa2e2 100644 --- a/pkg/webhooks/resource/handlers.go +++ b/pkg/webhooks/resource/handlers.go @@ -23,18 +23,18 @@ import ( engineutils "github.com/kyverno/kyverno/pkg/utils/engine" jsonutils "github.com/kyverno/kyverno/pkg/utils/json" "github.com/kyverno/kyverno/pkg/webhooks" + "github.com/kyverno/kyverno/pkg/webhooks/handlers" "github.com/kyverno/kyverno/pkg/webhooks/resource/imageverification" "github.com/kyverno/kyverno/pkg/webhooks/resource/mutation" "github.com/kyverno/kyverno/pkg/webhooks/resource/validation" webhookgenerate "github.com/kyverno/kyverno/pkg/webhooks/updaterequest" webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils" - admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/runtime/schema" corev1listers "k8s.io/client-go/listers/core/v1" rbacv1listers "k8s.io/client-go/listers/rbac/v1" ) -type handlers struct { +type resourceHandlers struct { // clients client dclient.Interface kyvernoClient versioned.Interface @@ -81,7 +81,7 @@ func NewHandlers( openApiManager openapi.ValidateInterface, admissionReports bool, ) webhooks.ResourceHandlers { - return &handlers{ + return &resourceHandlers{ engine: engine, client: client, kyvernoClient: kyvernoClient, @@ -101,7 +101,7 @@ func NewHandlers( } } -func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, failurePolicy string, startTime time.Time) admissionv1.AdmissionResponse { +func (h *resourceHandlers) Validate(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, failurePolicy string, startTime time.Time) handlers.AdmissionResponse { kind := request.Kind.Kind logger = logger.WithValues("kind", kind) logger.V(4).Info("received an admission request in validating webhook") @@ -120,7 +120,7 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request adm logger.V(4).Info("processing policies for validate admission request", "validate", len(policies), "mutate", len(mutatePolicies), "generate", len(generatePolicies)) - policyContext, err := h.pcBuilder.Build(&request) + policyContext, err := h.pcBuilder.Build(request.AdmissionRequest) if err != nil { return errorResponse(logger, request.UID, err, "failed create policy context") } @@ -132,18 +132,18 @@ func (h *handlers) Validate(ctx context.Context, logger logr.Logger, request adm policyContext = policyContext.WithNamespaceLabels(namespaceLabels) vh := validation.NewValidationHandler(logger, h.kyvernoClient, h.engine, h.pCache, h.pcBuilder, h.eventGen, h.admissionReports, h.metricsConfig, h.configuration) - ok, msg, warnings := vh.HandleValidation(ctx, &request, policies, policyContext, startTime) + ok, msg, warnings := vh.HandleValidation(ctx, request.AdmissionRequest, policies, policyContext, startTime) if !ok { logger.Info("admission request denied") return admissionutils.Response(request.UID, errors.New(msg), warnings...) } - if !admissionutils.IsDryRun(&request) { - go h.handleBackgroundApplies(ctx, logger, &request, policyContext, generatePolicies, mutatePolicies, startTime) + if !admissionutils.IsDryRun(request.AdmissionRequest) { + go h.handleBackgroundApplies(ctx, logger, request.AdmissionRequest, policyContext, generatePolicies, mutatePolicies, startTime) } return admissionutils.ResponseSuccess(request.UID, warnings...) } -func (h *handlers) Mutate(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, failurePolicy string, startTime time.Time) admissionv1.AdmissionResponse { +func (h *resourceHandlers) Mutate(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, failurePolicy string, startTime time.Time) handlers.AdmissionResponse { kind := request.Kind.Kind logger = logger.WithValues("kind", kind) logger.V(4).Info("received an admission request in mutating webhook") @@ -155,26 +155,26 @@ func (h *handlers) Mutate(ctx context.Context, logger logr.Logger, request admis return admissionutils.ResponseSuccess(request.UID) } logger.V(4).Info("processing policies for mutate admission request", "mutatePolicies", len(mutatePolicies), "verifyImagesPolicies", len(verifyImagesPolicies)) - policyContext, err := h.pcBuilder.Build(&request) + policyContext, err := h.pcBuilder.Build(request.AdmissionRequest) if err != nil { logger.Error(err, "failed to build policy context") return admissionutils.Response(request.UID, err) } mh := mutation.NewMutationHandler(logger, h.engine, h.eventGen, h.openApiManager, h.nsLister, h.metricsConfig) - mutatePatches, mutateWarnings, err := mh.HandleMutation(ctx, &request, mutatePolicies, policyContext, startTime) + mutatePatches, mutateWarnings, err := mh.HandleMutation(ctx, request.AdmissionRequest, mutatePolicies, policyContext, startTime) if err != nil { logger.Error(err, "mutation failed") return admissionutils.Response(request.UID, err) } - newRequest := patchRequest(mutatePatches, request, logger) + newRequest := patchRequest(mutatePatches, request.AdmissionRequest, logger) // rebuild context to process images updated via mutate policies - policyContext, err = h.pcBuilder.Build(&newRequest) + policyContext, err = h.pcBuilder.Build(newRequest) if err != nil { logger.Error(err, "failed to build policy context") return admissionutils.Response(request.UID, err) } ivh := imageverification.NewImageVerificationHandler(logger, h.kyvernoClient, h.engine, h.eventGen, h.admissionReports, h.configuration) - imagePatches, imageVerifyWarnings, err := ivh.Handle(ctx, &newRequest, verifyImagesPolicies, policyContext) + imagePatches, imageVerifyWarnings, err := ivh.Handle(ctx, newRequest, verifyImagesPolicies, policyContext) if err != nil { logger.Error(err, "image verification failed") return admissionutils.Response(request.UID, err) diff --git a/pkg/webhooks/resource/handlers_test.go b/pkg/webhooks/resource/handlers_test.go index a46d88f381..9b802b29e0 100644 --- a/pkg/webhooks/resource/handlers_test.go +++ b/pkg/webhooks/resource/handlers_test.go @@ -9,6 +9,7 @@ import ( kyverno "github.com/kyverno/kyverno/api/kyverno/v1" log "github.com/kyverno/kyverno/pkg/logging" "github.com/kyverno/kyverno/pkg/policycache" + "github.com/kyverno/kyverno/pkg/webhooks/handlers" "gotest.tools/assert" v1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -263,7 +264,7 @@ func Test_AdmissionResponseValid(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - handlers := NewFakeHandlers(ctx, policyCache) + resourceHandlers := NewFakeHandlers(ctx, policyCache) var validPolicy kyverno.ClusterPolicy err := json.Unmarshal([]byte(policyCheckLabel), &validPolicy) @@ -272,27 +273,29 @@ func Test_AdmissionResponseValid(t *testing.T) { key := makeKey(&validPolicy) policyCache.Set(key, &validPolicy, policycache.TestResourceFinder{}) - request := v1.AdmissionRequest{ - Operation: v1.Create, - Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, - Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Object: runtime.RawExtension{ - Raw: []byte(pod), + request := handlers.AdmissionRequest{ + AdmissionRequest: v1.AdmissionRequest{ + Operation: v1.Create, + Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, + Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Object: runtime.RawExtension{ + Raw: []byte(pod), + }, + RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, }, - RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, } - response := handlers.Mutate(ctx, logger, request, "", time.Now()) + response := resourceHandlers.Mutate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, true) - response = handlers.Validate(ctx, logger, request, "", time.Now()) + response = resourceHandlers.Validate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, true) assert.Equal(t, len(response.Warnings), 0) validPolicy.Spec.ValidationFailureAction = "Enforce" policyCache.Set(key, &validPolicy, policycache.TestResourceFinder{}) - response = handlers.Validate(ctx, logger, request, "", time.Now()) + response = resourceHandlers.Validate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, false) assert.Equal(t, len(response.Warnings), 0) @@ -306,27 +309,29 @@ func Test_AdmissionResponseInvalid(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - handlers := NewFakeHandlers(ctx, policyCache) + resourceHandlers := NewFakeHandlers(ctx, policyCache) var invalidPolicy kyverno.ClusterPolicy err := json.Unmarshal([]byte(policyInvalid), &invalidPolicy) assert.NilError(t, err) - request := v1.AdmissionRequest{ - Operation: v1.Create, - Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, - Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Object: runtime.RawExtension{ - Raw: []byte(pod), + request := handlers.AdmissionRequest{ + AdmissionRequest: v1.AdmissionRequest{ + Operation: v1.Create, + Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, + Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Object: runtime.RawExtension{ + Raw: []byte(pod), + }, + RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, }, - RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, } keyInvalid := makeKey(&invalidPolicy) invalidPolicy.Spec.ValidationFailureAction = "Enforce" policyCache.Set(keyInvalid, &invalidPolicy, policycache.TestResourceFinder{}) - response := handlers.Validate(ctx, logger, request, "", time.Now()) + response := resourceHandlers.Validate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, false) assert.Equal(t, len(response.Warnings), 0) @@ -334,7 +339,7 @@ func Test_AdmissionResponseInvalid(t *testing.T) { invalidPolicy.Spec.FailurePolicy = &ignore policyCache.Set(keyInvalid, &invalidPolicy, policycache.TestResourceFinder{}) - response = handlers.Validate(ctx, logger, request, "", time.Now()) + response = resourceHandlers.Validate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, true) assert.Equal(t, len(response.Warnings), 1) } @@ -346,7 +351,7 @@ func Test_ImageVerify(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - handlers := NewFakeHandlers(ctx, policyCache) + resourceHandlers := NewFakeHandlers(ctx, policyCache) var policy kyverno.ClusterPolicy err := json.Unmarshal([]byte(policyVerifySignature), &policy) @@ -355,20 +360,22 @@ func Test_ImageVerify(t *testing.T) { key := makeKey(&policy) policyCache.Set(key, &policy, policycache.TestResourceFinder{}) - request := v1.AdmissionRequest{ - Operation: v1.Create, - Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, - Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Object: runtime.RawExtension{ - Raw: []byte(pod), + request := handlers.AdmissionRequest{ + AdmissionRequest: v1.AdmissionRequest{ + Operation: v1.Create, + Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, + Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + Object: runtime.RawExtension{ + Raw: []byte(pod), + }, + RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, }, - RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, } policy.Spec.ValidationFailureAction = "Enforce" policyCache.Set(key, &policy, policycache.TestResourceFinder{}) - response := handlers.Mutate(ctx, logger, request, "", time.Now()) + response := resourceHandlers.Mutate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, false) assert.Equal(t, len(response.Warnings), 0) @@ -376,7 +383,7 @@ func Test_ImageVerify(t *testing.T) { policy.Spec.FailurePolicy = &ignore policyCache.Set(key, &policy, policycache.TestResourceFinder{}) - response = handlers.Mutate(ctx, logger, request, "", time.Now()) + response = resourceHandlers.Mutate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, false) assert.Equal(t, len(response.Warnings), 0) } @@ -388,7 +395,7 @@ func Test_MutateAndVerify(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - handlers := NewFakeHandlers(ctx, policyCache) + resourceHandlers := NewFakeHandlers(ctx, policyCache) var policy kyverno.ClusterPolicy err := json.Unmarshal([]byte(policyMutateAndVerify), &policy) @@ -397,17 +404,19 @@ func Test_MutateAndVerify(t *testing.T) { key := makeKey(&policy) policyCache.Set(key, &policy, policycache.TestResourceFinder{}) - request := v1.AdmissionRequest{ - Operation: v1.Create, - Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, - Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "Pod"}, - Object: runtime.RawExtension{ - Raw: []byte(resourceMutateAndVerify), + request := handlers.AdmissionRequest{ + AdmissionRequest: v1.AdmissionRequest{ + Operation: v1.Create, + Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, + Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "Pod"}, + Object: runtime.RawExtension{ + Raw: []byte(resourceMutateAndVerify), + }, + RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, }, - RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, } - response := handlers.Mutate(ctx, logger, request, "", time.Now()) + response := resourceHandlers.Mutate(ctx, logger, request, "", time.Now()) assert.Equal(t, response.Allowed, true) assert.Equal(t, len(response.Warnings), 0) } diff --git a/pkg/webhooks/resource/imageverification/handler.go b/pkg/webhooks/resource/imageverification/handler.go index 697b2eb840..5b23649072 100644 --- a/pkg/webhooks/resource/imageverification/handler.go +++ b/pkg/webhooks/resource/imageverification/handler.go @@ -25,7 +25,7 @@ import ( ) type ImageVerificationHandler interface { - Handle(context.Context, *admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext) ([]byte, []string, error) + Handle(context.Context, admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext) ([]byte, []string, error) } type imageVerificationHandler struct { @@ -57,7 +57,7 @@ func NewImageVerificationHandler( func (h *imageVerificationHandler) Handle( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, ) ([]byte, []string, error) { @@ -72,7 +72,7 @@ func (h *imageVerificationHandler) Handle( func (h *imageVerificationHandler) handleVerifyImages( ctx context.Context, logger logr.Logger, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext, policies []kyvernov1.PolicyInterface, ) (bool, string, []byte, []string) { @@ -147,7 +147,7 @@ func isResourceDeleted(policyContext *engine.PolicyContext) bool { func (v *imageVerificationHandler) handleAudit( ctx context.Context, resource unstructured.Unstructured, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, namespaceLabels map[string]string, engineResponses ...engineapi.EngineResponse, ) { diff --git a/pkg/webhooks/resource/mutation/mutation.go b/pkg/webhooks/resource/mutation/mutation.go index f7acba1b1d..dac969e946 100644 --- a/pkg/webhooks/resource/mutation/mutation.go +++ b/pkg/webhooks/resource/mutation/mutation.go @@ -27,7 +27,7 @@ type MutationHandler interface { // HandleMutation handles validating webhook admission request // If there are no errors in validating rule we apply generation rules // patchedResource is the (resource + patches) after applying mutation rules - HandleMutation(context.Context, *admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext, time.Time) ([]byte, []string, error) + HandleMutation(context.Context, admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext, time.Time) ([]byte, []string, error) } func NewMutationHandler( @@ -59,7 +59,7 @@ type mutationHandler struct { func (h *mutationHandler) HandleMutation( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, admissionRequestTimestamp time.Time, @@ -76,7 +76,7 @@ func (h *mutationHandler) HandleMutation( // return value: generated patches, triggered policies, engine responses correspdonding to the triggered policies func (v *mutationHandler) applyMutations( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, ) ([]byte, []engineapi.EngineResponse, error) { @@ -151,7 +151,7 @@ func (v *mutationHandler) applyMutations( return jsonutils.JoinPatches(patches...), engineResponses, nil } -func (h *mutationHandler) applyMutation(ctx context.Context, request *admissionv1.AdmissionRequest, policyContext *engine.PolicyContext) (*engineapi.EngineResponse, [][]byte, error) { +func (h *mutationHandler) applyMutation(ctx context.Context, request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext) (*engineapi.EngineResponse, [][]byte, error) { if request.Kind.Kind != "Namespace" && request.Namespace != "" { policyContext = policyContext.WithNamespaceLabels(engineutils.GetNamespaceSelectorsFromNamespaceLister(request.Kind.Kind, request.Namespace, h.nsLister, h.log)) } diff --git a/pkg/webhooks/resource/updaterequest.go b/pkg/webhooks/resource/updaterequest.go index 9bb08b020b..b634d05640 100644 --- a/pkg/webhooks/resource/updaterequest.go +++ b/pkg/webhooks/resource/updaterequest.go @@ -18,7 +18,7 @@ import ( ) // handleBackgroundApplies applies generate and mutateExisting policies, and creates update requests for background reconcile -func (h *handlers) handleBackgroundApplies(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, policyContext *engine.PolicyContext, generatePolicies, mutatePolicies []kyvernov1.PolicyInterface, ts time.Time) { +func (h *resourceHandlers) handleBackgroundApplies(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext, generatePolicies, mutatePolicies []kyvernov1.PolicyInterface, ts time.Time) { for _, username := range h.configuration.GetExcludedBackgroundUsernames() { if wildcard.Match(username, policyContext.AdmissionInfo().AdmissionUserInfo.Username) { return @@ -28,7 +28,7 @@ func (h *handlers) handleBackgroundApplies(ctx context.Context, logger logr.Logg h.handleGenerate(ctx, logger, request, generatePolicies, policyContext, ts) } -func (h *handlers) handleMutateExisting(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, admissionRequestTimestamp time.Time) { +func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, admissionRequestTimestamp time.Time) { if request.Operation == admissionv1.Delete { policyContext = policyContext.WithNewResource(policyContext.OldResource()) } @@ -77,7 +77,7 @@ func (h *handlers) handleMutateExisting(ctx context.Context, logger logr.Logger, } } -func (h *handlers) handleGenerate(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, generatePolicies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, ts time.Time) { +func (h *resourceHandlers) handleGenerate(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, generatePolicies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, ts time.Time) { gh := generation.NewGenerationHandler(logger, h.engine, h.client, h.kyvernoClient, h.nsLister, h.urLister, h.cpolLister, h.polLister, h.urGenerator, h.eventGen, h.metricsConfig) go gh.Handle(ctx, request, generatePolicies, policyContext) } diff --git a/pkg/webhooks/resource/utils.go b/pkg/webhooks/resource/utils.go index 9a3346bb0f..e5daf4338a 100644 --- a/pkg/webhooks/resource/utils.go +++ b/pkg/webhooks/resource/utils.go @@ -46,7 +46,7 @@ func processResourceWithPatches(patch []byte, resource []byte, log logr.Logger) func applyUpdateRequest( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, ruleType kyvernov1beta1.RequestType, urGenerator updaterequest.Generator, userRequestInfo kyvernov1beta1.RequestInfo, @@ -54,7 +54,7 @@ func applyUpdateRequest( engineResponses ...*engineapi.EngineResponse, ) (failedUpdateRequest []updateRequestResponse) { admissionRequestInfo := kyvernov1beta1.AdmissionRequestInfoObject{ - AdmissionRequest: request, + AdmissionRequest: &request, Operation: action, } diff --git a/pkg/webhooks/resource/validation/validation.go b/pkg/webhooks/resource/validation/validation.go index aa758f0c12..b207aca909 100644 --- a/pkg/webhooks/resource/validation/validation.go +++ b/pkg/webhooks/resource/validation/validation.go @@ -29,7 +29,7 @@ type ValidationHandler interface { // HandleValidation handles validating webhook admission request // If there are no errors in validating rule we apply generation rules // patchedResource is the (resource + patches) after applying mutation rules - HandleValidation(context.Context, *admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext, time.Time) (bool, string, []string) + HandleValidation(context.Context, admissionv1.AdmissionRequest, []kyvernov1.PolicyInterface, *engine.PolicyContext, time.Time) (bool, string, []string) } func NewValidationHandler( @@ -70,7 +70,7 @@ type validationHandler struct { func (v *validationHandler) HandleValidation( ctx context.Context, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, admissionRequestTimestamp time.Time, @@ -145,7 +145,7 @@ func (v *validationHandler) HandleValidation( func (v *validationHandler) buildAuditResponses( ctx context.Context, resource unstructured.Unstructured, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, namespaceLabels map[string]string, ) ([]engineapi.EngineResponse, error) { gvr := schema.GroupVersionResource(request.Resource) @@ -175,7 +175,7 @@ func (v *validationHandler) buildAuditResponses( func (v *validationHandler) handleAudit( ctx context.Context, resource unstructured.Unstructured, - request *admissionv1.AdmissionRequest, + request admissionv1.AdmissionRequest, namespaceLabels map[string]string, engineResponses ...engineapi.EngineResponse, ) { diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index ae0c046680..1ce78240de 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -39,21 +39,21 @@ type Server interface { type ExceptionHandlers interface { // Validate performs the validation check on exception resources - Validate(context.Context, logr.Logger, admissionv1.AdmissionRequest, time.Time) admissionv1.AdmissionResponse + Validate(context.Context, logr.Logger, handlers.AdmissionRequest, time.Time) admissionv1.AdmissionResponse } type PolicyHandlers interface { // Mutate performs the mutation of policy resources - Mutate(context.Context, logr.Logger, admissionv1.AdmissionRequest, time.Time) admissionv1.AdmissionResponse + Mutate(context.Context, logr.Logger, handlers.AdmissionRequest, time.Time) admissionv1.AdmissionResponse // Validate performs the validation check on policy resources - Validate(context.Context, logr.Logger, admissionv1.AdmissionRequest, time.Time) admissionv1.AdmissionResponse + Validate(context.Context, logr.Logger, handlers.AdmissionRequest, time.Time) admissionv1.AdmissionResponse } type ResourceHandlers interface { // Mutate performs the mutation of kube resources - Mutate(context.Context, logr.Logger, admissionv1.AdmissionRequest, string, time.Time) admissionv1.AdmissionResponse + Mutate(context.Context, logr.Logger, handlers.AdmissionRequest, string, time.Time) admissionv1.AdmissionResponse // Validate performs the validation check on kube resources - Validate(context.Context, logr.Logger, admissionv1.AdmissionRequest, string, time.Time) admissionv1.AdmissionResponse + Validate(context.Context, logr.Logger, handlers.AdmissionRequest, string, time.Time) admissionv1.AdmissionResponse } type server struct { @@ -245,24 +245,24 @@ func registerWebhookHandlers( mux *httprouter.Router, name string, basePath string, - handlerFunc func(context.Context, logr.Logger, admissionv1.AdmissionRequest, string, time.Time) admissionv1.AdmissionResponse, + handlerFunc func(context.Context, logr.Logger, handlers.AdmissionRequest, string, time.Time) admissionv1.AdmissionResponse, builder func(handler handlers.AdmissionHandler) handlers.HttpHandler, ) { all := handlers.FromAdmissionFunc( name, - func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + func(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { return handlerFunc(ctx, logger, request, "all", startTime) }, ) ignore := handlers.FromAdmissionFunc( name, - func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + func(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { return handlerFunc(ctx, logger, request, "ignore", startTime) }, ) fail := handlers.FromAdmissionFunc( name, - func(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { + func(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, startTime time.Time) admissionv1.AdmissionResponse { return handlerFunc(ctx, logger, request, "fail", startTime) }, ) diff --git a/pkg/webhooks/utils/policy_context_builder.go b/pkg/webhooks/utils/policy_context_builder.go index a90c2a685b..bc29846d27 100644 --- a/pkg/webhooks/utils/policy_context_builder.go +++ b/pkg/webhooks/utils/policy_context_builder.go @@ -13,7 +13,7 @@ import ( ) type PolicyContextBuilder interface { - Build(*admissionv1.AdmissionRequest) (*engine.PolicyContext, error) + Build(admissionv1.AdmissionRequest) (*engine.PolicyContext, error) } type policyContextBuilder struct { @@ -37,7 +37,7 @@ func NewPolicyContextBuilder( } } -func (b *policyContextBuilder) Build(request *admissionv1.AdmissionRequest) (*engine.PolicyContext, error) { +func (b *policyContextBuilder) Build(request admissionv1.AdmissionRequest) (*engine.PolicyContext, error) { userRequestInfo := kyvernov1beta1.RequestInfo{ AdmissionUserInfo: *request.UserInfo.DeepCopy(), }