gsadmin/PSProxmoxVE
1
0
Fork 0
mirror of https://github.com/GoodOlClint/PSProxmoxVE.git synced 2026-09-04 03:05:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: route the CloudInit/Templates cmdlets through their service seams (#126) (#200)

Browse Source 
Get-PveCloudInitConfig and Get-PveTemplate each built their own PveHttpClient
inline. They now go through CloudInitService and TemplateService, which is
offline-testable per ADR 0021.

Get-PveCloudInitConfig's shipped output is the full PveVmConfig (per its own
synopsis), not the CI-key-filtered PveCloudInitConfig that
CloudInitService.GetCloudInitConfig already returns for a different purpose.
CloudInitService gains GetFullVmConfig, a thin delegation to the existing
VmService.GetVmConfig (already used by Get-PveVmConfig) rather than a
parallel HTTP implementation.

Get-PveTemplate's TemplateService.GetTemplates now forwards VmService.GetVms's
onNodeSkipped callback, matching Get-PveVm/Get-PveContainer, so an unreachable
node during the all-nodes listing produces a warning instead of a silently
short result. Two review-found regressions were fixed before commit: an empty
-Node value used to mean "query all nodes" and now does again (it had
started reaching the API literally as nodes//qemu), and the per-node warning
above.

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:11:50 +00:00
committed by GitHub
parent 3fa7eeb023
commit 3f7f69b250
6 changed files with 215 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/PSProxmoxVE.Core/Services/CloudInitService.cs
+26 -2
View File
@@ -22,16 +22,40 @@ namespace PSProxmoxVE.Core.Services
"nameserver", "searchdomain", "cicustom"
};
private readonly VmService _vmService;
/// <summary>
/// Initializes a new instance of the <see cref="CloudInitService"/> class.
/// </summary>
public CloudInitService() { }
public CloudInitService()
{
_vmService = new VmService();
}
/// <summary>
/// Initializes a new instance of the <see cref="CloudInitService"/> class with an injected HTTP client.
/// </summary>
/// <param name="client">The HTTP client to use for API calls. The caller owns its lifetime.</param>
public CloudInitService(IPveHttpClient client) : base(client) { }
public CloudInitService(IPveHttpClient client) : base(client)
{
_vmService = new VmService(client);
}
/// <summary>
/// Returns the full VM configuration (delegates to <see cref="VmService.GetVmConfig"/>).
/// Cloud-Init fields (ciuser, ipconfig*, etc.) are present as ordinary properties of
/// the returned config alongside every other setting.
/// </summary>
/// <param name="session">The authenticated PVE session.</param>
/// <param name="node">The cluster node name.</param>
/// <param name="vmid">The VM ID.</param>
public PveVmConfig GetFullVmConfig(PveSession session, string node, int vmid)
{
if (session == null) throw new ArgumentNullException(nameof(session));
if (string.IsNullOrWhiteSpace(node)) throw new ArgumentNullException(nameof(node));
return _vmService.GetVmConfig(session, node, vmid);
}
/// <summary>
/// Retrieves the Cloud-Init specific configuration fields for a VM.
src/PSProxmoxVE.Core/Services/TemplateService.cs
+6 -2
View File
@@ -37,11 +37,15 @@ namespace PSProxmoxVE.Core.Services
/// </summary>
/// <param name="session">The authenticated PVE session.</param>
/// <param name="node">Optional cluster node name to filter templates by node.</param>
public PveVm[] GetTemplates(PveSession session, string? node = null)
/// <param name="onNodeSkipped">
/// Optional callback invoked with the node name and the exception when a node is
/// skipped because it is unreachable, forwarded to <see cref="VmService.GetVms"/>.
/// </param>
public PveVm[] GetTemplates(PveSession session, string? node = null, Action<string, Exception>? onNodeSkipped = null)
{
if (session == null) throw new ArgumentNullException(nameof(session));
var vms = _vmService.GetVms(session, node);
var vms = _vmService.GetVms(session, node, onNodeSkipped);
return vms.Where(v => v.Template == 1).ToArray();
}
src/PSProxmoxVE/Cmdlets/CloudInit/GetPveCloudInitConfigCmdlet.cs
+3 -9
View File
@@ -1,8 +1,6 @@
using System;
using System.Management.Automation;
using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Models.Vms;
using PSProxmoxVE.Core.Services;
namespace PSProxmoxVE.Cmdlets.CloudInit
{
@@ -31,14 +29,10 @@ namespace PSProxmoxVE.Cmdlets.CloudInit
{
var session = GetSession();
RequireVersion(session, "Cloud-Init management", 7, 2);
using var client = new PveHttpClient(session);
var service = new CloudInitService();
WriteVerbose($"Getting cloud-init config for VM {VmId}...");
var json = client.GetAsync($"nodes/{Uri.EscapeDataString(Node)}/qemu/{VmId}/config").GetAwaiter().GetResult();
var root = JObject.Parse(json);
var data = root["data"];
var config = data?.ToObject<PveVmConfig>() ?? new PveVmConfig();
var config = service.GetFullVmConfig(session, Node, VmId);
WriteObject(config);
}
}
src/PSProxmoxVE/Cmdlets/Templates/GetPveTemplateCmdlet.cs
+19 -45
View File
@@ -1,8 +1,6 @@
using System;
using System.Management.Automation;
using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Models.Vms;
using PSProxmoxVE.Core.Services;
namespace PSProxmoxVE.Cmdlets.Templates
{
@@ -28,57 +26,33 @@ namespace PSProxmoxVE.Cmdlets.Templates
protected override void ProcessRecord()
{
var session = GetSession();
using var client = new PveHttpClient(session);
var service = new TemplateService();
WriteVerbose("Getting templates...");
var nodesToQuery = new System.Collections.Generic.List<string>();
var queryNode = string.IsNullOrEmpty(Node) ? null : Node;
var templates = service.GetTemplates(session, queryNode,
onNodeSkipped: (nodeName, ex) => WriteWarning($"Skipping node '{nodeName}': {ex.Message}"));
if (!string.IsNullOrEmpty(Node))
foreach (var vm in templates)
{
nodesToQuery.Add(Node!);
}
else
{
var nodesJson = client.GetAsync("nodes").GetAwaiter().GetResult();
var nodesRoot = JObject.Parse(nodesJson);
var nodesData = nodesRoot["data"] as JArray ?? new JArray();
foreach (var n in nodesData)
nodesToQuery.Add(n["node"]?.ToString() ?? string.Empty);
}
if (vm.Node == null && queryNode != null) vm.Node = queryNode;
foreach (var node in nodesToQuery)
{
if (string.IsNullOrEmpty(node)) continue;
var json = client.GetAsync($"nodes/{Uri.EscapeDataString(node)}/qemu").GetAwaiter().GetResult();
var root = JObject.Parse(json);
var data = root["data"] as JArray ?? new JArray();
foreach (var item in data)
if (!string.IsNullOrEmpty(Name) && vm.Name != null)
{
var vm = item.ToObject<PveVm>()!;
if (vm.Node == null) vm.Node = node;
// Only return templates
if (vm.Template != 1) continue;
if (!string.IsNullOrEmpty(Name) && vm.Name != null)
var pattern = Name!.Replace("*", "");
if (Name.Contains("*"))
{
var pattern = Name!.Replace("*", "");
if (Name.Contains("*"))
{
if (vm.Name.IndexOf(pattern, System.StringComparison.OrdinalIgnoreCase) < 0)
continue;
}
else
{
if (!string.Equals(vm.Name, Name, System.StringComparison.OrdinalIgnoreCase))
continue;
}
if (vm.Name.IndexOf(pattern, System.StringComparison.OrdinalIgnoreCase) < 0)
continue;
}
else
{
if (!string.Equals(vm.Name, Name, System.StringComparison.OrdinalIgnoreCase))
continue;
}
WriteObject(vm);
}
WriteObject(vm);
}
}
}
tests/PSProxmoxVE.Core.Tests/Services/CloudInitServiceTests.cs
+49
View File
@@ -156,5 +156,54 @@ namespace PSProxmoxVE.Core.Tests.Services
{
Assert.Throws<ArgumentNullException>(() => new CloudInitService(null!));
}
[Fact]
public void GetFullVmConfig_ReturnsFullConfig_IncludingCloudInitAndOtherFields()
{
// Arrange
var json = @"{""data"": {
""ciuser"": ""ubuntu"",
""ipconfig0"": ""ip=dhcp"",
""cores"": 4,
""memory"": 8192
}}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes/pve1/qemu/100/config")).ReturnsAsync(json);
var service = new CloudInitService(mockClient.Object);
// Act
var config = service.GetFullVmConfig(CreateSession(), "pve1", 100);
// Assert — cloud-init fields and every other config field are both present
Assert.Equal("ubuntu", config.CiUser);
Assert.Equal("ip=dhcp", config.IpConfig0);
Assert.Equal(4, config.Cores);
Assert.Equal(8192, config.Memory);
mockClient.Verify(c => c.GetAsync("nodes/pve1/qemu/100/config"), Times.Once);
}
[Fact]
public void GetFullVmConfig_EscapesNodeInPath()
{
// Arrange
var json = @"{""data"": {}}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes/pve%20node/qemu/100/config")).ReturnsAsync(json);
var service = new CloudInitService(mockClient.Object);
// Act
service.GetFullVmConfig(CreateSession(), "pve node", 100);
// Assert
mockClient.Verify(c => c.GetAsync("nodes/pve%20node/qemu/100/config"), Times.Once);
}
[Fact]
public void GetFullVmConfig_NullSession_ThrowsArgumentNullException()
{
var service = new CloudInitService(new Mock<IPveHttpClient>().Object);
Assert.Throws<ArgumentNullException>(() => service.GetFullVmConfig(null!, "pve1", 100));
}
}
}
tests/PSProxmoxVE.Core.Tests/Services/TemplateServiceTests.cs
+112
View File
@@ -1,10 +1,12 @@
using System;
using System.Collections.Generic;
using System.Net;
using System.Threading.Tasks;
using Moq;
using Xunit;
using PSProxmoxVE.Core.Authentication;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Exceptions;
using PSProxmoxVE.Core.Services;
namespace PSProxmoxVE.Core.Tests.Services
@@ -101,5 +103,115 @@ namespace PSProxmoxVE.Core.Tests.Services
{
Assert.Throws<ArgumentNullException>("client", () => new TemplateService(null!));
}
[Fact]
public void GetTemplates_SingleNode_ReturnsOnlyTemplateFlaggedVms()
{
// Arrange
var json = @"{""data"": [
{""vmid"": 100, ""name"": ""web-template"", ""template"": 1},
{""vmid"": 101, ""name"": ""running-vm"", ""template"": 0},
{""vmid"": 102, ""name"": ""db-template"", ""template"": 1}
]}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync($"nodes/{Node}/qemu")).ReturnsAsync(json);
var service = new TemplateService(mockClient.Object);
// Act
var templates = service.GetTemplates(CreateSession(), Node);
// Assert
Assert.Equal(2, templates.Length);
Assert.All(templates, t => Assert.Equal(1, t.Template));
Assert.Contains(templates, t => t.VmId == 100);
Assert.Contains(templates, t => t.VmId == 102);
mockClient.Verify(c => c.GetAsync($"nodes/{Node}/qemu"), Times.Once);
}
[Fact]
public void GetTemplates_EscapesNodeInPath()
{
// Arrange
var json = @"{""data"": []}";
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes/pve%20node/qemu")).ReturnsAsync(json);
var service = new TemplateService(mockClient.Object);
// Act
service.GetTemplates(CreateSession(), "pve node");
// Assert
mockClient.Verify(c => c.GetAsync("nodes/pve%20node/qemu"), Times.Once);
}
[Fact]
public void GetTemplates_NullSession_ThrowsArgumentNullException()
{
var service = new TemplateService(new Mock<IPveHttpClient>().Object);
Assert.Throws<ArgumentNullException>("session", () => service.GetTemplates(null!, Node));
}
[Fact]
public void GetTemplates_AllNodes_AggregatesAcrossNodesAndStampsNode()
{
// Arrange
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes"))
.ReturnsAsync(@"{""data"": [{""node"": ""pve1""}, {""node"": ""pve2""}]}");
mockClient.Setup(c => c.GetAsync("nodes/pve1/qemu"))
.ReturnsAsync(@"{""data"": [{""vmid"": 100, ""template"": 1}, {""vmid"": 101, ""template"": 0}]}");
mockClient.Setup(c => c.GetAsync("nodes/pve2/qemu"))
.ReturnsAsync(@"{""data"": [{""vmid"": 200, ""template"": 1}]}");
var service = new TemplateService(mockClient.Object);
// Act
var templates = service.GetTemplates(CreateSession());
// Assert
Assert.Equal(2, templates.Length);
Assert.Contains(templates, t => t.VmId == 100 && t.Node == "pve1");
Assert.Contains(templates, t => t.VmId == 200 && t.Node == "pve2");
}
[Fact]
public void GetTemplates_AllNodes_UnreachableNodeIsSkippedAndReported()
{
// Arrange
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes"))
.ReturnsAsync(@"{""data"": [{""node"": ""pve1""}, {""node"": ""pve2""}]}");
mockClient.Setup(c => c.GetAsync("nodes/pve1/qemu"))
.ReturnsAsync(@"{""data"": [{""vmid"": 100, ""template"": 1}]}");
mockClient.Setup(c => c.GetAsync("nodes/pve2/qemu"))
.ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, "internal error", "nodes/pve2/qemu", "GET"));
var service = new TemplateService(mockClient.Object);
// Act
var skipped = new List<string>();
var templates = service.GetTemplates(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node));
// Assert
var template = Assert.Single(templates);
Assert.Equal(100, template.VmId);
Assert.Equal(new[] { "pve2" }, skipped);
}
[Fact]
public void GetTemplates_AllNodes_PermissionErrorOnOneNodePropagates()
{
// Arrange
var mockClient = new Mock<IPveHttpClient>();
mockClient.Setup(c => c.GetAsync("nodes"))
.ReturnsAsync(@"{""data"": [{""node"": ""pve1""}, {""node"": ""pve2""}]}");
mockClient.Setup(c => c.GetAsync("nodes/pve1/qemu"))
.ReturnsAsync(@"{""data"": [{""vmid"": 100, ""template"": 1}]}");
mockClient.Setup(c => c.GetAsync("nodes/pve2/qemu"))
.ThrowsAsync(new PveApiException(HttpStatusCode.Forbidden, "permission denied", "nodes/pve2/qemu", "GET"));
var service = new TemplateService(mockClient.Object);
// Act & Assert
Assert.Throws<PveApiException>(() => service.GetTemplates(CreateSession()));
}
}
}