Add server lifecycle audit PowerShell script#47
Conversation
This script performs a comprehensive audit of server lifecycle aspects including OS details, domain roles, CPU, memory, disk layout, server roles, SQL instances, installed software, running services, file shares, printers, network configuration, DNS settings, DHCP scopes, RDS licensing, scheduled tasks, and Hyper-V VMs.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a new PowerShell script Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin/Invoker
participant Script as Invoke-ServerLifecycleAudit.ps1
participant Host as Local Host OS
participant AD as Active Directory / netdom
participant Registry as Uninstall Registry
participant Services as Windows Services
participant HyperV as Hyper-V / Get-VM
Admin->>Script: start (interactive or RMM)
Script->>Host: start transcript (compute path)
Script->>Host: gather host OS & domain role
Script->>AD: query FSMO roles (if netdom)
Script->>Host: collect CPU, memory, disk, SCSI
Script->>Services: list auto-start & SQL-related services
Script->>Registry: enumerate installed software (filtered)
Script->>Host: list SMB shares, printers, network adapters, IPv4
Script->>Host: query DNS/DHCP/RDS (conditional cmdlets)
Script->>HyperV: list VMs (if Get-VM)
Script->>Host: list scheduled tasks
Script->>Host: stop transcript
Script-->>Admin: AUDIT COMPLETE (transcript path)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
msft-windows/msft-windows-vm-lifecycle-audit (1)
1-5: Add transcript logging and an explicit RMM simulation path.The script currently has no transcript lifecycle, which makes audit traceability harder during field execution and validation.
Proposed logging/testability addition
+# Transcript path under %WINDIR%\logs +$logDir = Join-Path $env:WINDIR "logs" +$logFile = Join-Path $logDir ("vm-lifecycle-audit-{0:yyyyMMdd-HHmmss}.log" -f (Get-Date)) +if (-not (Test-Path $logDir)) { New-Item -Path $logDir -ItemType Directory -Force | Out-Null } +Start-Transcript -Path $logFile -Force + +# Optional RMM simulation switch +$RMM = 0 +# Set $RMM = 1 and predefine required variables during testing + try { # existing audit logic... } finally { + Stop-Transcript | Out-Null }Based on learnings: “Verify transcripts/logs in %WINDIR%\logs after execution; support RMM simulation by setting $RMM=1 and predefining required variables during testing”.
Also applies to: 65-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@msft-windows/msft-windows-vm-lifecycle-audit` around lines 1 - 5, Add PowerShell transcript start/stop around the main script execution by invoking Start-Transcript and Stop-Transcript and write the transcript to $env:windir\Logs with a timestamped filename so audit traces land in %WINDIR%\Logs; also add explicit test-mode/RMM simulation support by declaring and checking a $RMM variable (e.g., default $RMM = 0) and, when $RMM -eq 1, predefine or mock required input variables and skip any destructive operations to allow running under RMM testing; ensure Start-Transcript is invoked early (before major actions) and Stop-Transcript in a finally/cleanup path so transcripts are always closed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@msft-windows/msft-windows-vm-lifecycle-audit`:
- Around line 1-65: The script lacks the required three-part PowerShell template
(RMM variable declaration, input handling, script logic) so preserve backward
compatibility by adding the RMM variable declaration block and input parsing
before any immediate execution (before the first Write-Host), implement the
input handling/parameter mapping that mirrors script-template-powershell.ps1
(declare expected RMM variables and parse parameters/env), and then move the
existing audit commands into a clearly delimited "script logic" section;
reference the template name script-template-powershell.ps1 and the existing
top-level execution points (the initial Write-Host and subsequent audit command
blocks) to locate where to insert the RMM variables and input handling.
- Around line 12-14: The netdom call in the FSMO block (netdom query fsmo)
currently relies on try/catch which won't catch non-zero exit codes from the
native executable; update the block that calls netdom to capture its output,
examine $LASTEXITCODE immediately after the call, and handle non-zero values by
logging a clear message via Write-Host (including the captured output/error)
instead of silently relying on the catch. Ensure the modified logic still
handles the non-DC case and unavailability of netdom by checking for
$LASTEXITCODE and/or specific error text and emitting a concise failure message.
---
Nitpick comments:
In `@msft-windows/msft-windows-vm-lifecycle-audit`:
- Around line 1-5: Add PowerShell transcript start/stop around the main script
execution by invoking Start-Transcript and Stop-Transcript and write the
transcript to $env:windir\Logs with a timestamped filename so audit traces land
in %WINDIR%\Logs; also add explicit test-mode/RMM simulation support by
declaring and checking a $RMM variable (e.g., default $RMM = 0) and, when $RMM
-eq 1, predefine or mock required input variables and skip any destructive
operations to allow running under RMM testing; ensure Start-Transcript is
invoked early (before major actions) and Stop-Transcript in a finally/cleanup
path so transcripts are always closed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aec60540-d5ba-453e-a146-58ab3c2fd743
📒 Files selected for processing (1)
msft-windows/msft-windows-vm-lifecycle-audit
Updated the server lifecycle audit script to enhance functionality and output formatting.
Gumbees
left a comment
There was a problem hiding this comment.
Summary
Adds a comprehensive Windows server lifecycle audit script: OS, domain role, FSMO roles, CPU/memory/disk, server roles, SQL services, installed software (filtered), services, file shares, printers, network, DNS, DHCP, RDS licensing, scheduled tasks, Hyper-V VMs. Designed for replacement quote QA.
What works
- Follows the template's three-section structure (RMM Variables / Input Handling / Script Logic) with
$RMMmode detection,Start-Transcript/Stop-Transcript,$LogPathderived from$RMMScriptPathwith$ENV:WINDIR\logs\fallback, and$Descriptionaudit trail. This is the cleanest template adherence in the current PR backlog. - Graceful capability detection on every optional cmdlet (
Get-WindowsFeature,Get-SmbShare,Get-DnsServerZone,Get-DhcpServerv4Scope,Get-RDLicenseConfiguration,Get-VM) ... script runs on workstations, DCs, file servers, Hyper-V hosts without explosion. - Section banners use plain ASCII (
===== HOSTNAME & OS =====) which renders consistently across RMM consoles. - Read-only ... no filesystem or registry writes.
Concerns
BLOCKER: Filename has no .ps1 extension. The diff adds msft-windows/msft-windows-vm-lifecycle-audit ... no extension. PowerShell will not execute this as a script. Same root issue as PRs #43, #44, #48. Rename to msft-windows/msft-windows-vm-lifecycle-audit.ps1.
Filename mismatch with $ScriptLogName. File is msft-windows-vm-lifecycle-audit but $ScriptLogName = "ServerLifecycleAudit.log". Per the kebab-case convention in PR #51 the log name should be msft-windows-vm-lifecycle-audit.log to match the script name. Minor but worth fixing in the same pass.
Path checks use the template's inverted fallback. Line in the diff: if ($null -ne $RMMScriptPath) { $LogPath = "$RMMScriptPath\logs\$ScriptLogName" }. That's correct ... but worth noting this is the OPPOSITE of the bug in the template (which has if ($null -eq $RMMScriptPath)). PR #53 fixes the template. Good catch that this script got it right; just flagging that future scripts copying the template will need to know.
Suggestions
Get-CimInstance Win32_Service | Where-Object { $_.PathName -notmatch 'Windows|Microsoft|svchost' }will miss third-party services living inC:\Windows\(e.g. Veeam Backup Service) and include false positives (anything with "Microsoft" in the path even if it's a third-party tool linked against MS libraries). Consider usingGet-Servicefiltered byStartType -eq 'Automatic'with the path filter as a refinement, not the primary gate.- Same suggestion as #48: BookStack Page 1787 should be a clickable link, not a bare ID.
- Consider structured output (JSON/CSV) alongside the transcript so the audit can feed a quote tool programmatically.
Verdict
REQUEST CHANGES ... .ps1 extension is the only hard blocker. Rename the file, optionally update $ScriptLogName to match, and this is approveable. The script body is the most template-compliant in the current backlog.
This script performs a comprehensive audit of server lifecycle aspects including OS details, domain roles, CPU, memory, disk layout, server roles, SQL instances, installed software, running services, file shares, printers, network configuration, DNS settings, DHCP scopes, RDS licensing, scheduled tasks, and Hyper-V VMs.
Summary by CodeRabbit