fix: address Copilot review feedback on PR #27

- Extract ParseLinks helper to PveCmdletBase for shared link parsing
  with WriteWarning on malformed entries (was duplicated in 3 cmdlets)
- Fix GetClusterConfig to return data payload, not full API envelope
- Fix OutputType on GetPveClusterConfigCmdlet to JObject
- Fix link doc comments to use correct key format (link0..link7)
- Add null-safe Properties hashtable conversion in HA rule cmdlets
- Use case-insensitive Mode comparison in MovePveHaResourceCmdlet
- Remove unused using directives

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clint Branham
2026-03-24 18:40:10 -05:00
parent da2037d0c5
commit f1a3676723
9 changed files with 43 additions and 42 deletions
@@ -41,7 +41,8 @@ namespace PSProxmoxVE.Core.Services
try
{
var response = client.GetAsync("cluster/config").GetAwaiter().GetResult();
return JObject.Parse(response);
var data = JObject.Parse(response)["data"];
return data as JObject ?? new JObject();
}
finally
{
@@ -54,7 +55,7 @@ namespace PSProxmoxVE.Core.Services
/// </summary>
/// <param name="session">The authenticated PVE session.</param>
/// <param name="clusterName">The name for the new cluster.</param>
/// <param name="links">Optional Corosync link addresses (e.g., "0=10.0.0.1,1=10.0.1.1").</param>
/// <param name="links">Optional Corosync link addresses, using keys link0..link7 (e.g., "link0=10.0.0.1").</param>
/// <param name="nodeid">Optional node ID for this node.</param>
/// <param name="votes">Optional number of quorum votes for this node.</param>
/// <returns>The UPID of the cluster creation task.</returns>
@@ -1,4 +1,3 @@
using System.Collections.Generic;
using System.Management.Automation;
using PSProxmoxVE.Core.Services;
@@ -55,17 +54,7 @@ namespace PSProxmoxVE.Cmdlets.Cluster
var session = GetSession();
var service = new ClusterConfigService();
Dictionary<string, string>? linkDict = null;
if (Links != null)
{
linkDict = new Dictionary<string, string>();
foreach (var link in Links)
{
var parts = link.Split(new[] { '=' }, 2);
if (parts.Length == 2)
linkDict[parts[0]] = parts[1];
}
}
var linkDict = ParseLinks(Links);
WriteVerbose($"Adding node '{Node}' to cluster configuration...");
var upid = service.AddConfigNode(session, Node, NewNodeIp, linkDict, NodeId, Votes,
@@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using System.Management.Automation;
using System.Runtime.InteropServices;
using System.Security;
@@ -64,17 +63,7 @@ namespace PSProxmoxVE.Cmdlets.Cluster
ptr = Marshal.SecureStringToGlobalAllocUnicode(Password);
var plainPassword = Marshal.PtrToStringUni(ptr)!;
Dictionary<string, string>? linkDict = null;
if (Links != null)
{
linkDict = new Dictionary<string, string>();
foreach (var link in Links)
{
var parts = link.Split(new[] { '=' }, 2);
if (parts.Length == 2)
linkDict[parts[0]] = parts[1];
}
}
var linkDict = ParseLinks(Links);
WriteVerbose($"Joining cluster via '{Hostname}'...");
var upid = service.JoinCluster(session, Hostname, Fingerprint, plainPassword,
@@ -12,7 +12,7 @@ namespace PSProxmoxVE.Cmdlets.Cluster
/// </para>
/// </summary>
[Cmdlet(VerbsCommon.Get, "PveClusterConfig")]
[OutputType(typeof(PSObject))]
[OutputType(typeof(JObject))]
public sealed class GetPveClusterConfigCmdlet : PveCmdletBase
{
protected override void ProcessRecord()
@@ -1,4 +1,3 @@
using System.Collections.Generic;
using System.Management.Automation;
using PSProxmoxVE.Core.Services;
@@ -44,17 +43,7 @@ namespace PSProxmoxVE.Cmdlets.Cluster
var session = GetSession();
var service = new ClusterConfigService();
Dictionary<string, string>? linkDict = null;
if (Links != null)
{
linkDict = new Dictionary<string, string>();
foreach (var link in Links)
{
var parts = link.Split(new[] { '=' }, 2);
if (parts.Length == 2)
linkDict[parts[0]] = parts[1];
}
}
var linkDict = ParseLinks(Links);
WriteVerbose($"Creating cluster '{ClusterName}'...");
var upid = service.CreateCluster(session, ClusterName, linkDict, NodeId, Votes);
@@ -39,7 +39,7 @@ namespace PSProxmoxVE.Cmdlets.HA
var service = new HaService();
WriteVerbose($"{Mode} HA resource '{Sid}' to node '{Node}'...");
if (Mode == "Relocate")
if (string.Equals(Mode, "Relocate", System.StringComparison.OrdinalIgnoreCase))
service.RelocateResource(session, Sid, Node);
else
service.MigrateResource(session, Sid, Node);
@@ -47,7 +47,11 @@ namespace PSProxmoxVE.Cmdlets.HA
if (Properties != null)
{
foreach (var key in Properties.Keys)
data[key.ToString()!] = Properties[key]!.ToString()!;
{
var value = Properties[key]?.ToString();
if (key != null && value != null)
data[key.ToString()!] = value;
}
}
WriteVerbose($"Creating HA rule of type '{Type}'...");
@@ -48,7 +48,11 @@ namespace PSProxmoxVE.Cmdlets.HA
if (Properties != null)
{
foreach (var key in Properties.Keys)
data[key.ToString()!] = Properties[key]!.ToString()!;
{
var value = Properties[key]?.ToString();
if (key != null && value != null)
data[key.ToString()!] = value;
}
}
WriteVerbose($"Updating HA rule '{Rule}'...");
+25
View File
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Management.Automation;
using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Authentication;
@@ -158,5 +159,29 @@ namespace PSProxmoxVE.Cmdlets
task.Upid ?? "unknown",
TimeSpan.FromSeconds(timeoutSeconds));
}
/// <summary>
/// Parses an array of Corosync link strings (e.g. "link0=10.0.0.1") into a dictionary.
/// Emits a warning for entries that do not match the expected "key=value" format.
/// </summary>
/// <param name="links">Array of link strings in "linkN=address" format.</param>
/// <returns>Dictionary of parsed link entries, or null if input is null.</returns>
protected Dictionary<string, string>? ParseLinks(string[]? links)
{
if (links == null) return null;
var result = new Dictionary<string, string>();
foreach (var link in links)
{
var parts = link.Split(new[] { '=' }, 2);
if (parts.Length != 2 || string.IsNullOrWhiteSpace(parts[0]) || string.IsNullOrWhiteSpace(parts[1]))
{
WriteWarning($"Ignoring malformed link entry '{link}'. Expected format: 'link0=10.0.0.1'");
continue;
}
result[parts[0].Trim()] = parts[1].Trim();
}
return result.Count > 0 ? result : null;
}
}
}