Skip to content

Commit

Permalink
Minimal fix for audit storage config. (#13686)
Browse files Browse the repository at this point in the history
Fix divide by zero issue.
Fix typo and flake8 issue.
Update CI tests to include disable of quota and reservation.
  • Loading branch information
mgrimesix committed May 8, 2024
1 parent 0b48ec0 commit 97b64fa
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
13 changes: 8 additions & 5 deletions src/middlewared/middlewared/plugins/audit/audit.py
Expand Up @@ -382,9 +382,10 @@ def cleanup_reports(self):

@private
async def validate_local_storage(self, new, old, verrors):
new_volsize = new['quota'] * _GIB
used = new['space']['used_by_dataset'] + new['space']['used_by_snapshots']
if old['quota'] != new['quota']:
# A quota of `0` == `disable`
if new['quota'] and (old['quota'] != new['quota']):
new_volsize = new['quota'] * _GIB
used = new['space']['used_by_dataset'] + new['space']['used_by_snapshots']
if used / new_volsize > new['quota_fill_warning'] / 100:
verrors.add(
'audit_update.quota',
Expand All @@ -410,10 +411,12 @@ async def update_audit_dataset(self, new):

payload = {}
if new['quota'] != old_quota / _GIB:
payload['refquota'] = {'parsed': f'{new["quota"]}G'}
quota_val = "none" if new['quota'] == 0 else f'{new["quota"]}G'
payload['refquota'] = {'parsed': quota_val}

if new['reservation'] != old_reservation / _GIB:
payload['refreservation'] = {'parsed': f'{new["reservation"]}G'}
reservation_val = "none" if new['reservation'] == 0 else f'{new["reservation"]}G'
payload['refreservation'] = {'parsed': reservation_val}

if new["quota_fill_warning"] != old_warn:
payload[QUOTA_WARN] = {'parsed': str(new['quota_fill_warning'])}
Expand Down
4 changes: 2 additions & 2 deletions src/middlewared/middlewared/plugins/audit/utils.py
Expand Up @@ -118,7 +118,7 @@ def parse_query_filters(
if not services_to_check:
# These filters are guaranteed to have no results. Bail
# early and let caller handle it.
break;
break

if skip_sql_filters:
# User has manually specified to pass all these filters to datastore
Expand Down Expand Up @@ -159,7 +159,7 @@ def requires_python_filtering(
# Field is being selected that may not be safe for SQL select
for entry in to_investigate:
# Selecting subkey in entry is not currently supported
if '.' in entry or isiinstance(entry, tuple):
if '.' in entry or isinstance(entry, tuple):
return True

if len(services) > 1:
Expand Down
27 changes: 26 additions & 1 deletion tests/api2/test_audit_basic.py
Expand Up @@ -36,6 +36,21 @@ def initialize_for_smb_tests(request):
yield {'dataset': ds, 'share': s, 'user': u}


@pytest.fixture(scope='module')
def audit_config(request):
defaults = {
'retention': 7,
'quota': 0,
'reservation': 0,
'quota_fill_warning': 80,
'quota_fill_critical': 95
}
try:
yield defaults
finally:
call('audit.update', defaults)


def test_audit_config_defaults(request):
config = call('audit.config')
for key in [
Expand All @@ -61,7 +76,7 @@ def test_audit_config_defaults(request):
assert 'SMB' in config['enabled_services']


def test_audit_config_updates(request):
def test_audit_config_updates(audit_config):
"""
This test just validates that setting values has expected results.
"""
Expand All @@ -87,6 +102,16 @@ def test_audit_config_updates(request):
assert new_config['quota_fill_warning'] == 70
assert new_config['quota_fill_critical'] == 80

# Test disable reservation
new_config = call('audit.update', {'reservation': 0})
assert new_config['reservation'] == 0

# Test disable quota
new_config = call('audit.update', {'quota': 0})
assert new_config['quota'] == 0

# TODO: Get values from zfs.dataset.query


@pytest.mark.dependency(name="AUDIT_OPS_PERFORMED")
def test_audit_query(initialize_for_smb_tests):
Expand Down

0 comments on commit 97b64fa

Please sign in to comment.