Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement plugin system for archivists' parsers #295

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

fkautz
Copy link
Contributor

@fkautz fkautz commented Jun 5, 2024

This commit introduces a flexible plugin system for archivists' parsers,
allowing new parsers to be registered dynamically. The system utilizes an
initialization function that registers parsers via a Register function. The
primary changes include:

  • Creation of parser_registry.go to handle parser registration and storage.
  • Modification of parserstorer.go to integrate the new parser registration
    system and utilize registered parsers during attestation storage.

This system enhances the extensibility of the parser functionality and improves
maintainability by decoupling the parser registration from the core logic.

Changes:

  • Added internal/metadatastorage/attestationcollection/parser_registry.go for
    parser registration.
  • Updated internal/metadatastorage/attestationcollection/parserstorer.go to
    support the new plugin system.

Signed-off-by: Frederick F. Kautz IV [email protected]

@fkautz
Copy link
Contributor Author

fkautz commented Jun 5, 2024

For reference, this is the same pattern used by go-witness to add attestation types.

@fkautz fkautz force-pushed the attestation_parser_plugins branch from 3f9db83 to f6fdc33 Compare June 5, 2024 04:16
Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 41.66667% with 7 lines in your changes missing coverage. Please review.

Project coverage is 1.62%. Comparing base (a035c62) to head (96ea1a0).
Report is 96 commits behind head on main.

Files Patch % Lines
...adatastorage/attestationcollection/parserstorer.go 0.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #295       +/-   ##
==========================================
- Coverage   82.40%   1.62%   -80.78%     
==========================================
  Files          10     120      +110     
  Lines         358   28856    +28498     
==========================================
+ Hits          295     470      +175     
- Misses         43   28329    +28286     
- Partials       20      57       +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This commit introduces a flexible plugin system for archivists' parsers,
allowing new parsers to be registered dynamically. The system utilizes an
initialization function that registers parsers via a `Register` function. The
primary changes include:

- Creation of `parser_registry.go` to handle parser registration and storage.
- Modification of `parserstorer.go` to integrate the new parser registration
  system and utilize registered parsers during attestation storage.

This system enhances the extensibility of the parser functionality and improves
maintainability by decoupling the parser registration from the core logic.

Changes:
- Added `internal/metadatastorage/attestationcollection/parser_registry.go` for
  parser registration.
- Updated `internal/metadatastorage/attestationcollection/parserstorer.go` to
  support the new plugin system.

Signed-off-by: Frederick F. Kautz IV <[email protected]>
@fkautz fkautz force-pushed the attestation_parser_plugins branch from f6fdc33 to f481817 Compare June 5, 2024 04:20
@fkautz
Copy link
Contributor Author

fkautz commented Jun 5, 2024

I have two features coming up after this that depend on this patch:

  • git attestations
  • will refactor the omnitrail attestation for this

Signed-off-by: Frederick F. Kautz IV <[email protected]>
@fkautz fkautz force-pushed the attestation_parser_plugins branch from 6aae5dd to 96ea1a0 Compare June 5, 2024 05:06
Copy link
Member

@jkjell jkjell left a comment

Choose a reason for hiding this comment

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

Just a couple of questions centered around how to build out support for attestations outside of collections, as we keep generating more and more of those. 😅

@fkautz
Copy link
Contributor Author

fkautz commented Jun 6, 2024

Just a couple of questions centered around how to build out support for attestations outside of collections, as we keep generating more and more of those. 😅

You know... we could refactor the whole code base to be plugin-based over time. (not serious but actually am?) 😅

I think there are several places where we can use a plugin system. We just need to make sure we can consistently trigger the correct plugin to avoid archivista getting confused.

Let me know if you think this would be a good idea. We can refactor this over time. I have two attestations to add after this patch. The subsequent patches will show off how an attestation parser would register and work. I was just waiting for @kairoaraujo's work to merge first because his most recent refactor patch changes what the details look like.

@jkjell jkjell merged commit d1df59f into in-toto:main Jun 6, 2024
11 of 13 checks passed
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.

3 participants