Skip to content

NAS backup: compression, encryption, bandwidth throttle, integrity check #12898

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.

Sign up for GitHub

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

Open
jmsperu wants to merge 2 commits into
base: 4.22
Choose a base branch
from
+480 −15
Open

NAS backup: compression, encryption, bandwidth throttle, integrity check #12898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c40fdd
NAS backup enhancements: compression, encryption, bandwidth throttle,…
jmsperu Mar 26, 2026
05076db
Address Copilot review feedback on NAS backup PR #12898
jmsperu Mar 30, 2026
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
16 changes: 16 additions & 0 deletions core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
import com.cloud.agent.api.LogLevel;
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class TakeBackupCommand extends Command {
private String vmName;
Expand All @@ -35,6 +37,8 @@ public class TakeBackupCommand extends Command {
private Boolean quiesce;
@LogLevel(LogLevel.Log4jLevel.Off)
private String mountOptions;
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

details may carry sensitive values (e.g., an encryption passphrase). CloudStack’s Gson logging uses LoggingExclusionStrategy with @LogLevel to exclude fields, so leaving this unannotated can leak secrets in debug logs. Annotate details with @LogLevel(Off) (or avoid putting secrets in this map).

Suggested change
private String mountOptions;
private String mountOptions;
@LogLevel(LogLevel.Log4jLevel.Off)

Copilot uses AI. Check for mistakes.
@LogLevel(LogLevel.Log4jLevel.Off)
private Map<String, String> details = new HashMap<>();

public TakeBackupCommand(String vmName, String backupPath) {
super();
Expand Down Expand Up @@ -106,6 +110,18 @@ public void setQuiesce(Boolean quiesce) {
this.quiesce = quiesce;
}

public Map<String, String> getDetails() {
return details;
}

public void setDetails(Map<String, String> details) {
this.details = details != null ? details : new HashMap<>();
}

public void addDetail(String key, String value) {
this.details.put(key, value);
}

@Override
public boolean executeInSequence() {
return true;
Expand Down
68 changes: 67 additions & 1 deletion plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,46 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
true,
BackupFrameworkEnabled.key());

ConfigKey<Boolean> NASBackupCompressionEnabled = new ConfigKey<>("Advanced", Boolean.class,
"nas.backup.compression.enabled",
"false",
"Enable qcow2 compression for NAS backup files.",
true,
ConfigKey.Scope.Zone,
BackupFrameworkEnabled.key());

ConfigKey<Boolean> NASBackupEncryptionEnabled = new ConfigKey<>("Advanced", Boolean.class,
"nas.backup.encryption.enabled",
"false",
"Enable LUKS encryption for NAS backup files.",
true,
ConfigKey.Scope.Zone,
BackupFrameworkEnabled.key());

ConfigKey<String> NASBackupEncryptionPassphrase = new ConfigKey<>("Secure", String.class,
"nas.backup.encryption.passphrase",
"",
"Passphrase for LUKS encryption of NAS backup files. Required when encryption is enabled.",
true,
ConfigKey.Scope.Zone,
BackupFrameworkEnabled.key());

ConfigKey<Integer> NASBackupBandwidthLimitMbps = new ConfigKey<>("Advanced", Integer.class,
"nas.backup.bandwidth.limit.mbps",
"0",
"Bandwidth limit in MiB/s for backup operations (0 = unlimited).",
true,
ConfigKey.Scope.Zone,
BackupFrameworkEnabled.key());

ConfigKey<Boolean> NASBackupIntegrityCheckEnabled = new ConfigKey<>("Advanced", Boolean.class,
"nas.backup.integrity.check",
"false",
"Run qemu-img check on backup files after creation to verify integrity.",
true,
ConfigKey.Scope.Zone,
BackupFrameworkEnabled.key());

@Inject
private BackupDao backupDao;

Expand Down Expand Up @@ -205,6 +245,27 @@ public Pair<Boolean, Backup> takeBackup(final VirtualMachine vm, Boolean quiesce
command.setMountOptions(backupRepository.getMountOptions());
command.setQuiesce(quiesceVM);

// Pass optional backup enhancement settings from zone-scoped configs
Long zoneId = vm.getDataCenterId();
if (Boolean.TRUE.equals(NASBackupCompressionEnabled.valueIn(zoneId))) {
command.addDetail("compression", "true");
}
if (Boolean.TRUE.equals(NASBackupEncryptionEnabled.valueIn(zoneId))) {
String passphrase = NASBackupEncryptionPassphrase.valueIn(zoneId);
if (passphrase == null || passphrase.isEmpty()) {
throw new CloudRuntimeException("NAS backup encryption is enabled but no passphrase is configured (nas.backup.encryption.passphrase)");
}
command.addDetail("encryption", "true");
command.addDetail("encryption_passphrase", passphrase);
}
Integer bandwidthLimit = NASBackupBandwidthLimitMbps.valueIn(zoneId);
if (bandwidthLimit != null && bandwidthLimit > 0) {
command.addDetail("bandwidth_limit", String.valueOf(bandwidthLimit));
}
if (Boolean.TRUE.equals(NASBackupIntegrityCheckEnabled.valueIn(zoneId))) {
command.addDetail("integrity_check", "true");
}
Comment on lines +248 to +267
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new zone-scoped feature flags are translated into TakeBackupCommand details here, but the existing NASBackupProviderTest.takeBackupSuccessfully doesn’t assert the details map contents. Add/extend unit tests to verify the correct details are added for each config (compression, bandwidth limit, integrity check, and encryption+passphrase; and that encryption without passphrase fails).

Copilot uses AI. Check for mistakes.

if (VirtualMachine.State.Stopped.equals(vm.getState())) {
List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
vmVolumes.sort(Comparator.comparing(Volume::getDeviceId));
Expand Down Expand Up @@ -594,7 +655,12 @@ public Boolean crossZoneInstanceCreationEnabled(BackupOffering backupOffering) {
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey[]{
NASBackupRestoreMountTimeout
NASBackupRestoreMountTimeout,
NASBackupCompressionEnabled,
NASBackupEncryptionEnabled,
NASBackupEncryptionPassphrase,
NASBackupBandwidthLimitMbps,
NASBackupIntegrityCheckEnabled
};
}

Expand Down
204 changes: 204 additions & 0 deletions plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,18 @@
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.mock;

import java.lang.reflect.Field;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

import org.apache.cloudstack.framework.config.ConfigKey;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
Expand All @@ -47,6 +51,7 @@
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.dao.VMInstanceDao;

Expand Down Expand Up @@ -349,4 +354,203 @@ public void testGetVMHypervisorHostFallbackToZoneWideKVMHost() {
Mockito.verify(hostDao).findHypervisorHostInCluster(clusterId);
Mockito.verify(resourceManager).findOneRandomRunningHostByHypervisor(Hypervisor.HypervisorType.KVM, zoneId);
}

private void overrideConfigValue(final ConfigKey configKey, final Object value) {
try {
Field f = ConfigKey.class.getDeclaredField("_value");
f.setAccessible(true);
f.set(configKey, value);
} catch (IllegalAccessException | NoSuchFieldException e) {
Assert.fail(e.getMessage());
}
}

private VMInstanceVO setupVmForTakeBackup(Long vmId, Long hostId, Long backupOfferingId,
Long accountId, Long domainId, Long zoneId) {
VMInstanceVO vm = mock(VMInstanceVO.class);
Mockito.when(vm.getId()).thenReturn(vmId);
Mockito.when(vm.getHostId()).thenReturn(hostId);
Mockito.when(vm.getInstanceName()).thenReturn("test-vm");
Mockito.when(vm.getBackupOfferingId()).thenReturn(backupOfferingId);
Mockito.when(vm.getAccountId()).thenReturn(accountId);
Mockito.when(vm.getDomainId()).thenReturn(domainId);
Mockito.when(vm.getDataCenterId()).thenReturn(zoneId);
Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running);
return vm;
}

private void setupHostAndRepo(Long hostId, Long backupOfferingId) {
BackupRepository backupRepository = mock(BackupRepository.class);
Mockito.when(backupRepository.getType()).thenReturn("nfs");
Mockito.when(backupRepository.getAddress()).thenReturn("address");
Mockito.when(backupRepository.getMountOptions()).thenReturn("sync");
Mockito.when(backupRepositoryDao.findByBackupOfferingId(backupOfferingId)).thenReturn(backupRepository);

HostVO host = mock(HostVO.class);
Mockito.when(host.getId()).thenReturn(hostId);
Mockito.when(host.getStatus()).thenReturn(Status.Up);
Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
Mockito.when(hostDao.findById(hostId)).thenReturn(host);
}

@Test
public void testTakeBackupDetailsCompressionEnabled() throws AgentUnavailableException, OperationTimedoutException {
Long vmId = 1L; Long hostId = 2L; Long backupOfferingId = 3L;
Long accountId = 4L; Long domainId = 5L; Long zoneId = 6L;

VMInstanceVO vm = setupVmForTakeBackup(vmId, hostId, backupOfferingId, accountId, domainId, zoneId);
setupHostAndRepo(hostId, backupOfferingId);

VolumeVO volume = mock(VolumeVO.class);
Mockito.when(volume.getState()).thenReturn(Volume.State.Ready);
Mockito.when(volume.getSize()).thenReturn(100L);
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(volume));

overrideConfigValue(nasBackupProvider.NASBackupCompressionEnabled, "true");

BackupAnswer answer = mock(BackupAnswer.class);
Mockito.when(answer.getResult()).thenReturn(true);
Mockito.when(answer.getSize()).thenReturn(100L);

ArgumentCaptor<TakeBackupCommand> cmdCaptor = ArgumentCaptor.forClass(TakeBackupCommand.class);
Mockito.when(agentManager.send(anyLong(), cmdCaptor.capture())).thenReturn(answer);
Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0));
Mockito.when(backupDao.update(Mockito.anyLong(), Mockito.any(BackupVO.class))).thenReturn(true);

nasBackupProvider.takeBackup(vm, false);

TakeBackupCommand capturedCmd = cmdCaptor.getValue();
Map<String, String> details = capturedCmd.getDetails();
Assert.assertEquals("true", details.get("compression"));

// Reset config
overrideConfigValue(nasBackupProvider.NASBackupCompressionEnabled, "false");
}

@Test
public void testTakeBackupDetailsBandwidthLimit() throws AgentUnavailableException, OperationTimedoutException {
Long vmId = 1L; Long hostId = 2L; Long backupOfferingId = 3L;
Long accountId = 4L; Long domainId = 5L; Long zoneId = 6L;

VMInstanceVO vm = setupVmForTakeBackup(vmId, hostId, backupOfferingId, accountId, domainId, zoneId);
setupHostAndRepo(hostId, backupOfferingId);

VolumeVO volume = mock(VolumeVO.class);
Mockito.when(volume.getState()).thenReturn(Volume.State.Ready);
Mockito.when(volume.getSize()).thenReturn(100L);
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(volume));

overrideConfigValue(nasBackupProvider.NASBackupBandwidthLimitMbps, "50");

BackupAnswer answer = mock(BackupAnswer.class);
Mockito.when(answer.getResult()).thenReturn(true);
Mockito.when(answer.getSize()).thenReturn(100L);

ArgumentCaptor<TakeBackupCommand> cmdCaptor = ArgumentCaptor.forClass(TakeBackupCommand.class);
Mockito.when(agentManager.send(anyLong(), cmdCaptor.capture())).thenReturn(answer);
Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0));
Mockito.when(backupDao.update(Mockito.anyLong(), Mockito.any(BackupVO.class))).thenReturn(true);

nasBackupProvider.takeBackup(vm, false);

TakeBackupCommand capturedCmd = cmdCaptor.getValue();
Map<String, String> details = capturedCmd.getDetails();
Assert.assertEquals("50", details.get("bandwidth_limit"));

overrideConfigValue(nasBackupProvider.NASBackupBandwidthLimitMbps, "0");
}

@Test
public void testTakeBackupDetailsIntegrityCheck() throws AgentUnavailableException, OperationTimedoutException {
Long vmId = 1L; Long hostId = 2L; Long backupOfferingId = 3L;
Long accountId = 4L; Long domainId = 5L; Long zoneId = 6L;

VMInstanceVO vm = setupVmForTakeBackup(vmId, hostId, backupOfferingId, accountId, domainId, zoneId);
setupHostAndRepo(hostId, backupOfferingId);

VolumeVO volume = mock(VolumeVO.class);
Mockito.when(volume.getState()).thenReturn(Volume.State.Ready);
Mockito.when(volume.getSize()).thenReturn(100L);
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(volume));

overrideConfigValue(nasBackupProvider.NASBackupIntegrityCheckEnabled, "true");

BackupAnswer answer = mock(BackupAnswer.class);
Mockito.when(answer.getResult()).thenReturn(true);
Mockito.when(answer.getSize()).thenReturn(100L);

ArgumentCaptor<TakeBackupCommand> cmdCaptor = ArgumentCaptor.forClass(TakeBackupCommand.class);
Mockito.when(agentManager.send(anyLong(), cmdCaptor.capture())).thenReturn(answer);
Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0));
Mockito.when(backupDao.update(Mockito.anyLong(), Mockito.any(BackupVO.class))).thenReturn(true);

nasBackupProvider.takeBackup(vm, false);

TakeBackupCommand capturedCmd = cmdCaptor.getValue();
Map<String, String> details = capturedCmd.getDetails();
Assert.assertEquals("true", details.get("integrity_check"));

overrideConfigValue(nasBackupProvider.NASBackupIntegrityCheckEnabled, "false");
}

@Test
public void testTakeBackupDetailsEncryptionWithPassphrase() throws AgentUnavailableException, OperationTimedoutException {
Long vmId = 1L; Long hostId = 2L; Long backupOfferingId = 3L;
Long accountId = 4L; Long domainId = 5L; Long zoneId = 6L;

VMInstanceVO vm = setupVmForTakeBackup(vmId, hostId, backupOfferingId, accountId, domainId, zoneId);
setupHostAndRepo(hostId, backupOfferingId);

VolumeVO volume = mock(VolumeVO.class);
Mockito.when(volume.getState()).thenReturn(Volume.State.Ready);
Mockito.when(volume.getSize()).thenReturn(100L);
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(volume));

overrideConfigValue(nasBackupProvider.NASBackupEncryptionEnabled, "true");
overrideConfigValue(nasBackupProvider.NASBackupEncryptionPassphrase, "my-secret-passphrase");

BackupAnswer answer = mock(BackupAnswer.class);
Mockito.when(answer.getResult()).thenReturn(true);
Mockito.when(answer.getSize()).thenReturn(100L);

ArgumentCaptor<TakeBackupCommand> cmdCaptor = ArgumentCaptor.forClass(TakeBackupCommand.class);
Mockito.when(agentManager.send(anyLong(), cmdCaptor.capture())).thenReturn(answer);
Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0));
Mockito.when(backupDao.update(Mockito.anyLong(), Mockito.any(BackupVO.class))).thenReturn(true);

nasBackupProvider.takeBackup(vm, false);

TakeBackupCommand capturedCmd = cmdCaptor.getValue();
Map<String, String> details = capturedCmd.getDetails();
Assert.assertEquals("true", details.get("encryption"));
Assert.assertEquals("my-secret-passphrase", details.get("encryption_passphrase"));

overrideConfigValue(nasBackupProvider.NASBackupEncryptionEnabled, "false");
overrideConfigValue(nasBackupProvider.NASBackupEncryptionPassphrase, "");
}

@Test(expected = CloudRuntimeException.class)
public void testTakeBackupEncryptionWithoutPassphraseThrows() throws AgentUnavailableException, OperationTimedoutException {
Long vmId = 1L; Long hostId = 2L; Long backupOfferingId = 3L;
Long accountId = 4L; Long domainId = 5L; Long zoneId = 6L;

VMInstanceVO vm = setupVmForTakeBackup(vmId, hostId, backupOfferingId, accountId, domainId, zoneId);
setupHostAndRepo(hostId, backupOfferingId);

VolumeVO volume = mock(VolumeVO.class);
Mockito.when(volume.getState()).thenReturn(Volume.State.Ready);
Mockito.when(volume.getSize()).thenReturn(100L);
Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(volume));

overrideConfigValue(nasBackupProvider.NASBackupEncryptionEnabled, "true");
overrideConfigValue(nasBackupProvider.NASBackupEncryptionPassphrase, "");

Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0));

try {
nasBackupProvider.takeBackup(vm, false);
} finally {
overrideConfigValue(nasBackupProvider.NASBackupEncryptionEnabled, "false");
}
}
}
Loading
Loading