From 4bf11e3cd6b15385c0a08856f565120c01689e3c Mon Sep 17 00:00:00 2001 From: Tatiana Viana Date: Tue, 16 Jul 2024 23:47:38 -0700 Subject: [PATCH] chore: disable redundant validation from the handler execution --- .../cloudformation/AbstractWrapper.java | 5 +- .../cloudformation/ExecutableWrapperTest.java | 7 - .../cloudformation/LambdaWrapperTest.java | 7 - .../amazon/cloudformation/WrapperTest.java | 140 ------------------ 4 files changed, 4 insertions(+), 155 deletions(-) diff --git a/src/main/java/software/amazon/cloudformation/AbstractWrapper.java b/src/main/java/software/amazon/cloudformation/AbstractWrapper.java index 8bedfad1..c3a1bc6b 100644 --- a/src/main/java/software/amazon/cloudformation/AbstractWrapper.java +++ b/src/main/java/software/amazon/cloudformation/AbstractWrapper.java @@ -76,7 +76,10 @@ public abstract class AbstractWrapper { public static final SdkHttpClient HTTP_CLIENT = ApacheHttpClient.builder().build(); private static final Set MUTATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.DELETE, Action.UPDATE); - private static final Set VALIDATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.UPDATE); + + // Soft disable redundant schema validation + // Next action remove all validation code + private static final Set VALIDATING_ACTIONS = ImmutableSet.of(); protected final Serializer serializer; protected LoggerProxy loggerProxy; diff --git a/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java b/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java index ace8f2f4..37c153c8 100644 --- a/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java +++ b/src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java @@ -17,7 +17,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import com.fasterxml.jackson.core.type.TypeReference; import java.io.ByteArrayOutputStream; @@ -27,7 +26,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import org.json.JSONObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; @@ -133,11 +131,6 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // 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)); - } - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().status(OperationStatus.SUCCESS).build()); diff --git a/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java b/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java index 0faa3d1a..6562d132 100644 --- a/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java +++ b/src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java @@ -18,7 +18,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import com.amazonaws.services.lambda.runtime.Context; import com.amazonaws.services.lambda.runtime.LambdaLogger; @@ -30,7 +29,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import org.json.JSONObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; @@ -146,11 +144,6 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // 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)); - } - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().status(OperationStatus.SUCCESS).build()); diff --git a/src/test/java/software/amazon/cloudformation/WrapperTest.java b/src/test/java/software/amazon/cloudformation/WrapperTest.java index 94ac089c..bb334ac3 100755 --- a/src/test/java/software/amazon/cloudformation/WrapperTest.java +++ b/src/test/java/software/amazon/cloudformation/WrapperTest.java @@ -32,7 +32,6 @@ import java.time.Instant; import java.util.HashMap; import java.util.Map; -import org.json.JSONObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -59,7 +58,6 @@ import software.amazon.cloudformation.resource.SchemaValidator; import software.amazon.cloudformation.resource.Serializer; import software.amazon.cloudformation.resource.Validator; -import software.amazon.cloudformation.resource.exceptions.ValidationException; @ExtendWith(MockitoExtension.class) public class WrapperTest { @@ -155,11 +153,6 @@ 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 - if (action == Action.CREATE || action == Action.UPDATE) { - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - } - verify(providerEventsLogger).refreshClient(); verify(providerEventsLogger, times(2)).publishLogEvent(any()); verifyNoMoreInteractions(providerEventsLogger); @@ -176,37 +169,6 @@ public void invokeHandler_nullResponse_returnsFailure(final String requestDataPa } } - @Test - public void invokeHandler_SchemaFailureOnNestedProperties() throws IOException { - // use actual validator to verify behaviour - final WrapperOverride wrapper = new WrapperOverride(providerLoggingCredentialsProvider, platformEventsLogger, - providerEventsLogger, providerMetricsPublisher, new Validator() { - }, httpClient); - - wrapper.setTransformResponse(resourceHandlerRequest); - - try (final InputStream in = loadRequestStream("create.request.with-extraneous-model-object.json"); - final OutputStream out = new ByteArrayOutputStream()) { - - wrapper.processRequest(in, out); - // validation failure metric should be published but no others - verify(providerMetricsPublisher).publishExceptionMetric(any(Instant.class), eq(Action.CREATE), any(Exception.class), - any(HandlerErrorCode.class)); - - // all metrics should be published, even for a single invocation - verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(Action.CREATE)); - - // verify initialiseRuntime was called and initialised dependencies - verifyInitialiseRuntime(); - - // verify output response - verifyHandlerResponse(out, - ProgressEvent.builder().errorCode(HandlerErrorCode.InvalidRequest) - .status(OperationStatus.FAILED).message("Resource properties validation failed with invalid configuration") - .build()); - } - } - @Test public void invokeHandlerForCreate_without_customer_loggingCredentials() throws IOException { invokeHandler_without_customerLoggingCredentials("create.request-without-logging-credentials.json", Action.CREATE); @@ -237,11 +199,6 @@ private void invokeHandler_without_customerLoggingCredentials(final String reque verify(providerMetricsPublisher, times(0)).publishInvocationMetric(any(Instant.class), eq(action)); verify(providerMetricsPublisher, times(0)).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(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - } - verifyNoMoreInteractions(providerEventsLogger); // verify output response @@ -270,11 +227,6 @@ public void invokeHandler_handlerFailed_returnsFailure(final String requestDataP // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // 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 verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.InternalFailure) .status(OperationStatus.FAILED).message("Custom Fault").build()); @@ -326,11 +278,6 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ // verify initialiseRuntime was called and initialised dependencies verifyInitialiseRuntime(); - // 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 verifyHandlerResponse(out, ProgressEvent.builder().status(OperationStatus.SUCCESS).build()); @@ -401,11 +348,6 @@ 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 - if (action == Action.CREATE || action == Action.UPDATE) { - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - } - 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()); @@ -514,49 +456,12 @@ public void reInvokeHandler_InProgress_returnsInProgressWithoutContext(final Str // validation failure metric should not be published verifyNoMoreInteractions(providerMetricsPublisher); - // verify that model validation occurred for CREATE/UPDATE - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().status(OperationStatus.IN_PROGRESS) .resourceModel(TestModel.builder().property1("abc").property2(123).build()).build()); } } - @ParameterizedTest - @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); - - doThrow(ValidationException.class).when(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - - wrapper.setTransformResponse(resourceHandlerRequest); - - try (final InputStream in = loadRequestStream(requestDataPath); final OutputStream out = new ByteArrayOutputStream()) { - wrapper.processRequest(in, out); - - // verify initialiseRuntime was called and initialised dependencies - verifyInitialiseRuntime(); - - // validation failure metric should be published but no others - verify(providerMetricsPublisher, times(1)).publishExceptionMetric(any(Instant.class), eq(action), - any(Exception.class), any(HandlerErrorCode.class)); - - // 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 - if (action == Action.CREATE || action == Action.UPDATE) { - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - } - - // verify output response - verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.InvalidRequest) - .status(OperationStatus.FAILED).message("Model validation failed with unknown cause.").build()); - } - } - @ParameterizedTest @CsvSource({ "create.request.json,CREATE", "update.request.json,UPDATE", "delete.request.json,DELETE", "read.request.json,READ", "list.request.json,LIST" }) @@ -650,36 +555,6 @@ providerEventsLogger, providerMetricsPublisher, new Validator() { } } - @Test - public void invokeHandler_extraneousModelFields_causesSchemaValidationFailure() throws IOException { - // use actual validator to verify behaviour - final WrapperOverride wrapper = new WrapperOverride(providerLoggingCredentialsProvider, platformEventsLogger, - providerEventsLogger, providerMetricsPublisher, new Validator() { - }, httpClient); - - wrapper.setTransformResponse(resourceHandlerRequest); - - try (final InputStream in = loadRequestStream("create.request.with-extraneous-model-fields.json"); - final OutputStream out = new ByteArrayOutputStream()) { - wrapper.processRequest(in, out); - - // validation failure metric should be published but no others - verify(providerMetricsPublisher, times(1)).publishExceptionMetric(any(Instant.class), eq(Action.CREATE), - any(Exception.class), any(HandlerErrorCode.class)); - - // all metrics should be published, even for a single invocation - verify(providerMetricsPublisher, times(1)).publishInvocationMetric(any(Instant.class), eq(Action.CREATE)); - - // verify initialiseRuntime was called and initialised dependencies - verifyInitialiseRuntime(); - - // verify output response - verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.InvalidRequest) - .status(OperationStatus.FAILED) - .message("Model validation failed (#: extraneous key [fieldCausesValidationError] is not permitted)").build()); - } - } - @Test public void invokeHandler_withMalformedRequest_causesSchemaValidationFailure() throws IOException { final TestModel model = new TestModel(); @@ -814,9 +689,6 @@ public void invokeHandler_throwsAmazonServiceException_returnsServiceException() verify(providerMetricsPublisher, times(1)).publishExceptionMetric(any(Instant.class), any(Action.class), any(AmazonServiceException.class), any(HandlerErrorCode.class)); - // verify that model validation occurred for CREATE/UPDATE/DELETE - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.GeneralServiceException) @@ -847,9 +719,6 @@ public void invokeHandler_throwsSDK2ServiceException_returnsServiceException() t verify(providerMetricsPublisher, times(1)).publishExceptionMetric(any(Instant.class), any(Action.class), any(CloudWatchLogsException.class), any(HandlerErrorCode.class)); - // verify that model validation occurred for CREATE/UPDATE/DELETE - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.GeneralServiceException) @@ -882,9 +751,6 @@ public void invokeHandler_throwsThrottlingException_returnsCFNThrottlingExceptio verify(providerMetricsPublisher, times(1)).publishExceptionMetric(any(Instant.class), any(Action.class), any(AmazonServiceException.class), any(HandlerErrorCode.class)); - // verify that model validation occurred for CREATE/UPDATE/DELETE - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.Throttling) @@ -916,9 +782,6 @@ public void invokeHandler_throwsResourceAlreadyExistsException_returnsAlreadyExi verify(providerMetricsPublisher, times(1)).publishExceptionMetric(any(Instant.class), any(Action.class), any(ResourceAlreadyExistsException.class), any(HandlerErrorCode.class)); - // verify that model validation occurred for CREATE/UPDATE/DELETE - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.AlreadyExists) @@ -949,9 +812,6 @@ public void invokeHandler_throwsResourceNotFoundException_returnsNotFound() thro verify(providerMetricsPublisher, times(1)).publishExceptionMetric(any(Instant.class), any(Action.class), any(ResourceNotFoundException.class), any(HandlerErrorCode.class)); - // verify that model validation occurred for CREATE/UPDATE/DELETE - verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class)); - // verify output response verifyHandlerResponse(out, ProgressEvent.builder().errorCode(HandlerErrorCode.NotFound)