[draft] copy over 1es files - #385
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a set of PowerShell modules/scripts and workflow updates to support VM-backed driver test execution (including 1ES runner support), along with packaging the required test payload files into build artifacts.
Changes:
- Add new CI/CD test orchestration scripts/modules for setting up VMs, executing kernel driver tests, collecting logs/dumps/traces, and cleaning up.
- Extend build/test GitHub workflows to support 1ES runner execution and to bundle required VM test payload files (including an eBPF-for-Windows MSI).
- Enhance the existing installer download script with additional parameters and signature validation.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/vm_run_tests.psm1 | Adds helpers to run tests on host vs VM and trigger kernel dumps. |
| scripts/tracing_utils.psm1 | Adds simplified WPR ETW tracing helpers. |
| scripts/test_execution.json | Adds VM-to-runner mapping and driver test suite definitions. |
| scripts/setup_ebpf_cicd_tests.ps1 | Adds CI/CD setup script to prep VMs/host for driver tests. |
| scripts/setup_build/setup_build.vcxproj | Adds additional scripts/json into the setup_build copy payload. |
| scripts/run_driver_tests.psm1 | Adds driver test execution wrapper and kernel dump trigger helper. |
| scripts/Install-eBpfForWindows.ps1 | Adds parameters, download-only mode, and Authenticode validation. |
| scripts/install_ebpf.psm1 | Adds MSI-based eBPF component install/stop/uninstall helpers. |
| scripts/execute_ebpf_cicd_tests.ps1 | Adds CI/CD execution script to run selected driver test suites. |
| scripts/config_test_vm.psm1 | Adds VM initialization, artifact export, and result import logic. |
| scripts/common.psm1 | Adds shared logging, timeout/copy helpers, and VM credential utilities. |
| scripts/cleanup_ebpf_cicd_tests.ps1 | Adds CI/CD cleanup script to collect results and stop VMs. |
| .github/workflows/reusable-test.yml | Updates reusable test workflow for 1ES runner paths and longer timeouts. |
| .github/workflows/reusable-build.yml | Downloads MSI into build output and validates required VM payload contents. |
| .github/workflows/cicd.yml | Moves driver tests to 1ES runner execution and adjusts reusable workflow inputs. |
Suppressed comments (5)
scripts/execute_ebpf_cicd_tests.ps1:55
- This
Import-Modulepath is not quoted/constructed;$WorkingDirectory\common.psm1will not expand as a filesystem path and can fail whenWorkingDirectorycontains spaces. UseJoin-Path(or a quoted string) to build the path.
Import-Module $WorkingDirectory\common.psm1 -Force -ArgumentList ($LogFileName) -WarningAction SilentlyContinue
scripts/execute_ebpf_cicd_tests.ps1:134
- This
Import-Modulepath is not quoted/constructed;$WorkingDirectory\common.psm1will not expand as a filesystem path reliably. UseJoin-Path(or a quoted string) to build the module path.
Import-Module $WorkingDirectory\common.psm1 -Force -ArgumentList ($LogFileName) -ErrorAction SilentlyContinue
scripts/config_test_vm.psm1:322
- Same path construction issue here:
$WorkingDirectory\common.psm1/$WorkingDirectory\install_ebpf.psm1are not reliably expanded as filesystem paths. UseJoin-Path(or a quoted string).
Import-Module $WorkingDirectory\common.psm1 -ArgumentList ($LogFileName) -Force -WarningAction SilentlyContinue
Import-Module $WorkingDirectory\install_ebpf.psm1 -ArgumentList ($WorkingDirectory, $LogFileName) -Force -WarningAction SilentlyContinue
scripts/config_test_vm.psm1:347
- This
Import-Moduleuses$WorkingDirectory\common.psm1, which is not reliable path construction (and can fail if the path has spaces). UseJoin-Path(or a quoted string) here as well.
Import-Module $WorkingDirectory\common.psm1 `
scripts/config_test_vm.psm1:350
- This
Import-Moduleuses$WorkingDirectory\install_ebpf.psm1, which is not reliable path construction (and can fail if the path has spaces). UseJoin-Path(or a quoted string) here as well.
Import-Module $WorkingDirectory\install_ebpf.psm1 `
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| { | ||
| if (![System.String]::IsNullOrEmpty($TraceMessage)) { | ||
| $timestamp = (Get-Date).ToString('HH:mm:ss') | ||
| Write-Host "[$timestamp] :: $TraceMessage"-ForegroundColor $ForegroundColor |
| [Parameter(Mandatory=$true)] [string] $WorkingDirectory | ||
| ) | ||
|
|
||
| Import-Module $WorkingDirectory\common.psm1 -Force -ArgumentList ($LogFileName) -WarningAction SilentlyContinue |
|
|
||
| Push-Location $WorkingDirectory | ||
|
|
||
| Import-Module $WorkingDirectory\common.psm1 -Force -ArgumentList ($LogFileName) -ErrorAction Stop |
| $VMName = $null | ||
| $TestWorkingDirectory = $WorkingDirectory | ||
| } | ||
| Import-Module $WorkingDirectory\vm_run_tests.psm1 ` |
|
|
||
| # Re-import common.psm1 in case Wait-TestJobToComplete's timeout handler | ||
| # forcefully re-imported it (via vm_run_tests.psm1), removing it from this scope. | ||
| Import-Module $WorkingDirectory\common.psm1 -Force -ArgumentList ($LogFileName) -ErrorAction SilentlyContinue |
| Import-Module $WorkingDirectory\common.psm1 -ArgumentList ($LogFileName) -Force -WarningAction SilentlyContinue | ||
| Import-Module $WorkingDirectory\install_ebpf.psm1 -ArgumentList ($WorkingDirectory, $LogFileName) -Force -WarningAction SilentlyContinue |
| # Start the VMs. | ||
| Start-AllVMs -VMList $VMList | ||
|
|
||
| if (-not $VMIsRemote) { |
| { | ||
| param([parameter(Mandatory=$true)][string] $VMName) | ||
|
|
||
| Write-Log "Unnstalling eBPF components on $VMName" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
Previously missed (5) — in code that hasn't changed since the last review.
scripts/execute_ebpf_cicd_tests.ps1:66
- When ExecuteOnVM is true, $Config.VMMap.$SelfHostedRunnerName can be $null (missing key) or empty; indexing $VMList[0] will throw with a non-obvious error. Add an explicit validation and error message before using the first VM.
if ($ExecuteOnVM) {
Write-Log "Tests will be executed on VM" -ForegroundColor Cyan
$VMList = $Config.VMMap.$SelfHostedRunnerName
$VMName = $VMList[0].Name
$TestWorkingDirectory = "C:\ebpf"
scripts/setup_ebpf_cicd_tests.ps1:82
- When ExecuteOnVM is true, $Config.VMMap.$SelfHostedRunnerName can be missing/empty; later VM initialization and export steps will fail with unclear errors. Validate VMList early and throw a clear message.
if ($ExecuteOnVM) {
$VMList = $Config.VMMap.$SelfHostedRunnerName
if (-not $VMIsRemote) {
# Get all local VMs to ready state.
Initialize-AllVMs -VMList $VMList -ErrorAction Stop
scripts/cleanup_ebpf_cicd_tests.ps1:44
- When ExecuteOnVM is true, $Config.VMMap.$SelfHostedRunnerName can be missing/empty; subsequent cleanup steps will fail and may obscure the real root cause. Validate VMList before using it.
if ($ExecuteOnVM) {
$VMList = $Config.VMMap.$SelfHostedRunnerName
# Wait for all VMs to be in ready state, in case the test run caused any VM to crash.
Wait-AllVMsToInitialize -VMList $VMList -VMIsRemote $VMIsRemote
.github/workflows/reusable-test.yml:105
- This reusable workflow is triggered via workflow_call, so github.event.workflow_run.head_branch is not defined. Using it for actions/checkout can result in checking out the wrong ref or failing to resolve the ref; use github.sha (or github.ref) to ensure the workflow checks out the exact commit being tested.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
# Check out source code if code coverage is being gathered or if crash dumps
# (which require ProcDump downloaded via the repo scripts) are being gathered.
if: ((inputs.code_coverage == true) || (inputs.gather_dumps == true)) && !contains(inputs.environment, '1ES') && (steps.skip_check.outputs.should_skip != 'true')
with:
scripts/common.psm1:563
- $JobTimedOut is only assigned in the timeout path; when the job completes normally this returns $null, which is easy to mis-handle and makes the return type inconsistent. Initialize it to $false before the loop.
$TimeElapsed = 0
scripts/config_test_vm.psm1:314
- Typo in log message: "Unnstalling" -> "Uninstalling".
Write-Log "Unnstalling eBPF components on $VMName"
| param( | ||
| [Parameter(Mandatory = $false)][bool] $ExecuteOnHost = $false, | ||
| [Parameter(Mandatory = $false)][bool] $ExecuteOnVM = $false, | ||
| [Parameter(Mandatory = $false)][bool] $VMIsRemote = $false, | ||
| [Parameter(Mandatory = $true)][string] $VMName, | ||
| [Parameter(Mandatory = $true)][string] $WorkingDirectory, | ||
| [Parameter(Mandatory = $true)][string] $LogFileName, | ||
| [Parameter(Mandatory = $false)][string] $TestMode = "CI/CD", | ||
| [Parameter(Mandatory = $false)][string[]] $Options = @("None"), | ||
| [Parameter(Mandatory = $false)][int] $TestHangTimeout = (30 * 60), | ||
| [Parameter(Mandatory = $false)][string] $UserModeDumpFolder = "C:\Dumps", | ||
| [Parameter(Mandatory = $false)][bool] $GranularTracing = $false, | ||
| [Parameter(Mandatory = $false)][bool] $RunXdpTests = $false | ||
| ) | ||
|
|
||
| Import-Module "$PSScriptRoot\common.psm1" -Force -ArgumentList $LogFileName -WarningAction SilentlyContinue | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
scripts/run_driver_tests.psm1:45
- On timeout, the test process is left running. That can leak resources and interfere with subsequent tests / cleanup. The script should terminate the hung process before throwing the TimeoutException.
if (-not $process.WaitForExit($Timeout * 1000)) {
Write-Log "$Name exceeded its timeout. A kernel dump will be requested."
throw [System.TimeoutException]::new("$Name timed out after $Timeout seconds.")
}
scripts/config_test_vm.psm1:178
- Initialize-AllVMs references $VMIsRemote, but that variable is not defined in module scope and isn’t passed as a parameter. This will always evaluate as $false (or rely on a global variable), which makes the remote-path logic unreliable and easy to break if the function is reused.
if (-not $VMIsRemote) {
# Wait for VMs to be ready.
Write-Log "Waiting for all the VMs to be in ready state..." -ForegroundColor Yellow
Wait-AllVMsToInitialize -VMList $VMList
} else {
scripts/config_test_vm.psm1:314
- Typo in log message: "Unnstalling" → "Uninstalling".
Write-Log "Unnstalling eBPF components on $VMName"
| function Get-VMPassword { | ||
| return 'eBPF4W!n' | ||
| } |
dc5b199 to
23ccb2d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
scripts/config_test_vm.psm1:166
- Initialize-AllVMs branches on $VMIsRemote, but $VMIsRemote is not defined/passed into the function. This makes the remote-vs-local behavior depend on an outer-scope variable (or default to $false), which can lead to incorrect behavior if the function is ever used for remote VMs.
function Initialize-AllVMs
{
param ([Parameter(Mandatory=$True)] $VMList)
scripts/config_test_vm.psm1:314
- Typo in log message: "Unnstalling" -> "Uninstalling".
Write-Log "Unnstalling eBPF components on $VMName"
| function Stop-eBPFComponents | ||
| { | ||
| param([Parameter(Mandatory = $false)][bool] $GranularTracing = $false) | ||
|
|
||
| $scriptBlock = { | ||
| param($WorkingDirectory, $LogFileName) | ||
| Import-Module "$WorkingDirectory\common.psm1" -Force -ArgumentList $LogFileName -WarningAction SilentlyContinue | ||
| Import-Module "$WorkingDirectory\install_ebpf.psm1" ` | ||
| -Force ` | ||
| -ArgumentList $WorkingDirectory, $LogFileName ` | ||
| -WarningAction SilentlyContinue | ||
| Stop-eBPFServiceAndDrivers | ||
| } | ||
|
|
||
| Invoke-OnHostOrVM -ScriptBlock $scriptBlock -ArgumentList @($script:WorkingDirectory, $script:LogFileName) | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (5)
Previously missed (3) — in code that hasn't changed since the last review.
scripts/vm_run_tests.psm1:93
- Stop-eBPFComponents accepts a GranularTracing parameter but never forwards it into the remote script block, so callers toggling GranularTracing currently have no effect. Pass GranularTracing through and call Stop-eBPFServiceAndDrivers with it.
param([Parameter(Mandatory = $false)][bool] $GranularTracing = $false)
$scriptBlock = {
param($WorkingDirectory, $LogFileName)
Import-Module "$WorkingDirectory\common.psm1" -Force -ArgumentList $LogFileName -WarningAction SilentlyContinue
scripts/setup_ebpf_cicd_tests.ps1:115
- The comment says installing eBPF components "requires psexec" and can’t run in a PowerShell job, but the code path calls Install-eBPFComponents (msiexec-based) inside the job. This comment is misleading and should be removed or corrected.
# Install eBPF components on host, but skip anything that requires reboot.
# Note that installing ebpf components requires psexec which does not run in a powershell job.
Write-Log "Installing eBPF components on host"
.github/workflows/cicd.yml:115
- The 1ES JobId string includes the matrix job-index twice ("${{ strategy.job-index }}" and the "{0}" placeholder). Since reusable-test.yml already injects strategy.job-index via format(..., strategy.job-index), this will produce duplicated suffixes like "...-0-0". Remove one of them (keep "{0}" for formatting).
This issue also appears on line 137 of the same file.
environment: '["self-hosted", "1ES.Pool=ebpf-extensions-cicd-runner-pool", "1ES.ImageOverride=${{ matrix.image }}_v2", "JobId=netevent-driver-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ strategy.job-index }}-{0}"]'
.github/workflows/cicd.yml:137
- The 1ES JobId string includes the matrix job-index twice ("${{ strategy.job-index }}" and the "{0}" placeholder). Since reusable-test.yml already injects strategy.job-index via format(..., strategy.job-index), this will produce duplicated suffixes like "...-0-0". Remove one of them (keep "{0}" for formatting).
environment: '["self-hosted", "1ES.Pool=ebpf-extensions-cicd-runner-pool", "1ES.ImageOverride=${{ matrix.image }}_v2", "JobId=ntos-driver-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ strategy.job-index }}-{0}"]'
scripts/config_test_vm.psm1:314
- Typo in log message: "Unnstalling" should be "Uninstalling".
Write-Log "Unnstalling eBPF components on $VMName"
ec0f682 to
0fa9ce5
Compare
537b7b3 to
07d3be7
Compare
07d3be7 to
c114e3d
Compare
Description
Describe the purpose of and changes within this Pull Request.
Testing
Do any existing tests cover this change? Are new tests needed?
Documentation
Is there any documentation impact for this change?
Installation
Is there any installer impact for this change?