refactor: one firewall scope validator instead of 18 copies (#204)

Collapses the ~24-line Level/Node/VmId/Group validation block duplicated
across 18 Firewall cmdlets into a single FirewallScope.TryValidate helper
in PSProxmoxVE.Core, beside FirewallService.BuildBasePath which already
owns the level-to-path mapping (ADR 0021: request-payload/validation
correctness is proven offline, not against a live cluster).

Each cmdlet now makes one TryValidate call and, on failure, one
ThrowTerminatingError with the same ErrorId (NodeRequired/VmIdRequired/
GroupRequired), ErrorCategory.InvalidArgument, target object (null) and
message text it used before, so the Pester assertions under
tests/PSProxmoxVE.Tests/Firewall/ keep passing unedited.

Part of #154.

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
This commit is contained in:
goodolclint-claude[bot]
2026-09-03 00:44:03 +00:00
committed by GitHub
parent b43948b696
commit 1f1c414632
20 changed files with 232 additions and 398 deletions
@@ -0,0 +1,49 @@
using System;
namespace PSProxmoxVE.Core.Services
{
/// <summary>
/// Validates the Level/Node/VmId/Group combination shared by the firewall cmdlets.
/// </summary>
public static class FirewallScope
{
/// <summary>
/// Returns <c>false</c> and sets <paramref name="errorId"/>/<paramref name="message"/> to the
/// first violated rule (Node, then VmId, then Group) when the identifiers required for
/// <paramref name="level"/> are missing; otherwise returns <c>true</c>.
/// </summary>
public static bool TryValidate(string level, string? node, int? vmid, string? group,
out string errorId, out string message)
{
var isCluster = string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase);
var isGroup = string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase);
var isVmOrContainer = string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase);
if (!isCluster && !isGroup && string.IsNullOrEmpty(node))
{
errorId = "NodeRequired";
message = "Node is required when Level is not Cluster.";
return false;
}
if (isVmOrContainer && !vmid.HasValue)
{
errorId = "VmIdRequired";
message = "VmId is required when Level is Vm or Container.";
return false;
}
if (isGroup && string.IsNullOrWhiteSpace(group))
{
errorId = "GroupRequired";
message = "Group is required when Level is Group.";
return false;
}
errorId = string.Empty;
message = string.Empty;
return true;
}
}
}
@@ -27,26 +27,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var session = GetSession();
@@ -27,26 +27,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var session = GetSession();
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -26,26 +25,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var session = GetSession();
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -23,26 +22,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var session = GetSession();
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -26,26 +25,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var session = GetSession();
@@ -30,37 +30,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase) &&
!string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, Group, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
{
if (string.IsNullOrWhiteSpace(Group))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Group is required when Level is Group."),
"GroupRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var session = GetSession();
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -32,26 +31,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"firewall alias '{Name}' ({Level})", "Create"))
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -29,26 +28,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"firewall IP set '{Name}' ({Level})", "Create"))
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -35,26 +34,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"IP set entry '{Cidr}' in '{Name}' ({Level})", "Create"))
@@ -65,37 +65,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase) &&
!string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, Group, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
{
if (string.IsNullOrWhiteSpace(Group))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Group is required when Level is Group."),
"GroupRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var target = string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase)
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -26,26 +25,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"firewall alias '{Name}' ({Level})", "Remove"))
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -26,26 +25,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"firewall IP set '{Name}' ({Level})", "Remove"))
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -29,26 +28,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"IP set entry '{Cidr}' from '{Name}' ({Level})", "Remove"))
@@ -29,37 +29,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase) &&
!string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, Group, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
{
if (string.IsNullOrWhiteSpace(Group))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Group is required when Level is Group."),
"GroupRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var target = string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase)
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -32,26 +31,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"firewall alias '{Name}' ({Level})", "Update"))
@@ -1,4 +1,3 @@
using System;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
using PSProxmoxVE.Core.Services;
@@ -35,26 +34,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"IP set entry '{Cidr}' in '{Name}' ({Level})", "Update"))
@@ -1,4 +1,3 @@
using System;
using System.Collections.Generic;
using System.Management.Automation;
using PSProxmoxVE.Core.Models.Firewall;
@@ -53,26 +52,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, null, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
if (!ShouldProcess($"firewall options ({Level})", "Update"))
@@ -68,37 +68,11 @@ namespace PSProxmoxVE.Cmdlets.Firewall
protected override void ProcessRecord()
{
var level = Level;
if (!string.Equals(level, "Cluster", StringComparison.OrdinalIgnoreCase) &&
!string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
if (!FirewallScope.TryValidate(level, Node, VmId, Group, out var scopeErrorId, out var scopeMessage))
{
if (string.IsNullOrEmpty(Node))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Node is required when Level is not Cluster."),
"NodeRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Vm", StringComparison.OrdinalIgnoreCase) ||
string.Equals(level, "Container", StringComparison.OrdinalIgnoreCase))
{
if (!VmId.HasValue)
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("VmId is required when Level is Vm or Container."),
"VmIdRequired", ErrorCategory.InvalidArgument, null));
return;
}
}
if (string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase))
{
if (string.IsNullOrWhiteSpace(Group))
{
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException("Group is required when Level is Group."),
"GroupRequired", ErrorCategory.InvalidArgument, null));
return;
}
ThrowTerminatingError(new ErrorRecord(
new PSArgumentException(scopeMessage), scopeErrorId, ErrorCategory.InvalidArgument, null));
return;
}
var target = string.Equals(level, "Group", StringComparison.OrdinalIgnoreCase)
@@ -326,5 +326,116 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.Throws<ArgumentNullException>("group", () => service.RemoveGroupRule(CreateSession(), " ", 0));
}
// -------------------------------------------------------------------------
// FirewallScope.TryValidate
// -------------------------------------------------------------------------
[Theory]
[InlineData("Cluster", null, null, null)]
[InlineData("cluster", null, null, null)]
[InlineData("Node", "pve1", null, null)]
[InlineData("NODE", "pve1", null, null)]
[InlineData("Vm", "pve1", 100, null)]
[InlineData("vm", "pve1", 100, null)]
[InlineData("Container", "pve1", 101, null)]
[InlineData("Group", null, null, "web-servers")]
[InlineData("GROUP", null, null, "web-servers")]
public void TryValidate_WithRequiredIdentifiers_ReturnsTrue(string level, string? node, int? vmid, string? group)
{
var result = FirewallScope.TryValidate(level, node, vmid, group, out var errorId, out var message);
Assert.True(result);
Assert.Equal(string.Empty, errorId);
Assert.Equal(string.Empty, message);
}
[Theory]
[InlineData("Node")]
[InlineData("Vm")]
[InlineData("Container")]
public void TryValidate_MissingNode_ReturnsFalseWithNodeRequired(string level)
{
var result = FirewallScope.TryValidate(level, null, 100, null, out var errorId, out var message);
Assert.False(result);
Assert.Equal("NodeRequired", errorId);
Assert.Equal("Node is required when Level is not Cluster.", message);
}
[Fact]
public void TryValidate_WhitespaceNode_IsAcceptedLikeTheOriginalIsNullOrEmptyCheck()
{
var result = FirewallScope.TryValidate("Node", " ", null, null, out var errorId, out var message);
Assert.True(result);
Assert.Equal(string.Empty, errorId);
Assert.Equal(string.Empty, message);
}
[Fact]
public void TryValidate_VmMissingNodeAndVmId_ReturnsFalseWithNodeRequired()
{
var result = FirewallScope.TryValidate("Vm", null, null, null, out var errorId, out var message);
Assert.False(result);
Assert.Equal("NodeRequired", errorId);
Assert.Equal("Node is required when Level is not Cluster.", message);
}
[Theory]
[InlineData("Vm")]
[InlineData("Container")]
public void TryValidate_MissingVmId_ReturnsFalseWithVmIdRequired(string level)
{
var result = FirewallScope.TryValidate(level, "pve1", null, null, out var errorId, out var message);
Assert.False(result);
Assert.Equal("VmIdRequired", errorId);
Assert.Equal("VmId is required when Level is Vm or Container.", message);
}
[Fact]
public void TryValidate_GroupMissingGroup_ReturnsFalseWithGroupRequired()
{
var result = FirewallScope.TryValidate("Group", null, null, null, out var errorId, out var message);
Assert.False(result);
Assert.Equal("GroupRequired", errorId);
Assert.Equal("Group is required when Level is Group.", message);
}
[Fact]
public void TryValidate_GroupWhitespaceGroup_ReturnsFalseWithGroupRequired()
{
var result = FirewallScope.TryValidate("Group", null, null, " ", out var errorId, out var message);
Assert.False(result);
Assert.Equal("GroupRequired", errorId);
Assert.Equal("Group is required when Level is Group.", message);
}
// -------------------------------------------------------------------------
// GetRules base path (cross-checked against an accepted FirewallScope.TryValidate scope)
// -------------------------------------------------------------------------
[Theory]
[InlineData("Cluster", null, null, "cluster/firewall")]
[InlineData("Node", "pve1", null, "nodes/pve1/firewall")]
[InlineData("Vm", "pve1", 100, "nodes/pve1/qemu/100/firewall")]
[InlineData("Container", "pve1", 101, "nodes/pve1/lxc/101/firewall")]
public void GetRules_AcceptedScope_UsesExpectedBasePath(
string level, string? node, int? vmid, string expectedPath)
{
Assert.True(FirewallScope.TryValidate(level, node, vmid, null, out _, out _));
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync(It.IsAny<string>())).ReturnsAsync(RulesJson());
var service = new FirewallService(mockClient.Object);
service.GetRules(CreateSession(), level, node, vmid);
mockClient.Verify(c => c.GetAsync($"{expectedPath}/rules"), Times.Once);
}
}
}