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 9 commits into
base: 4.22
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 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
4884980
Fix tee masking exit codes and missing return in nasbackup.sh
jmsperu Apr 1, 2026
3d65f34
Fix NASBackupProviderTest failures for zone-scoped ConfigKey values
jmsperu Apr 2, 2026
c11e0b8
Fix build: use reflection for protected ConfigKey.valueOf() in test
jmsperu Apr 8, 2026
45ce015
Merge upstream/4.22 + address Copilot feedback on #12898
jmsperu May 14, 2026
2150959
refactor(backup): consolidate detail keys in TakeBackupCommand
jmsperu May 22, 2026
8a84d8e
ci: retrigger workflow (flaky/stale shards)
jmsperu May 26, 2026
0effda2
refactor(backup): extract NAS backup enhancement logic into dedicated…
jmsperu Jun 20, 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
25 changes: 25 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,9 +23,20 @@
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 {
// Detail-map keys shared between the management server (NASBackupProvider) and
// the KVM agent wrapper. Defining them once here avoids drift between producer
// and consumer when a key is renamed.
public static final String DETAIL_COMPRESSION = "compression";
public static final String DETAIL_ENCRYPTION = "encryption";
public static final String DETAIL_ENCRYPTION_PASSPHRASE = "encryption_passphrase";
public static final String DETAIL_BANDWIDTH_LIMIT = "bandwidth_limit";
public static final String DETAIL_INTEGRITY_CHECK = "integrity_check";

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

Copilot AI Mar 30, 2026

Copy link

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 +119,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
76 changes: 75 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 @@ -85,6 +85,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 @@ -206,6 +246,9 @@ 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
applyBackupEnhancementDetails(command, vm.getDataCenterId());

if (VirtualMachine.State.Stopped.equals(vm.getState())) {
List<VolumeVO> vmVolumes = volumeDao.findByInstance(vm.getId());
vmVolumes.sort(Comparator.comparing(Volume::getDeviceId));
Expand Down Expand Up @@ -254,6 +297,32 @@ public Pair<Boolean, Backup> takeBackup(final VirtualMachine vm, Boolean quiesce
}
}

/**
* Translates the zone-scoped backup-enhancement settings (compression, encryption,
* bandwidth limit, integrity check) into details on the {@link TakeBackupCommand}.
* Fails fast if encryption is enabled without a configured passphrase.
*/
protected void applyBackupEnhancementDetails(TakeBackupCommand command, Long zoneId) {
if (Boolean.TRUE.equals(NASBackupCompressionEnabled.valueIn(zoneId))) {
command.addDetail(TakeBackupCommand.DETAIL_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(TakeBackupCommand.DETAIL_ENCRYPTION, "true");
command.addDetail(TakeBackupCommand.DETAIL_ENCRYPTION_PASSPHRASE, passphrase);
}
Integer bandwidthLimit = NASBackupBandwidthLimitMbps.valueIn(zoneId);
if (bandwidthLimit != null && bandwidthLimit > 0) {
command.addDetail(TakeBackupCommand.DETAIL_BANDWIDTH_LIMIT, String.valueOf(bandwidthLimit));
}
if (Boolean.TRUE.equals(NASBackupIntegrityCheckEnabled.valueIn(zoneId))) {
command.addDetail(TakeBackupCommand.DETAIL_INTEGRITY_CHECK, "true");
}
}

private BackupVO createBackupObject(VirtualMachine vm, String backupPath) {
BackupVO backup = new BackupVO();
backup.setVmId(vm.getId());
Expand Down Expand Up @@ -618,7 +687,12 @@ public Boolean crossZoneInstanceCreationEnabled(BackupOffering backupOffering) {
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey[]{
NASBackupRestoreMountTimeout
NASBackupRestoreMountTimeout,
NASBackupCompressionEnabled,
NASBackupEncryptionEnabled,
NASBackupEncryptionPassphrase,
NASBackupBandwidthLimitMbps,
NASBackupIntegrityCheckEnabled
};
}

Expand Down
213 changes: 213 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,212 @@ 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 {
// Use reflection to invoke protected valueOf()
java.lang.reflect.Method valueOfMethod = ConfigKey.class.getDeclaredMethod("valueOf", String.class);
valueOfMethod.setAccessible(true);
Object typedValue = value != null ? valueOfMethod.invoke(configKey, String.valueOf(value)) : null;

// Set _value for value() calls
Field f = ConfigKey.class.getDeclaredField("_value");
f.setAccessible(true);
f.set(configKey, typedValue);

// Also set _defaultValue via Spring's ReflectionTestUtils (handles final fields)
ReflectionTestUtils.setField(configKey, "_defaultValue", String.valueOf(value));
} catch (Exception 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(TakeBackupCommand.DETAIL_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(TakeBackupCommand.DETAIL_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(TakeBackupCommand.DETAIL_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(TakeBackupCommand.DETAIL_ENCRYPTION));
Assert.assertEquals("my-secret-passphrase", details.get(TakeBackupCommand.DETAIL_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