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

Fix: computation of the chaining of the certificates in tests #2032

Merged
merged 5 commits into from
Oct 22, 2024
Merged

Fix: computation of the chaining of the certificates in tests #2032

merged 5 commits into from
Oct 22, 2024

Conversation

jpraynaud
Copy link
Member

@jpraynaud jpraynaud commented Oct 21, 2024 •
edited
Loading

Content

This PR includes a fix to the chaining computation of the certificate chain in tests:

The previous certificate is either:

  • the first certificate of the previous epoch if the certificate is the first of the epoch
  • otherwise the first certificate of the epoch

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@jpraynaud jpraynaud self-assigned this Oct 21, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 21, 2024 •
edited
Loading

Test Results

    4 files     55 suites   10m 35s ⏱️
1 395 tests 1 395 ✅ 0 💤 0 ❌
1 609 runs  1 609 ✅ 0 💤 0 ❌

Results for commit 199d6c5.

♻️ This comment has been updated with latest results.

Alenar
Alenar approved these changes Oct 22, 2024
View reviewed changes
Copy link
Collaborator

@Alenar Alenar left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

mithril-common/src/certificate_chain/certificate_verifier.rs Outdated Show resolved Hide resolved
mithril-common/src/test_utils/certificate_chain_builder.rs Outdated Show resolved Hide resolved
sfauvel
sfauvel approved these changes Oct 22, 2024
View reviewed changes
Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM
Something strange in tests.

mithril-common/src/test_utils/certificate_chain_builder.rs Outdated Show resolved Hide resolved
@jpraynaud
fix: computation of the chaining of the certificates in the test chai…
cf2de5b 
…n builder

Previous certificate is either:
- the first certificate of the previous epoch if the certificate is the
  first of the epoch
- otherwise the first certificate of the epoch.
@jpraynaud
refactor: make 'builds_certificate_chain_correctly_chained' test easi…
9082f39 
…er to read
@jpraynaud
feat: implement FakeCertificaterRetriever for CertificateRetriever trait
84ba128
@jpraynaud
fix: verification tests for full valid and invalid chains
3831a3b
@jpraynaud
chore: bump crates versions
199d6c5 
- 'mithril-common' from '0.4.73' to '0.4.74'.
@jpraynaud jpraynaud force-pushed the jpraynaud/fix-certificate-chain-chaining branch from 4354dea to 199d6c5 Compare October 22, 2024 15:48
@jpraynaud jpraynaud merged commit 0cbc482 into main Oct 22, 2024
48 checks passed
@jpraynaud jpraynaud deleted the jpraynaud/fix-certificate-chain-chaining branch October 22, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alenar Alenar Alenar approved these changes

@sfauvel sfauvel sfauvel approved these changes

@dlachaume dlachaume dlachaume approved these changes

Assignees

@jpraynaud jpraynaud

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jpraynaud @Alenar @sfauvel @dlachaume