Skip to content

Commit 7aa1062

Browse files
author
Marcus Sorensen
committed
Address static code analysis issues for volume encryption
Signed-off-by: Marcus Sorensen <mls@apple.com>
1 parent 013e3e9 commit 7aa1062

28 files changed

Lines changed: 152 additions & 177 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public class DiskOfferingResponse extends BaseResponseWithAnnotations {
162162
private Boolean diskSizeStrictness;
163163

164164
@SerializedName(ApiConstants.ENCRYPT)
165-
@Param(description = "Whether disks using this offering will be encrypted on primary storage")
165+
@Param(description = "Whether disks using this offering will be encrypted on primary storage", since = "4.18")
166166
private Boolean encrypt;
167167

168168
@SerializedName(ApiConstants.DETAILS)

‎api/src/main/java/org/apache/cloudstack/api/response/HostResponse.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public class HostResponse extends BaseResponseWithAnnotations {
271271
private Boolean uefiCapabilty;
272272

273273
@SerializedName(ApiConstants.ENCRYPTION_SUPPORTED)
274-
@Param(description = "true if the host supports encryption")
274+
@Param(description = "true if the host supports encryption", since = "4.18")
275275
private Boolean encryptionSupported;
276276

277277
@Override

‎api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ public class ServiceOfferingResponse extends BaseResponseWithAnnotations {
227227
private String diskOfferingDisplayText;
228228

229229
@SerializedName(ApiConstants.ENCRYPT_ROOT)
230-
@Param(description = "true if virtual machine root disk will be encrypted on storage", since = "4.16")
230+
@Param(description = "true if virtual machine root disk will be encrypted on storage", since = "4.18")
231231
private Boolean encryptRoot;
232232

233233
public ServiceOfferingResponse() {

‎core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,4 +380,8 @@ public void clearPassphrase() {
380380
Arrays.fill(this.passphrase, (byte) 0);
381381
}
382382
}
383+
384+
public boolean requiresEncryption() {
385+
return passphrase != null && passphrase.length > 0;
386+
}
383387
}

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ public enum UserVmCloneType {
235235
@Inject
236236
VolumeApiService _volumeApiService;
237237
@Inject
238-
PassphraseDao _passphraseDao;
238+
PassphraseDao passphraseDao;
239239

240240
@Inject
241241
protected SnapshotHelper snapshotHelper;
@@ -310,7 +310,7 @@ public VolumeVO allocateDuplicateVolumeVO(Volume oldVol, Long templateId) {
310310
newVol.setFormat(oldVol.getFormat());
311311

312312
if (oldVol.getPassphraseId() != null) {
313-
PassphraseVO passphrase =_passphraseDao.persist(new PassphraseVO());
313+
PassphraseVO passphrase = passphraseDao.persist(new PassphraseVO());
314314
passphrase.clearPassphrase();
315315
newVol.setPassphraseId(passphrase.getId());
316316
}
@@ -1655,7 +1655,7 @@ private VolumeVO setPassphraseForVolumeEncryption(VolumeVO volume) {
16551655
}
16561656
s_logger.debug("Creating passphrase for the volume: " + volume.getName());
16571657
long startTime = System.currentTimeMillis();
1658-
PassphraseVO passphrase = _passphraseDao.persist(new PassphraseVO());
1658+
PassphraseVO passphrase = passphraseDao.persist(new PassphraseVO());
16591659
passphrase.clearPassphrase();
16601660
volume.setPassphraseId(passphrase.getId());
16611661
long finishTime = System.currentTimeMillis();

‎engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
import javax.inject.Inject;
2727

28-
import org.apache.cloudstack.secret.dao.PassphraseDao;
2928
import org.apache.log4j.Logger;
3029
import org.springframework.stereotype.Component;
3130

@@ -69,8 +68,6 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol
6968
protected GenericSearchBuilder<VolumeVO, SumCount> secondaryStorageSearch;
7069
@Inject
7170
ResourceTagDao _tagsDao;
72-
@Inject
73-
PassphraseDao _passphraseDao;
7471

7572
protected static final String SELECT_VM_SQL = "SELECT DISTINCT instance_id from volumes v where v.host_id = ? and v.mirror_state = ?";
7673
// need to account for zone-wide primary storage where storage_pool has

‎engine/schema/src/main/resources/META-INF/db/schema-41600to41610-cleanup.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@
1717

1818
--;
1919
-- Schema upgrade cleanup from 4.16.0.0 to 4.16.1.0
20-
--;
20+
--;

‎engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@
8181
@Component
8282
public class AncientDataMotionStrategy implements DataMotionStrategy {
8383
private static final Logger s_logger = Logger.getLogger(AncientDataMotionStrategy.class);
84+
private static final String NO_REMOTE_ENDPOINT_SSVM = "No remote endpoint to send command, check if host or ssvm is down?";
85+
private static final String NO_REMOTE_ENDPOINT_WITH_ENCRYPTION = "No remote endpoint to send command, unable to find a valid endpoint. Requires encryption support: %s";
86+
8487
@Inject
8588
EndPointSelector selector;
8689
@Inject
@@ -171,9 +174,8 @@ protected Answer copyObject(DataObject srcData, DataObject destData, Host destHo
171174
VirtualMachineManager.ExecuteInSequence.value());
172175
EndPoint ep = destHost != null ? RemoteHostEndPoint.getHypervisorHostEndPoint(destHost) : selector.select(srcForCopy, destData);
173176
if (ep == null) {
174-
String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
175-
s_logger.error(errMsg);
176-
answer = new Answer(cmd, false, errMsg);
177+
s_logger.error(NO_REMOTE_ENDPOINT_SSVM);
178+
answer = new Answer(cmd, false, NO_REMOTE_ENDPOINT_SSVM);
177179
} else {
178180
answer = ep.sendMessage(cmd);
179181
}
@@ -295,9 +297,8 @@ protected Answer copyVolumeFromSnapshot(DataObject snapObj, DataObject volObj) {
295297

296298
Answer answer = null;
297299
if (ep == null) {
298-
String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
299-
s_logger.error(errMsg);
300-
answer = new Answer(cmd, false, errMsg);
300+
s_logger.error(NO_REMOTE_ENDPOINT_SSVM);
301+
answer = new Answer(cmd, false, NO_REMOTE_ENDPOINT_SSVM);
301302
} else {
302303
answer = ep.sendMessage(cmd);
303304
}
@@ -320,9 +321,8 @@ protected Answer cloneVolume(DataObject template, DataObject volume) {
320321
EndPoint ep = selector.select(volume, anyVolumeRequiresEncryption(volume));
321322
Answer answer = null;
322323
if (ep == null) {
323-
String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
324-
s_logger.error(errMsg);
325-
answer = new Answer(cmd, false, errMsg);
324+
s_logger.error(NO_REMOTE_ENDPOINT_SSVM);
325+
answer = new Answer(cmd, false, NO_REMOTE_ENDPOINT_SSVM);
326326
} else {
327327
answer = ep.sendMessage(cmd);
328328
}
@@ -360,7 +360,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData)
360360
EndPoint ep = selector.select(srcData, destData, encryptionRequired);
361361
Answer answer = null;
362362
if (ep == null) {
363-
String errMsg = String.format("No remote endpoint to send command, unable to find a valid endpoint. Requires encryption support: %s", encryptionRequired);
363+
String errMsg = String.format(NO_REMOTE_ENDPOINT_WITH_ENCRYPTION, encryptionRequired);
364364
s_logger.error(errMsg);
365365
answer = new Answer(cmd, false, errMsg);
366366
} else {
@@ -399,7 +399,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData)
399399
CopyCommand cmd = new CopyCommand(objOnImageStore.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(destData.getTO()), _copyvolumewait, VirtualMachineManager.ExecuteInSequence.value());
400400
EndPoint ep = selector.select(objOnImageStore, destData, encryptionRequired);
401401
if (ep == null) {
402-
String errMsg = String.format("No remote endpoint to send command, unable to find a valid endpoint. Requires encryption support: %s", encryptionRequired);
402+
String errMsg = String.format(NO_REMOTE_ENDPOINT_WITH_ENCRYPTION, encryptionRequired);
403403
s_logger.error(errMsg);
404404
answer = new Answer(cmd, false, errMsg);
405405
} else {
@@ -432,7 +432,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData)
432432
EndPoint ep = selector.select(cacheData, destData, encryptionRequired);
433433
Answer answer = null;
434434
if (ep == null) {
435-
String errMsg = String.format("No remote endpoint to send command, unable to find a valid endpoint. Requires encryption support: %s", encryptionRequired);
435+
String errMsg = String.format(NO_REMOTE_ENDPOINT_WITH_ENCRYPTION, encryptionRequired);
436436
s_logger.error(errMsg);
437437
answer = new Answer(cmd, false, errMsg);
438438
} else {
@@ -464,7 +464,7 @@ protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) {
464464
EndPoint ep = selector.select(srcData, StorageAction.MIGRATEVOLUME);
465465
Answer answer = null;
466466
if (ep == null) {
467-
String errMsg = String.format("No remote endpoint to send command, unable to find a valid endpoint. Requires encryption support: %s", encryptionRequired);
467+
String errMsg = String.format(NO_REMOTE_ENDPOINT_WITH_ENCRYPTION, encryptionRequired);
468468
s_logger.error(errMsg);
469469
answer = new Answer(command, false, errMsg);
470470
} else {
@@ -560,9 +560,8 @@ protected Answer createTemplateFromSnapshot(DataObject srcData, DataObject destD
560560
CopyCommand cmd = new CopyCommand(srcData.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(destData.getTO()), _createprivatetemplatefromsnapshotwait, VirtualMachineManager.ExecuteInSequence.value());
561561
Answer answer = null;
562562
if (ep == null) {
563-
String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
564-
s_logger.error(errMsg);
565-
answer = new Answer(cmd, false, errMsg);
563+
s_logger.error(NO_REMOTE_ENDPOINT_SSVM);
564+
answer = new Answer(cmd, false, NO_REMOTE_ENDPOINT_SSVM);
566565
} else {
567566
answer = ep.sendMessage(cmd);
568567
}
@@ -601,9 +600,8 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) {
601600
cmd.setOptions(options);
602601
EndPoint ep = selector.select(srcData, destData, encryptionRequired);
603602
if (ep == null) {
604-
String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
605-
s_logger.error(errMsg);
606-
answer = new Answer(cmd, false, errMsg);
603+
s_logger.error(NO_REMOTE_ENDPOINT_SSVM);
604+
answer = new Answer(cmd, false, NO_REMOTE_ENDPOINT_SSVM);
607605
} else {
608606
answer = ep.sendMessage(cmd);
609607
}
@@ -613,9 +611,8 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) {
613611
cmd.setOptions(options);
614612
EndPoint ep = selector.select(srcData, destData, StorageAction.BACKUPSNAPSHOT, encryptionRequired);
615613
if (ep == null) {
616-
String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";
617-
s_logger.error(errMsg);
618-
answer = new Answer(cmd, false, errMsg);
614+
s_logger.error(NO_REMOTE_ENDPOINT_SSVM);
615+
answer = new Answer(cmd, false, NO_REMOTE_ENDPOINT_SSVM);
619616
} else {
620617
answer = ep.sendMessage(cmd);
621618
}
@@ -648,6 +645,7 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeMap, VirtualMachineTO vmT
648645
*/
649646
private boolean anyVolumeRequiresEncryption(DataObject ... objects) {
650647
for (DataObject o : objects) {
648+
// this fails code smell for returning true twice, but it is more readable than combining all tests into one statement
651649
if (o instanceof VolumeInfo && ((VolumeInfo) o).getPassphraseId() != null) {
652650
return true;
653651
} else if (o instanceof SnapshotInfo && ((SnapshotInfo) o).getBaseVolume().getPassphraseId() != null) {

‎engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import org.apache.cloudstack.framework.async.AsyncCallFuture;
6060
import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
6161
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
62-
import org.apache.cloudstack.secret.dao.PassphraseDao;
6362
import org.apache.cloudstack.storage.command.CopyCmdAnswer;
6463
import org.apache.cloudstack.storage.command.CopyCommand;
6564
import org.apache.cloudstack.storage.command.ResignatureAnswer;
@@ -162,8 +161,6 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
162161
@Inject
163162
private HostDao _hostDao;
164163
@Inject
165-
private PassphraseDao _passphraseDao;
166-
@Inject
167164
protected PrimaryDataStoreDao _storagePoolDao;
168165
@Inject
169166
private SnapshotDao _snapshotDao;
@@ -2227,7 +2224,6 @@ protected void prepareDiskWithSecretConsumerDetail(VirtualMachineTO vmTO, Volume
22272224
if (vmTO.getDisks() != null) {
22282225
LOGGER.debug(String.format("Preparing VM TO '%s' disks with migration data", vmTO));
22292226
Arrays.stream(vmTO.getDisks()).filter(diskTO -> diskTO.getData().getId() == srcVolume.getId()).forEach( diskTO -> {
2230-
Map<String, String> details = diskTO.getDetails();
22312227
if (diskTO.getDetails() == null) {
22322228
diskTO.setDetails(new HashMap<>());
22332229
}

‎engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ public class DefaultEndPointSelector implements EndPointSelector {
7575
@Inject
7676
private DedicatedResourceDao dedicatedResourceDao;
7777

78-
private static final String volEncryptColumnName = "volume_encryption_support";
78+
private static final String VOL_ENCRYPT_COLUMN_NAME = "volume_encryption_support";
7979
private final String findOneHostOnPrimaryStorage = "select t.id from "
80-
+ "(select h.id, cd.value, hd.value as " + volEncryptColumnName + " "
80+
+ "(select h.id, cd.value, hd.value as " + VOL_ENCRYPT_COLUMN_NAME + " "
8181
+ "from host h join storage_pool_host_ref s on h.id = s.host_id "
8282
+ "join cluster c on c.id=h.cluster_id "
8383
+ "left join cluster_details cd on c.id=cd.cluster_id and cd.name='" + CapacityManager.StorageOperationsExcludeCluster.key() + "' "
@@ -154,7 +154,7 @@ protected EndPoint findEndPointInScope(Scope scope, String sqlBase, Long poolId,
154154
sbuilder.append(") t where t.value<>'true' or t.value is null"); //Added for exclude cluster's subquery
155155

156156
if (volumeEncryptionSupportRequired) {
157-
sbuilder.append(String.format(" and t.%s='true'", volEncryptColumnName));
157+
sbuilder.append(String.format(" and t.%s='true'", VOL_ENCRYPT_COLUMN_NAME));
158158
}
159159

160160
// TODO: order by rand() is slow if there are lot of hosts

0 commit comments

Comments
 (0)