Skip to content

Commit 3bbb783

Browse files
weizhouapacheweizhouapache
committed
extension: fix update registered extension resource
1 parent 1f9897b commit 3bbb783

File tree

3 files changed

+154
-32
lines changed

3 files changed

+154
-32
lines changed

‎framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java‎

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,39 @@ protected Pair<Boolean, String> getChecksumForExtensionPathOnMSPeer(Extension ex
370370
return getResultFromAnswersString(answersStr, extension, msHost, "get path checksum");
371371
}
372372

373+
protected List<ExtensionResourceMapDetailsVO> buildExtensionResourceDetailsArray(long extensionResourceMapId,
374+
Map<String, String> details) {
375+
List<ExtensionResourceMapDetailsVO> detailsList = new ArrayList<>();
376+
if (MapUtils.isEmpty(details)) {
377+
return detailsList;
378+
}
379+
for (Map.Entry<String, String> entry : details.entrySet()) {
380+
boolean display = !SENSITIVE_DETAIL_KEYS.contains(entry.getKey().toLowerCase());
381+
detailsList.add(new ExtensionResourceMapDetailsVO(extensionResourceMapId, entry.getKey(),
382+
entry.getValue(), display));
383+
}
384+
return detailsList;
385+
}
386+
387+
protected void appendHiddenExtensionResourceDetails(long extensionResourceMapId,
388+
List<ExtensionResourceMapDetailsVO> detailsList) {
389+
if (CollectionUtils.isEmpty(detailsList)) {
390+
return;
391+
}
392+
Map<String, String> hiddenDetails = extensionResourceMapDetailsDao.listDetailsKeyPairs(extensionResourceMapId, false);
393+
if (MapUtils.isEmpty(hiddenDetails)) {
394+
return;
395+
}
396+
Set<String> requestedKeys = detailsList.stream()
397+
.map(ExtensionResourceMapDetailsVO::getName)
398+
.collect(Collectors.toSet());
399+
hiddenDetails.forEach((key, value) -> {
400+
if (!requestedKeys.contains(key)) {
401+
detailsList.add(new ExtensionResourceMapDetailsVO(extensionResourceMapId, key, value, false));
402+
}
403+
});
404+
}
405+
373406
protected List<ExtensionCustomAction.Parameter> getParametersListFromMap(String actionName, Map parametersMap) {
374407
if (MapUtils.isEmpty(parametersMap)) {
375408
return Collections.emptyList();
@@ -1021,31 +1054,18 @@ public Extension updateRegisteredExtensionWithResource(UpdateRegisteredExtension
10211054

10221055
if (Boolean.TRUE.equals(cleanupDetails)) {
10231056
extensionResourceMapDetailsDao.removeDetails(targetMapping.getId());
1024-
}
1025-
if (MapUtils.isNotEmpty(details)) {
1026-
// For sensitive keys (password, sshkey): if the caller explicitly
1027-
// passes a non-blank value → update; passes null/blank → remove the
1028-
// stored entry; omits the key entirely → leave existing value intact.
1029-
Map<String, String> nonSensitive = new HashMap<>();
1030-
for (Map.Entry<String, String> entry : details.entrySet()) {
1031-
String key = entry.getKey();
1032-
String value = entry.getValue();
1033-
if (SENSITIVE_DETAIL_KEYS.contains(key.toLowerCase())) {
1034-
if (StringUtils.isNotBlank(value)) {
1035-
// Explicit non-blank value: update the stored secret.
1036-
extensionResourceMapDetailsDao.addDetail(
1037-
targetMapping.getId(), key, value, false);
1038-
} else {
1039-
// Explicit null / blank value: remove the stored secret.
1040-
extensionResourceMapDetailsDao.removeDetail(
1041-
targetMapping.getId(), key);
1042-
}
1043-
} else {
1044-
nonSensitive.put(key, value);
1045-
}
1046-
}
1047-
if (MapUtils.isNotEmpty(nonSensitive)) {
1048-
updateExtensionResourceMapDetails(targetMapping.getId(), nonSensitive);
1057+
} else {
1058+
List<ExtensionResourceMapDetailsVO> detailsList = buildExtensionResourceDetailsArray(targetMapping.getId(), details);
1059+
if (CollectionUtils.isNotEmpty(detailsList)) {
1060+
appendHiddenExtensionResourceDetails(targetMapping.getId(), detailsList);
1061+
}
1062+
detailsList = detailsList.stream()
1063+
.filter(detail -> detail.getValue() != null)
1064+
.collect(Collectors.toList());
1065+
if (CollectionUtils.isNotEmpty(detailsList)) {
1066+
extensionResourceMapDetailsDao.saveDetails(detailsList);
1067+
} else {
1068+
extensionResourceMapDetailsDao.removeDetails(targetMapping.getId());
10491069
}
10501070
}
10511071

@@ -1080,8 +1100,9 @@ public ExtensionResourceMap registerExtensionWithCluster(Cluster cluster, Extens
10801100
List<ExtensionResourceMapDetailsVO> detailsVOList = new ArrayList<>();
10811101
if (MapUtils.isNotEmpty(details)) {
10821102
for (Map.Entry<String, String> entry : details.entrySet()) {
1103+
boolean display = !SENSITIVE_DETAIL_KEYS.contains(entry.getKey().toLowerCase());
10831104
detailsVOList.add(new ExtensionResourceMapDetailsVO(savedExtensionMap.getId(),
1084-
entry.getKey(), entry.getValue()));
1105+
entry.getKey(), entry.getValue(), display));
10851106
}
10861107
extensionResourceMapDetailsDao.saveDetails(detailsVOList);
10871108
}
@@ -2152,11 +2173,8 @@ public void updateExtensionResourceMapDetails(long extensionResourceMapId, Map<S
21522173
if (MapUtils.isEmpty(details)) {
21532174
return;
21542175
}
2155-
List<ExtensionResourceMapDetailsVO> detailsList = new ArrayList<>();
2156-
for (Map.Entry<String, String> entry : details.entrySet()) {
2157-
detailsList.add(new ExtensionResourceMapDetailsVO(extensionResourceMapId, entry.getKey(),
2158-
entry.getValue()));
2159-
}
2176+
List<ExtensionResourceMapDetailsVO> detailsList =
2177+
buildExtensionResourceDetailsArray(extensionResourceMapId, details);
21602178
extensionResourceMapDetailsDao.saveDetails(detailsList);
21612179
}
21622180

‎framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,14 @@
8787
import org.apache.cloudstack.framework.extensions.dao.ExtensionResourceMapDetailsDao;
8888
import org.apache.cloudstack.framework.extensions.vo.ExtensionCustomActionDetailsVO;
8989
import org.apache.cloudstack.framework.extensions.vo.ExtensionCustomActionVO;
90+
import org.apache.cloudstack.framework.extensions.vo.ExtensionResourceMapDetailsVO;
9091
import org.apache.cloudstack.framework.extensions.vo.ExtensionResourceMapVO;
9192
import org.apache.cloudstack.framework.extensions.vo.ExtensionVO;
9293
import org.apache.cloudstack.utils.identity.ManagementServerNode;
9394
import org.junit.Before;
9495
import org.junit.Test;
9596
import org.junit.runner.RunWith;
97+
import org.mockito.ArgumentCaptor;
9698
import org.mockito.InjectMocks;
9799
import org.mockito.Mock;
98100
import org.mockito.MockedStatic;
@@ -1124,6 +1126,102 @@ public void updateRegisteredExtensionWithResourceUpdatesDetailsForExistingMappin
11241126
verify(extensionResourceMapDetailsDao).saveDetails(any());
11251127
}
11261128

1129+
@Test
1130+
public void updateRegisteredExtensionWithResourceCleanupDetailsFirstThenSaveRequested() {
1131+
UpdateRegisteredExtensionCmd cmd = mock(UpdateRegisteredExtensionCmd.class);
1132+
when(cmd.getResourceType()).thenReturn(ExtensionResourceMap.ResourceType.PhysicalNetwork.name());
1133+
when(cmd.getResourceId()).thenReturn("physnet-uuid");
1134+
when(cmd.getExtensionId()).thenReturn(1L);
1135+
when(cmd.getDetails()).thenReturn(Map.of("username", "root", "password", "secret"));
1136+
when(cmd.isCleanupDetails()).thenReturn(true);
1137+
1138+
ExtensionVO extension = mock(ExtensionVO.class);
1139+
when(extensionDao.findById(1L)).thenReturn(extension);
1140+
1141+
PhysicalNetworkVO physicalNetwork = mock(PhysicalNetworkVO.class);
1142+
when(physicalNetwork.getId()).thenReturn(42L);
1143+
when(physicalNetworkDao.findByUuid("physnet-uuid")).thenReturn(physicalNetwork);
1144+
1145+
ExtensionResourceMapVO existing = mock(ExtensionResourceMapVO.class);
1146+
when(existing.getExtensionId()).thenReturn(1L);
1147+
when(existing.getId()).thenReturn(100L);
1148+
when(extensionResourceMapDao.listByResourceIdAndType(42L, ExtensionResourceMap.ResourceType.PhysicalNetwork))
1149+
.thenReturn(List.of(existing));
1150+
1151+
extensionsManager.updateRegisteredExtensionWithResource(cmd);
1152+
1153+
verify(extensionResourceMapDetailsDao).removeDetails(100L);
1154+
verify(extensionResourceMapDetailsDao, never()).saveDetails(any());
1155+
}
1156+
1157+
@Test
1158+
@SuppressWarnings("unchecked")
1159+
public void updateRegisteredExtensionWithResourceStoresSensitiveDetailsWithDisplayFalse() {
1160+
UpdateRegisteredExtensionCmd cmd = mock(UpdateRegisteredExtensionCmd.class);
1161+
when(cmd.getResourceType()).thenReturn(ExtensionResourceMap.ResourceType.PhysicalNetwork.name());
1162+
when(cmd.getResourceId()).thenReturn("physnet-uuid");
1163+
when(cmd.getExtensionId()).thenReturn(1L);
1164+
when(cmd.getDetails()).thenReturn(Map.of("username", "root", "password", "newSecret"));
1165+
when(cmd.isCleanupDetails()).thenReturn(false);
1166+
1167+
ExtensionVO extension = mock(ExtensionVO.class);
1168+
when(extensionDao.findById(1L)).thenReturn(extension);
1169+
1170+
PhysicalNetworkVO physicalNetwork = mock(PhysicalNetworkVO.class);
1171+
when(physicalNetwork.getId()).thenReturn(42L);
1172+
when(physicalNetworkDao.findByUuid("physnet-uuid")).thenReturn(physicalNetwork);
1173+
1174+
ExtensionResourceMapVO existing = mock(ExtensionResourceMapVO.class);
1175+
when(existing.getExtensionId()).thenReturn(1L);
1176+
when(existing.getId()).thenReturn(100L);
1177+
when(extensionResourceMapDao.listByResourceIdAndType(42L, ExtensionResourceMap.ResourceType.PhysicalNetwork))
1178+
.thenReturn(List.of(existing));
1179+
extensionsManager.updateRegisteredExtensionWithResource(cmd);
1180+
1181+
ArgumentCaptor<List<ExtensionResourceMapDetailsVO>> captor = ArgumentCaptor.forClass(List.class);
1182+
verify(extensionResourceMapDetailsDao).saveDetails(captor.capture());
1183+
verify(extensionResourceMapDetailsDao, never()).removeDetails(anyLong());
1184+
List<ExtensionResourceMapDetailsVO> savedDetails = captor.getValue();
1185+
1186+
ExtensionResourceMapDetailsVO passwordDetail = savedDetails.stream()
1187+
.filter(detail -> "password".equals(detail.getName()))
1188+
.findFirst()
1189+
.orElse(null);
1190+
assertNotNull(passwordDetail);
1191+
assertFalse(passwordDetail.isDisplay());
1192+
assertEquals("newSecret", passwordDetail.getValue());
1193+
}
1194+
1195+
@Test
1196+
@SuppressWarnings("unchecked")
1197+
public void registerExtensionWithClusterStoresSensitiveDetailsWithDisplayFalse() {
1198+
Cluster cluster = mock(Cluster.class);
1199+
when(cluster.getId()).thenReturn(12L);
1200+
when(cluster.getName()).thenReturn("cluster-12");
1201+
when(cluster.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.External);
1202+
1203+
Extension extension = mock(Extension.class);
1204+
when(extension.getId()).thenReturn(5L);
1205+
1206+
ExtensionResourceMapVO persistedMap = mock(ExtensionResourceMapVO.class);
1207+
when(persistedMap.getId()).thenReturn(120L);
1208+
when(extensionResourceMapDao.persist(any())).thenReturn(persistedMap);
1209+
1210+
extensionsManager.registerExtensionWithCluster(cluster, extension,
1211+
Map.of("username", "admin", "password", "s3cr3t"));
1212+
1213+
ArgumentCaptor<List<ExtensionResourceMapDetailsVO>> captor = ArgumentCaptor.forClass(List.class);
1214+
verify(extensionResourceMapDetailsDao).saveDetails(captor.capture());
1215+
List<ExtensionResourceMapDetailsVO> savedDetails = captor.getValue();
1216+
1217+
ExtensionResourceMapDetailsVO passwordDetail = savedDetails.stream()
1218+
.filter(detail -> "password".equals(detail.getName()))
1219+
.findFirst()
1220+
.orElse(null);
1221+
assertNotNull(passwordDetail);
1222+
assertFalse(passwordDetail.isDisplay());
1223+
}
1224+
11271225
@Test
11281226
public void testCreateExtensionResponse_BasicFields() {
11291227
Extension extension = mock(Extension.class);

‎ui/src/views/extension/ExtensionResourcesTab.vue‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@
7777
<update-registered-extension
7878
:resource="resource"
7979
:extension-resource="selectedResource"
80-
@refresh-data="$emit('refresh-data')"
80+
@refresh-data="handleRefreshData"
8181
@close-action="closeUpdateModal" />
8282
</a-modal>
8383
</div>
@@ -97,6 +97,7 @@ export default {
9797
TooltipButton,
9898
UpdateRegisteredExtension
9999
},
100+
inject: ['parentFetchData'],
100101
props: {
101102
resource: {
102103
type: Object,
@@ -145,6 +146,11 @@ export default {
145146
this.updateModalVisible = false
146147
this.selectedResource = null
147148
},
149+
handleRefreshData () {
150+
if (this.parentFetchData) {
151+
this.parentFetchData()
152+
}
153+
},
148154
unregisterExtension (record) {
149155
const params = {
150156
extensionid: this.resource.id,

0 commit comments

Comments
 (0)