From e5bd0a1b50672095469c597261a4511a2a8051a9 Mon Sep 17 00:00:00 2001 From: perf3ct Date: Wed, 6 Aug 2025 20:32:45 +0000 Subject: [PATCH 1/3] fix(ssh): fix the error message parsing that the `Target with this name already exists` looks for, and provide better output to user --- src/driver/freenas/ssh.js | 76 ++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 8 deletions(-) diff --git a/src/driver/freenas/ssh.js b/src/driver/freenas/ssh.js index 7163d7f..18c9e30 100644 --- a/src/driver/freenas/ssh.js +++ b/src/driver/freenas/ssh.js @@ -176,6 +176,48 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver { } } + /** + * Check if an error response indicates a target already exists. + * This method handles variations in TrueNAS API error messages across different API versions. + * + * @param {string|Object} responseBody - The HTTP response body (string or object) + * @returns {boolean} - true if the error indicates target already exists + */ + isTargetAlreadyExistsError(responseBody) { + // Extract error message more efficiently + let errorString = ''; + + if (typeof responseBody === 'string') { + errorString = responseBody; + } else if (responseBody && typeof responseBody === 'object') { + // Try common error message fields first to avoid full JSON.stringify + errorString = responseBody.message || + responseBody.error || + responseBody.detail || + JSON.stringify(responseBody); + } else { + return false; + } + + // Handle multiple variations of the target already exists error message + const targetExistsPatterns = [ + "Target name already exists", // Original pattern in code (API v1) + "Target with this name already exists", // Actual TrueNAS error message (API v2) + "Target\\b.*\\balready\\b.*\\bexists" // Flexible pattern with word boundaries + ]; + + return targetExistsPatterns.some(pattern => { + if (pattern.includes("\\")) { + // Use regex for flexible patterns with word boundaries + const regex = new RegExp(pattern, "i"); + return regex.test(errorString); + } else { + // Use case-insensitive simple string matching for exact patterns + return errorString.toLowerCase().includes(pattern.toLowerCase()); + } + }); + } + async findResourceByProperties(endpoint, match) { if (!match) { return; @@ -924,21 +966,30 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver { target ); - // 409 if invalid + // 409 Conflict - target already exists or other validation errors if (response.statusCode != 201) { target = null; if ( response.statusCode == 409 && - JSON.stringify(response.body).includes( - "Target name already exists" - ) + this.isTargetAlreadyExistsError(response.body) ) { + this.ctx.logger.debug( + "iSCSI target already exists, attempting to find existing target with name: %s", + iscsiName + ); target = await this.findResourceByProperties( "/services/iscsi/target", { iscsi_target_name: iscsiName, } ); + if (target) { + this.ctx.logger.debug( + "Found existing iSCSI target with ID: %s, name: %s", + target.id, + target.iscsi_target_name + ); + } } else { throw new GrpcError( grpc.status.UNKNOWN, @@ -1212,21 +1263,30 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver { response = await httpClient.post("/iscsi/target", target); - // 409 if invalid + // 422 Unprocessable Entity - validation errors including duplicate targets if (response.statusCode != 200) { target = null; if ( response.statusCode == 422 && - JSON.stringify(response.body).includes( - "Target name already exists" - ) + this.isTargetAlreadyExistsError(response.body) ) { + this.ctx.logger.debug( + "iSCSI target already exists, attempting to find existing target with name: %s", + iscsiName + ); target = await this.findResourceByProperties( "/iscsi/target", { name: iscsiName, } ); + if (target) { + this.ctx.logger.debug( + "Found existing iSCSI target with ID: %s, name: %s", + target.id, + target.name + ); + } } else { throw new GrpcError( grpc.status.UNKNOWN, From 8ce5243c7bb65199c170a30f3a166907a6f1d1c0 Mon Sep 17 00:00:00 2001 From: perf3ct Date: Wed, 6 Aug 2025 20:39:20 +0000 Subject: [PATCH 2/3] fix(api): fix the error message parsing that the `Target with this name already exists` looks for, and provide better output to user --- src/driver/freenas/api.js | 54 +++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/src/driver/freenas/api.js b/src/driver/freenas/api.js index b47286b..0ffa1bd 100644 --- a/src/driver/freenas/api.js +++ b/src/driver/freenas/api.js @@ -150,6 +150,48 @@ class FreeNASApiDriver extends CsiBaseDriver { } } + /** + * Check if an error response indicates a target already exists. + * This method handles variations in TrueNAS API error messages across different API versions. + * + * @param {string|Object} responseBody - The HTTP response body (string or object) + * @returns {boolean} - true if the error indicates target already exists + */ + isTargetAlreadyExistsError(responseBody) { + // Extract error message more efficiently + let errorString = ''; + + if (typeof responseBody === 'string') { + errorString = responseBody; + } else if (responseBody && typeof responseBody === 'object') { + // Try common error message fields first to avoid full JSON.stringify + errorString = responseBody.message || + responseBody.error || + responseBody.detail || + JSON.stringify(responseBody); + } else { + return false; + } + + // Handle multiple variations of the target already exists error message + const targetExistsPatterns = [ + "Target name already exists", // Original pattern in code (API v1) + "Target with this name already exists", // Actual TrueNAS error message (API v2) + "Target\\b.*\\balready\\b.*\\bexists" // Flexible pattern with word boundaries + ]; + + return targetExistsPatterns.some(pattern => { + if (pattern.includes("\\")) { + // Use regex for flexible patterns with word boundaries + const regex = new RegExp(pattern, "i"); + return regex.test(errorString); + } else { + // Use case-insensitive simple string matching for exact patterns + return errorString.toLowerCase().includes(pattern.toLowerCase()); + } + }); + } + /** * only here for the helpers * @returns @@ -835,14 +877,12 @@ class FreeNASApiDriver extends CsiBaseDriver { target ); - // 409 if invalid + // 409 Conflict - target already exists or other validation errors if (response.statusCode != 201) { target = null; if ( response.statusCode == 409 && - JSON.stringify(response.body).includes( - "Target name already exists" - ) + this.isTargetAlreadyExistsError(response.body) ) { target = await httpApiClient.findResourceByProperties( "/services/iscsi/target", @@ -1123,14 +1163,12 @@ class FreeNASApiDriver extends CsiBaseDriver { response = await httpClient.post("/iscsi/target", target); - // 409 if invalid + // 422 Unprocessable Entity - validation errors including duplicate targets if (response.statusCode != 200) { target = null; if ( response.statusCode == 422 && - JSON.stringify(response.body).includes( - "Target name already exists" - ) + this.isTargetAlreadyExistsError(response.body) ) { target = await httpApiClient.findResourceByProperties( "/iscsi/target", From 10aa9c539a854c3351f158f7dac51c7b58bb75df Mon Sep 17 00:00:00 2001 From: perf3ct Date: Wed, 6 Aug 2025 22:11:36 +0000 Subject: [PATCH 3/3] feat(ssh/api): resolve circular structure issue --- src/driver/freenas/http/api.js | 3 ++- src/driver/freenas/ssh.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/driver/freenas/http/api.js b/src/driver/freenas/http/api.js index 79d7311..596b973 100644 --- a/src/driver/freenas/http/api.js +++ b/src/driver/freenas/http/api.js @@ -66,7 +66,8 @@ class Api { // crude stoppage attempt let response = await httpClient.get(endpoint, queryParams); if (lastReponse) { - if (JSON.stringify(lastReponse) == JSON.stringify(response)) { + // Compare only the response body to avoid circular reference issues + if (JSON.stringify(lastReponse.body) == JSON.stringify(response.body)) { break; } } diff --git a/src/driver/freenas/ssh.js b/src/driver/freenas/ssh.js index 18c9e30..3c6040e 100644 --- a/src/driver/freenas/ssh.js +++ b/src/driver/freenas/ssh.js @@ -252,7 +252,8 @@ class FreeNASSshDriver extends ControllerZfsBaseDriver { // crude stoppage attempt let response = await httpClient.get(endpoint, queryParams); if (lastReponse) { - if (JSON.stringify(lastReponse) == JSON.stringify(response)) { + // Compare only the response body to avoid circular reference issues + if (JSON.stringify(lastReponse.body) == JSON.stringify(response.body)) { break; } }