Skip to content

Commit

Permalink
NAS-133124 / 25.04 / Convert iscsi.target.* to new api (#15223)
Browse files Browse the repository at this point in the history
* Convert iscsi.target.* to new api

* Correct network specification in tests
  • Loading branch information
bmeagherix authored Dec 17, 2024
1 parent e7ffb75 commit 5d1a781
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 46 deletions.
5 changes: 3 additions & 2 deletions src/middlewared/middlewared/api/v25_04_0/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from .group import * # noqa
from .iscsi_auth import * # noqa
from .iscsi_extent import * # noqa
from .iscsi_target import * # noqa
from .keychain import * # noqa
from .k8s_to_docker import * # noqa
from .netdata import * # noqa
Expand All @@ -32,10 +33,10 @@
from .pool_scrub import * # noqa
from .pool_snapshottask import * # noqa
from .privilege import * # noqa
from .reporting import * # noqa
from .reporting_exporters import * # noqa
from .rdma import * # noqa
from .rdma_interface import * # noqa
from .reporting import * # noqa
from .reporting_exporters import * # noqa
from .smartctl import * # noqa
from .smb import * # noqa
from .snmp import * # noqa
Expand Down
100 changes: 100 additions & 0 deletions src/middlewared/middlewared/api/v25_04_0/iscsi_target.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import re
from typing import Literal

from pydantic import AfterValidator, StringConstraints
from typing_extensions import Annotated

from middlewared.api.base import (BaseModel, Excluded, ForUpdateMetaclass, IscsiAuthType, NonEmptyString,
excluded_field, match_validator)

RE_TARGET_NAME = re.compile(r'^[-a-z0-9\.:]+$')

__all__ = [
"IscsiTargetEntry",
"IscsiTargetValidateNameArgs",
"IscsiTargetValidateNameResult",
"IscsiTargetCreateArgs",
"IscsiTargetCreateResult",
"IscsiTargetUpdateArgs",
"IscsiTargetUpdateResult",
"IscsiTargetDeleteArgs",
"IscsiTargetDeleteResult",
"IscsiTargetRemoveArgs",
"IscsiTargetRemoveResult"
]


class IscsiGroup(BaseModel):
portal: int
initiator: int | None = None
authmethod: IscsiAuthType = 'NONE'
auth: int | None = None


class IscsiTargetEntry(BaseModel):
id: int
name: Annotated[NonEmptyString,
AfterValidator(
match_validator(
RE_TARGET_NAME,
"Name can only contain lowercase alphanumeric charactersplus dot (.), dash (-), and colon (:)",
)
),
StringConstraints(max_length=120)]
alias: str | None = None
mode: Literal['ISCSI', 'FC', 'BOTH'] = 'ISCSI'
groups: list[IscsiGroup] = []
auth_networks: list[str] = [] # IPvAnyNetwork: "Object of type IPv4Network is not JSON serializable", etc
rel_tgt_id: int


class IscsiTargetValidateNameArgs(BaseModel):
name: str
existing_id: int | None = None


class IscsiTargetValidateNameResult(BaseModel):
result: str | None


class IscsiTargetCreate(IscsiTargetEntry):
id: Excluded = excluded_field()
rel_tgt_id: Excluded = excluded_field()


class IscsiTargetCreateArgs(BaseModel):
iscsi_target_create: IscsiTargetCreate


class IscsiTargetCreateResult(BaseModel):
result: IscsiTargetEntry


class IscsiTargetUpdate(IscsiTargetCreate, metaclass=ForUpdateMetaclass):
pass


class IscsiTargetUpdateArgs(BaseModel):
id: int
iscsi_target_update: IscsiTargetUpdate


class IscsiTargetUpdateResult(BaseModel):
result: IscsiTargetEntry


class IscsiTargetDeleteArgs(BaseModel):
id: int
force: bool = False


class IscsiTargetDeleteResult(BaseModel):
result: Literal[True]


class IscsiTargetRemoveArgs(BaseModel):
name: str


class IscsiTargetRemoveResult(BaseModel):
result: None
79 changes: 43 additions & 36 deletions src/middlewared/middlewared/plugins/iscsi_/targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@
import subprocess
from collections import defaultdict

from pydantic import IPvAnyNetwork

import middlewared.sqlalchemy as sa
from middlewared.schema import Bool, Dict, Int, IPAddr, List, Patch, Str, accepts
from middlewared.api import api_method
from middlewared.api.current import (IscsiTargetCreateArgs, IscsiTargetCreateResult, IscsiTargetDeleteArgs,
IscsiTargetDeleteResult, IscsiTargetEntry, IscsiTargetRemoveArgs,
IscsiTargetRemoveResult, IscsiTargetUpdateArgs, IscsiTargetUpdateResult,
IscsiTargetValidateNameArgs, IscsiTargetValidateNameResult)
from middlewared.service import CallError, CRUDService, ValidationErrors, private
from middlewared.utils import UnexpectedFailure, run
from .utils import AUTHMETHOD_LEGACY_MAP
Expand Down Expand Up @@ -56,6 +62,7 @@ class Config:
datastore_extend = 'iscsi.target.extend'
cli_namespace = 'sharing.iscsi.target'
role_prefix = 'SHARING_ISCSI_TARGET'
entry = IscsiTargetEntry

@private
async def extend(self, data):
Expand All @@ -80,23 +87,12 @@ async def extend(self, data):
)
return data

@accepts(Dict(
'iscsi_target_create',
Str('name', required=True),
Str('alias', null=True),
Str('mode', enum=['ISCSI', 'FC', 'BOTH'], default='ISCSI'),
List('groups', items=[
Dict(
'group',
Int('portal', required=True),
Int('initiator', default=None, null=True),
Str('authmethod', enum=['NONE', 'CHAP', 'CHAP_MUTUAL'], default='NONE'),
Int('auth', default=None, null=True),
),
]),
List('auth_networks', items=[IPAddr('ip', network=True)]),
register=True
), audit='Create iSCSI target', audit_extended=lambda data: data["name"])
@api_method(
IscsiTargetCreateArgs,
IscsiTargetCreateResult,
audit='Create iSCSI target',
audit_extended=lambda data: data['name']
)
async def do_create(self, data):
"""
Create an iSCSI Target.
Expand Down Expand Up @@ -258,9 +254,20 @@ async def __validate(self, verrors, data, schema_name, old=None):
f'Authentication group {group["auth"]} does not support CHAP Mutual'
)

@accepts(Str('name'),
Int('existing_id', null=True, default=None),
roles=['SHARING_ISCSI_TARGET_WRITE'])
for i, network in enumerate(data['auth_networks']):
try:
IPvAnyNetwork(network)
except Exception:
verrors.add(
f'{schema_name}.auth_networks.{i}',
f'Auth network "{network}" is not a valid IPv4 or IPv6 network'
)

@api_method(
IscsiTargetValidateNameArgs,
IscsiTargetValidateNameResult,
roles=['SHARING_ISCSI_TARGET_WRITE']
)
async def validate_name(self, name, existing_id):
"""
Returns validation error for iSCSI target name
Expand All @@ -279,15 +286,11 @@ async def validate_name(self, name, existing_id):
if names:
return 'Target with this name already exists'

@accepts(
Int('id'),
Patch(
'iscsi_target_create',
'iscsi_target_update',
('attr', {'update': True})
),
@api_method(
IscsiTargetUpdateArgs,
IscsiTargetUpdateResult,
audit='Update iSCSI target',
audit_callback=True,
audit_callback=True
)
async def do_update(self, audit_callback, id_, data):
"""
Expand Down Expand Up @@ -325,11 +328,12 @@ async def do_update(self, audit_callback, id_, data):

return await self.get_instance(id_)

@accepts(Int('id'),
Bool('force', default=False),
audit='Delete iSCSI target',
audit_callback=True,
)
@api_method(
IscsiTargetDeleteArgs,
IscsiTargetDeleteResult,
audit='Delete iSCSI target',
audit_callback=True
)
async def do_delete(self, audit_callback, id_, force):
"""
Delete iSCSI Target of `id`.
Expand Down Expand Up @@ -374,8 +378,11 @@ async def do_delete(self, audit_callback, id_, force):
self.logger.error('Failed to clean up target initiators for %r', target['name'], exc_info=True)
return rv

@private
@accepts(Str('name'))
@api_method(
IscsiTargetRemoveArgs,
IscsiTargetRemoveResult,
private=True
)
async def remove_target(self, name):
# We explicitly need to do this unfortunately as scst does not accept these changes with a reload
# So this is the best way to do this without going through a restart of the service
Expand Down
6 changes: 3 additions & 3 deletions src/middlewared/middlewared/plugins/iscsi_/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ class IscsiAuthType(StrEnum):


AUTHMETHOD_LEGACY_MAP = bidict.bidict({
'None': IscsiAuthType.NONE,
'CHAP': IscsiAuthType.CHAP,
'CHAP Mutual': IscsiAuthType.CHAP_MUTUAL,
'None': IscsiAuthType.NONE.value,
'CHAP': IscsiAuthType.CHAP.value,
'CHAP Mutual': IscsiAuthType.CHAP_MUTUAL.value,
})

# Currently SCST has this limit (scst_vdisk_dev->name)
Expand Down
9 changes: 4 additions & 5 deletions tests/api2/test_iscsi_auth_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def test_iscsi_auth_networks_netmask_24(my_ip4, valid):
call(
'iscsi.target.update',
config['target']['id'],
{'auth_networks': ["8.8.8.8/24", f"{good_ip}/24"] if valid else ["8.8.8.8/24", f"{bad_ip}/24"]}
{'auth_networks': ["8.8.8.0/24", f"{good_ip}/24"] if valid else ["8.8.8.0/24", f"{bad_ip}/24"]}
)
portal_listen_details = config['portal']['listen'][0]
assert target_login_test(
Expand All @@ -170,9 +170,8 @@ def test_iscsi_auth_networks_netmask_24(my_ip4, valid):

@pytest.mark.parametrize('valid', [True, False])
def test_iscsi_auth_networks_netmask_16(my_ip4, valid):
# good_ip will be our IP with the second last byte changed and last byte cleared
n = (int(my_ip4.split('.')[2]) + 1) % 256
good_ip = '.'.join(my_ip4.split('.')[:2] + [str(n), '0'])
# good_ip will be our IP with the last 2 bytes cleared
good_ip = '.'.join(my_ip4.split('.')[:2] + ['0', '0'])
# bad_ip will be the good_ip with the second byte changed
ip_list = good_ip.split('.')
n = (int(ip_list[1]) + 1) % 256
Expand All @@ -181,7 +180,7 @@ def test_iscsi_auth_networks_netmask_16(my_ip4, valid):
call(
'iscsi.target.update',
config['target']['id'],
{'auth_networks': ["8.8.8.8/16", f"{good_ip}/16"] if valid else ["8.8.8.8/16", f"{bad_ip}/16"]}
{'auth_networks': ["8.8.0.0/16", f"{good_ip}/16"] if valid else ["8.8.0.0/16", f"{bad_ip}/16"]}
)
portal_listen_details = config['portal']['listen'][0]
assert target_login_test(
Expand Down

0 comments on commit 5d1a781

Please sign in to comment.