Skip to content

minio: fix store user creation #8425

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

Merged
shwstppr merged 5 commits into from
Jan 9, 2024
Merged

minio: fix store user creation #8425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa22ba6
minio: fix store user creation
shwstppr Jan 2, 2024
adb8785
test fix
shwstppr Jan 2, 2024
fba6c56
log fix
shwstppr Jan 4, 2024
a28b78a
prevent secret key update when not returned in minio response
shwstppr Jan 5, 2024
2a6de43
handle RuntimeException
shwstppr Jan 8, 2024
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
100 changes: 69 additions & 31 deletions .../main/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,38 @@
*/
package org.apache.cloudstack.storage.datastore.driver;

import java.io.IOException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
import javax.inject.Inject;

import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreDao;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreDetailsDao;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO;
import org.apache.cloudstack.storage.object.BaseObjectStoreDriverImpl;
import org.apache.cloudstack.storage.object.Bucket;
import org.apache.cloudstack.storage.object.BucketObject;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;

import com.amazonaws.services.s3.model.AccessControlList;
import com.amazonaws.services.s3.model.BucketPolicy;
import com.cloud.agent.api.to.DataStoreTO;
import org.apache.cloudstack.storage.object.Bucket;
import com.cloud.storage.BucketVO;
import com.cloud.storage.dao.BucketDao;
import com.cloud.user.Account;
import com.cloud.user.AccountDetailsDao;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.exception.CloudRuntimeException;

import io.minio.BucketExistsArgs;
import io.minio.DeleteBucketEncryptionArgs;
import io.minio.MakeBucketArgs;
Expand All @@ -42,26 +64,10 @@
import io.minio.admin.messages.DataUsageInfo;
import io.minio.messages.SseConfiguration;
import io.minio.messages.VersioningConfiguration;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreDao;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreDetailsDao;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO;
import org.apache.cloudstack.storage.object.BaseObjectStoreDriverImpl;
import org.apache.cloudstack.storage.object.BucketObject;
import org.apache.commons.codec.binary.Base64;
import org.apache.log4j.Logger;

import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
import javax.inject.Inject;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class MinIOObjectStoreDriverImpl extends BaseObjectStoreDriverImpl {
private static final Logger s_logger = Logger.getLogger(MinIOObjectStoreDriverImpl.class);
protected static final String ACS_PREFIX = "acs";

@Inject
AccountDao _accountDao;
Expand All @@ -81,14 +87,18 @@ public class MinIOObjectStoreDriverImpl extends BaseObjectStoreDriverImpl {
private static final String ACCESS_KEY = "accesskey";
private static final String SECRET_KEY = "secretkey";

private static final String MINIO_ACCESS_KEY = "minio-accesskey";
private static final String MINIO_SECRET_KEY = "minio-secretkey";
protected static final String MINIO_ACCESS_KEY = "minio-accesskey";
protected static final String MINIO_SECRET_KEY = "minio-secretkey";

@Override
public DataStoreTO getStoreTO(DataStore store) {
return null;
}

protected String getUserOrAccessKeyForAccount(Account account) {
return String.format("%s-%s", ACS_PREFIX, account.getUuid());
}

@Override
public Bucket createBucket(Bucket bucket, boolean objectLock) {
//ToDo Client pool mgmt
Expand Down Expand Up @@ -135,8 +145,8 @@ public Bucket createBucket(Bucket bucket, boolean objectLock) {
" \"Version\": \"2012-10-17\"\n" +
" }";
MinioAdminClient minioAdminClient = getMinIOAdminClient(storeId);
String policyName = "acs-"+account.getAccountName()+"-policy";
String userName = "acs-"+account.getAccountName();
String policyName = getUserOrAccessKeyForAccount(account) + "-policy";
String userName = getUserOrAccessKeyForAccount(account);
try {
minioAdminClient.addCannedPolicy(policyName, policy);
minioAdminClient.setPolicy(userName, false, policyName);
Expand Down Expand Up @@ -250,22 +260,53 @@ public void deleteBucketPolicy(String bucketName, long storeId) {

}

protected void updateAccountCredentials(final long accountId, final String accessKey, final String secretKey, final boolean checkIfNotPresent) {
Map<String, String> details = _accountDetailsDao.findDetails(accountId);
boolean updateNeeded = false;
if (!checkIfNotPresent || StringUtils.isBlank(details.get(MINIO_ACCESS_KEY))) {
details.put(MINIO_ACCESS_KEY, accessKey);
updateNeeded = true;
}
if (StringUtils.isAllBlank(secretKey, details.get(MINIO_SECRET_KEY))) {
s_logger.error(String.format("Failed to retrieve secret key for MinIO user: %s from store and account details", accessKey));
}
if (StringUtils.isNotBlank(secretKey) && (!checkIfNotPresent || StringUtils.isBlank(details.get(MINIO_SECRET_KEY)))) {
details.put(MINIO_SECRET_KEY, secretKey);
updateNeeded = true;
}
if (!updateNeeded) {
return;
}
_accountDetailsDao.persist(accountId, details);
}

@Override
public boolean createUser(long accountId, long storeId) {
Account account = _accountDao.findById(accountId);
MinioAdminClient minioAdminClient = getMinIOAdminClient(storeId);
String accessKey = "acs-"+account.getAccountName();
String accessKey = getUserOrAccessKeyForAccount(account);
// Check user exists
try {
UserInfo userInfo = minioAdminClient.getUserInfo(accessKey);
if(userInfo != null) {
s_logger.debug("User already exists in MinIO store: "+accessKey);
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Skipping user creation as the user already exists in MinIO store: %s", accessKey));
}
updateAccountCredentials(accountId, accessKey, userInfo.secretKey(), true);
return true;
}
} catch (Exception e) {
s_logger.debug("User does not exist. Creating user: "+accessKey);
} catch (NoSuchAlgorithmException | IOException | InvalidKeyException e) {
s_logger.error(String.format("Error encountered while retrieving user: %s for existing MinIO store user check", accessKey), e);
return false;
} catch (RuntimeException e) { // MinIO lib may throw RuntimeException with code: XMinioAdminNoSuchUser
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Ignoring error encountered while retrieving user: %s for existing MinIO store user check", accessKey));
}
s_logger.trace("Exception during MinIO user check", e);
}
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("MinIO store user does not exist. Creating user: %s", accessKey));
}

KeyGenerator generator = null;
try {
generator = KeyGenerator.getInstance("HmacSHA1");
Expand All @@ -280,10 +321,7 @@ public boolean createUser(long accountId, long storeId) {
throw new CloudRuntimeException(e);
}
// Store user credentials
Map<String, String> details = new HashMap<>();
details.put(MINIO_ACCESS_KEY, accessKey);
details.put(MINIO_SECRET_KEY, secretKey);
_accountDetailsDao.persist(accountId, details);
updateAccountCredentials(accountId, accessKey, secretKey, false);
return true;
}

Expand Down
76 changes: 54 additions & 22 deletions ...t/java/org/apache/cloudstack/storage/datastore/driver/MinIOObjectStoreDriverImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,23 @@
// under the License.
package org.apache.cloudstack.storage.datastore.driver;

import com.cloud.storage.BucketVO;
import com.cloud.storage.dao.BucketDao;
import com.cloud.user.AccountDetailVO;
import com.cloud.user.AccountDetailsDao;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
import io.minio.BucketExistsArgs;
import io.minio.MinioClient;
import io.minio.RemoveBucketArgs;
import io.minio.admin.MinioAdminClient;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyLong;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;

import org.apache.cloudstack.storage.datastore.db.ObjectStoreDao;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreDetailsDao;
import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO;
Expand All @@ -34,22 +41,24 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.stubbing.Answer;

import java.util.ArrayList;
import com.cloud.storage.BucketVO;
import com.cloud.storage.dao.BucketDao;
import com.cloud.user.AccountDetailVO;
import com.cloud.user.AccountDetailsDao;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyLong;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import io.minio.BucketExistsArgs;
import io.minio.MinioClient;
import io.minio.RemoveBucketArgs;
import io.minio.admin.MinioAdminClient;
import io.minio.admin.UserInfo;

@RunWith(MockitoJUnitRunner.class)
public class MinIOObjectStoreDriverImplTest {
Expand Down Expand Up @@ -97,7 +106,7 @@ public void testCreateBucket() throws Exception {
doReturn(minioClient).when(minioObjectStoreDriverImpl).getMinIOClient(anyLong());
doReturn(minioAdminClient).when(minioObjectStoreDriverImpl).getMinIOAdminClient(anyLong());
when(bucketDao.listByObjectStoreIdAndAccountId(anyLong(), anyLong())).thenReturn(new ArrayList<BucketVO>());
when(account.getAccountName()).thenReturn("admin");
when(account.getUuid()).thenReturn(UUID.randomUUID().toString());
when(accountDao.findById(anyLong())).thenReturn(account);
when(accountDetailsDao.findDetail(anyLong(),anyString())).
thenReturn(new AccountDetailVO(1L, "abc","def"));
Expand All @@ -119,4 +128,27 @@ public void testDeleteBucket() throws Exception {
verify(minioClient, times(1)).bucketExists(any());
verify(minioClient, times(1)).removeBucket(any());
}

@Test
public void testCreateUserExisting() throws Exception {
String uuid = "uuid";
String accessKey = MinIOObjectStoreDriverImpl.ACS_PREFIX + "-" + uuid;
String secretKey = "secret";

doReturn(minioAdminClient).when(minioObjectStoreDriverImpl).getMinIOAdminClient(anyLong());
when(accountDao.findById(anyLong())).thenReturn(account);
when(account.getUuid()).thenReturn(uuid);
UserInfo info = mock(UserInfo.class);
when(info.secretKey()).thenReturn(secretKey);
when(minioAdminClient.getUserInfo(accessKey)).thenReturn(info);
final Map<String, String> persistedMap = new HashMap<>();
Mockito.doAnswer((Answer<Void>) invocation -> {
persistedMap.putAll((Map<String, String>)invocation.getArguments()[1]);
return null;
}).when(accountDetailsDao).persist(Mockito.anyLong(), Mockito.anyMap());
boolean result = minioObjectStoreDriverImpl.createUser(1L, 1L);
assertTrue(result);
assertEquals(accessKey, persistedMap.get(MinIOObjectStoreDriverImpl.MINIO_ACCESS_KEY));
assertEquals(secretKey, persistedMap.get(MinIOObjectStoreDriverImpl.MINIO_SECRET_KEY));
}
}