Merge pull request #1101 from ovh/dev/aamstutz/fix-ip-move

fix: Correctly handle blocks in resource ovh_ip_move

Author: amstuta

.cds/terraform-provider-ovh.yml

@@ -736,14 +736,14 @@ workflow:
     pipeline: terraform-provider-ovh-testacc
     when:
     - success
-  Tests_TestAccIpMove_basic:
+  Tests_IpMove:
     application: terraform-provider-ovh
     depends_on:
     - terraform-provider-ovh-pre-sweepers
     environment: acctests
     one_at_a_time: true
     parameters:
-      testargs: -run TestAccIpMove_basic
+      testargs: -run TestAccIpMove
       skipthispipeline: "{{.workflow.terraform-provider-ovh.pip.skipthistest.ip}}"
     pipeline: terraform-provider-ovh-testacc
     when:
@@ -923,7 +923,7 @@ workflow:
     - Tests_TestAccResourceCloudProjectFailoverIpAttachTestAccResourceCloudProject_basic
     - Tests_TestAccCloudProjectWorkflowBackup
     - Tests_TestAccResourceIpService_basic
-    - Tests_TestAccIpMove_basic
+    - Tests_IpMove
     - Tests_IPFirewall
     - Tests_IPMitigation
     - Tests_Okms

ovh/helpers/helpers.go

@@ -496,16 +496,24 @@ func ValidateHostingPrivateDatabaseUserGrant(value string) error {
 	})
 }
 
-func ServiceNameFromIpBlock(ip string) (*string, error) {
-	parsedIp := net.ParseIP(ip)
-	if parsedIp == nil {
-		var err error
-		parsedIp, _, err = net.ParseCIDR(ip)
-		if err != nil {
-			return nil, fmt.Errorf("ip %s is not valid IP nor a valid CIDR", ip)
+func ServiceNameFromIpBlock(ip string) (string, error) {
+	if parsedIP, ipNet, err := net.ParseCIDR(ip); err == nil {
+		prefixLength, _ := ipNet.Mask.Size()
+
+		if prefixLength == 32 {
+			// Single IP, omit the mask
+			return fmt.Sprintf("ip-%s", parsedIP.String()), nil
 		}
+
+		// IP block, keep the mask
+		return fmt.Sprintf("ip-%s/%d", parsedIP.String(), prefixLength), nil
+	}
+
+	// If not a CIDR, try parsing as a single IP
+	parsedIP := net.ParseIP(ip)
+	if parsedIP == nil {
+		return "", fmt.Errorf("ip %s is not a valid IP nor a valid CIDR", ip)
 	}
-	serviceName := fmt.Sprintf("ip-%s", parsedIp)
 
-	return &serviceName, nil
+	return fmt.Sprintf("ip-%s", parsedIP.String()), nil
 }

ovh/resource_ip_move.go

@@ -115,7 +115,13 @@ func resourceIpMoveSchema() map[string]*schema.Schema {
 func resoursIpMoveCreate(d *schema.ResourceData, meta interface{}) error {
 	// later on this ID will be replaced by the task if when we need to create it (see resourceIpMoveUpdate)
 	d.SetId(d.Get("ip").(string))
-	return resourceIpMoveUpdate(d, meta)
+
+	err := resourceIpMoveUpdate(d, meta)
+	if err != nil {
+		d.SetId("")
+	}
+
+	return err
 }
 
 // resourceIpMoveUpdate will move an ip to a provided service name or detach (= park) it otherwise
@@ -160,19 +166,19 @@ func resourceIpMoveUpdate(d *schema.ResourceData, meta interface{}) error {
 	} else {
 		log.Printf("[WARNING] - resource ID %s is not an int64/not a task ID. Cannot get last task state", d.Id())
 	}
-	err = resourceIpServiceReadByServiceName(d, *serviceName, config)
+	err = resourceIpServiceReadByServiceName(d, serviceName, config)
 	if err != nil {
 		return err
 	}
 
 	currentlyRoutedService := GetRoutedToServiceName(d)
 	// no need to update if ip is already routed to the appropriate service
 	if reflect.DeepEqual(currentlyRoutedService, opts.To) {
-		log.Printf("[DEBUG] Won't do anything as ip %s (service name = %s) is already routed to service %v", ip, *serviceName, currentlyRoutedService)
+		log.Printf("[DEBUG] Won't do anything as ip %s (service name = %s) is already routed to service %v", ip, serviceName, currentlyRoutedService)
 		return nil
 	} else {
 		if opts.To == nil {
-			log.Printf("[DEBUG] Will move ip %s (service name = %s) from service %s to IP parking", ip, *serviceName, *currentlyRoutedService)
+			log.Printf("[DEBUG] Will move ip %s (service name = %s) from service %s to IP parking", ip, serviceName, *currentlyRoutedService)
 			endpoint := fmt.Sprintf("/ip/%s/park",
 				url.PathEscape(ip),
 			)
@@ -181,7 +187,7 @@ func resourceIpMoveUpdate(d *schema.ResourceData, meta interface{}) error {
 				return fmt.Errorf("calling Post %s: %q", endpoint, err)
 			}
 		} else {
-			log.Printf("[DEBUG] Will move ip %s (service name = %s) from service %v to service %s", ip, *serviceName, currentlyRoutedService, *opts.To)
+			log.Printf("[DEBUG] Will move ip %s (service name = %s) from service %v to service %s", ip, serviceName, currentlyRoutedService, *opts.To)
 			endpoint := fmt.Sprintf("/ip/%s/move",
 				url.PathEscape(ip),
 			)
@@ -269,7 +275,7 @@ func resourceIpRead(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 	config := meta.(*Config)
-	return resourceIpServiceReadByServiceName(d, *serviceName, config)
+	return resourceIpServiceReadByServiceName(d, serviceName, config)
 }
 
 // resourceIpMoveDelete is an empty implementation as move do not actually create API objects but rather updates the underlying ip spec (by modifying its routed_to service)

ovh/resource_ip_move_test.go

@@ -70,3 +70,41 @@ func TestAccIpMove_basic(t *testing.T) {
 		},
 	})
 }
+
+func TestAccIpMove_block(t *testing.T) {
+	ipBlock := os.Getenv("OVH_IP_BLOCK_MOVE_TEST")
+	routedToServiceName := os.Getenv("OVH_IP_MOVE_SERVICE_NAME_TEST")
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheckIpMove(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+					resource "ovh_ip_move" "move" {
+						ip = "%s"
+						routed_to {
+							service_name = "%s"
+						}
+					}`,
+					ipBlock, routedToServiceName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("ovh_ip_move.move", "routed_to.0.service_name", routedToServiceName),
+				),
+			},
+			{
+				Config: fmt.Sprintf(`
+					resource "ovh_ip_move" "move" {
+						ip = "%s"
+						routed_to {
+							service_name = ""
+						}
+					}`,
+					ipBlock),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("ovh_ip_move.move", "routed_to.0.service_name", ""),
+				),
+			},
+		},
+	})
+}