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 42dad91c..ffc3e275 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 @@ -11,11 +11,8 @@ import java.util.Optional; import java.util.Set; import java.util.function.BiFunction; -import java.util.function.Function; import java.util.stream.Collectors; -import org.apache.commons.lang3.BooleanUtils; - import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -24,7 +21,6 @@ import com.google.common.collect.Maps; import lombok.NonNull; import software.amazon.awssdk.services.rds.RdsClient; -import software.amazon.awssdk.services.rds.model.DBCluster; import software.amazon.awssdk.services.rds.model.DbClusterParameterGroupNotFoundException; import software.amazon.awssdk.services.rds.model.DbParameterGroupAlreadyExistsException; import software.amazon.awssdk.services.rds.model.DbParameterGroupNotFoundException; @@ -33,8 +29,6 @@ import software.amazon.awssdk.services.rds.model.DescribeDbClusterParameterGroupsResponse; import software.amazon.awssdk.services.rds.model.DescribeDbClusterParametersRequest; import software.amazon.awssdk.services.rds.model.DescribeDbClusterParametersResponse; -import software.amazon.awssdk.services.rds.model.DescribeDbClustersRequest; -import software.amazon.awssdk.services.rds.model.DescribeDbClustersResponse; import software.amazon.awssdk.services.rds.model.DescribeEngineDefaultClusterParametersRequest; import software.amazon.awssdk.services.rds.model.DescribeEngineDefaultClusterParametersResponse; import software.amazon.awssdk.services.rds.model.EngineDefaults; @@ -57,13 +51,13 @@ import software.amazon.rds.common.error.ErrorStatus; import software.amazon.rds.common.handler.Commons; import software.amazon.rds.common.handler.HandlerConfig; -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; import software.amazon.rds.common.printer.FilteredJsonPrinter; import software.amazon.rds.common.util.ParameterGrouper; + public abstract class BaseHandlerStd extends BaseHandler { public static final List> PARAMETER_DEPENDENCIES = ImmutableList.of( ImmutableSet.of("collation_server", "character_set_server"), @@ -99,16 +93,6 @@ public abstract class BaseHandlerStd extends BaseHandler { DbParameterGroupNotFoundException.class) .build(); - protected static final ErrorRuleSet DB_CLUSTERS_STABILIZATION_ERROR_RULE_SET = ErrorRuleSet - .extend(Commons.DEFAULT_ERROR_RULE_SET) - .withErrorCodes(ErrorStatus.ignore(), - ErrorCode.AccessDeniedException, - ErrorCode.AccessDenied, - ErrorCode.NotAuthorized) - .withErrorClasses(ErrorStatus.failWith(HandlerErrorCode.NotStabilized), - Exception.class) - .build(); - protected static final ErrorRuleSet SOFT_FAIL_IN_PROGRESS_ERROR_RULE_SET = ErrorRuleSet .extend(DEFAULT_DB_CLUSTER_PARAMETER_GROUP_ERROR_RULE_SET) .withErrorCodes(ErrorStatus.ignore(OperationStatus.IN_PROGRESS), @@ -211,17 +195,15 @@ protected ProgressEvent updateTags( } protected ProgressEvent applyParameters( - final AmazonWebServicesClientProxy proxy, final ProxyClient proxyClient, final ProgressEvent progress, final Map currentClusterParameters, - final RequestLogger requestLogger + final Map desiredClusterParameters ) { return ProgressEvent.progress(progress.getResourceModel(), progress.getCallbackContext()) - .then(progressEvent -> validateModelParameters(progressEvent, currentClusterParameters)) - .then(progressEvent -> Commons.execOnce(progressEvent, () -> modifyParameters(progressEvent, currentClusterParameters, proxy, proxyClient), - CallbackContext::isParametersModified, CallbackContext::setParametersModified)) - .then(progressEvent -> waitForDbClustersStabilization(progressEvent, proxy, proxyClient)); + .then(progressEvent -> validateModelParameters(progressEvent, currentClusterParameters, desiredClusterParameters)) + .then(progressEvent -> Commons.execOnce(progressEvent, () -> modifyParameters(progressEvent, proxyClient, currentClusterParameters, desiredClusterParameters), + CallbackContext::isParametersModified, CallbackContext::setParametersModified)); } protected Completed describeDbClusterParameterGroup( requestLogger)); } + /** + * This function validates that all the parameters we want to modify can be modified + * A parameter is considered invalid to be modified, if it is both not modifiable, and the value has actually changed + * + * @param progress CloudFormation progress event + * @param currentClusterParameters Cluster parameters currently attached to the physical Cluster Parameter Group Resource + * @param desiredClusterParameters Cluster parameters which are desired, and we wish to apply + * @return CloudFormation progress event + */ private ProgressEvent validateModelParameters( final ProgressEvent progress, - final Map defaultEngineParameters + final Map currentClusterParameters, + final Map desiredClusterParameters ) { + // we don't care about putAll overwriting overlapping keys, because we just need the map + // for the isModifiable field + final Map combinedParams = Maps.newHashMap(); + combinedParams.putAll(currentClusterParameters); + combinedParams.putAll(desiredClusterParameters); + final Map modelParameters = Optional.ofNullable(progress.getResourceModel().getParameters()).orElse(Collections.emptyMap()); final List invalidParameters = modelParameters.keySet().stream() - .filter(parameterName -> { - if (!defaultEngineParameters.containsKey(parameterName)) { + .filter(modelParameterName -> { + final String newParameterValue = String.valueOf(modelParameters.get(modelParameterName)); + final Parameter currentParameterValue = currentClusterParameters.get(modelParameterName); + + if (combinedParams.get(modelParameterName) == null) { + // it should not go in here, because combinedParams should contain all + // current and desired parameters + requestLogger.log("DBClusterParameters did not contain %s", modelParameterName); return true; } - final String newParameterValue = String.valueOf(modelParameters.get(parameterName)); - final Parameter defaultParameter = defaultEngineParameters.get(parameterName); - //Parameter is not modifiable and input model contains different value from default value - return newParameterValue != null && BooleanUtils.isNotTrue(defaultParameter.isModifiable()) - && !newParameterValue.equals(defaultParameter.parameterValue()); + + final boolean isParameterModifiable = combinedParams.get(modelParameterName).isModifiable(); + + // if the parameter is not currently set on our parameter group + // and the parameter is not modifiable, it means it is an invalid change + if (currentParameterValue == null) { + return !isParameterModifiable; + } + // Parameter is not modifiable and the desired value is different to the current value + return !isParameterModifiable && !newParameterValue.equals(currentParameterValue.parameterValue()); }) - .collect(Collectors.toList()); + .toList(); if (!invalidParameters.isEmpty()) { return ProgressEvent.failed( @@ -288,6 +297,7 @@ private Iterable fetchDBClusterParametersIterable( request.toBuilder().marker(marker).build(), proxyClient.client()::describeDBClusterParameters ); + if (response.parameters() != null) { result = Iterables.concat(result, response.parameters()); } @@ -309,6 +319,7 @@ private Iterable fetchDBClusterParametersIterableWithFilters( final DescribeDbClusterParametersRequest request = DescribeDbClusterParametersRequest.builder() .dbClusterParameterGroupName(dbClusterParameterGroupName) .build(); + iterable = fetchDBClusterParametersIterable(proxyClient, request); } else { for (final List partition : Lists.partition(filterParameterNames, MAX_PARAMETER_FILTER_SIZE)) { @@ -324,7 +335,7 @@ private Iterable fetchDBClusterParametersIterableWithFilters( return iterable; } - protected ProgressEvent describeCurrentDBClusterParameters( + protected ProgressEvent describeDBClusterParameters( final AmazonWebServicesClientProxy proxy, final ProxyClient proxyClient, final ProgressEvent progress, @@ -427,16 +438,17 @@ protected ProgressEvent describeEngineDefaultClu private Map getParametersToModify( final Map modelParameters, - final Map currentDBParameters + final Map currentDBParameters, + final Map desiredDBParameters ) { - final Map parametersToModify = Maps.newHashMap(currentDBParameters); - parametersToModify.keySet().retainAll(Optional.ofNullable(modelParameters).orElse(Collections.emptyMap()).keySet()); + final Map parametersToModify = Maps.newHashMap(desiredDBParameters); + return parametersToModify.entrySet() .stream() //filter to parameters want to modify and its value is different from already exist value .filter(entry -> { final String parameterName = entry.getKey(); - final String currentParameterValue = entry.getValue().parameterValue(); + final String currentParameterValue = currentDBParameters.get(entry.getKey()) != null ? currentDBParameters.get(entry.getKey()).parameterValue() : null; final String newParameterValue = String.valueOf(modelParameters.get(parameterName)); return !newParameterValue.equals(currentParameterValue); }) @@ -459,6 +471,7 @@ static Map computeModifiedDBParameters( for (final String paramName : currentDBParameters.keySet()) { final Parameter currentParam = currentDBParameters.get(paramName); final Parameter defaultParam = engineDefaultParameters.get(paramName); + if (defaultParam == null || !Objects.equals(defaultParam.parameterValue(), currentParam.parameterValue())) { modifiedParameters.put(paramName, currentParam); } @@ -467,16 +480,32 @@ static Map computeModifiedDBParameters( return modifiedParameters; } + /** + * Checks if parameter has been modified out of band and no longer matches + * the CloudFormation model. This can happen if the parameter is modified + * directly in the AWS console or API. + * + * @return True if the parameter has been overridden out of band and the + * physical resource no longer matching with the CloudFormation model + */ + private boolean isParameterModifiedOutOfBand(final String prevModelParamValue, final String currentParameterValue) { + return !prevModelParamValue.equals(currentParameterValue); + } + protected ProgressEvent resetParameters( final ProgressEvent progress, final AmazonWebServicesClientProxy proxy, final ProxyClient proxyClient, + final Map previousModelParams, final Map currentParameters, - final Map desiredParams + final Map desiredParameters ) { // reset only parameters that are missing from desired params. If parameter is in desired param, its value will be updated anyway. final Map toBeReset = currentParameters.keySet().stream() - .filter(p -> !desiredParams.containsKey(p)) + .filter(p -> { + return !desiredParameters.containsKey(p) && + !isParameterModifiedOutOfBand(previousModelParams.get(p).toString(), currentParameters.get(p).parameterValue()); + }) .collect(Collectors.toMap(kv -> kv, currentParameters::get)); if (toBeReset.isEmpty()) { @@ -497,13 +526,13 @@ protected ProgressEvent resetParameters( private ProgressEvent modifyParameters( final ProgressEvent progress, + final ProxyClient proxyClient, final Map currentDBParameters, - final AmazonWebServicesClientProxy proxy, - final ProxyClient proxyClient + final Map desiredDBParameters ) { final ResourceModel model = progress.getResourceModel(); final CallbackContext context = progress.getCallbackContext(); - final Map parametersToModify = getParametersToModify(model.getParameters(), currentDBParameters); + final Map parametersToModify = getParametersToModify(model.getParameters(), currentDBParameters, desiredDBParameters); try { for (final List partition : ParameterGrouper.partition(parametersToModify, PARAMETER_DEPENDENCIES, MAX_PARAMETERS_PER_REQUEST)) { @@ -522,32 +551,6 @@ private ProgressEvent modifyParameters( return ProgressEvent.progress(model, context); } - protected boolean isDBClustersAvailable(final ProxyClient proxyClient, final ResourceModel model) { - String marker = null; - int page = 1; - final DescribeDbClustersRequest request = Translator.describeDbClustersRequestForDBClusterParameterGroup(model); - - do { - if (page > MAX_DESCRIBE_PAGE_DEPTH) { - throw new CfnInvalidRequestException("Max describeDBClusters response page reached."); - } - final DescribeDbClustersResponse response = proxyClient.injectCredentialsAndInvokeV2( - request.toBuilder().marker(marker).build(), - proxyClient.client()::describeDBClusters - ); - final List dbClusters = Optional.ofNullable(response.dbClusters()).orElse(Collections.emptyList()); - for (final DBCluster dbCluster : dbClusters) { - if (!AVAILABLE.equalsIgnoreCase(dbCluster.status())) { - return false; - } - } - marker = response.marker(); - page++; - } while (marker != null); - - return true; - } - private void resourceStabilizationTime(final CallbackContext callbackContext) { callbackContext.timestampOnce(DB_CLUSTER_PARAMETER_GROUP_REQUEST_STARTED_AT, Instant.now()); callbackContext.timestamp(DB_CLUSTER_PARAMETER_GROUP_REQUEST_IN_PROGRESS_AT, Instant.now()); @@ -555,26 +558,4 @@ private void resourceStabilizationTime(final CallbackContext callbackContext) { callbackContext.getTimestamp(DB_CLUSTER_PARAMETER_GROUP_REQUEST_IN_PROGRESS_AT), callbackContext.getTimestamp(DB_CLUSTER_PARAMETER_GROUP_REQUEST_STARTED_AT)); } - - protected ProgressEvent waitForDbClustersStabilization( - final ProgressEvent progress, - final AmazonWebServicesClientProxy proxy, - final ProxyClient proxyClient - ) { - return proxy.initiate("rds::stabilize-db-cluster-parameter-group-db-clusters", proxyClient, progress.getResourceModel(), progress.getCallbackContext()) - .translateToServiceRequest(Function.identity()) - .backoffDelay(config.getBackoff()) - .makeServiceCall(EMPTY_CALL) - .stabilize((request, response, proxyInvocation, model, context) -> Probing.withProbing(context.getProbingContext(), - "db-cluster-parameter-group-db-clusters-available", - 3, - () -> isDBClustersAvailable(proxyInvocation, model))) - .handleError((describeDbParameterGroupsRequest, exception, client, resourceModel, ctx) -> - Commons.handleException( - ProgressEvent.progress(resourceModel, ctx), - exception, - DB_CLUSTERS_STABILIZATION_ERROR_RULE_SET, - requestLogger)) - .progress(); - } } 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 1a7602aa..70179c16 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 @@ -1,6 +1,7 @@ package software.amazon.rds.dbclusterparametergroup; import java.util.ArrayList; +import java.util.Collections; import java.util.Map; import com.amazonaws.util.StringUtils; @@ -46,7 +47,7 @@ protected ProgressEvent handleRequest( .build(); final Map desiredParams = request.getDesiredResourceState().getParameters(); - final Map currentClusterParameters = Maps.newHashMap(); + final Map desiredClusterParameters = Maps.newHashMap(); return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext) .then(progress -> setDbClusterParameterGroupNameIfMissing(request, progress)) @@ -59,8 +60,8 @@ protected ProgressEvent handleRequest( return updateTags(proxy, proxyClient, progress, Tagging.TagSet.emptySet(), extraTags); }, CallbackContext::isAddTagsComplete, CallbackContext::setAddTagsComplete)) .then(progress -> Commons.execOnce(progress, () -> - describeCurrentDBClusterParameters(proxy, proxyClient, progress, new ArrayList<>(desiredParams.keySet()), currentClusterParameters) - .then(p -> applyParameters(proxy, proxyClient, progress, currentClusterParameters, requestLogger) + describeDBClusterParameters(proxy, proxyClient, progress, new ArrayList<>(desiredParams.keySet()), desiredClusterParameters) + .then(p -> applyParameters(proxyClient, progress, Collections.emptyMap(), desiredClusterParameters) ), CallbackContext::isParametersApplied, CallbackContext::setParametersApplied)) .then(progress -> new ReadHandler().handleRequest(proxy, proxyClient, request, callbackContext)); 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 a7b97af8..66d376c4 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 @@ -63,7 +63,7 @@ private ProgressEvent readParameters( return progress .then(p -> describeEngineDefaultClusterParameters(proxy, proxyClient, p, null, engineDefaultClusterParameters)) - .then(p -> describeCurrentDBClusterParameters(proxy, proxyClient, p, null, currentDBClusterParameters)) + .then(p -> describeDBClusterParameters(proxy, proxyClient, p, null, currentDBClusterParameters)) .then(p -> { p.getResourceModel().setParameters( Translator.translateParametersFromSdk(computeModifiedDBParameters(engineDefaultClusterParameters, currentDBClusterParameters)) diff --git a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/UpdateHandler.java b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/UpdateHandler.java index afcadd29..f0f7bf15 100644 --- a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/UpdateHandler.java +++ b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/UpdateHandler.java @@ -1,7 +1,9 @@ package software.amazon.rds.dbclusterparametergroup; -import java.util.ArrayList; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import com.google.common.collect.Maps; import software.amazon.awssdk.services.rds.RdsClient; @@ -46,30 +48,49 @@ protected ProgressEvent handleRequest( .resourceTags(Translator.translateTagsToSdk(request.getDesiredResourceState().getTags())) .build(); - final Map previousParams = request.getPreviousResourceState().getParameters(); - final Map desiredParams = request.getDesiredResourceState().getParameters(); - final boolean shouldUpdateParameters = !DifferenceUtils.diff(previousParams, desiredParams).isEmpty(); + final Map previousModelParams = request.getPreviousResourceState().getParameters() == null ? Map.of() : request.getPreviousResourceState().getParameters(); + final Map desiredModelParams = request.getDesiredResourceState().getParameters() == null ? Map.of() : request.getDesiredResourceState().getParameters(); + final boolean shouldUpdateParameters = !DifferenceUtils.diff(previousModelParams, desiredModelParams).isEmpty(); final Map currentClusterParameters = Maps.newHashMap(); + final Map desiredClusterParameters = Maps.newHashMap(); + + // contains cluster parameters from current and desired parameters so we can use to access the + // metadata from the describeDbClusterParameters response + final Map allClusterParameters = Maps.newHashMap(); return ProgressEvent.progress(model, callbackContext) - .then(progress -> { - if (shouldUpdateParameters) { - return describeCurrentDBClusterParameters(proxy, proxyClient, progress, new ArrayList<>(desiredParams.keySet()), currentClusterParameters); - } - return progress; - }).then(progress -> Commons.execOnce(progress, () -> updateTags(proxy, proxyClient, progress, previousTags, desiredTags), CallbackContext::isAddTagsComplete, CallbackContext::setAddTagsComplete)) - .then(progress -> { - if (shouldUpdateParameters) { - return resetParameters(progress, proxy, proxyClient, currentClusterParameters, desiredParams); + .then(progress -> { + if (shouldUpdateParameters) { + // we need to query for all parameters in the filter from current resource and desired model + // because it's possible for a parameter to be removed when updating to desired model + final Set filter = Stream.concat(desiredModelParams.keySet().stream(), previousModelParams.keySet().stream()).collect(Collectors.toSet()); + return describeDBClusterParameters(proxy, proxyClient, progress, filter.stream().toList(), allClusterParameters); + } + return progress; + }).then(progress -> { + for (final Map.Entry entry : allClusterParameters.entrySet()) { + if (previousModelParams.containsKey(entry.getKey())) { + currentClusterParameters.put(entry.getKey(), entry.getValue()); } - return progress; - }).then(progress -> Commons.execOnce(progress, () -> { - if (shouldUpdateParameters) { - return applyParameters(proxy, proxyClient, progress, currentClusterParameters, requestLogger); + if (desiredModelParams.containsKey(entry.getKey())) { + desiredClusterParameters.put(entry.getKey(), entry.getValue()); } - return progress; - }, CallbackContext::isParametersApplied, CallbackContext::setParametersApplied)) - .then(progress -> new ReadHandler().handleRequest(proxy, proxyClient, request, callbackContext)); + } + return progress; + }) + .then(progress -> Commons.execOnce(progress, () -> updateTags(proxy, proxyClient, progress, previousTags, desiredTags), CallbackContext::isAddTagsComplete, CallbackContext::setAddTagsComplete)) + .then(progress -> { + if (shouldUpdateParameters) { + return resetParameters(progress, proxy, proxyClient, previousModelParams, currentClusterParameters, desiredClusterParameters); + } + return progress; + }).then(progress -> Commons.execOnce(progress, () -> { + if (shouldUpdateParameters) { + return applyParameters(proxyClient, progress, currentClusterParameters, desiredClusterParameters); + } + return progress; + }, CallbackContext::isParametersApplied, CallbackContext::setParametersApplied)) + .then(progress -> new ReadHandler().handleRequest(proxy, proxyClient, request, callbackContext)); } } diff --git a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java index ec79d2aa..8ce4ee3e 100644 --- a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java +++ b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java @@ -183,11 +183,25 @@ public RdsClient client() { }; } - protected List translateParamMapToCollection(final Map params) { + /** + * Translates a map of any such key value pairs to a List of RDS-model Parameters + * + * The modifiable parameter sets all the parameters in the map to be modifiable or not + * See: https://docs.aws.amazon.com/cli/latest/reference/rds/describe-db-parameters.html for modifiable + * + * This function is needed for unit tests because only modifiable parameters can be modified. + * Non modifiable parameters cannot be reset or modified. + * + * @param params Map of key value pairs, eg. {"client_encoding": "UTF-8"} + * @param modifiable Should all the parameters in the map be modifiable + * @return List of RDS-model Parameters for DBClusterParameterGroups or DBParameterGroups + */ + protected List translateParamMapToCollection(final Map params, final boolean modifiable) { return params.entrySet().stream().map(entry -> Parameter.builder() .parameterName(entry.getKey()) .parameterValue((String) entry.getValue()) .applyType(ParameterType.Dynamic.toString()) + .isModifiable(modifiable) .build()) .collect(Collectors.toList()); } 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 53487de3..679c9f70 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 @@ -3,12 +3,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doAnswer; 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 static org.mockito.Mockito.doAnswer; import java.time.Duration; import java.time.Instant; @@ -20,7 +20,10 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.IntStream; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import lombok.Getter; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -29,10 +32,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; -import lombok.Getter; import software.amazon.awssdk.awscore.exception.AwsErrorDetails; import software.amazon.awssdk.awscore.exception.AwsServiceException; import software.amazon.awssdk.services.rds.RdsClient; @@ -349,11 +348,6 @@ public void handleRequest_MissingDescribeDBClusterParameterGroupsPermission() { AwsErrorDetails.builder().errorCode(HandlerErrorCode.AccessDenied.toString()).build() ).build()); - when(rdsClient.describeDBClusters(any(DescribeDbClustersRequest.class))) - .thenReturn(DescribeDbClustersResponse.builder() - .dbClusters(DBCluster.builder().dbClusterParameterGroup("group").status("available").build()) - .build()); - when(rdsClient.listTagsForResource(any(ListTagsForResourceRequest.class))) .thenReturn(ListTagsForResourceResponse.builder().build()); @@ -368,38 +362,6 @@ public void handleRequest_MissingDescribeDBClusterParameterGroupsPermission() { verify(rdsProxy.client(), times(1)).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); verify(rdsProxy.client(), times(1)).modifyDBClusterParameterGroup(any(ModifyDbClusterParameterGroupRequest.class)); - verify(rdsProxy.client(), times(1)).describeDBClusters(any(DescribeDbClustersRequest.class)); - } - - @Test - public void handleRequest_ThrottleOnDescribeDBClusters() { - when(rdsClient.createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class))) - .thenReturn(CreateDbClusterParameterGroupResponse.builder() - .dbClusterParameterGroup(DB_CLUSTER_PARAMETER_GROUP) - .build()); - - when(rdsClient.modifyDBClusterParameterGroup(any(ModifyDbClusterParameterGroupRequest.class))) - .thenReturn(ModifyDbClusterParameterGroupResponse.builder().build()); - - when(rdsClient.describeDBClusters(any(DescribeDbClustersRequest.class))) - .thenThrow(AwsServiceException.builder() - .awsErrorDetails(AwsErrorDetails.builder() - .errorCode(HandlerErrorCode.Throttling.toString()) - .build()) - .build()); - - mockDescribeDbClusterParametersResponse("static", "dynamic", true); - - test_handleRequest_base( - new CallbackContext(), - null, - () -> RESOURCE_MODEL, - expectFailed(HandlerErrorCode.Throttling) - ); - - verify(rdsProxy.client(), times(1)).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); - verify(rdsProxy.client(), times(1)).modifyDBClusterParameterGroup(any(ModifyDbClusterParameterGroupRequest.class)); - verify(rdsProxy.client(), times(1)).describeDBClusters(any(DescribeDbClustersRequest.class)); } @Test @@ -413,11 +375,6 @@ public void handleRequest_SuccessWithParameters() { when(rdsClient.modifyDBClusterParameterGroup(any(ModifyDbClusterParameterGroupRequest.class))) .thenReturn(ModifyDbClusterParameterGroupResponse.builder().build()); - when(rdsClient.describeDBClusters(any(DescribeDbClustersRequest.class))) - .thenReturn(DescribeDbClustersResponse.builder() - .dbClusters(DBCluster.builder().dbClusterParameterGroup("group").status("available").build()) - .build()); - final DBClusterParameterGroup dbClusterParameterGroup = DBClusterParameterGroup.builder() .dbClusterParameterGroupArn(ARN) .dbClusterParameterGroupName(RESOURCE_MODEL.getDBClusterParameterGroupName()) @@ -440,7 +397,6 @@ public void handleRequest_SuccessWithParameters() { verify(rdsProxy.client(), times(1)).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); verify(rdsProxy.client(), times(1)).modifyDBClusterParameterGroup(any(ModifyDbClusterParameterGroupRequest.class)); - verify(rdsProxy.client(), times(1)).describeDBClusters(any(DescribeDbClustersRequest.class)); } /** @@ -468,11 +424,6 @@ public void handleRequest_SuccessSplitParameters() { when(rdsClient.modifyDBClusterParameterGroup(any(ModifyDbClusterParameterGroupRequest.class))) .thenReturn(ModifyDbClusterParameterGroupResponse.builder().build()); - when(rdsClient.describeDBClusters(any(DescribeDbClustersRequest.class))) - .thenReturn(DescribeDbClustersResponse.builder() - .dbClusters(DBCluster.builder().dbClusterParameterGroup("group").status("available").build()) - .build()); - final DBClusterParameterGroup dbClusterParameterGroup = DBClusterParameterGroup.builder() .dbClusterParameterGroupArn(ARN) .dbClusterParameterGroupName(RESOURCE_MODEL.getDBClusterParameterGroupName()) @@ -509,7 +460,6 @@ public void handleRequest_SuccessSplitParameters() { verify(rdsProxy.client(), times(1)).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); verify(rdsProxy.client(), times(3)).modifyDBClusterParameterGroup(captor.capture()); - verify(rdsProxy.client(), times(1)).describeDBClusters(any(DescribeDbClustersRequest.class)); ModifyDbClusterParameterGroupRequest firstRequest = captor.getAllValues().get(0); assertThat(verifyParameterExistsInRequest("aurora_enhanced_binlog", firstRequest)).isEqualTo(true); @@ -524,11 +474,6 @@ public void handleRequest_SuccessWithEmptyParameters() { .dbClusterParameterGroup(DB_CLUSTER_PARAMETER_GROUP) .build()); - when(rdsClient.describeDBClusters(any(DescribeDbClustersRequest.class))) - .thenReturn(DescribeDbClustersResponse.builder() - .dbClusters(DBCluster.builder().dbClusterParameterGroup("group").status("available").build()) - .build()); - final DBClusterParameterGroup dbClusterParameterGroup = DBClusterParameterGroup.builder() .dbClusterParameterGroupArn(ARN) .dbClusterParameterGroupName(RESOURCE_MODEL.getDBClusterParameterGroupName()) @@ -550,7 +495,6 @@ public void handleRequest_SuccessWithEmptyParameters() { ); verify(rdsProxy.client()).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); - verify(rdsProxy.client()).describeDBClusters(any(DescribeDbClustersRequest.class)); } @Test @@ -582,16 +526,16 @@ public void handleRequest_InProgressFailedUnsupportedParams() { .dbClusterParameterGroup(DB_CLUSTER_PARAMETER_GROUP) .build()); - mockDescribeDbClusterParametersResponse("static", "dynamic", true); + mockDescribeDbClusterParametersResponse("static", "dynamic", false); final ProgressEvent response = test_handleRequest_base( new CallbackContext(), null, - () -> RESOURCE_MODEL.toBuilder().parameters(Collections.singletonMap("Wrong Key", "Wrong value")).build(), + () -> RESOURCE_MODEL.toBuilder().parameters(Collections.singletonMap("param", "updated value")).build(), expectFailed(HandlerErrorCode.InvalidRequest) ); - assertThat(response.getMessage()).isEqualTo("Invalid / Unmodifiable / Unsupported DB Parameter: Wrong Key"); + assertThat(response.getMessage()).isEqualTo("Invalid / Unmodifiable / Unsupported DB Parameter: param"); verify(rdsProxy.client()).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class)); verify(rdsProxy.client(), times(2)).describeDBClusterParameters(any(DescribeDbClusterParametersRequest.class)); diff --git a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/UpdateHandlerTest.java b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/UpdateHandlerTest.java index 07b1ffff..a224f353 100644 --- a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/UpdateHandlerTest.java +++ b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/UpdateHandlerTest.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; +import lombok.Getter; import org.assertj.core.api.Assertions; import org.assertj.core.util.Lists; import org.junit.jupiter.api.AfterEach; @@ -22,8 +23,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; - -import lombok.Getter; import software.amazon.awssdk.services.rds.RdsClient; import software.amazon.awssdk.services.rds.model.AddTagsToResourceRequest; import software.amazon.awssdk.services.rds.model.AddTagsToResourceResponse; @@ -165,7 +164,10 @@ public void handleRequest_ResetRemovedParameters() { when(rdsClient.describeDBClusterParameters(any(DescribeDbClusterParametersRequest.class))) .thenReturn(DescribeDbClusterParametersResponse.builder() - .parameters(Parameter.builder().parameterName("parameter1").build()).build()); + .parameters( + Parameter.builder().parameterName("param").parameterValue("value").build(), + Parameter.builder().parameterName("param2").parameterValue("value").build() + ).build()); CallbackContext callbackContext = new CallbackContext(); callbackContext.setParametersApplied(true); @@ -173,7 +175,7 @@ public void handleRequest_ResetRemovedParameters() { test_handleRequest_base( callbackContext, () -> DBClusterParameterGroup.builder().dbClusterParameterGroupArn(ARN).build(), - () -> RESOURCE_MODEL_PREV, + () -> RESOURCE_MODEL, () -> RESOURCE_MODEL_UPD, expectSuccess() ); @@ -181,11 +183,11 @@ public void handleRequest_ResetRemovedParameters() { ArgumentCaptor captor = ArgumentCaptor.forClass(ResetDbClusterParameterGroupRequest.class); verify(rdsProxy.client(), times(1)).resetDBClusterParameterGroup(captor.capture()); Assertions.assertThat(captor.getValue().parameters()).hasSize(1); - Assertions.assertThat(captor.getValue().parameters().get(0).parameterName()).isEqualTo("parameter1"); - + Assertions.assertThat(captor.getValue().parameters().get(0).parameterName()).isEqualTo("param2"); verify(rdsProxy.client(), times(1)).describeDBClusterParameterGroups(any(DescribeDbClusterParameterGroupsRequest.class)); verify(rdsProxy.client(), times(1)).addTagsToResource(any(AddTagsToResourceRequest.class)); + verify(rdsProxy.client(), times(1)).removeTagsFromResource(any(RemoveTagsFromResourceRequest.class)); } @Test @@ -220,7 +222,7 @@ public void handleRequest_StabilizeDBClusters() { when(rdsClient.describeDBClusterParameters(any(DescribeDbClusterParametersRequest.class))) .thenReturn(DescribeDbClusterParametersResponse.builder() - .parameters(translateParamMapToCollection(PARAMS)) + .parameters(translateParamMapToCollection(PARAMS, true)) .build()); final DBClusterParameterGroup dbClusterParameterGroup = DBClusterParameterGroup.builder() @@ -242,14 +244,6 @@ public void handleRequest_StabilizeDBClusters() { .dbClusterParameterGroup("group-name") .build(); - when(rdsClient.describeDBClusters(any(DescribeDbClustersRequest.class))) - .thenReturn(DescribeDbClustersResponse.builder() - .dbClusters(dbCluster.toBuilder().status("modifying").build()) - .build()) - .thenReturn(DescribeDbClustersResponse.builder() - .dbClusters(DBCluster.builder().status("available").build()) - .build()); - test_handleRequest_base( new CallbackContext(), () -> dbClusterParameterGroup, @@ -260,7 +254,6 @@ public void handleRequest_StabilizeDBClusters() { verify(rdsProxy.client(), times(1)).resetDBClusterParameterGroup(any(ResetDbClusterParameterGroupRequest.class)); verify(rdsProxy.client(), times(1)).describeDBClusterParameters(any(DescribeDbClusterParametersRequest.class)); - verify(rdsProxy.client(), times(2)).describeDBClusters(any(DescribeDbClustersRequest.class)); verify(rdsProxy.client(), times(1)).describeDBClusterParameterGroups(any(DescribeDbClusterParameterGroupsRequest.class)); verify(rdsProxy.client(), times(1)).listTagsForResource(any(ListTagsForResourceRequest.class)); verify(rdsProxy.client(), times(1)).addTagsToResource(any(AddTagsToResourceRequest.class));