refactor(instance_server): use instance_block helpers (#2636)

* refactor(instance_server): use instance_block helpers

* extract root volume creation

* remove unused sanitizeVolumes function

* remove update specific code

* lint

Author: Codelax

internal/services/instance/helpers_instance.go

@@ -254,51 +254,6 @@ func validateLocalVolumeSizes(volumes map[string]*instance.VolumeServerTemplate,
 	return nil
 }
 
-// sanitizeVolumeMap removes extra data for API validation.
-//
-// On the api side, there are two possibles validation schemas for volumes and the validator will be chosen dynamically depending on the passed JSON request
-// - With an image (in that case the root volume can be skipped because it is taken from the image)
-// - Without an image (in that case, the root volume must be defined)
-func sanitizeVolumeMap(volumes map[string]*instance.VolumeServerTemplate) map[string]*instance.VolumeServerTemplate {
-	m := make(map[string]*instance.VolumeServerTemplate)
-
-	for index, v := range volumes {
-		// Remove extra data for API validation.
-		switch {
-		// If a volume already got an ID it is passed as it to the API without specifying the volume type.
-		// TODO: Fix once instance accept volume type in the schema validation
-		case v.ID != nil:
-			if strings.HasPrefix(string(v.VolumeType), "sbs") {
-				// If volume is from SBS api, the type must be passed
-				// This rules come from instance API and may not be documented
-				v = &instance.VolumeServerTemplate{
-					ID:         v.ID,
-					Boot:       v.Boot,
-					VolumeType: v.VolumeType,
-				}
-			} else {
-				v = &instance.VolumeServerTemplate{
-					ID:   v.ID,
-					Name: v.Name,
-					Boot: v.Boot,
-				}
-			}
-		// For the root volume (index 0) if the size is 0, it is considered as a volume created from an image.
-		// The size is not passed to the API, so it's computed by the API
-		case index == "0" && v.Size == nil:
-			v = &instance.VolumeServerTemplate{
-				VolumeType: v.VolumeType,
-				Boot:       v.Boot,
-			}
-		// If none of the above conditions are met, the volume is passed as it to the API
-		default:
-		}
-		m[index] = v
-	}
-
-	return m
-}
-
 func preparePrivateNIC(
 	ctx context.Context, data interface{},
 	server *instance.Server, vpcAPI *vpc.API,
@@ -535,34 +490,53 @@ func instanceIPHasMigrated(d *schema.ResourceData) bool {
 }
 
 func instanceServerAdditionalVolumeTemplate(api *BlockAndInstanceAPI, zone scw.Zone, volumeID string) (*instance.VolumeServerTemplate, error) {
-	vol, err := api.GetVolume(&instance.GetVolumeRequest{
-		Zone:     zone,
+	vol, err := api.GetUnknownVolume(&GetUnknownVolumeRequest{
 		VolumeID: locality.ExpandID(volumeID),
+		Zone:     zone,
 	})
-	if err == nil {
-		return &instance.VolumeServerTemplate{
-			ID:         &vol.Volume.ID,
-			Name:       &vol.Volume.Name,
-			VolumeType: vol.Volume.VolumeType,
-			Size:       &vol.Volume.Size,
-		}, nil
-	}
-	if !httperrors.Is404(err) {
+	if err != nil {
 		return nil, err
 	}
+	return vol.VolumeTemplate(), nil
+}
+
+func prepareRootVolume(rootVolumeI map[string]any, serverType *instance.ServerType, image string) *UnknownVolume {
+	serverTypeCanBootOnBlock := serverType.VolumesConstraint.MaxSize == 0
+
+	rootVolumeIsBootVolume := types.ExpandBoolPtr(types.GetMapValue[bool](rootVolumeI, "boot"))
+	rootVolumeType := types.GetMapValue[string](rootVolumeI, "volume_type")
+	sizeInput := types.GetMapValue[int](rootVolumeI, "size_in_gb")
+	rootVolumeID := zonal.ExpandID(types.GetMapValue[string](rootVolumeI, "volume_id")).ID
+
+	// If the rootVolumeType is not defined, define it depending on the offer
+	if rootVolumeType == "" {
+		if serverTypeCanBootOnBlock {
+			rootVolumeType = instance.VolumeVolumeTypeBSSD.String()
+		} else {
+			rootVolumeType = instance.VolumeVolumeTypeLSSD.String()
+		}
+	}
 
-	blockVol, err := api.blockAPI.GetVolume(&blockSDK.GetVolumeRequest{
-		Zone:     zone,
-		VolumeID: locality.ExpandID(volumeID),
-	})
-	if err == nil {
-		return &instance.VolumeServerTemplate{
-			ID:         &blockVol.ID,
-			Name:       &blockVol.Name,
-			VolumeType: "sbs_volume",
-			Size:       &blockVol.Size,
-		}, nil
+	rootVolumeName := ""
+	if image == "" { // When creating an instance from an image, volume should not have a name
+		rootVolumeName = types.NewRandomName("vol")
 	}
 
-	return nil, err
+	var rootVolumeSize *scw.Size
+	if sizeInput == 0 && rootVolumeType == instance.VolumeVolumeTypeLSSD.String() {
+		// Compute the rootVolumeSize so it will be valid against the local volume constraints
+		// It wouldn't be valid if another local volume is added, but in this case
+		// the user would be informed that it does not fulfill the local volume constraints
+		rootVolumeSize = scw.SizePtr(serverType.VolumesConstraint.MaxSize)
+	} else if sizeInput > 0 {
+		rootVolumeSize = scw.SizePtr(scw.Size(uint64(sizeInput) * gb))
+	}
+
+	return &UnknownVolume{
+		Name:               rootVolumeName,
+		ID:                 rootVolumeID,
+		InstanceVolumeType: instance.VolumeVolumeType(rootVolumeType),
+		Size:               rootVolumeSize,
+		Boot:               rootVolumeIsBootVolume,
+	}
 }

internal/services/instance/helpers_instance_block.go

@@ -25,24 +25,33 @@ type UnknownVolume struct {
 	Zone     scw.Zone
 	ID       string
 	Name     string
-	Size     scw.Size
+	Size     *scw.Size
 	ServerID *string
+	Boot     *bool
 
 	// IsBlockVolume is true if volume is managed by block API
 	IsBlockVolume bool
 
 	InstanceVolumeType instance.VolumeVolumeType
 }
 
-// VolumeTemplateUpdate return a VolumeServerTemplate for an UpdateServer request
-func (volume *UnknownVolume) VolumeTemplateUpdate() *instance.VolumeServerTemplate {
-	template := &instance.VolumeServerTemplate{
-		ID:   scw.StringPtr(volume.ID),
-		Name: &volume.Name, // name is ignored by the API, any name will work here
+// VolumeTemplate returns a template to be used for servers requests.
+func (volume *UnknownVolume) VolumeTemplate() *instance.VolumeServerTemplate {
+	template := &instance.VolumeServerTemplate{}
+	if volume.ID != "" {
+		template.ID = &volume.ID
+	} else {
+		template.Size = volume.Size
 	}
+
+	if volume.Boot != nil {
+		template.Boot = volume.Boot
+	}
+
 	if volume.IsBlockVolume {
-		template.Name = nil
 		template.VolumeType = volume.InstanceVolumeType
+	} else {
+		template.Name = &volume.Name
 	}
 
 	return template
@@ -73,7 +82,7 @@ func (api *BlockAndInstanceAPI) GetUnknownVolume(req *GetUnknownVolumeRequest, o
 			Zone:               getVolumeResponse.Volume.Zone,
 			ID:                 getVolumeResponse.Volume.ID,
 			Name:               getVolumeResponse.Volume.Name,
-			Size:               getVolumeResponse.Volume.Size,
+			Size:               &getVolumeResponse.Volume.Size,
 			IsBlockVolume:      false,
 			InstanceVolumeType: getVolumeResponse.Volume.VolumeType,
 		}
@@ -96,7 +105,7 @@ func (api *BlockAndInstanceAPI) GetUnknownVolume(req *GetUnknownVolumeRequest, o
 		Zone:               blockVolume.Zone,
 		ID:                 blockVolume.ID,
 		Name:               blockVolume.Name,
-		Size:               blockVolume.Size,
+		Size:               &blockVolume.Size,
 		IsBlockVolume:      true,
 		InstanceVolumeType: instance.VolumeVolumeTypeSbsVolume,
 	}

internal/services/instance/server.go

@@ -430,43 +430,9 @@ func ResourceInstanceServerCreate(ctx context.Context, d *schema.ResourceData, m
 	}
 
 	req.Volumes = make(map[string]*instanceSDK.VolumeServerTemplate)
-	serverTypeCanBootOnBlock := serverType.VolumesConstraint.MaxSize == 0
-	rootVolumeIsBootVolume := types.ExpandBoolPtr(d.Get("root_volume.0.boot"))
-	rootVolumeType := d.Get("root_volume.0.volume_type").(string)
-	sizeInput := d.Get("root_volume.0.size_in_gb").(int)
-	rootVolumeID := zonal.ExpandID(d.Get("root_volume.0.volume_id").(string)).ID
-
-	// If the rootVolumeType is not defined, define it depending on the offer
-	if rootVolumeType == "" {
-		if serverTypeCanBootOnBlock {
-			rootVolumeType = instanceSDK.VolumeVolumeTypeBSSD.String()
-		} else {
-			rootVolumeType = instanceSDK.VolumeVolumeTypeLSSD.String()
-		}
-	}
-
-	rootVolumeName := ""
-	if req.Image == "" { // When creating an instanceSDK from an image, volume should not have a name
-		rootVolumeName = types.NewRandomName("vol")
-	}
+	rootVolume := d.Get("root_volume.0").(map[string]any)
 
-	var rootVolumeSize *scw.Size
-	if sizeInput == 0 && rootVolumeType == instanceSDK.VolumeVolumeTypeLSSD.String() {
-		// Compute the rootVolumeSize so it will be valid against the local volume constraints
-		// It wouldn't be valid if another local volume is added, but in this case
-		// the user would be informed that it does not fulfill the local volume constraints
-		rootVolumeSize = scw.SizePtr(serverType.VolumesConstraint.MaxSize)
-	} else if sizeInput > 0 {
-		rootVolumeSize = scw.SizePtr(scw.Size(uint64(sizeInput) * gb))
-	}
-
-	req.Volumes["0"] = &instanceSDK.VolumeServerTemplate{
-		Name:       types.ExpandStringPtr(rootVolumeName),
-		ID:         types.ExpandStringPtr(rootVolumeID),
-		VolumeType: instanceSDK.VolumeVolumeType(rootVolumeType),
-		Size:       rootVolumeSize,
-		Boot:       rootVolumeIsBootVolume,
-	}
+	req.Volumes["0"] = prepareRootVolume(rootVolume, serverType, req.Image).VolumeTemplate()
 	if raw, ok := d.GetOk("additional_volume_ids"); ok {
 		for i, volumeID := range raw.([]interface{}) {
 			// We have to get the volume to know whether it is a local or a block volume
@@ -483,9 +449,6 @@ func ResourceInstanceServerCreate(ctx context.Context, d *schema.ResourceData, m
 		return diag.FromErr(err)
 	}
 
-	// Sanitize the volume map to respect API schemas
-	req.Volumes = sanitizeVolumeMap(req.Volumes)
-
 	res, err := api.CreateServer(req, scw.WithContext(ctx))
 	if err != nil {
 		return diag.FromErr(err)
@@ -837,7 +800,7 @@ func ResourceInstanceServerUpdate(ctx context.Context, d *schema.ResourceData, m
 			if volumeHasChange && !isStopped && volume.IsLocal() && volume.IsAttached() {
 				return diag.FromErr(errors.New("instanceSDK must be stopped to change local volumes"))
 			}
-			volumes[strconv.Itoa(i+1)] = volume.VolumeTemplateUpdate()
+			volumes[strconv.Itoa(i+1)] = volume.VolumeTemplate()
 		}
 
 		serverShouldUpdate = true

internal/types/map.go

@@ -58,3 +58,14 @@ func ExpandMapStringString(data any) map[string]string {
 	}
 	return m
 }
+
+// GetMapValue returns the value for a key from a map.
+// returns zero value if key does not exist in map.
+func GetMapValue[T any](m map[string]any, key string) T { //nolint: ireturn
+	var val T
+	valI, exists := m[key]
+	if exists {
+		val = valI.(T)
+	}
+	return val
+}