From 5e93292b26bde54fd55b753dca5d0ead4f18d854 Mon Sep 17 00:00:00 2001 From: John Tompkins Date: Wed, 11 Nov 2020 13:16:13 -0800 Subject: [PATCH] Skip validation on delete requests #318 --- .../cloudformation/AbstractWrapper.java | 17 +++++++---- .../cloudformation/ExecutableWrapperTest.java | 4 +-- .../cloudformation/LambdaWrapperTest.java | 4 +-- .../amazon/cloudformation/WrapperTest.java | 29 ++++++++++--------- 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/main/java/software/amazon/cloudformation/AbstractWrapper.java b/src/main/java/software/amazon/cloudformation/AbstractWrapper.java index c3c2bb92..1f02a9cf 100644 --- a/src/main/java/software/amazon/cloudformation/AbstractWrapper.java +++ b/src/main/java/software/amazon/cloudformation/AbstractWrapper.java @@ -24,11 +24,11 @@ import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.time.Instant; -import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; @@ -73,7 +73,8 @@ public abstract class AbstractWrapper { public static final SdkHttpClient HTTP_CLIENT = ApacheHttpClient.builder().build(); - private static final List MUTATING_ACTIONS = Arrays.asList(Action.CREATE, Action.DELETE, Action.UPDATE); + private static final Set MUTATING_ACTIONS = Set.of(Action.CREATE, Action.DELETE, Action.UPDATE); + private static final Set VALIDATING_ACTIONS = Set.of(Action.CREATE, Action.UPDATE); protected final Serializer serializer; protected LoggerProxy loggerProxy; @@ -245,7 +246,8 @@ public void processRequest(final InputStream inputStream, final OutputStream out throw new TerminalException("Invalid request object received"); } - if (MUTATING_ACTIONS.contains(request.getAction())) { + final boolean isMutatingAction = MUTATING_ACTIONS.contains(request.getAction()); + if (isMutatingAction) { if (request.getRequestData().getResourceProperties() == null) { throw new TerminalException("Invalid resource properties object received"); } @@ -271,13 +273,16 @@ public void processRequest(final InputStream inputStream, final OutputStream out this.metricsPublisherProxy.publishInvocationMetric(Instant.now(), request.getAction()); - // for CUD actions, validate incoming model - any error is a terminal failure on + // for create and update actions, validate incoming model - any error is a + // terminal failure on // the invocation // NOTE: we validate the raw pre-deserialized payload to account for lenient // serialization. + // NOTE: Validation is not required on deletion as only the primary identifier + // is required to delete. // Here, we want to surface ALL input validation errors to the caller. - boolean isMutatingAction = MUTATING_ACTIONS.contains(request.getAction()); - if (isMutatingAction) { + final boolean shouldValidate = VALIDATING_ACTIONS.contains(request.getAction()); + if (shouldValidate) { // validate entire incoming payload, including extraneous fields which // are stripped by the Serializer (due to FAIL_ON_UNKNOWN_PROPERTIES setting) JSONObject rawModelObject = rawRequest.getJSONObject("requestData").getJSONObject("resourceProperties"); diff --git a/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java b/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java index 881c9e7e..9849dc4a 100644 --- a/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java +++ b/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java @@ -132,8 +132,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator, times(1)).validateObject(any(JSONObject.class), any(JSONObject.class)); } diff --git a/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java b/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java index 49658c9b..4534e13c 100644 --- a/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java +++ b/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java @@ -145,8 +145,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator, times(1)).validateObject(any(JSONObject.class), any(JSONObject.class)); } diff --git a/src/test/java/software/amazon/cloudformation/WrapperTest.java b/src/test/java/software/amazon/cloudformation/WrapperTest.java index d935b2f3..d83f96cd 100755 --- a/src/test/java/software/amazon/cloudformation/WrapperTest.java +++ b/src/test/java/software/amazon/cloudformation/WrapperTest.java @@ -154,8 +154,8 @@ public void invokeHandler_nullResponse_returnsFailure(final String requestDataPa verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action)); verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong()); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); } @@ -269,8 +269,8 @@ public void invokeHandler_handlerFailed_returnsFailure(final String requestDataP // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); } @@ -325,8 +325,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); } @@ -400,11 +400,12 @@ public void invokeHandler_InProgress_returnsInProgress(final String requestDataP verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action)); verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong()); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); + } - // verify output response + if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { verifyHandlerResponse(out, ProgressEvent.builder().status(OperationStatus.IN_PROGRESS) .resourceModel(TestModel.builder().property1("abc").property2(123).build()).build()); } else { @@ -465,8 +466,8 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat // validation failure metric should not be published verifyNoMoreInteractions(providerMetricsPublisher); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); } @@ -477,7 +478,7 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat } @ParameterizedTest - @CsvSource({ "create.request.json,CREATE", "update.request.json,UPDATE", "delete.request.json,DELETE" }) + @CsvSource({ "create.request.json,CREATE", "update.request.json,UPDATE" }) public void invokeHandler_SchemaValidationFailure(final String requestDataPath, final String actionAsString) throws IOException { final Action action = Action.valueOf(actionAsString); @@ -499,8 +500,8 @@ public void invokeHandler_SchemaValidationFailure(final String requestDataPath, // all metrics should be published, even for a single invocation verify(providerMetricsPublisher, times(1)).publishInvocationMetric(any(Instant.class), eq(action)); - // verify that model validation occurred for CREATE/UPDATE/DELETE - if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) { + // verify that model validation occurred for CREATE/UPDATE + if (action == Action.CREATE || action == Action.UPDATE) { verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); }