081f7e1d16
- plans/archived/2026-05-13/: final-master-plan (done), 4 AI analysis/review docs (gpt55-analysis, gpt55-review-of-opus, opus47-analysis, opus47-review-of-gpt55) - docs/archived/2026-05-13/: TEST-PLAN-v1.3-to-HEAD (sprint complete)
493 lines
53 KiB
Markdown
493 lines
53 KiB
Markdown
# Technical Review: Local CI/CD System
|
|
|
|
Date: 2026-05-11
|
|
|
|
Scope reviewed: all requested project files in the workspace, including orchestration scripts, template provisioning scripts, runner configuration, workflows, Pester tests, and operational documentation.
|
|
|
|
## 1. Executive Summary
|
|
|
|
This project is a self-hosted homelab CI/CD platform built around Gitea Actions, act_runner, VMware Workstation linked clones, Windows PowerShell 5.1, WinRM for Windows guests, and SSH for Linux guests. The core idea is sound: keep the Windows host as a thin orchestrator, create a fresh linked clone for each job, run the build inside an isolated VM, collect artifacts, and destroy the clone in a `finally` block. For a single-host homelab, this is the right architecture. It uses the tools already available on the host, avoids containerizing workloads that need full Windows build tooling, and gives repeatable isolation without introducing Kubernetes, ESXi, or unnecessary service layers.
|
|
|
|
Overall, the system is well-built for its intended environment. The strongest parts are the lifecycle decomposition, the cold-template workflow, the PS5.1 discipline in the main scripts, the Windows template hardening/provisioning gates, and the Linux cloud-init/open-vm-tools work. The project has clearly gone through real failure-driven refinement: the notes about VMware Tools timing, cloud-init BOM issues, headless `getGuestIPAddress`, WinRM HTTPS, snapshot state, and Linux netplan MAC matching are exactly the kind of details that only appear after serious testing.
|
|
|
|
I would not yet call it fully trustworthy for unattended daily use without a small stabilization pass. The core VM lifecycle is close, but a few issues are operationally important: the composite action output block is malformed, Linux secret injection can leak secrets to logs and process arguments, host bootstrap and backup defaults drift from the current directory layout, stale `vm-start.lock` is not automatically cleared, and some documentation/test-plan commands no longer match the actual parameter names or PS5.1 constraint. These are fixable, homelab-scale problems, not architectural failures.
|
|
|
|
## 2. Project State Assessment
|
|
|
|
Genuinely done:
|
|
|
|
- Windows VM lifecycle exists end to end: [scripts/New-BuildVM.ps1](../scripts/New-BuildVM.ps1), [scripts/Wait-VMReady.ps1](../scripts/Wait-VMReady.ps1), [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1), [scripts/Get-BuildArtifacts.ps1](../scripts/Get-BuildArtifacts.ps1), and [scripts/Remove-BuildVM.ps1](../scripts/Remove-BuildVM.ps1) form a coherent pipeline.
|
|
- Linux build VM support is implemented, not just planned. [template/Deploy-LinuxBuild2404.ps1](../template/Deploy-LinuxBuild2404.ps1), [template/Prepare-LinuxBuild2404.ps1](../template/Prepare-LinuxBuild2404.ps1), [template/Install-CIToolchain-Linux2404.sh](../template/Install-CIToolchain-Linux2404.sh), [scripts/_Transport.psm1](../scripts/_Transport.psm1), and the Linux branches in [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1), [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1), and [scripts/Get-BuildArtifacts.ps1](../scripts/Get-BuildArtifacts.ps1) are real.
|
|
- WinRM HTTPS migration is implemented in the active scripts. The Windows paths use `-UseSSL -Port 5986 -Authentication Basic` and a shared `New-CISessionOption` helper in [scripts/_Common.psm1](../scripts/_Common.psm1).
|
|
- The current orchestrator has a real clone/start/IP serialization lock and lease file defense in [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1). This is stronger than some older documentation implies.
|
|
- Scheduled maintenance scripts exist: cleanup, retention, disk alert, runner health, and scheduled task registration are all present.
|
|
- Unit tests exist and cover the shared module, clone creation, removal, and readiness timeout/validation behavior.
|
|
- The Windows and Linux templates have serious pre-snapshot validation gates. They are not just installation scripts; they assert the state they intend to snapshot.
|
|
|
|
Documented as done but not fully validated or internally inconsistent:
|
|
|
|
- The README claims production-ready status, while [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md) still marks several real-VM validations as skipped or partial. The honest state is "feature-complete, with remaining real-VM validation and workflow defects." The README is ahead of the evidence.
|
|
- The composite action is documented as reusable and complete, but [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml) places `artifact-path` and the second `artifact-name` under `inputs` instead of a top-level `outputs` block. That can break artifact upload steps that use `steps.invoke-ci.outputs.*`.
|
|
- The test plan contains PS7-only `ForEach-Object -Parallel`, even though the project standard is PS5.1 only. This is a documentation/test executability issue, not a production-script syntax issue.
|
|
- [docs/CI-FLOW.md](../docs/CI-FLOW.md) and [docs/ARCHITECTURE.md](../docs/ARCHITECTURE.md) describe some older mechanics such as `getGuestIPAddress` as the main flow, while [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) now primarily reads `guestinfo.ci-ip` and uses `getGuestIPAddress` as fallback.
|
|
- [docs/CI-FLOW.md](../docs/CI-FLOW.md) says partial artifacts are collected after build failure, but the current orchestrator enters `catch` and cleanup directly; artifact collection happens only after successful `Invoke-RemoteBuild.ps1`.
|
|
- TODO says some security tasks are deferred even though corresponding implementation exists. The `PAT mai persistito` item still says deferred, but the Windows path has an implementation and the Linux path has a problematic implementation.
|
|
- [Setup-Host.ps1](../Setup-Host.ps1) still creates `F:\CI\Templates\WinBuild`, not the current `WinBuild2025`, `WinBuild2022`, and `LinuxBuild2404` structure. It also does not create `F:\CI\State`, `F:\CI\keys`, or `F:\CI\Cache\pip`.
|
|
|
|
## 3. Architecture Analysis
|
|
|
|
The overall design quality is high for a homelab system. The host-as-orchestrator model is appropriate: act_runner runs workflow logic on the host, but build commands execute inside disposable VMs. This gives a strong isolation boundary without the cost of an enterprise virtualization stack. The template and linked-clone model is the right tradeoff for VMware Workstation: fast clone creation, small per-job disk footprint, and deterministic toolchain state.
|
|
|
|
VM lifecycle management is mostly sound. [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) owns the lifecycle and wraps destructive cleanup in `finally`. Clone creation is delegated to [scripts/New-BuildVM.ps1](../scripts/New-BuildVM.ps1), readiness to [scripts/Wait-VMReady.ps1](../scripts/Wait-VMReady.ps1), build execution to [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1), artifacts to [scripts/Get-BuildArtifacts.ps1](../scripts/Get-BuildArtifacts.ps1), and teardown to [scripts/Remove-BuildVM.ps1](../scripts/Remove-BuildVM.ps1). This is a good separation of concerns. The teardown script has retry logic around `deleteVM`, which is useful because VMware often releases locks late.
|
|
|
|
The weak point in lifecycle management is bounded execution. The primary orchestrator still calls `vmrun start`, `readVariable`, `getGuestIPAddress`, and other native tools directly without a general timeout wrapper. Some deployment scripts have `Invoke-VmrunBounded`, but the active CI scripts do not use a common bounded process helper. If `vmrun` hangs, the runner slot can hang until act_runner job timeout, and cleanup may never run. This is the highest reliability gap left in the core lifecycle.
|
|
|
|
The transport duality is clean enough but not fully abstracted. Windows and Linux branches are explicit in the top-level scripts, which is a good homelab choice: the code is understandable, and the two transports differ enough that over-abstracting would make troubleshooting harder. [scripts/_Transport.psm1](../scripts/_Transport.psm1) covers SSH helpers only, while [scripts/_Common.psm1](../scripts/_Common.psm1) covers WinRM session options and vmrun. This is acceptable, but function naming could be clearer: `_Transport.psm1` sounds like a cross-transport abstraction even though it is SSH-specific.
|
|
|
|
The IP allocation mechanism is better than the TODO summary suggests. The actual implementation serializes clone/start/IP detection through `F:\CI\State\vm-start.lock`, detects IP through `guestinfo.ci-ip` when available, writes an IP lease file, and releases it in `finally`. This reduces the practical risk of two concurrent builds talking to the same VM. However, it is not true deterministic IP allocation. It still depends on DHCP, detects collisions after the fact, and a stale `vm-start.lock` from a host crash is not automatically pruned by the scheduled tasks. For capacity 4, it is probably reliable enough after one fix: automatic stale lock cleanup.
|
|
|
|
Credential management is pragmatic but mixed. Windows guest credentials use Windows Credential Manager via `BuildVMGuest`, which is appropriate. Linux uses a host-side private key at `F:\CI\keys\ci_linux`, also appropriate for a homelab, provided ACLs are correct. The weakest credential path is `-UseGitClone` and extra guest environment variables. The Windows `git -c http.extraHeader=Authorization: Basic ...` approach avoids putting credentials in the clone URL, but the header value still appears in the guest process command line. The Linux `GIT_CLONE_URL` implementation embeds a credential-bearing URL into the SSH command string. The Linux extra-env path logs the composed build command including exported secret values. These need fixing before secret-bearing builds are safe.
|
|
|
|
Script interdependencies are reasonable. [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) is intentionally the hub. The coupling to hardcoded host paths is expected for a fixed homelab, but inconsistent defaults undercut that: [runner/config.yaml](../runner/config.yaml), [Setup-Host.ps1](../Setup-Host.ps1), [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1), and docs do not all agree on template paths. A single path contract in [AGENTS.md](../AGENTS.md) plus corrected defaults would make the system easier to rebuild.
|
|
|
|
The module organization is helpful but incomplete. [scripts/_Common.psm1](../scripts/_Common.psm1) covers three high-value helpers, but many scripts still implement local `Invoke-Vmrun`, `Assert-Step`, `Set-VmxKey`, ISO writing, and polling patterns. For template deployment scripts, duplication is acceptable because they are long, one-off provisioning flows. For production CI scripts, move bounded native process execution, IPv4 validation, and stale state cleanup into `_Common.psm1`.
|
|
|
|
The host-as-orchestrator pattern has clear pros: simple mental model, native access to VMware Workstation, easy file-based state, easy Event Log and Scheduled Task integration, and no new services. The cons are also clear: host filesystem paths leak into workflows, host compromise compromises the whole CI system, concurrency depends on local file locking, and observability is local unless forwarded. For this homelab, the pros outweigh the cons.
|
|
|
|
## 4. Code Quality and PS5.1 Compliance
|
|
|
|
Project-wide PS5.1 compliance is mostly good in production PowerShell. I did not find actual PS7 null-coalescing, ternary, or `ForEach-Object -Parallel` in the production `.ps1` scripts that were reviewed. Bash `&&` appears inside commands intentionally executed by Linux guests; that is acceptable. The major PS5.1 compliance problem is in documentation/test-plan snippets, especially [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md), which uses `ForEach-Object -Parallel`.
|
|
|
|
[scripts/_Common.psm1](../scripts/_Common.psm1): Good minimal shared module. PS5.1 compliant. Error handling in `Invoke-Vmrun` checks `$LASTEXITCODE` and optionally throws. Gap: no timeout support, no standard native command result object with elapsed time, and no shared IPv4 validator.
|
|
|
|
[scripts/_Transport.psm1](../scripts/_Transport.psm1): Clear SSH/SCP wrapper. PS5.1 compliant. It uses `StrictHostKeyChecking=no` and `UserKnownHostsFile=NUL`, which is operationally convenient but weakens MITM protection. Error handling checks exit codes. Gap: it can throw command text containing secrets when callers pass secret-bearing commands.
|
|
|
|
[scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1): Strong orchestration structure and good `finally` cleanup. PS5.1 compliant. Smells: many hardcoded `F:\CI` paths despite parameters for others; direct `vmrun` calls instead of `_Common.psm1` wrapper; stale `vm-start.lock` behavior causes a 10-minute delay then failure; JSONL logging is best-effort but lacks duration calculations; artifact collection is skipped on build failure. Parameter validation gaps: `JobId` is not sanitized before use in paths, so hostile or malformed IDs could create awkward paths. `ExtraGuestEnv` keys are documented as valid env names but not validated in the orchestrator.
|
|
|
|
[scripts/New-BuildVM.ps1](../scripts/New-BuildVM.ps1): Simple and solid. Uses `_Common.psm1`. PS5.1 compliant. Gap: `JobId` is directly used in clone folder names; add a conservative validation pattern or sanitize to `[A-Za-z0-9_.-]`. It does not preflight snapshot existence; `vmrun clone` failure is probably enough, but a `listSnapshots` check could improve diagnostics.
|
|
|
|
[scripts/Remove-BuildVM.ps1](../scripts/Remove-BuildVM.ps1): Practical cleanup with `SupportsShouldProcess`. PS5.1 compliant. Smell: `$ErrorActionPreference = 'Continue'` makes it resilient but can hide failures. It warns if the directory remains, but does not emit structured state or Event Log. Direct `vmrun` calls lack timeout. It can still hang on `getState`, `list`, or `deleteVM` if VMware blocks.
|
|
|
|
[scripts/Wait-VMReady.ps1](../scripts/Wait-VMReady.ps1): Good improvement over `getGuestIPAddress` for power-state detection by using `vmrun list`. PS5.1 compliant. The Windows readiness check only tests TCP 5986, not an authenticated WinRM session, so it can declare ready before credentials/remoting policy are actually usable. This is acceptable for speed but can shift failure into Phase 5. The docblock still references older `getState`/`Test-WSMan` concepts.
|
|
|
|
[scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1): Functionally rich but now the densest and riskiest script. PS5.1 compliant. Good: mode validation, WinRM/SSH separation, 7-Zip fallback, and Linux artifact staging. Problems: Linux secret values are logged through `Write-Host "Running build: $buildCmd"`; Linux `GIT_CLONE_URL` embeds PAT in a remote command string; Windows PAT is passed to `git -c http.extraHeader=...`, which can be visible in guest process command line; `ExtraGuestEnv` keys are not validated; custom Windows build command uses `cmd /c "$cmd 2>&1"`, so quoting/injection is controlled only by trusted workflow inputs. For a trusted homelab this is tolerable, but it contradicts the stated secret-safety claims.
|
|
|
|
[scripts/Get-BuildArtifacts.ps1](../scripts/Get-BuildArtifacts.ps1): Clear and mostly robust. PS5.1 compliant. Windows branch retries WinRM session creation. Linux branch copies a directory recursively and validates at least one file. Gap: no manifest or artifact metadata is written, and failure diagnostics from guest output directories are limited.
|
|
|
|
[scripts/Cleanup-OrphanedBuildVMs.ps1](../scripts/Cleanup-OrphanedBuildVMs.ps1): Good simple orphan cleanup. PS5.1 compliant. Gap: does not clear stale `F:\CI\State\vm-start.lock`; does not remove stale IP leases unless retention runs; uses directory `LastWriteTime`, which can be misleading if VMware updates files late or not at all.
|
|
|
|
[scripts/Invoke-RetentionPolicy.ps1](../scripts/Invoke-RetentionPolicy.ps1): Useful and safe. PS5.1 compliant. It cleans artifacts, logs, and stale IP leases. Gap: hardcoded leases dir and no cleanup for `vm-start.lock`. It does not write Event Log entries, despite some test-plan expectations.
|
|
|
|
[scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1): Good minimal disk monitor. PS5.1 compliant. Gap: webhook content includes emoji despite the user's stated no-emoji preference; more importantly, the Event Log source is `CI-DiskAlert`, but the test plan expects other names. Exit code 1 on low disk is fine for Scheduled Task visibility.
|
|
|
|
[scripts/Watch-RunnerHealth.ps1](../scripts/Watch-RunnerHealth.ps1): Good rate-limited restart design. PS5.1 compliant. Similar webhook emoji issue. It checks only local Windows service status, not whether Gitea sees the runner online. For daily use, add optional API check later.
|
|
|
|
[scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1): Conceptually important, but defaults are stale. It backs up `F:\CI\Templates\WinBuild`, while the current primary template is `F:\CI\Templates\WinBuild2025`. That makes the default backup procedure fail or back up the wrong thing. High operational risk before template refresh.
|
|
|
|
[scripts/Register-CIScheduledTasks.ps1](../scripts/Register-CIScheduledTasks.ps1): Good, idempotent scheduled task registration. PS5.1 compliant. Gaps: default `ScriptRoot` is hardcoded to `N:\Code\Workspace\Local-CI-CD-System\scripts`; acceptable on this host but not portable. Task names in docs/test plan do not always match actual registered names (`CI-DiskSpaceAlert` versus `CI-DiskSpace`).
|
|
|
|
[scripts/Set-TemplateSharedFolders.ps1](../scripts/Set-TemplateSharedFolders.ps1): Clean idempotent VMX editor. PS5.1 compliant. Risk: shared folders are writable from guest to host; this is useful for caches but weakens isolation. Use only for cache directories and never map broad host paths.
|
|
|
|
[scripts/Measure-CIBenchmark.ps1](../scripts/Measure-CIBenchmark.ps1): Useful baseline tool. PS5.1 compliant. Gaps: it still uses `getGuestIPAddress` rather than the newer `guestinfo.ci-ip` path, so Linux timing may not match the production orchestrator. JSONL structure differs from the test plan's expected `phase` and `duration_ms` fields.
|
|
|
|
[Setup-Host.ps1](../Setup-Host.ps1): Helpful bootstrap, but drifted. It creates old template paths, does not create Linux key/state/cache directories, registers only `windows-build:host,dotnet:host,msbuild:host` labels during runner registration, and leaves Linux label only in config. It uses plaintext guest password as a parameter, although default is empty and prompt is preferred. It should be corrected before claiming rebuild-from-scratch reliability.
|
|
|
|
[runner/Install-Runner.ps1](../runner/Install-Runner.ps1): Marked deprecated. It is fine as reference but should not be presented as an active path. It lacks newer labels and may drift further.
|
|
|
|
[template/Deploy-WinBuild2025.ps1](../template/Deploy-WinBuild2025.ps1) and [template/Deploy-WinBuild2022.ps1](../template/Deploy-WinBuild2022.ps1): Sophisticated and PS5.1 compliant. Good use of bounded vmrun for stop. Risks: admin password and autologin password are embedded into generated unattend/post-install content and registry. That is accepted in the lab threat model but should be clearly treated as snapshot secret material. Both scripts duplicate a lot of logic; tolerable, but fixes must be applied to both.
|
|
|
|
[template/Prepare-WinBuild2025.ps1](../template/Prepare-WinBuild2025.ps1): Strong validation and reboot loop for Windows Update. PS5.1 compliant. Operational smell: it prompts interactively for snapshot creation and deletion, which makes fully unattended template refresh harder. It also uses `ConvertTo-SecureString -AsPlainText` for admin password conversion, which is expected when bridging from a string parameter but will trigger static warnings.
|
|
|
|
[template/Install-CIToolchain-WinBuild2025.ps1](../template/Install-CIToolchain-WinBuild2025.ps1): Very detailed and mostly well engineered. PS5.1 compliant. Main debt: many installer SHA256 values are empty, so the supply-chain pinning structure mostly warns rather than enforces. It also uses a public KMS endpoint by default, which is not appropriate outside a private lab or unless the user has explicitly chosen that activation model.
|
|
|
|
[template/Deploy-LinuxBuild2404.ps1](../template/Deploy-LinuxBuild2404.ps1): Good cloud-init implementation with BOM/LF handling and guestinfo fallback. PS5.1 compliant. Gap: no hash verification for downloaded Ubuntu cloud VMDK. A corrupted or malicious base image would poison every Linux build.
|
|
|
|
[template/Prepare-LinuxBuild2404.ps1](../template/Prepare-LinuxBuild2404.ps1): Practical SSH provisioning. PS5.1 compliant. It validates only a loose IPv4 pattern, not per-octet. It uses remote bash `&&` and `||` inside strings, which is correct for Linux. It uses `StrictHostKeyChecking=no` in several places despite comments mentioning accept-new.
|
|
|
|
[template/Install-CIToolchain-Linux2404.sh](../template/Install-CIToolchain-Linux2404.sh): Clean bash script with `set -euo pipefail`. It installs a solid basic build environment. Gap: no apt package version pinning, no snapshot of apt sources, and no retry wrapper for apt transient failures.
|
|
|
|
[template/Validate-DeployState.ps1](../template/Validate-DeployState.ps1) and [template/Validate-SetupState.ps1](../template/Validate-SetupState.ps1): Useful validation scripts. Docblocks still say WinRM HTTP/Basic even though the implementation uses HTTPS/5986. [template/Validate-SetupState.ps1](../template/Validate-SetupState.ps1) assumes `PasswordExpires` exists, while the install script already handles property variation; this can fail on different LocalAccounts module versions.
|
|
|
|
## 5. Security Analysis (Homelab-realistic)
|
|
|
|
WinRM with self-signed certificate plus Basic auth is acceptable in this lab if VMnet8 is isolated and the host is trusted. The important improvement is that `AllowUnencrypted=false` and traffic uses HTTPS/5986. Skipping CA/CN/revocation checks is normal for a self-signed lab certificate, but it means the host does not authenticate the VM identity beyond IP and network position. A malicious VM on VMnet8 could theoretically impersonate another VM if the IP handling failed. The current IP lock and lease design reduces but does not eliminate this risk.
|
|
|
|
SSH key storage is acceptable if ACLs on `F:\CI\keys\ci_linux` restrict access to the runner service identity and Administrators. The repo documents the path but does not include an ACL validation script. Add one. Because the key has no passphrase, filesystem ACLs are the only real protection.
|
|
|
|
PAT handling in `-UseGitClone` is mixed. The Windows path avoids putting credentials in the URL, which is good, but it uses `git -c http.extraHeader=Authorization: Basic <base64>` so the token-derived header may be visible in process arguments inside the guest. The Linux path is worse: it builds a credential-bearing URL and embeds it in a remote command assigned to `GIT_CLONE_URL`. If SSH command logging, PowerShell error messages, or process inspection catches that string, the PAT leaks. The TODO and docs claim PAT safety more strongly than the code supports.
|
|
|
|
Secrets via workflow-level `extra-guest-env-json` are not currently safe on Linux. [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1) builds an `export KEY='value';` prefix and then logs the full build command. That directly prints secret values. Windows is safer because it sets environment variables inside the session and does not print the hashtable values, but it still passes secrets as WinRM arguments.
|
|
|
|
Runner environment variables in [runner/config.yaml](../runner/config.yaml) are not secrets. They are paths and target names. That is fine. Do not add PATs or passwords there. The composite action correctly passes most inputs through env vars to reduce shell injection risk, but `extra-guest-env-json` itself is an env var containing secrets; ensure act_runner does not echo it.
|
|
|
|
Template hardening disables Defender, Firewall, and UAC. For this homelab, this is acceptable only if all builds are trusted. The documentation in [docs/BEST-PRACTICES.md](../docs/BEST-PRACTICES.md) is honest about this. The most important nuance is that a compromised build VM is not just a guest problem: it can reach the NAT network, Gitea, package registries, and any host-exposed VMware shared folders. If shared cache folders are writable, malicious code can poison cache content for future builds.
|
|
|
|
A compromised build VM could likely reach:
|
|
|
|
- Gitea over LAN/NAT if routing allows it.
|
|
- The internet through VMnet8 NAT.
|
|
- Host VMware shared folders configured for caches.
|
|
- WinRM/SSH ports of other concurrently running build VMs if IP/network isolation is not stricter.
|
|
- Potentially host services bound to VMnet8.
|
|
|
|
It should not have host filesystem access beyond configured shared folders, and it should not have the Linux private key or Windows Credential Manager secrets unless scripts explicitly copy them in. Keep it that way.
|
|
|
|
## 6. Reliability and Resilience Analysis
|
|
|
|
VM leak scenarios remain possible but bounded. Normal build failures are covered by `finally`. Host power loss, hung PowerShell process, forced runner termination, or a `vmrun` call that never returns can bypass cleanup. [scripts/Cleanup-OrphanedBuildVMs.ps1](../scripts/Cleanup-OrphanedBuildVMs.ps1) handles stale clone directories, and scheduled tasks run it every 6 hours plus startup. That is good. Add Event Log output and stale lock cleanup to make recovery visible.
|
|
|
|
IP lease race conditions are mostly mitigated by serializing clone/start/IP detection. This is not a true static lease allocator; it is a concurrency gate plus collision detector. For capacity 4, this is probably good enough. The risky case is host crash after creating `vm-start.lock` or an IP lease. IP leases older than 12 hours are cleaned by retention, but the start lock is not. That means a crash can cause every new job to wait 10 minutes and fail until manual cleanup.
|
|
|
|
The error recovery quality in try/finally blocks is good in the main orchestrator. The Windows template prepare script also has careful `finally` restoration of TrustedHosts. The removal script tries soft stop, hard stop, deleteVM retries, and directory removal. However, there is no outer watchdog for stuck native processes. If PowerShell is blocked inside native `vmrun`, `finally` cannot execute until the native process returns.
|
|
|
|
On host reboot mid-build, the expected behavior is: act_runner service restarts, Gitea marks the job failed or times it out, clone directories and possibly running VMs remain, scheduled cleanup removes clones older than threshold, retention later removes stale leases. This is close to acceptable. Add startup cleanup for `vm-start.lock` and leases to avoid avoidable first-job failures after reboot.
|
|
|
|
If `vmrun` hangs, the system is weak. act_runner timeout may eventually terminate the job, but PowerShell may remain stuck and the clone may leak. Some deploy scripts use bounded process execution; move that pattern into [scripts/_Common.psm1](../scripts/_Common.psm1) and use it in production lifecycle scripts.
|
|
|
|
Disk exhaustion is handled by monitoring and retention but not by preflight gates. [scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1) alerts under 50 GB, and [scripts/Invoke-RetentionPolicy.ps1](../scripts/Invoke-RetentionPolicy.ps1) uses aggressive retention under that threshold. But [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) does not check free space before cloning. Add a fast preflight check before Phase 2 so clone failure is clear.
|
|
|
|
Artifact loss scenarios exist. Windows custom build mode packages artifacts only if build exit is 0. Linux mode copies artifact source if it exists and logs a warning if not, but then `Get-BuildArtifacts.ps1` fails only if the output directory is empty. On build failure, [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) does not attempt to collect partial logs/artifacts from the guest before teardown. This hurts failure triage.
|
|
|
|
## 7. CI/CD Workflow Analysis
|
|
|
|
The composite action idea is excellent. A reusable [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml) hides the VM lifecycle and lets application repositories specify only build command, artifact source, guest OS, and repo URL. That is exactly the right abstraction for a homelab with several repositories.
|
|
|
|
The current action has a structural YAML problem: it appears to define outputs under `inputs`, and duplicates `artifact-name`. Composite action outputs should be top-level `outputs`. As written, the `Upload artifacts` step may not receive `steps.invoke-ci.outputs.artifact-path` and `artifact-name` as intended. This can make builds succeed inside the VM but fail at artifact upload.
|
|
|
|
Workflow flexibility is otherwise good. [gitea/workflows/build-nsInnoUnp.yml](../gitea/workflows/build-nsInnoUnp.yml) uses a Windows/Linux matrix and disambiguates job IDs with `job-id-suffix`. That is the right pattern. Missing patterns: PR checks are not represented in the live workflow, scheduled builds are not represented, and dependency caching via `actions/cache` is not shown even though act_runner cache is enabled. For this system, PR checks and manual rebuild are more valuable than scheduled builds.
|
|
|
|
Artifact upload to Gitea is done through `actions/upload-artifact@v4` in the composite action. The VM pipeline collects files into `F:\CI\Artifacts\<jobId>`, then upload-artifact reads that host-side directory. That is a good bridge between local VM isolation and Gitea's artifact store, once the outputs block is fixed.
|
|
|
|
Build cache effectiveness is partial. NuGet/pip shared folders exist for Windows when `-UseSharedCache` is passed, but the composite action does not expose a `use-shared-cache` input and [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) does not forward `UseSharedCache` to [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1). The feature exists at the lower layer but is not wired into the main workflow path.
|
|
|
|
Matrix build design quality is good: `target: [windows, linux]`, `runs-on: ${{ matrix.target }}-build`, `fail-fast: false`, and target-specific artifact names are exactly the right shape.
|
|
|
|
## 8. Observability and Monitoring
|
|
|
|
The JSONL logging design is useful. [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) emits phase start/success/failure events with ISO timestamps, job ID, phase, status, and data. This is enough to compute phase durations after the fact. It is not yet a full metrics layer, but it is the right low-cost foundation.
|
|
|
|
Disk space monitoring is adequate for a homelab. [scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1) writes Event Log warnings and optionally posts a webhook. [scripts/Invoke-RetentionPolicy.ps1](../scripts/Invoke-RetentionPolicy.ps1) can reclaim space. Add a pre-clone free-space gate to turn "cryptic vmrun failure" into "free disk below threshold."
|
|
|
|
Runner health monitoring is useful but local-only. [scripts/Watch-RunnerHealth.ps1](../scripts/Watch-RunnerHealth.ps1) restarts the Windows service with a cooldown. It does not check the Gitea API to prove the runner is online from Gitea's perspective. For daily use, local service status is enough most of the time; a Gitea API check is a phase-2 improvement.
|
|
|
|
After a build failure, you would know the high-level phase and the top-level exception. You might not know enough about guest state because failed builds do not trigger artifact/log collection. You also would not know exact CPU, memory, disk I/O, or network state at failure time. The JSONL events do not currently include durations, vmrun exit outputs for all calls, free disk at start, template snapshot name validation, or guest OS tool versions.
|
|
|
|
Missing metrics and alerting:
|
|
|
|
- Per-phase duration summary emitted at job end.
|
|
- Free disk before and after job.
|
|
- Clone directory size at teardown.
|
|
- VM start lock wait time.
|
|
- Guest IP detection method used: guestinfo or fallback.
|
|
- Count of cleanup failures and leftover directories.
|
|
- Gitea runner online API status.
|
|
|
|
## 9. Operational Analysis
|
|
|
|
Backup/recovery procedures are conceptually complete but suffer from path drift. [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1) is valuable, but its default points to `F:\CI\Templates\WinBuild`. The current primary template is `F:\CI\Templates\WinBuild2025`. Fix that before relying on backups.
|
|
|
|
The template refresh process is detailed and realistic. Windows deployment, preparation, validation, shutdown, and snapshotting are all documented. The main problem is that some steps remain interactive, especially snapshot replacement in [template/Prepare-WinBuild2025.ps1](../template/Prepare-WinBuild2025.ps1). For a homelab this is acceptable, but for repeatability add a `-ForceSnapshot` or `-NoPrompt` option.
|
|
|
|
Scheduled task coverage is good: orphan cleanup, retention, disk alert, and runner health. Gaps: no scheduled template backup, no startup task to clear stale `vm-start.lock`, no task to validate runner online from Gitea, and no periodic smoke build.
|
|
|
|
Manual steps that remain manual but should be automated or at least scripted:
|
|
|
|
- Verifying ACLs on `F:\CI\keys\ci_linux`.
|
|
- Verifying Credential Manager entries under the same identity that act_runner uses.
|
|
- Updating and restarting the installed runner config after [runner/config.yaml](../runner/config.yaml) changes.
|
|
- Snapshot version rotation and rollback.
|
|
- Running one post-refresh smoke build and only then flipping `GITEA_CI_SNAPSHOT_NAME`.
|
|
|
|
Day-2 operations under-documented:
|
|
|
|
- How to rotate the Linux SSH key.
|
|
- How to recover if `actions/upload-artifact` fails after VM build success.
|
|
- How to inspect `invoke-ci.jsonl` to compute durations.
|
|
- How to confirm the runner service account can read Credential Manager secrets.
|
|
- How to validate shared folder cache poisoning or corruption.
|
|
|
|
## 10. Test Coverage
|
|
|
|
The Pester tests cover the right first layer: `_Common.psm1`, clone naming and cleanup on clone failure, VM removal behavior, `-WhatIf`, readiness timeout, and IP parameter validation. They are fast and do not require real VMware infrastructure, which is good for frequent local validation.
|
|
|
|
What they do not cover is more important for daily trust:
|
|
|
|
- [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) orchestration state machine.
|
|
- The file lock and lease behavior under concurrency.
|
|
- JSONL event emission.
|
|
- `UseGitClone` credential handling.
|
|
- Linux `ExtraGuestEnv` handling and secret redaction.
|
|
- Composite action YAML validity.
|
|
- Scheduled task registration names and arguments.
|
|
- Backup path correctness.
|
|
- Retention policy stale lock cleanup.
|
|
|
|
Missing integration tests:
|
|
|
|
- One Windows real-VM smoke build.
|
|
- One Linux real-VM smoke build.
|
|
- One 2-job or 4-job parallel run with lease verification.
|
|
- One failure build that proves logs/artifacts are collected or intentionally not collected.
|
|
- One workflow-level test of the composite action and upload-artifact path.
|
|
- One host reboot simulation or manual recovery test.
|
|
|
|
The test plan is not executable as-is. It contains wrong parameter names (`-RepositoryUrl` instead of `-RepoUrl`), PS7-only `ForEach-Object -Parallel`, expected task names that do not match actual task names, and expected JSONL shapes that do not match [scripts/Measure-CIBenchmark.ps1](../scripts/Measure-CIBenchmark.ps1). Treat it as an audit checklist, not a copy-paste runbook, until corrected.
|
|
|
|
## 11. Technical Debt
|
|
|
|
1. Stale path defaults. What: old `WinBuild` paths remain in setup/backup docs/scripts. Why deferred: project evolved from one template to multiple templates. Should it stay deferred: no. Cost to fix: under 1 hour.
|
|
|
|
2. Secret handling in Linux path. What: secret env values and PAT URLs can be logged or exposed in command strings. Why deferred: functionality was prioritized. Should it stay deferred: no if any secret-bearing builds run. Cost to fix: 2-4 hours.
|
|
|
|
3. No bounded native process wrapper in production scripts. What: `vmrun`, `ssh`, `scp`, `git`, and upload steps can hang. Why deferred: simple direct calls are easier. Should it stay deferred: no for `vmrun` lifecycle calls. Cost to fix: 2-3 hours for common helper and core calls.
|
|
|
|
4. Composite action output structure. What: outputs are under inputs. Why deferred: YAML shape likely not tested end to end. Should it stay deferred: no. Cost to fix: under 30 minutes.
|
|
|
|
5. Incomplete supply-chain pinning. What: many installer hashes are empty; Ubuntu VMDK lacks hash verification. Why deferred: version churn and convenience. Should it stay deferred: acceptable only for trusted homelab, but pin Git/7-Zip/Ubuntu immediately. Cost to fix: 1-2 hours.
|
|
|
|
6. Documentation drift. What: README, CI flow, test plan, and TODO disagree. Why deferred: rapid implementation. Should it stay deferred: no, because operators rely on docs during failures. Cost to fix: 2-3 hours.
|
|
|
|
7. Partial failure diagnostics. What: artifacts/logs not collected on build failure. Why deferred: success path came first. Should it stay deferred: no. Cost to fix: 1-2 hours.
|
|
|
|
8. Limited integration coverage. What: unit tests do not cover orchestration, workflow YAML, or real VM flows. Why deferred: real VMs are slow. Should it stay deferred: partly; add at least smoke tests. Cost to fix: 4-8 hours.
|
|
|
|
9. Interactive template refresh. What: Prepare scripts prompt for snapshot actions. Why deferred: manual maintenance is acceptable. Should it stay deferred: yes for homelab, but add optional noninteractive switch. Cost to fix: 1-2 hours.
|
|
|
|
10. Shared cache trust model. What: writable host cache folders can be poisoned by compromised guests. Why deferred: performance. Should it stay deferred: yes for trusted repos; revisit before untrusted code. Cost to fix: varies, 1 hour to document, more for read-only/cache validation.
|
|
|
|
## 12. Issues by Severity
|
|
|
|
### CRITICAL
|
|
|
|
Issue: Composite action outputs are declared under `inputs`, and `artifact-name` is duplicated.
|
|
Location: [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml), middle of metadata block around artifact output declarations.
|
|
Impact: The VM build can succeed but `actions/upload-artifact` may receive empty `steps.invoke-ci.outputs.artifact-path` or `artifact-name`, causing workflow failure or missing artifacts.
|
|
Fix: Add a top-level `outputs:` block sibling to `inputs:` and `runs:`. Move `artifact-path` and output `artifact-name` there. Keep only one `artifact-name` input.
|
|
|
|
Issue: Linux extra guest environment secrets are logged in clear text.
|
|
Location: [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1), Linux build command assembly and `Write-Host` before `Invoke-SshCommand`.
|
|
Impact: Any secret passed via `extra-guest-env-json` can appear in Gitea job logs and local transcripts.
|
|
Fix: Never log the expanded command. Log only a redacted command summary and pass secrets through a temporary env file copied over SSH with strict permissions, or base64-encoded stdin decoded on the guest and deleted immediately.
|
|
|
|
Issue: Linux `-UseGitClone` PAT can be embedded in the SSH command string.
|
|
Location: [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1), Linux guest clone branch using `GIT_CLONE_URL`.
|
|
Impact: PAT may leak via PowerShell error text, process arguments, SSH command history/audit, or debug logs.
|
|
Fix: Use an askpass helper or temporary credential file created on the guest with `chmod 600`, run git with `GIT_ASKPASS` or `http.extraHeader` from a file/stdin, then shred/delete it. Do not embed credential-bearing URLs in command strings.
|
|
|
|
### HIGH
|
|
|
|
Issue: `vmrun` lifecycle calls in production scripts have no hard timeout.
|
|
Location: [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1), [scripts/Remove-BuildVM.ps1](../scripts/Remove-BuildVM.ps1), [scripts/Measure-CIBenchmark.ps1](../scripts/Measure-CIBenchmark.ps1).
|
|
Impact: A hung VMware CLI call can consume an act_runner slot indefinitely and prevent cleanup.
|
|
Fix: Move `Invoke-VmrunBounded` into [scripts/_Common.psm1](../scripts/_Common.psm1), return exit code/output/timeout flag, and use it for `start`, `stop`, `deleteVM`, `list`, `readVariable`, and `getGuestIPAddress` where practical.
|
|
|
|
Issue: Stale `vm-start.lock` is not automatically cleaned after host crash.
|
|
Location: [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1), [scripts/Invoke-RetentionPolicy.ps1](../scripts/Invoke-RetentionPolicy.ps1), [scripts/Cleanup-OrphanedBuildVMs.ps1](../scripts/Cleanup-OrphanedBuildVMs.ps1).
|
|
Impact: New builds wait 10 minutes and fail until manual lock removal.
|
|
Fix: On startup cleanup and retention, remove `vm-start.lock` if older than a conservative threshold such as 30 minutes and no `powershell`/act_runner job owns it. Also write lock owner metadata into the lock file.
|
|
|
|
Issue: Host bootstrap creates stale/incomplete directory structure.
|
|
Location: [Setup-Host.ps1](../Setup-Host.ps1), directory creation and runner registration sections.
|
|
Impact: Rebuilding the host from scratch misses Linux template dirs, `State`, `keys`, `Cache\pip`, and current template naming; Linux builds and leases can fail later.
|
|
Fix: Create `Templates\WinBuild2025`, `Templates\WinBuild2022`, `Templates\LinuxBuild2404`, `State\ip-leases`, `keys`, `Cache\pip`, and register labels matching [runner/config.yaml](../runner/config.yaml).
|
|
|
|
Issue: Template backup default points to the wrong template path.
|
|
Location: [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1), `TemplatePath` default and restore instructions.
|
|
Impact: Operator can believe a backup exists while the active WinBuild2025/Linux template was never backed up.
|
|
Fix: Change defaults to `F:\CI\Templates\WinBuild2025`, add `-TemplateName` or support backing up all template directories, and update docs.
|
|
|
|
Issue: Failure path does not collect guest diagnostics before destroying the VM.
|
|
Location: [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1), `catch`/`finally` flow.
|
|
Impact: Build failures lose guest-side logs, intermediate artifacts, and environment state, making triage harder.
|
|
Fix: In `catch`, if VM is running and IP is known, attempt best-effort diagnostic collection to `F:\CI\Artifacts\<jobId>\diagnostics` before teardown.
|
|
|
|
Issue: Test plan uses PS7-only syntax and wrong parameter names.
|
|
Location: [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md), parallel job and e2e command examples.
|
|
Impact: Operators following the plan on PS5.1 hit syntax or parameter errors and lose confidence in validation.
|
|
Fix: Replace `ForEach-Object -Parallel` with `Start-Job` or sequential/manual commands, and update `-RepositoryUrl` to `-RepoUrl`.
|
|
|
|
Issue: Shared cache is not wired into the top-level workflow path.
|
|
Location: [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1), [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml).
|
|
Impact: Documented cache feature is not available from normal workflow usage, so restores/downloads remain slower.
|
|
Fix: Add `-UseSharedCache` to `Invoke-CIJob.ps1`, forward it to `Invoke-RemoteBuild.ps1`, and add a composite action input.
|
|
|
|
### MEDIUM
|
|
|
|
Issue: `JobId` is not sanitized before path use.
|
|
Location: [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1), [scripts/New-BuildVM.ps1](../scripts/New-BuildVM.ps1).
|
|
Impact: Unusual workflow IDs or malicious input can create invalid paths or confusing clone names.
|
|
Fix: Add `[ValidatePattern('^[A-Za-z0-9_.-]+$')]` or normalize to a safe filename segment.
|
|
|
|
Issue: Windows readiness only checks TCP 5986, not authenticated WinRM.
|
|
Location: [scripts/Wait-VMReady.ps1](../scripts/Wait-VMReady.ps1).
|
|
Impact: Phase 4 can pass even if authentication/remoting fails, shifting errors into build phase.
|
|
Fix: Optionally accept credentials and perform a tiny `Invoke-Command { 'ready' }` after TCP open for Windows jobs, or document the lighter check.
|
|
|
|
Issue: Documentation claims partial artifacts are collected on build failure.
|
|
Location: [docs/CI-FLOW.md](../docs/CI-FLOW.md), failure scenario table.
|
|
Impact: Operator expectation is wrong during failure triage.
|
|
Fix: Either implement best-effort failure diagnostics or update the documentation to say artifacts are collected only after successful build packaging.
|
|
|
|
Issue: `Validate-SetupState.ps1` does not mirror LocalAccounts property compatibility.
|
|
Location: [template/Validate-SetupState.ps1](../template/Validate-SetupState.ps1), `PasswordExpires` check.
|
|
Impact: Validation can fail on systems where `PasswordNeverExpires` is the available property.
|
|
Fix: Copy the compatibility logic from [template/Install-CIToolchain-WinBuild2025.ps1](../template/Install-CIToolchain-WinBuild2025.ps1).
|
|
|
|
Issue: Ubuntu cloud VMDK download is not hash-verified.
|
|
Location: [template/Deploy-LinuxBuild2404.ps1](../template/Deploy-LinuxBuild2404.ps1).
|
|
Impact: Corrupt or tampered base image becomes the parent of all Linux builds.
|
|
Fix: Add SHA256 parameter or checksum file validation from Ubuntu, fail if mismatch.
|
|
|
|
Issue: Installer hash pinning is mostly placeholders.
|
|
Location: [template/Install-CIToolchain-WinBuild2025.ps1](../template/Install-CIToolchain-WinBuild2025.ps1).
|
|
Impact: Supply-chain protection is documented but often not enforced.
|
|
Fix: Fill hashes for Python, dotnet-install, VS bootstrapper, 7-Zip, PowerShell, NSIS, CMake, Node, and gh for the pinned versions.
|
|
|
|
Issue: Event Log sources and task names drift from test plan.
|
|
Location: [scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1), [scripts/Register-CIScheduledTasks.ps1](../scripts/Register-CIScheduledTasks.ps1), [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md).
|
|
Impact: Tests fail even when scripts work.
|
|
Fix: Standardize names in docs and scripts: `CI-DiskSpaceAlert`, `CI-DiskAlert`, `CI-RunnerHealth`, etc.
|
|
|
|
Issue: `_Transport.psm1` disables host key persistence/checking.
|
|
Location: [scripts/_Transport.psm1](../scripts/_Transport.psm1).
|
|
Impact: SSH MITM protection is off on VMnet8.
|
|
Fix: For template/provisioning keep permissive mode if needed; for CI jobs, use `StrictHostKeyChecking=accept-new` and a CI-specific known_hosts file under `F:\CI\State`.
|
|
|
|
### LOW
|
|
|
|
Issue: Deprecated runner installer remains likely to drift.
|
|
Location: [runner/Install-Runner.ps1](../runner/Install-Runner.ps1).
|
|
Impact: A user may run the deprecated path and miss current labels/config.
|
|
Fix: Replace body with a hard stop pointing to [Setup-Host.ps1](../Setup-Host.ps1), or update it fully.
|
|
|
|
Issue: Webhook messages include emoji despite current output preference.
|
|
Location: [scripts/Watch-DiskSpace.ps1](../scripts/Watch-DiskSpace.ps1), [scripts/Watch-RunnerHealth.ps1](../scripts/Watch-RunnerHealth.ps1).
|
|
Impact: Cosmetic inconsistency with project/user style.
|
|
Fix: Use plain prefixes such as `[WARN] CI Disk Alert` and `[ERROR] CI Runner Alert`.
|
|
|
|
Issue: Architecture diagram has minor indentation/rendering damage.
|
|
Location: [docs/ARCHITECTURE.md](../docs/ARCHITECTURE.md), system diagram script box.
|
|
Impact: Documentation polish issue.
|
|
Fix: Reformat the box so all lines align.
|
|
|
|
Issue: Several docs still say HTTP/Basic where implementation uses HTTPS.
|
|
Location: [template/Validate-DeployState.ps1](../template/Validate-DeployState.ps1), [template/Validate-SetupState.ps1](../template/Validate-SetupState.ps1) docblocks.
|
|
Impact: Confusing security documentation.
|
|
Fix: Update docblocks to HTTPS/5986 Basic over TLS.
|
|
|
|
Issue: Benchmark script JSON shape differs from test-plan expectations.
|
|
Location: [scripts/Measure-CIBenchmark.ps1](../scripts/Measure-CIBenchmark.ps1), [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md).
|
|
Impact: Copy-paste parsing commands do not work.
|
|
Fix: Either change the JSON schema or update test-plan queries.
|
|
|
|
### NICE TO HAVE
|
|
|
|
Issue: No Gitea API runner-online check.
|
|
Location: [scripts/Watch-RunnerHealth.ps1](../scripts/Watch-RunnerHealth.ps1).
|
|
Impact: Service can be running but disconnected/auth-broken.
|
|
Fix: Add optional `-GiteaUrl` and `-TokenTarget` to query runner status.
|
|
|
|
Issue: No automatic smoke build after template refresh.
|
|
Location: operational process, no single file.
|
|
Impact: Bad snapshots can be promoted manually.
|
|
Fix: Add a `Test-CITemplateSmoke.ps1` that runs a tiny Windows and Linux build before updating runner config.
|
|
|
|
Issue: No artifact manifest.
|
|
Location: [scripts/Get-BuildArtifacts.ps1](../scripts/Get-BuildArtifacts.ps1).
|
|
Impact: Harder to compare artifacts across Windows/Linux legs.
|
|
Fix: Write `manifest.json` with file names, sizes, hashes, guest OS, commit, and job ID.
|
|
|
|
Issue: No VM resource override per workflow.
|
|
Location: [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1), VMX/template flow.
|
|
Impact: Light jobs over-allocate and heavy jobs may be constrained.
|
|
Fix: Add optional post-clone VMX patching for CPU/RAM, default off.
|
|
|
|
Issue: No structured summary printed at job end.
|
|
Location: [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1).
|
|
Impact: Operators must parse JSONL manually.
|
|
Fix: At job end, compute phase durations from in-memory timestamps and print a compact table.
|
|
|
|
## 13. Quick Wins
|
|
|
|
Exactly 10 tasks under 1 hour:
|
|
|
|
1. Fix [gitea/actions/local-ci-build/action.yml](../gitea/actions/local-ci-build/action.yml) by moving `artifact-path` and output `artifact-name` to top-level `outputs`.
|
|
2. Remove secret values from the Linux `Write-Host "Running build"` line in [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1).
|
|
3. Add stale `vm-start.lock` cleanup to [scripts/Invoke-RetentionPolicy.ps1](../scripts/Invoke-RetentionPolicy.ps1).
|
|
4. Update [Setup-Host.ps1](../Setup-Host.ps1) to create `State`, `keys`, `Cache\pip`, and current template directories.
|
|
5. Change [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1) default from `WinBuild` to `WinBuild2025` or add an `-AllTemplates` mode.
|
|
6. Replace `ForEach-Object -Parallel` in [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md) with PS5.1-compatible `Start-Job` examples.
|
|
7. Update `-RepositoryUrl` examples to `-RepoUrl` in [docs/TEST-PLAN-v1.3-to-HEAD.md](../docs/TEST-PLAN-v1.3-to-HEAD.md).
|
|
8. Add `JobId` validation to [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1) and [scripts/New-BuildVM.ps1](../scripts/New-BuildVM.ps1).
|
|
9. Add a pre-clone free-space check to [scripts/Invoke-CIJob.ps1](../scripts/Invoke-CIJob.ps1).
|
|
10. Remove emoji text from webhook payloads in watcher scripts.
|
|
|
|
## 14. Roadmap
|
|
|
|
### Phase 1 - Stabilization (complete before trusting for real builds)
|
|
|
|
- Fix composite action outputs and run one workflow end to end through artifact upload.
|
|
- Redact/fix Linux secret handling for `ExtraGuestEnv` and `-UseGitClone`.
|
|
- Add stale `vm-start.lock` cleanup and startup validation.
|
|
- Fix stale defaults in [Setup-Host.ps1](../Setup-Host.ps1) and [scripts/Backup-CITemplate.ps1](../scripts/Backup-CITemplate.ps1).
|
|
- Correct the test plan so every command is PS5.1-compatible and uses current parameter names.
|
|
- Add pre-clone disk free-space gate.
|
|
- Run one real Windows VM smoke build and one real Linux VM smoke build from the composite action.
|
|
|
|
### Phase 2 - Daily Operations (comfortable daily use)
|
|
|
|
- Add best-effort diagnostics collection on build failure.
|
|
- Add phase duration summary to job logs.
|
|
- Add Gitea API runner-online check as optional runner health mode.
|
|
- Add ACL validation script for `F:\CI\keys\ci_linux` and Credential Manager access under the runner identity.
|
|
- Add artifact manifest with file hashes.
|
|
- Add `UseSharedCache` wiring through `Invoke-CIJob.ps1` and the composite action.
|
|
- Add a documented template refresh checklist with backup, validation, smoke build, and rollback.
|
|
|
|
### Phase 3 - Optimization
|
|
|
|
- Benchmark `guestinfo.ci-ip` versus `getGuestIPAddress` and update [scripts/Measure-CIBenchmark.ps1](../scripts/Measure-CIBenchmark.ps1).
|
|
- Tune capacity using real build metrics; keep default 4 unless evidence supports 5 or 6.
|
|
- Consider optional per-workflow CPU/RAM overrides for unusually light or heavy jobs.
|
|
- Evaluate shared cache hit rates and add cache cleanup/validation.
|
|
- Consider a warm clone pool only if measured startup overhead dominates short builds.
|
|
|
|
### Deferred/Optional (genuinely not needed for homelab)
|
|
|
|
- Multi-host runner federation.
|
|
- Prometheus/Grafana/Loki stack.
|
|
- Full PKI for WinRM certificates.
|
|
- Full antivirus/firewall hardening while only trusted private code is built.
|
|
- Enterprise secret vault integration.
|
|
- Kubernetes, ESXi, or container orchestration.
|
|
|
|
## 15. Architectural Recommendations
|
|
|
|
Keep the single-host design. It matches the actual constraints and avoids needless complexity. The main architectural improvement should be making the existing local design more deterministic, not replacing it.
|
|
|
|
Use three small shared contracts:
|
|
|
|
- A path contract in [AGENTS.md](../AGENTS.md) and [Setup-Host.ps1](../Setup-Host.ps1): exact directories created and owned by the CI system.
|
|
- A process contract in [scripts/_Common.psm1](../scripts/_Common.psm1): all native lifecycle calls go through a bounded wrapper.
|
|
- A secret contract in [scripts/Invoke-RemoteBuild.ps1](../scripts/Invoke-RemoteBuild.ps1): no secret ever appears in command text, logs, URL strings, or artifact manifests.
|
|
|
|
Do not over-abstract WinRM and SSH into a single generic transport. Keep explicit branches, but share small helper functions. The current clarity is valuable when something fails at 2 AM.
|
|
|
|
Do not pursue deterministic static IP assignment yet. The current lock-plus-lease design is good enough after stale lock cleanup. Static IP injection into clones or VMware DHCP reservations adds more moving parts than it removes for capacity 4.
|
|
|
|
Treat templates as release artifacts. Back them up, version snapshots, run a smoke build, then promote by updating runner config. This is the homelab equivalent of a golden image promotion flow, and it can stay simple.
|
|
|
|
Keep security proportional. For trusted private repositories, disabled Defender/Firewall/UAC is acceptable if documented. But fix secret logging immediately because it is cheap and prevents painful accidents.
|
|
|
|
## 16. Final Assessment
|
|
|
|
Completeness: 82/100
|
|
|
|
Code quality: 78/100
|
|
|
|
Operational readiness: 72/100
|
|
|
|
Overall: 77/100
|
|
|
|
Honest verdict: This is a strong homelab CI/CD system with a sensible architecture and a lot of hard-earned VMware/PowerShell knowledge baked in. The core VM lifecycle is close to daily-use quality, and the Windows/Linux dual-template direction is well chosen. I would trust it for supervised real builds now, but I would not yet leave it fully unattended for daily production-like use until the composite action output bug, Linux secret leakage, stale lock recovery, and setup/backup path drift are fixed. Those are not big redesigns. They are the last tightening pass between an impressive lab build and a reliable daily tool.
|