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.

Sign up for GitHub

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

Jump to bottom

cargo: Replace compress-tools with zip crate #739

Merged
merged 5 commits into from
Feb 12, 2024
Merged

cargo: Replace compress-tools with zip crate #739

merged 5 commits into from
Feb 12, 2024

Conversation

ansasaki
Copy link
Contributor

@ansasaki ansasaki commented Feb 8, 2024

Replace compress-tools with zip crate
Remove mentions of libarchive dependency from README, as well as from rpm spec files and dockerfiles
Remove keylime user creation from non-fedora based dockerfiles.
Add wget installation to wolfi based image.

Fixes #733
Fixes #671

@ansasaki ansasaki force-pushed the drop-compress-tools branch from 5c1a881 to 542fa66 Compare February 8, 2024 17:48
@ansasaki
Copy link
Contributor Author

ansasaki commented Feb 8, 2024

This is very similar to previous attempt from @THS-on on #530

aplanas
aplanas reviewed Feb 9, 2024
View reviewed changes
Copy link
Contributor

@aplanas aplanas left a comment

Choose a reason for hiding this comment

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

LGTM, the only question is the test fails in Fedora

docker/release/Dockerfile.wolfi Outdated Show resolved Hide resolved
rpm/fedora/rust-keylime-metadata.patch Show resolved Hide resolved
THS-on
THS-on approved these changes Feb 9, 2024
View reviewed changes
Copy link
Member

@THS-on THS-on left a comment

Choose a reason for hiding this comment

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

Rust code changes LGTM.

At some point I should look again at building a fully static agent, but this will also require some changes to tss library to allow static linking.

@ansasaki
Copy link
Contributor Author

ansasaki commented Feb 9, 2024

LGTM, the only question is the test fails in Fedora

Yes, it is currently failing due to the ahash crate, which is failing to build with nightly rustc. I proposed a workaround in the tests repository via RedHat-SP-Security/keylime-tests#550, but we also need to bump the ahash crate.

I'll extract the commit where I bump the ahash crate on my other open PR (d210efd) and create a dedicated PR

@ansasaki ansasaki force-pushed the drop-compress-tools branch from 542fa66 to 4cc20c8 Compare February 9, 2024 10:47
@ansasaki
cargo: Replace compress-tools with zip crate
11281d9 
The zip crate brings less external dependencies. With this change, we
can remove libarchive from the dependencies.

Fixes keylime#733

Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@ansasaki
rpm: Remove libarchive from dependencies
b7bd6ff 
Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@ansasaki
docker: Remove libarchive as a dependency
2ef519c 
Also remove keylime user creation from non-fedora distributions and add
wget to wolfi-based dockerfile

Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@ansasaki
Dockerfile.wolfi: Update clang to version 17
e882038 
Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@ansasaki
README: remove mentions of libarchive as a dependency
6dc2a2d 
Signed-off-by: Anderson Toshiyuki Sasaki <[email protected]>
@ansasaki ansasaki force-pushed the drop-compress-tools branch from 4cc20c8 to 6dc2a2d Compare February 9, 2024 12:19
@codecov Codecov
Copy link

codecov bot commented Feb 9, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (ead2f0d) 65.28% compared to head (6dc2a2d) 65.06%.
Report is 1 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 58.77% <50.00%> (-0.38%) ⬇️
upstream-unit-tests 52.55% <50.00%> (+0.24%) ⬆️

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

Files Coverage Δ
keylime-agent/src/payloads.rs 83.82% <100.00%> (+1.06%) ⬆️
keylime-agent/src/error.rs 15.25% <0.00%> (-0.18%) ⬇️

... and 5 files with indirect coverage changes

@ansasaki
Copy link
Contributor Author

/packit retest-failed

@ansasaki
Copy link
Contributor Author

The proposed workaround in tests repo PR (RedHat-SP-Security/keylime-tests#550) was merged and the tests re-run passed, making this ready to merge

@ansasaki ansasaki merged commit 409d2e8 into keylime:master Feb 12, 2024
10 of 11 checks passed
@ansasaki ansasaki deleted the drop-compress-tools branch February 12, 2024 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@THS-on THS-on THS-on approved these changes

@aplanas aplanas aplanas approved these changes

@maugustosilva maugustosilva Awaiting requested review from maugustosilva

Assignees

@maugustosilva maugustosilva

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@ansasaki @aplanas @THS-on @maugustosilva