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

Update push model structures to latest values #961

Merged
merged 1 commit into from
Mar 25, 2025
Merged

Update push model structures to latest values #961

merged 1 commit into from
Mar 25, 2025

Conversation

sarroutbi
Copy link
Contributor

No description provided.

@sarroutbi sarroutbi force-pushed the 202503211102-update-to-latest-values branch 4 times, most recently from 92a55fd to 51adbd2 Compare March 24, 2025 09:25
@codecov Codecov
Copy link

codecov bot commented Mar 24, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.23%. Comparing base (8d03131) to head (ad16c53).
Report is 1 commits behind head on master.

Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 62.23% <100.00%> (-0.06%) ⬇️
upstream-unit-tests 62.23% <100.00%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
keylime/src/structures/capabilities_negotiation.rs 71.42% <100.00%> (+1.86%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sarroutbi sarroutbi requested a review from ansasaki March 24, 2025 10:30
@sarroutbi sarroutbi force-pushed the 202503211102-update-to-latest-values branch from 51adbd2 to ec7b284 Compare March 24, 2025 13:28
ansasaki
ansasaki reviewed Mar 24, 2025
View reviewed changes
Copy link
Contributor

@ansasaki ansasaki left a comment

Choose a reason for hiding this comment

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

I added some comments, probably we can discuss/clarify them quickly via slack. Otherwise we can bring this topic to the weekly meeting.

keylime/src/structures/capabilities_negotiation.rs Outdated
},
certification_keys: vec![
CertificationKey {
key_class: "pair".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, we should use asymmetric here instead of pair. It is better aligned with crypto lingo.
And to me this information is redundant if we keep the key_algorithm field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is OK for me to change it, but we can wait for next meeting to clarify it

Copy link

Choose a reason for hiding this comment

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

I am happy to have it as asymmetric. I agree that it is more in line with usual terminology

keylime/src/structures/capabilities_negotiation.rs Outdated
}

#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct AttestationKey {
pub struct CertificationKey {
key_class: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, key_class is redundant with key_algorithm as the type of the key (symmetric of asymmetric) is implicit in the key_algorithm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is OK for me to change it or remove the redundant fields, but we can wait for next meeting to clarify it

@sarroutbi sarroutbi force-pushed the 202503211102-update-to-latest-values branch 5 times, most recently from 4b93217 to af086c6 Compare March 25, 2025 12:42
@sarroutbi sarroutbi requested a review from ansasaki March 25, 2025 13:45
@sarroutbi
Update push model structures to latest values
ad16c53 
Resolves: keylime#962

Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
@sarroutbi sarroutbi force-pushed the 202503211102-update-to-latest-values branch from af086c6 to ad16c53 Compare March 25, 2025 15:38
ansasaki
ansasaki approved these changes Mar 25, 2025
View reviewed changes
Copy link
Contributor

@ansasaki ansasaki left a comment

Choose a reason for hiding this comment

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

LGTM! Let's merge as it is to unblock the next steps

@ansasaki ansasaki merged commit 4a55074 into keylime:master Mar 25, 2025
11 checks passed
@sarroutbi sarroutbi deleted the 202503211102-update-to-latest-values branch March 25, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stringlytyped stringlytyped stringlytyped left review comments

@ansasaki ansasaki ansasaki approved these changes

Assignees

@ansasaki ansasaki

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sarroutbi @stringlytyped @ansasaki