Skip to content

Commit 294ca3b

Browse files
vishesh92vishesh92
committed
fix marvin tests
1 parent 1469411 commit 294ca3b

3 files changed

Lines changed: 108 additions & 14 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,8 +1235,9 @@ public boolean deleteHSMProfile(DeleteHSMProfileCmd cmd) throws KMSException {
12351235
Account caller = CallContext.current().getCallingAccount();
12361236
checkHSMProfileAccess(caller, profile, true);
12371237

1238-
if (profile.getProtocol().equals(DATABASE_PROVIDER_NAME)) {
1239-
throw new InvalidParameterValueException("Database HSM profile is default and cannot be deleted");
1238+
if (profile.getProtocol().equals(DATABASE_PROVIDER_NAME)
1239+
&& profile.getAccountId() == Account.ACCOUNT_ID_SYSTEM) {
1240+
throw new InvalidParameterValueException("Default database HSM profile cannot be deleted");
12401241
}
12411242

12421243
long keyCount = kmsKeyDao.countByHsmProfileId(profile.getId());

‎server/src/test/java/org/apache/cloudstack/kms/KMSManagerImplHSMTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,19 @@
2020
import com.cloud.api.ApiResponseHelper;
2121
import com.cloud.dc.dao.DataCenterDao;
2222
import com.cloud.domain.dao.DomainDao;
23+
import com.cloud.exception.InvalidParameterValueException;
24+
import com.cloud.user.Account;
2325
import com.cloud.user.AccountManager;
2426
import com.cloud.user.dao.AccountDao;
27+
import org.apache.cloudstack.api.command.user.kms.hsm.DeleteHSMProfileCmd;
2528
import org.apache.cloudstack.api.response.HSMProfileResponse;
29+
import org.apache.cloudstack.context.CallContext;
30+
import org.apache.cloudstack.framework.kms.KMSProvider;
2631
import org.apache.cloudstack.kms.dao.HSMProfileDao;
2732
import org.apache.cloudstack.kms.dao.HSMProfileDetailsDao;
33+
import org.apache.cloudstack.kms.dao.KMSKekVersionDao;
34+
import org.apache.cloudstack.kms.dao.KMSKeyDao;
35+
import org.apache.cloudstack.kms.dao.KMSWrappedKeyDao;
2836
import org.junit.Test;
2937
import org.junit.runner.RunWith;
3038
import org.mockito.InjectMocks;
@@ -35,10 +43,12 @@
3543
import org.mockito.junit.MockitoJUnitRunner;
3644

3745
import java.util.Arrays;
46+
import java.util.Collections;
3847

3948
import static org.junit.Assert.assertFalse;
4049
import static org.junit.Assert.assertNotNull;
4150
import static org.junit.Assert.assertTrue;
51+
import static org.mockito.Mockito.doReturn;
4252
import static org.mockito.Mockito.mock;
4353
import static org.mockito.Mockito.verify;
4454
import static org.mockito.Mockito.when;
@@ -66,6 +76,12 @@ public class KMSManagerImplHSMTest {
6676
private DomainDao domainDao;
6777
@Mock
6878
private AccountDao accountDao;
79+
@Mock
80+
private KMSKeyDao kmsKeyDao;
81+
@Mock
82+
private KMSKekVersionDao kmsKekVersionDao;
83+
@Mock
84+
private KMSWrappedKeyDao kmsWrappedKeyDao;
6985

7086
/**
7187
* Test: isSensitiveKey correctly identifies "pin" as sensitive
@@ -157,4 +173,70 @@ public void testCreateHSMProfileResponse_PopulatesDetails() {
157173
verify(hsmProfileDetailsDao).listByProfileId(profileId);
158174
}
159175
}
176+
177+
/**
178+
* Test: the seeded default (system-owned) database profile cannot be deleted.
179+
*/
180+
@Test(expected = InvalidParameterValueException.class)
181+
public void testDeleteHSMProfile_RejectsSystemOwnedDatabaseProfile() {
182+
Long profileId = 10L;
183+
HSMProfileVO profile = mock(HSMProfileVO.class);
184+
when(profile.getProtocol()).thenReturn("database");
185+
when(profile.getAccountId()).thenReturn(Account.ACCOUNT_ID_SYSTEM);
186+
when(profile.getIsPublic()).thenReturn(true);
187+
when(hsmProfileDao.findById(profileId)).thenReturn(profile);
188+
189+
Account caller = mock(Account.class);
190+
when(caller.getId()).thenReturn(2L);
191+
when(accountManager.isRootAdmin(2L)).thenReturn(true);
192+
193+
DeleteHSMProfileCmd cmd = mock(DeleteHSMProfileCmd.class);
194+
when(cmd.getId()).thenReturn(profileId);
195+
196+
try (MockedStatic<CallContext> mockedCallContext = Mockito.mockStatic(CallContext.class)) {
197+
CallContext callContext = mock(CallContext.class);
198+
mockedCallContext.when(CallContext::current).thenReturn(callContext);
199+
when(callContext.getCallingAccount()).thenReturn(caller);
200+
201+
kmsManager.deleteHSMProfile(cmd);
202+
}
203+
}
204+
205+
/**
206+
* Test: an admin-created (non-system) database profile with no keys can be deleted.
207+
*/
208+
@Test
209+
public void testDeleteHSMProfile_AllowsAdminOwnedDatabaseProfile() {
210+
Long profileId = 10L;
211+
HSMProfileVO profile = mock(HSMProfileVO.class);
212+
when(profile.getId()).thenReturn(profileId);
213+
when(profile.getProtocol()).thenReturn("database");
214+
when(profile.getAccountId()).thenReturn(200L);
215+
when(profile.getIsPublic()).thenReturn(false);
216+
when(hsmProfileDao.findById(profileId)).thenReturn(profile);
217+
218+
when(kmsKeyDao.countByHsmProfileId(profileId)).thenReturn(0L);
219+
when(kmsKekVersionDao.listByHsmProfileId(profileId)).thenReturn(Collections.emptyList());
220+
221+
KMSProvider kmsProvider = mock(KMSProvider.class);
222+
doReturn(kmsProvider).when(kmsManager).getKMSProvider("database");
223+
when(hsmProfileDao.remove(profileId)).thenReturn(true);
224+
225+
Account caller = mock(Account.class);
226+
227+
DeleteHSMProfileCmd cmd = mock(DeleteHSMProfileCmd.class);
228+
when(cmd.getId()).thenReturn(profileId);
229+
230+
try (MockedStatic<CallContext> mockedCallContext = Mockito.mockStatic(CallContext.class)) {
231+
CallContext callContext = mock(CallContext.class);
232+
mockedCallContext.when(CallContext::current).thenReturn(callContext);
233+
when(callContext.getCallingAccount()).thenReturn(caller);
234+
235+
boolean result = kmsManager.deleteHSMProfile(cmd);
236+
237+
assertTrue("Admin-owned database profile should be deletable", result);
238+
verify(kmsProvider).invalidateProfileCache(profileId);
239+
verify(hsmProfileDao).remove(profileId);
240+
}
241+
}
160242
}

‎test/integration/smoke/test_kms_lifecycle.py‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,27 @@ def setUpClass(cls):
6868

6969
cls._cleanup = []
7070

71-
cls.default_profile = None
72-
profiles = HSMProfile.list(cls.apiclient, name="HSM Database Provider")
73-
if profiles and len(profiles) > 0:
74-
cls.default_profile = profiles[0]
75-
if hasattr(cls.default_profile, 'enabled') and not cls.default_profile.enabled:
76-
hsm = HSMProfile({"id": cls.default_profile.id})
77-
hsm.update(cls.apiclient, enabled=True)
71+
# The built-in database provider is seeded as a public, system-owned profile named
72+
# "HSM Database Provider". listHSMProfiles has no server-side name filter, and a root
73+
# admin's listing without listall is restricted to admin-owned profiles, so we must
74+
# pass listall=True and match the name client-side.
75+
cls.default_profile = cls._find_default_profile(cls.apiclient)
76+
if cls.default_profile and not cls.default_profile.enabled:
77+
hsm = HSMProfile({"id": cls.default_profile.id})
78+
hsm.update(cls.apiclient, enabled=True)
7879
# Re-fetch to get updated state
79-
profiles = HSMProfile.list(cls.apiclient, name="HSM Database Provider")
80-
if profiles and len(profiles) > 0:
81-
cls.default_profile = profiles[0]
80+
cls.default_profile = cls._find_default_profile(cls.apiclient)
81+
82+
@staticmethod
83+
def _find_default_profile(apiclient):
84+
"""Locate the seeded public 'HSM Database Provider' profile."""
85+
profiles = HSMProfile.list(
86+
apiclient, listall=True, keyword="HSM Database Provider"
87+
) or []
88+
for profile in profiles:
89+
if profile.name == "HSM Database Provider":
90+
return profile
91+
return None
8292

8393

8494
@classmethod
@@ -139,13 +149,12 @@ def _create_kms_key(self, name, profile_id, apiclient=None, zoneid=None, purpose
139149
self.cleanup.append(key)
140150
return key
141151

142-
def _create_hsm_profile(self, name, protocol="database", system=True, zoneid=None):
152+
def _create_hsm_profile(self, name, protocol="database", zoneid=None):
143153
zone_id = zoneid or self.zone.id
144154
profile = HSMProfile.create(
145155
self.apiclient,
146156
name=name,
147157
protocol=protocol,
148-
system=system,
149158
zoneid=zone_id,
150159
)
151160
self.cleanup.append(profile)
@@ -344,6 +353,7 @@ def test_09_delete_kms_key(self):
344353
key = self._create_kms_key(name=_random_name("key-del"), profile_id=profile.id)
345354

346355
key.delete(self.apiclient)
356+
self.cleanup.remove(key)
347357

348358
# Verify the key no longer appears in listings
349359
keys = KMSKey.list(self.apiclient, id=key.id)
@@ -480,6 +490,7 @@ def test_13_delete_hsm_profile(self):
480490
profile = self._create_hsm_profile(name=_random_name("hsm-gone"))
481491

482492
profile.delete(self.apiclient)
493+
self.cleanup.remove(profile)
483494

484495
profiles = HSMProfile.list(self.apiclient, id=profile.id)
485496
self.assertTrue(

0 commit comments

Comments
 (0)