Skip to content

Conversation

@G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Dec 2, 2025

I initially tried to have the extractor pass through the information needed, but quickly found that was too config-y so I've just opted for re-doing the extraction in the enricher, with the bulk of the logic being literally just ripped from pomxmlnet

Resolves #1488

@G-Rath
Copy link
Collaborator Author

G-Rath commented Dec 2, 2025

@cuixq I'd appreciate if you could contribute some tests depending on what you feel is needed, as I am not a Java person so don't have Maven available to generate fixtures 😅

@G-Rath G-Rath force-pushed the pom-enricher branch 2 times, most recently from b4558dc to 8f0a85e Compare December 2, 2025 04:02
@erikvarga
Copy link
Collaborator

I initially tried to have the extractor pass through the information needed, but quickly found that was too config-y so I've just opted for re-doing the extraction in the enricher, with the bulk of the logic being literally just ripped from pomxmlnet

The main use case we have for enrichers internally is separating filesystem access from network access. This allows us to extract inventory from a host without doing any network calls, then separately enrich the results on a server that no longer has any access to the host's filesystem.

Enrichers that still access the filesystem aren't really usable for this setup so there's not much gained compared to the existing pomxmlnet extractor setup. Thus we'd prefer if you could store the filesystem info needed for transitive lookup during the extraction step (in the pomxml extractor) and only use that data in the enricher code itself.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Dec 2, 2025

@erikvarga this approach was the result of discussion with @another-rex and @cuixq, and I think one of the challenges here is that (if I understand correctly) the information required for resolving transitive dependencies for pom.xml requires information that is not directly tied to specific packages (not to mention requires reasonably more work just for this enricher)

@erikvarga
Copy link
Collaborator

erikvarga commented Dec 4, 2025

@cuixq pointed out that there are other benefits with the current approach such as receiving individual results if regular extraction succeeds but the transitive extraction fails. In this case it should be fine to go forward with this split, and we'll track filesystem-less enrichment as a separate follow-up item.

So SGTM to start with the current approach. I'll leave it up to @another-rex or @cuixq to review the PR since they have more context on the pomxmlnet extractor.

Copy link
Collaborator

@cuixq cuixq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering we are

}

// Add handles supplementing an inventory with enriched packages
func Add(enrichedPkgs []*extractor.Package, inv *inventory.Inventory, pluginName string, existingPackages map[string]PackageWithIndex) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if its possible to do cleanly, but I feel like it would be ideal if we didn't need the existingPackages parameter

@G-Rath G-Rath requested a review from cuixq December 8, 2025 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move pomxmlnet extractor to transitivedependency enricher

3 participants