fix(rdb): fix instance and read replica endpoints by enforcing IP config choice (#2400)

Author: Mia-Cross

docs/resources/rdb_instance.md

@@ -79,7 +79,7 @@ resource "scaleway_rdb_instance" "main" {
   private_network {
     pn_id  = scaleway_vpc_private_network.pn.id
     ip_net = "172.16.20.4/22"   # IP address within a given IP network
-    (enable_ipam = false)
+    # enable_ipam = false
   }
 }
 ```
@@ -94,7 +94,7 @@ resource "scaleway_rdb_instance" "main" {
   engine         = "PostgreSQL-11"
   private_network {
     pn_id = scaleway_vpc_private_network.pn.id
-    (enable_ipam = true)
+    enable_ipam = true
   }
   load_balancer {}
 }
@@ -179,8 +179,8 @@ Please consult the [GoDoc](https://pkg.go.dev/github.com/scaleway/scaleway-sdk-g
 
     - `pn_id` - (Required) The ID of the private network.
     - `enable_ipam` - (Optional) Whether the endpoint should be configured with IPAM. Defaults to `false` if `ip_net` is defined, `true` otherwise.
-    - `ip_net` - (Optional) The IP network address within the private subnet. This must be an IPv4 address with a CIDR notation.
-    The IP network address within the private subnet is determined by the IP Address Management (IPAM) service if not set.
+    - `ip_net` - (Optional) The IP network address within the private subnet. This must be an IPv4 address with a CIDR notation. If not set, The IP network address within the private subnet is determined by the IP Address Management (IPAM) service.
+    - `enable_ipam` - (Optional) If true, the IP network address within the private subnet is determined by the IP Address Management (IPAM) service.
 
 ~> **NOTE:** Please calculate your host IP using [cidrhost](https://developer.hashicorp.com/terraform/language/functions/cidrhost). Otherwise, let IPAM service
 handle the host IP on the network.

docs/resources/rdb_read_replica.md

@@ -30,7 +30,7 @@ resource scaleway_rdb_read_replica "replica" {
 }
 ```
 
-### Private network
+### Private network with static endpoint
 
 ```terraform
 resource "scaleway_rdb_instance" "instance" {
@@ -49,7 +49,32 @@ resource "scaleway_rdb_read_replica" "replica" {
   instance_id = scaleway_rdb_instance.instance.id
   private_network {
     private_network_id = scaleway_vpc_private_network.pn.id
-    service_ip         = "192.168.1.254/24" // omit this attribute if private IP is determined by the IP Address Management (IPAM)
+    service_ip         = "192.168.1.254/24"
+    # enable_ipam = false
+  }
+}
+```
+
+### Private network with IPAM
+
+```terraform
+resource "scaleway_rdb_instance" "instance" {
+  name           = "rdb_instance"
+  node_type      = "db-dev-s"
+  engine         = "PostgreSQL-14"
+  is_ha_cluster  = false
+  disable_backup = true
+  user_name      = "my_initial_user"
+  password       = "thiZ_is_v&ry_s3cret"
+}
+
+resource "scaleway_vpc_private_network" "pn" {}
+
+resource "scaleway_rdb_read_replica" "replica" {
+  instance_id = scaleway_rdb_instance.instance.id
+  private_network {
+    private_network_id = scaleway_vpc_private_network.pn.id
+    enable_ipam = true
   }
 }
 ```
@@ -66,9 +91,8 @@ The following arguments are supported:
 
 - `private_network` - (Optional) Create an endpoint in a private network.
     - `private_network_id` - (Required) UUID of the private network to be connected to the read replica.
-    - `service_ip` - (Optional) The IP network address within the private subnet. This must be an IPv4 address with a
-      CIDR notation. The IP network address within the private subnet is determined by the IP Address Management (IPAM)
-      service if not set.
+    - `service_ip` - (Optional) The IP network address within the private subnet. This must be an IPv4 address with a CIDR notation. If not set, The IP network address within the private subnet is determined by the IP Address Management (IPAM) service.
+    - `enable_ipam` - (Optional) If true, the IP network address within the private subnet is determined by the IP Address Management (IPAM) service.
 
 - `same_zone` - (Defaults to `true`) Defines whether to create the replica in the same availability zone as the main instance nodes or not.
 
@@ -96,6 +120,7 @@ they are of the form `{region}/{id}`, e.g. `fr-par/11111111-1111-1111-1111-11111
     - `port` - TCP port of the endpoint.
     - `name` - Name of the endpoint.
     - `hostname` - Hostname of the endpoint. Only one of ip and hostname may be set.
+    - `enable_ipam` - Indicates whether the IP is managed by IPAM.
 
 ## Import
 

scaleway/data_source_ipam_ip_test.go

@@ -151,6 +151,7 @@ func TestAccScalewayDataSourceIPAMIP_RDB(t *testing.T) {
 						volume_size_in_gb = 10
 						private_network {
 							pn_id = "${scaleway_vpc_private_network.main.id}"
+							enable_ipam = true
 						}
 					}
 

scaleway/helpers_rdb.go

@@ -110,35 +110,42 @@ func waitForRDBReadReplica(ctx context.Context, api *rdb.API, region scw.Region,
 	}, scw.WithContext(ctx))
 }
 
-func expandPrivateNetwork(data interface{}, exist bool, enableIpam bool) ([]*rdb.EndpointSpec, error) {
+func expandPrivateNetwork(data interface{}, exist bool, ipamConfig *bool, staticConfig *string) ([]*rdb.EndpointSpec, diag.Diagnostics) {
 	if data == nil || !exist {
 		return nil, nil
 	}
+	var diags diag.Diagnostics
 
 	res := make([]*rdb.EndpointSpec, 0, len(data.([]interface{})))
 	for _, pn := range data.([]interface{}) {
 		r := pn.(map[string]interface{})
 		spec := &rdb.EndpointSpec{
 			PrivateNetwork: &rdb.EndpointSpecPrivateNetwork{
 				PrivateNetworkID: expandID(r["pn_id"].(string)),
+				IpamConfig:       &rdb.EndpointSpecPrivateNetworkIpamConfig{},
 			},
 		}
-		if enableIpam {
-			spec.PrivateNetwork.IpamConfig = &rdb.EndpointSpecPrivateNetworkIpamConfig{}
-		} else {
-			ipNet := r["ip_net"].(string)
-			if len(ipNet) > 0 {
-				ip, err := expandIPNet(r["ip_net"].(string))
-				if err != nil {
-					return res, err
-				}
-				spec.PrivateNetwork.ServiceIP = &ip
+
+		if staticConfig != nil {
+			ip, err := expandIPNet(*staticConfig)
+			if err != nil {
+				return nil, append(diags, diag.FromErr(fmt.Errorf("failed to parse private_network ip_net (%s): %s", r["ip_net"], err))...)
 			}
+			spec.PrivateNetwork.ServiceIP = &ip
+			spec.PrivateNetwork.IpamConfig = nil
+			if ipamConfig != nil && *ipamConfig {
+				diags = append(diags, diag.Diagnostic{
+					Severity: diag.Warning,
+					Detail:   "`ip_net` field is set so `enable_ipam` field will be ignored",
+				})
+			}
+		} else if ipamConfig == nil || !*ipamConfig {
+			return nil, diag.FromErr(errors.New("at least one of `ip_net` or `enable_ipam` (set to true) must be set"))
 		}
 		res = append(res, spec)
 	}
 
-	return res, nil
+	return res, diags
 }
 
 func expandLoadBalancer() *rdb.EndpointSpec {
@@ -221,34 +228,41 @@ func expandReadReplicaEndpointsSpecDirectAccess(data interface{}) *rdb.ReadRepli
 }
 
 // expandReadReplicaEndpointsSpecPrivateNetwork expand read-replica private network endpoints from schema to specs
-func expandReadReplicaEndpointsSpecPrivateNetwork(data interface{}, enableIpam bool) (*rdb.ReadReplicaEndpointSpec, error) {
+func expandReadReplicaEndpointsSpecPrivateNetwork(data interface{}, ipamConfig *bool, staticConfig *string) (*rdb.ReadReplicaEndpointSpec, diag.Diagnostics) {
 	if data == nil || len(data.([]interface{})) == 0 {
 		return nil, nil
 	}
 	// private_network is a list of size 1
 	data = data.([]interface{})[0]
 
 	rawEndpoint := data.(map[string]interface{})
+	var diags diag.Diagnostics
 
-	endpoint := new(rdb.ReadReplicaEndpointSpec)
-	endpoint.PrivateNetwork = &rdb.ReadReplicaEndpointSpecPrivateNetwork{
-		PrivateNetworkID: expandID(rawEndpoint["private_network_id"]),
+	endpoint := &rdb.ReadReplicaEndpointSpec{
+		PrivateNetwork: &rdb.ReadReplicaEndpointSpecPrivateNetwork{
+			PrivateNetworkID: expandID(rawEndpoint["private_network_id"]),
+			IpamConfig:       &rdb.ReadReplicaEndpointSpecPrivateNetworkIpamConfig{},
+		},
 	}
 
-	if enableIpam {
-		endpoint.PrivateNetwork.IpamConfig = &rdb.ReadReplicaEndpointSpecPrivateNetworkIpamConfig{}
-	} else {
-		serviceIP := rawEndpoint["service_ip"].(string)
-		if len(serviceIP) > 0 {
-			ipNet, err := expandIPNet(serviceIP)
-			if err != nil {
-				return nil, fmt.Errorf("failed to parse private_network service_ip (%s): %w", rawEndpoint["service_ip"], err)
-			}
-			endpoint.PrivateNetwork.ServiceIP = &ipNet
+	if staticConfig != nil {
+		ipNet, err := expandIPNet(*staticConfig)
+		if err != nil {
+			return nil, append(diags, diag.FromErr(fmt.Errorf("failed to parse private_network service_ip (%s): %s", rawEndpoint["service_ip"], err))...)
+		}
+		endpoint.PrivateNetwork.ServiceIP = &ipNet
+		endpoint.PrivateNetwork.IpamConfig = nil
+		if ipamConfig != nil && !*ipamConfig {
+			diags = append(diags, diag.Diagnostic{
+				Severity: diag.Warning,
+				Detail:   "`service_ip` field is set so `enable_ipam` field will be ignored",
+			})
 		}
+	} else if ipamConfig == nil || !*ipamConfig {
+		return nil, diag.FromErr(errors.New("at least one of `service_ip` or `enable_ipam` (set to true) must be set"))
 	}
 
-	return endpoint, nil
+	return endpoint, diags
 }
 
 // flattenReadReplicaEndpoints flatten read-replica endpoints to directAccess and privateNetwork
@@ -326,33 +340,76 @@ func rdbPrivilegeUpgradeV1SchemaType() cty.Type {
 	})
 }
 
-func isIpamEndpoint(resource interface{}, meta interface{}) (bool, error) {
+func getIPConfigCreate(d *schema.ResourceData, ipFieldName string) (ipamConfig *bool, staticConfig *string) {
+	enableIpam, enableIpamSet := d.GetOk("private_network.0.enable_ipam")
+	if enableIpamSet {
+		ipamConfig = expandBoolPtr(enableIpam)
+	}
+	customIP, customIPSet := d.GetOk("private_network.0." + ipFieldName)
+	if customIPSet {
+		staticConfig = expandStringPtr(customIP)
+	}
+	return ipamConfig, staticConfig
+}
+
+// getIPConfigUpdate forces the provider to read the user's config instead of checking the state, because "enable_ipam" is not readable from the API
+func getIPConfigUpdate(d *schema.ResourceData, ipFieldName string) (ipamConfig *bool, staticConfig *string) {
+	if rawConfig := d.GetRawConfig(); !rawConfig.IsNull() {
+		pnRawConfig := rawConfig.AsValueMap()["private_network"].AsValueSlice()[0].AsValueMap()
+		if !pnRawConfig["enable_ipam"].IsNull() {
+			if pnRawConfig["enable_ipam"].False() {
+				ipamConfig = scw.BoolPtr(false)
+			} else {
+				ipamConfig = scw.BoolPtr(true)
+			}
+		}
+		if !pnRawConfig[ipFieldName].IsNull() {
+			value := pnRawConfig[ipFieldName].AsString()
+			staticConfig = &value
+		}
+	}
+	return ipamConfig, staticConfig
+}
+
+func getIPAMConfigRead(resource interface{}, meta interface{}) (bool, error) {
 	ipamAPI := ipam.NewAPI(meta.(*Meta).scwClient)
 	request := &ipam.ListIPsRequest{
 		ResourceType: "rdb_instance",
 		IsIPv6:       scw.BoolPtr(false),
 	}
+	var privateEndpoint *rdb.EndpointPrivateNetworkDetails
 
 	switch res := resource.(type) {
 	case *rdb.Instance:
 		request.Region = res.Region
 		request.ResourceID = &res.ID
+		for _, e := range res.Endpoints {
+			if e.PrivateNetwork != nil {
+				privateEndpoint = e.PrivateNetwork
+			}
+		}
 	case *rdb.ReadReplica:
 		request.Region = res.Region
 		request.ResourceID = &res.InstanceID
+		for _, e := range res.Endpoints {
+			if e.PrivateNetwork != nil {
+				privateEndpoint = e.PrivateNetwork
+			}
+		}
+	}
+	if privateEndpoint == nil {
+		return false, nil
 	}
 
 	ips, err := ipamAPI.ListIPs(request, scw.WithAllPages())
 	if err != nil {
 		return false, fmt.Errorf("could not list IPs: %w", err)
 	}
 
-	switch ips.TotalCount {
-	case 1:
-		return true, nil
-	case 0:
-		return false, nil
-	default:
-		return false, fmt.Errorf("expected no more than 1 IP for instance, got %d", ips.TotalCount)
+	for _, ip := range ips.IPs {
+		if ip.Address.String() == privateEndpoint.ServiceIP.String() {
+			return true, nil
+		}
 	}
+	return false, nil
 }

scaleway/resource_rdb_instance.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -150,12 +151,6 @@ func resourceScalewayRdbInstance() *schema.Resource {
 							DiffSuppressFunc: diffSuppressFuncLocality,
 							Description:      "The private network ID",
 						},
-						"enable_ipam": {
-							Type:        schema.TypeBool,
-							Optional:    true,
-							Computed:    true,
-							Description: "Whether or not the private network endpoint should be configured with IPAM",
-						},
 						// Computed
 						"endpoint_id": {
 							Type:        schema.TypeString,
@@ -191,6 +186,12 @@ func resourceScalewayRdbInstance() *schema.Resource {
 							Computed:    true,
 							Description: "The hostname of your endpoint",
 						},
+						"enable_ipam": {
+							Type:        schema.TypeBool,
+							Optional:    true,
+							Computed:    true,
+							Description: "Whether or not the private network endpoint should be configured with IPAM",
+						},
 						"zone": zoneSchema(),
 					},
 				},
@@ -312,13 +313,14 @@ func resourceScalewayRdbInstanceCreate(ctx context.Context, d *schema.ResourceDa
 
 	// Init Endpoints
 	if pn, pnExist := d.GetOk("private_network"); pnExist {
-		enableIpam := true
-		if _, ipNetSet := d.GetOk("private_network.0.ip_net"); ipNetSet {
-			enableIpam = false
+		ipamConfig, staticConfig := getIPConfigCreate(d, "ip_net")
+		var diags diag.Diagnostics
+		createReq.InitEndpoints, diags = expandPrivateNetwork(pn, pnExist, ipamConfig, staticConfig)
+		if diags.HasError() {
+			return diags
 		}
-		createReq.InitEndpoints, err = expandPrivateNetwork(pn, pnExist, enableIpam)
-		if err != nil {
-			return diag.FromErr(err)
+		for _, warning := range diags {
+			tflog.Warn(ctx, warning.Detail)
 		}
 	}
 	if _, lbExists := d.GetOk("load_balancer"); lbExists {
@@ -467,7 +469,7 @@ func resourceScalewayRdbInstanceRead(ctx context.Context, d *schema.ResourceData
 	_ = d.Set("init_settings", flattenInstanceSettings(res.InitSettings))
 
 	// set endpoints
-	enableIpam, err := isIpamEndpoint(res, meta)
+	enableIpam, err := getIPAMConfigRead(res, meta)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -710,19 +712,13 @@ func resourceScalewayRdbInstanceUpdate(ctx context.Context, d *schema.ResourceDa
 		// set new endpoint
 		pn, pnExist := d.GetOk("private_network")
 		if pnExist {
-			// "enable_ipam" is not readable from the API, so we just read the user's config
-			enableIpam := true
-			if rawConfig := d.GetRawConfig(); !rawConfig.IsNull() {
-				pnRawConfig := rawConfig.AsValueMap()["private_network"].AsValueSlice()[0].AsValueMap()
-				if !pnRawConfig["enable_ipam"].IsNull() && pnRawConfig["enable_ipam"].False() ||
-					!pnRawConfig["ip_net"].IsNull() {
-					enableIpam = false
-				}
+			ipamConfig, staticConfig := getIPConfigUpdate(d, "ip_net")
+			privateEndpoints, diags := expandPrivateNetwork(pn, pnExist, ipamConfig, staticConfig)
+			if diags.HasError() {
+				return diags
 			}
-
-			privateEndpoints, err := expandPrivateNetwork(pn, pnExist, enableIpam)
-			if err != nil {
-				return diag.FromErr(err)
+			for _, warning := range diags {
+				tflog.Warn(ctx, warning.Detail)
 			}
 			for _, e := range privateEndpoints {
 				_, err := rdbAPI.CreateEndpoint(