Skip to content

Refactor everything relating to OCI client#1601

Merged
zregvart merged 1 commit into
conforma:mainfrom
zregvart:pr/cleanup-oci-client
May 16, 2024
Merged

Refactor everything relating to OCI client#1601
zregvart merged 1 commit into
conforma:mainfrom
zregvart:pr/cleanup-oci-client

Conversation

@zregvart

@zregvart zregvart commented May 7, 2024

Copy link
Copy Markdown
Contributor

This refactors all codepaths to use the single OCI client abstraction and a single fake/mock in testing.

Now all options provided to go-containerregistry are maintaned in the single place internal/utils/oci/client.go.

@codecov

codecov Bot commented May 7, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 65.30612% with 34 lines in your changes are missing coverage. Please review.

Project coverage is 87.25%. Comparing base (e72483b) to head (0c8f7b3).
Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1601      +/-   ##
==========================================
+ Coverage   80.79%   87.25%   +6.46%     
==========================================
  Files          65       75      +10     
  Lines        4738     5045     +307     
==========================================
+ Hits         3828     4402     +574     
+ Misses        910      643     -267     
Flag Coverage Δ
acceptance 72.71% <64.28%> (?)
generative 80.70% <48.97%> (-0.10%) ⬇️
integration 80.70% <48.97%> (-0.10%) ⬇️
unit 80.70% <48.97%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
internal/applicationsnapshot/input.go 93.38% <100.00%> (+10.53%) ⬆️
internal/fetchers/oci/config/config.go 96.87% <100.00%> (ø)
internal/fetchers/oci/files/files.go 91.30% <100.00%> (ø)
internal/rego/oci/oci.go 96.59% <100.00%> (-0.15%) ⬇️
internal/rego/sigstore/sigstore.go 94.94% <100.00%> (+0.56%) ⬆️
internal/utils/testing_image.go 0.00% <ø> (ø)
...ation_snapshot_image/application_snapshot_image.go 77.71% <87.50%> (+15.15%) ⬆️
internal/utils/oci/client.go 57.69% <57.69%> (ø)

... and 38 files with indirect coverage changes

@zregvart zregvart force-pushed the pr/cleanup-oci-client branch from 41b78e3 to 83af984 Compare May 9, 2024 09:21

@simonbaird simonbaird left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm reviewing the intention more than the code changes, but lgtm afaict. The tests are green so should be good right?

Comment thread cmd/validate/image_test.go Outdated
Comment thread internal/applicationsnapshot/input.go
Comment thread internal/rego/sigstore/sigstore_test.go
Comment thread internal/utils/oci/client.go
Comment thread internal/utils/oci/client.go
Comment thread internal/rego/oci/oci.go Outdated
Comment thread internal/utils/testing_image.go
@zregvart zregvart force-pushed the pr/cleanup-oci-client branch from 83af984 to 0412135 Compare May 14, 2024 09:25
Comment thread internal/rego/oci/oci.go Outdated
@lcarva

lcarva commented May 14, 2024

Copy link
Copy Markdown
Contributor

Thanks for doing this much needed re-factoring. It's beautiful 🤌

This refactors all codepaths to use the single OCI client abstraction
and a single fake/mock in testing.

Now all options provided to go-containerregistry are maintaned in the
single place `internal/utils/oci/client.go`.
@zregvart zregvart force-pushed the pr/cleanup-oci-client branch from 0412135 to 0c8f7b3 Compare May 15, 2024 10:07
@zregvart

Copy link
Copy Markdown
Contributor Author

Merging this now, if there are any followups feel free to create issues for them...

@zregvart zregvart merged commit e65de76 into conforma:main May 16, 2024
@zregvart zregvart deleted the pr/cleanup-oci-client branch May 16, 2024 10:08
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