fix: make deployment service robust (#1010)

aws-greengrass · Jun 29, 2021 · 1fd760b · 1fd760b
1 parent be848f7
commit 1fd760b
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 15 deletions.
diff --git a/src/main/java/com/aws/greengrass/deployment/DeploymentDocumentDownloader.java b/src/main/java/com/aws/greengrass/deployment/DeploymentDocumentDownloader.java
@@ -19,7 +19,6 @@
 import com.aws.greengrass.util.RetryUtils;
 import com.aws.greengrass.util.SerializerFactory;
 import com.aws.greengrass.util.Utils;
-import com.fasterxml.jackson.core.JsonProcessingException;
 import org.apache.commons.lang3.StringUtils;
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
 import software.amazon.awssdk.core.exception.SdkClientException;
@@ -99,9 +98,7 @@ public DeploymentDocument download(String deploymentId)
             throw new DeploymentTaskFailureException("Error downloading deployment configuration", e);
         }
         // 3. deserialize and convert to device model
-        Configuration configuration = deserializeDeploymentDoc(configurationString);
-        return DeploymentDocumentConverter.convertFromDeploymentConfiguration(configuration);
-
+        return deserializeDeploymentDoc(configurationString);
     }
 
     protected String downloadDeploymentDocument(String deploymentId) throws DeploymentTaskFailureException,
@@ -202,12 +199,14 @@ private void validateHttpExecuteResponse(HttpExecuteResponse executeResponse)
         }
     }
 
-    private Configuration deserializeDeploymentDoc(String configurationInString)
+    @SuppressWarnings("PMD.AvoidCatchingGenericException")
+    private DeploymentDocument deserializeDeploymentDoc(String configurationInString)
             throws DeploymentTaskFailureException {
         try {
-            return SerializerFactory.getFailSafeJsonObjectMapper()
+            Configuration configuration =  SerializerFactory.getFailSafeJsonObjectMapper()
                     .readValue(configurationInString, Configuration.class);
-        } catch (JsonProcessingException e) {
+            return DeploymentDocumentConverter.convertFromDeploymentConfiguration(configuration);
+        } catch (Exception e) {
             throw new DeploymentTaskFailureException("Failed to deserialize deployment document.", e);
         }
 

diff --git a/src/main/java/com/aws/greengrass/deployment/DeploymentService.java b/src/main/java/com/aws/greengrass/deployment/DeploymentService.java
@@ -608,12 +608,15 @@ private KernelUpdateDeploymentTask createKernelUpdateDeployment(Deployment deplo
         return new KernelUpdateDeploymentTask(kernel, logger.createChild(), deployment, componentManager);
     }
 
+    @SuppressWarnings("PMD.AvoidCatchingGenericException")
+    //Catching generic exception here to make sure any exception while parsing deployment document will not cause
+    //deployment service to move to errored state.
     private DefaultDeploymentTask createDefaultNewDeployment(Deployment deployment) {
         try {
             logger.atInfo().kv("document", deployment.getDeploymentDocument())
                     .log("Received deployment document in queue");
             parseAndValidateJobDocument(deployment);
-        } catch (InvalidRequestException e) {
+        } catch (Exception e) {
             logger.atError().cause(e).kv(DEPLOYMENT_ID_LOG_KEY_NAME, deployment.getId())
                     .kv("DeploymentType", deployment.getDeploymentType().toString())
                     .log("Invalid document for deployment");

diff --git a/src/main/java/com/aws/greengrass/deployment/converter/DeploymentDocumentConverter.java b/src/main/java/com/aws/greengrass/deployment/converter/DeploymentDocumentConverter.java
@@ -13,6 +13,7 @@
 import com.amazon.aws.iot.greengrass.configuration.common.ComponentUpdate;
 import com.amazon.aws.iot.greengrass.configuration.common.Configuration;
 import com.amazon.aws.iot.greengrass.configuration.common.ConfigurationUpdate;
+import com.aws.greengrass.deployment.exceptions.InvalidRequestException;
 import com.aws.greengrass.deployment.model.ComponentUpdatePolicy;
 import com.aws.greengrass.deployment.model.ConfigurationUpdateOperation;
 import com.aws.greengrass.deployment.model.DeploymentDocument;
@@ -23,6 +24,7 @@
 import com.aws.greengrass.logging.api.Logger;
 import com.aws.greengrass.logging.impl.LogManager;
 import com.aws.greengrass.util.SerializerFactory;
+import com.aws.greengrass.util.Utils;
 import org.apache.commons.lang3.StringUtils;
 import software.amazon.awssdk.arns.Arn;
 import software.amazon.awssdk.services.greengrassv2.model.DeploymentComponentUpdatePolicyAction;
@@ -33,7 +35,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -139,8 +140,10 @@ private static List<DeploymentPackageConfiguration> buildDeploymentPackageConfig
      * @param config Fleet configuration that is generated by CreateDeployment and gets sent down via IoT Job and
      *               shadow
      * @return Nucleus's core {@link DeploymentDocument}
+     * @throws InvalidRequestException if failed to parsing deployment document from configuration.
      */
-    public static DeploymentDocument convertFromDeploymentConfiguration(Configuration config) {
+    public static DeploymentDocument convertFromDeploymentConfiguration(Configuration config)
+            throws InvalidRequestException {
 
         DeploymentDocument.DeploymentDocumentBuilder builder =
                 DeploymentDocument.builder().configurationArn(config.getConfigurationArn())
@@ -195,17 +198,27 @@ private static String parseGroupNameFromConfigurationArn(Configuration config) {
     }
 
     private static List<DeploymentPackageConfiguration> convertComponents(
-            @Nullable Map<String, ComponentUpdate> components) {
+            @Nullable Map<String, ComponentUpdate> components) throws InvalidRequestException {
         if (components == null || components.isEmpty()) {
             return Collections.emptyList();
         }
-
-        return components.entrySet().stream().map(e -> convertComponent(e.getKey(), e.getValue()))
-                .collect(Collectors.toList());
+        List<DeploymentPackageConfiguration> deploymentPackageConfiguration = new ArrayList<>();
+        for (Map.Entry<String, ComponentUpdate> e : components.entrySet()) {
+            deploymentPackageConfiguration.add(convertComponent(e.getKey(), e.getValue()));
+        }
+        return deploymentPackageConfiguration;
     }
 
     private static DeploymentPackageConfiguration convertComponent(String componentName,
-            ComponentUpdate componentUpdate) {
+            ComponentUpdate componentUpdate) throws InvalidRequestException {
+
+        if (Utils.isEmpty(componentName)) {
+            throw new InvalidRequestException("Target component name is empty");
+        }
+
+        if (componentUpdate == null || componentUpdate.getVersion() == null) {
+            throw new InvalidRequestException("Version for target component " + componentName + " is empty");
+        }
 
         DeploymentPackageConfiguration.DeploymentPackageConfigurationBuilder builder =
                 DeploymentPackageConfiguration.builder().packageName(componentName)