Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions manila/api/v2/shares.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,10 @@ def _validate_metadata_for_update(self, req, share_id, metadata,
persistent_keys = []

current_share_metadata = db.share_metadata_get(context, share_id)

new_set_once = self.share_api.validate_set_once_metadata(
context, share_id, metadata)

if delete:
_metadata = metadata
for key in persistent_keys:
Expand All @@ -676,7 +680,7 @@ def _validate_metadata_for_update(self, req, share_id, metadata,
_metadata = current_share_metadata.copy()
_metadata.update(metadata_copy)

return _metadata
return _metadata, new_set_once

# NOTE: (ashrod98) original metadata method and policy overrides
@wsgi.Controller.api_version("2.0")
Expand All @@ -691,15 +695,16 @@ def create_metadata(self, req, resource_id, body):
if not self.is_valid_body(body, 'metadata'):
expl = _('Malformed request body')
raise exc.HTTPBadRequest(explanation=expl)
_metadata = self._validate_metadata_for_update(req, resource_id,
body['metadata'],
delete=False)
_metadata, new_set_once = self._validate_metadata_for_update(
req, resource_id, body['metadata'], delete=False)
body['metadata'] = _metadata
metadata = self._create_metadata(req, resource_id, body)

context = req.environ['manila.context']
self.share_api.update_share_from_metadata(context, resource_id,
metadata.get('metadata'))
self.share_api.update_share_from_set_once_metadata(
context, resource_id, new_set_once)
return metadata

@wsgi.Controller.api_version("2.0")
Expand All @@ -708,14 +713,16 @@ def update_all_metadata(self, req, resource_id, body):
if not self.is_valid_body(body, 'metadata'):
expl = _('Malformed request body')
raise exc.HTTPBadRequest(explanation=expl)
_metadata = self._validate_metadata_for_update(req, resource_id,
body['metadata'])
_metadata, new_set_once = self._validate_metadata_for_update(
req, resource_id, body['metadata'])
body['metadata'] = _metadata
metadata = self._update_all_metadata(req, resource_id, body)

context = req.environ['manila.context']
self.share_api.update_share_from_metadata(context, resource_id,
metadata.get('metadata'))
self.share_api.update_share_from_set_once_metadata(
context, resource_id, new_set_once)
return metadata

@wsgi.Controller.api_version("2.0")
Expand All @@ -724,15 +731,16 @@ def update_metadata_item(self, req, resource_id, body, key):
if not self.is_valid_body(body, 'meta'):
expl = _('Malformed request body')
raise exc.HTTPBadRequest(explanation=expl)
_metadata = self._validate_metadata_for_update(req, resource_id,
body['metadata'],
delete=False)
_metadata, new_set_once = self._validate_metadata_for_update(
req, resource_id, body['metadata'], delete=False)
body['metadata'] = _metadata
metadata = self._update_metadata_item(req, resource_id, body, key)

context = req.environ['manila.context']
self.share_api.update_share_from_metadata(context, resource_id,
metadata.get('metadata'))
self.share_api.update_share_from_set_once_metadata(
context, resource_id, new_set_once)
return metadata

@wsgi.Controller.api_version("2.0")
Expand Down
7 changes: 7 additions & 0 deletions manila/common/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@
'(element of the list is <driver_updatable_key>, '
'i.e max_files) can be passed to share drivers as part '
'of metadata create/update operations.'),
cfg.ListOpt('driver_set_once_metadata',
default=['nfs_full_permission'],
help='Metadata keys that can be set only once per share '
'lifetime. On the first set the update is passed to '
'the share driver. Any subsequent attempt to update '
'a key in this list returns HTTP 400. '
'Example: nfs_full_permission'),
cfg.ListOpt('driver_updatable_subnet_metadata',
default=[],
help='Metadata keys that will decide which share network '
Expand Down
6 changes: 6 additions & 0 deletions manila/exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,12 @@ class InvalidMetadataSize(Invalid):
message = _("Invalid metadata size.")


class MetadataSetOnceViolation(Invalid):
message = _("Metadata key '%(key)s' can only be set once and already "
"has value '%(current_value)s'. Updating set-once metadata "
"is not allowed.")


class SecurityServiceNotFound(NotFound):
message = _("Security service %(security_service_id)s could not be found.")

Expand Down
35 changes: 35 additions & 0 deletions manila/share/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,41 @@ def update_share_from_metadata(self, context, share_id, metadata):
self.share_rpcapi.update_share_from_metadata(context, share,
driver_metadata)

def validate_set_once_metadata(self, context, share_id, metadata):
"""Raise MetadataSetOnceViolation if a set-once key is being re-set.

Returns a dict of set-once key/value pairs that are new (not yet in
the DB), so the caller can forward them to the driver after the DB
write without a second DB query.
"""
set_once_keys = getattr(CONF, 'driver_set_once_metadata', [])
if not set_once_keys:
return {}
existing = self.db.share_metadata_get(context, share_id)
new_set_once = {}
for key in set_once_keys:
if key not in metadata:
continue
if key in existing:
raise exception.MetadataSetOnceViolation(
key=key, current_value=existing[key])
new_set_once[key] = metadata[key]
return new_set_once

def update_share_from_set_once_metadata(self, context, share_id,
new_set_once_metadata):
"""Pass new set-once metadata keys to the driver.

Expects only the keys that were not previously in the DB (as returned
by validate_set_once_metadata). No DB re-query is performed here
because the caller already has the pre-write snapshot of what is new.
"""
if not new_set_once_metadata:
return
share = self.get(context, share_id)
self.share_rpcapi.update_share_from_metadata(
context, share, new_set_once_metadata)

def update_share_network_subnet_from_metadata(self, context,
share_network_id,
share_network_subnet_id,
Expand Down
23 changes: 23 additions & 0 deletions manila/share/drivers/netapp/dataontap/client/client_cmode.py
Original file line number Diff line number Diff line change
Expand Up @@ -2862,6 +2862,29 @@ def update_volume_snapshot_policy(self, volume_name, snapshot_policy):
}
self.send_request('volume-modify-iter', api_args)

@na_utils.trace
def set_volume_unix_permissions(self, volume_name, unix_permissions):
"""Set unix permissions on the specified volume root."""
api_args = {
'query': {
'volume-attributes': {
'volume-id-attributes': {
'name': volume_name,
},
},
},
'attributes': {
'volume-attributes': {
'volume-security-attributes': {
'volume-security-unix-attributes': {
'permissions': unix_permissions,
},
},
},
},
}
self.send_request('volume-modify-iter', api_args)

@na_utils.trace
def set_sis_config(self, volume_name, api_args):
api_args.update({'path': '/vol/%s' % volume_name})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1219,6 +1219,16 @@ def update_volume_snapshot_policy(self, volume_name, snapshot_policy):
# update snapshot policy
self.send_request(f'/storage/volumes/{uuid}', 'patch', body=body)

@na_utils.trace
def set_volume_unix_permissions(self, volume_name, unix_permissions):
"""Set unix permissions on the specified volume root."""
volume = self._get_volume_by_args(vol_name=volume_name)
uuid = volume['uuid']
body = {
'nas.unix_permissions': unix_permissions,
}
self.send_request(f'/storage/volumes/{uuid}', 'patch', body=body)

@na_utils.trace
def reset_autosize_attributes(self, aggr, volume_name):
'''Reset autosize attributes according to Volume type (RW or DP)
Expand Down
24 changes: 24 additions & 0 deletions manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6400,6 +6400,29 @@ def update_volume_snapshot_policy(self, share, snapshot_policy,
vserver_client.update_volume_snapshot_policy(share_name,
snapshot_policy)

@na_utils.trace
def update_nfs_full_permission(self, share, value, share_server=None):
"""Set unix permissions to 0777 on the share volume.

Triggered by the metadata key 'nfs_full_permission' = 'true'.
The value 'false' is intentionally a no-op here; the API layer
prevents re-setting a key that was already written so this method
is only called with value='true'.
"""
value = value.lower()
if value not in ('true', 'false'):
err_msg = _("Invalid nfs_full_permission value '%s'. "
"Accepted values are 'true' or 'false'.") % value
raise exception.NetAppException(err_msg)
if value == 'false':
return
share_name = self._get_backend_share_name(share['id'])
vserver, vserver_client = self._get_vserver(share_server=share_server)
volume = vserver_client.get_volume(share_name)
volume_type = volume.get('type')
if volume_type != 'dp':
vserver_client.set_volume_unix_permissions(share_name, '0777')

@na_utils.trace
def update_showmount(self, showmount, share_server=None):
showmount = showmount.lower()
Expand All @@ -6425,6 +6448,7 @@ def update_share_from_metadata(self, context, share, metadata,
metadata_update_func_map = {
"snapshot_policy": "update_volume_snapshot_policy",
"cross_volume_dedupe": "update_cross_volume_dedupe",
"nfs_full_permission": "update_nfs_full_permission",
}

for k, v in metadata.items():
Expand Down
69 changes: 67 additions & 2 deletions manila/tests/api/v2/test_shares.py
Original file line number Diff line number Diff line change
Expand Up @@ -2323,11 +2323,12 @@ def test_create_metadata(self):
body = {'metadata': {'key1': 'val1', 'key2': 'val2'}}
mock_validate = self.mock_object(
self.controller, '_validate_metadata_for_update',
mock.Mock(return_value=body['metadata']))
mock.Mock(return_value=(body['metadata'], {})))
mock_create = self.mock_object(
self.controller, '_create_metadata',
mock.Mock(return_value=body))
self.mock_object(share_api.API, 'update_share_from_metadata')
self.mock_object(share_api.API, 'update_share_from_set_once_metadata')

req = fakes.HTTPRequest.blank(
'/v2/shares/%s/metadata' % id)
Expand All @@ -2343,11 +2344,12 @@ def test_update_all_metadata(self):
body = {'metadata': {'key1': 'val1', 'key2': 'val2'}}
mock_validate = self.mock_object(
self.controller, '_validate_metadata_for_update',
mock.Mock(return_value=body['metadata']))
mock.Mock(return_value=(body['metadata'], {})))
mock_update = self.mock_object(
self.controller, '_update_all_metadata',
mock.Mock(return_value=body))
self.mock_object(share_api.API, 'update_share_from_metadata')
self.mock_object(share_api.API, 'update_share_from_set_once_metadata')

req = fakes.HTTPRequest.blank(
'/v2/shares/%s/metadata' % id)
Expand All @@ -2365,6 +2367,69 @@ def test_delete_metadata(self):
self.controller.delete_metadata(req, id, 'fake_key')
mock_delete.assert_called_once_with(req, id, 'fake_key')

def test_create_metadata_set_once_blocks_re_set(self):
share_id = 'fake_share_id'
self.mock_object(db, 'share_metadata_get',
mock.Mock(
return_value={'nfs_full_permission': 'true'}))
self.mock_object(
share_api.API, 'update_share_from_metadata')
self.mock_object(
share_api.API, 'update_share_from_set_once_metadata')
self.mock_object(
share_api.API, 'validate_set_once_metadata',
mock.Mock(side_effect=exception.MetadataSetOnceViolation(
key='nfs_full_permission', current_value='true')))

body = {'metadata': {'nfs_full_permission': 'false'}}
req = fakes.HTTPRequest.blank('/v2/shares/%s/metadata' % share_id)
self.assertRaises(exception.MetadataSetOnceViolation,
self.controller.create_metadata,
req, share_id, body)

def test_update_metadata_item_set_once_blocks_re_set(self):
share_id = 'fake_share_id'
self.mock_object(db, 'share_metadata_get',
mock.Mock(
return_value={'nfs_full_permission': 'true'}))
self.mock_object(
share_api.API, 'update_share_from_metadata')
self.mock_object(
share_api.API, 'update_share_from_set_once_metadata')
self.mock_object(
share_api.API, 'validate_set_once_metadata',
mock.Mock(side_effect=exception.MetadataSetOnceViolation(
key='nfs_full_permission', current_value='true')))

body = {'metadata': {'nfs_full_permission': 'false'},
'meta': {'nfs_full_permission': 'false'}}
req = fakes.HTTPRequest.blank(
'/v2/shares/%s/metadata/nfs_full_permission' % share_id)
self.assertRaises(exception.MetadataSetOnceViolation,
self.controller.update_metadata_item,
req, share_id, body, 'nfs_full_permission')

def test_create_metadata_set_once_first_set_allowed(self):
share_id = 'fake_share_id'
body = {'metadata': {'nfs_full_permission': 'true'}}
CONF.set_override('driver_set_once_metadata', ['nfs_full_permission'])
self.mock_object(db, 'share_metadata_get',
mock.Mock(return_value={}))
self.mock_object(self.controller, '_create_metadata',
mock.Mock(return_value=body))
mock_updatable = self.mock_object(
share_api.API, 'update_share_from_metadata')
mock_set_once = self.mock_object(
share_api.API, 'update_share_from_set_once_metadata')

req = fakes.HTTPRequest.blank('/v2/shares/%s/metadata' % share_id)
result = self.controller.create_metadata(req, share_id, body)

self.assertEqual(body, result)
mock_updatable.assert_called_once()
mock_set_once.assert_called_once_with(
mock.ANY, share_id, {'nfs_full_permission': 'true'})


def _fake_access_get(self, ctxt, access_id):

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3862,6 +3862,32 @@ def test_update_volume_snapshot_policy(self):
self.client.send_request.assert_called_once_with(
'volume-modify-iter', volume_modify_iter_api_args)

def test_set_volume_unix_permissions(self):
self.mock_object(self.client, 'send_request')

self.client.set_volume_unix_permissions(fake.SHARE_NAME, '0777')

expected_args = {
'query': {
'volume-attributes': {
'volume-id-attributes': {
'name': fake.SHARE_NAME,
},
},
},
'attributes': {
'volume-attributes': {
'volume-security-attributes': {
'volume-security-unix-attributes': {
'permissions': '0777',
},
},
},
},
}
self.client.send_request.assert_called_once_with(
'volume-modify-iter', expected_args)

def test_enable_dedup(self):

self.mock_object(self.client, 'send_request')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3238,6 +3238,19 @@ def test_update_volume_snapshot_policy(self):
'patch', body=body)
mock_get_vol.assert_called_once_with(vol_name='fake_volume_name')

def test_set_volume_unix_permissions(self):
return_uuid = {'uuid': 'fake_uuid'}
mock_get_vol = self.mock_object(self.client, '_get_volume_by_args',
mock.Mock(return_value=return_uuid))
mock_sr = self.mock_object(self.client, 'send_request')

self.client.set_volume_unix_permissions('fake_volume_name', '0777')

body = {'nas.unix_permissions': '0777'}
mock_sr.assert_called_once_with('/storage/volumes/fake_uuid',
'patch', body=body)
mock_get_vol.assert_called_once_with(vol_name='fake_volume_name')

@ddt.data(True, False)
def test_update_volume_efficiency_attributes(self, status):
response = {
Expand Down
Loading