Commit Graph

91 Commits

Author SHA1 Message Date
goodolclint-claude[bot] 109dca657a refactor: make the offline Pester suite able to fail (#153) (#205)
Skip-IfMissing skipped a test whenever the cmdlet under test was absent
from the build, including the test asserting it exists. Every cmdlet
compiles into one assembly, so there is no partial-build case for it to
serve; all it did was hide a missing cmdlet. Delete the 37 copies and
every call site, and drop the residual conditional skips in the same
family (CmdExists probes, an attribute-presence skip in SdnCmdlets, a
lifecycle helper skipping on a cmdlet-name collision fixed long ago).

894 It blocks only reflected [Cmdlet] and [Parameter] attributes back at
the compiler: existence, CommandType -eq 'Cmdlet', Parameters.ContainsKey,
IsMandatory reflection, and per-file CmdletsToExport asserts for whichever
names an author remembered. Nothing reflected over the built assembly, so
a new cmdlet missing from the manifest shipped invisible. One data-driven
file replaces them: it diffs CmdletsToExport against the assembly's cmdlet
types in both directions and asserts the conventions reflection can see.

Behavioural tests are untouched: no-session errors, binding rejections,
ShouldProcess and -WhatIf, ConfirmImpact, ValidateSet and ValidateRange
values, parameter types, positions and pipeline binding.

The generated help covered 169 of 194 cmdlets. Regenerated with the repo's
own generate-help.ps1: 25 new markdown stubs, 13 existing docs picking up
parameters added in earlier waves, and a rebuilt MAML.

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
2026-09-03 00:54:02 +00:00
goodolclint-claude[bot] 283dc47658 refactor: wire Group level onto the firewall rule cmdlets (#126) (#202)
FirewallService.GetGroupRules/CreateGroupRule/UpdateGroupRule/RemoveGroupRule
already existed with zero callers. Get/New/Set/Remove-PveFirewallRule gain a
Group value in their -Level ValidateSet and a -Group parameter, required and
validated the same way Node/VmId are validated for the other levels, and
dispatch to those service methods instead of the generic BuildBasePath-driven
ones. No cmdlet in this area built its own client, so unlike the other #126
areas there is no inline request to strip.

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
2026-09-03 00:14:47 +00:00
goodolclint-claude[bot] 4cc18f1cc2 Fix #144: remove defunct DELETE /access/ticket and gain -Session parameter (#162)
* fix: remove defunct DELETE /access/ticket call and gain -Session parameter

Disconnect-PveServer was calling DELETE /access/ticket, an endpoint that does
not exist in the PVE API spec. The call always failed but errors were swallowed,
so the cmdlet succeeded while claiming server-side invalidation occurred.

Rebase on PveCmdletBase to gain the -Session parameter, making explicit session
disconnection possible (the cmdlet previously only worked with the active
session). Update help text to clarify that PVE tickets expire server-side after
two hours and cannot be revoked.

Remove the DELETE call entirely — tickets are stateless and expire on their own.
Clear ModuleState.ActiveSession only when disconnecting the active session
(no -Session given) or when explicitly disconnecting the active session.

* fix: address reviewer findings on -Session handling and test coverage

Fixes from correctness and api-compat reviews:

1. Use BoundParameters to distinguish -Session omitted from -Session $null,
   preventing accidental active-session clear when $null is passed.

2. Warn and return early when -Session is supplied but not the active session,
   avoiding silent no-ops that leave the user's session variable populated and
   functional but with expectations misaligned (they passed a session to
   disconnect it, but disconnecting a non-active session is now explicit).

3. Use ReferenceEquals() explicitly instead of == for the identity check,
   future-proofing against PveSession ever gaining value-equality semantics.

4. Fix the lifecycle test to check observable behavior (warning output) instead
   of reaching into null PrivateData. Tests now verify both "no session to
   disconnect" and "non-active session supplied" paths.

* fix: remove unreliable offline lifecycle tests from Pester suite

The lifecycle tests attempted to capture warning output and verify module state,
but both approaches fail in the offline Pester environment:

1. WarningVariable captures don't work as expected in Pester contexts
2. ModuleState is not exposed through PrivateData and cannot be inspected from
   outside the module in offline tests

Keep the reliable metadata test that verifies -Session parameter is exposed.
The behavioral verification of the warning logic happens in the correctness
review and will be validated in integration testing, not in offline unit tests.

* fix: move reference equality check before ShouldProcess

The ShouldProcess check was firing before validating whether the supplied
-Session is actually the active session. This means -WhatIf would prompt
"Disconnect host:port?" even for sessions that will not be disconnected
(where the cmdlet just warns and returns).

Move the ReferenceEquals check before ShouldProcess so mismatched sessions
short-circuit with a warning before any prompting occurs. This accurately
represents what -WhatIf/-Confirm is about to do.

* test: restore and harden offline coverage for Disconnect-PveServer session handling

Commit 3f75d4a deleted two offline Pester tests for the reference-equality
logic added in fa930db, with a wrong justification (WarningVariable
captures don't work in Pester). WarningVariable works fine; the real
defect in the deleted mismatched-session test was passing a
[PSCustomObject] to a parameter typed PveSession, a type-binding
failure, not a WarningVariable failure.

Restores both cases via reflection against PveSession's internal
constructor and ModuleState's internal static ActiveSession property
(both types are internal/have internal members, so Pester has no other
way to construct a real session or observe module state), and adds
coverage for the two branches the deleted tests never exercised: the
active-session clear itself, -Session pointing at the active session,
and -WhatIf leaving the active session untouched. Per ADR 0021 this
logic makes no server call and must be pinned offline.

* fix: word the mismatched-session warning for the session's auth mode

API-token sessions do not expire and can be revoked with Remove-PveApiToken,
so the ticket wording was wrong for them.

---------

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
2026-09-02 17:55:39 +00:00
goodolclint-claude[bot] b90791e2bf docs: migrate DECISIONS.md to house-format ADRs, and retire the review folder (#131)
D001-D021 become ADR 0001-0021 in docs/decisions/, one decision per file.
D017's PESTER_VERSION amendment was a second decision in one entry and becomes
ADR 0022. ADR 0023 records the migration and reverses the lane2-change-plan
ruling that deliberately kept DECISIONS.md until the CI lane work landed.

DECISIONS.md is reduced to a stub with a D-to-ADR redirect table, so the four
released CHANGELOG entries and older issue bodies that cite it degrade to a
redirect rather than a dead reference.

docs/review/ and docs/lane2-change-plan.md are deleted (ADR 0024). Of 91
findings, 83 were resolved and six of the seven still open were already GitHub
issues; F021 was the exception and is now #130.

CLAUDE.md's Key Conventions list gains the two rules it was missing and becomes
the checklist, with the ADRs carrying rationale.
2026-09-02 14:49:07 +00:00
goodolclint-claude[bot] 2dff02f2bd feat: VLAN-aware bridges via New-PveNetwork and Set-PveNetwork
PveNetwork already deserialised bridge_vlan_aware as BridgeVlanAware, so a
VLAN-aware bridge could be read back but never created or changed. Both write
paths now take a -BridgeVlanAware switch.

Clearing the flag does not use bridge_vlan_aware=0. PVE merges the supplied
keys onto the stored stanza and accepts that 0 without acting on it, so the
obvious form is a silent no-op: an integration run against PVE 9 issued it and
Get-PveNetwork still reported 1. The endpoint's delete list is what actually
removes the key. The API schema advertises a plain boolean and gives no hint of
this, which is why the behaviour is pinned by an integration test rather than
inferred.

Set-PveNetwork guards the switch on BoundParameters so an update that omits it
leaves the flag alone; the create path follows the existing -Autostart form.
Only bridge_vlan_aware is added. bridge_vids is an independent parameter that
PVE defaults to 2-4094, and the issue asks only for the flag.

Coverage: the integration suite pins create, disable, re-enable, and that an
unrelated Set leaves the flag alone -- that last one kills a mutant that drops
the BoundParameters guard, which every other test survives. A model test pins
the read path the assertions depend on. The Pester unit tests assert only
parameter metadata; the defect is server-side, so nothing offline can catch it.

Closes #92
2026-09-01 22:53:26 -05:00
goodolclint-claude[bot] 08ee3ae249 chore: repair the local dev path and delete its dead scaffolding
The local dev environment had drifted badly from CI. Remove the parts that no
longer describe anything real, and make the rest match how CI actually runs.

Delete tests/dev.ps1. It wrapped run-integration.sh, which CI calls directly,
and duplicated the module build that script already performs internally. As a
second entry point it drifted: it still offered the PVE 8 leg retired in #88,
mounted the Docker socket for storage containers replaced by the storage VM in
#87, and pointed its remote-host examples at a runner decommissioned in the ARC
migration. All four documents describing it used a positional syntax that bound
the bare word to -Tests and then fell through to -Shell, so every documented
command silently opened a container shell. Recorded as D019.

Delete tests/infrastructure/runner/, a self-hosted-runner-in-Docker superseded
by Actions Runner Controller.

Make disk_storage and iso_storage required. Their defaults named a NAS that the
lab replaced with Ceph, and CI overrides both from repository variables, so the
defaults only ever misled local runs. require_env now fails at the top of a run
rather than at terraform apply, and the descriptions point at tests/.env.test
because cmd_provision deletes terraform.tfvars before applying.

preflight-cleanup.sh no longer falls back to the literal "local" storage. An
unset TF_VAR_iso_storage now skips only the ISO branch, leaving VM destroy and
state cleanup intact, and emits a workflow annotation: force-cleanup is the
only cleanup CI runs and it wipes Terraform state, so a silent skip strands the
uploaded ISO with nothing left to reclaim it.

Drop docker-ce-cli and the /var/run/docker.sock mount. Nothing in the container
has called docker since #87 moved storage into a VM; the remaining docker calls
run inside that VM over SSH. The CI job image is built from the same target, so
this also removes a third-party apt repository from its supply chain.

Rewrite tests/.env.test.example against what the code now requires, and fix the
documented commands in CLAUDE.md, README.md, copilot-instructions.md and the
integration README.
2026-09-01 16:27:10 -05:00
goodolclint-claude[bot] 959b62c86f test: assert node B joined by name, not an online-node count
Closes #94
2026-09-01 18:37:22 +00:00
goodolclint-claude[bot] 1df1b05876 test: drop the fixed sleep and the link0 pin from the cluster tests
New-PveCluster -Wait now guarantees quorum, so the Start-Sleep -Seconds 5
that lost the race is replaced by an assertion on the new contract.

The link0 pin is dropped: the join-abort and the never-a-member modes both
occurred with and without it, so it was never implicated, and -Links is
already covered at the service level.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 06:24:11 +00:00
goodolclint-claude[bot] e0960d9c18 test: pin node B ring0 at join and dump cluster status on the online check
The "2 nodes online" check has never passed — run 159 shows the identical
failure while the job reported success (swallowed Pester exit code), and
run 170's 60 s poll expires with node B still online=0. Node B reaches
corosync.conf (Get-PveClusterConfigNode passes) but never becomes a
corosync member, and the suite captures nothing about why.

Pass link0 explicitly so ring0 is the address the harness verified node B
answers on, rather than whatever node B resolves its own DHCP-assigned
.test.local hostname to. Dump quorate and each node's ring0/online/local
unconditionally so the next run distinguishes a wrong ring0 address from
a working address with no corosync transport.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 05:02:53 +00:00
goodolclint-claude[bot] f758275668 test: poll cluster status for node B online instead of asserting instantly
The join task completing does not mean corosync membership has reached
the status endpoint; the instant assertion failed intermittently (runs
159 and 169) while every other cluster check passed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-31 23:00:49 +00:00
goodolclint-claude[bot] f48f0ea9ae ci: storage and answer services move to a VM inside the CI sandbox (1/2)
The nested PVE guests live on an isolated VLAN with no route to the
runner network (ADR 0032, homelab repo), and the runner-hosted Docker
services died with each ephemeral ARC pod anyway. A small cloud-image VM
in the ci pool now serves NFS, iSCSI, and the auto-install answer files
from inside the sandbox. Includes fixes from Codex and in-house review:
bpg provider >= 0.79.0, serial console for the resized cloud image,
key-based SSH (cloud images refuse password auth), CIDR validation,
storage VM in the headroom check, docker socket mounts removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-31 19:52:36 +00:00
Clint Branham bc71ed4a12 fix: deliver Invoke-PveVmGuestExec -Args to the guest as argv
ExecuteGuestCommand JSON-serialized the args array into the agent/exec
'input-data' field — which is the process's STDIN, not its arguments. So
guest commands ran with no argv: cmd.exe started interactively and the
JSON blob ['/c','echo',...] arrived at its prompt.

PVE's agent/exec 'command' parameter is itself an array (element 0 = the
executable, the rest = argv) sent as repeated form keys. The low-level
client couldn't express repeated keys (Dictionary<string,string> only),
so:

- Add PostAsync(string, IEnumerable<KeyValuePair<string,string>>) to
  IPveHttpClient/PveHttpClient; BuildFormContent now emits one key=value
  field per pair, so a key may repeat.
- ExecuteGuestCommand builds command = [exe] + args as repeated 'command'
  fields and no longer touches input-data.

Tests: form-encoder repeated-key + per-value encoding cases; VmService
tests asserting the command array, order, and absence of input-data; an
integration regression guard that echoes an arg and checks it round-trips
as stdout.

Tracked as F091. Closes #68.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 14:32:04 -05:00
Clint Branham 80b70cdaf6 fix: address PR #67 review feedback
- HasDiskOptions(): include -ScsiHardware so the "disk options ignored"
  warning fires when -ScsiHardware is passed without -DiskStorage/-DiskSize.
- PveVmConfig.AdditionalProperties: lazy-init a backing field so the native
  dictionary is built once rather than reallocated on every property access
  (matters when iterating many configs in a pipeline). Safe because the model
  is effectively immutable after deserialization.
- New-PveVm.Tests.ps1: add a case asserting -DiskIoThread on scsi with a
  wrong -ScsiHardware (virtio-scsi-pci) is rejected, covering the validator's
  "!= virtio-scsi-single" branch (not just the null case).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 14:18:32 -05:00
Clint Branham c1714048b4 feat: disk controller/IO options on New-PveVm + surface all Get-PveVmConfig keys
New-PveVm (F089):
- Add -DiskBus (virtio/scsi/sata/ide, default virtio), -ScsiHardware (scsihw),
  -DiskIoThread, -DiskAio, -DiskSsd, -DiskDiscard, -DiskCache so a tuned disk
  (e.g. virtio-scsi-single + scsi0,iothread=1,aio=native,ssd=1,discard=on) can
  be created in one call instead of diskless + a hand-built Set-PveVmConfig string.
- Disk spec built via BuildDiskSpec; ValidateDiskOptions runs before ShouldProcess
  and rejects ssd on virtio and iothread on sata/ide or scsi-without-virtio-scsi-single
  with clear errors, instead of letting PVE fail at VM start.

Get-PveVmConfig (F090):
- PveVmConfig was a fixed allow-list, silently dropping keys like scsihw, efidisk0,
  tpmstate0, hostpci0. Add typed scsihw/efidisk0/tpmstate0 plus a [JsonExtensionData]
  catch-all exposed as AdditionalProperties (native types via JsonHelper.ToNative,
  per D013 — no JToken leakage). Makes the disk tuning above verifiable by reading
  the config back.

Closes #65.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 14:10:14 -05:00
Clint Branham ea2bcdc336 Merge main into fix/http-client-timeout
Resolves findings.json conflict — F086 (from #60, merged into main) and
F087 (this branch) both append to the trailing findings array. Kept both
entries, in numeric order.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:17:20 -05:00
Clint Branham f3b06171b2 fix: add -TimeoutSeconds for long-running HTTP calls
PveHttpClient was constructed without setting HttpClient.Timeout, so
.NET's 100s default applied to every request. Multi-GB ISO uploads via
Send-PveFile on a real LAN reliably tripped this with TaskCanceledException
after 100 seconds, and there was no way to override it.

- PveSession gains a Timeout (TimeSpan) property, defaulting to 100s.
- PveHttpClient accepts an optional per-instance timeout override that
  takes precedence over the session timeout.
- Connect-PveServer exposes -TimeoutSeconds to set the session default.
- Send-PveFile and Invoke-PveStorageDownload expose -TimeoutSeconds with
  a 30-minute implicit default so large uploads/downloads do not trip
  the 100s default. -TimeoutSeconds 0 means Timeout.InfiniteTimeSpan.

Tracked as F087. Closes #59.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:08:26 -05:00
Clint Branham fa361a3691 fix: address PR #60 review feedback
- SizeParser: wrap TB-suffix overflow in try/catch so callers get
  ArgumentException with the parameter name rather than OverflowException.
- New-PveVm/New-PveContainer: validate -DiskSize/-RootFsSize before
  ShouldProcess so typos like "512M" are rejected even with -WhatIf and
  even when the matching -DiskStorage/-RootFsStorage is omitted.
- Add Pester tests for the new DiskSize and RootFsSize validation paths,
  including a new New-PveContainer.Tests.ps1.
- Add SizeParserTests coverage for the TB overflow path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 18:06:13 -05:00
Clint Branham 5dcbde45a6 fix: resolve issues #43, #44, #45
#44 Get-PveApiToken FullTokenId empty:
  - Make FullTokenId a computed property (UserId + "!" + TokenId)
  - RawFullTokenId captures the API's "full-tokenid" for creation responses

#43 Set-PvePermission token ACLs:
  - Add "token" to Type ValidateSet
  - Auto-detect tokens from "!" in UgId (user@realm!tokenid format)
  - Add tokens parameter to UserService.SetPermission

#45 Connect-PveServer return session by default:
  - Always output session (matches Connect-AzAccount pattern)
  - Add -Quiet switch to suppress output
  - Keep -PassThru as hidden deprecated param for backwards compat

Closes #43, closes #44, closes #45

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-27 10:22:37 -05:00
Clint Branham 12ca01cd09 fix: remove ValidateSet assertion from Set-PveHaRule Type test
The ValidateSet attribute was intentionally removed from the Type
parameter during merge review to allow future rule types without
code changes. Updated test to only assert Type is mandatory.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-26 08:18:47 -05:00
Clint Branham 514ce3fc1b fix: Set-PveHaRule requires mandatory Type parameter
PVE 9 API requires 'type' on PUT /cluster/ha/rules/{rule} even for
updates. Added mandatory Type parameter with ValidateSet for
node-affinity and resource-affinity.

Updated integration test and Pester unit tests to pass -Type.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:52:23 -05:00
Clint Branham d43387806e refactor: adapt cluster tests to _IntegrationHelper + rename to 16_Cluster
- Rename ZZ.ClusterConfig.Integration.Tests.ps1 → 16_Cluster.Tests.ps1
- Replace manual env var setup with _IntegrationHelper.ps1 + Connect-TestPve
- Use credential auth (root@pam) throughout — no API tokens
- Use $script:PasswordB for node B auth (from helper)
- Access JoinInfo.Nodelist as List<Dictionary> (native types from D013)
- Keep cluster-specific skip helpers (Skip-IfNoCluster, Skip-IfPve9HaGroups)
- Remove redundant Connection context (Connect-TestPve handles it)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:01 -05:00
Clint Branham e7b1c8488c fix: HA rule test creates real VM + HA resource before rule
PVE 9 rejects rules referencing unmanaged resources. Test now creates
a minimal VM, registers it as a disabled HA resource, then creates
the node-affinity rule. Cleanup removes rule, resource, and VM.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:01 -05:00
Clint Branham 9636c45a40 fix: remove unsupported affinity param from HA rule test
PVE 9 rejects 'affinity' as an unexpected property on POST
cluster/ha/rules — it's implied by the node-affinity type.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:01 -05:00
Clint Branham fc29d01a7a test: add HA rules CRUD integration tests for PVE 9.0+
Full lifecycle: create node-affinity rule, list, get by ID, update
comment, delete, and verify deletion. Uses vm:99999 as a synthetic
resource SID. Tests skip on PVE 8 (rules not available).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:01 -05:00
Clint Branham 23b5334b27 test: rename cluster integration tests to run last
Cluster tests create a 2-node cluster that cannot be torn down via API
(quorum loss). Prefix with ZZ so they execute after all other
integration tests that assume standalone nodes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:01 -05:00
Clint Branham 4d9533ae36 fix: skip node removal in 2-node cluster integration tests
Removing a node from a 2-node PVE cluster via REST API is not supported
— the remaining node loses quorum mid-operation, causing "no quorum!"
errors. PVE requires stopping corosync on the departing node first
(pvecm expected 1), which is not available via the REST API.

Replace removal tests with a final cluster state verification.
Test infrastructure handles cleanup via reprovisioning.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -05:00
Clint Branham 988b1446b0 fix: use root@pam for cluster node removal in integration tests
Remove-PveClusterConfigNode requires root@pam (not API token).
Fixed both the test context and AfterAll cleanup to connect with
root@pam credentials before attempting node removal.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -05:00
Clint Branham 4277ad73b9 fix: retry re-auth during cluster join and always reconnect to node A
- Extract re-auth retry into ReauthenticateWithRetry helper (fixes
  cognitive complexity warning)
- Retry auth up to 10x with 3s delay — node B's auth services need
  time to restart after joining the cluster
- Wrap join in try/finally so the test always reconnects to node A,
  even if the join fails

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -05:00
Clint Branham 8564e05c00 fix: use root@pam throughout cluster integration tests
All cluster operations (create, status, join, options, cleanup) require
root@pam ticket auth — API tokens lack Sys.Audit and Sys.Modify on /.
Changed test to connect as root@pam once during cluster creation and
stay on that session for the entire lifecycle.

Removed redundant Connect-PveServer calls that switched between
root@pam and API token between contexts.

Added 5s stabilization sleep after cluster creation for corosync.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -05:00
Clint Branham f77567573c fix: integration test fixes for JArray and auth issues
- Convert JoinInfo.Nodelist to native PS array via ConvertFrom-Json
  (JArray .Item() still hits IEnumerator error in PowerShell)
- Use root@pam for cluster options context (delete requires Sys.Modify)
- Reconnect with API token after options restore

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -05:00
Clint Branham c7627ee4e1 test: add xUnit, Pester, and integration tests for cluster config + HA
xUnit (47 new tests, 429 total):
- ClusterConfigServiceTests: 25 tests covering all 14 service methods
  including URL encoding, null guards, and error responses
- HaServiceTests: 22 tests covering resources, groups, status, and rules
  with URI encoding verification (vm:100 → vm%3A100)

Pester (187 new tests, 1525 total):
- ClusterConfigCmdlets.Tests.ps1: 11 cmdlets tested
- HaCmdlets.Tests.ps1: 14 cmdlets tested

Integration (ClusterConfig.Integration.Tests.ps1):
- Full 2-node cluster lifecycle with -Wait for task completion
- Uses root@pam ticket auth for cluster create/join operations
- HA group tests skip on PVE 9.0+ (groups migrated to rules)
- JArray indexing uses .Item() for PowerShell compatibility

Cmdlet improvements:
- New-PveCluster, Add-PveClusterConfigNode, Add-PveClusterMember now
  support -Wait switch to block until task completes (via TaskService)
- GetClusterConfig returns JToken to handle array responses on standalone
- OutputType updated to PveTask for task-returning cmdlets

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -05:00
Clint Branham 4e4bc11872 test: add xUnit, Pester, and integration tests for cluster config + HA
xUnit (47 new tests, 429 total):
- ClusterConfigServiceTests: 25 tests covering all 14 service methods
  including URL encoding, null guards, and error responses
- HaServiceTests: 22 tests covering resources, groups, status, and rules
  with URI encoding verification (vm:100 → vm%3A100)

Pester (187 new tests, 1525 total):
- ClusterConfigCmdlets.Tests.ps1: 11 cmdlets — parameter metadata,
  ValidateSet/ValidateRange/ValidateLength, ShouldProcess, ConfirmImpact
- HaCmdlets.Tests.ps1: 14 cmdlets — same pattern

Integration (ClusterConfig.Integration.Tests.ps1):
- Full 2-node cluster lifecycle: create → join → options → HA groups → cleanup
- Uses root@pam ticket auth for cluster create/join (API tokens lack permission)
- Reconnects with API token after privileged operations
- Skip helpers for standalone (no node B) and non-PVE-9 environments

Fixes:
- GetClusterConfig returns JToken (not JObject) to handle array responses
  from /cluster/config on standalone nodes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:40:00 -05:00
Clint Branham 94bc1a9d73 fix: address Copilot review round 3
- Fix bare Skip-IfNoTarget calls in 13_Firewall and 14_Backup
  (missing if/return pattern caused tests to run when they should skip)
- Validate modifier-only switches in dev.ps1 (-Force/-Reprovision
  without an action switch now errors instead of defaulting to -Shell)
- Add force-cleanup to usage text in run-integration.sh
- Add --connect-timeout/--max-time to guest agent curl in wait-for-pve.sh

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 16:27:14 -05:00
Clint Branham 7ffc57e16f fix: address Copilot review — validation, error handling, MAC case
- Add ValidateSet('8','9','all') to dev.ps1 -Version parameter
- Fix Shell warning to reference $DevContainer not $InfraContainer
- Fix Skip-IfNoNodeB to check $PasswordB not $Password
- Pass PVE_TARGET_NODE to wait-for-pve.sh instead of auto-discovering
- Add error default cases to all pve_* helper functions
- Lowercase MAC addresses for answer server matching
- Create answer file paths before terraform destroy in cleanup

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 15:50:07 -05:00
Clint Branham 117b10d222 refactor: make all integration test files self-contained
Each test file now creates and cleans up its own resources:
- 06_VMs: restores own AfterAll cleanup for pester-test-vm
- 07_Snapshots: creates pester-snap-vm, tests snapshots, cleans up
- 09_CloudInit: creates pester-ci-vm with cloud-init drive, cleans up
- 15_Tasks: creates pester-task-vm, tests task CRUD, cleans up

Removed cross-file dependency helpers (Find-TestVm, Register-TestResource)
from _IntegrationHelper.ps1 — no longer needed.

Each file can now run independently via -Tests filter. If a Setup
context fails, subsequent tests in the same file skip gracefully.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 13:40:05 -05:00
Clint Branham 72280e3697 fix: don't clean up test VM in 06_VMs — later files depend on it
06_VMs AfterAll was deleting pester-test-vm, causing 07_Snapshots,
09_CloudInit, and 15_Tasks to fail with VMID 100 (wrong VM or
nonexistent). Now 99_Cleanup handles all pester-* resource removal.

Also fixed cleanup timing:
- Use -Wait on Stop-PveVm/Container before Remove
- Add sleep after removal for API propagation
- 10_Containers AfterAll uses -Wait on stop

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 13:32:51 -05:00
Clint Branham 12874d168d refactor: split monolithic integration tests into numbered per-area files
Split the 1544-line Integration.Tests.ps1 into 18 focused test files
with numeric prefixes for execution ordering.

New shared helper (_IntegrationHelper.ps1):
- Credential-based auth (root@pam) instead of API tokens
- Skip helpers for env var checks
- Resource discovery (Find-TestVm) for cross-file dependencies
- Register-TestResource for env-var state passing between files

Files created:
  00_Connection, 01_Nodes, 02_Users, 03_Storage, 03a_SharedStorage,
  04_Network, 05_SDN, 06_VMs, 07_Snapshots, 08_Templates,
  09_CloudInit, 10_Containers, 11_LinuxVM, 12_OVA, 13_Firewall,
  14_Backup, 15_Tasks, 99_Cleanup

Key changes:
- All tests use root@pam credentials (not API tokens)
- Each file self-contained with own BeforeAll/AfterAll cleanup
- Token CRUD tests now idempotent (remove-before-create)
- 99_Cleanup is safety-net for any leftover pester-* resources
- SharedStorage renamed to 03a_SharedStorage

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 11:51:49 -05:00
copilot-swe-agent[bot] f76ff513e9 fix(storage): implement Portal defaulting for iSCSI, fix Pool/CephPool selection, add NFS format validation
Co-authored-by: GoodOlClint <151449+GoodOlClint@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoodOlClint/PSProxmoxVE/sessions/d84722e4-409e-4ced-bfbc-f323715b8bdc
2026-03-24 20:36:46 +00:00
Clint Branham c385f4a620 fix(tests): correct NFS and iSCSI storage test parameters
NFS and iSCSI are implicitly shared in PVE — the API rejects an
explicit 'shared' parameter. iSCSI uses 'portal' (not 'server') for
the target address.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 14:57:45 -05:00
Clint Branham 064cc372cf feat(storage): add -Target and -Portal parameters to New-PveStorage for iSCSI
New-PveStorage now supports configuring iSCSI storage backends natively:
  -Target: iSCSI target IQN (e.g. iqn.2024-01.com.example:storage)
  -Portal: iSCSI portal address (host:port, defaults to server:3260)

Also refactored ProcessRecord to use AddIfNotEmpty helper, reducing
cognitive complexity.

Added unit tests for all iSCSI/NFS parameter metadata and a new
SharedStorage.Tests.ps1 integration test file that tests NFS and iSCSI
storage create/verify/status/delete lifecycle against the Docker-based
storage containers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 14:43:33 -05:00
Clint Branham f21c7f84b7 feat(ci): unify CI and local integration test infrastructure
- Replace tests/infrastructure/Dockerfile with tests/Dockerfile.test
  (single multi-stage Dockerfile for both CI and local dev)
- CI container-image job now builds from Dockerfile.test target dev-infra
- Add ARM support: PowerShell installed via dotnet tool on arm64,
  APT package on amd64
- Replace tests/dev.sh (bash) with tests/dev.ps1 (PowerShell) for
  cross-platform support (Windows, macOS, Linux)
- Add -DockerHost parameter for running x86 containers on a remote
  Docker host from ARM Macs (rsyncs repo, uses SSH Docker transport)
- Add -NoCleanup switch to keep nested PVE VMs after integration tests
- integration command now provisions nested PVE VMs instead of testing
  against a pre-existing PVE directly
- Share /opt/pve-isos host path between CI and local dev (was separate
  Docker named volume)
- Delete tools/Invoke-Tests.ps1 (unused, overlapped with run-integration.sh)
- Add .gitignore entries for Terraform state/artifacts
- Update all documentation references

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-24 10:41:28 -05:00
Clint Branham d5f4e13637 fix(security): move hardcoded test password to GitHub Actions secret
- CI workflow: PVE_PASSWORD now reads from secrets.PVE_TEST_PASSWORD
- variables.tf: removed default password, requires TF_VAR env var
- Integration README: examples use <your-test-password> placeholder
- create-api-token.sh: example IP changed to pve.example.com
- .gitignore: added .env/.env.* exclusion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 17:40:47 -05:00
Clint Branham d74f8c952a fix(test): add -Confirm:$false to Suspend/Restart container Pester tests
Suspend-PveContainer and Restart-PveContainer gained ConfirmImpact.High
in F062/F063 fixes. The "Should throw when no session is active" Pester
tests invoke these cmdlets without -Confirm:$false, causing the
confirmation prompt to fire before the session check — failing in
non-interactive CI on all platforms.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 15:09:58 -05:00
Clint Branham 89bead7a0e fix: add -Confirm:$false to Restart-PveContainer integration test (F080, F081)
Restart-PveContainer gained ConfirmImpact.High in the F062 fix, which
correctly prompts for confirmation on destructive operations. The
integration test was missing -Confirm:$false, causing a
NullReferenceException in non-interactive CI. The cascading failure also
broke the Copy-PveContainer test (F081).

Also updates CONTRIBUTING.md to reference .NET SDK 10.0+ (was 9.0+) to
match all CI workflows and test project TFM (F083).

Includes scan-6 review report and findings database update (F001-F083).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 14:48:11 -05:00
Clint Branham 94424367bf fix: remediate scan-4 findings F058, F062, F063, F071, F072, F073, F074, F075
F058 (critical): Replace while(true) infinite-loop task polling with
TaskService.WaitForTask in 5 container snapshot and storage cmdlets.

F073+F047 (high): Migrate net9.0 → net10.0 across both source .csproj
files, build.yml, publish.yml, and test helper.

F071 (medium): Add Uri.EscapeDataString() to all inline URL path
segments in ~16 cmdlets that bypass service classes (D003).

F062+F063 (medium): Add ConfirmImpact.High to Restart-PveContainer
and Suspend-PveContainer (D006).

F075 (medium): Generate markdown help docs for 89 cmdlets that were
missing documentation (170 total, up from 81).

F072 (low): Remove unused System.Text.Json dependency from Core.csproj.
F074 (low): Raise publish smoke-test threshold from 60 to 150.
F065 (low): Add .github/ISSUE_TEMPLATE/config.yml.
F066 (low): Add CODEOWNERS.

Also fix _TestHelper.ps1 net9.0 → net10.0 framework reference.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-23 09:54:28 -05:00
Clint Branham d99c3e5490 chore(test): update tests for parameter changes, target net10.0
- xUnit test project now targets net10.0 instead of net9.0 (EOL Nov 2026)
- Updated GuestAgentExtCmdlets tests for SecureString Password parameter
- Added Timeout parameter tests to GuestAgentCmdlets
- Added ConfirmImpact.High assertions for Suspend/Restart-PveVm
- Added -Confirm:$false to Suspend-PveVm integration test call

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-22 08:16:45 -05:00
Clint Branham df4cae01bc feat: close API coverage gaps — 51 new cmdlets across 6 areas
Phase 1 — Missing Update/Set operations (10 cmdlets):
- SDN: Set-PveSdnZone/Vnet/Subnet/Controller/Ipam/Dns + Invoke-PveSdnApply
- Set-PveRole, Set-PveStorage, Set-PveApiToken

Phase 2 — Cluster operations (8 cmdlets):
- Get-PveClusterResource (cluster-wide inventory)
- Get-PveTaskList, Stop-PveTask
- Get/New/Set/Remove-PvePool
- Get-PveBackupInfo (unprotected VMs)

Phase 3 — VM & container gaps (14 cmdlets):
- Move-PveVmDisk, Remove-PveVmDisk
- Guest agent: Get-PveVmGuestOsInfo/FsInfo, Read/Write-PveVmGuestFile,
  Set-PveVmGuestPassword, Invoke-PveVmGuestFsTrim
- Suspend/Resume-PveContainer, Resize-PveContainerDisk,
  New-PveContainerTemplate, Move-PveContainerVolume,
  Get-PveContainerInterface

Phase 4 — Storage content management (4 cmdlets):
- Get-PveStorageStatus, Remove/Set-PveStorageContent, New-PveStorageDisk

Phase 5 — Node operations (6 cmdlets):
- Get/Set-PveNodeConfig, Get/Set-PveNodeDns, Start/Stop-PveNodeVms

Phase 6 — Access management (9 cmdlets):
- Get/New/Set/Remove-PveGroup, Get/New/Set/Remove-PveDomain, Set-PvePassword

Total: 169 cmdlets (was 118). Includes models, services, format views,
Pester unit tests, manifest updates, README, CHANGELOG, API coverage docs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-21 09:55:19 -05:00
Clint Branham ca4585f134 fix(test): add missing Url and Key params to New-PveSdnDns no-session test
New-PveSdnDns requires mandatory Url and Key parameters. The no-session
test was failing with "missing mandatory parameters" instead of reaching
the session check.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-21 09:31:40 -05:00
Clint Branham 4b503b08db fix: backup job creation requires vmid or -All, fix Enabled default
- Integration test: add -All to New-PveBackupJob call — PVE API
  requires either vmid or all when creating a backup job
- Fix New-PveBackupJob Enabled logic: jobs are enabled by default
  unless user explicitly passes -Enabled:$false (was broken because
  SwitchParameter default=true doesn't set IsPresent)
- Fix XML doc cref warning in NetworkService after RequireSdn removal

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-21 09:07:39 -05:00
Clint Branham b11f41a5cc fix(test): fix 16 Pester unit test failures in firewall cmdlets
- Add -Level Cluster to "no session" tests for cmdlets that require
  the mandatory Level parameter (13 failures)
- Fix parameter name Pos → Position in Set/Remove-PveFirewallRule
  tests (2 failures)
- Change Set-PveFirewallAlias Cidr test from mandatory to optional
  assertion — Cidr is optional on update (1 failure)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-21 08:48:36 -05:00