feat: support storage iso local paths #1406
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for using local file paths in the proxmox_storage_iso resource instead of requiring remote HTTP URLs. The enhancement allows users to upload ISO files directly from their local filesystem without needing to host them remotely first.
Key Changes
- Added conditional logic to detect absolute paths and open them directly
- Modified file handling to skip the temporary file creation and HTTP download steps for local paths
- Maintained backward compatibility with existing HTTP URL functionality
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if filepath.IsAbs(url) { | ||
| file, err = os.Open(url) | ||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
The logic assumes any absolute path is a local file, but HTTP/HTTPS URLs also contain absolute paths (e.g., 'https://example.com/file.iso'). This will cause HTTP URLs to be treated as local file paths. Consider checking if the URL starts with a scheme like 'http://' or 'https://' instead of using filepath.IsAbs().
| file, err = os.CreateTemp(os.TempDir(), fileName) | ||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
| err = _downloadFile(url, file) | ||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| } | ||
| file.Seek(0, 0) |
There was a problem hiding this comment.
The temporary file cleanup logic is missing when _downloadFile fails. If the download fails, the temporary file created by os.CreateTemp will not be cleaned up, potentially leaving orphaned files in the temp directory. Consider adding proper cleanup with defer os.Remove(file.Name()) after successful temp file creation.
1 participant
Allows a
proxmox_storage_isoresource to accept a local path to the iso. Remote HTTP ISOs are automatically fetched first and stored locally in a tmpdir before uploading. This basically skips the download step and uploads any local path as is. Right now this supports absolute paths, but open to having the format befile:///if we want the value to always be some kind of URI.