From ebc3a127bea7034ac80ad743e43c45e39fc836e0 Mon Sep 17 00:00:00 2001 From: Mark Kuhn Date: Wed, 5 Apr 2023 09:09:23 -0700 Subject: [PATCH] change to update strategy & bux fixes (#106) --- .../aws-logs-destination.json | 4 +- .../logs/destination/BaseHandlerStd.java | 84 +++++++--- .../logs/destination/ClientBuilder.java | 42 ++++- .../logs/destination/CreateHandler.java | 9 +- .../logs/destination/DeleteHandler.java | 16 +- .../amazon/logs/destination/ListHandler.java | 4 +- .../amazon/logs/destination/ReadHandler.java | 20 +-- .../amazon/logs/destination/Translator.java | 1 - .../logs/destination/UpdateHandler.java | 14 +- .../logs/destination/AbstractTestBase.java | 2 +- .../logs/destination/CreateHandlerTest.java | 22 ++- .../logs/destination/ListHandlerTest.java | 7 +- .../logs/destination/TranslatorTests.java | 39 ++--- .../logs/destination/UpdateHandlerTest.java | 2 - .../aws-logs-metricfilter.json | 1 + .../logs/metricfilter/BaseHandlerStdTest.java | 4 +- .../aws-logs-subscriptionfilter.json | 17 +- aws-logs-subscriptionfilter/docs/README.md | 10 +- .../subscriptionfilter/ClientBuilder.java | 48 +++--- .../subscriptionfilter/CreateHandler.java | 22 ++- .../logs/subscriptionfilter/Translator.java | 2 - .../subscriptionfilter/CreateHandlerTest.java | 146 ++++++++++++++++-- .../subscriptionfilter/TranslatorTest.java | 4 - 23 files changed, 354 insertions(+), 166 deletions(-) diff --git a/aws-logs-destination/aws-logs-destination.json b/aws-logs-destination/aws-logs-destination.json index 2f89196a..b9bd84ba 100644 --- a/aws-logs-destination/aws-logs-destination.json +++ b/aws-logs-destination/aws-logs-destination.json @@ -2,8 +2,8 @@ "typeName": "AWS::Logs::Destination", "description": "The AWS::Logs::Destination resource specifies a CloudWatch Logs destination. A destination encapsulates a physical resource (such as an Amazon Kinesis data stream) and enables you to subscribe that resource to a stream of log events.", "sourceUrl": "https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-logs.git", - "tagging" : { - "taggable" : false + "tagging": { + "taggable": false }, "properties": { "Arn": { diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/BaseHandlerStd.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/BaseHandlerStd.java index a3b69a0f..83ae16d2 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/BaseHandlerStd.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/BaseHandlerStd.java @@ -6,9 +6,10 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeDestinationsResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.InvalidParameterException; import software.amazon.awssdk.services.cloudwatchlogs.model.PutDestinationPolicyResponse; -import software.amazon.awssdk.services.cloudwatchlogs.model.PutDestinationResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException; import software.amazon.awssdk.services.cloudwatchlogs.model.ServiceUnavailableException; +import software.amazon.awssdk.services.cloudwatchlogs.model.OperationAbortedException; +import software.amazon.awssdk.services.cloudwatchlogs.model.LimitExceededException; import software.amazon.cloudformation.Action; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.CallChain; @@ -18,19 +19,30 @@ import software.amazon.cloudformation.proxy.ProxyClient; import software.amazon.cloudformation.proxy.ResourceHandlerRequest; +import java.util.NoSuchElementException; + public abstract class BaseHandlerStd extends BaseHandler { @Override - public final ProgressEvent handleRequest(final AmazonWebServicesClientProxy proxy, - final ResourceHandlerRequest request, final CallbackContext callbackContext, + public final ProgressEvent handleRequest( + final AmazonWebServicesClientProxy proxy, + final ResourceHandlerRequest request, + final CallbackContext callbackContext, final Logger logger) { - return handleRequest(proxy, request, callbackContext != null ? callbackContext : new CallbackContext(), - proxy.newProxy(ClientBuilder::getClient), logger); + return handleRequest( + proxy, + request, + callbackContext != null ? callbackContext : new CallbackContext(), + proxy.newProxy(ClientBuilder::getClient), + logger + ); } protected abstract ProgressEvent handleRequest( - final AmazonWebServicesClientProxy proxy, final ResourceHandlerRequest request, - final CallbackContext callbackContext, final ProxyClient proxyClient, + final AmazonWebServicesClientProxy proxy, + final ResourceHandlerRequest request, + final CallbackContext callbackContext, + final ProxyClient proxyClient, final Logger logger); protected CallChain.Completed preCreateCheck( @@ -75,24 +87,25 @@ protected boolean destinationNameExists(final DescribeDestinationsResponse respo } protected ProgressEvent putDestination(final AmazonWebServicesClientProxy proxy, - final CallbackContext callbackContext, final ProxyClient proxyClient, - final ResourceModel model, final String callGraph, final Logger logger, Action handlerAction) { + final CallbackContext callbackContext, + final ProxyClient proxyClient, + final ResourceModel model, + String stackId, + final String callGraph, + final Logger logger) { return proxy.initiate(callGraph, proxyClient, model, callbackContext) .translateToServiceRequest(Translator::translateToPutDestinationRequest) - .makeServiceCall((awsRequest, sdkProxyClient) -> { - PutDestinationResponse putDestinationResponse = null; - try { - putDestinationResponse = proxyClient.injectCredentialsAndInvokeV2(awsRequest, - proxyClient.client()::putDestination); - logger.log(String.format("%s resource with name %s has been successfully %s", - ResourceModel.TYPE_NAME, model.getDestinationName(), handlerAction.name())); - } catch (CloudWatchLogsException e) { - logger.log(String.format( - "Exception while invoking the putDestination API for the destination ID %s. %s ", - model.getDestinationName(), e)); - Translator.translateException(e); + .makeServiceCall((destinationRequest, client) -> client + .injectCredentialsAndInvokeV2(destinationRequest, + client.client()::putDestination)) + .handleError((res, e, proxyClient1, model1, context) -> { + // Check if multiple concurrent requests to update the same resource are in conflict + if (e instanceof OperationAbortedException) { + return ProgressEvent.defaultInProgressHandler(context, 5, model); } - return putDestinationResponse; + + final HandlerErrorCode errorCode = getExceptionDetails(e, logger, stackId); + return ProgressEvent.defaultFailureHandler(e, errorCode); }) .progress(); } @@ -122,4 +135,31 @@ protected ProgressEvent putDestinationPolicy( .progress(); } + protected HandlerErrorCode getExceptionDetails(final Exception e, final Logger logger, final String stackId) { + HandlerErrorCode errorCode = HandlerErrorCode.GeneralServiceException; + if (e instanceof InvalidParameterException) { + errorCode = HandlerErrorCode.InvalidRequest; + } else if (e instanceof LimitExceededException) { + errorCode = HandlerErrorCode.ServiceLimitExceeded; + } else if (e instanceof ServiceUnavailableException) { + errorCode = HandlerErrorCode.InternalFailure; + } else if (e instanceof NoSuchElementException) { + errorCode = HandlerErrorCode.NotFound; + } + + logExceptionDetails(e, logger, stackId); + return errorCode; + } + + /** + * Log the details of the exception thrown + * + * @param e - the exception + * @param logger - a reference to the logger + * @param stackId - the id of the stack where the exception was thrown + */ + protected void logExceptionDetails(Exception e, Logger logger, final String stackId) { + logger.log(String.format("Stack with ID: %s got exception: %s Message: %s Cause: %s", stackId, + e.toString(), e.getMessage(), e.getCause())); + } } diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/ClientBuilder.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/ClientBuilder.java index e3eb2c18..39ca5e52 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/ClientBuilder.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/ClientBuilder.java @@ -1,14 +1,46 @@ package software.amazon.logs.destination; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.core.internal.retry.SdkDefaultRetrySetting; +import software.amazon.awssdk.core.retry.RetryPolicy; +import software.amazon.awssdk.core.retry.backoff.BackoffStrategy; +import software.amazon.awssdk.core.retry.backoff.EqualJitterBackoffStrategy; +import software.amazon.awssdk.core.retry.conditions.RetryCondition; import software.amazon.awssdk.services.cloudwatchlogs.CloudWatchLogsClient; -import software.amazon.cloudformation.LambdaWrapper; +import software.amazon.cloudformation.AbstractWrapper; + +import java.time.Duration; public class ClientBuilder { + private static CloudWatchLogsClient cloudWatchLogsClient; - public static CloudWatchLogsClient getClient() { - return CloudWatchLogsClient.builder() - .httpClient(LambdaWrapper.HTTP_CLIENT) - .build(); + private ClientBuilder() { } + private static final BackoffStrategy BACKOFF_STRATEGY = + EqualJitterBackoffStrategy.builder() + .baseDelay(Duration.ofSeconds(2)) + .maxBackoffTime(SdkDefaultRetrySetting.MAX_BACKOFF) + .build(); + + private static final RetryPolicy RETRY_POLICY = + RetryPolicy.builder() + .numRetries(4) + .retryCondition(RetryCondition.defaultRetryCondition()) + .throttlingBackoffStrategy(BACKOFF_STRATEGY) + .build(); + + public static CloudWatchLogsClient getClient() { + if (cloudWatchLogsClient == null) { + cloudWatchLogsClient = CloudWatchLogsClient.builder() + .httpClient(AbstractWrapper.HTTP_CLIENT) + .overrideConfiguration(ClientOverrideConfiguration.builder() + .retryPolicy(RETRY_POLICY) + .apiCallTimeout(Duration.ofSeconds(55)) + .build()) + .build(); + return cloudWatchLogsClient; + } + return cloudWatchLogsClient; + } } diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/CreateHandler.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/CreateHandler.java index 124a95f2..9bf4aa91 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/CreateHandler.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/CreateHandler.java @@ -16,13 +16,15 @@ public class CreateHandler extends BaseHandlerStd { public static final String DESTINATION_POLICY_CREATE_GRAPH = "AWS-Logs-DestinationPolicy::Create"; - protected ProgressEvent handleRequest(final AmazonWebServicesClientProxy proxy, + protected ProgressEvent handleRequest( + final AmazonWebServicesClientProxy proxy, final ResourceHandlerRequest request, final CallbackContext callbackContext, final ProxyClient proxyClient, final Logger logger) { final ResourceModel model = request.getDesiredResourceState(); + final String stackId = request.getStackId(); // Verify if a destination is already present with same identifier // Create destination policy command checks to see if optional destination/access policy is passed in before attempting create @@ -36,12 +38,11 @@ protected ProgressEvent handleRequest(final Amaz } return ProgressEvent.progress(model, callbackContext); })) - .then(progress -> putDestination(proxy, callbackContext, proxyClient, model, DESTINATION_CREATE_GRAPH, - logger, Action.CREATE)) + .then(progress -> putDestination(proxy, callbackContext, proxyClient, model, stackId, DESTINATION_CREATE_GRAPH, + logger)) .then(progress -> model.getDestinationPolicy() != null ? putDestinationPolicy(proxy, callbackContext, proxyClient, model, DESTINATION_POLICY_CREATE_GRAPH, logger, Action.CREATE) : progress) .then(progress -> new ReadHandler().handleRequest(proxy, request, callbackContext, proxyClient, logger)); } - } diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/DeleteHandler.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/DeleteHandler.java index 4f1b3274..6c967c15 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/DeleteHandler.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/DeleteHandler.java @@ -14,22 +14,24 @@ public class DeleteHandler extends BaseHandlerStd { private Logger logger; + public static final String DESTINATION_POLICY_DELETE_GRAPH = "AWS-Logs-Destination::Delete"; protected ProgressEvent handleRequest(final AmazonWebServicesClientProxy proxy, - final ResourceHandlerRequest request, - final CallbackContext callbackContext, - final ProxyClient proxyClient, - final Logger logger) { + final ResourceHandlerRequest request, + final CallbackContext callbackContext, + final ProxyClient proxyClient, + final Logger logger) { this.logger = logger; final ResourceModel model = request.getDesiredResourceState(); - return proxy.initiate("AWS-Logs-Destination::Delete", proxyClient, model, callbackContext) + return proxy.initiate(DESTINATION_POLICY_DELETE_GRAPH, proxyClient, model, callbackContext) .translateToServiceRequest(Translator::translateToDeleteRequest) .makeServiceCall(this::deleteResource) - .done((x)-> ProgressEvent.builder().status(OperationStatus.SUCCESS).build()); + .done(x -> ProgressEvent.builder().status(OperationStatus.SUCCESS).build()); } - private DeleteDestinationResponse deleteResource(final DeleteDestinationRequest awsRequest, + private DeleteDestinationResponse deleteResource( + final DeleteDestinationRequest awsRequest, final ProxyClient proxyClient) { DeleteDestinationResponse awsResponse = null; diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/ListHandler.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/ListHandler.java index 117e71ce..60047c7b 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/ListHandler.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/ListHandler.java @@ -13,6 +13,7 @@ public class ListHandler extends BaseHandlerStd { private Logger logger; + public static final String DESTINATION_POLICY_LIST_GRAPH = "AWS-Logs-Destination::List"; @Override public ProgressEvent handleRequest( @@ -23,7 +24,7 @@ public ProgressEvent handleRequest( final Logger logger) { this.logger = logger; - return proxy.initiate("AWS-Logs-Destination::List", proxyClient, request.getDesiredResourceState(), callbackContext) + return proxy.initiate(DESTINATION_POLICY_LIST_GRAPH, proxyClient, request.getDesiredResourceState(), callbackContext) .translateToServiceRequest(Translator::translateToListRequest) .makeServiceCall((describeDestinationsRequest, client) -> { DescribeDestinationsResponse awsResponse = null; @@ -40,5 +41,4 @@ public ProgressEvent handleRequest( .nextToken(describeDestinationsResponse.nextToken()) .build()); } - } diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/ReadHandler.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/ReadHandler.java index efce358b..47eb3a59 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/ReadHandler.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/ReadHandler.java @@ -16,10 +16,10 @@ public class ReadHandler extends BaseHandlerStd { private Logger logger; protected ProgressEvent handleRequest(final AmazonWebServicesClientProxy proxy, - final ResourceHandlerRequest request, - final CallbackContext callbackContext, - final ProxyClient proxyClient, - final Logger logger) { + final ResourceHandlerRequest request, + final CallbackContext callbackContext, + final ProxyClient proxyClient, + final Logger logger) { this.logger = logger; final ResourceModel model = request.getDesiredResourceState(); @@ -31,8 +31,8 @@ protected ProgressEvent handleRequest(final Amaz } private DescribeDestinationsResponse readResource(final DescribeDestinationsRequest awsRequest, - final ProxyClient proxyClient, - final ResourceModel model) { + final ProxyClient proxyClient, + final ResourceModel model) { DescribeDestinationsResponse awsResponse = null; try { @@ -49,10 +49,10 @@ private DescribeDestinationsResponse readResource(final DescribeDestinationsRequ } private ProgressEvent constructResourceModelFromResponse(DescribeDestinationsRequest describeDestinationsRequest, - DescribeDestinationsResponse describeDestinationsResponse, - ProxyClient cloudWatchLogsClientProxyClient, - ResourceModel resourceModel, - CallbackContext callbackContext) { + DescribeDestinationsResponse describeDestinationsResponse, + ProxyClient cloudWatchLogsClientProxyClient, + ResourceModel resourceModel, + CallbackContext callbackContext) { ResourceModel translatedResourceModel = Translator.translateFromReadResponse(describeDestinationsResponse); if (translatedResourceModel == null) { diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/Translator.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/Translator.java index 4d4a71c1..53fb1009 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/Translator.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/Translator.java @@ -110,5 +110,4 @@ static void translateException(AwsServiceException exception) { } throw new CfnGeneralServiceException(exception); } - } diff --git a/aws-logs-destination/src/main/java/software/amazon/logs/destination/UpdateHandler.java b/aws-logs-destination/src/main/java/software/amazon/logs/destination/UpdateHandler.java index 01b4a92f..9e9d1ba5 100644 --- a/aws-logs-destination/src/main/java/software/amazon/logs/destination/UpdateHandler.java +++ b/aws-logs-destination/src/main/java/software/amazon/logs/destination/UpdateHandler.java @@ -17,11 +17,12 @@ public class UpdateHandler extends BaseHandlerStd { private static final String DESTINATION_POLICY_UPDATE_GRAPH = "AWS-Logs-DestinationPolicy::Update"; protected ProgressEvent handleRequest(final AmazonWebServicesClientProxy proxy, - final ResourceHandlerRequest request, - final CallbackContext callbackContext, - final ProxyClient proxyClient, - final Logger logger) { + final ResourceHandlerRequest request, + final CallbackContext callbackContext, + final ProxyClient proxyClient, + final Logger logger) { final ResourceModel model = request.getDesiredResourceState(); + final String stackId = request.getStackId(); return ProgressEvent.progress(model, callbackContext) .then(progress -> preCreateCheck(proxy, callbackContext, proxyClient, model).done(response -> { @@ -33,12 +34,11 @@ protected ProgressEvent handleRequest(final Amaz } return ProgressEvent.progress(model, callbackContext); })) - .then(progress -> putDestination(proxy, callbackContext, proxyClient, model, DESTINATION_UPDATE_GRAPH, - logger, Action.UPDATE)) + .then(progress -> putDestination(proxy, callbackContext, proxyClient, model, stackId, DESTINATION_UPDATE_GRAPH, + logger)) .then(progress -> putDestinationPolicy(proxy, callbackContext, proxyClient, model, DESTINATION_POLICY_UPDATE_GRAPH, logger, Action.UPDATE)) .then(progress -> new ReadHandler().handleRequest(proxy, request, callbackContext, proxyClient, logger)); } - } diff --git a/aws-logs-destination/src/test/java/software/amazon/logs/destination/AbstractTestBase.java b/aws-logs-destination/src/test/java/software/amazon/logs/destination/AbstractTestBase.java index 2a6f79a9..1d27a851 100644 --- a/aws-logs-destination/src/test/java/software/amazon/logs/destination/AbstractTestBase.java +++ b/aws-logs-destination/src/test/java/software/amazon/logs/destination/AbstractTestBase.java @@ -35,7 +35,7 @@ public class AbstractTestBase { } static ProxyClient MOCK_PROXY(final AmazonWebServicesClientProxy proxy, - final CloudWatchLogsClient sdkClient) { + final CloudWatchLogsClient sdkClient) { return new ProxyClient() { @Override diff --git a/aws-logs-destination/src/test/java/software/amazon/logs/destination/CreateHandlerTest.java b/aws-logs-destination/src/test/java/software/amazon/logs/destination/CreateHandlerTest.java index 1c0ca3fc..a7f5c283 100644 --- a/aws-logs-destination/src/test/java/software/amazon/logs/destination/CreateHandlerTest.java +++ b/aws-logs-destination/src/test/java/software/amazon/logs/destination/CreateHandlerTest.java @@ -204,14 +204,14 @@ public void handleRequest_Should_ReturnFailureProgressEvent_When_DestinationIsFo @Test public void handleRequest_Should_ThrowCfnResourceConflictException_When_PutOperationIsAborted() { - final DescribeDestinationsResponse describeResponse = DescribeDestinationsResponse.builder() - .destinations(destination) - .build(); - Mockito.when(proxyClient.client() .describeDestinations(any(DescribeDestinationsRequest.class))) .thenThrow(ResourceNotFoundException.class) - .thenReturn(describeResponse); + .thenReturn(DescribeDestinationsResponse + .builder() + .destinations(destination) + .build()); + Mockito.when(proxyClient.client() .putDestination(ArgumentMatchers.any(PutDestinationRequest.class))) .thenThrow(OperationAbortedException.class); @@ -220,9 +220,15 @@ public void handleRequest_Should_ThrowCfnResourceConflictException_When_PutOpera getDefaultRequestBuilder().desiredResourceState(testResourceModel) .build(); - Assertions.assertThrows(CfnResourceConflictException.class, - () -> handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger)); + final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.IN_PROGRESS); + assertThat(response.getCallbackDelaySeconds()).isNotZero(); + assertThat(response.getResourceModel()).isNotNull(); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); } @Test @@ -286,6 +292,7 @@ public void handleRequest_Should_ReturnFailureProgressEvent_When_DestinationRead assertThat(progressEvent.getStatus()).isEqualTo(OperationStatus.FAILED); assertThat(progressEvent.getErrorCode()).isEqualTo(HandlerErrorCode.GeneralServiceException); } + // tests for optional parameter, destination policy not provided tests @Test public void handleRequest_Should_ReturnSuccess_When_DestinationNotFound_and_DestinationPolicyNotProvided() { @@ -404,5 +411,4 @@ public void handleRequest_Should_ReturnSuccess_When_DescribeDestinationsResponse assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } - } diff --git a/aws-logs-destination/src/test/java/software/amazon/logs/destination/ListHandlerTest.java b/aws-logs-destination/src/test/java/software/amazon/logs/destination/ListHandlerTest.java index 8d078eb0..b7a763cf 100644 --- a/aws-logs-destination/src/test/java/software/amazon/logs/destination/ListHandlerTest.java +++ b/aws-logs-destination/src/test/java/software/amazon/logs/destination/ListHandlerTest.java @@ -29,9 +29,9 @@ public class ListHandlerTest extends AbstractTestBase { @Mock private CloudWatchLogsClient sdkClient; - + private AmazonWebServicesClientProxy proxy; - + private ProxyClient proxyClient; private ResourceModel testResourceModel; @@ -55,7 +55,7 @@ public void handleRequest_ShouldReturnSuccess_When_DestinationIsFound() { final DescribeDestinationsResponse describeResponse = DescribeDestinationsResponse.builder() .destinations(getTestDestination()) .build(); - + Mockito.when(proxyClient.client() .describeDestinations(any(DescribeDestinationsRequest.class))) .thenReturn(describeResponse); @@ -93,5 +93,4 @@ public void handleRequest_ShouldThrowInternalFailureException_When_ServiceIsUnav assertThrows(CfnServiceInternalErrorException.class, () -> handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger)); } - } diff --git a/aws-logs-destination/src/test/java/software/amazon/logs/destination/TranslatorTests.java b/aws-logs-destination/src/test/java/software/amazon/logs/destination/TranslatorTests.java index 3a439906..0d99300b 100644 --- a/aws-logs-destination/src/test/java/software/amazon/logs/destination/TranslatorTests.java +++ b/aws-logs-destination/src/test/java/software/amazon/logs/destination/TranslatorTests.java @@ -8,7 +8,6 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeDestinationsRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeDestinationsResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.InvalidParameterException; -import software.amazon.awssdk.services.cloudwatchlogs.model.OperationAbortedException; import software.amazon.awssdk.services.cloudwatchlogs.model.PutDestinationPolicyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.PutDestinationRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException; @@ -16,7 +15,6 @@ import software.amazon.cloudformation.exceptions.CfnGeneralServiceException; import software.amazon.cloudformation.exceptions.CfnInvalidRequestException; import software.amazon.cloudformation.exceptions.CfnNotFoundException; -import software.amazon.cloudformation.exceptions.CfnResourceConflictException; import software.amazon.cloudformation.exceptions.CfnServiceInternalErrorException; import java.util.Arrays; @@ -29,6 +27,8 @@ public class TranslatorTests extends AbstractTestBase { private ResourceModel resourceModel; + private static final String TOKEN = "token"; + @BeforeEach public void setup() { resourceModel = getTestResourceModel(); @@ -103,9 +103,9 @@ public void translateToReadResponse_Should_ReturnNull_When_DestinationIsEmpty() public void translateFromListResponse_Should_ReturnSuccess() { final DescribeDestinationsResponse response = DescribeDestinationsResponse.builder() .destinations(Collections.singletonList(getTestDestination())) - .nextToken("token") + .nextToken(TOKEN) .build(); - final List expectedModels = Arrays.asList(getTestResourceModel()); + final List expectedModels = Collections.singletonList(getTestResourceModel()); Assertions.assertThat(Translator.translateFromListResponse(response)) .isEqualTo(expectedModels); } @@ -114,7 +114,7 @@ public void translateFromListResponse_Should_ReturnSuccess() { public void translateFromListResponse_Should_ReturnEmptyList_WhenDestinationsIsEmpty() { final DescribeDestinationsResponse response = DescribeDestinationsResponse.builder() .destinations(Collections.emptyList()) - .nextToken("token") + .nextToken(TOKEN) .build(); final List expectedModels = Collections.emptyList(); Assertions.assertThat(Translator.translateFromListResponse(response)) @@ -124,7 +124,7 @@ public void translateFromListResponse_Should_ReturnEmptyList_WhenDestinationsIsE @Test public void translateFromListResponse_Should_ReturnEmptyList_WhenDestinationsIsNull() { final DescribeDestinationsResponse response = DescribeDestinationsResponse.builder() - .nextToken("token") + .nextToken(TOKEN) .build(); final List expectedModels = Collections.emptyList(); Assertions.assertThat(Translator.translateFromListResponse(response)) @@ -141,36 +141,25 @@ public void translateToListRequest_Should_ReturnSuccess() { @Test public void translateException_Should_ThrowCfnInvalidRequestException() { - assertThrows(CfnInvalidRequestException.class, () -> Translator.translateException( - InvalidParameterException.builder() - .build())); + InvalidParameterException exception = InvalidParameterException.builder().build(); + assertThrows(CfnInvalidRequestException.class, () -> Translator.translateException(exception)); } @Test public void translateException_Should_ThrowCfnServiceInternalErrorException() { - assertThrows(CfnServiceInternalErrorException.class, () -> Translator.translateException( - ServiceUnavailableException.builder() - .build())); - } - - @Test - public void translateException_Should_ThrowCfnResourceConflictException() { - assertThrows(CfnResourceConflictException.class, () -> Translator.translateException( - OperationAbortedException.builder() - .build())); + ServiceUnavailableException exception = ServiceUnavailableException.builder().build(); + assertThrows(CfnServiceInternalErrorException.class, () -> Translator.translateException(exception)); } @Test public void translateException_Should_ThrowCfnNotFoundException() { - assertThrows(CfnNotFoundException.class, () -> Translator.translateException(ResourceNotFoundException.builder() - .build())); + ResourceNotFoundException exception = ResourceNotFoundException.builder().build(); + assertThrows(CfnNotFoundException.class, () -> Translator.translateException(exception)); } @Test public void translateException_Should_ThrowCfnGeneralServiceException() { - assertThrows(CfnGeneralServiceException.class, () -> Translator.translateException( - CloudWatchLogsException.builder() - .build())); + CloudWatchLogsException exception = (CloudWatchLogsException) CloudWatchLogsException.builder().build(); + assertThrows(CfnGeneralServiceException.class, () -> Translator.translateException(exception)); } - } diff --git a/aws-logs-destination/src/test/java/software/amazon/logs/destination/UpdateHandlerTest.java b/aws-logs-destination/src/test/java/software/amazon/logs/destination/UpdateHandlerTest.java index 01c1773f..aaa00008 100644 --- a/aws-logs-destination/src/test/java/software/amazon/logs/destination/UpdateHandlerTest.java +++ b/aws-logs-destination/src/test/java/software/amazon/logs/destination/UpdateHandlerTest.java @@ -259,6 +259,4 @@ public void handleRequest_Should_ReturnSuccess_When_DestinationIsFound_and_Desti assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } - - } diff --git a/aws-logs-metricfilter/aws-logs-metricfilter.json b/aws-logs-metricfilter/aws-logs-metricfilter.json index 337ff34c..80e94753 100644 --- a/aws-logs-metricfilter/aws-logs-metricfilter.json +++ b/aws-logs-metricfilter/aws-logs-metricfilter.json @@ -12,6 +12,7 @@ "tagging" : { "taggable" : false }, + "replacementStrategy": "delete_then_create", "definitions": { "Dimension": { "description": "the key-value pairs that further define a metric.", diff --git a/aws-logs-metricfilter/src/test/java/software/amazon/logs/metricfilter/BaseHandlerStdTest.java b/aws-logs-metricfilter/src/test/java/software/amazon/logs/metricfilter/BaseHandlerStdTest.java index aed605ea..1d6ab526 100644 --- a/aws-logs-metricfilter/src/test/java/software/amazon/logs/metricfilter/BaseHandlerStdTest.java +++ b/aws-logs-metricfilter/src/test/java/software/amazon/logs/metricfilter/BaseHandlerStdTest.java @@ -114,7 +114,7 @@ void handleNonRetryableError() { assertThat(response).isNotNull(); assertThat(response.getStatus()).isEqualTo(OperationStatus.FAILED); assertThat(response.getCallbackDelaySeconds()).isZero(); - assertThat(response.getMessage()).contains("Invalid request (Service: CloudWatchLogs, Status Code: 400, Request ID: e689f8f9-bc25-48de-86be-4cee73125707"); + assertEquals("Invalid request (Service: CloudWatchLogs, Status Code: 400, Request ID: e689f8f9-bc25-48de-86be-4cee73125707)", response.getMessage()); assertThat(response.getErrorCode()).isNotNull(); verify(proxyClient.client(), never()).putMetricFilter(any(PutMetricFilterRequest.class)); } @@ -151,4 +151,4 @@ private static Stream exists() { Arguments.of("filter-name", "", false) ); } -} +} \ No newline at end of file diff --git a/aws-logs-subscriptionfilter/aws-logs-subscriptionfilter.json b/aws-logs-subscriptionfilter/aws-logs-subscriptionfilter.json index 351f4650..ae8017f3 100644 --- a/aws-logs-subscriptionfilter/aws-logs-subscriptionfilter.json +++ b/aws-logs-subscriptionfilter/aws-logs-subscriptionfilter.json @@ -1,7 +1,7 @@ { "typeName": "AWS::Logs::SubscriptionFilter", - "$schema": "https://raw.githubusercontent.com/aws-cloudformation/cloudformation-resource-schema/blob/master/src/main/resources/schema/provider.definition.schema.v1.json", - "description": "An example resource schema demonstrating some basic constructs and validation rules.", + "$schema": "https://raw.githubusercontent.com/aws-cloudformation/cloudformation-cli/master/src/rpdk/core/data/schema/provider.definition.schema.v1.json", + "description": "Subscription filters allow you to subscribe to a real-time stream of log events and have them delivered to a specific destination.", "sourceUrl": "https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-logs", "tagging": { "taggable": false, @@ -9,6 +9,7 @@ "tagUpdatable": false, "cloudFormationSystemTags": false }, + "replacementStrategy": "delete_then_create", "properties": { "FilterName": { "description": "The name of the filter generated by resource.", @@ -52,6 +53,12 @@ "logs:DescribeSubscriptionFilters" ] }, + "update": { + "permissions": [ + "logs:PutSubscriptionFilter", + "logs:DescribeSubscriptionFilters" + ] + }, "delete": { "permissions": [ "logs:DeleteSubscriptionFilter" @@ -70,11 +77,7 @@ ], "createOnlyProperties": [ "/properties/FilterName", - "/properties/DestinationArn", - "/properties/FilterPattern", - "/properties/LogGroupName", - "/properties/RoleArn", - "/properties/Distribution" + "/properties/LogGroupName" ], "primaryIdentifier": [ "/properties/FilterName", diff --git a/aws-logs-subscriptionfilter/docs/README.md b/aws-logs-subscriptionfilter/docs/README.md index f28dcf4b..a7bb5414 100644 --- a/aws-logs-subscriptionfilter/docs/README.md +++ b/aws-logs-subscriptionfilter/docs/README.md @@ -1,6 +1,6 @@ # AWS::Logs::SubscriptionFilter -An example resource schema demonstrating some basic constructs and validation rules. +Subscription filters allow you to subscribe to a real-time stream of log events and have them delivered to a specific destination. ## Syntax @@ -55,7 +55,7 @@ _Required_: Yes _Type_: String -_Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement) +_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) #### FilterPattern @@ -65,7 +65,7 @@ _Required_: Yes _Type_: String -_Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement) +_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) #### LogGroupName @@ -85,7 +85,7 @@ _Required_: No _Type_: String -_Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement) +_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) #### Distribution @@ -97,5 +97,5 @@ _Type_: String _Allowed Values_: Random | ByLogStream -_Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement) +_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) diff --git a/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/ClientBuilder.java b/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/ClientBuilder.java index 0ada74d4..b0866cdc 100644 --- a/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/ClientBuilder.java +++ b/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/ClientBuilder.java @@ -7,37 +7,37 @@ import software.amazon.awssdk.core.retry.backoff.EqualJitterBackoffStrategy; import software.amazon.awssdk.core.retry.conditions.RetryCondition; import software.amazon.awssdk.services.cloudwatchlogs.CloudWatchLogsClient; -import software.amazon.cloudformation.LambdaWrapper; +import software.amazon.cloudformation.AbstractWrapper; import java.time.Duration; public class ClientBuilder { - private static CloudWatchLogsClient cloudWatchLogsClient; + private static CloudWatchLogsClient cloudWatchLogsClient; - private static final BackoffStrategy BACKOFF_STRATEGY = - EqualJitterBackoffStrategy.builder() - .baseDelay(Duration.ofSeconds(2)) - .maxBackoffTime(SdkDefaultRetrySetting.MAX_BACKOFF) - .build(); + private static final BackoffStrategy BACKOFF_STRATEGY = + EqualJitterBackoffStrategy.builder() + .baseDelay(Duration.ofSeconds(2)) + .maxBackoffTime(SdkDefaultRetrySetting.MAX_BACKOFF) + .build(); - private static final RetryPolicy RETRY_POLICY = - RetryPolicy.builder() - .numRetries(4) - .retryCondition(RetryCondition.defaultRetryCondition()) - .throttlingBackoffStrategy(BACKOFF_STRATEGY) - .build(); + private static final RetryPolicy RETRY_POLICY = + RetryPolicy.builder() + .numRetries(4) + .retryCondition(RetryCondition.defaultRetryCondition()) + .throttlingBackoffStrategy(BACKOFF_STRATEGY) + .build(); - public static CloudWatchLogsClient getClient() { - if (cloudWatchLogsClient == null) { - cloudWatchLogsClient = CloudWatchLogsClient.builder() - .httpClient(LambdaWrapper.HTTP_CLIENT) - .overrideConfiguration(ClientOverrideConfiguration.builder() - .retryPolicy(RETRY_POLICY) - .apiCallTimeout(Duration.ofSeconds(55)) - .build()) - .build(); + public static CloudWatchLogsClient getClient() { + if (cloudWatchLogsClient == null) { + cloudWatchLogsClient = CloudWatchLogsClient.builder() + .httpClient(AbstractWrapper.HTTP_CLIENT) + .overrideConfiguration(ClientOverrideConfiguration.builder() + .retryPolicy(RETRY_POLICY) + .apiCallTimeout(Duration.ofSeconds(55)) + .build()) + .build(); + return cloudWatchLogsClient; + } return cloudWatchLogsClient; - } - return cloudWatchLogsClient; } } diff --git a/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/CreateHandler.java b/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/CreateHandler.java index d5631e4f..7e20d895 100644 --- a/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/CreateHandler.java +++ b/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/CreateHandler.java @@ -4,8 +4,15 @@ import org.apache.commons.lang3.StringUtils; import software.amazon.awssdk.awscore.exception.AwsServiceException; import software.amazon.awssdk.services.cloudwatchlogs.CloudWatchLogsClient; -import software.amazon.cloudformation.exceptions.*; -import software.amazon.cloudformation.proxy.*; +import software.amazon.awssdk.services.cloudwatchlogs.model.OperationAbortedException; +import software.amazon.cloudformation.exceptions.CfnAlreadyExistsException; +import software.amazon.cloudformation.exceptions.CfnThrottlingException; +import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; +import software.amazon.cloudformation.proxy.Logger; +import software.amazon.cloudformation.proxy.ProxyClient; +import software.amazon.cloudformation.proxy.HandlerErrorCode; +import software.amazon.cloudformation.proxy.ProgressEvent; +import software.amazon.cloudformation.proxy.ResourceHandlerRequest; import software.amazon.cloudformation.resource.IdentifierUtils; public class CreateHandler extends BaseHandlerStd { @@ -74,21 +81,26 @@ protected ProgressEvent handleRequest( } return ProgressEvent.progress(model, callbackContext); })) - .then(progress -> + .then(progress -> proxy.initiate(CALL_GRAPH_STRING, proxyClient, model, callbackContext) .translateToServiceRequest(Translator::translateToCreateRequest) .makeServiceCall((filterRequest, client) -> client .injectCredentialsAndInvokeV2(filterRequest, client.client()::putSubscriptionFilter)) - .handleError((req, e, proxyClient1, model1, context) -> { + .handleError((req, e, proxyClient1, model1, context) -> { // invalid parameter exception needs to be retried - if (e instanceof AwsServiceException && ((AwsServiceException)e).awsErrorDetails() != null) { + if (e instanceof AwsServiceException && ((AwsServiceException) e).awsErrorDetails() != null) { final AwsServiceException awsServiceException = (AwsServiceException) e; if (awsServiceException.awsErrorDetails().errorCode().equals(ERROR_CODE_INVALID_PARAMETER_EXCEPTION)) { throw new CfnThrottlingException(e); } } + // Check if multiple concurrent requests to update the same resource are in conflict + if (e instanceof OperationAbortedException) { + return ProgressEvent.defaultInProgressHandler(context, 5, model1); + } + final HandlerErrorCode handlerErrorCode = getExceptionDetails(e, logger, stackId); return ProgressEvent.defaultFailureHandler(e, handlerErrorCode); }).progress() diff --git a/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/Translator.java b/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/Translator.java index befd3993..fc437156 100644 --- a/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/Translator.java +++ b/aws-logs-subscriptionfilter/src/main/java/software/amazon/logs/subscriptionfilter/Translator.java @@ -31,8 +31,6 @@ public static BaseHandlerException translateException(final AwsServiceException return new CfnInvalidRequestException(String.format("%s. %s", ResourceModel.TYPE_NAME, e.getMessage()), e); } else if (e instanceof LimitExceededException) { return new CfnServiceLimitExceededException(e); - } else if (e instanceof OperationAbortedException) { - return new CfnResourceConflictException(e); } else if (e instanceof ResourceNotFoundException) { return new CfnNotFoundException(e); } else if (e instanceof ServiceUnavailableException) { diff --git a/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/CreateHandlerTest.java b/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/CreateHandlerTest.java index 5f5a7213..6b53a400 100644 --- a/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/CreateHandlerTest.java +++ b/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/CreateHandlerTest.java @@ -7,14 +7,30 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import software.amazon.awssdk.services.cloudwatchlogs.CloudWatchLogsClient; -import software.amazon.awssdk.services.cloudwatchlogs.model.*; -import software.amazon.cloudformation.proxy.*; +import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeSubscriptionFiltersRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeSubscriptionFiltersResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.PutSubscriptionFilterRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.PutSubscriptionFilterResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException; +import software.amazon.awssdk.services.cloudwatchlogs.model.OperationAbortedException; +import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsException; +import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; +import software.amazon.cloudformation.proxy.ProxyClient; +import software.amazon.cloudformation.proxy.HandlerErrorCode; +import software.amazon.cloudformation.proxy.ProgressEvent; +import software.amazon.cloudformation.proxy.ResourceHandlerRequest; +import software.amazon.cloudformation.proxy.OperationStatus; import java.time.Duration; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.times; @ExtendWith(MockitoExtension.class) class CreateHandlerTest extends AbstractTestBase { @@ -54,15 +70,13 @@ void handleRequest_Success() { .build(); when(proxyClient.client().describeSubscriptionFilters(any(DescribeSubscriptionFiltersRequest.class))) - .thenReturn(DescribeSubscriptionFiltersResponse.builder().build()) + .thenThrow(ResourceNotFoundException.class) .thenReturn(describeResponse); when(proxyClient.client().putSubscriptionFilter(any(PutSubscriptionFilterRequest.class))) .thenReturn(createResponse); - final ResourceHandlerRequest request = ResourceHandlerRequest.builder() - .desiredResourceState(model) - .build(); + final ResourceHandlerRequest request = buildResourceHandlerRequest(model); final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); @@ -85,7 +99,7 @@ void handleRequest_Success2() { // return no existing Subscriptions for pre-create and then success response for create when(proxyClient.client().describeSubscriptionFilters(any(DescribeSubscriptionFiltersRequest.class))) - .thenReturn(DescribeSubscriptionFiltersResponse.builder().build()) + .thenThrow(ResourceNotFoundException.class) .thenReturn(DescribeSubscriptionFiltersResponse.builder() .subscriptionFilters(Translator.translateToSDK(model)) .build()); @@ -93,9 +107,7 @@ void handleRequest_Success2() { when(proxyClient.client().putSubscriptionFilter(any(PutSubscriptionFilterRequest.class))) .thenReturn(createResponse); - final ResourceHandlerRequest request = ResourceHandlerRequest.builder() - .desiredResourceState(model) - .build(); + final ResourceHandlerRequest request = buildResourceHandlerRequest(model); final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); @@ -119,7 +131,7 @@ void handleRequest_Success_WithGeneratedName() { .build(); when(proxyClient.client().describeSubscriptionFilters(any(DescribeSubscriptionFiltersRequest.class))) - .thenReturn(DescribeSubscriptionFiltersResponse.builder().build()) + .thenThrow(ResourceNotFoundException.class) .thenReturn(DescribeSubscriptionFiltersResponse.builder() .subscriptionFilters(Translator.translateToSDK(model)) .build()); @@ -127,11 +139,7 @@ void handleRequest_Success_WithGeneratedName() { when(proxyClient.client().putSubscriptionFilter(any(PutSubscriptionFilterRequest.class))) .thenReturn(PutSubscriptionFilterResponse.builder().build()); - final ResourceHandlerRequest request = ResourceHandlerRequest.builder() - .desiredResourceState(model) - .logicalResourceIdentifier("logicalResourceIdentifier") - .clientRequestToken("requestToken") - .build(); + final ResourceHandlerRequest request = buildResourceHandlerRequest(model); final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); @@ -143,4 +151,108 @@ void handleRequest_Success_WithGeneratedName() { assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } + + @Test + public void handleRequest_Should_ReturnInProgress_When_PutOperationIsAborted() { + final ResourceModel model = buildDefaultModel(); + + when(proxyClient.client().describeSubscriptionFilters(any(DescribeSubscriptionFiltersRequest.class))) + .thenThrow(ResourceNotFoundException.class) + .thenReturn(DescribeSubscriptionFiltersResponse.builder() + .subscriptionFilters(Translator.translateToSDK(model)) + .build()); + + when(proxyClient.client().putSubscriptionFilter(any(PutSubscriptionFilterRequest.class))) + .thenThrow(OperationAbortedException.class); + + final ResourceHandlerRequest request = buildResourceHandlerRequest(model); + + final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.IN_PROGRESS); + assertThat(response.getCallbackDelaySeconds()).isNotEqualTo(0); + assertThat(response.getResourceModel()).isNotNull(); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + @Test + public void handleRequest_Should_ReturnFailureProgressEvent_When_SubscriptionFilterExists() { + final ResourceModel model = buildDefaultModel(); + + when(proxyClient.client().describeSubscriptionFilters(any(DescribeSubscriptionFiltersRequest.class))) + .thenReturn(DescribeSubscriptionFiltersResponse.builder() + .subscriptionFilters(Translator.translateToSDK(model)) + .build()); + + final ResourceHandlerRequest request = buildResourceHandlerRequest(model); + + final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.FAILED); + assertThat(response.getCallbackDelaySeconds()).isZero(); + assertThat(response.getResourceModel()).isNull(); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getMessage()).isNotNull(); + assertThat(response.getErrorCode()).isNotNull(); + } + + @Test + public void handleRequest_Should_ReturnFailureProgressEvent_When_SubscriptionFilterReadFailsWithCloudWatchLogsException() { + when(proxyClient.client().describeSubscriptionFilters(any(DescribeSubscriptionFiltersRequest.class))) + .thenThrow(CloudWatchLogsException.class); + + final ResourceHandlerRequest request = buildResourceHandlerRequest(buildDefaultModel()); + + ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.FAILED); + assertThat(response.getErrorCode()).isEqualTo(HandlerErrorCode.GeneralServiceException); + } + + @Test + public void handleRequest_Should_ReturnSuccess_When_FilterEmpty_and_RoleEmpty_and_DistributionEmpty() { + final ResourceModel model = buildDefaultModel(); + model.setFilterPattern(null); + model.setRoleArn(null); + model.setDistribution(null); + + when(proxyClient.client() + .describeSubscriptionFilters(any(DescribeSubscriptionFiltersRequest.class))) + .thenThrow(ResourceNotFoundException.class) + .thenReturn(DescribeSubscriptionFiltersResponse.builder() + .subscriptionFilters(Translator.translateToSDK(model)) + .build()); + + final PutSubscriptionFilterResponse putSubscriptionFilterResponse = PutSubscriptionFilterResponse.builder() + .build(); + + final ResourceHandlerRequest request = buildResourceHandlerRequest(model); + + when(proxyClient.client() + .putSubscriptionFilter(any(PutSubscriptionFilterRequest.class))) + .thenReturn(putSubscriptionFilterResponse); + + final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackDelaySeconds()).isZero(); + assertThat(response.getResourceModel()).isEqualTo(request.getDesiredResourceState()); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + private ResourceHandlerRequest buildResourceHandlerRequest(final ResourceModel model) { + return ResourceHandlerRequest.builder() + .desiredResourceState(model) + .logicalResourceIdentifier("logicalResourceIdentifier") + .clientRequestToken("requestToken") + .build(); + } } diff --git a/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/TranslatorTest.java b/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/TranslatorTest.java index 1c584cbd..ff4aed93 100644 --- a/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/TranslatorTest.java +++ b/aws-logs-subscriptionfilter/src/test/java/software/amazon/logs/subscriptionfilter/TranslatorTest.java @@ -142,10 +142,6 @@ void testExceptionTranslation() { final CfnServiceLimitExceededException cfnServiceLimitExceededException = new CfnServiceLimitExceededException(e); assertThat(translateException(limitExceededException)).isEqualToComparingFieldByField(cfnServiceLimitExceededException); - final OperationAbortedException operationAbortedException = OperationAbortedException.builder().build(); - final CfnResourceConflictException cfnResourceConflictException = new CfnResourceConflictException(e); - assertThat(translateException(operationAbortedException)).isEqualToComparingFieldByField(cfnResourceConflictException); - final InvalidParameterException invalidParameterException = InvalidParameterException.builder().build(); final CfnInvalidRequestException cfnInvalidRequestException = new CfnInvalidRequestException(e); assertThat(translateException(invalidParameterException)).isEqualToComparingFieldByField(cfnInvalidRequestException);