Skip to content

Conversation

@ansasaki
Copy link
Contributor

@ansasaki ansasaki commented Apr 4, 2023

Allow setting the path to the binary bios measurements log file via the TPM_BINARY_MEASUREMENTS environment variable when build with the testing feature enabled.

The binaries in the RPM are build with the testing feature enabled.

Fixes #490

@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #554 (cd9c214) into master (bf33c9a) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

❗ Current head cd9c214 differs from pull request most recent head 2e9a322. Consider uploading reports for the commit 2e9a322 to get more accurate results

Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 58.72% <ø> (ø)
upstream-unit-tests 58.89% <50.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
keylime-agent/src/common.rs 70.18% <ø> (ø)
keylime-agent/src/main.rs 35.38% <50.00%> (+0.33%) ⬆️

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.

I am a bit concern that this rpm/ can confuse later. It is to generate RPMs for the testing farm, and somehow unrelated with the project.

IMHO all this RPM code should be living in the test side and not in the project side, as in any case the testing farm is not really testing the released fedora nor centos package.

If this is not possible, at least we should add a README under rpm/ to clarify that "If you are looking for the reference spec file, do not look here"

@ansasaki
Copy link
Contributor Author

ansasaki commented Apr 5, 2023

I am a bit concern that this rpm/ can confuse later. It is to generate RPMs for the testing farm, and somehow unrelated with the project.

IMHO all this RPM code should be living in the test side and not in the project side, as in any case the testing farm is not really testing the released fedora nor centos package.

If this is not possible, at least we should add a README under rpm/ to clarify that "If you are looking for the reference spec file, do not look here"

I agree that it may be confusing at the moment, but the goal in future is to maintain the specfile here and automate downstream maintenance as well, so it will become the "reference specfile".

For now, the rpms will be used to avoid recompiling the agent for testing both in this repo, as well as in keylime/keylime. It makes easier for testing farm to have the rpm packages in Copr.

I'll add a commit to explain this in README.

@ansasaki ansasaki force-pushed the testing_mb branch 2 times, most recently from 0ac932a to 12b97d6 Compare April 6, 2023 12:52
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.

Seems that the Centos package fails for some GPG key. Not sure why, but the changes LGTM

Allow setting the binary measured boot log path via
TPM_BINARY_MEASUREMENTS environment variable when build with the
'testing' feature.

When building the RPMs, enable the testing feature.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
@ansasaki
Copy link
Contributor Author

ansasaki commented Apr 11, 2023

I added a small change to make the RPMS more usable: packit was picking the version from the tag (v0.2.0), which made RPM to not consider it an update because keylime-0.1.0 > keylime-v0.2.0 (note the v prepending the version number). I had to manually change the version to result in keylime-0.2.0 for the RPM.

I'll drop the temporary commit and push the changes.

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.

Make measured boot event log location configurable

4 participants