Skip to content

Commit c454174

Browse files
authored
Adding extra condition to avoid subsequent validation (#371)
1 parent 7b33efd commit c454174

File tree

6 files changed

+155
-14
lines changed

6 files changed

+155
-14
lines changed

src/main/java/software/amazon/cloudformation/AbstractWrapper.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ public void processRequest(final InputStream inputStream, final OutputStream out
285285
// NOTE: Validation is not required on deletion as only the primary identifier
286286
// is required to delete.
287287
// Here, we want to surface ALL input validation errors to the caller.
288-
final boolean shouldValidate = VALIDATING_ACTIONS.contains(request.getAction());
288+
final boolean shouldValidate = VALIDATING_ACTIONS.contains(request.getAction()) && request.getCallbackContext() == null;
289289
if (shouldValidate) {
290290
// validate entire incoming payload, including extraneous fields which
291291
// are stripped by the Serializer (due to FAIL_ON_UNKNOWN_PROPERTIES setting)
@@ -388,8 +388,7 @@ private void logUnhandledError(final String errorDescription,
388388

389389
}
390390

391-
protected void writeResponse(final OutputStream outputStream,
392-
final ProgressEvent<ResourceT, CallbackT> response)
391+
protected void writeResponse(final OutputStream outputStream, final ProgressEvent<ResourceT, CallbackT> response)
393392
throws IOException {
394393
if (response.getResourceModel() != null) {
395394
// strip write only properties on final results, we will need the intact model
Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
/*
2+
* Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
115
package software.amazon.cloudformation.exceptions;
216

317
import software.amazon.cloudformation.proxy.HandlerErrorCode;
@@ -6,8 +20,9 @@ public class CfnInvalidTypeConfigurationException extends BaseHandlerException {
620

721
private static final long serialVersionUID = -1646136434112354328L;
822

9-
public CfnInvalidTypeConfigurationException(String resourceTypeName, String message) {
10-
super(String.format(HandlerErrorCode.InvalidTypeConfiguration.getMessage(), resourceTypeName, message),
11-
null, HandlerErrorCode.InvalidTypeConfiguration);
23+
public CfnInvalidTypeConfigurationException(String resourceTypeName,
24+
String message) {
25+
super(String.format(HandlerErrorCode.InvalidTypeConfiguration.getMessage(), resourceTypeName, message), null,
26+
HandlerErrorCode.InvalidTypeConfiguration);
1227
}
1328
}

src/test/java/software/amazon/cloudformation/WrapperTest.java

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ public void invokeHandler_InProgress_returnsInProgress(final String requestDataP
435435
@ParameterizedTest
436436
@CsvSource({ "create.with-callback-context.request.json,CREATE", "update.with-callback-context.request.json,UPDATE",
437437
"delete.with-callback-context.request.json,DELETE" })
438-
public void reInvokeHandler_InProgress_returnsInProgress(final String requestDataPath, final String actionAsString)
438+
public void reInvokeHandler_InProgress_returnsInProgressWithContext(final String requestDataPath, final String actionAsString)
439439
throws IOException {
440440
final Action action = Action.valueOf(actionAsString);
441441
final TestModel model = TestModel.builder().property1("abc").property2(123).build();
@@ -445,8 +445,55 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat
445445
// interval >= 1 minute is scheduled
446446
// against CloudWatch. Shorter intervals are able to run locally within same
447447
// function context if runtime permits
448-
final ProgressEvent<TestModel, TestContext> pe = ProgressEvent.<TestModel, TestContext>builder()
449-
.status(OperationStatus.IN_PROGRESS).callbackDelaySeconds(60).resourceModel(model).build();
448+
final ProgressEvent<TestModel,
449+
TestContext> pe = ProgressEvent.<TestModel, TestContext>builder().status(OperationStatus.IN_PROGRESS)
450+
.callbackDelaySeconds(60).resourceModel(model)
451+
.callbackContext(TestContext.builder().contextPropertyA("Value").build()).build();
452+
wrapper.setInvokeHandlerResponse(pe);
453+
454+
wrapper.setTransformResponse(resourceHandlerRequest);
455+
456+
try (final InputStream in = loadRequestStream(requestDataPath); final OutputStream out = new ByteArrayOutputStream()) {
457+
wrapper.processRequest(in, out);
458+
459+
// verify initialiseRuntime was called and initialised dependencies
460+
verifyInitialiseRuntime();
461+
462+
// all metrics should be published, once for a single invocation
463+
verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action));
464+
verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong());
465+
verify(providerMetricsPublisher).publishExceptionByErrorCodeAndCountBulkMetrics(any(Instant.class), eq(action),
466+
any());
467+
468+
// validation failure metric should not be published
469+
verifyNoMoreInteractions(providerMetricsPublisher);
470+
471+
// verify that NO model validation occurred for CREATE/UPDATE
472+
verifyNoMoreInteractions(validator);
473+
474+
// verify output response
475+
verifyHandlerResponse(out, ProgressEvent.<TestModel, TestContext>builder().status(OperationStatus.IN_PROGRESS)
476+
.resourceModel(TestModel.builder().property1("abc").property2(123).build()).build());
477+
}
478+
}
479+
480+
@ParameterizedTest
481+
@CsvSource({ "create.without-callback-context.request.json,CREATE", "update.without-callback-context.request.json,UPDATE" })
482+
public void reInvokeHandler_InProgress_returnsInProgressWithoutContext(final String requestDataPath,
483+
final String actionAsString)
484+
throws IOException {
485+
final Action action = Action.valueOf(actionAsString);
486+
final TestModel model = TestModel.builder().property1("abc").property2(123).build();
487+
488+
// an InProgress response is always re-scheduled.
489+
// If no explicit time is supplied, a 1-minute interval is used, and any
490+
// interval >= 1 minute is scheduled
491+
// against CloudWatch. Shorter intervals are able to run locally within same
492+
// function context if runtime permits
493+
final ProgressEvent<TestModel,
494+
TestContext> pe = ProgressEvent.<TestModel, TestContext>builder().status(OperationStatus.IN_PROGRESS)
495+
.callbackDelaySeconds(60).resourceModel(model)
496+
.callbackContext(TestContext.builder().contextPropertyA("Value").build()).build();
450497
wrapper.setInvokeHandlerResponse(pe);
451498

452499
wrapper.setTransformResponse(resourceHandlerRequest);
@@ -467,9 +514,7 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat
467514
verifyNoMoreInteractions(providerMetricsPublisher);
468515

469516
// verify that model validation occurred for CREATE/UPDATE
470-
if (action == Action.CREATE || action == Action.UPDATE) {
471-
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
472-
}
517+
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
473518

474519
// verify output response
475520
verifyHandlerResponse(out, ProgressEvent.<TestModel, TestContext>builder().status(OperationStatus.IN_PROGRESS)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
{
2+
"awsAccountId": "123456789012",
3+
"bearerToken": "123456",
4+
"region": "us-east-1",
5+
"action": "CREATE",
6+
"responseEndpoint": "https://cloudformation.us-west-2.amazonaws.com",
7+
"resourceType": "AWS::Test::TestModel",
8+
"resourceTypeVersion": "1.0",
9+
"requestData": {
10+
"callerCredentials": {
11+
"accessKeyId": "IASAYK835GAIFHAHEI23",
12+
"secretAccessKey": "66iOGPN5LnpZorcLr8Kh25u8AbjHVllv5/poh2O0",
13+
"sessionToken": "lameHS2vQOknSHWhdFYTxm2eJc1JMn9YBNI4nV4mXue945KPL6DHfW8EsUQT5zwssYEC1NvYP9yD6Y5s5lKR3chflOHPFsIe6eqg"
14+
},
15+
"providerCredentials": {
16+
"accessKeyId": "HDI0745692Y45IUTYR78",
17+
"secretAccessKey": "4976TUYVI234/5GW87ERYG823RF87GY9EIUH452I3",
18+
"sessionToken": "842HYOFIQAEUDF78R8T7IU43HSADYGIFHBJSDHFA87SDF9PYvN1CEYASDUYFT5TQ97YASIHUDFAIUEYRISDKJHFAYSUDTFSDFADS"
19+
},
20+
"providerLogGroupName": "providerLoggingGroupName",
21+
"logicalResourceId": "myBucket",
22+
"resourceProperties": {},
23+
"systemTags": {
24+
"aws:cloudformation:stack-id": "SampleStack"
25+
},
26+
"stackTags": {
27+
"tag1": "abc"
28+
},
29+
"previousStackTags": {
30+
"tag1": "def"
31+
}
32+
},
33+
"stackId": "arn:aws:cloudformation:us-east-1:123456789012:stack/SampleStack/e722ae60-fe62-11e8-9a0e-0ae8cc519968"
34+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"awsAccountId": "123456789012",
3+
"bearerToken": "123456",
4+
"region": "us-east-1",
5+
"action": "UPDATE",
6+
"responseEndpoint": "https://cloudformation.us-west-2.amazonaws.com",
7+
"resourceType": "AWS::Test::TestModel",
8+
"resourceTypeVersion": "1.0",
9+
"requestData": {
10+
"callerCredentials": {
11+
"accessKeyId": "IASAYK835GAIFHAHEI23",
12+
"secretAccessKey": "66iOGPN5LnpZorcLr8Kh25u8AbjHVllv5/poh2O0",
13+
"sessionToken": "lameHS2vQOknSHWhdFYTxm2eJc1JMn9YBNI4nV4mXue945KPL6DHfW8EsUQT5zwssYEC1NvYP9yD6Y5s5lKR3chflOHPFsIe6eqg"
14+
},
15+
"providerCredentials": {
16+
"accessKeyId": "HDI0745692Y45IUTYR78",
17+
"secretAccessKey": "4976TUYVI234/5GW87ERYG823RF87GY9EIUH452I3",
18+
"sessionToken": "842HYOFIQAEUDF78R8T7IU43HSADYGIFHBJSDHFA87SDF9PYvN1CEYASDUYFT5TQ97YASIHUDFAIUEYRISDKJHFAYSUDTFSDFADS"
19+
},
20+
"providerLogGroupName": "providerLoggingGroupName",
21+
"logicalResourceId": "myBucket",
22+
"resourceProperties": {},
23+
"previousResourceProperties": {},
24+
"systemTags": {
25+
"aws:cloudformation:stack-id": "SampleStack"
26+
},
27+
"stackTags": {
28+
"tag1": "abc"
29+
},
30+
"previousStackTags": {
31+
"tag1": "def"
32+
}
33+
},
34+
"stackId": "arn:aws:cloudformation:us-east-1:123456789012:stack/SampleStack/e722ae60-fe62-11e8-9a0e-0ae8cc519968"
35+
}

src/test/java/software/amazon/cloudformation/exceptions/CfnInvalidTypeConfigurationExceptionTests.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
/*
2+
* Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
115
package software.amazon.cloudformation.exceptions;
216

317
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -9,8 +23,7 @@ public class CfnInvalidTypeConfigurationExceptionTests {
923
public void cfnInvalidTypeConfigurationException_isBaseHandlerException() {
1024
assertThatExceptionOfType(BaseHandlerException.class).isThrownBy(() -> {
1125
throw new CfnInvalidTypeConfigurationException("AWS::Type::Resource", "<request>");
12-
}).withNoCause().withMessageContaining("<request>")
13-
.withMessageContaining("AWS::Type::Resource")
26+
}).withNoCause().withMessageContaining("<request>").withMessageContaining("AWS::Type::Resource")
1427
.withMessageContaining("Invalid TypeConfiguration");
1528
}
1629

0 commit comments

Comments
 (0)