mirror of
https://github.com/GoodOlClint/PSProxmoxVE.git
synced 2026-08-12 15:16:52 +00:00
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>
This commit is contained in:
Clint Branham
@@ -35,12 +35,12 @@ Describe 'Firewall — Integration' -Tag 'Integration' {
|
||||
|
||||
Context 'Firewall — Cluster Rules' {
|
||||
It 'Should create a firewall rule' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ New-PveFirewallRule -Level Cluster -Type in -Action ACCEPT -Proto tcp -Dport '8080' -Comment 'pester-test-rule' -Enable -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
|
||||
It 'Should list firewall rules and find the test rule' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
$rules = Get-PveFirewallRule -Level Cluster
|
||||
$rules | Should -Not -BeNullOrEmpty
|
||||
$testRule = $rules | Where-Object { $_.Comment -eq 'pester-test-rule' }
|
||||
@@ -49,7 +49,7 @@ Describe 'Firewall — Integration' -Tag 'Integration' {
|
||||
}
|
||||
|
||||
It 'Should update the firewall rule' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
if ($null -eq $script:FirewallTestRulePos) {
|
||||
Set-ItResult -Skipped -Because 'No test rule was created'
|
||||
}
|
||||
@@ -57,7 +57,7 @@ Describe 'Firewall — Integration' -Tag 'Integration' {
|
||||
}
|
||||
|
||||
It 'Should remove the firewall rule' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
if ($null -eq $script:FirewallTestRulePos) {
|
||||
Set-ItResult -Skipped -Because 'No test rule was created'
|
||||
}
|
||||
@@ -65,7 +65,7 @@ Describe 'Firewall — Integration' -Tag 'Integration' {
|
||||
}
|
||||
|
||||
It 'Should get firewall options' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
$opts = Get-PveFirewallOptions -Level Cluster
|
||||
$opts | Should -Not -BeNullOrEmpty
|
||||
}
|
||||
@@ -73,45 +73,45 @@ Describe 'Firewall — Integration' -Tag 'Integration' {
|
||||
|
||||
Context 'Firewall — Aliases and IP Sets' {
|
||||
It 'Should create a firewall alias' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ New-PveFirewallAlias -Level Cluster -Name 'pester-alias' -Cidr '192.168.99.0/24' -Comment 'test alias' -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
|
||||
It 'Should list and find the alias' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
$aliases = Get-PveFirewallAlias -Level Cluster
|
||||
$aliases | Where-Object { $_.Name -eq 'pester-alias' } | Should -Not -BeNullOrEmpty
|
||||
}
|
||||
|
||||
It 'Should remove the alias' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ Remove-PveFirewallAlias -Level Cluster -Name 'pester-alias' -Confirm:$false -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
|
||||
It 'Should create an IP set' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ New-PveFirewallIpSet -Level Cluster -Name 'pester-ipset' -Comment 'test ipset' -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
|
||||
It 'Should add an entry to the IP set' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ New-PveFirewallIpSetEntry -Level Cluster -Name 'pester-ipset' -Cidr '10.99.0.0/16' -Comment 'test entry' -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
|
||||
It 'Should list IP set entries' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
$entries = Get-PveFirewallIpSetEntry -Level Cluster -Name 'pester-ipset'
|
||||
$entries | Should -Not -BeNullOrEmpty
|
||||
($entries | Where-Object { $_.Cidr -like '10.99.0.0*' }) | Should -Not -BeNullOrEmpty
|
||||
}
|
||||
|
||||
It 'Should remove the IP set entry' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ Remove-PveFirewallIpSetEntry -Level Cluster -Name 'pester-ipset' -Cidr '10.99.0.0/16' -Confirm:$false -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
|
||||
It 'Should remove the IP set' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ Remove-PveFirewallIpSet -Level Cluster -Name 'pester-ipset' -Confirm:$false -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,12 +22,12 @@ Describe 'Backup Jobs — Integration' -Tag 'Integration' {
|
||||
|
||||
Context 'Backup Jobs' {
|
||||
It 'Should create a backup job' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
{ New-PveBackupJob -Schedule 'sat 03:00' -Storage $script:Storage -Mode snapshot -All -Comment 'pester-test-backup' -ErrorAction Stop } | Should -Not -Throw
|
||||
}
|
||||
|
||||
It 'Should list backup jobs and find the test job' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
$jobs = Get-PveBackupJob
|
||||
$testJob = $jobs | Where-Object { $_.Comment -eq 'pester-test-backup' }
|
||||
$testJob | Should -Not -BeNullOrEmpty
|
||||
@@ -35,7 +35,7 @@ Describe 'Backup Jobs — Integration' -Tag 'Integration' {
|
||||
}
|
||||
|
||||
It 'Should update the backup job' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
if (-not $script:BackupTestJobId) {
|
||||
Set-ItResult -Skipped -Because 'No test backup job was created'
|
||||
}
|
||||
@@ -43,7 +43,7 @@ Describe 'Backup Jobs — Integration' -Tag 'Integration' {
|
||||
}
|
||||
|
||||
It 'Should remove the backup job' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
if (-not $script:BackupTestJobId) {
|
||||
Set-ItResult -Skipped -Because 'No test backup job was created'
|
||||
}
|
||||
@@ -51,7 +51,7 @@ Describe 'Backup Jobs — Integration' -Tag 'Integration' {
|
||||
}
|
||||
|
||||
It 'Should verify the backup job was removed' {
|
||||
Skip-IfNoTarget
|
||||
if (Skip-IfNoTarget) { return }
|
||||
if (-not $script:BackupTestJobId) {
|
||||
Set-ItResult -Skipped -Because 'No test backup job was created'
|
||||
}
|
||||
|
||||
+4
-1
@@ -114,9 +114,12 @@ param(
|
||||
|
||||
$ErrorActionPreference = 'Stop'
|
||||
|
||||
# If no switches specified, default to -Shell
|
||||
# If no action switches specified, default to -Shell
|
||||
$anySwitchSet = $Shell -or $Build -or $Test -or $Provision -or $Integration -or $Cleanup -or $Stop -or $Rebuild
|
||||
if (-not $anySwitchSet) {
|
||||
if ($Force -or $Reprovision) {
|
||||
throw "-Force and -Reprovision are modifiers — combine with an action switch (e.g. -Cleanup -Force, -Provision -Reprovision)."
|
||||
}
|
||||
$Shell = $true
|
||||
}
|
||||
|
||||
|
||||
@@ -707,12 +707,13 @@ main() {
|
||||
taint) cmd_taint "$@" ;;
|
||||
all) cmd_all "$@" ;;
|
||||
*)
|
||||
echo "Usage: $(basename "$0") {provision|test|cleanup|taint|all} [8|9|all] [test-filter]"
|
||||
echo "Usage: $(basename "$0") {provision|test|cleanup|force-cleanup|taint|all} [8|9|all] [test-filter]"
|
||||
echo ""
|
||||
echo "Subcommands:"
|
||||
echo " provision [8|9|all] Provision nested PVE VMs + storage containers"
|
||||
echo " test [8|9|all] [filter] Run integration tests (default: all versions, no filter)"
|
||||
echo " cleanup [8|9|all] Destroy resources via terraform destroy (default: all)"
|
||||
echo " force-cleanup [8|9|all] Bypass Terraform — destroy via API + wipe state (recovery)"
|
||||
echo " taint [8|9|all] Mark VMs for recreation on next provision"
|
||||
echo " all [8|9|all] Full lifecycle: provision → test → cleanup"
|
||||
echo ""
|
||||
|
||||
@@ -22,7 +22,7 @@ echo "Waiting for guest agent on VM ${VM_ID} (node: ${PARENT_NODE})..."
|
||||
VM_IP=""
|
||||
elapsed=0
|
||||
while [ $elapsed -lt $MAX_WAIT ]; do
|
||||
AGENT_RESPONSE=$(curl -sk \
|
||||
AGENT_RESPONSE=$(curl -sk --connect-timeout 5 --max-time 10 \
|
||||
-H "Authorization: PVEAPIToken=${PARENT_TOKEN}" \
|
||||
"${PARENT_API}/nodes/${PARENT_NODE}/qemu/${VM_ID}/agent/network-get-interfaces" 2>/dev/null || true)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user