Copilot commented on code in PR #13281:
URL: https://github.com/apache/cloudstack/pull/13281#discussion_r4216878098
##########
python/lib/cloudutils/serviceConfig.py:
##########
@@ -652,7 +583,6 @@ def config(self):
filename = "/etc/libvirt/qemu.conf"
cfo = configFileOps(filename, self)
- cfo.addEntry("security_driver", "\"none\"")
cfo.addEntry("user", "\"root\"")
cfo.addEntry("group", "\"root\"")
cfo.addEntry("vnc_listen", "\"0.0.0.0\"")
Review Comment:
Removing the explicit `security_driver` line changes libvirt behavior
depending on distro defaults and existing config, and operators no longer have
a CloudStack-managed way to set it intentionally. If the goal is “don’t enforce
a choice,” an alternative is to support an explicit, opt-in agent/global
setting (e.g., `libvirt.security_driver=none|selinux|apparmor|unset`) and only
write `security_driver` when the operator sets it. This keeps the setup neutral
by default while still providing a supported knob for controlled environments.
##########
python/lib/cloudutils/serviceConfig.py:
##########
@@ -519,75 +519,6 @@ def config(self):
logging.debug(e)
return False
-class securityPolicyConfigUbuntu(serviceCfgBase):
- def __init__(self, syscfg):
- super(securityPolicyConfigUbuntu, self).__init__(syscfg)
- self.serviceName = "Apparmor"
-
- def config(self):
- try:
- cmd = bash("service apparmor status")
- if not cmd.isSuccess() or cmd.getStdout() == "":
- self.spRunning = False
- return True
-
- if not bash("apparmor_status |grep libvirt").isSuccess():
- return True
-
- bash("ln -s /etc/apparmor.d/usr.sbin.libvirtd
/etc/apparmor.d/disable/")
- bash("ln -s /etc/apparmor.d/usr.lib.libvirt.virt-aa-helper
/etc/apparmor.d/disable/")
- bash("apparmor_parser -R /etc/apparmor.d/usr.sbin.libvirtd")
- bash("apparmor_parser -R
/etc/apparmor.d/usr.lib.libvirt.virt-aa-helper")
-
- return True
- except:
- raise CloudRuntimeException("Failed to configure apparmor, please
see the /var/log/cloudstack/agent/setup.log for detail, \
- or you can manually disable it before
starting myCloud")
-
- def restore(self):
- try:
- self.syscfg.svo.enableService("apparmor")
- self.syscfg.svo.startService("apparmor")
- return True
- except:
- logging.debug(formatExceptionInfo())
- return False
-
-class securityPolicyConfigRedhat(serviceCfgBase):
- def __init__(self, syscfg):
- super(securityPolicyConfigRedhat, self).__init__(syscfg)
- self.serviceName = "SElinux"
-
- def config(self):
- selinuxEnabled = True
-
- if not bash("selinuxenabled").isSuccess():
- selinuxEnabled = False
-
- if selinuxEnabled:
- try:
- bash("setenforce 0")
- cfo = configFileOps("/etc/selinux/config", self)
- cfo.replace_line("SELINUX=", "SELINUX=permissive")
- return True
- except:
- raise CloudRuntimeException("Failed to configure selinux,
please see the /var/log/cloudstack/agent/setup.log for detail, \
- or you can manually disable it
before starting myCloud")
- else:
- return True
-
- def restore(self):
- try:
- bash("setenforce 1")
- return True
- except:
- logging.debug(formatExceptionInfo())
- return False
-
-class securityPolicyConfigSUSE(securityPolicyConfigRedhat):
- pass
-
-
def configure_libvirt_tls(tls_enabled=False, cfo=None):
save = False
if not cfo:
Review Comment:
The PR description focuses on disabling SELinux/AppArmor configuration
during setup, but the PR also removes a previously CloudStack-applied libvirt
setting (`security_driver=\"none\"`) and deletes a setup script. These are
potentially breaking behavior changes for new installs/automations; please
reflect this in the PR’s type/upgrade notes (e.g., mark as breaking change or
document expected operator actions when hosts run with enforcing MAC policies).
##########
python/lib/cloudutils/syscfg.py:
##########
@@ -167,7 +167,6 @@ def __init__(self, glbEnv):
self.svo = serviceOpsUbuntu()
self.services = [hostConfig(self),
- securityPolicyConfigUbuntu(self),
networkConfigUbuntu(self),
libvirtConfigUbuntu(self),
firewallConfigUbuntu(self),
Review Comment:
With the security policy configurators removed from the provisioning
pipeline, hosts that rely on SELinux/AppArmor adjustments for CloudStack
compatibility may now fail later in less-obvious ways (e.g., libvirt/QEMU
denials). Consider adding explicit setup-time logging (or a preflight check)
that detects enforcing SELinux/AppArmor + active libvirt confinement and
clearly points operators to the expected manual configuration path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
