Java V2 Add Inspector examples #7660
Conversation
scmacdon
added
Java-v1
github-actions
bot
added
Java-v2
scmacdon
changed the title
debora-ito
left a comment
debora-ito
left a comment
There was a problem hiding this comment.
Just left one comment, looks good otherwise.
| ## Scenario | ||
|
|
||
| The Hello example is a separate runnable example. - Set up the Inspector service client - Check the current account status for Inspector - Display available scan types and regions | ||
| This scenario demonstrates the basic usage of **Amazon Inspector** using a Java program. It focuses on checking account status, enabling Inspector, listing findings, reviewing coverage, and managing filters. |
There was a problem hiding this comment.
The specification shouldn't mention a particular SDK, it should be generic for all SDKs.
| * Display final account status | ||
| ### Notes | ||
|
|
||
| * The program **does not retrieve detailed vulnerability (CVE) information**. |
There was a problem hiding this comment.
What do these notes mean? Why are we specifying features that are not in the example?
| |`ListCoverage` |ValidationException |Validate filter and pagination parameters. | | ||
| |`Disable` |ValidationException |Validate resource types for disabling. | | ||
| |`Disable` |ConflictException |Handle cases where Inspector cannot be disabled. | | ||
| The Java SDK examples include basic exception handling for each Inspector action. The table below describes the exceptions actually handled in the program for each action. |
|
|
||
| | Action | Exception | Handling | | ||
| |-------------------------------|---------------------------|--------------------------------------------------------------------------| | ||
| | `Enable` | `ValidationException` | Prints a message indicating Inspector may already be enabled. | |
There was a problem hiding this comment.
The table should only include one specific exception for each action, not the more generic Inspector2Exception (which also looks like a Java class).
| | `ListCoverageStatistics` | `Exception` | Wraps and throws a `RuntimeException` for unexpected errors. | | ||
| | `ListUsageTotals` | `ValidationException` | Prints validation error details. | | ||
| | `ListUsageTotals` | `Inspector2Exception` | Prints AWS service error details and rethrows the exception. | | ||
| | `ListUsageTotals` | `Exception` | Wraps and throws a `RuntimeException` for unexpected errors. | |
There was a problem hiding this comment.
Is this action even used? I don't see it in the metadata.
There was a problem hiding this comment.
Both operations are in metadata - ListCoverageStatistics and ListUsageTotals
| System.out.println("Step 2: Ensuring Inspector is enabled..."); | ||
|
|
||
| try { | ||
| inspectorActions.enableInspector(inspectorClient, null); |
There was a problem hiding this comment.
There is a try and a catch in each of the inspectorAction methods, so why do we need an additional try/catch here? Shouldn't there be one try/catch around the scenario with a cleanup in the catch? This applies to all the steps in the scenario.
| inspectorActions.getAccountStatus(inspectorClient); | ||
|
|
||
| } catch (Exception e) { | ||
| System.err.println(" Could not create example filter: " + e.getMessage()); |
There was a problem hiding this comment.
These error messages don't match the actions called? Seems to be the same error message (copy/paste?) for all the actions. But I'm not sure they are needed anyway - see my other comment about the try/catches.
| * @throws Exception If an unexpected error occurs during the filter creation process. | ||
| * | ||
| */ | ||
| public void createFilter(Inspector2Client inspector2Client, String description) { |
There was a problem hiding this comment.
If this is creating a low severity filter, the name of the method should reflect that. Right now the name indicates you can create any filter.
| System.out.println(" • Generate detailed findings reports"); | ||
| System.out.println(" • Integrate with AWS Security Hub"); | ||
| waitForInputToContinue(scanner); | ||
|
|
There was a problem hiding this comment.
Needs a cleanup method that removes the example filters and gives the option to disable inspector.
| public void testScenario() { | ||
| assertDoesNotThrow(() -> { | ||
| int maxResults = 10; | ||
| inspectorActions.getAccountStatus(inspector); |
There was a problem hiding this comment.
This test doesn't run the scenario, it runs the actions only. Needs a true integration test that actually runs the scenario.
| Collections.synchronizedList(new ArrayList<>()); | ||
|
|
||
| CompletableFuture<String> future = new CompletableFuture<>(); | ||
| paginator.subscribe(new Subscriber<ListFindingsResponse>() { |
There was a problem hiding this comment.
The pagination in the latest updates looks way more complicated than necessary. It should follow the simplest pagination pattern, without page limits or overrides, like this:
`
List<JobSummary> jobSummaries = new ArrayList<>();
ListJobsPublisher listJobsPaginator = getAsyncClient().listJobsPaginator(listJobsRequest);
CompletableFuture<Void> future = listJobsPaginator.subscribe(response -> {
jobSummaries.addAll(response.jobSummaryList());
});
future.join();
return jobSummaries;
`
3 participants
This pull request adds Javas V2 Inspector examples
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.