Add trustify-cli as a subfolder with preserved history#2237
Conversation
Reviewer's GuideAdds a new etc/trustify-cli workspace crate that implements a containerized CLI for Trustify with OAuth2-based auth, a reusable API client (with retries and token refresh), and SBOM management commands including listing, fetching, deletion, and duplicate detection/cleanup workflows, while wiring it into the Cargo workspace and documenting its usage. Sequence diagram for sbom duplicates find workflowsequenceDiagram
actor User
participant TrustifyCli as trustify_binary
participant Commands
participant SbomCommands
participant DuplicatesCommands
participant SbomApi as sbom_api
participant ApiClient
participant TrustifyApi as Trustify_API
User->>TrustifyCli: invoke "trustify sbom duplicates find"
TrustifyCli->>Commands: parse CLI and dispatch
Commands->>SbomCommands: run(ctx)
SbomCommands->>DuplicatesCommands: run(ctx)
DuplicatesCommands->>SbomApi: find_duplicates(client, params, output_file)
SbomApi->>ApiClient: list(SBOM_PATH, ListParams{limit=1,offset=0})
ApiClient->>TrustifyApi: HTTP GET /api/v2/sbom?limit=1&offset=0
TrustifyApi-->>ApiClient: 200 OK {total,items}
ApiClient-->>SbomApi: JSON body
loop for each worker page
SbomApi->>ApiClient: get_with_query(SBOM_PATH, ListParams{limit=batch_size,offset})
ApiClient->>TrustifyApi: HTTP GET /api/v2/sbom?limit=batch_size&offset=offset
TrustifyApi-->>ApiClient: 200 OK page of SBOMs
ApiClient-->>SbomApi: JSON page
SbomApi->>SbomApi: collect SbomEntry and group by document_id
end
SbomApi->>SbomApi: build DuplicateGroup list
SbomApi->>SbomApi: write duplicate_groups to output JSON file
SbomApi-->>DuplicatesCommands: Vec<DuplicateGroup>
DuplicatesCommands-->>SbomCommands: result summary
SbomCommands-->>Commands: print count and output path
Commands-->>TrustifyCli: ExitCode::SUCCESS
TrustifyCli-->>User: summary message
Sequence diagram for API client retry and token refreshsequenceDiagram
actor User
participant TrustifyCli as trustify_binary
participant ApiClient
participant TrustifyApi as Trustify_API
participant SsoServer as SSO
User->>TrustifyCli: run command requiring API call
TrustifyCli->>ApiClient: get(path)
ApiClient->>ApiClient: execute_with_retry(f)
loop first attempt
ApiClient->>TrustifyApi: HTTP GET /api/v2/sbom
TrustifyApi-->>ApiClient: 401 Unauthorized
ApiClient->>ApiClient: handle_response -> TokenExpired
end
ApiClient->>ApiClient: refresh_token()
ApiClient->>SsoServer: POST /protocol/openid-connect/token
SsoServer-->>ApiClient: 200 OK {access_token}
ApiClient->>ApiClient: store new token
ApiClient->>TrustifyApi: HTTP GET /api/v2/sbom (with new token)
TrustifyApi-->>ApiClient: 200 OK
ApiClient-->>TrustifyCli: JSON body
TrustifyCli-->>User: command output
Class diagram for new trustify-cli core typesclassDiagram
class Cli {
+Config config
+Commands command
}
class Config {
+String url
+Option~String~ sso_url
+Option~String~ client_id
+Option~String~ client_secret
+bool has_auth()
+Option~(str,str,str)~ auth_credentials()
}
class Context {
+Config config
+ApiClient client
}
class Commands {
<<enum>>
Sbom
Auth
+run(ctx: Context) Result~ExitCode~
}
class SbomCommands {
<<enum>>
Get
List
Delete
Duplicates
+run(ctx: Context) Result~ExitCode~
}
class DuplicatesCommands {
<<enum>>
Find
Delete
+run(ctx: Context) Result~ExitCode~
}
class AuthCommands {
<<enum>>
Token
+run(ctx: Context) Result~ExitCode~
}
class ApiClient {
-Client client
-String base_url
-Arc~RwLock~Option~String~~ token
-Option~AuthCredentials~ auth_credentials
+new(base_url: str, token: Option~String~, auth_credentials: Option~AuthCredentials~) ApiClient
+url(path: str) String
+get(path: str) Result~String,ApiError~
+get_with_query(path: str, query: T) Result~String,ApiError~
+delete(path: str) Result~String,ApiError~
+authorize(request: RequestBuilder) RequestBuilder
+refresh_token() Result~(),ApiError~
+execute_with_retry(f: F) Result~String,ApiError~
+handle_response(response: Response) Result~String,ApiError~
}
class ApiError {
<<enum>>
NetworkError
HttpError
NotFound
Unauthorized
TokenExpired
Timeout
ServerError
InternalError
TemplateError
}
class AuthCredentials {
+String token_url
+String client_id
+String client_secret
+new(sso_url: str, client_id: str, client_secret: str) AuthCredentials
+get_token() Result~String,AuthError~
}
class AuthError {
<<enum>>
ConnectionError
AuthenticationFailed
ServerError
}
class ListParams {
+Option~String~ q
+Option~u32~ limit
+Option~u32~ offset
+Option~String~ sort
}
class FindDuplicatesParams {
+u32 batch_size
+usize concurrency
}
class DuplicateGroup {
+String document_id
+Option~String~ published
+String id
+Vec~String~ duplicates
}
class DeleteResult {
+u32 deleted
+u32 skipped
+u32 failed
+u32 total
}
class DeleteEntry {
+String id
+String document_id
}
class ListFormat {
<<enum>>
Id
Name
Short
Full
}
Cli --> Config
Cli --> Commands
Context --> Config
Context --> ApiClient
Commands --> SbomCommands
Commands --> AuthCommands
SbomCommands --> DuplicatesCommands
SbomCommands --> ListFormat
ApiClient --> ApiError
ApiClient --> AuthCredentials
AuthCredentials --> AuthError
ListParams ..> ApiClient
FindDuplicatesParams ..> ApiClient
DuplicateGroup ..> FindDuplicatesParams
DeleteResult ..> DeleteEntry
DeleteEntry ..> DuplicateGroup
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
e0e8819 to
d0a3feb
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2237 +/- ##
==========================================
- Coverage 70.44% 68.30% -2.15%
==========================================
Files 414 422 +8
Lines 23917 24676 +759
Branches 23917 24676 +759
==========================================
+ Hits 16849 16854 +5
- Misses 6152 6900 +748
- Partials 916 922 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I don't see particular need for this, but if it's OK with @ruromero (as original PR author), I'm OK with it. I'll take a deeper look into it on Monday. One thing that stands out at the moment is the test coverage. Is there anything we can do to improve that? |
|
fine by me |
dejanb
left a comment
There was a problem hiding this comment.
I think we're close to finish this phase. Just a few comments.
Done |
9769c27 to
cc611f2
Compare
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
sbom::delete_by_queryyou only act on the first page of results (whatever fits in thelimit), so large result sets will leave many matching SBOMs undeleted; consider iterating pages until all matching items are processed or making that limitation explicit in the interface. - The
trustify-clicrate is set toedition = "2024", which is not yet stable and may not match the rest of the workspace; aligning the edition with the workspace (e.g. 2021) will avoid toolchain and build issues. - Both
find_duplicatesanddelete_listload all SBOM metadata or delete entries into memory (Vec<SbomEntry>/Vec<DeleteEntry>); if this CLI is used against very large datasets you may want to switch to a streaming/chunked approach to avoid excessive memory usage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `sbom::delete_by_query` you only act on the first page of results (whatever fits in the `limit`), so large result sets will leave many matching SBOMs undeleted; consider iterating pages until all matching items are processed or making that limitation explicit in the interface.
- The `trustify-cli` crate is set to `edition = "2024"`, which is not yet stable and may not match the rest of the workspace; aligning the edition with the workspace (e.g. 2021) will avoid toolchain and build issues.
- Both `find_duplicates` and `delete_list` load all SBOM metadata or delete entries into memory (`Vec<SbomEntry>` / `Vec<DeleteEntry>`); if this CLI is used against very large datasets you may want to switch to a streaming/chunked approach to avoid excessive memory usage.
## Individual Comments
### Comment 1
<location> `etc/trustify-cli/src/api/client.rs:21-22` </location>
<code_context>
+ #[error("HTTP {0}: {1}")]
+ HttpError(u16, String),
+
+ #[error("HTTP 404: Resource not found")]
+ NotFound(String),
+
+ #[error("HTTP 401: Please check your authentication credentials")]
</code_context>
<issue_to_address>
**suggestion:** The `NotFound` variant carries a `String` that is never surfaced in the error message.
`NotFound` is defined as `NotFound(String)` but always constructed with the same string and the error is hard-coded as `"HTTP 404: Resource not found"`, so the inner `String` is effectively unused. Consider removing the payload (i.e. `NotFound`) or incorporating it into the `#[error]` format string so any extra context is surfaced.
```suggestion
#[error("HTTP 404: Resource not found: {0}")]
NotFound(String),
```
</issue_to_address>
### Comment 2
<location> `etc/trustify-cli/src/api/client.rs:30-31` </location>
<code_context>
+ #[error("HTTP 401: Token expired")]
+ TokenExpired,
+
+ #[error("HTTP {0}: Server timeout")]
+ Timeout(u16),
+
+ #[error("HTTP {0}: {1}")]
</code_context>
<issue_to_address>
**issue:** Using code `0` for network-level timeouts leads to confusing error messages like `HTTP 0: Server timeout`.
In `impl From<reqwest::Error> for ApiError`, network timeouts (no HTTP response) are converted to `ApiError::Timeout(0)`, which then renders as `"HTTP 0: Server timeout"`. This conflates network and server timeouts and can mislead users. Either introduce a separate variant for network timeouts (e.g. `NetworkTimeout`) or special-case `0` in the display implementation to show a more accurate message like `"Network timeout"`.
</issue_to_address>
### Comment 3
<location> `etc/trustify-cli/src/commands/sbom.rs:138-127` </location>
<code_context>
+ format_list_output(&json, format)?;
+ Ok(ExitCode::SUCCESS)
+ }
+ SbomCommands::Delete {
+ id,
+ query,
+ dry_run,
+ concurrency,
+ limit,
+ } => {
+ if let Some(i) = id {
+ sbom_api::delete(&ctx.client, i).await?;
</code_context>
<issue_to_address>
**issue:** Delete command silently succeeds when neither `id` nor `query` is provided.
When both `id` and `query` are `None`, this branch is a no-op but still returns `ExitCode::SUCCESS`, which can mislead users (e.g., on a mistyped or missing flag). Please add a check that requires at least one of `id` or `query` and return an error or clear message if both are absent.
</issue_to_address>
### Comment 4
<location> `etc/trustify-cli/src/commands/sbom.rs:283-292` </location>
<code_context>
+ }
+ Ok(ExitCode::SUCCESS)
+ }
+ ListFormat::Name => {
+ let result: Vec<Value> = items
+ .iter()
+ .map(|item| {
+ serde_json::json!({
+ "id": item.get("id"),
+ "name": item.get("name"),
+ "document_id": item.get("document_id")
+ })
+ })
+ .collect();
+ println!("{}", serde_json::to_string(&result).unwrap_or_default());
+ Ok(ExitCode::SUCCESS)
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** JSON serialization failures in list formatting are silently swallowed.
Using `serde_json::to_string(&result).unwrap_or_default()` in both branches hides serialization errors by printing an empty string with no diagnostics. Please handle the `Result` explicitly (e.g., log the error and return a non-success `ExitCode`, or fall back to printing the raw `json` input) so failures are visible instead of silently ignored.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dejanb
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks @bxf12315
@ruromero Can you give it one more look as well
Note: it would be great to use conventional commits as for the rest of the project (https://www.conventionalcommits.org/en/v1.0.0/) and be a bit more descriptive, but I don't want to hold this on that
cd6b4ea to
e3a8a50
Compare
Done |
f8421af to
a776548
Compare
5163375 to
d7ab7dc
Compare
d7ab7dc to
a468e9c
Compare
|
I think it looks good now @bxf12315 . Can you rebase it and add it to the merge queue? |
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com> Assisted by: Cursor
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com> Assisted-by: Cursor
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com> Assisted-by: Cursor
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com> Assisted-by: Cursor
… format the code, and replace unwrap to make clippy happy.
f62b6bf to
f287610
Compare
f287610 to
57524c5
Compare
ok |
Summary by Sourcery
Add a new Trustify CLI workspace member that provides authenticated SBOM management and duplicate cleanup capabilities.
New Features:
trustify-clibinary crate underetc/trustify-clifor interacting with the Trustify API.Enhancements:
Documentation: