fix: handle non-idempotent create APIs

aws-rds-cfn-common/src/main/java/software/amazon/rds/common/util/IdempotencyHelper.java

@@ -0,0 +1,87 @@
+package software.amazon.rds.common.util;
+
+import java.util.function.Function;
+import java.util.function.UnaryOperator;
+
+import com.google.common.annotations.VisibleForTesting;
+import software.amazon.cloudformation.exceptions.CfnAlreadyExistsException;
+import software.amazon.cloudformation.exceptions.CfnNotFoundException;
+import software.amazon.cloudformation.proxy.HandlerErrorCode;
+import software.amazon.cloudformation.proxy.ProgressEvent;
+import software.amazon.rds.common.logging.RequestLogger;
+
+/**
+ * Utility class containing functions to handle non-idempotent APIs.
+ */
+public final class IdempotencyHelper {
+
+    private static boolean bypass = false;
+
+    @VisibleForTesting
+    public static void setBypass(boolean bypass) {
+        IdempotencyHelper.bypass = bypass;
+    }
+
+    /**
+     * Wraps a non-idempotent create operation to prevent spurious "already exists" errors.
+     *
+     * @param checkExistenceFunction a function that checks if the resource exists and returns the resource if it does,
+     *                               or if it does not, then null or a CfnNotFoundException.
+     * @param createFunction         the non-idempotent create operation
+     * @param resourceTypeName       the resource type name (used to form the error message)
+     * @param resourceIdentifier     the resource identifier (used to form the error message)
+     */
+    public static <ResourceT, CallbackT extends PreExistenceContext> ProgressEvent<ResourceT, CallbackT> safeCreate(
+        final Function<ResourceT, ?> checkExistenceFunction,
+        final UnaryOperator<ProgressEvent<ResourceT, CallbackT>> createFunction,
+        final String resourceTypeName,
+        final String resourceIdentifier,
+        final ProgressEvent<ResourceT, CallbackT> progress,
+        final RequestLogger requestLogger
+    ) {
+        // The approach recommended by CloudFormation is as follows:
+        // - First, check whether the requested resource already exists. If it does, then fail immediately. Otherwise,
+        //   return IN_PROGRESS with a non-zero callback delay. This forces the handler to return back to CFN, which
+        //   will persist the status so that the pre-existence check is not repeated in case the next step fails.
+        // - Then, perform the create operation. If it returns an AlreadyExists error, then ignore it.
+
+        if (bypass) {
+            return createFunction.apply(progress);
+        }
+
+        final var preExistenceCheckDone = progress.getCallbackContext().getPreExistenceCheckDone();
+        if (preExistenceCheckDone == null || !preExistenceCheckDone) {
+            try {
+                final var existingResource = checkExistenceFunction.apply(progress.getResourceModel());
+                if (existingResource != null) {
+                    requestLogger.log("Resource already exists");
+                    throw new CfnAlreadyExistsException(resourceTypeName, resourceIdentifier);
+                }
+            } catch (final CfnNotFoundException ignored) {
+                requestLogger.log("CfnNotFoundException thrown during pre-existence check (all good)");
+            }
+
+            progress.getCallbackContext().setPreExistenceCheckDone(true);
+
+            // !!!: The callbackDelaySeconds of 1 is important here. (See canContinueProgress in ProgressEvent)
+            return ProgressEvent.defaultInProgressHandler(progress.getCallbackContext(), 1,
+                progress.getResourceModel());
+        } else {
+            final var result = createFunction.apply(progress);
+            if (result.isFailed() && result.getErrorCode() == HandlerErrorCode.AlreadyExists) {
+                requestLogger.log("Ignoring AlreadyExists error from create operation");
+                return ProgressEvent.defaultInProgressHandler(progress.getCallbackContext(), 0,
+                    progress.getResourceModel());
+            } else {
+                return result;
+            }
+        }
+    }
+
+    public interface PreExistenceContext {
+        Boolean getPreExistenceCheckDone();
+
+        void setPreExistenceCheckDone(Boolean preExistenceCheckDone);
+    }
+
+}

aws-rds-cfn-common/src/test/java/software/amazon/rds/common/util/IdempotencyHelperTest.java

@@ -0,0 +1,121 @@
+package software.amazon.rds.common.util;
+
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.Mockito;
+import software.amazon.cloudformation.exceptions.CfnAlreadyExistsException;
+import software.amazon.cloudformation.exceptions.CfnNotFoundException;
+import software.amazon.cloudformation.proxy.HandlerErrorCode;
+import software.amazon.cloudformation.proxy.ProgressEvent;
+import software.amazon.rds.common.logging.RequestLogger;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Function;
+
+class IdempotencyHelperTest {
+    private static final String MODEL = "blah";
+
+    private RequestLogger mockRequestLogger = Mockito.mock(RequestLogger.class);
+    private Context context = new Context();
+
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    void safeCreate_happy(boolean existenceCheckException) {
+        AtomicBoolean checkedExistence = new AtomicBoolean(false);
+        AtomicBoolean created = new AtomicBoolean(false);
+
+        final var afterExistenceCheck = IdempotencyHelper.safeCreate(
+            model -> {
+                checkedExistence.set(true);
+                Assertions.assertThat(model).isSameAs(MODEL);
+                if (existenceCheckException) {
+                    throw new CfnNotFoundException(new RuntimeException());
+                } else {
+                    return null;
+                }
+            },
+            null,
+            "resourceTypeName",
+            "resourceIdentifier",
+            ProgressEvent.progress(MODEL, context),
+            mockRequestLogger
+        );
+
+        Assertions.assertThat(checkedExistence).isTrue();
+        Assertions.assertThat(afterExistenceCheck.isInProgressCallbackDelay()).isTrue();
+        Assertions.assertThat(afterExistenceCheck.getCallbackContext().getPreExistenceCheckDone()).isTrue();
+
+        final var afterCreate = IdempotencyHelper.safeCreate(
+            null,
+            p -> {
+                created.set(true);
+                return ProgressEvent.defaultInProgressHandler(p.getCallbackContext(), 0, p.getResourceModel());
+            },
+            "resourceTypeName",
+            "resourceIdentifier",
+            ProgressEvent.progress(MODEL, context),
+            mockRequestLogger
+        );
+
+        Assertions.assertThat(created).isTrue();
+        Assertions.assertThat(afterCreate.canContinueProgress()).isTrue();
+        Assertions.assertThat(afterCreate.getResourceModel()).isSameAs(MODEL);
+        Assertions.assertThat(afterCreate.getCallbackContext()).isSameAs(context);
+    }
+
+    @Test
+    void safeCreate_pre_already_exists() {
+        Assertions.assertThatThrownBy(() -> {
+            IdempotencyHelper.safeCreate(
+                Function.identity(),
+                null,
+                "resourceTypeName",
+                "resourceIdentifier",
+                ProgressEvent.progress(MODEL, context),
+                mockRequestLogger
+            );
+        }).isInstanceOf(CfnAlreadyExistsException.class);
+    }
+
+    @Test
+    void safeCreate_failed_create() {
+        context.setPreExistenceCheckDone(true);
+
+        final var afterCreate = IdempotencyHelper.safeCreate(
+            null,
+            p -> ProgressEvent.defaultFailureHandler(new RuntimeException(), HandlerErrorCode.InternalFailure),
+            "resourceTypeName",
+            "resourceIdentifier",
+            ProgressEvent.progress(MODEL, context),
+            mockRequestLogger
+        );
+
+        Assertions.assertThat(afterCreate.isFailed()).isTrue();
+        Assertions.assertThat(afterCreate.getErrorCode()).isEqualTo(HandlerErrorCode.InternalFailure);
+    }
+
+    @Test
+    void safeCreate_create_already_exists_after_pre_existence_check() {
+        context.setPreExistenceCheckDone(true);
+
+        final var afterCreate = IdempotencyHelper.safeCreate(
+            null,
+            p -> ProgressEvent.defaultFailureHandler(new RuntimeException(), HandlerErrorCode.AlreadyExists),
+            "resourceTypeName",
+            "resourceIdentifier",
+            ProgressEvent.progress(MODEL, context),
+            mockRequestLogger
+        );
+
+        Assertions.assertThat(afterCreate.canContinueProgress()).isTrue();
+        Assertions.assertThat(afterCreate.getResourceModel()).isSameAs(MODEL);
+        Assertions.assertThat(afterCreate.getCallbackContext()).isSameAs(context);
+    }
+
+    @lombok.Data
+    private static class Context implements IdempotencyHelper.PreExistenceContext {
+        private Boolean preExistenceCheckDone;
+    }
+}

aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/BaseHandlerStd.java

@@ -16,6 +16,7 @@
 import software.amazon.awssdk.services.rds.model.InvalidS3BucketException;
 import software.amazon.awssdk.services.rds.model.KmsKeyNotAccessibleException;
 import software.amazon.awssdk.utils.StringUtils;
+import software.amazon.cloudformation.exceptions.CfnNotFoundException;
 import software.amazon.cloudformation.exceptions.CfnNotStabilizedException;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
 import software.amazon.cloudformation.proxy.HandlerErrorCode;
@@ -164,15 +165,19 @@ private void resourceStabilizationTime(final CallbackContext callbackContext) {
 
     protected DBEngineVersion fetchDBEngineVersion(final ResourceModel model,
                                                    final ProxyClient<RdsClient> proxyClient) {
-        DescribeDbEngineVersionsResponse response = proxyClient.injectCredentialsAndInvokeV2(
+        try {
+            DescribeDbEngineVersionsResponse response = proxyClient.injectCredentialsAndInvokeV2(
                 Translator.describeDbEngineVersionsRequest(model),
                 proxyClient.client()::describeDBEngineVersions);
 
-        final Optional<DBEngineVersion> engineVersion = response
-                .dbEngineVersions().stream().findFirst();
+            if (!response.hasDbEngineVersions() || response.dbEngineVersions().isEmpty()) {
+                throw new CfnNotFoundException(ResourceModel.TYPE_NAME, model.getEngineVersion());
+            }
 
-        return engineVersion.orElseThrow(() -> CustomDbEngineVersionNotFoundException.builder().message(
-                "CustomDBEngineVersion " + model.getEngineVersion() + " not found").build());
+            return response.dbEngineVersions().get(0);
+        } catch (CustomDbEngineVersionNotFoundException e) {
+            throw new CfnNotFoundException(e);
+        }
     }
 
 

aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/CallbackContext.java

@@ -3,6 +3,7 @@
 import software.amazon.cloudformation.proxy.StdCallbackContext;
 import software.amazon.rds.common.handler.TaggingContext;
 import software.amazon.rds.common.handler.TimestampContext;
+import software.amazon.rds.common.util.IdempotencyHelper;
 
 import java.time.Duration;
 import java.time.Instant;
@@ -13,7 +14,9 @@
 @lombok.Setter
 @lombok.ToString
 @lombok.EqualsAndHashCode(callSuper = true)
-public class CallbackContext extends StdCallbackContext implements TaggingContext.Provider, TimestampContext.Provider {
+public class CallbackContext extends StdCallbackContext implements TaggingContext.Provider, TimestampContext.Provider,
+    IdempotencyHelper.PreExistenceContext {
+    private Boolean preExistenceCheckDone;
     private boolean modified;
     private TaggingContext taggingContext;
     private Map<String, Long> timestamps;

aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/CreateHandler.java

@@ -10,6 +10,7 @@
 import software.amazon.rds.common.handler.Commons;
 import software.amazon.rds.common.handler.HandlerConfig;
 import software.amazon.rds.common.handler.Tagging;
+import software.amazon.rds.common.util.IdempotencyHelper;
 import software.amazon.rds.common.util.IdentifierFactory;
 
 public class CreateHandler extends BaseHandlerStd {
@@ -48,7 +49,14 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
                 .build();
 
         return ProgressEvent.progress(model, callbackContext)
-                .then(progress -> safeCreateCustomEngineVersion(proxy, proxyClient, progress, allTags))
+                .then(progress -> IdempotencyHelper.safeCreate(
+                        m -> fetchDBEngineVersion(m, proxyClient),
+                        p -> safeCreateCustomEngineVersion(proxy, proxyClient, p, allTags),
+                        ResourceModel.TYPE_NAME,
+                        model.getEngineVersion(),
+                        progress,
+                        requestLogger
+                ))
                 .then(progress -> {
                     if (shouldModifyEngineVersionAfterCreate(progress)) {
                         return Commons.execOnce(

aws-rds-customdbengineversion/src/main/java/software/amazon/rds/customdbengineversion/DeleteHandler.java

@@ -3,9 +3,8 @@
 import java.util.function.Function;
 
 import software.amazon.awssdk.services.rds.RdsClient;
-import software.amazon.awssdk.services.rds.model.CustomDbEngineVersionNotFoundException;
+import software.amazon.cloudformation.exceptions.CfnNotFoundException;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
-import software.amazon.cloudformation.proxy.Logger;
 import software.amazon.cloudformation.proxy.ProgressEvent;
 import software.amazon.cloudformation.proxy.ProxyClient;
 import software.amazon.cloudformation.proxy.ResourceHandlerRequest;
@@ -67,7 +66,7 @@ protected boolean isDeleted(final ResourceModel model,
         try {
             fetchDBEngineVersion(model, proxyClient);
             return false;
-        } catch (CustomDbEngineVersionNotFoundException e) {
+        } catch (CfnNotFoundException e) {
             return true;
         }
     }

aws-rds-customdbengineversion/src/test/java/software/amazon/rds/customdbengineversion/CreateHandlerTest.java

@@ -43,6 +43,7 @@
 import software.amazon.rds.common.error.ErrorCode;
 import software.amazon.rds.common.handler.HandlerConfig;
 import software.amazon.rds.common.handler.Tagging;
+import software.amazon.rds.common.util.IdempotencyHelper;
 import software.amazon.rds.test.common.core.HandlerName;
 
 @ExtendWith(MockitoExtension.class)
@@ -73,6 +74,7 @@ public void setup() {
         rdsClient = mock(RdsClient.class);
         proxy = new AmazonWebServicesClientProxy(logger, MOCK_CREDENTIALS, () -> Duration.ofSeconds(600).toMillis());
         rdsProxy = mockProxy(proxy, rdsClient);
+        IdempotencyHelper.setBypass(true);
     }
 
     @AfterEach

aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java

@@ -69,6 +69,7 @@
 import software.amazon.awssdk.services.rds.paginators.DescribeDBClustersIterable;
 import software.amazon.awssdk.utils.StringUtils;
 import software.amazon.cloudformation.exceptions.CfnInvalidRequestException;
+import software.amazon.cloudformation.exceptions.CfnNotFoundException;
 import software.amazon.cloudformation.exceptions.CfnNotStabilizedException;
 import software.amazon.cloudformation.exceptions.ResourceNotFoundException;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
@@ -139,6 +140,7 @@ public abstract class BaseHandlerStd extends BaseHandler<CallbackContext> {
             .withErrorClasses(ErrorStatus.failWith(HandlerErrorCode.AlreadyExists),
                     DbClusterAlreadyExistsException.class)
             .withErrorClasses(ErrorStatus.failWith(HandlerErrorCode.NotFound),
+                    CfnNotFoundException.class, // FIXME: handle BaseHandlerException in ErrorRuleSet
                     DbClusterNotFoundException.class,
                     DbClusterSnapshotNotFoundException.class,
                     DbClusterParameterGroupNotFoundException.class,
@@ -267,18 +269,20 @@ protected DBCluster fetchDBCluster(
             final ProxyClient<RdsClient> proxyClient,
             final ResourceModel model
     ) {
-        final DescribeDbClustersResponse response = proxyClient.injectCredentialsAndInvokeV2(
-                Translator.describeDbClustersRequest(model),
-                proxyClient.client()::describeDBClusters
-        );
+        try {
+            final DescribeDbClustersResponse response = proxyClient.injectCredentialsAndInvokeV2(
+                    Translator.describeDbClustersRequest(model),
+                    proxyClient.client()::describeDBClusters
+            );
 
-        if (response.dbClusters().isEmpty()) {
-            throw DbClusterNotFoundException.builder()
-                .message(String.format("No clusters of identifier %s returned from describe call", model.getDBClusterIdentifier()))
-                .build();
-        }
+            if (!response.hasDbClusters() || response.dbClusters().isEmpty()) {
+                throw new CfnNotFoundException(ResourceModel.TYPE_NAME, model.getDBClusterIdentifier());
+            }
 
-        return response.dbClusters().get(0);
+            return response.dbClusters().get(0);
+        } catch (DbClusterNotFoundException e) {
+            throw new CfnNotFoundException(e);
+        }
     }
 
     protected GlobalCluster fetchGlobalCluster(
@@ -474,7 +478,7 @@ protected boolean isDBClusterDeleted(
     ) {
         try {
             fetchDBCluster(proxyClient, model);
-        } catch (DbClusterNotFoundException e) {
+        } catch (CfnNotFoundException e) {
             return true;
         }
         return false;

aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/CallbackContext.java

@@ -10,12 +10,14 @@
 import software.amazon.rds.common.handler.ProbingContext;
 import software.amazon.rds.common.handler.TaggingContext;
 import software.amazon.rds.common.handler.TimestampContext;
+import software.amazon.rds.common.util.IdempotencyHelper;
 
 @lombok.Getter
 @lombok.Setter
 @lombok.ToString
 @lombok.EqualsAndHashCode(callSuper = true)
-public class CallbackContext extends StdCallbackContext implements TaggingContext.Provider, ProbingContext.Provider, TimestampContext.Provider {
+public class CallbackContext extends StdCallbackContext implements TaggingContext.Provider, ProbingContext.Provider, TimestampContext.Provider, IdempotencyHelper.PreExistenceContext {
+    private Boolean preExistenceCheckDone;
     private boolean modified;
     private boolean rebooted;
     private boolean deleting;