fix(server): map network interface plan to state using indices and ad…

…dresses (#667)

Co-authored-by: Ville Välimäki <[email protected]>

CHANGELOG.md

@@ -8,10 +8,12 @@ See updating [Changelog example here](https://keepachangelog.com/en/1.0.0/)
 ### Added
 
 - upcloud_managed_object_storage_bucket resource for managing buckets in managed object storage services.
+- upcloud_server: `index` field to `network_interfaces`.
 
 ### Changed
 
 - upcloud_managed_database_\*: Update available properties to match listing provided by the API, see [#626](https://github.com/UpCloudLtd/terraform-provider-upcloud/pull/626) for details.
+- upcloud_server: When modifying `network_interfaces`, match configured network interfaces to the server's actual network interfaces by `index` and `ip_address` (in addition to list order). This is to avoid public and utility network interfaces being re-assigned when the interfaces are re-ordered or when interface is removed from middle of the list. This might result to inaccurate diffs in the Terraform plan when interfaces are re-ordered or when interface is removed from middle of the list. We recommend explicitly setting the value for `index` in configuration, when interfaces are re-ordered or when interface is removed from middle of the list.
 
 ## [5.14.0] - 2024-10-28
 

docs/resources/server.md

@@ -77,7 +77,11 @@ resource "upcloud_server" "example" {
 ### Blocks
 
 - `login` (Block Set, Max: 1) Configure access credentials to the server (see [below for nested schema](#nestedblock--login))
-- `network_interface` (Block List, Min: 1) One or more blocks describing the network interfaces of the server. (see [below for nested schema](#nestedblock--network_interface))
+- `network_interface` (Block List, Min: 1) One or more blocks describing the network interfaces of the server.
+
+    In addition to list order, the configured network interfaces are matched to the server's actual network interfaces by `index` and `ip_address` fields. This is to avoid public and utility network interfaces being re-assigned when the server is updated. This might result to inaccurate diffs in the plan, when interfaces are re-ordered or when interface is removed from the middle of the list.
+
+    We recommend explicitly setting the value for `index` in configuration, when re-ordering interfaces or when removing interface from middle of the list. (see [below for nested schema](#nestedblock--network_interface))
 - `simple_backup` (Block Set, Max: 1) Simple backup schedule configuration
 
     The simple backups provide a simplified way to back up *all* of the storages attached to a given server. This means you cannot have simple backup set for a server, and individual `backup_rules` on the storages attached to the server. Such configuration will throw an error during execution. This also applies to `backup_rules` defined for server templates.
@@ -111,6 +115,7 @@ Required Attributes:
 Optional Attributes:
 
 - `bootable` (Boolean) `true` if this interface should be used for network booting.
+- `index` (Number) The interface index.
 - `ip_address` (String) The assigned primary IP address.
 - `ip_address_family` (String) The type of the primary IP address of this interface (one of `IPv4` or `IPv6`).
 - `network` (String) The unique ID of a network to attach this network to.

internal/service/database/properties/schema.go

@@ -96,8 +96,8 @@ func stringSlice(val interface{}) ([]string, bool) {
 func getKeyDiffSuppressFunc(key string) schema.SchemaDiffSuppressFunc {
 	switch key {
 	case "ip_filter":
-		return func(_, old, new string, _ *schema.ResourceData) bool {
-			return strings.TrimSuffix(old, "/32") == strings.TrimSuffix(new, "/32")
+		return func(_, oldValue, newValue string, _ *schema.ResourceData) bool {
+			return strings.TrimSuffix(oldValue, "/32") == strings.TrimSuffix(newValue, "/32")
 		}
 	default:
 		return nil

internal/service/server/interface_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud"
-	"github.com/UpCloudLtd/upcloud-go-api/v8/upcloud/request"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -55,48 +54,3 @@ func TestResolveInterfaceIPAddress(t *testing.T) {
 	}, want)
 	assert.Error(t, err)
 }
-
-func TestInterfacesEquals(t *testing.T) {
-	assert.False(t, interfacesEquals(upcloud.ServerInterface{
-		Index:       1,
-		IPAddresses: []upcloud.IPAddress{},
-		Type:        "",
-	}, request.CreateNetworkInterfaceRequest{
-		Index:       0,
-		IPAddresses: []request.CreateNetworkInterfaceIPAddress{},
-		Type:        "",
-	}))
-	assert.False(t, interfacesEquals(upcloud.ServerInterface{
-		Index:       0,
-		IPAddresses: []upcloud.IPAddress{},
-		Type:        upcloud.NetworkTypePublic,
-	}, request.CreateNetworkInterfaceRequest{
-		Index:       0,
-		IPAddresses: []request.CreateNetworkInterfaceIPAddress{},
-		Type:        upcloud.NetworkTypePrivate,
-	}))
-	assert.False(t, interfacesEquals(upcloud.ServerInterface{
-		Index: 0,
-		IPAddresses: []upcloud.IPAddress{{
-			Family: upcloud.IPAddressFamilyIPv4,
-		}},
-		Type: upcloud.NetworkTypePublic,
-	}, request.CreateNetworkInterfaceRequest{
-		Index:       0,
-		IPAddresses: []request.CreateNetworkInterfaceIPAddress{},
-		Type:        upcloud.NetworkTypePublic,
-	}))
-	assert.True(t, interfacesEquals(upcloud.ServerInterface{
-		Index: 0,
-		IPAddresses: []upcloud.IPAddress{{
-			Family: upcloud.IPAddressFamilyIPv4,
-		}},
-		Type: upcloud.NetworkTypePublic,
-	}, request.CreateNetworkInterfaceRequest{
-		Index: 0,
-		IPAddresses: []request.CreateNetworkInterfaceIPAddress{{
-			Family: upcloud.IPAddressFamilyIPv4,
-		}},
-		Type: upcloud.NetworkTypePublic,
-	}))
-}