Skip to content

Latest commit

 

History

History
170 lines (143 loc) · 10.2 KB

File metadata and controls

170 lines (143 loc) · 10.2 KB

Architecture review & improvement proposals

A review of the vCheck/VCF Checker framework as it stands after the PowerShell 7 + VCF 9 modernization, with prioritized improvements. Grounded in the actual code (engine vCheck.ps1, vCheckUtils.ps1, the connection plugin, the ~145 plugins, the style engine, and the settings mechanism).

How it works today

vCheck.ps1 (engine)
 ├─ discovers Plugins/**/*.ps1, orders Initialize → categories → Finish
 ├─ dot-sources GlobalVariables.ps1 (config) and Styles/<Style>/Style.ps1
 ├─ 000 Connection plugin: connects, pre-collects inventory into script-scope
 │   globals ($VM,$VMH,$Clusters,$Datastores,$FullVM,$HostsViews,$clusviews,
 │   $storageviews,$valarms…) and defines helpers (Get-VIEventPlus, …)
 ├─ for each plugin: dot-source it, capture stdout as $Details, read its magic
 │   variables ($Title,$Header,$Display,$Comments,$TableFormat,$PluginVersion…)
 ├─ render: Get-HTMLTable/List → style token replacement → one HTML document
 └─ deliver: display / MailKit email / EndScript

Contract: a plugin is a .ps1 that (a) writes objects to stdout (the report rows) and (b) sets a set of magic variables the engine reads afterward.

Strengths

  • Low barrier to extend: a plugin is a tiny script; the community wrote ~150.
  • "Report only problems" ethos: empty output = no section; keeps reports signal-dense.
  • Pluggable styles: the report renderer is swappable (Get-ReportHTML).
  • Pre-collected inventory: the connection plugin amortizes expensive queries so plugins can reuse $VM/$VMH/$HostsViews instead of re-querying (mostly honored).
  • i18n via Import-LocalizedData.

Architectural debt (observed)

  1. Implicit magic-variable contract. Metadata is plain script variables with no schema and order-sensitivity (e.g. $Header built before the value it interpolates; metadata declared after the body). It works only because the engine reads the variables after execution. Fragile and hard to validate.
  2. Global mutable state with no isolation. Plugins share one script scope. A plugin can clobber another's variables, and (critically) several VCSA plugins call exit() in a dead version branch, which would terminate the entire run, not just the plugin. There is no per-plugin error boundary.
  3. Settings are stored by rewriting source files. Invoke-Settings / Import-vCheckSettings parse each plugin by line number and overwrite the .ps1 to bake in values. Meanwhile Get-vCheckSetting is a no-op stub (return $default), so runtime setting overrides do nothing. This blocks GitOps/multi-instance, mutates tracked source, and is brittle (line-number parsing).
  4. No first-class severity. Severity exists only as per-cell CSS classes applied by $TableFormat rules. The new scorecard/TOC infer severity by regex-scanning rendered HTML, a workaround. There's no per-finding severity, so no clean filtering, ranking, thresholds, or "problems only" enforcement.
  5. Sequential execution. The plugin loop runs one at a time; runtime is dominated by PowerCLI/Get-View round-trips (88 Get-View, 68 Get-CisService). No parallelism.
  6. Invoke-Expression on data. Used for settings eval, table format-rule evaluation (operates on report cell values), and style token replacement: an injection surface and a source of subtle bugs (the $-as-backreference issue fixed this run).
  7. Not a module. Dot-sourced scripts, no manifest, not on PSGallery, so no versioned distribution, discovery, or import isolation.
  8. HTML is the only output. No machine-readable result, so the data can't feed a dashboard, alerting, or ticketing system without scraping HTML.
  9. Monolithic, unconditional inventory collection. The connection plugin always collects the full inventory (all VMs, all host views, …) even when few plugins are enabled; the cost is paid on every run.

Improvement proposals (prioritized)

P1: high value, enables everything else

  • First-class severity + structured results. Give plugins a real contract: emit objects plus a per-row/per-plugin severity (OK/Warning/Critical) or a threshold declaration, and have the engine produce a structured result object (which the HTML style, a JSON export, and the scorecard all consume). Removes the HTML-regex hack and unlocks filtering, ranking, and real alerting. (Pairs with the new style's scorecard.)
  • External settings store. Replace file-rewriting with a per-config JSON (or .psd1) settings file, and make Get-vCheckSetting actually read it. Source files stop being mutated; enables GitOps, multiple environments, and the -job model cleanly.

P2: performance & safety

  • Parallel plugin execution. PS7 ForEach-Object -Parallel / runspaces, passing the pre-collected inventory in as a context object instead of script-scope globals. Given the Get-View/CIS round-trip dominance, this is potentially a multiplicative speedup. Prerequisite: decouple the global state (below).
  • Per-plugin error boundary + ban exit. Wrap each plugin in try/catch, surface failures as a report row instead of aborting, and forbid exit() (lint rule). One misbehaving plugin should never kill the run.
  • Replace Invoke-Expression. Parse comparisons/format rules explicitly; dispatch style tokens via a function map. Closes the injection surface.

P3: modernization & packaging

  • Module-ize. Ship a vCheck/VCFChecker module (.psm1 + manifest) exporting the engine; plugins remain data. Publish to PSGallery for versioned install. (CI + Pester added this run are the foundation.)
  • Adopt VCF API token auth. VCF.PowerCLI 9.1 added token auth to Connect-VIServer; wire it through the connection plugin for modern, non-interactive, secret-manager- friendly credentials (complements the env-var/credential-store path already added).
  • Context-object inventory, collected lazily. Collect only what enabled plugins declare they need (a RequiresInventory hint), or cache per object type on first use.
  • Secret management. Integrate PowerShell SecretManagement for the SMTP and vCenter credentials (the plaintext $SMTPPassword/VCHECK_PASSWORD paths are interim).

P4: quality of life

  • Manifest-based plugin metadata (comment-based-help or .psd1 sidecar) to retire the order-sensitive magic variables.
  • Structured logging (Write-CustomOut → a proper logger with levels), and a --format json|html switch so the same run can emit both.

Suggested sequencing vs the phase plan

These map onto the existing phases: the severity model + structured results belongs with the new style (Phase 4/8); the settings store, modular packaging, and parallelism are natural Phase 8/10 work; token auth and the VCSA/vSAN ESA rewrites are Phase 7 (lab). None of them block the current v9.0.0 line; they're the roadmap past it (a v9.1/v10 architecture track).

Status update (2026-06-05, alpha.46)

Partially addressed since this review was written:

  • Debt #2 (exit() killers): fixed across the enabled set (alpha.23, with a regression test) and the disabled set (alpha.46). The per-plugin error boundary itself still does not exist → stays on the v9.1 track.
  • Debt #5 (round-trip dominance): Phase 9 (alpha.44) batched the worst offenders (Get-Stat, alarm views, module import, esxcli software.* → SDK); full run 200s → 110s. The loop itself is still sequential → parallelism stays on the v10 track (requires the context-object decoupling).
  • P3 token auth: researched (alpha.43): New-VcfOAuthSecurityContext + Connect-VIServer -VcfOAuthSecurityContext is the wire-in point.
  • ELM/AllLinked: removed entirely (alpha.43); vCenter Groups documented.

Post-9.0 roadmap (architecture track)

Sequencing principle: split by whether the plugin contract breaks. Ship v9.0.0 first (stable, measured baseline), then:

v9.1: non-breaking engine work (each item independently shippable)

  • Settings store: implement Get-vCheckSetting against a JSON/psd1 config file (today it is a stub returning the default). Backward compatible: every plugin already calls it. Retires the file-rewriting settings system; enables GitOps/multi-env.
  • Per-plugin error boundary: engine-level try/catch; a failing plugin becomes a report row instead of damaging the run.
  • Replace Invoke-Expression in table-format evaluation and style token replacement (explicit parsing / function map), closing the injection surface.
  • JSON output alongside HTML (-Format json|html): machine-readable results for dashboards/alerting without scraping.
  • SecretManagement + VCF token auth (New-VcfOAuthSecurityContext) in the connection plugin, replacing plaintext $SMTPPassword/VCHECK_PASSWORD paths.

v10: contract-breaking architecture (needs its own design doc first)

  • Severity-first structured results: plugins emit per-finding severity; engine produces a structured result object consumed by HTML style, JSON export, scorecard (retires the style's regex-over-rendered-HTML severity inference).
  • Context object + parallel plugin execution: kill shared script-scope globals, pass pre-collected inventory explicitly, then runspace-pool the plugin loop.
  • Module-ization + PSGallery packaging; manifest-based plugin metadata replaces the order-sensitive magic variables.

Research track (no version assigned): VCF Operations as a data source

  • See OPERATIONS.md. Consume VCF Operations /suite-api for verdicts (alerts, findings, capacity forecast, reclamation) rather than re-deriving them from vCenter, via the VMware.Sdk.Vcf.Ops module that VCF.PowerCLI 9.1 already ships. Strictly additive: absent or unreachable Operations must leave today's report unchanged. No longer blocked: lab-verified 2026-08-31: the resource join works via (VMEntityVCID, VMEntityObjectID), measured cost is 8.54s (5.4s of it the module import), and 47 of 75 active lab alerts join to vCenter objects. The first slice shipped in v9.0.5; what is still open is listed in OPERATIONS.md.