-
Notifications
You must be signed in to change notification settings - Fork 168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Newly added test cases : disable/enable audit_logging in libvirtd.log #4421
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
- conf_file.libvirtd_conf.set_audit_logging: | ||
type = set_audit_logging | ||
start_vm = no | ||
variants: | ||
- positive_test: | ||
variants: | ||
- disable_audit_log: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dzhengfy |
||
enable_audit = no | ||
- enable_audit_log: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dzhengfy |
||
enable_audit = yes |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
import logging | ||
import os | ||
|
||
from avocado.utils import process | ||
from avocado.utils import service | ||
|
||
from virttest import data_dir | ||
from virttest import utils_libvirtd | ||
from virttest import utils_split_daemons | ||
|
||
from virttest.utils_test import libvirt | ||
|
||
from virttest.utils_config import LibvirtdConfig | ||
from virttest.utils_config import VirtQemudConfig | ||
|
||
LOG = logging.getLogger('avocado.' + __name__) | ||
|
||
|
||
def run(test, params, env): | ||
""" | ||
Test Disable/enable audit_logging in libvirtd.log. | ||
|
||
1) Enable/disable audit_logging in libvirtd.conf; | ||
2) Restart libvirtd daemon; | ||
3) Check if audit logging as expected; | ||
""" | ||
# Here it needs manipulate libvird config as test step, so it doesn't use default config | ||
# enabled by avocado-vt framework at start | ||
def config_libvirtd_log(enable_audit_logging=False): | ||
""" | ||
Configure audit log level" | ||
|
||
:param enable_audit_logging: bool indicating enable/disable audit_logging | ||
""" | ||
libvirtd_config.log_outputs = "1:file:%s" % log_config_path | ||
libvirtd_config.log_level = 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Above two lines are unnecessary because the avocado-vt env_process.py has enabled the debug log by default and you could get the default log file path via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dzhengfy There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vasekhodina , what's your opinion ? since I noticed you have some thoughts about clean code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with Chunfu. Plus the log location is already configured on line 83, so if I would do any changes I would do them there. |
||
libvirtd_config.audit_level = 1 | ||
if enable_audit_logging: | ||
libvirtd_config.audit_logging = 1 | ||
utils_libvirtd.Libvirtd('virtqemud').restart() | ||
|
||
def ensure_auditd_started(): | ||
""" | ||
Check audit service status and start it if it's not running | ||
""" | ||
service_name = 'auditd' | ||
service_mgr = service.ServiceManager() | ||
status = service_mgr.status(service_name) | ||
LOG.debug('Service status is %s', status) | ||
if not status: | ||
service_mgr.start(service_name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the clean code rule of "Functions should do one thing. They should do it well. They should do it only." broken here since we're not only checking but also starting the service? Wouldn't it be better to move lines 52 and 53 out of this function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rename the function name to ensure_auditd_started |
||
|
||
def check_virt_type_from_audit_log(): | ||
""" | ||
Check whether virt type: VIRT_CONTROL|VIRT_MACHINE_ID|VIRT_RESOURCE in audit.log | ||
""" | ||
cmd = 'cat /var/log/audit/audit.log |grep -E "type=VIRT_CONTROL|VIRT_MACHINE_ID|VIRT_RESOURCE"' | ||
if process.system(cmd, ignore_status=True, shell=True): | ||
test.fail("Check virt type failed with %s" % cmd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before test.fail(), we'd better to log the content of 'cat /var/log/audit/audit.log' There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dzhengfy |
||
|
||
def clean_up_audit_log_file(): | ||
""" | ||
Clean up audit message in log file. | ||
""" | ||
cmd = "> /var/log/audit/audit.log; > %s" % log_config_path | ||
process.run(cmd, ignore_status=False, | ||
shell=True) | ||
|
||
def check_msg_in_libvirtd_log(str_to_grep): | ||
""" | ||
Check audit message in libvirtd log file | ||
|
||
param: str_to_grep: captured message in string | ||
""" | ||
if not libvirt.check_logfile(str_to_grep, log_config_path, str_in_log=enable_audit_logging): | ||
test.fail("Check message log:%s failed in log file:%s" % (str_to_grep, log_config_path)) | ||
|
||
vm_name = params.get("main_vm", "avocado-vt-vm1") | ||
vm = env.get_vm(vm_name) | ||
enable_audit_logging = "yes" == params.get("enable_audit", "no") | ||
|
||
# Get LibvirtdConfig or VirtQemuConfig | ||
log_config_path = os.path.join(data_dir.get_tmp_dir(), "libvirtd.log") | ||
libvirtd_config = VirtQemudConfig() if utils_split_daemons.is_modular_daemon() else LibvirtdConfig() | ||
|
||
try: | ||
# Need stop VM first | ||
if vm.is_alive(): | ||
vm.destroy() | ||
config_libvirtd_log(enable_audit_logging) | ||
# Clean up old audit message from log file | ||
clean_up_audit_log_file() | ||
|
||
# Check and start audit daemon service if necessary | ||
ensure_auditd_started() | ||
|
||
vm.start() | ||
check_virt_type_from_audit_log() | ||
check_msg_in_libvirtd_log("virDomainAudit") | ||
finally: | ||
libvirtd_config.restore() | ||
utils_libvirtd.Libvirtd('virtqemud').restart() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refer to other naming style, we do not add verb in the fie name. So would you consider to change it like
audit_log.cfg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dzhengfy
I have no idea why file name don't add verb, could you point to me the link?
In libvirt virtualization, update/attach/detach/set/enable/disable are fundamental features and API exposed by libvirt.
So I am confused why not those key actions can be not in filename