Skip to content

Commit

Permalink
fix: proper error handling for bad component versions (#1620)
Browse files Browse the repository at this point in the history
  • Loading branch information
junfuchen99 authored Jul 18, 2024
1 parent f225de5 commit aa13b92
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import java.util.stream.Collectors;
import javax.inject.Inject;

import static com.aws.greengrass.deployment.errorcode.DeploymentErrorCode.COMPONENT_VERSION_NOT_VALID;

@NoArgsConstructor
public class DependencyResolver {
static final String NON_EXPLICIT_NUCLEUS_UPDATE_ERROR_MESSAGE_FMT = "The deployment attempts to update the "
Expand Down Expand Up @@ -71,6 +73,7 @@ public class DependencyResolver {
* @throws PackagingException for other component operation errors
* @throws InterruptedException InterruptedException
*/
@SuppressWarnings("PMD.AvoidCatchingGenericException")
public List<ComponentIdentifier> resolveDependencies(DeploymentDocument document,
Map<String, Set<ComponentRequirementIdentifier>>
otherGroupsToRootComponents)
Expand All @@ -83,26 +86,36 @@ public List<ComponentIdentifier> resolveDependencies(DeploymentDocument document
// dependency tree.
Map<String, Map<String, Requirement>> componentNameToVersionConstraints = new HashMap<>();

// A map of component name to count of components that depend on it
Map<String, Integer> componentIncomingReferenceCount = new HashMap<>();

// A map of component name to its resolved version.
Map<String, ComponentMetadata> resolvedComponents = new HashMap<>();

// Get the target components with version requirements in the deployment document
List<String> targetComponentsToResolve = new ArrayList<>();
// Add the constraints of the target group to componentNameToVersionConstraints before
// trying to resolve the other group so that the resolved version will be compatible with
// both versions (if possible)
document.getDeploymentPackageConfigurationList().stream()
.filter(DeploymentPackageConfiguration::isRootComponent).forEach(e -> {
logger.atDebug().kv(COMPONENT_NAME_KEY, e.getPackageName()).kv(VERSION_KEY, e.getResolvedVersion())
.log("Found component configuration");
componentNameToVersionConstraints.putIfAbsent(e.getPackageName(), new HashMap<>());
for (DeploymentPackageConfiguration e : document.getDeploymentPackageConfigurationList()) {
if (e.isRootComponent()) {
logger.atDebug().kv(COMPONENT_NAME_KEY, e.getPackageName()).kv(VERSION_KEY, e.getResolvedVersion())
.log("Found component configuration");
componentNameToVersionConstraints.putIfAbsent(e.getPackageName(), new HashMap<>());
try {
componentNameToVersionConstraints.get(e.getPackageName())
.put(document.getGroupName(), Requirement.buildNPM(e.getResolvedVersion()));
targetComponentsToResolve.add(e.getPackageName());
});
} catch (Exception exception) {
throw new PackagingException(
String.format("Unsupported component version '%s' for component '%s'. For pre-release "
+ "versions, please start the version tag with a non-numeric character",
e.getResolvedVersion(),
e.getPackageName()),
exception, COMPONENT_VERSION_NOT_VALID);
}
targetComponentsToResolve.add(e.getPackageName());
}
}

// A map of component name to count of components that depend on it
Map<String, Integer> componentIncomingReferenceCount = new HashMap<>();

// A map of component name to its resolved version.
Map<String, ComponentMetadata> resolvedComponents = new HashMap<>();

Set<String> combinedTargetComponents = new LinkedHashSet<>(targetComponentsToResolve);
combinedTargetComponents
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public enum DeploymentErrorCode {
ARTIFACT_CHECKSUM_MISMATCH(DeploymentErrorType.COMPONENT_RECIPE_ERROR),
COMPONENT_DEPENDENCY_NOT_VALID(DeploymentErrorType.COMPONENT_RECIPE_ERROR),
CONFIG_INTERPOLATE_ERROR(DeploymentErrorType.COMPONENT_RECIPE_ERROR),
COMPONENT_VERSION_NOT_VALID(DeploymentErrorType.COMPONENT_RECIPE_ERROR),

/* Config issues */
DEVICE_CONFIG_NOT_VALID_FOR_ARTIFACT_DOWNLOAD(DeploymentErrorType.DEVICE_ERROR),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import static software.amazon.awssdk.services.greengrassv2.model.DeploymentComponentUpdatePolicyAction.NOTIFY_COMPONENTS;

@ExtendWith({GGExtension.class, MockitoExtension.class})
@SuppressWarnings("PMD.ExcessiveClassLength")
class DependencyResolverTest {

private static final Semver v1_5_0 = new Semver("1.5.0");
Expand Down Expand Up @@ -1053,4 +1054,19 @@ void GIVEN_resolved_components_contains_nucleus_WHEN_active_nucleus_version_conf
Arrays.asList(componentA, componentB, customNucleus)));
assertEquals(NO_ACTIVE_NUCLEUS_VERSION_ERROR_MSG, e.getMessage());
}

@Test
void GIVEN_invalid_component_version_WHEN_resolve_dependencies_THEN_throw() {
DeploymentDocument doc = new DeploymentDocument("mockId","mockJob1", Collections
.singletonList(
new DeploymentPackageConfiguration(componentA, true, "0.0.0-not-valid")),
Collections.emptyList(),
"mockGroup1", "mockGroup1", "mockGroup1", 1L, FailureHandlingPolicy.DO_NOTHING, componentUpdatePolicy, configurationValidationPolicy);

context.runOnPublishQueueAndWait(() -> System.out.println("Waiting for queue to finish updating the config"));

Exception e = assertThrows(PackagingException.class,
() -> dependencyResolver.resolveDependencies(doc, new HashMap<>()));
assertTrue(e.getMessage().contains("Unsupported component version '0.0.0-not-valid' for component 'A'"));
}
}

0 comments on commit aa13b92

Please sign in to comment.