Correct issues discovered with the self-service integration tests

The self-service integration tests revealed that the handlers had
mistakes and needs corrections. These are all the changes necessary to
pass self-service integration tests fully when running against Uluru
resources.

Author: Alex-Vol-Amz

aws-transfer-agreement/.rpdk-config

@@ -22,5 +22,7 @@
         "protocolVersion": "2.0.0"
     },
     "logProcessorEnabled": "true",
-    "executableEntrypoint": "software.amazon.transfer.agreement.HandlerWrapperExecutable"
+    "executableEntrypoint": "software.amazon.transfer.agreement.HandlerWrapperExecutable",
+    "contractSettings": {},
+    "canarySettings": {}
 }

aws-transfer-certificate/.rpdk-config

@@ -25,5 +25,7 @@
         "protocolVersion": "2.0.0"
     },
     "logProcessorEnabled": "true",
-    "executableEntrypoint": "software.amazon.transfer.certificate.HandlerWrapperExecutable"
+    "executableEntrypoint": "software.amazon.transfer.certificate.HandlerWrapperExecutable",
+    "contractSettings": {},
+    "canarySettings": {}
 }

aws-transfer-connector/.rpdk-config

@@ -22,5 +22,7 @@
         "protocolVersion": "2.0.0"
     },
     "logProcessorEnabled": "true",
-    "executableEntrypoint": "software.amazon.transfer.connector.HandlerWrapperExecutable"
+    "executableEntrypoint": "software.amazon.transfer.connector.HandlerWrapperExecutable",
+    "contractSettings": {},
+    "canarySettings": {}
 }

aws-transfer-profile/.rpdk-config

@@ -22,5 +22,7 @@
         "protocolVersion": "2.0.0"
     },
     "logProcessorEnabled": "true",
-    "executableEntrypoint": "software.amazon.transfer.profile.HandlerWrapperExecutable"
+    "executableEntrypoint": "software.amazon.transfer.profile.HandlerWrapperExecutable",
+    "contractSettings": {},
+    "canarySettings": {}
 }

aws-transfer-server/.rpdk-config

@@ -26,5 +26,7 @@
         "protocolVersion": "2.0.0"
     },
     "logProcessorEnabled": "true",
-    "executableEntrypoint": "software.amazon.transfer.server.HandlerWrapperExecutable"
+    "executableEntrypoint": "software.amazon.transfer.server.HandlerWrapperExecutable",
+    "contractSettings": {},
+    "canarySettings": {}
 }

aws-transfer-server/src/main/java/software/amazon/transfer/server/BaseHandlerStd.java

@@ -310,6 +310,9 @@ protected boolean isVpcServerEndpoint(ResourceModel model) {
     }
 
     protected boolean privateIpsAvailable(List<String> allocationIds, ProxyClient<Ec2Client> ec2Client) {
+        if (allocationIds.isEmpty()) {
+            return true; // no IPs to check against, so assume available
+        }
         DescribeAddressesRequest request =
                 DescribeAddressesRequest.builder().allocationIds(allocationIds).build();
         try (Ec2Client client = ec2Client.client()) {

aws-transfer-server/src/main/java/software/amazon/transfer/server/CreateHandler.java

@@ -28,7 +28,7 @@
 import software.amazon.transfer.server.translators.WorkflowDetailsTranslator;
 
 import com.amazonaws.regions.Region;
-import com.amazonaws.regions.Regions;
+import com.amazonaws.regions.RegionUtils;
 
 public class CreateHandler extends BaseHandlerStd {
 
@@ -45,7 +45,7 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
         final String clientRequestToken = request.getClientRequestToken();
         ResourceModel newModel = request.getDesiredResourceState();
 
-        prepareDesiredResourceModel(request, newModel);
+        prepareDesiredResourceModel(request, newModel, true);
 
         return ProgressEvent.progress(newModel, callbackContext)
                 .then(progress -> proxy.initiate(
@@ -106,7 +106,7 @@ private boolean stabilizeAfterCreate(
             CallbackContext ignored2) {
 
         String serverId = awsResponse.serverId();
-        Region region = Region.getRegion(Regions.fromName(request.getRegion()));
+        Region region = RegionUtils.getRegion(request.getRegion());
         ServerArn serverArn = new ServerArn(region, request.getAwsAccountId(), serverId);
         model.setArn(serverArn.getArn());
         model.setServerId(serverId);

aws-transfer-server/src/main/java/software/amazon/transfer/server/UpdateHandler.java

@@ -2,15 +2,16 @@
 
 import static software.amazon.transfer.server.translators.ResourceModelAdapter.prepareDesiredResourceModel;
 import static software.amazon.transfer.server.translators.ResourceModelAdapter.preparePreviousResourceModel;
+import static software.amazon.transfer.server.translators.Translator.emptyListIfNull;
+import static software.amazon.transfer.server.translators.Translator.emptyStringIfNull;
 import static software.amazon.transfer.server.translators.Translator.translateToSdkProtocols;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.Set;
 
 import org.apache.commons.lang3.StringUtils;
@@ -58,7 +59,7 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
         Translator.ensureServerIdInModel(oldModel);
         Translator.ensureServerIdInModel(newModel);
 
-        prepareDesiredResourceModel(request, newModel);
+        prepareDesiredResourceModel(request, newModel, false);
         preparePreviousResourceModel(request, oldModel);
 
         return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext)
@@ -101,13 +102,21 @@ private Boolean stabilizeAfterUpdate(
         String serverId = model.getServerId();
         DescribedServer describedServer = describeServer(client, model);
 
+        // Always check if the VPC endpoint is ready to modify
+        if (EndpointType.VPC.equals(describedServer.endpointType())) {
+            String vpcEndpointId = describedServer.endpointDetails().vpcEndpointId();
+            if (!waitForVpcEndpoint(vpcEndpointId, ec2Client, model)) {
+                return false;
+            }
+        }
+
         List<String> proposedSubnetIds;
         List<String> proposedAddressAllocationIds;
 
         if (model.getEndpointDetails() != null) {
-            proposedSubnetIds = nullableList(model.getEndpointDetails().getSubnetIds());
+            proposedSubnetIds = emptyListIfNull(model.getEndpointDetails().getSubnetIds());
             proposedAddressAllocationIds =
-                    nullableList(model.getEndpointDetails().getAddressAllocationIds());
+                    emptyListIfNull(model.getEndpointDetails().getAddressAllocationIds());
         } else {
             proposedSubnetIds = Collections.emptyList();
             proposedAddressAllocationIds = Collections.emptyList();
@@ -124,15 +133,23 @@ private Boolean stabilizeAfterUpdate(
             currentAddressAllocationIds = Collections.emptyList();
         }
 
+        log(String.format("Endpoint current subnet IDs: %s", currentSubnetIds), serverId);
+        log(String.format("Endpoint proposed subnet IDs: %s", proposedSubnetIds), serverId);
+        log(String.format("Endpoint current address allocation IDs: %s", currentAddressAllocationIds), serverId);
+        log(String.format("Endpoint proposed address allocation IDs: %s", proposedAddressAllocationIds), serverId);
+
         State state = describedServer.state();
         switch (state) {
             case OFFLINE:
                 if (!Objects.equals(proposedSubnetIds, currentSubnetIds)) {
-                    EndpointDetails removeAddressAllocationIds = EndpointDetails.builder()
-                            .addressAllocationIds(Collections.emptyList())
-                            .build();
-                    updateServerEndpointDetails(client, serverId, removeAddressAllocationIds);
-
+                    if (!currentAddressAllocationIds.isEmpty()) {
+                        EndpointDetails removeAddressAllocationIds = EndpointDetails.builder()
+                                .addressAllocationIds(Collections.emptyList())
+                                .build();
+                        log("EIP address allocation IDs are removed for subnet update.", serverId);
+                        updateServerEndpointDetails(client, serverId, removeAddressAllocationIds);
+                        return false;
+                    }
                     EndpointDetails updateSubnets = EndpointDetails.builder()
                             .subnetIds(proposedSubnetIds)
                             .build();
@@ -151,8 +168,7 @@ private Boolean stabilizeAfterUpdate(
                     return false;
                 }
 
-                if (!currentAddressAllocationIds.isEmpty()
-                        && !privateIpsAvailable(currentAddressAllocationIds, ec2Client)) {
+                if (!privateIpsAvailable(currentAddressAllocationIds, ec2Client)) {
                     log("is waiting for endpoint private IPs", serverId);
                     return false;
                 }
@@ -161,14 +177,8 @@ private Boolean stabilizeAfterUpdate(
                 log("is going ONLINE after update.", serverId);
                 return false;
             case ONLINE:
-                if (EndpointType.VPC.equals(describedServer.endpointType())) {
-                    String vpcEndpointId = describedServer.endpointDetails().vpcEndpointId();
-                    if (!waitForVpcEndpoint(vpcEndpointId, ec2Client, model)) {
-                        return false;
-                    }
-                }
-
-                if (Objects.equals(proposedAddressAllocationIds, currentAddressAllocationIds)) {
+                if (Objects.equals(proposedSubnetIds, currentSubnetIds)
+                        && Objects.equals(proposedAddressAllocationIds, currentAddressAllocationIds)) {
                     log("update has been stabilized.", serverId);
                     return true; // no update needed, we are done
                 }
@@ -182,10 +192,6 @@ private Boolean stabilizeAfterUpdate(
         }
     }
 
-    private List<String> nullableList(List<String> subnetIds) {
-        return Optional.ofNullable(subnetIds).orElse(Collections.emptyList());
-    }
-
     private ProgressEvent<ResourceModel, CallbackContext> updateSecurityGroups(
             ProgressEvent<ResourceModel, CallbackContext> progress,
             ResourceModel oldModel,
@@ -201,52 +207,57 @@ private ProgressEvent<ResourceModel, CallbackContext> updateSecurityGroups(
             return progress; // skip this step
         }
 
-        Set<String> sgIdSet = new HashSet<>();
-        Set<String> prevSgIdSet = new HashSet<>();
+        List<String> requested = List.of();
+        List<String> previous = List.of();
         if (isVpcServerEndpoint(oldModel)) {
-            prevSgIdSet = new HashSet<>(
-                    Optional.ofNullable(oldModel.getEndpointDetails().getSecurityGroupIds())
-                            .orElse(Collections.emptyList()));
+            previous = emptyListIfNull(oldModel.getEndpointDetails().getSecurityGroupIds());
         }
         if (isVpcServerEndpoint(newModel)) {
-            sgIdSet = new HashSet<>(
-                    Optional.ofNullable(newModel.getEndpointDetails().getSecurityGroupIds())
-                            .orElse(Collections.emptyList()));
-            ;
+            requested = emptyListIfNull(newModel.getEndpointDetails().getSecurityGroupIds());
         }
 
-        Set<String> sgIdsToAdd = new HashSet<>(sgIdSet);
-        sgIdsToAdd.removeAll(prevSgIdSet);
-        Set<String> sgIdsToRemove = new HashSet<>(prevSgIdSet);
-        sgIdsToRemove.removeAll(sgIdSet);
+        List<String> toAdd = new ArrayList<>(requested);
+        toAdd.removeAll(previous);
+        List<String> toRemove = new ArrayList<>(previous);
+        toRemove.removeAll(requested);
 
-        if (sgIdsToAdd.isEmpty() && sgIdsToRemove.isEmpty()) {
+        String serverId = newModel.getServerId();
+        if (toAdd.isEmpty() && toRemove.isEmpty()) {
+            log("VPC endpoint has no modifications for security groups", serverId);
             return progress; // skip this step
         }
 
+        if (!toAdd.isEmpty()) {
+            log(String.format("security group IDs to add: %s", toAdd), serverId);
+        }
+        if (!toRemove.isEmpty()) {
+            log(String.format("security group IDs to remove: %s", toRemove), serverId);
+        }
+
         return proxy.initiate(
                         "AWS-Transfer-Server::Update::updateSecurityGroups",
                         proxyEc2Client,
                         progress.getResourceModel(),
                         progress.getCallbackContext())
-                .translateToServiceRequest(m -> modifyVpcEndpointRequest(vpcEndpointId, sgIdsToAdd, sgIdsToRemove))
-                .makeServiceCall((awsRequest, client) -> {
-                    try (Ec2Client ec2Client = client.client()) {
-                        ModifyVpcEndpointResponse awsResponse =
-                                client.injectCredentialsAndInvokeV2(awsRequest, ec2Client::modifyVpcEndpoint);
-                        log(
-                                "VPC Endpoint has successfully been updated.",
-                                progress.getResourceModel().getServerId());
-                        return awsResponse;
-                    }
-                })
+                .translateToServiceRequest(m -> modifyVpcEndpointRequest(vpcEndpointId, toAdd, toRemove))
+                .makeServiceCall((awsRequest, client) -> modifyVpcEndpoint(serverId, awsRequest, client))
                 .stabilize((awsRequest, awsResponse, client, model, context) ->
                         waitForVpcEndpoint(awsRequest.vpcEndpointId(), client, model))
                 .handleError((ignored, exception, proxyClient1, model1, callbackContext1) ->
                         handleError(UPDATE, exception, model1, callbackContext1, clientRequestToken))
                 .progress();
     }
 
+    private ModifyVpcEndpointResponse modifyVpcEndpoint(
+            String serverId, ModifyVpcEndpointRequest awsRequest, ProxyClient<Ec2Client> client) {
+        try (Ec2Client ec2Client = client.client()) {
+            ModifyVpcEndpointResponse awsResponse =
+                    client.injectCredentialsAndInvokeV2(awsRequest, ec2Client::modifyVpcEndpoint);
+            log("VPC Endpoint has been updated successfully.", serverId);
+            return awsResponse;
+        }
+    }
+
     private Boolean waitForVpcEndpoint(String vpcEndpointId, ProxyClient<Ec2Client> client, ResourceModel model) {
         if (!isVpcEndpointAvailable(vpcEndpointId, client)) {
             log("VPC Endpoint is not available yet.", model.getServerId());
@@ -298,14 +309,14 @@ private UpdateServerRequest translateToFirstUpdateRequest(final ResourceModel ol
                 .endpointType(endpointType)
                 .endpointDetails(endpointDetails)
                 .identityProviderDetails(identityProviderDetails)
-                .loggingRole(newModel.getLoggingRole())
-                .preAuthenticationLoginBanner(newModel.getPreAuthenticationLoginBanner())
-                .postAuthenticationLoginBanner(newModel.getPostAuthenticationLoginBanner())
+                .loggingRole(emptyStringIfNull(newModel.getLoggingRole()))
+                .preAuthenticationLoginBanner(emptyStringIfNull(newModel.getPreAuthenticationLoginBanner()))
+                .postAuthenticationLoginBanner(emptyStringIfNull(newModel.getPostAuthenticationLoginBanner()))
                 .protocols(translateToSdkProtocols(newModel.getProtocols()))
                 .protocolDetails(ProtocolDetailsTranslator.toSdk(newModel.getProtocolDetails()))
                 .securityPolicyName(newModel.getSecurityPolicyName())
                 .serverId(newModel.getServerId())
-                .structuredLogDestinations(newModel.getStructuredLogDestinations())
+                .structuredLogDestinations(emptyListIfNull(newModel.getStructuredLogDestinations()))
                 .workflowDetails(WorkflowDetailsTranslator.toSdk(newModel.getWorkflowDetails(), true))
                 .s3StorageOptions(S3StorageOptionsTranslator.toSdk(newModel.getS3StorageOptions()))
                 .build();
@@ -335,12 +346,15 @@ private UpdateServerResponse updateServer(UpdateServerRequest awsRequest, ProxyC
     }
 
     private ModifyVpcEndpointRequest modifyVpcEndpointRequest(
-            String vpcEndpointId, Set<String> sgIdsToAdd, Set<String> sgIdsToRemove) {
-        return ModifyVpcEndpointRequest.builder()
-                .vpcEndpointId(vpcEndpointId)
-                .addSecurityGroupIds(sgIdsToAdd)
-                .removeSecurityGroupIds(sgIdsToRemove)
-                .build();
+            String vpcEndpointId, Collection<String> sgIdsToAdd, Collection<String> sgIdsToRemove) {
+        var builder = ModifyVpcEndpointRequest.builder().vpcEndpointId(vpcEndpointId);
+        if (!sgIdsToAdd.isEmpty()) {
+            builder.addSecurityGroupIds(sgIdsToAdd);
+        }
+        if (!sgIdsToRemove.isEmpty()) {
+            builder.removeSecurityGroupIds(sgIdsToRemove);
+        }
+        return builder.build();
     }
 
     private ProgressEvent<ResourceModel, CallbackContext> addTags(

aws-transfer-server/src/main/java/software/amazon/transfer/server/translators/EndpointDetailsTranslator.java

@@ -1,8 +1,8 @@
 package software.amazon.transfer.server.translators;
 
+import static software.amazon.transfer.server.translators.Translator.nullIfEmptyList;
+
 import java.util.Collection;
-import java.util.Collections;
-import java.util.Optional;
 
 import software.amazon.transfer.server.EndpointDetails;
 
@@ -16,11 +16,9 @@ public static EndpointDetails fromSdk(
             return null;
         }
         return EndpointDetails.builder()
-                .addressAllocationIds(Optional.ofNullable(endpointDetails.addressAllocationIds())
-                        .orElse(Collections.emptyList()))
-                .subnetIds(Optional.ofNullable(endpointDetails.subnetIds()).orElse(Collections.emptyList()))
-                .securityGroupIds(
-                        Optional.ofNullable(endpointDetails.securityGroupIds()).orElse(Collections.emptyList()))
+                .addressAllocationIds(nullIfEmptyList(endpointDetails.addressAllocationIds()))
+                .subnetIds(nullIfEmptyList(endpointDetails.subnetIds()))
+                .securityGroupIds(nullIfEmptyList(endpointDetails.securityGroupIds()))
                 .vpcId(endpointDetails.vpcId())
                 .vpcEndpointId(endpointDetails.vpcEndpointId())
                 .build();

aws-transfer-server/src/main/java/software/amazon/transfer/server/translators/ResourceModelAdapter.java

@@ -31,14 +31,13 @@ private ResourceModelAdapter() {}
     public static final String DEFAULT_SECURITY_POLICY = "TransferSecurityPolicy-2018-11";
 
     public static void prepareDesiredResourceModel(
-            ResourceHandlerRequest<ResourceModel> request, ResourceModel resourceModel) {
-        setDefaults(resourceModel);
+            ResourceHandlerRequest<ResourceModel> request, ResourceModel resourceModel, boolean create) {
+        setDefaults(resourceModel, create);
         setDesiredTags(request, resourceModel);
     }
 
     public static void preparePreviousResourceModel(
             ResourceHandlerRequest<ResourceModel> request, ResourceModel resourceModel) {
-        setDefaults(resourceModel);
         setPreviousTags(request, resourceModel);
     }
 
@@ -57,7 +56,7 @@ private static void setPreviousTags(
         previousResourceModel.setTags(Translator.translateTagMapToTagList(tagsMap));
     }
 
-    private static void setDefaults(ResourceModel resourceModel) {
+    private static void setDefaults(ResourceModel resourceModel, boolean create) {
         if (resourceModel.getEndpointType() == null) {
             resourceModel.setEndpointType(DEFAULT_ENDPOINT_TYPE);
         }
@@ -67,8 +66,11 @@ private static void setDefaults(ResourceModel resourceModel) {
         if (resourceModel.getProtocols() == null) {
             resourceModel.setProtocols(DEFAULT_PROTOCOLS);
         }
-        if (resourceModel.getSecurityPolicyName() == null) {
-            resourceModel.setSecurityPolicyName(DEFAULT_SECURITY_POLICY);
+        // Handle update differently because of https://i.amazon.com/XFER-10446
+        if (create) {
+            if (resourceModel.getSecurityPolicyName() == null) {
+                resourceModel.setSecurityPolicyName(DEFAULT_SECURITY_POLICY);
+            }
         }
     }
 }