Remove validation and custom options

docs/storage-class-parameters.md

@@ -15,44 +15,60 @@ metadata:
   name: synology-iscsi
 parameters:
     fsType: ext4
-    # The following options affect the LUN representing the volume
+    # The following options affect the LUN representing the volume. These options are passed directly to the Synology API.
-    lunType: BLUN       # Btrfs thin provisioning
+    # The following options are known.
-    lunType: BLUN_THICK # Btrfs thick provisioning
-    lunType: THIN       # Ext4 thin provisioning
-    lunType: ADV        # Ext4 thin provisioning with legacy advanced feature set
-    lunType: FILE       # Ext4 thick provisioning
-    lunDescription: Some Description
-    hardwareAssistedZeroing: true
-    hardwareAssistedLocking: true
-    hardwareAssistedDataTransfer: true
-    spaceReclamation: true
-    allowSnapshots: true
-    enableFuaWrite: false
-    enableSyncCache: false
-    ioPolicy: Buffered  # or Direct
-    # The following options affect the iSCSI target
-    headerDigenst: false
-    dataDigest: false
-    maxSessions: 1 # Note that this option requires a compatible filesystem. Use 0 for unlimited sessions (default).
-    maxRecieveSegmentBytes: 262144
-    maxSendSegmentBytes: 262144
     lunTemplate: |
-      # This inline yaml object will be passed to the Synology API when creating the LUN. Use this for custom options.
+      type: BLUN       # Btrfs thin provisioning
+      type: BLUN_THICK # Btrfs thick provisioning
+      type: THIN       # Ext4 thin provisioning
+      type: ADV        # Ext4 thin provisioning with legacy advanced feature set
+      type: FILE       # Ext4 thick provisioning
+      description: Some Description
+      
+      # Only for thick provisioned volumes. Known values:
+      # 0: Buffered Writes
+      # 3: Direct Write
+      direct_io_pattern: 0
+      
+      # Device Attributes. See below for more info
       dev_attribs:
-      - dev_attrib: emulate_caw
+      - dev_attrib: emulate_tpws
         enable: 1
+      - ...
+
+    # The following options affect the iSCSI target. These options will be passed directly to the Synology API.
+    # The following options are known.
     targetTemplate: |
-      # This inline yaml object will be passed to the Synology API when creating the target. Use this for custom
+      has_header_checksum: false
-      # options.
+      has_data_checksum: false
+      
+      # Note that this option requires a compatible filesystem. Use 0 for unlimited sessions.
       max_sessions: 0
+      multi_sessions: true
+      max_recv_seg_bytes: 262144
+      max_send_seg_bytes: 262144
+
+      # Use this to disable authentication. To configure authentication see below
+      auth_type: 0
 ```
 
-About extended features:
+#### About LUN Types
-- For `BLUN_THICK` volumes only hardware assisted zeroing and locking can be configured.
+The availability of the different types of LUNs depends on the filesystem used on your Synology volume. For Btrfs volumes
-- For `THIN` volumes none of the extended features can be configured.
+you can use `BLUN` and `BLUN_THICK` volumes. For Ext4 volumes you can use `THIN`, `ADV` or `FILE` volumes. These
-- For `ADV` volumes only space reclamation can be configured.
+correspond to the options available in the UI.
-- For `FILE` volumes only hardware assisted locking can be configured.
+
-- `ioPolicy` is only available for thick provisioned volumes.
+#### About `dev_attribs`
+Most of the LUN options are configured via the `dev_attribs` list. This list can be specified both in the `lunTemplate`
+of the global configuration and in the `lunTemplate` of the `StorageClass`. If both lists are present they will be merged
+(with the `StorageClass` taking precedence). The following  `dev_attribs` are known to work:
+
+- `emulate_tpws`: Hardware-assisted zeroing
+- `emulate_caw`: Hardware-assisted locking
+- `emulate_3pc`: Hardware-assisted data transfer
+- `emulate_tpu`: Space Reclamation
+- `emulate_fua_write`: Enable the FUA iSCSI command (DSM 7+)
+- `emulate_sync_cache`: Enable the Sync Cache iSCSI command (DSM 7+)
+- `can_snapshot`: Enable snapshots for this volume. Only works for thin provisioned volumes.
 
 ### Configure Snapshot Classes
 `synology-iscsi` can also configure different parameters on snapshot classes:
@@ -63,15 +79,14 @@ kind: VolumeSnapshotClass
 metadata:
   name: synology-iscsi-snapshot
 parameters:
-  isLocked: true
+  # This inline yaml object will be passed to the Synology API when creating the snapshot.
-  # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot
-  # Note that AppConsistent snapshots require a working Synology Storage Console. Otherwise both values will have
-  # equivalent behavior.
-  consistency: AppConsistent # Or CrashConsistent
   lunSnapshotTemplate: |
-    # This inline yaml object will be passed to the Synology API when creating the snapshot. Use this for custom
-    # options.
     is_locked: true
+    
+    # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot
+    # Note that app consistent snapshots require a working Synology Storage Console. Otherwise both values will have
+    # equivalent behavior.
+    is_app_consistent: true
 ...
 ```
 
@@ -90,8 +105,9 @@ metadata:
   name: synology-iscsi-chap
 parameters:
   fsType: ext4
-  lunType: BLUN
+  lunTemplate: |
-  lunDescription: iSCSI volumes with CHAP Authentication
+    type: BLUN
+    description: iSCSI volumes with CHAP Authentication
 secrets:
   # Use this to configure a single set of credentials for all volumes of this StorageClass
   csi.storage.k8s.io/provisioner-secret-name: chap-secret
@@ -112,7 +128,7 @@ stringData:
   password: MySecretPassword
   # Mutual CHAP Credentials. If these are specified mutual CHAP will be enabled.
   mutualUser: server
-  password: MyOtherPassword
+  mutualPassword: MyOtherPassword
 ```
 
 Note that CHAP authentication will only be enabled if the secret contains a username and password. If e.g. a password is

examples/synology-iscsi.yaml

@@ -27,5 +27,65 @@ iscsi:
   # full iqn limit is 223 bytes, plan accordingly
   namePrefix: ""
   nameSuffix: ""
-  # LUN options and CHAP authentication can be configured using StorageClasses.
+
+  # documented below are several blocks
+  # pick the option appropriate for you based on what your backing fs is and desired features
+  # you do not need to alter dev_attribs under normal circumstances but they may be altered in advanced use-cases
+  # These options can also be configured per storage-class:
   # See https://github.com/democratic-csi/democratic-csi/blob/master/docs/storage-class-parameters.md
+  lunTemplate:
+    # btrfs thin provisioning
+    type: "BLUN"
+    # tpws = Hardware-assisted zeroing
+    # caw = Hardware-assisted locking
+    # 3pc = Hardware-assisted data transfer
+    # tpu = Space reclamation
+    # can_snapshot = Snapshot
+    #dev_attribs:
+    #- dev_attrib: emulate_tpws
+    #  enable: 1
+    #- dev_attrib: emulate_caw
+    #  enable: 1
+    #- dev_attrib: emulate_3pc
+    #  enable: 1
+    #- dev_attrib: emulate_tpu
+    #  enable: 0
+    #- dev_attrib: can_snapshot
+    #  enable: 1
+
+    # btfs thick provisioning
+    # only zeroing and locking supported
+    #type: "BLUN_THICK"
+    # tpws = Hardware-assisted zeroing
+    # caw = Hardware-assisted locking
+    #dev_attribs:
+    #- dev_attrib: emulate_tpws
+    #  enable: 1
+    #- dev_attrib: emulate_caw
+    #  enable: 1
+
+    # ext4 thinn provisioning UI sends everything with enabled=0
+    #type: "THIN"
+
+    # ext4 thin with advanced legacy features set
+    # can only alter tpu (all others are set as enabled=1)
+    #type: "ADV"
+    #dev_attribs:
+    #- dev_attrib: emulate_tpu
+    #  enable: 1
+
+    # ext4 thick
+    # can only alter caw
+    #type: "FILE"
+    #dev_attribs:
+    #- dev_attrib: emulate_caw
+    #  enable: 1
+
+  lunSnapshotTemplate:
+    is_locked: true
+    # https://kb.synology.com/en-me/DSM/tutorial/What_is_file_system_consistent_snapshot
+    is_app_consistent: true
+
+  targetTemplate:
+    auth_type: 0
+    max_sessions: 0

src/driver/controller-synology/http/index.js

@@ -11,6 +11,7 @@ const __REGISTRY_NS__ = "SynologyHttpClient";
 
 SYNO_ERRORS = {
   18990002: { status: grpc.status.RESOURCE_EXHAUSTED, message: "The synology volume is out of disk space." },
+  18990318: { status: grpc.status.INVALID_ARGUMENT, message: "The requested lun type is incompatible with the Synology filesystem." },
   18990538: { status: grpc.status.ALREADY_EXISTS, message: "A LUN with this name already exists." },
   18990541: { status: grpc.status.RESOURCE_EXHAUSTED, message: "The maximum number of LUNS has been reached." },
   18990542: { status: grpc.status.RESOURCE_EXHAUSTED, message: "The maximum number if iSCSI target has been reached." },

src/driver/controller-synology/index.js

@@ -143,22 +143,21 @@ class ControllerSynologyDriver extends CsiBaseDriver {
     }
   }
 
-  /**
+  getObjectFromDevAttribs(list = []) {
-   * Parses a boolean value (e.g. from the value of a parameter. This recognizes
+    if (!list) {
-   * strings containing boolean literals as well as the numbers 1 and 0.
+      return {}
-   *
-   * @param {String} value - The value to be parsed.
-   * @returns {boolean} The parsed boolean value.
-   */
-  parseBoolean(value) {
-    if (value === undefined) {
-      return undefined;
     }
-    const parsed = parseInt(value)
+    return list.reduce(
-    if (!isNaN(parsed)) {
+      (obj, item) => Object.assign(obj, {[item.dev_attrib]: item.enable}), {}
-      return Boolean(parsed)
+    )
-    }
+  }
-    return "true".localeCompare(value, undefined, {sensitivity: "accent"}) === 0
+
+  getDevAttribsFromObject(obj, keepNull = false) {
+    return Object.entries(obj).filter(
+      e => keepNull || (e[1] != null)
+    ).map(
+      e => ({dev_attrib: e[0], enable: e[1]})
+    );
   }
 
   buildIscsiName(name) {
@@ -366,6 +365,7 @@ class ControllerSynologyDriver extends CsiBaseDriver {
         break;
       case "iscsi":
         let iscsiName = driver.buildIscsiName(name);
+        let storageClassTemplate;
         let data;
         let target;
         let lun_mapping;
@@ -491,106 +491,47 @@ class ControllerSynologyDriver extends CsiBaseDriver {
           }
         } else {
           // create lun
-          data = Object.assign({}, driver.options.iscsi.lunTemplate, yaml.load(normalizedParameters.lunTemplate), {
+          try {
-            name: iscsiName,
+            storageClassTemplate = yaml.load(normalizedParameters.lunTemplate ?? "")
-            location: driver.getLocation(),
+            const devAttribs = driver.getDevAttribsFromObject(Object.assign(
-            size: capacity_bytes
+              {},
-          });
+              driver.getObjectFromDevAttribs(driver.options.iscsi.lunTemplate?.dev_attribs),
-          data.type = normalizedParameters.lunType ?? data.type;
+              driver.getObjectFromDevAttribs(storageClassTemplate?.dev_attribs)
-          if ('lunDescription' in normalizedParameters) {
+            ))
-            data.description = normalizedParameters.lunDescription;
+            data = Object.assign({}, driver.options.iscsi.lunTemplate, storageClassTemplate, {
-          }
+              name: iscsiName,
-          if (normalizedParameters.ioPolicy === "Direct") {
+              location: driver.getLocation(),
-            data.direct_io_pattern = 3;
+              size: capacity_bytes,
-          } else if (normalizedParameters.ioPolicy === "Buffered") {
+              dev_attribs: devAttribs
-            data.direct_io_pattern = 0;
+            });
-          } else if (normalizedParameters.ioPolicy !== undefined) {
-            throw new GrpcError(
-              grpc.status.INVALID_ARGUMENT,
-              `snapshot consistency must be either CrashConsistent or AppConsistent`
-            );
-          }
 
-          const dev_attribs = (data.dev_attribs ?? []).reduce(
+            lun_uuid = await httpClient.CreateLun(data);
-            (obj, item) => Object.assign(obj, {[item.dev_attrib]: driver.parseBoolean(item.enable)}), {}
+          } catch (err) {
-          );
+            if (err instanceof yaml.YAMLException) {
-          dev_attribs.emulate_tpws = driver.parseBoolean(normalizedParameters.hardwareAssistedZeroing) ?? dev_attribs.emulate_tpws;
+              throw new GrpcError(
-          dev_attribs.emulate_caw = driver.parseBoolean(normalizedParameters.hardwareAssistedLocking) ?? dev_attribs.emulate_caw;
+                grpc.status.INVALID_ARGUMENT,
-          dev_attribs.emulate_3pc = driver.parseBoolean(normalizedParameters.hardwareAssistedDataTransfer) ?? dev_attribs.emulate_3pc;
+                `The lunTemplate on StorageClass is not a valid YAML document.`
-          dev_attribs.emulate_tpu = driver.parseBoolean(normalizedParameters.spaceReclamation) ?? dev_attribs.emulate_tpu;
+              );
-          dev_attribs.emulate_fua_write = driver.parseBoolean(normalizedParameters.enableFuaWrite) ?? dev_attribs.emulate_fua_write;
+            } else {
-          dev_attribs.emulate_sync_cache = driver.parseBoolean(normalizedParameters.enableSyncCache) ?? dev_attribs.emulate_sync_cache;
+              throw err
-          dev_attribs.can_snapshot = driver.parseBoolean(normalizedParameters.allowSnapshots) ?? dev_attribs.can_snapshot;
+            }
-          data.dev_attribs = Object.entries(dev_attribs).filter(
-            e => e[1] !== undefined
-          ).map(
-            e => ({dev_attrib: e[0], enable: Number(e[1])})
-          );
-
-          if (["BLUN", "THIN", "ADV"].includes(data.type) && 'direct_io_pattern' in data) {
-            throw new GrpcError(
-              grpc.status.INVALID_ARGUMENT,
-              `ioPolicy can only be used with thick provisioning.`
-            );
           }
-          if (["BLUN_THICK", "FILE"].includes(data.type) && dev_attribs.emulate_tpu) {
-            throw new GrpcError(
-              grpc.status.INVALID_ARGUMENT,
-              `spaceReclamation can only be used with thin provisioning.`
-            );
-          }
-          if (["BLUN_THICK", "FILE"].includes(data.type) && dev_attribs.can_snapshot) {
-            throw new GrpcError(
-              grpc.status.INVALID_ARGUMENT,
-              `allowSnapshots can only be used with thin provisioning.`
-            );
-          }
-
-          lun_uuid = await httpClient.CreateLun(data);
         }
 
         // create target
         let iqn = driver.options.iscsi.baseiqn + iscsiName;
-        data = Object.assign({}, driver.options.iscsi.targetTemplate, yaml.load(normalizedParameters.targetTemplate), {
+        try {
+          storageClassTemplate = yaml.load(normalizedParameters.targetTemplate ?? "")
+        } catch (err) {
+          throw new GrpcError(
+            grpc.status.INVALID_ARGUMENT,
+            `The targetTemplate on StorageClass is not a valid YAML document.`
+          );
+        }
+        data = Object.assign({}, driver.options.iscsi.targetTemplate, storageClassTemplate, {
           name: iscsiName,
           iqn,
         });
-        if ('headerChecksum' in normalizedParameters) {
-          data.has_data_checksum = normalizedParameters.headerChecksum;
-        }
-        if ('dataChecksum' in normalizedParameters) {
-          data.has_data_checksum = normalizedParameters.dataChecksum;
-        }
-        if ('maxSessions' in normalizedParameters) {
-          data.max_sessions = Number(normalizedParameters.maxSessions);
-          if (isNaN(data.max_sessions)) {
-            throw new GrpcError(
-              grpc.status.INVALID_ARGUMENT,
-              `maxSessions must be a number.`
-            );
-          }
-        }
-        if (!('multi_sessions' in data) && 'max_sessions' in data) {
-          data.multi_sessions = data.max_sessions == 1;
-        }
-        if ('maxReceiveSegmentBytes' in normalizedParameters) {
-          data.max_recv_seg_bytes = Number(normalizedParameters.maxReceiveSegmentBytes);
-          if (isNaN(data.max_recv_seg_bytes)) {
-            throw new GrpcError(
-              grpc.status.INVALID_ARGUMENT,
-              `maxReceiveSegmentBytes must be a number.`
-            );
-          }
-        }
-        if ('maxSendSegmentBytes' in normalizedParameters) {
-          data.max_send_seg_bytes = Number(normalizedParameters.maxSendSegmentBytes);
-          if (isNaN(data.max_send_seg_bytes)) {
-            throw new GrpcError(
-              grpc.status.INVALID_ARGUMENT,
-              `maxSendSegmentBytes must be a number.`
-            );
-          }
-        }
 
         if ('user' in call.request.secrets && 'password' in call.request.secrets) {
           data.user = call.request.secrets.user;
@@ -605,9 +546,6 @@ class ControllerSynologyDriver extends CsiBaseDriver {
             data.auth_type = 1;
             data.mutual_chap = false;
           }
-        } else {
-          data.auth_type ??= 0;
-          data.chap ??= false;
         }
         let target_id = await httpClient.CreateTarget(data);
         //target = await httpClient.GetTargetByTargetID(target_id);
@@ -998,27 +936,23 @@ class ControllerSynologyDriver extends CsiBaseDriver {
 
     // check for already exists
     let snapshot;
+    let snapshotClassTemplate;
     snapshot = await httpClient.GetSnapshotByLunUUIDAndName(lun.uuid, name);
     if (!snapshot) {
       const normalizedParameters = driver.getNormalizedParameters(call.request.parameters);
-      let data = Object.assign({}, driver.options.iscsi.lunSnapshotTemplate, yaml.load(normalizedParameters.lunSnapshotTemplate), {
+      try {
+        snapshotClassTemplate = yaml.load(normalizedParameters.lunSnapshotTemplate ?? "");
+      } catch (err) {
+        throw new GrpcError(
+          grpc.status.INVALID_ARGUMENT,
+          `The snapshotTemplate on VolumeSnapshotClass is not a valid YAML document.`
+        );
+      }
+      let data = Object.assign({}, driver.options.iscsi.lunSnapshotTemplate, snapshotClassTemplate, {
         src_lun_uuid: lun.uuid,
         taken_by: "democratic-csi",
         description: name, //check
       });
-      if ('isLocked' in normalizedParameters) {
-        data.is_locked = driver.parseBoolean(normalizedParameters.isLocked);
-      }
-      if (normalizedParameters.consistency === "AppConsistent") {
-        data.is_app_consistent = true;
-      } else if (normalizedParameters.consistency === 'CrashConsistent') {
-        data.is_app_consistent = false;
-      } else if ('consistency' in normalizedParameters.consistency) {
-        throw new GrpcError(
-          grpc.status.INVALID_ARGUMENT,
-          `snapshot consistency must be either CrashConsistent or AppConsistent`
-        );
-      }
 
       await httpClient.CreateSnapshot(data);
       snapshot = await httpClient.GetSnapshotByLunUUIDAndName(lun.uuid, name);