Basic auth (username/password) has been supported since the plugin's
origin but TrueNAS API keys have been available since TrueNAS 12 (2020).
Sending credentials on every API call is a security risk and TrueNAS
may stop accepting it in future releases.
Log a syslog(warn) on every connection attempt that uses Basic Auth so
operators see the deprecation message in their Proxmox logs. The warning
links to issue #244 which tracks removal in v3.0.0.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat: auto-detect and correct ZFS blocksize for TrueNAS SCALE/CORE (#241)
SCALE requires >= 16k volblocksize; CORE works with 8k. Without correction,
creating a disk on SCALE with blocksize=8k triggers a ZFS warning and wastes
space on SCALE's minimum-4k-block pool layout.
Three changes:
- freenas_get_recommended_blocksize: fixes freenas_api_connect → freenas_api_check
so product_name is actually populated before the SCALE check (was always returning
8192 before this fix)
- freenas_parse_blocksize: new helper to compare "8k"/"16k"/integer blocksize strings
- alloc_image (both 8.x and 8.4.x patches): always detect recommended blocksize for
freenas provider; if configured < recommended, override for this call AND persist
the correction back to storage.cfg via lock_storage_config
- on_add_hook (8.4.x patch only): detect at storage creation time and correct $scfg
before write_config saves it — no extra write needed
Tested on pve01-hq (PVE 8.4.19) against Tank02 (SCALE 24.10.2.1):
- Disk created with blocksize=8k → task log shows correction message, storage.cfg
updated to 16384, no volblocksize warning from TrueNAS
- Disk created with blocksize=16384 → no-op, clean TASK OK
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* ci: add validate-patches step for ZFSPlugin PVE 8.4.x patch
ZFSPlugin-8.4.14_1.pm.patch has been in the build since v2.3.0 but was
never dry-run validated in CI. Now that we have the 8.4 orig committed
(ZFSPlugin-8.4.14_1.pm.orig from libpve-storage-perl 8.3.8), wire it up.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: replace explicit return undef with bare return (perlcritic)
Subroutines::ProhibitExplicitReturnUndef violation in
freenas_get_recommended_blocksize. Bare return in list context returns
an empty list rather than a list containing undef, which is the
correct Perl idiom.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
ZFSPlugin's free_image deletes the zvol via SSH after calling our
run_delete_lu. Our earlier addition of freenas_delete_zvol in
run_delete_lu caused a double-delete: SSH would fail, free_image's
error recovery would call run_create_lu on the now-missing zvol,
producing a spurious "Unable to create lun (rolled back)" in the task
log even though the migration had already succeeded.
freenas_delete_zvol remains in run_create_lu's rollback path where it
correctly cleans up zvols orphaned by a failed LUN creation (#239).
Also documents ZFS blocksize requirements for TrueNAS SCALE (16k) vs
CORE (8k) in README configuration and troubleshooting sections (#241).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
remove:true in the iSCSI extent DELETE body is unreliable on TrueNAS
SCALE 24.10+ — the extent config is removed but the underlying zvol is
left behind as an orphaned dataset. Add freenas_delete_zvol() which calls
DELETE /api/v2.0/pool/dataset/id/{path}/ explicitly after every extent
removal, covering both the rollback path in run_create_lu and normal
disk deletion in run_delete_lu. Guarded to v2.0 API only using the
per-host cache to avoid stale package-global state when two storage
hosts with different API versions are active simultaneously.
Verified on TrueNAS SCALE 24.10.2.1: zvol is fully removed from
Datasets after a forced rollback test (FREENAS_TEST_ROLLBACK=1).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the ad-hoc if/rollback in run_create_lu with an eval block that
tracks both extent_id and link_id so either resource is cleaned up if
anything fails at any point during creation (#214 only covered the
link-creation step).
Also adds a FREENAS_TEST_ROLLBACK env-var trigger: set it in pvedaemon's
environment to force a rollback after a successful create, allowing
verification that orphaned extents and links are removed without needing
a real failure condition.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two bugs caused clone/migrate to fail with 'setHost on undefined value'
followed by 'Loop recursion prevention':
1. freenas_api_check reads $freenas_server_list->{$apihost}{client} to
check if initialized. Perl autovivifies {$apihost} as an empty {} on
that read. freenas_api_connect then saw a defined (but empty) hash and
skipped initialization, leaving client undef → setHost crash.
Fix: guard also checks !defined ...{client}.
2. $ping was a local variable reset to v1.0 on each recursive call.
TrueNAS 25.04+ removed the v1.0 endpoint (returns 404), so the
v1.0→v2.0 upgrade branch looped until the runaway counter tripped.
Fix: accept $ping as optional second parameter (defaults to v1.0)
and pass it through on both recursive call sites.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
RequireUseStrict and RequireUseWarnings require that strict and warnings
are the first statements after the package declaration. Move our \$VERSION
to after the two use pragmas.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
our \$VERSION = '2.3.0' is now the single source of truth for the release
series. The build.yml extracts it with a perl one-liner (no module load,
no stubs needed) and uses it as BASE_VERSION for alpha/beta channel builds.
Git tags remain the release trigger: pushing an exact vX.Y.Z tag publishes
to the stable Cloudsmith channel. A mismatch warning fires if the tag
version and \$VERSION disagree, catching forgotten version bumps.
The v2.3.0-pre anchor tag approach is superseded by this; the anchor tag
can stay in history as a breadcrumb but \$VERSION is now what drives builds.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two problems in run_create_lu:
1. If freenas_iscsi_create_extent returned undef, the next line accessed
$extent->{'id'} unconditionally, causing a crash rather than a clean error.
2. If freenas_iscsi_create_target_to_extent failed after a successful extent
creation, the code just died with "Unable to create lun", leaving the
newly created extent orphaned on TrueNAS with no target association.
Subsequent delete_lu calls could not find or clean up this extent because
it was never in the LUN list, requiring manual TrueNAS cleanup.
Fix: check extent creation result before dereferencing; on target-to-extent
failure, call freenas_iscsi_remove_extent to roll back the extent before
dying. The rollback is best-effort (failure is logged but not re-thrown)
so a secondary API error does not mask the primary failure message.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All of these were module-level scalars that the last freenas_api_check call
would overwrite. With two TrueNAS hosts in a Proxmox config (e.g., one CORE
and one SCALE), the second host's API check would clobber the first host's
product_name, dev_prefix, and api_version, causing wrong API paths and wrong
pool-name slash-vs-dash conversion on subsequent calls to the first host.
Changes:
- $freenas_server_list->{$host} now stores a hashref {client, product_name,
api_version, dev_prefix, runaway_count} instead of a bare REST::Client
- freenas_api_connect: recursion counter is now per-host (runaway_count);
$apiping made local so v1->v2 detection for one host does not affect others
- freenas_api_check: fixed broken init-guard (was testing $freenas_rest_connection->{$host}
which always evaluates undef on a REST::Client object); saves detected
product_name/api_version/dev_prefix back into the per-host hash after detection
- freenas_api_call: restores all module-level pointers (product_name, dev_prefix,
api_methods, api_variables) from per-host storage on every call, so the
correct host context is active regardless of which host was accessed last
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
'! $method =~ /pattern/' is parsed as '(! $method) =~ /pattern/' due to
precedence, so the regex ran against "" and always returned false. The
guard never fired and any method string passed through to the API call.
Changed to '$method !~ /^(?:GET|DELETE|POST)$/' which correctly tests the
method string. Also replaced atomic group (?>...) with non-capturing (?:...)
since there is no backtracking concern here.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TrueNAS 25.04 removed the v1.0 REST API entirely. Previously it redirected
(302) to v2.0; now it returns 404. The plugin treated 404 as a fatal
connection failure, preventing any operation on 25.04 hosts.
Fix: extend the v1.0→v2.0 upgrade condition in freenas_api_connect to also
trigger on HTTP 404, but only while probing the v1.0 endpoint (guarded by
`$apiping =~ /v1\.0/`). A 404 on the v2.0 endpoint still fails cleanly.
Also add a syslog warn when Bearer Token auth is attempted over plain HTTP:
TrueNAS 25.04+ revokes API keys sent without SSL, causing auth failures that
are otherwise invisible in the plugin log. The warning points admins to the
"Use SSL" storage config option.
Closes#205
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
freenas_iscsi_create_extent and freenas_iscsi_create_target_to_extent
both used `eval $value` to expand template variable names like '$name'
into their runtime values. Replace with a local %vars hash and an
`exists` lookup — same behavior, no stringy eval, no perlcritic warning.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ProhibitExplicitReturnUndef (severity 5): returning undef explicitly is
wrong because it forces scalar context on callers. 'return;' returns
undef in scalar context and an empty list in list context, which is the
correct idiom. Fixed all 7 occurrences in FreeNAS.pm.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- When SSL redirect is enabled on TrueNAS and a change from V1 to V2
api's needs to happen then we overrun the limit of 1 on the recursion
loop. Set it to 2.
- Added some logging for TrueNAS-Scale conversion from slash (/) to dash
(-).
- When changing from slashes (/) to dashes (-), I did not provide the
global parameter so a Storage/Tank/Disks would turn out to be
Storage-Tank/Disks causing a iSCSI: Failed to connect to LUN :
iscsi_service failed with : iscsi_service_reconnect_if_loggedin. Can not
reconnect right now.
- Created conditional to parse new TrueNAS 13 version.
- Created conditional to check for a 200 and that the Content-Type is
text/html so it will change to v2.0 of the API's. This is new behavior
in TrueNAS-Core 13.
- Add the true path to the debug statement when comparing targets.
- Add patch set for ZFSPlugin.pm for Storage lib 7.1-1
- Add patch set for pvemanagerlib.js for pve-manager version 7.1-11
- Add patch set for apidoc.js for pve-docs version 7.1-2.
- Create stable-7 directory for future installs based on Proxmox major
version each modules version.
- The eval{} block was originally returning a value that would allow
execution in the $@ conditional. This changed between Proxmox 6 and 7.
Why, no idea at the moment. Also, the original HASH was no longer valid
as a variable. I don't like the code block at this moment, I need to do
some regression testing to Proxmox 5 and FreeNAS/TrueNAS 11 and 12 to
see if I can clean up the conditional block. It was there for a reason I
just don't know at this moment.
- Added a '}' at line 515.
- Fixes issues reported in #98 abd #75.
- freenas-proxmox-packer also fixed. The stable build directory somehow
was moved to the development building directory.
- Adjusted Regex to parse the current TrueNAS-SCALE product and
versioning. Also changed the FreeNAS and TrueNAS so releases will be a
variable (e.g. - U, BETA, RC, MASTER)
- Used the $product_name to change use either a slash (/) for
FreeNAS/TrueNAS-CORE and a dash (-) for FreeNAS-SCALE.
- Under a certain condition, the variable $freenas_rest_connection would
not be defined resulting in a perl ‘blessed’ error. Added logic to see
if a variable is being passed in the @_ and use it, else use
$freenas_rest_connection.
- Use caller (1)[3] instead of passing the function name through the @_.
- Just addign a 'last' statement once an extent is found. No since
looking for more after the extent is found because there is no more.
Should make plugin run a bit faster.
- When cleaning up code from last commit, I removed a line from the
run_lun_command that initialize the code thinking that the way it would
work would be throught the freenas_api_check when calling the
freenas_api_call. Well certain procedures from the Proxmox ZFSPlugin
call procedures that do not have the $freenas_api_methods variable set
yet because freenas_api_check has NOT been called yet. Inserted the
freenas_api_check() in the run_lun_command() to get the variable
initialized. Fixes#62 and other tests that I found to have this issue.
freenas-proxmox-2.0.0-0beta4 will also have the fix.
- When optimizing the code to remove the redundant calls to create a
'new' REST::Client this had a single variable to hold the instance. If
you were moving between two different servers then the variables would
not match the destination parameters of the [Free|True]NAS server.
Implemented a hashref of the construct of REST:Client with the hashref
being the IP address of the portal or API IP. Fixes#61.
- Remove some white spacing and superfluous code.
- Added some comments so I know what I was changing.
other changes.
- Add logic to freenas_api_connect to handle a '302 Found' which seems
to mean it does not handle v1.0 API's. Fixes#60.
- Add logic to freenas_api_connect to handle a '307 Temporary Redirect'
which is when the redirect http to https box is checked. Fixes#59.
- Redesign the RegEx that validates and pulls the version information
from the API system/version call. This is due to a format like
TrueNAS-12.0-BETA instead of [FreeNAS|TrueNAS]-12.0-U1.1. Also make sure
empty RegEx return variables are zero (0) so the version number that is
formulated is correct (e.g. 12000000, 11030302). Fixes#60.
- During testing needed to add a check due to the
api/v2.0/system/version returning a quoted string where
api/v1.0/system/version returned a JSON-format data.
- Tested against baremetal FreeNAS 11.3-U3.2 and virtual TrueNAS
12.0-BETA machines.
- Cleaned up the API calling routine so it will only create one instance
of the REST::Client module. Typically was 10 to 30+ instances per VM
disk CRUD.
- Due to this some variables needed to be “global” to the plugin.
- Removed some superfluous hashes and variables.
- Repetitive code reduced to only on call.
- Created one “Frankenstein” hash that retains all API methods,
variables and json body structures for each API version used by the
plugin.
- One call to the global configuration in the version check to reduce
execution time.
- Cleanup some code and formatting.
- Added some comments.
- Allows the use of FreeNAS API v2.0 so we can finally remove a VM drive
from the Proxmox VE GUI.
- Created a Frankenstein of out of FreeNAS.pm to allow 'one module to
rule them all' for v1.0 and v2.0 API's to FreeNAS.
- librest-client-perl changes so the call can have a <body/> to it on
GET and DELETE methods.
- Some spelling mistakes.
- Better syslog entries.
- More debbugging for testing purposes to the syslog.
- Update pvemanagerlib.js patch for Proxmox version to 6.0-7
- Update ZFSPlugin.pm patch for Proxmox version 6.0-8
- Adjusted new directory structure and added some missing files.
button.
- Update README.md to include a requested donation button for the
project. Thank you.
- Updated patch for pvemanagerlib.js for Proxmox VE pve-manager: 6.0-5
- Updated patch for apidoc.js for Proxmox VE pve-docs: 6.0-4
- Updated patch for ZFSPlugin.pm for Proxmox VE libpve-storage-perl:
6.0-6
- Patch created for pve-manager 5.4-11 specifically for pvemanagerlib.js
- Patch created for pve-manager 5.4-13 specifically for pvemanagerlib.js
- Patch created for libpve-storage-perl 5.0-44 specifically for
ZFSPlugin.pm
- Updated apidoc.js.patch to reflect pve-docs: 5.4-2
- New apidoc-5.4-2_1.js.patch
- Updated pvemanagerlib.js.patch to reflect pve-manager: 5.4-5
- New pvemanagerlib-5.4-5_1.js.patch
- Updated ZFSPlugin.pm.patch to reflect libpve-storage-perl: 5.0-42
- New ZFSPlugin-5.0-42.pm.patch
- Revised the README.md
Kevin Adams
Kevin Scott Adams
Hans
sontru
Hans