Tools and CI Meeting - Date 8/5

[makubacki]

Status

Scheduled for 8/5 @ 4:30pm Pacific Time

Agenda

1. Update: Reviewers automatically added to pull requests
   • [Completed] Basic support enabled in edk2
   • [Merging] Workflow Enhancements (See PR for Details):
     • .github: Request PR reviewers enhancements #5992
2. Overview of quick changes to improve CI time
   • Collection of improvement ideas: edk2 CI Time Improvement Ideas #6017
   • [In Progress] Change to add code coverage pipeline publishing back:
     • .azurepipelines: upload code coverage per job #6000
3. RFC: Bugzilla to GitHub issues
   • Details: RFC: Transitioning from Bugzilla to GitHub Issues #5926
     • UI Prototype: New Issue - makubacki/edk2
   • [Todo] @makubacki: Set a close date for comments on the RFC (give enough time for implementation)
     • Close date will be this Friday (August 16, 2024)
   • [Done] @makubacki: Suggestion is to change "Firmware" and "Non-Firmware" options in the bug form to "Firmware", "Tool", and "Unit Test"
   • [Done] @makubacki: Suggestion to add additional context to the "steps to reproduce" field in the bug form to help a developer exactly reproduce the problem - "How was the code acquired (git, edkrepo, etc.)?", "How was the built (stuart, edksetup/build, etc.)?"
   • [Done] @makubacki: Clarify in the template what "version" information is being asked for
   • [Done] @makubacki: Request Liming review the RFC in detail
4. Current status of enabling PRs in edk2-platforms
   • Last week's update: Tools and CI Meeting - Date 7/29 #6001 (comment)
     • "Sent patch to create CODEOWNERS, REVIEWERS, CONTRIBUTORS and the GitHub workflow script to assign reviewers, currently waiting for people to review it."
     • Latest Patch: [PATCH edk2-platforms v3 0/1] Move to the GitHub Pull Request workflow
     • Todo: Determine whether to require participants must make their email public in GitHub so its accessible via the GitHub API or required to make the email only public in the CONTRIBUTORS.md file or you can participate without a known email address.
       • @mdkinney to follow up at the community level about what to do.
     • edk2-libc and edk2-test are interested in moving to PRs as well. Need to keep the process consistent.
5. Process to merge GHSA into edk2 repo
   • Emails recently between edk2 owners and the infosec team about using GHSA
   • Current Open: Once a security issue is resolved and tested and the embargo period is over, it has been reviewed by infosec and tested by several third parties at that time. Not much further review expected to be needed by the edk2 community. Want to expedite the process to get the change merged.
   • Earlier feedback (altered for a GHSA repo per issue as opposed to branches on single private repo as previously mentioned):
     • The idea here is to use the GHSA item to track a link to the PR in the private GHSA repo that is available for review to those with permission prior to it going public. Once it goes public, the GHSA repo would be closed, and the code at that time would be taken to a public PR. At this point, it should have been reviewed and successfully voted upon by stakeholders to address the problem. The public PR would be added to the GHSA item, so it now tracks both the GHSA and public PR. Because the GHSA repo is long lived, the GHSA PR would always be accessible in the future for reference. The GHSA PR discussion would also not leak to the public PR which itself may contain information about other embargoed security issues, etc. The relevant maintainers would now have an opportunity to review the code for its submission to the public master branch. We could add a “type:cve-fix” tag to indicate it is time sensitive and should be prioritized. This would also filter the CVE fixes for edk2 consumers in the future and highlight them in stable tag release notes if desired.
     • 8/12/2024 - @Flickdm: Next step is to take a GHSA item recently closed and see if it can easily be taken to the main repo (a PR is created there). A button may be available to do that - need to check.
       • Note: Can create a test GHSA issue to test the flow.
       • Note: If necessary, can override CODEOWNERS enforcement with a steward override.
     • Separate open: Need to follow up on why some GHSA links are broken today.
6. Easier way to see Uncrustify issues
   • @makubacki: Add the errors to the main build log
7. Easier way to see ECC issues and see all ECC failures (not just first failure)
   • It's been reported that ECC stops on the first error
   • Request is to add all failure info in one run to prevent the need for additional reruns
   • Also need to make sure the error info is in the main build log (if not already there)
   • @makubacki [DONE]: Rebase MdeModulePkg: Add VariablePolicyLib unit tests #5930 so @mdkinney can look at the output
8. @mdkinney: Review of Package scope and File scope disables of Uncrustify, ECC, and SpellCheck CI checks

• Opens to answer:
  1. Is package and file scope available for all plugins listed here?
  2. Are they consistent in how similar options are configured?

Note:

• CodeQL pin update
  • [Done] BaseTools/Plugin/CodeQlQueries.qls: Pin to the 1.1.0 codeq/cpp-queries pack #5986

Follow Up

• Open from last week deferred until next week (8/12) when Sean can confirm his schedule:
  • Can this meeting be moved to a European friendly time?
    • Can potentially switch meetings between time zones on alternating weeks.
    • Likely range would 7am pacific to 8am pacific
    • Could propose a start around September
  • Decision (8/12/2024): Suggest the first meeting of each month start at 7am pacific
    • @spbrogan: Send email out to propose the time

Unit test related opens around testing asserts:

• Are we using asserts properly? Versus error code, panic, etc.
• Are we depending on asserts for functional behavior?

---

• @Javagedes: Add PR Eval DSC to platforms.
  • More details & example: 87ef986

Todos From Previous Meeting

• Investigate GitHub merge queue as a replacement for Mergify.
• Next Meeting: Follow up on Bugzilla to GitHub Issue RFC
  • Mike Kinney's preference: Switch to GitHub by Monday, August 26, 2024 and switch the TianoCore Bugzilla to read only at that time
• Investigate making most jobs depend on a small Basic Acceptance Test (BAT) pool of checks to pass
• Can someone add measurements of CI information so we can make optimizations based on the data?
  • Example: Queue time, Queue depth, etc.
• Check the PR activity before pipeline is finished scenario. Does it cancel the current run?

Groups.io meeting invite:

https://edk2.groups.io/g/devel/viewevent?repeatid=43269&eventid=2159616&calstart=2024-01-08

Microsoft Teams meeting

Join on your computer or mobile app
Click here to join the meeting
Or call in (audio only)
+1 425-616-0754,,330776518# United States, Seattle
Phone Conference ID: 330 776 518#
Find a local number | Reset PIN
Learn More | Meeting options

[Flickdm]

Process to merge GHSA into edk2 repo -- Update

• Creating a new Advisory must be done by a "Publisher" which is today a "Tianocore Owner" as a "Contributor" I cannot create a private fork.
• Private forks will be created for each advisory

• Once the PR is made the Azure Pipelines appear to be unable to connect to Github leaving the pipelines continuously "waiting"

As I continue this process - I will update this thread.

[Flickdm]

Once the PR is added the following options are available:

• Merging may be done by a "Publisher" as defined before.
• Unsure if this means merge into public github project

Reviews are still required for this PR / Potentially need to fix the CI pipelines - Will update with more information as available

[Flickdm]

Once the PR is added the following options are available:

• Merging may be done by a "Publisher" as defined before.
• Unsure if this means merge into public github project

Reviews are still required for this PR / Potentially need to fix the CI pipelines - Will update with more information as available

UPDATE:

Once the temporary fork is made - vulnerability fixes may begin happening even while the CVE is still in DRAFT.

The coordinator may begin to proceed to acquire a CVE from a CNA.

The developer can begin to create merge requests for each issue.

Initially these merge requests should target the default branch. Once these are ready to be made public, the maintainer who will be responsible for ensuring these patches make it into their packages should create a new /security-advisory/cve-xxx-xxxx/advisory-x branch for each issue and that should be set as the target for the merge request.

Once done each pull request should be approved and then the advisory is ready to be made public.

Once public the patch writer should create a second public PR - that has a link to the initial advisory. Then the maintainer should merge the patches in within 24 hours (or sooner).

[Javagedes]

Per this discussion and the action item assigned to me, I have created a PR that adds the PrEval entry to all package CI configuration files that did not previous have it. See #6082