Skip to content

Commit b58c46c

Browse files
vishesh92vishesh92
committed
Address comments
1 parent 520da69 commit b58c46c

5 files changed

Lines changed: 88 additions & 56 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/command/user/kms/hsm/CreateHSMProfileCmd.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -126,17 +126,7 @@ public String getVendorName() {
126126
}
127127

128128
public Map<String, String> getDetails() {
129-
Map<String, String> detailsMap = new HashMap<>();
130-
if (MapUtils.isNotEmpty(details)) {
131-
Collection<?> props = details.values();
132-
for (Object prop : props) {
133-
HashMap<String, String> detail = (HashMap<String, String>) prop;
134-
for (Map.Entry<String, String> entry : detail.entrySet()) {
135-
detailsMap.put(entry.getKey(), entry.getValue());
136-
}
137-
}
138-
}
139-
return detailsMap;
129+
return convertDetailsToMap(details);
140130
}
141131

142132
@Override

‎api/src/main/java/org/apache/cloudstack/kms/KMSManager.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,27 @@ public interface KMSManager extends Manager, Configurable {
103103
ConfigKey.Scope.Global
104104
);
105105

106+
ConfigKey<Integer> KMSOperationPoolCoreSize = new ConfigKey<>(
107+
"Advanced",
108+
Integer.class,
109+
"kms.operation.pool.core.size",
110+
"2",
111+
"Minimum number of threads kept alive for KMS cryptographic operations",
112+
true,
113+
ConfigKey.Scope.Global
114+
);
115+
116+
ConfigKey<Integer> KMSOperationPoolMaxSize = new ConfigKey<>(
117+
"Advanced",
118+
Integer.class,
119+
"kms.operation.pool.max.size",
120+
"100",
121+
"Maximum number of concurrent threads for KMS cryptographic operations. " +
122+
"Set this to match the concurrency limit of your HSM appliance or external KMS provider.",
123+
true,
124+
ConfigKey.Scope.Global
125+
);
126+
106127
/**
107128
* List all registered KMS providers
108129
*

‎engine/schema/src/main/java/org/apache/cloudstack/kms/KMSKekVersionVO.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,22 +62,30 @@ public enum Status {
6262
@GeneratedValue(strategy = GenerationType.IDENTITY)
6363
@Column(name = "id")
6464
private Long id;
65+
6566
@Column(name = "uuid", nullable = false)
6667
private String uuid;
68+
6769
@Column(name = "kms_key_id", nullable = false)
6870
private Long kmsKeyId;
71+
6972
@Column(name = "version_number", nullable = false)
7073
private Integer versionNumber;
74+
7175
@Column(name = "kek_label", nullable = false)
7276
private String kekLabel;
77+
7378
@Column(name = "status", nullable = false, length = 32)
7479
@Enumerated(EnumType.STRING)
7580
private Status status;
81+
7682
@Column(name = "hsm_profile_id")
7783
private Long hsmProfileId;
84+
7885
@Column(name = GenericDao.CREATED_COLUMN, nullable = false)
7986
@Temporal(TemporalType.TIMESTAMP)
8087
private Date created;
88+
8189
@Column(name = GenericDao.REMOVED_COLUMN)
8290
@Temporal(TemporalType.TIMESTAMP)
8391
private Date removed;

‎engine/schema/src/main/java/org/apache/cloudstack/kms/KMSKeyVO.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,10 @@ public class KMSKeyVO implements KMSKey {
9292
@Temporal(TemporalType.TIMESTAMP)
9393
private Date removed;
9494

95-
public KMSKeyVO(String name, String description, String kekLabel, KeyPurpose purpose,
96-
Long accountId, Long domainId, Long zoneId,
97-
String algorithm, Integer keyBits) {
95+
public KMSKeyVO(String name, String description, String kekLabel,
96+
KeyPurpose purpose, Long accountId, Long domainId,
97+
Long zoneId, String algorithm, Integer keyBits
98+
) {
9899
this();
99100
this.name = name;
100101
this.description = description;
@@ -255,8 +256,9 @@ public Class<?> getEntityType() {
255256

256257
@Override
257258
public String toString() {
258-
return String.format("KMSKey %s",
259-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "uuid", "name", "purpose",
260-
"accountId", "zoneId", "enabled"));
259+
return String.format("KMSKey %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
260+
this, "id", "uuid", "name", "purpose",
261+
"accountId", "zoneId", "enabled"
262+
));
261263
}
262264
}

‎server/src/main/java/org/apache/cloudstack/kms/KMSManagerImpl.java‎

Lines changed: 50 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import com.cloud.utils.db.Transaction;
4747
import com.cloud.utils.db.TransactionCallbackWithException;
4848
import com.cloud.utils.exception.CloudRuntimeException;
49+
import org.apache.cloudstack.api.ApiCommandResourceType;
4950
import org.apache.cloudstack.api.command.admin.kms.MigrateVolumesToKMSCmd;
5051
import org.apache.cloudstack.api.command.user.kms.CreateKMSKeyCmd;
5152
import org.apache.cloudstack.api.command.user.kms.DeleteKMSKeyCmd;
@@ -56,7 +57,6 @@
5657
import org.apache.cloudstack.api.command.user.kms.hsm.DeleteHSMProfileCmd;
5758
import org.apache.cloudstack.api.command.user.kms.hsm.ListHSMProfilesCmd;
5859
import org.apache.cloudstack.api.command.user.kms.hsm.UpdateHSMProfileCmd;
59-
import org.apache.cloudstack.api.ApiCommandResourceType;
6060
import org.apache.cloudstack.api.response.HSMProfileResponse;
6161
import org.apache.cloudstack.api.response.KMSKeyResponse;
6262
import org.apache.cloudstack.api.response.ListResponse;
@@ -99,28 +99,32 @@
9999
public class KMSManagerImpl extends ManagerBase implements KMSManager, PluggableService {
100100
private static final Logger logger = LogManager.getLogger(KMSManagerImpl.class);
101101
private static final Map<String, KMSProvider> kmsProviderMap = new HashMap<>();
102-
private final ExecutorService kmsOperationExecutor = new ThreadPoolExecutor(
103-
2, 100, 60L, TimeUnit.SECONDS, new SynchronousQueue<>(), r -> {
104-
Thread t = new Thread(r, "kms-operation");
105-
t.setDaemon(true);
106-
return t;
107-
});
102+
private ExecutorService kmsOperationExecutor;
103+
108104
@Inject
109105
private KMSWrappedKeyDao kmsWrappedKeyDao;
106+
110107
@Inject
111108
private KMSKeyDao kmsKeyDao;
109+
112110
@Inject
113111
private KMSKekVersionDao kmsKekVersionDao;
112+
114113
@Inject
115114
private HSMProfileDao hsmProfileDao;
115+
116116
@Inject
117117
private HSMProfileDetailsDao hsmProfileDetailsDao;
118+
118119
@Inject
119120
private AccountManager accountManager;
121+
120122
@Inject
121123
private DataCenterDao dataCenterDao;
124+
122125
@Inject
123126
private VolumeDao volumeDao;
127+
124128
@Inject
125129
private PassphraseDao passphraseDao;
126130
private List<KMSProvider> kmsProviders;
@@ -183,7 +187,7 @@ public void checkKmsKeyForVolumeEncryption(Account caller, Long kmsKeyId, Long z
183187
if (key.getZoneId() != null && zoneId != null && !key.getZoneId().equals(zoneId)) {
184188
throw new InvalidParameterValueException(
185189
"KMS key belongs to zone " + key.getZoneId() +
186-
" but the target resource is in zone " + zoneId);
190+
" but the target resource is in zone " + zoneId);
187191
}
188192
if (!key.isEnabled()) {
189193
throw new InvalidParameterValueException(
@@ -252,7 +256,7 @@ public byte[] unwrapKey(Long wrappedKeyId) throws KMSException {
252256
}
253257

254258
private byte[] getUnwrappedKey(KMSWrappedKeyVO wrappedVO, KMSKeyVO kmsKey,
255-
KMSKekVersionVO version) throws Exception {
259+
KMSKekVersionVO version) throws Exception {
256260
HSMProfileVO hsmProfile = hsmProfileDao.findById(version.getHsmProfileId());
257261
KMSProvider provider = getKMSProvider(hsmProfile.getProtocol());
258262

@@ -399,7 +403,7 @@ KMSKeyResponse createKMSKeyResponse(KMSKey kmsKey) {
399403

400404
Account caller = CallContext.current().getCallingAccount();
401405
if (caller != null && (caller.getType() == Account.Type.ADMIN
402-
|| caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN)) {
406+
|| caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN)) {
403407
response.setKekLabel(kmsKey.getKekLabel());
404408
}
405409

@@ -408,8 +412,8 @@ KMSKeyResponse createKMSKeyResponse(KMSKey kmsKey) {
408412
}
409413

410414
KMSKey createUserKMSKey(Long accountId, Long domainId, Long zoneId,
411-
String name, String description, KeyPurpose purpose,
412-
Integer keyBits, long hsmProfileId) throws KMSException {
415+
String name, String description, KeyPurpose purpose,
416+
Integer keyBits, long hsmProfileId) throws KMSException {
413417
HSMProfileVO profile = hsmProfileDao.findById(hsmProfileId);
414418
if (profile == null) {
415419
throw KMSException.invalidParameter("HSM Profile not found");
@@ -480,7 +484,7 @@ public ListResponse<KMSKeyResponse> listKMSKeys(ListKMSKeysCmd cmd) {
480484
}
481485

482486
SearchBuilder<KMSKeyVO> getSearchBuilderForKMSKeys(Long domainId, Boolean isRecursive, List<Long> permittedAccounts,
483-
ListProjectResourcesCriteria listProjectResourcesCriteria) {
487+
ListProjectResourcesCriteria listProjectResourcesCriteria) {
484488
SearchBuilder<KMSKeyVO> sb = kmsKeyDao.createSearchBuilder();
485489
accountManager.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts,
486490
listProjectResourcesCriteria);
@@ -495,8 +499,8 @@ SearchBuilder<KMSKeyVO> getSearchBuilderForKMSKeys(Long domainId, Boolean isRecu
495499
}
496500

497501
SearchCriteria<KMSKeyVO> getSearchCriteriaForKMSKeys(SearchBuilder<KMSKeyVO> searchBuilder, ListKMSKeysCmd cmd,
498-
Long domainId, Boolean isRecursive, List<Long> permittedAccounts,
499-
ListProjectResourcesCriteria listProjectResourcesCriteria) {
502+
Long domainId, Boolean isRecursive, List<Long> permittedAccounts,
503+
ListProjectResourcesCriteria listProjectResourcesCriteria) {
500504
SearchCriteria<KMSKeyVO> sc = searchBuilder.create();
501505
accountManager.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts,
502506
listProjectResourcesCriteria);
@@ -567,12 +571,12 @@ void deleteUserKMSKey(KMSKeyVO key) throws KMSException {
567571
long wrappedKeyCount = kmsWrappedKeyDao.countByKmsKeyId(key.getId());
568572
if (wrappedKeyCount > 0) {
569573
throw new InvalidParameterValueException("Cannot delete KMS key: " + key + ". " + wrappedKeyCount +
570-
" wrapped key(s) still reference this key");
574+
" wrapped key(s) still reference this key");
571575
}
572576

573577
if (volumeDao.existsWithKmsKey(key.getId())) {
574578
throw new InvalidParameterValueException("Cannot delete KMS key: " + key + ". " +
575-
"There are Volumes which still reference this key");
579+
"There are Volumes which still reference this key");
576580
}
577581

578582
List<KMSKekVersionVO> kekVersions = kmsKekVersionDao.listByKmsKeyId(key.getId());
@@ -637,15 +641,15 @@ public String rotateKMSKey(RotateKMSKeyCmd cmd) throws KMSException {
637641
CallContext.current().setEventDetails(String.format("Rotated KMS key: %s (uuid: %s) from version %d to %d", kmsKey.getName(), kmsKey.getUuid(), currentActive.getVersionNumber(), newVersion.getVersionNumber()));
638642

639643
logger.info("KMS key rotation initiated: {} -> new KEK version {} (UUID: {}). " +
640-
"Background job will gradually rewrap {} wrapped key(s)",
644+
"Background job will gradually rewrap {} wrapped key(s)",
641645
kmsKey, newVersion.getVersionNumber(), newVersion.getUuid(),
642646
kmsWrappedKeyDao.countByKmsKeyId(kmsKey.getId()));
643647

644648
return newVersion.getUuid();
645649
}
646650

647651
String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int keyBits,
648-
HSMProfileVO newHSMProfile) throws KMSException {
652+
HSMProfileVO newHSMProfile) throws KMSException {
649653
if (StringUtils.isEmpty(oldKekLabel)) {
650654
throw KMSException.invalidParameter("oldKekLabel must be specified");
651655
}
@@ -662,7 +666,7 @@ String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int ke
662666
if (StringUtils.isEmpty(newKekLabel)) {
663667
List<KMSKekVersionVO> existingVersions = kmsKekVersionDao.listByKmsKeyId(kmsKey.getId());
664668
int nextVersion = existingVersions.stream().mapToInt(KMSKekVersionVO::getVersionNumber).max().orElse(0)
665-
+ 1;
669+
+ 1;
666670
newKekLabel = kmsKey.getPurpose().generateKekLabel(kmsKey.getDomainId(), kmsKey.getAccountId(),
667671
kmsKey.getUuid(), nextVersion);
668672
}
@@ -699,7 +703,7 @@ String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int ke
699703
} catch (KMSException e) {
700704
logger.error(
701705
"Database update failed during KEK rotation for kmsKey {}. Attempting to delete orphaned KEK "
702-
+ "{} from provider {}",
706+
+ "{} from provider {}",
703707
kmsKey, newKekId, provider.getProviderName());
704708
try {
705709
provider.deleteKek(newKekId);
@@ -723,9 +727,9 @@ String rotateKek(KMSKeyVO kmsKey, String oldKekLabel, String newKekLabel, int ke
723727
private KMSKekVersionVO createKekVersion(KMSKekVersionVO newVersion) throws KMSException {
724728
List<KMSKekVersionVO> existingVersions = kmsKekVersionDao.listByKmsKeyId(newVersion.getKmsKeyId());
725729
int nextVersion = existingVersions.stream()
726-
.mapToInt(KMSKekVersionVO::getVersionNumber)
727-
.max()
728-
.orElse(0) + 1;
730+
.mapToInt(KMSKekVersionVO::getVersionNumber)
731+
.max()
732+
.orElse(0) + 1;
729733

730734
KMSKekVersionVO currentActive = kmsKekVersionDao.getActiveVersion(newVersion.getKmsKeyId());
731735
if (currentActive != null) {
@@ -853,12 +857,12 @@ public int migrateVolumesToKMS(MigrateVolumesToKMSCmd cmd) throws KMSException {
853857
}
854858

855859
private boolean migrateVolumeToKmsKey(KMSProvider provider, VolumeVO volume, KMSKey kmsKey,
856-
KMSKekVersionVO activeVersion) {
860+
KMSKekVersionVO activeVersion) {
857861
PassphraseVO passphrase = passphraseDao.findById(volume.getPassphraseId());
858862
if (passphrase == null) {
859863
logger.warn(
860864
"Skipping migration of volume from to the KMS key {} because passphrase id: {} not found for "
861-
+ "volume {}",
865+
+ "volume {}",
862866
kmsKey, volume.getPassphraseId(), volume);
863867
return false;
864868
}
@@ -1067,7 +1071,7 @@ public ListResponse<HSMProfileResponse> listHSMProfiles(ListHSMProfilesCmd cmd)
10671071
// If the profile is owned by the user, they should see full details even if it
10681072
// is a system profile
10691073
boolean limited = profile.getIsPublic() && !isRootAdmin && !(cmd.getIsSystem() || cmd.listAll())
1070-
&& profile.getAccountId() != caller.getId();
1074+
&& profile.getAccountId() != caller.getId();
10711075
responses.add(createHSMProfileResponse(profile, limited));
10721076
}
10731077

@@ -1077,7 +1081,7 @@ public ListResponse<HSMProfileResponse> listHSMProfiles(ListHSMProfilesCmd cmd)
10771081
}
10781082

10791083
SearchBuilder<HSMProfileVO> getSearchBuilderForHSMProfiles(Long domainId, Boolean isRecursive,
1080-
List<Long> permittedAccounts, ListProjectResourcesCriteria listProjectResourcesCriteria) {
1084+
List<Long> permittedAccounts, ListProjectResourcesCriteria listProjectResourcesCriteria) {
10811085
SearchBuilder<HSMProfileVO> sb = hsmProfileDao.createSearchBuilder();
10821086
accountManager.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts,
10831087
listProjectResourcesCriteria);
@@ -1092,8 +1096,8 @@ SearchBuilder<HSMProfileVO> getSearchBuilderForHSMProfiles(Long domainId, Boolea
10921096
}
10931097

10941098
SearchCriteria<HSMProfileVO> getSearchCriteriaForHSMProfiles(SearchBuilder<HSMProfileVO> searchBuilder,
1095-
ListHSMProfilesCmd cmd, Account caller, Long domainId, Boolean isRecursive, List<Long> permittedAccounts,
1096-
ListProjectResourcesCriteria listProjectResourcesCriteria) {
1099+
ListHSMProfilesCmd cmd, Account caller, Long domainId, Boolean isRecursive, List<Long> permittedAccounts,
1100+
ListProjectResourcesCriteria listProjectResourcesCriteria) {
10971101
SearchCriteria<HSMProfileVO> sc = searchBuilder.create();
10981102

10991103
sc.setParametersIfNotNull("id", cmd.getId());
@@ -1157,7 +1161,7 @@ public boolean deleteHSMProfile(DeleteHSMProfileCmd cmd) throws KMSException {
11571161
if (keyCount > 0) {
11581162
throw new InvalidParameterValueException(
11591163
String.format("Cannot delete HSM profile '%s': it is referenced by %d KMS key(s). " +
1160-
"Please delete or reassign those keys first.", profile.getName(), keyCount));
1164+
"Please delete or reassign those keys first.", profile.getName(), keyCount));
11611165
}
11621166

11631167
// Check if any KEK versions reference this HSM profile
@@ -1171,8 +1175,8 @@ public boolean deleteHSMProfile(DeleteHSMProfileCmd cmd) throws KMSException {
11711175
if (wrappedKeyCount > 0) {
11721176
throw new InvalidParameterValueException(
11731177
String.format("Cannot delete HSM profile '%s': it is referenced by %d wrapped key(s) " +
1174-
"through KEK versions. Please wait for key rotation to complete or delete those"
1175-
+ " volumes first.",
1178+
"through KEK versions. Please wait for key rotation to complete or delete those"
1179+
+ " volumes first.",
11761180
profile.getName(), wrappedKeyCount));
11771181
}
11781182
}
@@ -1184,10 +1188,7 @@ public boolean deleteHSMProfile(DeleteHSMProfileCmd cmd) throws KMSException {
11841188
CallContext.current().setEventResourceType(ApiCommandResourceType.HsmProfile);
11851189
CallContext.current().setEventDetails(String.format("Deleted HSM profile: %s (uuid: %s)", profile.getName(), profile.getUuid()));
11861190

1187-
if (hsmProfileDao.remove(profile.getId())) {
1188-
return true;
1189-
}
1190-
return false;
1191+
return hsmProfileDao.remove(profile.getId());
11911192
}
11921193

11931194
@Override
@@ -1413,6 +1414,14 @@ private void initializeKmsProviderMap() {
14131414
public boolean start() {
14141415
super.start();
14151416
initializeKmsProviderMap();
1417+
kmsOperationExecutor = new ThreadPoolExecutor(
1418+
KMSOperationPoolCoreSize.value(),
1419+
KMSOperationPoolMaxSize.value(),
1420+
60L, TimeUnit.SECONDS, new SynchronousQueue<>(), r -> {
1421+
Thread t = new Thread(r, "kms-operation");
1422+
t.setDaemon(true);
1423+
return t;
1424+
});
14161425

14171426
for (KMSProvider provider : kmsProviderMap.values()) {
14181427
if (provider != null) {
@@ -1575,7 +1584,7 @@ private void processVersionRewrap(KMSKekVersionVO oldVersion, int batchSize) thr
15751584
}
15761585

15771586
void rewrapSingleKey(KMSWrappedKeyVO wrappedKeyVO, KMSKeyVO kmsKey,
1578-
KMSKekVersionVO newVersion, KMSProvider provider) {
1587+
KMSKekVersionVO newVersion, KMSProvider provider) {
15791588
byte[] dek = null;
15801589
try {
15811590
dek = unwrapKey(wrappedKeyVO.getId());
@@ -1624,7 +1633,9 @@ public ConfigKey<?>[] getConfigKeys() {
16241633
KMSRetryDelayMs,
16251634
KMSOperationTimeoutSec,
16261635
KMSRewrapBatchSize,
1627-
KMSRewrapIntervalMs
1636+
KMSRewrapIntervalMs,
1637+
KMSOperationPoolCoreSize,
1638+
KMSOperationPoolMaxSize,
16281639
};
16291640
}
16301641

0 commit comments

Comments
 (0)