Skip to content

Commit

Permalink
CFN-564(DBClusterParameterGroup) - Fix DBClusterParameterGroup not re…
Browse files Browse the repository at this point in the history 
…setting parameters

cr: https://code.amazon.com/reviews/CR-161170481
  • Loading branch information
@angusy29
angusy29 committed Jan 8, 2025
1 parent d2721ee commit 9a9fc6c
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 195 deletions.
161 changes: 71 additions & 90 deletions ...ametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java
View file

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions ...rametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/CreateHandler.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -46,7 +47,7 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
.build();

final Map<String, Object> desiredParams = request.getDesiredResourceState().getParameters();
final Map<String, Parameter> currentClusterParameters = Maps.newHashMap();
final Map<String, Parameter> desiredClusterParameters = Maps.newHashMap();

return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext)
.then(progress -> setDbClusterParameterGroupNameIfMissing(request, progress))
Expand All @@ -59,8 +60,8 @@ protected ProgressEvent<ResourceModel, CallbackContext> 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));
Expand Down
2 changes: 1 addition & 1 deletion ...parametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/ReadHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private ProgressEvent<ResourceModel, CallbackContext> 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))
Expand Down
61 changes: 41 additions & 20 deletions ...rametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/UpdateHandler.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -46,30 +48,49 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
.resourceTags(Translator.translateTagsToSdk(request.getDesiredResourceState().getTags()))
.build();

final Map<String, Object> previousParams = request.getPreviousResourceState().getParameters();
final Map<String, Object> desiredParams = request.getDesiredResourceState().getParameters();
final boolean shouldUpdateParameters = !DifferenceUtils.diff(previousParams, desiredParams).isEmpty();
final Map<String, Object> previousModelParams = request.getPreviousResourceState().getParameters() == null ? Map.of() : request.getPreviousResourceState().getParameters();
final Map<String, Object> desiredModelParams = request.getDesiredResourceState().getParameters() == null ? Map.of() : request.getDesiredResourceState().getParameters();
final boolean shouldUpdateParameters = !DifferenceUtils.diff(previousModelParams, desiredModelParams).isEmpty();

final Map<String, Parameter> currentClusterParameters = Maps.newHashMap();
final Map<String, Parameter> 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<String, Parameter> 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<String> 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<String, Parameter> 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));
}
}
16 changes: 15 additions & 1 deletion ...rgroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,25 @@ public RdsClient client() {
};
}

protected List<Parameter> translateParamMapToCollection(final Map<String, Object> 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<Parameter> translateParamMapToCollection(final Map<String, Object> 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());
}
Expand Down
70 changes: 7 additions & 63 deletions ...tergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/CreateHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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());

Expand All @@ -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
Expand All @@ -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())
Expand All @@ -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));
}

/**
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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);
Expand All @@ -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())
Expand All @@ -550,7 +495,6 @@ public void handleRequest_SuccessWithEmptyParameters() {
);

verify(rdsProxy.client()).createDBClusterParameterGroup(any(CreateDbClusterParameterGroupRequest.class));
verify(rdsProxy.client()).describeDBClusters(any(DescribeDbClustersRequest.class));
}

@Test
Expand Down Expand Up @@ -582,16 +526,16 @@ public void handleRequest_InProgressFailedUnsupportedParams() {
.dbClusterParameterGroup(DB_CLUSTER_PARAMETER_GROUP)
.build());

mockDescribeDbClusterParametersResponse("static", "dynamic", true);
mockDescribeDbClusterParametersResponse("static", "dynamic", false);

final ProgressEvent<ResourceModel, CallbackContext> 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));
Expand Down
Loading

0 comments on commit 9a9fc6c

Please sign in to comment.