329 Commits

Author SHA1 Message Date
Clint Branham d81c850b0b fix: validate API enum values against PVE OpenAPI spec
Add 70 xUnit tests that validate every ValidateSet in the module
against the PVE OpenAPI spec. Three bugs found and fixed:

- Storage: remove `glusterfs` (dropped in PVE 9), add `btrfs`, `esxi`
- Backup compression: `none` → `0` (PVE uses "0" not "none")
- Cluster resources: remove `lxc` filter (PVE uses `vm` for both)

The pve-api-enums.json fixture (199KB) is extracted from the full
OpenAPI spec and contains parameter enum values for 302 API paths.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-26 15:34:56 -05:00
GoodOlClint 5ceb2469c4 Merge pull request #40 from GoodOlClint/fix/publish-module-path
fix: publish workflow module path must match module name
v0.1.0-preview
2026-03-26 09:24:38 -05:00
Clint Branham 1f09ae6e94 fix: publish workflow module path must match module name
Publish-Module -Path requires the directory name to match the module
name. Changed artifact download path from ./publish/netstandard2.0/
to ./publish/PSProxmoxVE/ so Publish-Module can find the manifest.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-26 09:19:41 -05:00
GoodOlClint c92fb42d64 Merge pull request #39 from GoodOlClint/fix/ha-rule-type-test
chore: scan-9 review + publish secret fix
2026-03-26 09:13:26 -05:00
Clint Branham 13e0c7bdb1 fix: grant write permissions to Claude Code workflows
- claude-code-review.yml: pull-requests/issues read → write (post reviews)
- claude.yml: contents/pull-requests/issues read → write (respond to @claude)
- Upgrade checkout@v4 → v6, fetch-depth: 1 → 0 for full diff context
- Skip draft PRs in review workflow
- Remove boilerplate comments

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-26 09:11:54 -05:00
GoodOlClint 0cc493a561 Merge branch 'main' into fix/ha-rule-type-test 2026-03-26 08:59:28 -05:00
Clint Branham 197e206123 chore: scan-9 review findings + fix publish secret name
- Resolve F039 (bare catch regression), F053 (HA coverage), F060
  (cluster config coverage), F084 (PveSession secret hiding)
- Update findings.json: 11 open → 7 open, 0 regressions
- Update REVIEW_REPORT.md with scan-9 results
- Fix publish.yml to use NUGET_API_KEY secret (matches other projects)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-26 08:58:35 -05:00
GoodOlClint 008b6b67d3 Merge pull request #38 from GoodOlClint/fix/ha-rule-type-test
fix: remove ValidateSet assertion from Set-PveHaRule Type test
2026-03-26 08:22:22 -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
GoodOlClint d37569533d Merge pull request #36 from GoodOlClint/test/cluster-ha-tests 2026-03-25 19:51:55 -05:00
GoodOlClint 4ceb976136 Merge pull request #37 from GoodOlClint/copilot/sub-pr-36 2026-03-25 19:50:20 -05:00
copilot-swe-agent[bot] b1f93e655a fix: set task.Node from UPID + expand SetPveHaRule Type + extract GetNodeFromUpid helper
Co-authored-by: GoodOlClint <151449+GoodOlClint@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoodOlClint/PSProxmoxVE/sessions/42467843-de5e-4603-99e3-fb182eaf4251
2026-03-26 00:26:00 +00:00
copilot-swe-agent[bot] a9f6360fd2 Initial plan 2026-03-26 00:20:03 +00: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 598e9edef2 fix: replace FormUrlEncodedContent with minimal form encoding
.NET's FormUrlEncodedContent over-encodes characters like : and !
(%3A, %21) in form values. PVE's internal API consumers (specifically
the cluster join process) do not URL-decode these values before using
them, causing fingerprint comparison failures and password mismatches.

Replace with BuildFormContent/EncodeFormValue that only encodes
characters that break form parsing (&, =, +, space, %). This matches
curl's -d behavior and fixes cluster join "Cluster join aborted!" errors.

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 8c131a0609 fix: re-authenticate during cluster join -Wait on 401
Cluster join restarts auth services on the joining node, which
invalidates the PVE ticket mid-poll. When WaitForTask gets a 401,
catch it and re-authenticate with the password already available
in the cmdlet, then retry the wait.

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
GoodOlClint 4310869943 Merge pull request #35 from GoodOlClint/fix/answer-server-mount-v2
fix: mount answers/ and default.toml separately, not over /app
2026-03-25 17:23:13 -05:00
Clint Branham 8f36593b35 fix: always use shared mount for WORK_DIR, never RUNNER_TEMP
RUNNER_TEMP (/__w/_temp in CI) is container-local and invisible to
the Docker host. Files written there can't be bind-mounted into
sibling containers (answer server, storage). This was the root cause
of the "not a directory" mount failures in CI.

Removed RUNNER_TEMP from the WORK_DIR fallback chain. WORK_DIR now
always defaults to CACHE_DIR/work (/opt/pve-integration/work/) which
is on the shared mount visible to both the CI container and Docker host.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:22:00 -05:00
Clint Branham 0077a2d055 fix: mount answers/ and default.toml separately, not over /app
Mounting the whole directory to /app overwrites server.py inside the
container, causing "can't open file '/app/server.py'" errors.

Mount the two paths individually instead:
- answer_server_dir/answers → /app/answers
- answer_server_dir/default.toml → /app/default.toml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:18:47 -05:00
GoodOlClint 3cd8d02142 Merge pull request #34 from GoodOlClint/fix/answer-server-mount
fix: mount answer server directory instead of individual file
2026-03-25 17:14:32 -05:00
Clint Branham 6e9361eb7c fix: mount answer server directory instead of individual file
Docker bind mount fails when mounting a file that doesn't exist on
the host (creates a directory instead). In CI, the container's
$WORK_DIR path differs from the host path, so the file mount failed.

Changed to mount a single answer-server/ directory containing both
default.toml and answers/ subdirectory. Replaced two TF variables
(answer_files_dir, default_answer_file) with one (answer_server_dir).

Layout: $WORK_DIR/answer-server/default.toml + answers/<mac>.toml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:13:14 -05:00
GoodOlClint 9e17036169 Merge pull request #33 from GoodOlClint/fix/answer-server-dependency
fix: PVE VMs depend on answer server container
2026-03-25 17:07:48 -05:00
Clint Branham e90fb760f6 fix: use force-cleanup in CI for reliable teardown
CI cleanup should always succeed regardless of Terraform state
(cancelled runs, timeouts, partial failures). force-cleanup uses
direct API calls + docker rm instead of terraform destroy.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:06:22 -05:00
Clint Branham 226f9d8a12 fix: store Terraform state on shared mount for CI persistence
Terraform state was stored in the container's working directory
(fresh checkout), so it was lost between CI jobs. The cleanup job
couldn't destroy resources because it had no state.

Now stores state at /opt/pve-integration/work/terraform.tfstate via
-state flag on all terraform commands. This persists across the
provision → test → cleanup job chain in GitHub Actions.

Also:
- Force cleanup now removes state from both local and shared paths
- Added -reconfigure to terraform init (avoids backend mismatch errors)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:04:38 -05:00
Clint Branham 94d5fe9944 fix: PVE VMs depend on answer server container
Terraform creates resources in parallel. Without depends_on, PVE VMs
could boot and start the auto-installer before the HTTP answer server
container is running, causing "could not find answer file" errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 17:01:45 -05:00
GoodOlClint 1ccc046ade Merge pull request #31 from GoodOlClint/refactor/test-orchestration
refactor: redesign dev.ps1 with switch-based CLI and split integration tests
2026-03-25 16:28:20 -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
GoodOlClint ec9fde14bd Merge branch 'main' into refactor/test-orchestration 2026-03-25 16:21:40 -05:00
GoodOlClint 46d39b48c8 Merge pull request #32 from GoodOlClint/add-claude-github-actions-1774473566584
Add Claude Code GitHub Workflow
2026-03-25 16:21:15 -05:00
GoodOlClint 8555d6854e "Claude Code Review workflow" 2026-03-25 16:19:28 -05:00
GoodOlClint 33636027b7 "Claude PR Assistant workflow" 2026-03-25 16:19:27 -05:00
Clint Branham 03f6cc86eb fix: address Copilot review round 2
- Update header comments: PVETEST_APITOKEN → PVETEST_PASSWORD,
  CACHE_DIR default → /opt/pve-integration
- Pin ubuntu and NFS server Docker images to SHA256 digests
- Fix cmd_all to pass version to provision and cleanup
- Keep .terraform.lock.hcl in force cleanup (provider reproducibility)
- Remove || true from terraform destroy in cleanup (propagate errors;
  use -Force for best-effort recovery)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 16:00:16 -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 7cb64811b7 fix: pin answer server image digest + fix taint key mismatch
Copilot review fixes:
- Pin answer server Docker image to SHA256 digest instead of :latest
  for reproducible builds
- Fix cmd_taint: ISO resources are keyed by version ("9") not node
  ("9a"), so taint was no-op. Now taints ISOs by version and VMs
  by node separately.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 15:38:21 -05:00
Clint Branham a241714125 fix: update CloudInit regenerate test for PUT endpoint
Test was still expecting GetAsync on cloudinit/dump but the service
now calls PutAsync on cloudinit (regenerate endpoint). Updated mock
setup and assertions to match.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 15:32:25 -05:00
Clint Branham 5cfd33d921 fix: update GitHub workflow for credential auth + pve-integration paths
- Volume mounts: /opt/pve-isos → /opt/pve-integration
- CACHE_DIR env: /opt/pve-isos → /opt/pve-integration
- Test step: PVETEST_APITOKEN → PVETEST_PASSWORD (credential auth)
- Cleanup step: add PVE_PASSWORD (needed by terraform destroy)
- Remove PVETEST_APITOKEN from optional secrets docs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 15:27:48 -05:00
Clint Branham 61dca7acb1 chore: add apt-get upgrade to first-boot script
Ensures nested PVE nodes are fully patched before integration tests
run. Adds ~5-10min to first provision but gives more realistic test
results against current PVE releases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 15:05:24 -05:00
Clint Branham 589181e77c fix: use shared host mount for answer files + rename to pve-integration
WORK_DIR now defaults to /opt/pve-integration/work (under the shared
host mount) instead of /tmp/pve-integration (container-local).

This fixes the answer server seeing empty answer files — the dev-infra
container writes answer files to WORK_DIR, and the answer server
container (a sibling) needs to read them from the same host path.

Also:
- Renamed mount from /opt/pve-isos to /opt/pve-integration
- Added must_run=true, start=true to answer server container
- Updated docker-compose.test.yml and dev.ps1 remote override

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 14:49:59 -05:00