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.

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

ci: create .duvet/config.toml #5055

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Jan 22, 2025

Description of changes:

This change updates our duvet scripts to use the new config format from: awslabs/duvet#152.

$ ./compliance/generate_report.sh 
  Extracting requirements
   Extracted requirements from 8 specifications 40ms
    Scanning sources
     Scanned 1105 sources 56ms
     Parsing annotations
      Parsed 1368 annotations 216ms
     Loading specifications
      Loaded 18 specifications 9ms
     Mapping sections
      Mapped 219 sections 722µs
    Matching references
     Matched 4299 references 12ms
     Sorting references
      Sorted 4299 references 45ms
     Writing .duvet/reports/report.html
       Wrote .duvet/reports/report.html 8ms
     Writing .duvet/snapshot.txt
       Wrote .duvet/snapshot.txt 3ms

Callouts:

I split up the changes into 2 commits. The first creates the config, updates the scripts, and moves all of the requirement files from compliance/specs to .duvet/requirements: 132053b

The second commit pulls in the latest duvet extraction logic and updates the requirement files. This now preserves whitespace from the original RFC text: fcc7920

Testing:

Successful CI run here: https://github.com/aws/s2n-tls/actions/runs/12917769399/job/36024878800?pr=5055

The current mainline report is available here: https://d3fqnyekunr9xg.cloudfront.net/efb35006de554a9e9c0d74fb84e4fd74a039f144/compliance.html

Comparing the mainline report (on the left) to the new report (on the right), there is no change in requirement coverage:

Screenshot 2025-01-23 at 1 15 24 PM
Screenshot 2025-01-23 at 1 15 46 PM

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Jan 22, 2025
@camshaft camshaft changed the title ci: add duvet config ci: create .duvet/config.toml Jan 22, 2025
@camshaft camshaft marked this pull request as ready for review January 22, 2025 22:02
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a big PR, but I'm not seeing where these are moving to. They were manually generated because there aren't any "MUST"s in the original. Is the JA4 spec still being checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I didn't realize they were done manually. I can fix those.

Copy link
Contributor

@lrstewart lrstewart left a comment

Choose a reason for hiding this comment

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

Could you add more to the "Testing" section? I assume we're not supposed to review 361 files manually, so how have you confirmed that we're generating the same (or a better?) report? Especially given #5055 (comment)

@camshaft camshaft marked this pull request as draft January 22, 2025 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants