diff --git a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java index a13504d8a..ea276cc7c 100644 --- a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java +++ b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java @@ -22,6 +22,7 @@ import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; +import software.amazon.cloudformation.proxy.OperationStatus; import software.amazon.cloudformation.proxy.ProgressEvent; import software.amazon.cloudformation.proxy.ProxyClient; import software.amazon.rds.common.error.ErrorCode; @@ -29,27 +30,26 @@ import software.amazon.rds.common.error.ErrorStatus; public final class Tagging { - public static final ErrorRuleSet IGNORE_LIST_TAGS_PERMISSION_DENIED_ERROR_RULE_SET = ErrorRuleSet + public static final ErrorRuleSet SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET = ErrorRuleSet .extend(ErrorRuleSet.EMPTY_RULE_SET) - .withErrorCodes(ErrorStatus.ignore(), + .withErrorCodes(ErrorStatus.ignore(OperationStatus.IN_PROGRESS), ErrorCode.AccessDenied, ErrorCode.AccessDeniedException) .build(); - public static final ErrorRuleSet STACK_TAGS_ERROR_RULE_SET = ErrorRuleSet + public static final ErrorRuleSet SOFT_FAIL_TAG_ERROR_RULE_SET = ErrorRuleSet .extend(ErrorRuleSet.EMPTY_RULE_SET) - .withErrorCodes(ErrorStatus.failWith(HandlerErrorCode.UnauthorizedTaggingOperation), + .withErrorCodes(ErrorStatus.ignore(), ErrorCode.AccessDenied, ErrorCode.AccessDeniedException ).build(); - public static final ErrorRuleSet RESOURCE_TAG_ERROR_RULE_SET = ErrorRuleSet + public static final ErrorRuleSet HARD_FAIL_TAG_ERROR_RULE_SET = ErrorRuleSet .extend(ErrorRuleSet.EMPTY_RULE_SET) .withErrorCodes(ErrorStatus.failWith(HandlerErrorCode.AccessDenied), ErrorCode.AccessDenied, ErrorCode.AccessDeniedException ).build(); - public static final String RDS_ADD_TAGS_TO_RESOURCE_ACTION = "rds:AddTagsToResource"; public static TagSet exclude(final TagSet from, final TagSet what) { final Set systemTags = new LinkedHashSet<>(from.getSystemTags()); @@ -184,52 +184,56 @@ private static RemoveTagsFromResourceRequest removeTagsFromResourceRequest( .build(); } - public static ErrorRuleSet getUpdateTagsAccessDeniedRuleSet( + public static ErrorRuleSet bestEffortErrorRuleSet( final TagSet tagsToAdd, final TagSet tagsToRemove ) { - return getUpdateTagsAccessDeniedRuleSet(tagsToAdd, tagsToRemove, STACK_TAGS_ERROR_RULE_SET, RESOURCE_TAG_ERROR_RULE_SET); + return bestEffortErrorRuleSet(tagsToAdd, tagsToRemove, SOFT_FAIL_TAG_ERROR_RULE_SET, HARD_FAIL_TAG_ERROR_RULE_SET); } - public static ErrorRuleSet getUpdateTagsAccessDeniedRuleSet( + public static ErrorRuleSet bestEffortErrorRuleSet( final TagSet tagsToAdd, final TagSet tagsToRemove, - final ErrorRuleSet stackTagsErrorRuleSet, - final ErrorRuleSet resourceTagsErrorRuleSet + final ErrorRuleSet softFailErrorRuleSet, + final ErrorRuleSet hardFailErrorRuleSet ) { - /* If the tagging operation comes across an AccessDenied error, we will throw an UnauthorizedTaggingOperation - errorCode for stack level tags. For Resource tags, if they are included, we will throw an AccessDenied error. - This is done to ensure backward compatibility. */ + // Only soft fail if the customer provided no resource-level tags if (tagsToAdd.getResourceTags().isEmpty() && tagsToRemove.getResourceTags().isEmpty()) { - return stackTagsErrorRuleSet; + return softFailErrorRuleSet; } - return resourceTagsErrorRuleSet; + return hardFailErrorRuleSet; } - public static ProgressEvent createWithTaggingFallback( + public static ProgressEvent safeCreate( final AmazonWebServicesClientProxy proxy, final ProxyClient rdsProxyClient, final HandlerMethod handlerMethod, final ProgressEvent progress, final Tagging.TagSet allTags ) { - final ProgressEvent allTagsResult = handlerMethod.invoke(proxy, rdsProxyClient, progress, allTags); - if (allTagsResult.isFailed()) { - if (isUnauthorizedTaggingFailure(allTagsResult, allTags)) { - allTagsResult.setErrorCode(HandlerErrorCode.UnauthorizedTaggingOperation); + return progress.then(p -> { + final C context = p.getCallbackContext(); + if (context.getTaggingContext().isSoftFailTags()) { + return p; } + final ProgressEvent allTagsResult = handlerMethod.invoke(proxy, rdsProxyClient, p, allTags); + if (allTagsResult.isFailed()) { + if (HandlerErrorCode.AccessDenied.equals(allTagsResult.getErrorCode())) { + context.getTaggingContext().setSoftFailTags(true); + return ProgressEvent.progress(allTagsResult.getResourceModel(), context); + } + return allTagsResult; + } + allTagsResult.getCallbackContext().getTaggingContext().setAddTagsComplete(true); return allTagsResult; - } - allTagsResult.getCallbackContext().getTaggingContext().setAddTagsComplete(true); - return allTagsResult; - } - - private static boolean isUnauthorizedTaggingFailure(final ProgressEvent allTagsResult, - final TagSet allTags) { - return HandlerErrorCode.AccessDenied.equals(allTagsResult.getErrorCode()) && - allTags.getResourceTags().isEmpty() && - allTagsResult.getMessage() != null && - allTagsResult.getMessage().contains(RDS_ADD_TAGS_TO_RESOURCE_ACTION); + }).then(p -> { + final C context = p.getCallbackContext(); + if (!context.getTaggingContext().isSoftFailTags()) { + return p; + } + final Tagging.TagSet systemTags = Tagging.TagSet.builder().systemTags(allTags.getSystemTags()).build(); + return handlerMethod.invoke(proxy, rdsProxyClient, p, systemTags); + }); } private static void addToMapIfAbsent(Map allTags, Collection tags) { diff --git a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/TaggingContext.java b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/TaggingContext.java index efc0188fe..029d12daa 100644 --- a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/TaggingContext.java +++ b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/TaggingContext.java @@ -5,6 +5,7 @@ @lombok.ToString @lombok.EqualsAndHashCode public class TaggingContext { + private boolean softFailTags; private boolean addTagsComplete; public interface Provider { diff --git a/aws-rds-cfn-common/src/test/java/software/amazon/rds/common/handler/TaggingTest.java b/aws-rds-cfn-common/src/test/java/software/amazon/rds/common/handler/TaggingTest.java index e4bc2c674..78ef94189 100644 --- a/aws-rds-cfn-common/src/test/java/software/amazon/rds/common/handler/TaggingTest.java +++ b/aws-rds-cfn-common/src/test/java/software/amazon/rds/common/handler/TaggingTest.java @@ -18,6 +18,7 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; @@ -39,6 +40,7 @@ import software.amazon.cloudformation.proxy.ResourceHandlerRequest; import software.amazon.rds.common.error.ErrorRuleSet; import software.amazon.rds.common.error.ErrorStatus; +import software.amazon.rds.common.error.IgnoreErrorStatus; import software.amazon.rds.common.error.UnexpectedErrorStatus; import software.amazon.rds.common.logging.LoggingProxyClient; import software.amazon.rds.common.logging.RequestLogger; @@ -46,7 +48,7 @@ public class TaggingTest extends ProxyClientTestBase { - private final static String FAILED_MSG = "User is not authorized to do rds:AddTagsToResource action"; + private final static String FAILED_MSG = "Test message: failed"; final Set SYSTEM_TAGS = Stream.of( Tag.builder().key("system-tag-key-1").value("system-tag-value-1").build(), @@ -143,9 +145,9 @@ void test_SoftFailErrorRuleSet_AwsServiceException_AccessDenied() { .errorCode("AccessDenied") .build()) .build(); - final ErrorRuleSet ruleSet = Tagging.STACK_TAGS_ERROR_RULE_SET; + final ErrorRuleSet ruleSet = Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET; final ErrorStatus status = ruleSet.handle(exception); - assertThat(status).isInstanceOf(ErrorStatus.class); + assertThat(status).isInstanceOf(IgnoreErrorStatus.class); } @Test @@ -155,7 +157,7 @@ void test_SoftFailErrorRuleSet_AwsServiceException_OtherCode() { .errorCode("InternalFailure") .build()) .build(); - final ErrorRuleSet ruleSet = Tagging.STACK_TAGS_ERROR_RULE_SET; + final ErrorRuleSet ruleSet = Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET; final ErrorStatus status = ruleSet.handle(exception); assertThat(status).isInstanceOf(UnexpectedErrorStatus.class); } @@ -163,7 +165,7 @@ void test_SoftFailErrorRuleSet_AwsServiceException_OtherCode() { @Test void test_SoftFailErrorRuleSet_OtherException() { final Exception exception = new RuntimeException("test exception"); - final ErrorRuleSet ruleSet = Tagging.STACK_TAGS_ERROR_RULE_SET; + final ErrorRuleSet ruleSet = Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET; final ErrorStatus status = ruleSet.handle(exception); assertThat(status).isInstanceOf(UnexpectedErrorStatus.class); } @@ -270,7 +272,7 @@ void test_exclude() { @Test void test_bestEffortErrorRuleSet_emptyResourceTags() { - final ErrorRuleSet errorRuleSet = Tagging.getUpdateTagsAccessDeniedRuleSet( + final ErrorRuleSet errorRuleSet = Tagging.bestEffortErrorRuleSet( Tagging.TagSet.builder() .stackTags(Collections.singleton(Tag.builder().build())) .stackTags(Collections.singleton(Tag.builder().build())) @@ -281,12 +283,12 @@ void test_bestEffortErrorRuleSet_emptyResourceTags() { .build() ); - assertThat(errorRuleSet).isEqualTo(Tagging.STACK_TAGS_ERROR_RULE_SET); + assertThat(errorRuleSet).isEqualTo(Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET); } @Test void test_bestEffortErrorRuleSet_nonEmptyResourceTags() { - assertThat(Tagging.getUpdateTagsAccessDeniedRuleSet( + assertThat(Tagging.bestEffortErrorRuleSet( Tagging.TagSet.builder() .stackTags(Collections.singleton(Tag.builder().build())) .stackTags(Collections.singleton(Tag.builder().build())) @@ -297,9 +299,9 @@ void test_bestEffortErrorRuleSet_nonEmptyResourceTags() { .stackTags(Collections.singleton(Tag.builder().build())) .resourceTags(Collections.singleton(Tag.builder().build())) .build() - )).isEqualTo(Tagging.RESOURCE_TAG_ERROR_RULE_SET); + )).isEqualTo(Tagging.HARD_FAIL_TAG_ERROR_RULE_SET); - assertThat(Tagging.getUpdateTagsAccessDeniedRuleSet( + assertThat(Tagging.bestEffortErrorRuleSet( Tagging.TagSet.builder() .stackTags(Collections.singleton(Tag.builder().build())) .stackTags(Collections.singleton(Tag.builder().build())) @@ -310,9 +312,9 @@ void test_bestEffortErrorRuleSet_nonEmptyResourceTags() { .stackTags(Collections.singleton(Tag.builder().build())) .resourceTags(Collections.singleton(Tag.builder().build())) .build() - )).isEqualTo(Tagging.RESOURCE_TAG_ERROR_RULE_SET); + )).isEqualTo(Tagging.HARD_FAIL_TAG_ERROR_RULE_SET); - assertThat(Tagging.getUpdateTagsAccessDeniedRuleSet( + assertThat(Tagging.bestEffortErrorRuleSet( Tagging.TagSet.builder() .stackTags(Collections.singleton(Tag.builder().build())) .stackTags(Collections.singleton(Tag.builder().build())) @@ -323,7 +325,7 @@ void test_bestEffortErrorRuleSet_nonEmptyResourceTags() { .stackTags(Collections.singleton(Tag.builder().build())) .resourceTags(Collections.emptySet()) .build() - )).isEqualTo(Tagging.RESOURCE_TAG_ERROR_RULE_SET); + )).isEqualTo(Tagging.HARD_FAIL_TAG_ERROR_RULE_SET); } @Test @@ -341,7 +343,7 @@ public void safeCreate_allTagsSuccess() { Mockito.when(handlerMethod.invoke(Mockito.any(), Mockito.any(), Mockito.any(ProgressEvent.class), Mockito.any(Tagging.TagSet.class))) .thenReturn(ProgressEvent.success(null, progress.getCallbackContext())); - final ProgressEvent result = Tagging.createWithTaggingFallback(null, null, handlerMethod, progress, allTags); + final ProgressEvent result = Tagging.safeCreate(null, null, handlerMethod, progress, allTags); Assertions.assertThat(result.isSuccess()).isTrue(); Assertions.assertThat(result.getCallbackContext().getTaggingContext().isAddTagsComplete()).isTrue(); @@ -357,18 +359,29 @@ public void safeCreate_allTagsFailAccessDenied() { final Tagging.TagSet allTags = Tagging.TagSet.builder() .systemTags(SYSTEM_TAGS) .stackTags(STACK_TAGS) + .resourceTags(RESOURCE_TAGS) .build(); final HandlerMethod handlerMethod = Mockito.mock(HandlerMethod.class); Mockito.when(handlerMethod.invoke(Mockito.any(), Mockito.any(), Mockito.any(ProgressEvent.class), Mockito.any(Tagging.TagSet.class))) - .thenReturn(ProgressEvent.failed(null, progress.getCallbackContext(), HandlerErrorCode.AccessDenied, FAILED_MSG)); + .thenReturn(ProgressEvent.failed(null, progress.getCallbackContext(), HandlerErrorCode.AccessDenied, FAILED_MSG)) + .thenReturn(ProgressEvent.success(null, progress.getCallbackContext())); - final ProgressEvent result = Tagging.createWithTaggingFallback(null, null, handlerMethod, progress, allTags); + final ProgressEvent result = Tagging.safeCreate(null, null, handlerMethod, progress, allTags); - Assertions.assertThat(result.isFailed()).isTrue(); - Assertions.assertThat(result.getErrorCode()).isEqualTo(HandlerErrorCode.UnauthorizedTaggingOperation); + Assertions.assertThat(result.isSuccess()).isTrue(); + Assertions.assertThat(result.getCallbackContext().getTaggingContext().isSoftFailTags()).isTrue(); Assertions.assertThat(result.getCallbackContext().getTaggingContext().isAddTagsComplete()).isFalse(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Tagging.TagSet.class); + Mockito.verify(handlerMethod, Mockito.times(2)).invoke(Mockito.any(), Mockito.any(), Mockito.any(ProgressEvent.class), captor.capture()); + + final Tagging.TagSet tagSetInvoke1 = captor.getAllValues().get(0); + final Tagging.TagSet tagSetInvoke2 = captor.getAllValues().get(1); + + Assertions.assertThat(tagSetInvoke1).isEqualTo(allTags); + Assertions.assertThat(tagSetInvoke2).isEqualTo(Tagging.TagSet.builder().systemTags(SYSTEM_TAGS).build()); } @Test @@ -386,10 +399,11 @@ public void safeCreate_allTagsFailGeneric() { Mockito.when(handlerMethod.invoke(Mockito.any(), Mockito.any(), Mockito.any(ProgressEvent.class), Mockito.any(Tagging.TagSet.class))) .thenReturn(ProgressEvent.failed(null, progress.getCallbackContext(), HandlerErrorCode.InternalFailure, FAILED_MSG)); - final ProgressEvent result = Tagging.createWithTaggingFallback(null, null, handlerMethod, progress, allTags); + final ProgressEvent result = Tagging.safeCreate(null, null, handlerMethod, progress, allTags); Assertions.assertThat(result.isFailed()).isTrue(); Assertions.assertThat(result.getCallbackContext().getTaggingContext().isAddTagsComplete()).isFalse(); + Assertions.assertThat(result.getCallbackContext().getTaggingContext().isSoftFailTags()).isFalse(); Mockito.verify(handlerMethod, Mockito.times(1)) .invoke(Mockito.any(), Mockito.any(), Mockito.any(ProgressEvent.class), Mockito.any(Tagging.TagSet.class)); diff --git a/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/BaseHandlerStd.java b/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/BaseHandlerStd.java index 7ba8177d7..7dcd3d39c 100644 --- a/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/BaseHandlerStd.java +++ b/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/BaseHandlerStd.java @@ -185,9 +185,11 @@ private ProgressEvent getTaggingErrorRuleSet(fin progress, exception, DEFAULT_CUSTOM_DB_ENGINE_VERSION_ERROR_RULE_SET.extendWith( - Tagging.getUpdateTagsAccessDeniedRuleSet( + Tagging.bestEffortErrorRuleSet( tagsToAdd, - tagsToRemove + tagsToRemove, + Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, + Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) ) ); diff --git a/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/CreateHandler.java b/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/CreateHandler.java index 497ac20ab..5ef741918 100644 --- a/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/CreateHandler.java +++ b/aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/CreateHandler.java @@ -74,7 +74,7 @@ private ProgressEvent safeCreateCustomEngineVers final ProxyClient proxyClient, final ProgressEvent progress, final Tagging.TagSet allTags) { - return Tagging.createWithTaggingFallback(proxy, proxyClient, this::createCustomEngineVersion, progress, allTags) + return Tagging.safeCreate(proxy, proxyClient, this::createCustomEngineVersion, progress, allTags) .then(p -> Commons.execOnce(p, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(allTags.getStackTags()) diff --git a/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/CreateHandlerTest.java b/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/CreateHandlerTest.java index ddd31a482..55db057b6 100644 --- a/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/CreateHandlerTest.java +++ b/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/CreateHandlerTest.java @@ -1,7 +1,6 @@ package software.amazon.rds.customdbengineversion; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -10,7 +9,6 @@ import static org.mockito.Mockito.when; import java.time.Duration; -import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import org.assertj.core.api.Assertions; @@ -18,7 +16,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.platform.commons.util.StringUtils; import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -180,26 +177,47 @@ public void handleRequest_AccessDeniedTagging() { .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(CreateCustomDbEngineVersionResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); + + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); final ProgressEvent progress = test_handleRequest_base( new CallbackContext(), ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DB_ENGINE_VERSION_AVAILABLE, null, () -> RESOURCE_MODEL.toBuilder() .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.AccessDenied) + expectSuccess() ); + Assertions.assertThat(progress.getCallbackContext().isAddTagsComplete()).isTrue(); + Assertions.assertThat(progress.getCallbackContext().getTaggingContext().isSoftFailTags()).isTrue(); + ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateCustomDbEngineVersionRequest.class); - verify(rdsProxy.client(), times(1)).createCustomDBEngineVersion(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).createCustomDBEngineVersion(createCaptor.capture()); final CreateCustomDbEngineVersionRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final CreateCustomDbEngineVersionRequest requestWithSystemTags = createCaptor.getAllValues().get(1); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class)); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class)); + + verify(rdsProxy.client(), times(3)).describeDBEngineVersions(any(DescribeDbEngineVersionsRequest.class)); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + Assertions.assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class)); } @@ -208,33 +226,52 @@ public void handleRequest_HardFailTagging() { when(rdsProxy.client().createCustomDBEngineVersion(any(CreateCustomDbEngineVersionRequest.class))) .thenThrow( RdsException.builder() - .message("Role not authorized to execute rds:AddTagsToResource") + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode(ErrorCode.AccessDeniedException.toString()) + .build() + ).build()) + .thenReturn(CreateCustomDbEngineVersionResponse.builder().build()); + + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenThrow( + RdsException.builder() .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() ).build()); + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); + test_handleRequest_base( new CallbackContext(), ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DB_ENGINE_VERSION_AVAILABLE, null, () -> RESOURCE_MODEL.toBuilder() - .tags(null) + .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + expectFailed(HandlerErrorCode.AccessDenied) ); - final Tagging.TagSet expectedRequestTags = Tagging.TagSet.builder() - .stackTags(TAG_SET.getStackTags()) - .systemTags(TAG_SET.getSystemTags()) - .build(); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateCustomDbEngineVersionRequest.class); - verify(rdsProxy.client(), times(1)).createCustomDBEngineVersion(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).createCustomDBEngineVersion(createCaptor.capture()); final CreateCustomDbEngineVersionRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final CreateCustomDbEngineVersionRequest requestWithSystemTags = createCaptor.getAllValues().get(1); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( - Iterables.toArray(Tagging.translateTagsToSdk(expectedRequestTags), software.amazon.awssdk.services.rds.model.Tag.class)); + Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class)); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class)); + + verify(rdsProxy.client(), times(2)).describeDBEngineVersions(any(DescribeDbEngineVersionsRequest.class)); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + Assertions.assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class)); } } diff --git a/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/UpdateHandlerTest.java b/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/UpdateHandlerTest.java index 3395b3e70..abb42add4 100644 --- a/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/UpdateHandlerTest.java +++ b/aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/UpdateHandlerTest.java @@ -215,9 +215,7 @@ public void handleRequest_HardFailingTaggingOnAddTags() { } @Test - public void handleRequest_HardFailWithUnauthorizedTagsOnRemove() { - when(rdsProxy.client().modifyCustomDBEngineVersion(any(ModifyCustomDbEngineVersionRequest.class))) - .thenReturn(ModifyCustomDbEngineVersionResponse.builder().build()); + public void handleRequest_SoftFailingTaggingOnRemoveTags() { when(rdsProxy.client().removeTagsFromResource(any(RemoveTagsFromResourceRequest.class))) .thenThrow( RdsException.builder().awsErrorDetails(AwsErrorDetails.builder() @@ -228,22 +226,19 @@ public void handleRequest_HardFailWithUnauthorizedTagsOnRemove() { test_handleRequest_base( context, ResourceHandlerRequest.builder() - .previousResourceTags(Translator.translateTagsToRequest(TAG_LIST)) - .desiredResourceTags(Translator.translateTagsToRequest(TAG_LIST_EMPTY)), + .previousSystemTags(Translator.translateTagsToRequest(TAG_LIST)) + .systemTags(Translator.translateTagsToRequest(TAG_LIST_EMPTY)), () -> DB_ENGINE_VERSION_AVAILABLE, () -> RESOURCE_MODEL_BUILDER().build(), - () -> RESOURCE_MODEL_BUILDER().status("inactive").build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + () -> RESOURCE_MODEL_BUILDER().build(), + expectSuccess() ); - verify(rdsProxy.client(), times(1)).modifyCustomDBEngineVersion(any(ModifyCustomDbEngineVersionRequest.class)); verify(rdsProxy.client(), times(1)).removeTagsFromResource(any(RemoveTagsFromResourceRequest.class)); } @Test - public void handleRequest_HardFailWithUnauthorizedTagsOnAdd() { - when(rdsProxy.client().modifyCustomDBEngineVersion(any(ModifyCustomDbEngineVersionRequest.class))) - .thenReturn(ModifyCustomDbEngineVersionResponse.builder().build()); + public void handleRequest_SoftFailingTaggingOnAddTags() { when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) .thenThrow( RdsException.builder().awsErrorDetails(AwsErrorDetails.builder() @@ -258,11 +253,10 @@ public void handleRequest_HardFailWithUnauthorizedTagsOnAdd() { .systemTags(Translator.translateTagsToRequest(TAG_LIST)), () -> DB_ENGINE_VERSION_AVAILABLE, () -> RESOURCE_MODEL_BUILDER().build(), - () -> RESOURCE_MODEL_BUILDER().status("inactive").build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + () -> RESOURCE_MODEL_BUILDER().build(), + expectSuccess() ); - verify(rdsProxy.client(), times(1)).modifyCustomDBEngineVersion(any(ModifyCustomDbEngineVersionRequest.class)); verify(rdsProxy.client(), times(1)).addTagsToResource(any(AddTagsToResourceRequest.class)); } } diff --git a/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java b/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java index 4810b6c65..b3c55eb56 100644 --- a/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java +++ b/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java @@ -501,7 +501,7 @@ protected ProgressEvent updateTags( return Commons.handleException( progress, exception, - DEFAULT_DB_CLUSTER_ERROR_RULE_SET.extendWith(Tagging.getUpdateTagsAccessDeniedRuleSet(tagsToAdd, tagsToRemove)) + DEFAULT_DB_CLUSTER_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(tagsToAdd, tagsToRemove)) ); } diff --git a/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/CreateHandler.java b/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/CreateHandler.java index 196149fc8..b8bee7c1d 100644 --- a/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/CreateHandler.java +++ b/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/CreateHandler.java @@ -71,11 +71,11 @@ protected ProgressEvent handleRequest( return ProgressEvent.progress(model, callbackContext) .then(progress -> { if (isRestoreToPointInTime(model)) { - return Tagging.createWithTaggingFallback(proxy, rdsProxyClient, this::restoreDbClusterToPointInTime, progress, allTags); + return Tagging.safeCreate(proxy, rdsProxyClient, this::restoreDbClusterToPointInTime, progress, allTags); } else if (isRestoreFromSnapshot(model)) { - return Tagging.createWithTaggingFallback(proxy, rdsProxyClient, this::restoreDbClusterFromSnapshot, progress, allTags); + return Tagging.safeCreate(proxy, rdsProxyClient, this::restoreDbClusterFromSnapshot, progress, allTags); } - return Tagging.createWithTaggingFallback(proxy, rdsProxyClient, this::createDbCluster, progress, allTags); + return Tagging.safeCreate(proxy, rdsProxyClient, this::createDbCluster, progress, allTags); }) .then(progress -> Commons.execOnce(progress, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() diff --git a/aws-rds-dbcluster/src/test/java/software/amazon/rds/dbcluster/CreateHandlerTest.java b/aws-rds-dbcluster/src/test/java/software/amazon/rds/dbcluster/CreateHandlerTest.java index 947e6b053..d4ca05958 100644 --- a/aws-rds-dbcluster/src/test/java/software/amazon/rds/dbcluster/CreateHandlerTest.java +++ b/aws-rds-dbcluster/src/test/java/software/amazon/rds/dbcluster/CreateHandlerTest.java @@ -169,39 +169,55 @@ public void handleRequest_CreateDbCluster_ServerlessV2ScalingConfiguration() { } @Test - public void handleRequest_CreateDbCluster_UnauthorizedTaggingOperation() { + public void handleRequest_CreateDbCluster_AccessDeniedTagging() { when(rdsProxy.client().createDBCluster(any(CreateDbClusterRequest.class))) .thenThrow( RdsException.builder() - .message("Role not authorized to execute rds:AddTagsToResource") .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(CreateDbClusterResponse.builder().build()); + when(rdsProxy.client().addRoleToDBCluster(any(AddRoleToDbClusterRequest.class))) + .thenReturn(AddRoleToDbClusterResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); + + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); test_handleRequest_base( new CallbackContext(), ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DBCLUSTER_ACTIVE, null, () -> RESOURCE_MODEL.toBuilder() .associatedRoles(ImmutableList.of(ROLE)) - .tags(null) + .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + expectSuccess() ); - final Tagging.TagSet expectedRequestTags = Tagging.TagSet.builder() - .stackTags(TAG_SET.getStackTags()) - .systemTags(TAG_SET.getSystemTags()) - .build(); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateDbClusterRequest.class); - verify(rdsProxy.client(), times(1)).createDBCluster(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).createDBCluster(createCaptor.capture()); final CreateDbClusterRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final CreateDbClusterRequest requestWithSystemTags = createCaptor.getAllValues().get(1); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( - Iterables.toArray(Tagging.translateTagsToSdk(expectedRequestTags), software.amazon.awssdk.services.rds.model.Tag.class)); + Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class)); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class)); + + verify(rdsProxy.client(), times(1)).addRoleToDBCluster(any(AddRoleToDbClusterRequest.class)); + verify(rdsProxy.client(), times(4)).describeDBClusters(any(DescribeDbClustersRequest.class)); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + Assertions.assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class)); } @Test @@ -336,28 +352,51 @@ public void handleRequest_RestoreDbClusterFromSnapshot_AccessDeniedTagging() { .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(RestoreDbClusterFromSnapshotResponse.builder().build()); + when(rdsProxy.client().modifyDBCluster(any(ModifyDbClusterRequest.class))) + .thenReturn(ModifyDbClusterResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); + when(rdsProxy.client().describeEvents(any(DescribeEventsRequest.class))) + .thenReturn(DescribeEventsResponse.builder().build()); + + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); test_handleRequest_base( new CallbackContext(), ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DBCLUSTER_ACTIVE, null, () -> RESOURCE_MODEL_ON_RESTORE.toBuilder() .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.AccessDenied) + expectSuccess() ); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(RestoreDbClusterFromSnapshotRequest.class); - verify(rdsProxy.client(), times(1)).restoreDBClusterFromSnapshot(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).restoreDBClusterFromSnapshot(createCaptor.capture()); + verify(rdsProxy.client(), times(1)).describeEvents(any(DescribeEventsRequest.class)); final RestoreDbClusterFromSnapshotRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final RestoreDbClusterFromSnapshotRequest requestWithSystemTags = createCaptor.getAllValues().get(1); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class)); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class)); + + verify(rdsProxy.client(), times(4)).describeDBClusters(any(DescribeDbClustersRequest.class)); + verify(rdsProxy.client(), times(1)).modifyDBCluster(any(ModifyDbClusterRequest.class)); + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + Assertions.assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class)); } @Test @@ -528,7 +567,14 @@ public void handleRequest_RestoreDbClusterToPointInTime_AccessDeniedTagging() { .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(RestoreDbClusterToPointInTimeResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); + when(rdsProxy.client().modifyDBCluster(any(ModifyDbClusterRequest.class))) + .thenReturn(ModifyDbClusterResponse.builder().build()); + when(rdsProxy.client().describeEvents(any(DescribeEventsRequest.class))) + .thenReturn(DescribeEventsResponse.builder().build()); final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(TAG_SET.getStackTags()) @@ -540,20 +586,30 @@ public void handleRequest_RestoreDbClusterToPointInTime_AccessDeniedTagging() { ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DBCLUSTER_ACTIVE, null, () -> RESOURCE_MODEL_ON_RESTORE_IN_TIME.toBuilder() .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.AccessDenied) + expectSuccess() ); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(RestoreDbClusterToPointInTimeRequest.class); - verify(rdsProxy.client(), times(1)).restoreDBClusterToPointInTime(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).restoreDBClusterToPointInTime(createCaptor.capture()); final RestoreDbClusterToPointInTimeRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final RestoreDbClusterToPointInTimeRequest requestWithSystemTags = createCaptor.getAllValues().get(1); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class)); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class)); + + verify(rdsProxy.client(), times(4)).describeDBClusters(any(DescribeDbClustersRequest.class)); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + Assertions.assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class)); } @Test diff --git a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java index 8456da51f..26c6209d3 100644 --- a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java +++ b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java @@ -1,5 +1,8 @@ package software.amazon.rds.dbclusterendpoint; +import java.time.Duration; +import java.util.Optional; + import software.amazon.awssdk.services.rds.RdsClient; import software.amazon.awssdk.services.rds.model.DBClusterEndpoint; import software.amazon.awssdk.services.rds.model.DbClusterEndpointAlreadyExistsException; @@ -24,9 +27,6 @@ import software.amazon.rds.common.handler.HandlerConfig; import software.amazon.rds.common.handler.Tagging; -import java.time.Duration; -import java.util.Optional; - public abstract class BaseHandlerStd extends BaseHandler { protected static final int RESOURCE_ID_MAX_LENGTH = 63; protected static final String RESOURCE_IDENTIFIER = "dbclusterendpoint"; @@ -111,7 +111,7 @@ protected ProgressEvent updateTags( return Commons.handleException( progress, exception, - DEFAULT_DB_CLUSTER_ENDPOINT_ERROR_RULE_SET.extendWith(Tagging.getUpdateTagsAccessDeniedRuleSet(tagsToAdd, tagsToRemove)) + DEFAULT_DB_CLUSTER_ENDPOINT_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(tagsToAdd, tagsToRemove)) ); } return progress; diff --git a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/CreateHandler.java b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/CreateHandler.java index bf004ada9..c0be00f66 100644 --- a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/CreateHandler.java +++ b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/CreateHandler.java @@ -1,5 +1,7 @@ package software.amazon.rds.dbclusterendpoint; +import java.util.HashSet; + import com.amazonaws.util.StringUtils; import software.amazon.awssdk.services.rds.RdsClient; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; @@ -12,8 +14,6 @@ import software.amazon.rds.common.handler.Tagging; import software.amazon.rds.common.util.IdentifierFactory; -import java.util.HashSet; - public class CreateHandler extends BaseHandlerStd { private final IdentifierFactory dbClusterEndpointIdentifierFactory = new IdentifierFactory( DEFAULT_STACK_NAME, @@ -54,7 +54,7 @@ protected ProgressEvent handleRequest( .toString()); } return ProgressEvent.progress(model, callbackContext) - .then(progress -> Tagging.createWithTaggingFallback(proxy, proxyClient, this::createDbClusterEndpoint, progress, allTags)) + .then(progress -> Tagging.safeCreate(proxy, proxyClient, this::createDbClusterEndpoint, progress, allTags)) .then(progress -> Commons.execOnce(progress, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(Tagging.translateTagsToSdk(request.getDesiredResourceTags())) diff --git a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/ReadHandler.java b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/ReadHandler.java index 9babbde4c..bc983ab59 100644 --- a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/ReadHandler.java +++ b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/ReadHandler.java @@ -73,7 +73,7 @@ protected ProgressEvent readTags( return Commons.handleException( ProgressEvent.progress(model, context), exception, - DEFAULT_DB_CLUSTER_ENDPOINT_ERROR_RULE_SET.extendWith(Tagging.IGNORE_LIST_TAGS_PERMISSION_DENIED_ERROR_RULE_SET) + DEFAULT_DB_CLUSTER_ENDPOINT_ERROR_RULE_SET.extendWith(Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET) ); } return ProgressEvent.success(model, context); diff --git a/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/CreateHandlerTest.java b/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/CreateHandlerTest.java index 826c66f07..e4f0614c7 100644 --- a/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/CreateHandlerTest.java +++ b/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/CreateHandlerTest.java @@ -1,7 +1,17 @@ package software.amazon.rds.dbclusterendpoint; -import com.google.common.collect.Iterables; -import lombok.Getter; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import java.time.Duration; +import java.util.concurrent.atomic.AtomicBoolean; + import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -11,6 +21,9 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; + +import com.google.common.collect.Iterables; +import lombok.Getter; import software.amazon.awssdk.awscore.exception.AwsErrorDetails; import software.amazon.awssdk.services.rds.RdsClient; import software.amazon.awssdk.services.rds.model.AddTagsToResourceRequest; @@ -32,18 +45,6 @@ import software.amazon.rds.common.handler.Tagging; import software.amazon.rds.test.common.core.HandlerName; -import java.time.Duration; -import java.util.concurrent.atomic.AtomicBoolean; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; - @ExtendWith(MockitoExtension.class) public class CreateHandlerTest extends AbstractHandlerTest { @@ -179,12 +180,17 @@ public void handleRequest_AccessDeniedTagging() { .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(CreateDbClusterEndpointResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); + + when(rdsProxy.client().listTagsForResource(any(ListTagsForResourceRequest.class))) + .thenReturn(ListTagsForResourceResponse.builder().build()); final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(TAG_SET.getStackTags()) .resourceTags(TAG_SET.getResourceTags()) - .build(); final ProgressEvent progress = test_handleRequest_base( @@ -192,19 +198,32 @@ public void handleRequest_AccessDeniedTagging() { ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DB_CLUSTER_ENDPOINT_AVAILABLE, null, () -> RESOURCE_MODEL.toBuilder() .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.AccessDenied) + expectSuccess() ); + Assertions.assertThat(progress.getCallbackContext().isAddTagsComplete()).isTrue(); + Assertions.assertThat(progress.getCallbackContext().getTaggingContext().isSoftFailTags()).isTrue(); + ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateDbClusterEndpointRequest.class); - verify(rdsProxy.client(), times(1)).createDBClusterEndpoint(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).createDBClusterEndpoint(createCaptor.capture()); final CreateDbClusterEndpointRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final CreateDbClusterEndpointRequest requestWithSystemTags = createCaptor.getAllValues().get(1); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class)); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class)); + + verify(rdsProxy.client(), times(2)).describeDBClusterEndpoints(any(DescribeDbClusterEndpointsRequest.class)); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + Assertions.assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class)); } @@ -213,35 +232,52 @@ public void handleRequest_HardFailTagging() { when(rdsProxy.client().createDBClusterEndpoint(any(CreateDbClusterEndpointRequest.class))) .thenThrow( RdsException.builder() - .message("Role not authorized to execute rds:AddTagsToResource") .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(CreateDbClusterEndpointResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenThrow( + RdsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode(ErrorCode.AccessDeniedException.toString()) + .build() + ).build()); + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); test_handleRequest_base( new CallbackContext(), ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DB_CLUSTER_ENDPOINT_AVAILABLE, null, () -> RESOURCE_MODEL.toBuilder() - .tags(null) + .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + expectFailed(HandlerErrorCode.AccessDenied) ); - final Tagging.TagSet expectedRequestTags = Tagging.TagSet.builder() - .stackTags(TAG_SET.getStackTags()) - .systemTags(TAG_SET.getSystemTags()) - .build(); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateDbClusterEndpointRequest.class); - verify(rdsProxy.client(), times(1)).createDBClusterEndpoint(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).createDBClusterEndpoint(createCaptor.capture()); final CreateDbClusterEndpointRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final CreateDbClusterEndpointRequest requestWithSystemTags = createCaptor.getAllValues().get(1); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( - Iterables.toArray(Tagging.translateTagsToSdk(expectedRequestTags), software.amazon.awssdk.services.rds.model.Tag.class)); + Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class)); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class)); + + verify(rdsProxy.client(), times(1)).describeDBClusterEndpoints(any(DescribeDbClusterEndpointsRequest.class)); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + Assertions.assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class)); } } diff --git a/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/UpdateHandlerTest.java b/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/UpdateHandlerTest.java index 851d0c1ff..f7c2125da 100644 --- a/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/UpdateHandlerTest.java +++ b/aws-rds-dbclusterendpoint/src/test/java/software/amazon/rds/dbclusterendpoint/UpdateHandlerTest.java @@ -1,12 +1,24 @@ package software.amazon.rds.dbclusterendpoint; -import lombok.Getter; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import java.time.Duration; +import java.util.concurrent.atomic.AtomicBoolean; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; + +import lombok.Getter; import software.amazon.awssdk.awscore.exception.AwsErrorDetails; import software.amazon.awssdk.services.rds.RdsClient; import software.amazon.awssdk.services.rds.model.AddTagsToResourceRequest; @@ -29,17 +41,6 @@ import software.amazon.rds.common.handler.HandlerConfig; import software.amazon.rds.test.common.core.HandlerName; -import java.time.Duration; -import java.util.concurrent.atomic.AtomicBoolean; - -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; - @ExtendWith(MockitoExtension.class) public class UpdateHandlerTest extends AbstractHandlerTest { @@ -264,7 +265,7 @@ public void handleRequest_SoftFailingTaggingOnRemoveTags() { () -> DB_CLUSTER_ENDPOINT_AVAILABLE, () -> RESOURCE_MODEL_BUILDER().build(), () -> RESOURCE_MODEL_BUILDER().build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + expectSuccess() ); verify(rdsProxy.client(), times(1)).modifyDBClusterEndpoint(any(ModifyDbClusterEndpointRequest.class)); @@ -291,7 +292,7 @@ public void handleRequest_SoftFailingTaggingOnAddTags() { () -> DB_CLUSTER_ENDPOINT_AVAILABLE, () -> RESOURCE_MODEL_BUILDER().build(), () -> RESOURCE_MODEL_BUILDER().build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + expectSuccess() ); verify(rdsProxy.client(), times(1)).modifyDBClusterEndpoint(any(ModifyDbClusterEndpointRequest.class)); diff --git a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java index 2d652cc2b..ab9f3e1d2 100644 --- a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java +++ b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java @@ -174,9 +174,11 @@ protected ProgressEvent updateTags( progress, exception, DEFAULT_DB_CLUSTER_PARAMETER_GROUP_ERROR_RULE_SET.extendWith( - Tagging.getUpdateTagsAccessDeniedRuleSet( + Tagging.bestEffortErrorRuleSet( tagsToAdd, - tagsToRemove + tagsToRemove, + Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, + Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) ) ); diff --git a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/CreateHandler.java b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/CreateHandler.java index f12330795..f8cc9af1f 100644 --- a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/CreateHandler.java +++ b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/CreateHandler.java @@ -52,7 +52,7 @@ protected ProgressEvent handleRequest( return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext) .then(progress -> setDbClusterParameterGroupNameIfMissing(request, progress)) - .then(progress -> Tagging.createWithTaggingFallback(proxy, proxyClient, this::createDbClusterParameterGroup, progress, allTags)) + .then(progress -> Tagging.safeCreate(proxy, proxyClient, this::createDbClusterParameterGroup, progress, allTags)) .then(progress -> Commons.execOnce(progress, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(allTags.getStackTags()) diff --git a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/ReadHandler.java b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/ReadHandler.java index ae88ca519..6a6e6fe82 100644 --- a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/ReadHandler.java +++ b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/ReadHandler.java @@ -89,7 +89,7 @@ private ProgressEvent readTags( return Commons.handleException( ProgressEvent.progress(model, context), exception, - DEFAULT_DB_CLUSTER_PARAMETER_GROUP_ERROR_RULE_SET.extendWith(Tagging.IGNORE_LIST_TAGS_PERMISSION_DENIED_ERROR_RULE_SET) + DEFAULT_DB_CLUSTER_PARAMETER_GROUP_ERROR_RULE_SET.extendWith(Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET) ); } return ProgressEvent.success(model, context); diff --git a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/CreateHandlerTest.java b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/CreateHandlerTest.java index e2ac240e1..6fd17977c 100644 --- a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/CreateHandlerTest.java +++ b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/CreateHandlerTest.java @@ -153,20 +153,29 @@ public void handleRequest_Success() { } @Test - public void handleRequest_UnauthorizedTaggingOperationOnCreate() { + public void handleRequest_SuccessOnSoftCreate() { when(rdsProxy.client().createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class))) .thenThrow(RdsException.builder() - .message("Role not authorized to execute rds:AddTagsToResource") .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(CreateDbClusterParameterGroupResponse.builder() + .dbClusterParameterGroup(DB_CLUSTER_PARAMETER_GROUP) + .build()); + when(rdsProxy.client().listTagsForResource(any(ListTagsForResourceRequest.class))) + .thenReturn(ListTagsForResourceResponse.builder() + .tagList(Tag.builder().key(TAG_KEY).value(TAG_VALUE).build()) + .build()); - Map stackTags = ImmutableMap.of(TAG_KEY, TAG_VALUE); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); + + Map resourceTags = ImmutableMap.of(TAG_KEY, TAG_VALUE); Map systemTags = ImmutableMap.of("systemTag1", "value2", "systemTag2", "value3"); Map allTags = ImmutableMap.builder() - .putAll(stackTags) + .putAll(resourceTags) .putAll(systemTags) .build(); @@ -177,20 +186,29 @@ public void handleRequest_UnauthorizedTaggingOperationOnCreate() { callbackContext, ResourceHandlerRequest.builder() .systemTags(systemTags) - .desiredResourceTags(stackTags), - null, + .desiredResourceTags(resourceTags), + () -> DBClusterParameterGroup.builder().build(), null, - () -> RESOURCE_MODEL.toBuilder() - .tags(null) - .build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + () -> RESOURCE_MODEL, + expectSuccess() ); + assertThat(response.getCallbackContext().isAddTagsComplete()).isTrue(); + assertThat(response.getCallbackContext().getTaggingContext().isSoftFailTags()).isTrue(); + ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateDbClusterParameterGroupRequest.class); - verify(rdsProxy.client(), times(1)).createDBClusterParameterGroup(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).createDBClusterParameterGroup(createCaptor.capture()); final CreateDbClusterParameterGroupRequest requestWithAllTags = createCaptor.getAllValues().get(0); + final CreateDbClusterParameterGroupRequest requestWithSystemTags = createCaptor.getAllValues().get(1); assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder(asSdkTagArray(allTags)); + assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder(asSdkTagArray(systemTags)); + + verify(rdsProxy.client(), times(1)).describeDBClusterParameterGroups(any(DescribeDbClusterParameterGroupsRequest.class)); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + assertThat(addTagsCaptor.getValue().tags()).containsExactlyInAnyOrder(asSdkTagArray(resourceTags)); } @Test @@ -213,7 +231,7 @@ public void handleRequest_FailWithAccessDenied() { expectFailed(HandlerErrorCode.AccessDenied) ); - verify(rdsProxy.client(), times(1)).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); + verify(rdsProxy.client(), times(2)).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); } @Test diff --git a/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java b/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java index 0f0a43191..d32131069 100644 --- a/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java +++ b/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java @@ -15,9 +15,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import com.amazonaws.arn.Arn; import org.apache.commons.lang3.BooleanUtils; +import com.amazonaws.arn.Arn; import com.amazonaws.util.CollectionUtils; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -92,7 +92,6 @@ import software.amazon.rds.common.handler.Events; import software.amazon.rds.common.handler.HandlerConfig; import software.amazon.rds.common.handler.HandlerMethod; -import software.amazon.rds.common.handler.Probing; import software.amazon.rds.common.handler.Tagging; import software.amazon.rds.common.logging.LoggingProxyClient; import software.amazon.rds.common.logging.RequestLogger; @@ -1064,7 +1063,7 @@ protected ProgressEvent updateTags( return Commons.handleException( progress, exception, - DEFAULT_DB_INSTANCE_ERROR_RULE_SET.extendWith(Tagging.getUpdateTagsAccessDeniedRuleSet(tagsToAdd, tagsToRemove)) + DEFAULT_DB_INSTANCE_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(tagsToAdd, tagsToRemove)) ); } diff --git a/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/CreateHandler.java b/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/CreateHandler.java index 0ffd93edc..0500d8b2b 100644 --- a/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/CreateHandler.java +++ b/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/CreateHandler.java @@ -3,10 +3,6 @@ import java.time.Instant; import java.util.Collection; import java.util.Collections; -import java.util.LinkedHashSet; -import java.util.Optional; -import java.util.Set; -import java.util.function.Function; import org.apache.commons.lang3.BooleanUtils; @@ -32,7 +28,6 @@ import software.amazon.rds.common.request.Validations; import software.amazon.rds.common.util.IdentifierFactory; import software.amazon.rds.dbinstance.client.ApiVersion; -import software.amazon.rds.dbinstance.client.RdsClientProvider; import software.amazon.rds.dbinstance.client.VersionedProxyClient; import software.amazon.rds.dbinstance.util.ResourceModelHelper; @@ -203,7 +198,7 @@ ApiVersion.DEFAULT, safeAddTags(this::createDbInstance) } private HandlerMethod safeAddTags(final HandlerMethod handlerMethod) { - return (proxy, rdsProxyClient, progress, tagSet) -> progress.then(p -> Tagging.createWithTaggingFallback(proxy, rdsProxyClient, handlerMethod, progress, tagSet)); + return (proxy, rdsProxyClient, progress, tagSet) -> progress.then(p -> Tagging.safeCreate(proxy, rdsProxyClient, handlerMethod, progress, tagSet)); } private String fetchEngine(final ProxyClient client, final ResourceModel model) { diff --git a/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/CreateHandlerTest.java b/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/CreateHandlerTest.java index 68c6b3903..221e87850 100644 --- a/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/CreateHandlerTest.java +++ b/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/CreateHandlerTest.java @@ -36,7 +36,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; -import com.google.common.collect.Maps; import lombok.Getter; import software.amazon.awssdk.awscore.exception.AwsErrorDetails; import software.amazon.awssdk.core.util.SdkAutoConstructList; @@ -55,8 +54,8 @@ import software.amazon.awssdk.services.rds.model.DBCluster; import software.amazon.awssdk.services.rds.model.DBClusterSnapshot; import software.amazon.awssdk.services.rds.model.DBInstance; -import software.amazon.awssdk.services.rds.model.DBInstanceAutomatedBackupsReplication; import software.amazon.awssdk.services.rds.model.DBInstanceAutomatedBackup; +import software.amazon.awssdk.services.rds.model.DBInstanceAutomatedBackupsReplication; import software.amazon.awssdk.services.rds.model.DBSnapshot; import software.amazon.awssdk.services.rds.model.DbClusterNotFoundException; import software.amazon.awssdk.services.rds.model.DbClusterSnapshotNotFoundException; @@ -255,13 +254,15 @@ public void handleRequest_RestoreDBInstanceFromSnapshot_AccessDeniedTagging() { when(rdsProxy.client().restoreDBInstanceFromDBSnapshot(any(RestoreDbInstanceFromDbSnapshotRequest.class))) .thenThrow( RdsException.builder() - .message("Role not authorized to execute rds:AddTagsToResource") .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() ).build()) .thenReturn(RestoreDbInstanceFromDbSnapshotResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); + when(rdsProxy.client().describeDBSnapshots(any(DescribeDbSnapshotsRequest.class))) .thenReturn(DescribeDbSnapshotsResponse.builder() .dbSnapshots(DBSnapshot.builder() @@ -275,9 +276,9 @@ public void handleRequest_RestoreDBInstanceFromSnapshot_AccessDeniedTagging() { context.setRebooted(true); context.setUpdatedRoles(true); - final Tagging.TagSet expectedRequestTags = Tagging.TagSet.builder() + final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(TAG_SET.getStackTags()) - .systemTags(TAG_SET.getSystemTags()) + .resourceTags(TAG_SET.getResourceTags()) .build(); test_handleRequest_base( @@ -285,22 +286,35 @@ public void handleRequest_RestoreDBInstanceFromSnapshot_AccessDeniedTagging() { ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DB_INSTANCE_ACTIVE, null, () -> RESOURCE_MODEL_RESTORING_FROM_SNAPSHOT.toBuilder() - .tags(null) + .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + expectSuccess() ); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(RestoreDbInstanceFromDbSnapshotRequest.class); - verify(rdsProxy.client(), times(1)).restoreDBInstanceFromDBSnapshot(createCaptor.capture()); - + verify(rdsProxy.client(), times(2)).restoreDBInstanceFromDBSnapshot(createCaptor.capture()); final RestoreDbInstanceFromDbSnapshotRequest requestWithAllTags = createCaptor.getAllValues().get(0); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( - Iterables.toArray(Tagging.translateTagsToSdk(expectedRequestTags), software.amazon.awssdk.services.rds.model.Tag.class) + Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class) + ); + final RestoreDbInstanceFromDbSnapshotRequest requestWithSystemTags = createCaptor.getAllValues().get(1); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); + + verify(rdsProxy.client(), times(3)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + + ArgumentCaptor addTagCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagCaptor.capture()); + Assertions.assertThat(addTagCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class) ); + + verify(rdsProxy.client(), times(1)).describeDBSnapshots(any(DescribeDbSnapshotsRequest.class)); } @Test @@ -425,7 +439,10 @@ public void handleRequest_CreateReadReplica_AccessDeniedTagging() { .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(CreateDbInstanceReadReplicaResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); final CallbackContext context = new CallbackContext(); context.setCreated(false); @@ -440,15 +457,36 @@ public void handleRequest_CreateReadReplica_AccessDeniedTagging() { test_handleRequest_base( context, + ResourceHandlerRequest.builder() + .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) + .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), + () -> DB_INSTANCE_ACTIVE, null, () -> RESOURCE_MODEL_READ_REPLICA.toBuilder() .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.AccessDenied) + expectSuccess() ); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateDbInstanceReadReplicaRequest.class); - verify(rdsProxy.client(), times(1)).createDBInstanceReadReplica(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).createDBInstanceReadReplica(createCaptor.capture()); + + final CreateDbInstanceReadReplicaRequest requestWithAllTags = createCaptor.getAllValues().get(0); + Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class) + ); + final CreateDbInstanceReadReplicaRequest requestWithSystemTags = createCaptor.getAllValues().get(1); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); + + verify(rdsProxy.client(), times(3)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + + ArgumentCaptor addTagCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagCaptor.capture()); + Assertions.assertThat(addTagCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class) + ); } @Test @@ -545,7 +583,7 @@ public void handleRequest_CreateNewInstance_AccessDeniedException() { expectFailed(HandlerErrorCode.AccessDenied) ); - verify(rdsProxy.client(), times(1)).createDBInstance(any(CreateDbInstanceRequest.class)); + verify(rdsProxy.client(), times(2)).createDBInstance(any(CreateDbInstanceRequest.class)); } @Test @@ -613,49 +651,86 @@ public void handleRequest_CreateNewInstance_AccessDeniedTagging() { .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() - ).build()); + ).build()) + .thenReturn(CreateDbInstanceResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); final CallbackContext context = new CallbackContext(); context.setCreated(false); + context.setUpdated(true); + context.setRebooted(true); + context.setUpdatedRoles(true); + + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); test_handleRequest_base( context, + ResourceHandlerRequest.builder() + .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) + .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), + () -> DB_INSTANCE_ACTIVE, null, () -> RESOURCE_MODEL_BLDR() .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.AccessDenied) + expectSuccess() + ); + + ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateDbInstanceRequest.class); + verify(rdsProxy.client(), times(2)).createDBInstance(createCaptor.capture()); + + final CreateDbInstanceRequest requestWithAllTags = createCaptor.getAllValues().get(0); + Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class) + ); + final CreateDbInstanceRequest requestWithSystemTags = createCaptor.getAllValues().get(1); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); + + verify(rdsProxy.client(), times(3)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + + ArgumentCaptor addTagCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagCaptor.capture()); + Assertions.assertThat(addTagCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class) ); - verify(rdsProxy.client(), times(1)).createDBInstance(any(CreateDbInstanceRequest.class)); } @Test - public void handleRequest_CreateDBInstance_FailTagsNoReentranceForSystemTagsOnly() { + public void handleRequest_CreateDBInstance_SoftFailTagsReentrance() { when(rdsProxy.client().createDBInstance(any(CreateDbInstanceRequest.class))) - .thenThrow( - RdsException.builder() - .message("Role not authorized to execute rds:AddTagsToResource") - .awsErrorDetails(AwsErrorDetails.builder() - .errorCode(ErrorCode.AccessDeniedException.toString()) - .build() - ).build()); + .thenReturn(CreateDbInstanceResponse.builder().build()); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); final CallbackContext context = new CallbackContext(); context.setCreated(false); + context.getTaggingContext().setSoftFailTags(true); context.setUpdated(true); context.setRebooted(true); context.setUpdatedRoles(true); + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); + test_handleRequest_base( context, ResourceHandlerRequest.builder() - .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))), - null, + .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) + .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), + () -> DB_INSTANCE_ACTIVE, null, () -> RESOURCE_MODEL_BLDR() - .tags(null) + .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .build(), - expectFailed(HandlerErrorCode.UnauthorizedTaggingOperation) + expectSuccess() ); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(CreateDbInstanceRequest.class); @@ -666,6 +741,13 @@ public void handleRequest_CreateDBInstance_FailTagsNoReentranceForSystemTagsOnly Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class) ); + verify(rdsProxy.client(), times(3)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + + ArgumentCaptor addTagCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagCaptor.capture()); + Assertions.assertThat(addTagCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class) + ); } @Test @@ -1899,6 +1981,8 @@ public void handleRequest_RestoreDBInstanceToPointInTime_AccessDeniedTagging() { .build() ).build()) .thenReturn(restoreResponse); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenReturn(AddTagsToResourceResponse.builder().build()); final CallbackContext context = new CallbackContext(); context.setCreated(false); @@ -1906,28 +1990,44 @@ public void handleRequest_RestoreDBInstanceToPointInTime_AccessDeniedTagging() { context.setRebooted(true); context.setUpdatedRoles(true); + final Tagging.TagSet extraTags = Tagging.TagSet.builder() + .stackTags(TAG_SET.getStackTags()) + .resourceTags(TAG_SET.getResourceTags()) + .build(); test_handleRequest_base( context, ResourceHandlerRequest.builder() .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getSystemTags()))) .desiredResourceTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(TAG_SET.getStackTags()))), - null, + () -> DB_INSTANCE_ACTIVE, null, () -> RESOURCE_MODEL_RESTORING_TO_POINT_IN_TIME.toBuilder() .tags(Translator.translateTagsFromSdk(TAG_SET.getResourceTags())) .useLatestRestorableTime(true) .build(), - expectFailed(HandlerErrorCode.AccessDenied) + expectSuccess() ); ArgumentCaptor createCaptor = ArgumentCaptor.forClass(RestoreDbInstanceToPointInTimeRequest.class); - verify(rdsProxy.client(), times(1)).restoreDBInstanceToPointInTime(createCaptor.capture()); + verify(rdsProxy.client(), times(2)).restoreDBInstanceToPointInTime(createCaptor.capture()); final RestoreDbInstanceToPointInTimeRequest requestWithAllTags = createCaptor.getAllValues().get(0); Assertions.assertThat(requestWithAllTags.tags()).containsExactlyInAnyOrder( Iterables.toArray(Tagging.translateTagsToSdk(TAG_SET), software.amazon.awssdk.services.rds.model.Tag.class) ); + final RestoreDbInstanceToPointInTimeRequest requestWithSystemTags = createCaptor.getAllValues().get(1); + Assertions.assertThat(requestWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(TAG_SET.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); + + verify(rdsProxy.client(), times(3)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + + ArgumentCaptor addTagCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client(), times(1)).addTagsToResource(addTagCaptor.capture()); + Assertions.assertThat(addTagCaptor.getValue().tags()).containsExactlyInAnyOrder( + Iterables.toArray(Tagging.translateTagsToSdk(extraTags), software.amazon.awssdk.services.rds.model.Tag.class) + ); } @Test diff --git a/aws-rds-dbparametergroup/docs/tag.md b/aws-rds-dbparametergroup/docs/tag.md index 18ef5bbe8..a7fa40fe0 100644 --- a/aws-rds-dbparametergroup/docs/tag.md +++ b/aws-rds-dbparametergroup/docs/tag.md @@ -32,9 +32,9 @@ _Required_: Yes _Type_: String -_Minimum_: 1 +_Minimum Length_: 1 -_Maximum_: 128 +_Maximum Length_: 128 _Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) @@ -46,6 +46,6 @@ _Required_: No _Type_: String -_Maximum_: 256 +_Maximum Length_: 256 _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-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java index c47606b53..ed75cf6a9 100644 --- a/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java +++ b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java @@ -149,9 +149,11 @@ protected ProgressEvent updateTags( progress, exception, DEFAULT_DB_PARAMETER_GROUP_ERROR_RULE_SET.extendWith( - Tagging.getUpdateTagsAccessDeniedRuleSet( + Tagging.bestEffortErrorRuleSet( tagsToAdd, - tagsToRemove + tagsToRemove, + Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, + Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) ) ); diff --git a/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/CreateHandler.java b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/CreateHandler.java index f040fd3e9..87f7e2435 100644 --- a/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/CreateHandler.java +++ b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/CreateHandler.java @@ -64,7 +64,7 @@ private ProgressEvent safeCreateDBParameterGroup final RequestLogger logger ) { final HandlerMethod createMethod = (pxy, pcl, prg, tgs) -> createDBParameterGroup(pxy, pcl, prg, tgs, logger); - return Tagging.createWithTaggingFallback(proxy, proxyClient, createMethod, progress, allTags) + return Tagging.safeCreate(proxy, proxyClient, createMethod, progress, allTags) .then(p -> Commons.execOnce(p, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(allTags.getStackTags()) diff --git a/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/ReadHandler.java b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/ReadHandler.java index d87d30e0e..32ec0cfec 100644 --- a/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/ReadHandler.java +++ b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/ReadHandler.java @@ -103,7 +103,7 @@ private ProgressEvent readTags( return Commons.handleException( ProgressEvent.progress(model, context), exception, - DEFAULT_DB_PARAMETER_GROUP_ERROR_RULE_SET.extendWith(Tagging.IGNORE_LIST_TAGS_PERMISSION_DENIED_ERROR_RULE_SET) + DEFAULT_DB_PARAMETER_GROUP_ERROR_RULE_SET.extendWith(Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET) ); } return ProgressEvent.success(model, context); diff --git a/aws-rds-dbsubnetgroup/docs/tag.md b/aws-rds-dbsubnetgroup/docs/tag.md index 431dcde93..690803e7c 100644 --- a/aws-rds-dbsubnetgroup/docs/tag.md +++ b/aws-rds-dbsubnetgroup/docs/tag.md @@ -32,9 +32,9 @@ _Required_: Yes _Type_: String -_Minimum_: 1 +_Minimum Length_: 1 -_Maximum_: 128 +_Maximum Length_: 128 _Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) @@ -46,6 +46,6 @@ _Required_: No _Type_: String -_Maximum_: 256 +_Maximum Length_: 256 _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-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java index 205e8416c..a3921afb7 100644 --- a/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java +++ b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java @@ -121,9 +121,11 @@ protected ProgressEvent updateTags( progress, exception, DEFAULT_DB_SUBNET_GROUP_ERROR_RULE_SET.extendWith( - Tagging.getUpdateTagsAccessDeniedRuleSet( + Tagging.bestEffortErrorRuleSet( tagsToAdd, - tagsToRemove + tagsToRemove, + Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, + Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) ) ); diff --git a/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/CreateHandler.java b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/CreateHandler.java index 7f066e0ee..b798e8005 100644 --- a/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/CreateHandler.java +++ b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/CreateHandler.java @@ -55,7 +55,7 @@ private ProgressEvent safeCreateDbSubnetGroup(fi final ProxyClient proxyClient, final ProgressEvent progress, final Tagging.TagSet allTags) { - return Tagging.createWithTaggingFallback(proxy, proxyClient, this::createDbSubnetGroup, progress, allTags) + return Tagging.safeCreate(proxy, proxyClient, this::createDbSubnetGroup, progress, allTags) .then(p -> Commons.execOnce(p, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(allTags.getStackTags()) diff --git a/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/ReadHandler.java b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/ReadHandler.java index 5a629c127..d2e04b3a1 100644 --- a/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/ReadHandler.java +++ b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/ReadHandler.java @@ -70,7 +70,7 @@ protected ProgressEvent readTags( return Commons.handleException( ProgressEvent.progress(model, context), exception, - DEFAULT_DB_SUBNET_GROUP_ERROR_RULE_SET.extendWith(Tagging.IGNORE_LIST_TAGS_PERMISSION_DENIED_ERROR_RULE_SET) + DEFAULT_DB_SUBNET_GROUP_ERROR_RULE_SET.extendWith(Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET) ); } return ProgressEvent.success(model, context); diff --git a/aws-rds-eventsubscription/docs/README.md b/aws-rds-eventsubscription/docs/README.md index 1bd0f6e94..6d5b25d76 100644 --- a/aws-rds-eventsubscription/docs/README.md +++ b/aws-rds-eventsubscription/docs/README.md @@ -60,7 +60,7 @@ _Required_: No _Type_: String -_Maximum_: 255 +_Maximum Length_: 255 _Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement) diff --git a/aws-rds-eventsubscription/docs/tag.md b/aws-rds-eventsubscription/docs/tag.md index 41d5066c1..5ca7e0d16 100644 --- a/aws-rds-eventsubscription/docs/tag.md +++ b/aws-rds-eventsubscription/docs/tag.md @@ -32,9 +32,9 @@ _Required_: Yes _Type_: String -_Minimum_: 1 +_Minimum Length_: 1 -_Maximum_: 128 +_Maximum Length_: 128 _Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) @@ -46,6 +46,6 @@ _Required_: No _Type_: String -_Maximum_: 256 +_Maximum Length_: 256 _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-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java index a3aec1151..7ba768376 100644 --- a/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java +++ b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java @@ -135,9 +135,11 @@ protected ProgressEvent updateTags( progress, exception, DEFAULT_EVENT_SUBSCRIPTION_ERROR_RULE_SET.extendWith( - Tagging.getUpdateTagsAccessDeniedRuleSet( + Tagging.bestEffortErrorRuleSet( tagsToAdd, - tagsToRemove + tagsToRemove, + Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, + Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) ) ); diff --git a/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/CreateHandler.java b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/CreateHandler.java index 9e4acbddf..d98df5ae3 100644 --- a/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/CreateHandler.java +++ b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/CreateHandler.java @@ -47,7 +47,7 @@ private ProgressEvent safeCreateEventSubscriptio final ProxyClient proxyClient, final ProgressEvent progress, final Tagging.TagSet allTags) { - return Tagging.createWithTaggingFallback(proxy, proxyClient, this::createEventSubscription, progress, allTags) + return Tagging.safeCreate(proxy, proxyClient, this::createEventSubscription, progress, allTags) .then(p -> Commons.execOnce(p, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(allTags.getStackTags()) diff --git a/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/ReadHandler.java b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/ReadHandler.java index a15981680..63e873f25 100644 --- a/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/ReadHandler.java +++ b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/ReadHandler.java @@ -49,7 +49,7 @@ protected ProgressEvent readTags( return Commons.handleException( ProgressEvent.progress(model, context), exception, - DEFAULT_EVENT_SUBSCRIPTION_ERROR_RULE_SET.extendWith(Tagging.IGNORE_LIST_TAGS_PERMISSION_DENIED_ERROR_RULE_SET) + DEFAULT_EVENT_SUBSCRIPTION_ERROR_RULE_SET.extendWith(Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET) ); } return ProgressEvent.success(model, context); diff --git a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java index e3b8071fb..d25b0179d 100644 --- a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java +++ b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java @@ -135,9 +135,11 @@ protected ProgressEvent updateTags( progress, exception, DEFAULT_OPTION_GROUP_ERROR_RULE_SET.extendWith( - Tagging.getUpdateTagsAccessDeniedRuleSet( + Tagging.bestEffortErrorRuleSet( tagsToAdd, - tagsToRemove + tagsToRemove, + Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, + Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) ) ); diff --git a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/CreateHandler.java b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/CreateHandler.java index 723696679..a3a56385e 100644 --- a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/CreateHandler.java +++ b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/CreateHandler.java @@ -50,7 +50,7 @@ protected ProgressEvent handleRequest( return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext) .then(progress -> setOptionGroupNameIfEmpty(request, progress)) - .then(progress -> Tagging.createWithTaggingFallback(proxy, proxyClient, this::createOptionGroup, progress, allTags)) + .then(progress -> Tagging.safeCreate(proxy, proxyClient, this::createOptionGroup, progress, allTags)) .then(progress -> Commons.execOnce(progress, () -> { final Tagging.TagSet extraTags = Tagging.TagSet.builder() .stackTags(allTags.getStackTags()) diff --git a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/ReadHandler.java b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/ReadHandler.java index d7e7ff039..a815553af 100644 --- a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/ReadHandler.java +++ b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/ReadHandler.java @@ -75,7 +75,7 @@ protected ProgressEvent readTags( return Commons.handleException( ProgressEvent.progress(model, context), exception, - DEFAULT_OPTION_GROUP_ERROR_RULE_SET.extendWith(Tagging.IGNORE_LIST_TAGS_PERMISSION_DENIED_ERROR_RULE_SET) + DEFAULT_OPTION_GROUP_ERROR_RULE_SET.extendWith(Tagging.SOFT_FAIL_TAG_ERROR_RULE_SET) ); } return ProgressEvent.success(model, context); diff --git a/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/CreateHandlerTest.java b/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/CreateHandlerTest.java index e13d21d7f..2bda25c67 100644 --- a/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/CreateHandlerTest.java +++ b/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/CreateHandlerTest.java @@ -11,11 +11,9 @@ import java.time.Duration; -import com.google.common.collect.Iterables; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInfo; import org.junit.jupiter.api.extension.ExtendWith; @@ -23,6 +21,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import com.google.common.collect.Iterables; import lombok.Getter; import software.amazon.awssdk.awscore.exception.AwsErrorDetails; import software.amazon.awssdk.services.rds.RdsClient; @@ -39,10 +38,10 @@ import software.amazon.awssdk.services.rds.model.RdsException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; +import software.amazon.cloudformation.proxy.OperationStatus; import software.amazon.cloudformation.proxy.ProgressEvent; import software.amazon.cloudformation.proxy.ProxyClient; import software.amazon.cloudformation.proxy.ResourceHandlerRequest; -import software.amazon.cloudformation.proxy.OperationStatus; import software.amazon.rds.common.error.ErrorCode; import software.amazon.rds.common.handler.HandlerConfig; import software.amazon.rds.common.handler.Tagging; @@ -216,12 +215,27 @@ public void handleRequest_SoftFailingTagging() { when(proxyClient.client().createOptionGroup(any(CreateOptionGroupRequest.class))) .thenThrow( RdsException.builder() - .message("Role not authorized to execute rds:AddTagsToResource") + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode(ErrorCode.AccessDeniedException.toString()) + .build() + ).build()) + .thenReturn(CreateOptionGroupResponse.builder().build()); + + final DescribeOptionGroupsResponse describeDbClusterParameterGroupsResponse = DescribeOptionGroupsResponse.builder() + .optionGroupsList(OPTION_GROUP_ACTIVE).build(); + when(proxyClient.client().describeOptionGroups(any(DescribeOptionGroupsRequest.class))).thenReturn(describeDbClusterParameterGroupsResponse); + + when(proxyClient.client().addTagsToResource(any(AddTagsToResourceRequest.class))) + .thenThrow( + RdsException.builder() .awsErrorDetails(AwsErrorDetails.builder() .errorCode(ErrorCode.AccessDeniedException.toString()) .build() ).build()); + when(proxyClient.client().listTagsForResource(any(ListTagsForResourceRequest.class))) + .thenReturn(ListTagsForResourceResponse.builder().build()); + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .clientRequestToken(TestUtils.randomString(32, TestUtils.ALPHA)) .systemTags(Translator.translateTagsToRequest(Translator.translateTagsFromSdk(desiredTags.getSystemTags()))) @@ -233,19 +247,34 @@ public void handleRequest_SoftFailingTagging() { final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); - Assertions.assertThat(response).isNotNull(); - Assertions.assertThat(response.getStatus()).isEqualTo(OperationStatus.FAILED); - Assertions.assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); - Assertions.assertThat(response.getMessage()).isNotNull(); - Assertions.assertThat(response.getErrorCode()).isEqualTo(HandlerErrorCode.UnauthorizedTaggingOperation); - Assertions.assertThat(response.getResourceModels()).isNull(); + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + + verify(proxyClient.client(), times(1)).addTagsToResource(any(AddTagsToResourceRequest.class)); + verify(proxyClient.client(), times(2)).describeOptionGroups(any(DescribeOptionGroupsRequest.class)); + verify(proxyClient.client(), times(1)).listTagsForResource(any(ListTagsForResourceRequest.class)); ArgumentCaptor createOptionGroupCaptor = ArgumentCaptor.forClass(CreateOptionGroupRequest.class); - verify(proxyClient.client(), times(1)).createOptionGroup(createOptionGroupCaptor.capture()); + verify(proxyClient.client(), times(2)).createOptionGroup(createOptionGroupCaptor.capture()); final CreateOptionGroupRequest createOptionGroupWithAllTags = createOptionGroupCaptor.getAllValues().get(0); Assertions.assertThat(createOptionGroupWithAllTags.tags()).containsExactlyInAnyOrder( Iterables.toArray(Tagging.translateTagsToSdk(desiredTags), software.amazon.awssdk.services.rds.model.Tag.class) ); + final CreateOptionGroupRequest createOptionGroupWithSystemTags = createOptionGroupCaptor.getAllValues().get(1); + Assertions.assertThat(createOptionGroupWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(desiredTags.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(proxyClient.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + final AddTagsToResourceRequest requestWithStackTags = addTagsCaptor.getAllValues().get(0); + Assertions.assertThat(requestWithStackTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(desiredTags.getStackTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); } @Test @@ -258,6 +287,19 @@ public void handleRequest_HardFailingTagging() { .build(); when(proxyClient.client().createOptionGroup(any(CreateOptionGroupRequest.class))) + .thenThrow( + RdsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode(ErrorCode.AccessDeniedException.toString()) + .build() + ).build()) + .thenReturn(CreateOptionGroupResponse.builder().build()); + + final DescribeOptionGroupsResponse describeDbClusterParameterGroupsResponse = DescribeOptionGroupsResponse.builder() + .optionGroupsList(OPTION_GROUP_ACTIVE).build(); + when(proxyClient.client().describeOptionGroups(any(DescribeOptionGroupsRequest.class))).thenReturn(describeDbClusterParameterGroupsResponse); + + when(proxyClient.client().addTagsToResource(any(AddTagsToResourceRequest.class))) .thenThrow( RdsException.builder() .awsErrorDetails(AwsErrorDetails.builder() @@ -281,12 +323,26 @@ public void handleRequest_HardFailingTagging() { Assertions.assertThat(response.getErrorCode()).isEqualTo(HandlerErrorCode.AccessDenied); Assertions.assertThat(response.getResourceModels()).isNull(); + verify(proxyClient.client(), times(1)).addTagsToResource(any(AddTagsToResourceRequest.class)); + verify(proxyClient.client(), times(1)).describeOptionGroups(any(DescribeOptionGroupsRequest.class)); + ArgumentCaptor createOptionGroupCaptor = ArgumentCaptor.forClass(CreateOptionGroupRequest.class); - verify(proxyClient.client(), times(1)).createOptionGroup(createOptionGroupCaptor.capture()); + verify(proxyClient.client(), times(2)).createOptionGroup(createOptionGroupCaptor.capture()); final CreateOptionGroupRequest createOptionGroupWithAllTags = createOptionGroupCaptor.getAllValues().get(0); Assertions.assertThat(createOptionGroupWithAllTags.tags()).containsExactlyInAnyOrder( Iterables.toArray(Tagging.translateTagsToSdk(desiredTags), software.amazon.awssdk.services.rds.model.Tag.class) ); + final CreateOptionGroupRequest createOptionGroupWithSystemTags = createOptionGroupCaptor.getAllValues().get(1); + Assertions.assertThat(createOptionGroupWithSystemTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(desiredTags.getSystemTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); + + ArgumentCaptor addTagsCaptor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(proxyClient.client(), times(1)).addTagsToResource(addTagsCaptor.capture()); + final AddTagsToResourceRequest requestWithResourceTags = addTagsCaptor.getAllValues().get(0); + Assertions.assertThat(requestWithResourceTags.tags()).containsExactlyInAnyOrder( + Iterables.toArray(desiredTags.getResourceTags(), software.amazon.awssdk.services.rds.model.Tag.class) + ); } @Test diff --git a/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/UpdateHandlerTest.java b/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/UpdateHandlerTest.java index a8e59d221..332ffd090 100644 --- a/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/UpdateHandlerTest.java +++ b/aws-rds-optiongroup/src/test/java/software/amazon/rds/optiongroup/UpdateHandlerTest.java @@ -243,6 +243,8 @@ public void handleRequest_SoftFailingTaggingOnRemoveTags() { ) ); + when(proxyClient.client().listTagsForResource(any(ListTagsForResourceRequest.class))) + .thenReturn(ListTagsForResourceResponse.builder().build()); final DescribeOptionGroupsResponse describeDbClusterParameterGroupsResponse = DescribeOptionGroupsResponse.builder() .optionGroupsList(OPTION_GROUP_ACTIVE).build(); when(proxyClient.client().describeOptionGroups(any(DescribeOptionGroupsRequest.class))) @@ -267,14 +269,15 @@ public void handleRequest_SoftFailingTaggingOnRemoveTags() { final ProgressEvent response = handler.handleRequest(proxy, request, new CallbackContext(), proxyClient, logger); assertThat(response).isNotNull(); - assertThat(response.getStatus()).isEqualTo(OperationStatus.FAILED); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); - assertThat(response.getMessage()).isNotNull(); - assertThat(response.getErrorCode()).isEqualTo(HandlerErrorCode.UnauthorizedTaggingOperation); assertThat(response.getResourceModels()).isNull(); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); verify(proxyClient.client(), times(1)).removeTagsFromResource(any(RemoveTagsFromResourceRequest.class)); - verify(proxyClient.client(), times(1)).describeOptionGroups(any(DescribeOptionGroupsRequest.class)); + verify(proxyClient.client(), times(2)).describeOptionGroups(any(DescribeOptionGroupsRequest.class)); + verify(proxyClient.client(), times(1)).listTagsForResource(any(ListTagsForResourceRequest.class)); } @Test