feat(v): Add ability to override node IP used for SSH connection (#355)

* feat(v): Add ability to override node IP used for SSH connection

* add documentation

docs/index.md

@@ -97,6 +97,36 @@ Note that the target node is identified by the `node` argument in the resource,
 and may be different from the Proxmox API endpoint. Please refer to the
 section below for all the available arguments in the `ssh` block.
 
+#### Node IP address used for SSH connection
+
+In order to make the SSH connection, the provider needs to know the IP address
+of the node specified in the resource. The provider will attempt to resolve the
+node name to an IP address using Proxmox API to enumerate the node network
+interfaces, and use the first one that is not a loopback interface. In some
+cases this may not be the desired behavior, for example when the node has
+multiple network interfaces, and the one that should be used for SSH is not the
+first one.
+
+To override the node IP address used for SSH connection, you can use the
+optional
+`node` blocks in the `ssh` block. For example:
+
+```terraform
+  ssh {
+  agent    = true
+  username = "root"
+  node {
+    name    = "pve1"
+    address = "192.168.10.1"
+  }
+  node {
+    name    = "pve2"
+    address = "192.168.10.2"
+  }
+}
+
+```
+
 ### API Token authentication
 
 API Token authentication can be used to authenticate with the Proxmox API
@@ -160,8 +190,7 @@ Proxmox `provider` block:
   Environment API (can also be sourced from `PROXMOX_VE_API_TOKEN`). For
   example, `root@pam!for-terraform-provider=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx`.
 - `ssh` - (Optional) The SSH connection configuration to a Proxmox node. This is
-  a
+  a block, whose fields are documented below.
-  block, whose fields are documented below.
     - `username` - (Optional) The username to use for the SSH connection.
       Defaults to the username used for the Proxmox API connection. Can also be
       sourced from `PROXMOX_VE_SSH_USERNAME`. Required when using API Token.
@@ -174,3 +203,7 @@ Proxmox `provider` block:
     - `agent_socket` - (Optional) The path to the SSH agent socket.
       Defaults to the value of the `SSH_AUTH_SOCK` environment variable. Can
       also be sourced from `PROXMOX_VE_SSH_AUTH_SOCK`.
+    - `node` - (Optional) The node configuration for the SSH connection. Can be
+      specified multiple times to provide configuration fo multiple nodes.
+        - `name` - (Required) The name of the node.
+        - `address` - (Required) The IP address of the node.

proxmox/api/client.go

@@ -25,10 +25,32 @@ import (
 	"github.com/bpg/terraform-provider-proxmox/utils"
 )
 
+// ErrNoDataObjectInResponse is returned when the server does not include a data object in the response.
+var ErrNoDataObjectInResponse = errors.New("the server did not include a data object in the response")
+
 const (
 	basePathJSONAPI = "api2/json"
 )
 
+// Client is an interface for performing requests against the Proxmox API.
+type Client interface {
+	// DoRequest performs a request against the Proxmox API.
+	DoRequest(
+		ctx context.Context,
+		method, path string,
+		requestBody, responseBody interface{},
+	) error
+
+	// ExpandPath expands a path relative to the client's base path.
+	// For example, if the client is configured for a VM and the
+	// path is "firewall/options", the returned path will be
+	// "/nodes/<node>/qemu/<vmid>/firewall/options".
+	ExpandPath(path string) string
+
+	// IsRoot returns true if the client is configured with the root user.
+	IsRoot() bool
+}
+
 // Connection represents a connection to the Proxmox Virtual Environment API.
 type Connection struct {
 	endpoint   string

proxmox/api/client_types.go

@@ -7,34 +7,10 @@
 package api
 
 import (
-	"context"
-	"errors"
 	"io"
 	"os"
 )
 
-// ErrNoDataObjectInResponse is returned when the server does not include a data object in the response.
-var ErrNoDataObjectInResponse = errors.New("the server did not include a data object in the response")
-
-// Client is an interface for performing requests against the Proxmox API.
-type Client interface {
-	// DoRequest performs a request against the Proxmox API.
-	DoRequest(
-		ctx context.Context,
-		method, path string,
-		requestBody, responseBody interface{},
-	) error
-
-	// ExpandPath expands a path relative to the client's base path.
-	// For example, if the client is configured for a VM and the
-	// path is "firewall/options", the returned path will be
-	// "/nodes/<node>/qemu/<vmid>/firewall/options".
-	ExpandPath(path string) string
-
-	// IsRoot returns true if the client is configured with the root user.
-	IsRoot() bool
-}
-
 // MultiPartData enables multipart uploads in DoRequest.
 type MultiPartData struct {
 	Boundary string

proxmox/ssh/client.go

@@ -27,15 +27,30 @@ import (
 	"github.com/bpg/terraform-provider-proxmox/utils"
 )
 
+// Client is an interface for performing SSH requests against the Proxmox Nodes.
+type Client interface {
+	// ExecuteNodeCommands executes a command on a node.
+	ExecuteNodeCommands(ctx context.Context, nodeName string, commands []string) error
+
+	// NodeUpload uploads a file to a node.
+	NodeUpload(ctx context.Context, nodeName string,
+		remoteFileDir string, fileUploadRequest *api.FileUploadRequest) error
+}
+
 type client struct {
 	username    string
 	password    string
 	agent       bool
 	agentSocket string
+	nodeLookup  NodeResolver
 }
 
 // NewClient creates a new SSH client.
-func NewClient(username string, password string, agent bool, agentSocket string) (Client, error) {
+func NewClient(
+	username string, password string,
+	agent bool, agentSocket string,
+	nodeLookup NodeResolver,
+) (Client, error) {
 	//goland:noinspection GoBoolExpressions
 	if agent && runtime.GOOS != "linux" && runtime.GOOS != "darwin" && runtime.GOOS != "freebsd" {
 		return nil, errors.New(
@@ -44,19 +59,34 @@ func NewClient(username string, password string, agent bool, agentSocket string)
 		)
 	}
 
+	if nodeLookup == nil {
+		return nil, errors.New("node lookup is required")
+	}
+
 	return &client{
 		username:    username,
 		password:    password,
 		agent:       agent,
 		agentSocket: agentSocket,
+		nodeLookup:  nodeLookup,
 	}, nil
 }
 
 // ExecuteNodeCommands executes commands on a given node.
-func (c *client) ExecuteNodeCommands(ctx context.Context, nodeAddress string, commands []string) error {
+func (c *client) ExecuteNodeCommands(ctx context.Context, nodeName string, commands []string) error {
+	ip, err := c.nodeLookup.Resolve(ctx, nodeName)
+	if err != nil {
+		return fmt.Errorf("failed to find node endpoint: %w", err)
+	}
+
+	tflog.Debug(ctx, "executing commands on the node using SSH", map[string]interface{}{
+		"node_address": ip,
+		"commands":     commands,
+	})
+
 	closeOrLogError := utils.CloseOrLogError(ctx)
 
-	sshClient, err := c.openNodeShell(ctx, nodeAddress)
+	sshClient, err := c.openNodeShell(ctx, ip)
 	if err != nil {
 		return err
 	}
@@ -86,12 +116,19 @@ func (c *client) ExecuteNodeCommands(ctx context.Context, nodeAddress string, co
 }
 
 func (c *client) NodeUpload(
-	ctx context.Context, nodeAddress string, remoteFileDir string,
+	ctx context.Context,
+	nodeName string,
+	remoteFileDir string,
 	d *api.FileUploadRequest,
 ) error {
-	// We need to upload all other files using SFTP due to API limitations.
+	ip, err := c.nodeLookup.Resolve(ctx, nodeName)
-	// Hopefully, this will not be required in future releases of Proxmox VE.
+	if err != nil {
-	tflog.Debug(ctx, "uploading file to datastore using SFTP", map[string]interface{}{
+		return fmt.Errorf("failed to find node endpoint: %w", err)
+	}
+
+	tflog.Debug(ctx, "uploading file to the node datastore using SFTP", map[string]interface{}{
+		"node_address": ip,
+		"remote_dir":   remoteFileDir,
 		"file_name":    d.FileName,
 		"content_type": d.ContentType,
 	})
@@ -103,7 +140,7 @@ func (c *client) NodeUpload(
 
 	fileSize := fileInfo.Size()
 
-	sshClient, err := c.openNodeShell(ctx, nodeAddress)
+	sshClient, err := c.openNodeShell(ctx, ip)
 	if err != nil {
 		return fmt.Errorf("failed to open SSH client: %w", err)
 	}
@@ -237,7 +274,7 @@ func (c *client) openNodeShell(ctx context.Context, nodeAddress string) (*ssh.Cl
 		HostKeyAlgorithms: kh.HostKeyAlgorithms(sshHost),
 	}
 
-	tflog.Info(ctx, fmt.Sprintf("Agent is set to %t", c.agent))
+	tflog.Info(ctx, fmt.Sprintf("agent is set to %t", c.agent))
 
 	var sshClient *ssh.Client
 	if c.agent {

proxmox/ssh/client_types.go

@@ -1,28 +0,0 @@
-/*
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/.
- */
-
-package ssh
-
-import (
-	"context"
-
-	"github.com/bpg/terraform-provider-proxmox/proxmox/api"
-)
-
-// Client is an interface for performing SSH requests against the Proxmox Nodes.
-type Client interface {
-	// ExecuteNodeCommands executes a command on a node.
-	ExecuteNodeCommands(
-		ctx context.Context, nodeAddress string,
-		commands []string,
-	) error
-
-	// NodeUpload uploads a file to a node.
-	NodeUpload(
-		ctx context.Context, nodeAddress string,
-		remoteFileDir string, fileUploadRequest *api.FileUploadRequest,
-	) error
-}

proxmox/ssh/resolver.go

@@ -0,0 +1,16 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+package ssh
+
+import (
+	"context"
+)
+
+// NodeResolver is an interface for resolving node names to IP addresses to use for SSH connection.
+type NodeResolver interface {
+	Resolve(ctx context.Context, nodeName string) (string, error)
+}

proxmoxtf/provider/provider.go

@@ -8,32 +8,18 @@ package provider
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 
 	"github.com/bpg/terraform-provider-proxmox/proxmox/api"
+	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/ssh"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
 )
 
-const (
-	dvProviderOTP                = ""
-	mkProviderVirtualEnvironment = "virtual_environment"
-	mkProviderEndpoint           = "endpoint"
-	mkProviderInsecure           = "insecure"
-	mkProviderOTP                = "otp"
-	mkProviderPassword           = "password"
-	mkProviderUsername           = "username"
-	mkProviderAPIToken           = "api_token"
-	mkProviderSSH                = "ssh"
-	mkProviderSSHUsername        = "username"
-	mkProviderSSHPassword        = "password"
-	mkProviderSSHAgent           = "agent"
-	mkProviderSSHAgentSocket     = "agent_socket"
-)
-
 // ProxmoxVirtualEnvironment returns the object for this provider.
 func ProxmoxVirtualEnvironment() *schema.Provider {
 	return &schema.Provider{
@@ -124,11 +110,24 @@ func providerConfigure(ctx context.Context, d *schema.ResourceData) (interface{}
 		sshConf[mkProviderSSHAgentSocket] = ""
 	}
 
+	nodeOverrides := map[string]string{}
+
+	if ns, ok := sshConf[mkProviderSSHNode]; ok {
+		for _, n := range ns.([]interface{}) {
+			node := n.(map[string]interface{})
+			nodeOverrides[node[mkProviderSSHNodeName].(string)] = node[mkProviderSSHNodeAddress].(string)
+		}
+	}
+
 	sshClient, err = ssh.NewClient(
 		sshConf[mkProviderSSHUsername].(string),
 		sshConf[mkProviderSSHPassword].(string),
 		sshConf[mkProviderSSHAgent].(bool),
 		sshConf[mkProviderSSHAgentSocket].(string),
+		&apiResolverWithOverrides{
+			ar:        apiResolver{c: apiClient},
+			overrides: nodeOverrides,
+		},
 	)
 	if err != nil {
 		return nil, diag.Errorf("error creating SSH client: %s", err)
@@ -138,3 +137,31 @@ func providerConfigure(ctx context.Context, d *schema.ResourceData) (interface{}
 
 	return config, nil
 }
+
+type apiResolver struct {
+	c api.Client
+}
+
+func (r *apiResolver) Resolve(ctx context.Context, nodeName string) (string, error) {
+	nc := &nodes.Client{Client: r.c, NodeName: nodeName}
+
+	ip, err := nc.GetIP(ctx)
+	if err != nil {
+		return "", fmt.Errorf("failed to get node IP: %w", err)
+	}
+
+	return ip, nil
+}
+
+type apiResolverWithOverrides struct {
+	ar        apiResolver
+	overrides map[string]string
+}
+
+func (r *apiResolverWithOverrides) Resolve(ctx context.Context, nodeName string) (string, error) {
+	if ip, ok := r.overrides[nodeName]; ok {
+		return ip, nil
+	}
+
+	return r.ar.Resolve(ctx, nodeName)
+}

proxmoxtf/provider/schema.go

@@ -15,6 +15,26 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
 
+const (
+	dvProviderOTP                = ""
+	mkProviderVirtualEnvironment = "virtual_environment"
+	mkProviderEndpoint           = "endpoint"
+	mkProviderInsecure           = "insecure"
+	mkProviderOTP                = "otp"
+	mkProviderPassword           = "password"
+	mkProviderUsername           = "username"
+	mkProviderAPIToken           = "api_token"
+	mkProviderSSH                = "ssh"
+	mkProviderSSHUsername        = "username"
+	mkProviderSSHPassword        = "password"
+	mkProviderSSHAgent           = "agent"
+	mkProviderSSHAgentSocket     = "agent_socket"
+
+	mkProviderSSHNode        = "node"
+	mkProviderSSHNodeName    = "name"
+	mkProviderSSHNodeAddress = "address"
+)
+
 func createSchema() map[string]*schema.Schema {
 	providerSchema := nestedProviderSchema()
 	providerSchema[mkProviderVirtualEnvironment] = &schema.Schema{
@@ -173,6 +193,29 @@ func nestedProviderSchema() map[string]*schema.Schema {
 						),
 						ValidateFunc: validation.StringIsNotEmpty,
 					},
+					mkProviderSSHNode: {
+						Type:        schema.TypeList,
+						Optional:    true,
+						MinItems:    0,
+						MaxItems:    1,
+						Description: "Overrides for SSH connection configuration to a Proxmox node",
+						Elem: &schema.Resource{
+							Schema: map[string]*schema.Schema{
+								mkProviderSSHNodeName: {
+									Type:         schema.TypeString,
+									Required:     true,
+									Description:  "The name of the node to connect to",
+									ValidateFunc: validation.StringIsNotEmpty,
+								},
+								mkProviderSSHNodeAddress: {
+									Type:         schema.TypeString,
+									Required:     true,
+									Description:  "The address that should be used to connect to the node",
+									ValidateFunc: validation.IsIPAddress,
+								},
+							},
+						},
+					},
 				},
 			},
 		},

proxmoxtf/resource/file.go

@@ -394,11 +394,6 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 	default:
 		// For all other content types, we need to upload the file to the node's
 		// datastore using SFTP.
-		nodeAddress, err2 := capi.Node(nodeName).GetIP(ctx)
-		if err2 != nil {
-			return diag.Errorf("failed to get node IP: %s", err2)
-		}
-
 		datastore, err2 := capi.Storage().GetDatastore(ctx, datastoreID)
 		if err2 != nil {
 			return diag.Errorf("failed to get datastore: %s", err2)
@@ -410,7 +405,7 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag
 
 		remoteFileDir := *datastore.Path
 
-		err = capi.SSH().NodeUpload(ctx, nodeAddress, remoteFileDir, request)
+		err = capi.SSH().NodeUpload(ctx, nodeName, remoteFileDir, request)
 	}
 
 	if err != nil {

proxmoxtf/resource/vm.go

@@ -2229,12 +2229,7 @@ func vmCreateCustomDisks(ctx context.Context, d *schema.ResourceData, m interfac
 
 		nodeName := d.Get(mkResourceVirtualEnvironmentVMNodeName).(string)
 
-		nodeAddress, err := api.Node(nodeName).GetIP(ctx)
+		err = api.SSH().ExecuteNodeCommands(ctx, nodeName, commands)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-
-		err = api.SSH().ExecuteNodeCommands(ctx, nodeAddress, commands)
 		if err != nil {
 			return diag.FromErr(err)
 		}