Skip to content

Commit

Permalink
chore: disable redundant validation from the handler execution
Browse files Browse the repository at this point in the history
  • Loading branch information
@tvp-1
tvp-1 committed Jul 17, 2024
1 parent b9cfe28 commit 4bf11e3
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 155 deletions.
5 changes: 4 additions & 1 deletion src/main/java/software/amazon/cloudformation/AbstractWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ public abstract class AbstractWrapper<ResourceT, CallbackT, ConfigurationT> {
public static final SdkHttpClient HTTP_CLIENT = ApacheHttpClient.builder().build();

private static final Set<Action> MUTATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.DELETE, Action.UPDATE);
private static final Set<Action> VALIDATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.UPDATE);

// Soft disable redundant schema validation
// Next action remove all validation code
private static final Set<Action> VALIDATING_ACTIONS = ImmutableSet.of();

protected final Serializer serializer;
protected LoggerProxy loggerProxy;
Expand Down
7 changes: 0 additions & 7 deletions src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().status(OperationStatus.SUCCESS).build());

Expand Down
7 changes: 0 additions & 7 deletions src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().status(OperationStatus.SUCCESS).build());

Expand Down
140 changes: 0 additions & 140 deletions src/test/java/software/amazon/cloudformation/WrapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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.<TestModel, TestContext>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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.InternalFailure)
.status(OperationStatus.FAILED).message("Custom Fault").build());
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().status(OperationStatus.SUCCESS).build());

Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().status(OperationStatus.IN_PROGRESS)
.resourceModel(TestModel.builder().property1("abc").property2(123).build()).build());
Expand Down Expand Up @@ -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.<TestModel, TestContext>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.<TestModel, TestContext>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" })
Expand Down Expand Up @@ -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.<TestModel, TestContext>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();
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.GeneralServiceException)
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.GeneralServiceException)
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.Throttling)
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.AlreadyExists)
Expand Down Expand Up @@ -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.<TestModel, TestContext>builder().errorCode(HandlerErrorCode.NotFound)
Expand Down

0 comments on commit 4bf11e3

Please sign in to comment.