From bf93df37317567a16552612fb6bd8911b1fc69ce Mon Sep 17 00:00:00 2001 From: Valentin Delaye Date: Wed, 14 May 2025 16:48:25 +0200 Subject: [PATCH 1/2] Support for passing old resource to validator --- .../webhook/admission/AdmissionUtils.java | 6 ++++++ ...AsyncDefaultAdmissionRequestValidator.java | 5 ++++- .../DefaultAdmissionRequestValidator.java | 4 +++- .../admission/validation/Validator.java | 2 +- .../admission/AdmissionControllerTest.java | 9 +++++---- .../AsyncAdmissionControllerTest.java | 19 +++++++++++-------- .../sample/commons/AdmissionControllers.java | 16 +++++++++++----- 7 files changed, 41 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java b/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java index cdf044fb..1bf4af22 100644 --- a/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java +++ b/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java @@ -37,6 +37,12 @@ public static KubernetesResource getTargetResource(AdmissionRequest admissionReq : admissionRequest.getObject()); } + public static KubernetesResource getOldResource(AdmissionRequest admissionRequest, + Operation operation) { + return (KubernetesResource) (operation == Operation.UPDATE ? admissionRequest.getOldObject() + : getTargetResource(admissionRequest, operation)); + } + public static AdmissionResponse admissionResponseFromMutation(KubernetesResource originalResource, KubernetesResource mutatedResource) { var admissionResponse = new AdmissionResponse(); diff --git a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java index 8e10afb9..94362c89 100644 --- a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java +++ b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java @@ -12,6 +12,7 @@ import io.javaoperatorsdk.webhook.admission.Operation; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.allowedAdmissionResponse; +import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getOldResource; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getTargetResource; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.notAllowedExceptionToAdmissionResponse; @@ -29,8 +30,10 @@ public AsyncDefaultAdmissionRequestValidator(Validator validator) { public CompletionStage handle(AdmissionRequest admissionRequest) { var operation = Operation.valueOf(admissionRequest.getOperation()); var originalResource = (T) getTargetResource(admissionRequest, operation); + var oldResource = (T) getOldResource(admissionRequest, operation); var asyncValidate = - CompletableFuture.runAsync(() -> validator.validate(originalResource, operation)); + CompletableFuture + .runAsync(() -> validator.validate(originalResource, oldResource, operation)); return asyncValidate .thenApply(v -> allowedAdmissionResponse()) .exceptionally(e -> { diff --git a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java index 75f43b99..a099b708 100644 --- a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java +++ b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java @@ -8,6 +8,7 @@ import io.javaoperatorsdk.webhook.admission.Operation; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.allowedAdmissionResponse; +import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getOldResource; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getTargetResource; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.notAllowedExceptionToAdmissionResponse; @@ -25,8 +26,9 @@ public DefaultAdmissionRequestValidator(Validator validator) { public AdmissionResponse handle(AdmissionRequest admissionRequest) { var operation = Operation.valueOf(admissionRequest.getOperation()); var originalResource = (T) getTargetResource(admissionRequest, operation); + var oldResource = (T) getOldResource(admissionRequest, operation); try { - validator.validate(originalResource, operation); + validator.validate(originalResource, oldResource, operation); return allowedAdmissionResponse(); } catch (NotAllowedException e) { return notAllowedExceptionToAdmissionResponse(e); diff --git a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/Validator.java b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/Validator.java index b465bf0f..caaf8bf7 100644 --- a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/Validator.java +++ b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/Validator.java @@ -6,5 +6,5 @@ public interface Validator { - void validate(T resource, Operation operation) throws NotAllowedException; + void validate(T resource, T oldResource, Operation operation) throws NotAllowedException; } diff --git a/core/src/test/java/io/javaoperatorsdk/webhook/admission/AdmissionControllerTest.java b/core/src/test/java/io/javaoperatorsdk/webhook/admission/AdmissionControllerTest.java index d2dfaae2..764bbe1f 100644 --- a/core/src/test/java/io/javaoperatorsdk/webhook/admission/AdmissionControllerTest.java +++ b/core/src/test/java/io/javaoperatorsdk/webhook/admission/AdmissionControllerTest.java @@ -15,15 +15,16 @@ class AdmissionControllerTest { @Test void validatesResource() { - var admissionController = new AdmissionController((resource, operation) -> { - }); + var admissionController = + new AdmissionController((resource, oldResource, operation) -> { + }); admissionTestSupport.validatesResource(admissionController::handle); } @Test void validatesResource_whenNotAllowedException() { var admissionController = - new AdmissionController<>((Validator) (resource, operation) -> { + new AdmissionController<>((Validator) (resource, oldResource, operation) -> { throw new NotAllowedException(MISSING_REQUIRED_LABEL); }); admissionTestSupport.notAllowedException(admissionController::handle); @@ -32,7 +33,7 @@ void validatesResource_whenNotAllowedException() { @Test void validatesResource_whenOtherException() { var admissionController = - new AdmissionController<>((Validator) (resource, operation) -> { + new AdmissionController<>((Validator) (resource, oldResource, operation) -> { throw new IllegalArgumentException("Invalid resource"); }); diff --git a/core/src/test/java/io/javaoperatorsdk/webhook/admission/AsyncAdmissionControllerTest.java b/core/src/test/java/io/javaoperatorsdk/webhook/admission/AsyncAdmissionControllerTest.java index c3975912..fd04a867 100644 --- a/core/src/test/java/io/javaoperatorsdk/webhook/admission/AsyncAdmissionControllerTest.java +++ b/core/src/test/java/io/javaoperatorsdk/webhook/admission/AsyncAdmissionControllerTest.java @@ -20,8 +20,9 @@ class AsyncAdmissionControllerTest { @Test void validatesResource() { - var admissionController = new AsyncAdmissionController((resource, operation) -> { - }); + var admissionController = + new AsyncAdmissionController((resource, oldResource, operation) -> { + }); admissionTestSupport .validatesResource(res -> admissionController.handle(res).toCompletableFuture().join()); @@ -30,9 +31,10 @@ void validatesResource() { @Test void validatesResource_whenNotAllowedException() { var admissionController = - new AsyncAdmissionController<>((Validator) (resource, operation) -> { - throw new NotAllowedException(MISSING_REQUIRED_LABEL); - }); + new AsyncAdmissionController<>( + (Validator) (resource, oldResource, operation) -> { + throw new NotAllowedException(MISSING_REQUIRED_LABEL); + }); admissionTestSupport .notAllowedException(res -> admissionController.handle(res).toCompletableFuture().join()); @@ -41,9 +43,10 @@ void validatesResource_whenNotAllowedException() { @Test void validatesResource_whenOtherException() { var admissionController = - new AsyncAdmissionController<>((Validator) (resource, operation) -> { - throw new IllegalArgumentException("Invalid resource"); - }); + new AsyncAdmissionController<>( + (Validator) (resource, oldResource, operation) -> { + throw new IllegalArgumentException("Invalid resource"); + }); admissionTestSupport.assertThatThrownBy( res -> admissionController.handle(res).toCompletableFuture() diff --git a/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java b/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java index a9ad92f0..ab1856ac 100644 --- a/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java +++ b/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java @@ -36,7 +36,7 @@ public static AsyncAdmissionController asyncValidatingController() { } public static AdmissionController errorMutatingController() { - return new AdmissionController<>((Validator) (resource, operation) -> { + return new AdmissionController<>((Validator) (resource, oldResource, operation) -> { throw new IllegalStateException(ERROR_MESSAGE); }); } @@ -54,9 +54,10 @@ public static AsyncAdmissionController errorAsyncMutatingController() { } public static AsyncAdmissionController errorAsyncValidatingController() { - return new AsyncAdmissionController<>((Validator) (resource, operation) -> { - throw new IllegalStateException(ERROR_MESSAGE); - }); + return new AsyncAdmissionController<>( + (Validator) (resource, oldResource, operation) -> { + throw new IllegalStateException(ERROR_MESSAGE); + }); } private static class IngressMutator implements Mutator { @@ -72,11 +73,16 @@ public Ingress mutate(Ingress resource, Operation operation) throws NotAllowedEx private static class IngressValidator implements Validator { @Override - public void validate(Ingress resource, Operation operation) throws NotAllowedException { + public void validate(Ingress resource, Ingress oldResource, Operation operation) + throws NotAllowedException { if (resource.getMetadata().getLabels() == null || resource.getMetadata().getLabels().get(VALIDATION_TARGET_LABEL) == null) { throw new NotAllowedException("Missing label: " + VALIDATION_TARGET_LABEL); } + if (!resource.getSpec().getIngressClassName() + .equals(oldResource.getSpec().getIngressClassName())) { + throw new NotAllowedException("IngressClassName cannot be changed"); + } } } } From 222f77911bcfc6c394496bda97aa0e42080ec97e Mon Sep 17 00:00:00 2001 From: Valentin Delaye Date: Fri, 16 May 2025 06:20:56 +0200 Subject: [PATCH 2/2] Pass directly the resource/oldResource for validator --- .../javaoperatorsdk/webhook/admission/AdmissionUtils.java | 6 ------ .../validation/AsyncDefaultAdmissionRequestValidator.java | 6 ++---- .../validation/DefaultAdmissionRequestValidator.java | 6 ++---- .../webhook/sample/commons/AdmissionControllers.java | 8 ++++++-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java b/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java index 1bf4af22..cdf044fb 100644 --- a/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java +++ b/core/src/main/java/io/javaoperatorsdk/webhook/admission/AdmissionUtils.java @@ -37,12 +37,6 @@ public static KubernetesResource getTargetResource(AdmissionRequest admissionReq : admissionRequest.getObject()); } - public static KubernetesResource getOldResource(AdmissionRequest admissionRequest, - Operation operation) { - return (KubernetesResource) (operation == Operation.UPDATE ? admissionRequest.getOldObject() - : getTargetResource(admissionRequest, operation)); - } - public static AdmissionResponse admissionResponseFromMutation(KubernetesResource originalResource, KubernetesResource mutatedResource) { var admissionResponse = new AdmissionResponse(); diff --git a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java index 94362c89..b87e78d5 100644 --- a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java +++ b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/AsyncDefaultAdmissionRequestValidator.java @@ -12,8 +12,6 @@ import io.javaoperatorsdk.webhook.admission.Operation; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.allowedAdmissionResponse; -import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getOldResource; -import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getTargetResource; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.notAllowedExceptionToAdmissionResponse; public class AsyncDefaultAdmissionRequestValidator @@ -29,8 +27,8 @@ public AsyncDefaultAdmissionRequestValidator(Validator validator) { @SuppressWarnings("unchecked") public CompletionStage handle(AdmissionRequest admissionRequest) { var operation = Operation.valueOf(admissionRequest.getOperation()); - var originalResource = (T) getTargetResource(admissionRequest, operation); - var oldResource = (T) getOldResource(admissionRequest, operation); + var originalResource = (T) admissionRequest.getObject(); + var oldResource = (T) admissionRequest.getOldObject(); var asyncValidate = CompletableFuture .runAsync(() -> validator.validate(originalResource, oldResource, operation)); diff --git a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java index a099b708..ce3f1a4c 100644 --- a/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java +++ b/core/src/main/java/io/javaoperatorsdk/webhook/admission/validation/DefaultAdmissionRequestValidator.java @@ -8,8 +8,6 @@ import io.javaoperatorsdk.webhook.admission.Operation; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.allowedAdmissionResponse; -import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getOldResource; -import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getTargetResource; import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.notAllowedExceptionToAdmissionResponse; public class DefaultAdmissionRequestValidator @@ -25,8 +23,8 @@ public DefaultAdmissionRequestValidator(Validator validator) { @SuppressWarnings("unchecked") public AdmissionResponse handle(AdmissionRequest admissionRequest) { var operation = Operation.valueOf(admissionRequest.getOperation()); - var originalResource = (T) getTargetResource(admissionRequest, operation); - var oldResource = (T) getOldResource(admissionRequest, operation); + var originalResource = (T) admissionRequest.getObject(); + var oldResource = (T) admissionRequest.getOldObject(); try { validator.validate(originalResource, oldResource, operation); return allowedAdmissionResponse(); diff --git a/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java b/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java index ab1856ac..3c23f645 100644 --- a/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java +++ b/samples/commons/src/main/java/io/javaoperatorsdk/webhook/sample/commons/AdmissionControllers.java @@ -75,12 +75,16 @@ private static class IngressValidator implements Validator { @Override public void validate(Ingress resource, Ingress oldResource, Operation operation) throws NotAllowedException { + if (operation.equals(Operation.DELETE)) { + return; + } if (resource.getMetadata().getLabels() == null || resource.getMetadata().getLabels().get(VALIDATION_TARGET_LABEL) == null) { throw new NotAllowedException("Missing label: " + VALIDATION_TARGET_LABEL); } - if (!resource.getSpec().getIngressClassName() - .equals(oldResource.getSpec().getIngressClassName())) { + if (operation.equals(Operation.UPDATE) + && !resource.getSpec().getIngressClassName() + .equals(oldResource.getSpec().getIngressClassName())) { throw new NotAllowedException("IngressClassName cannot be changed"); } }