Copilot commented on code in PR #14160:
URL: https://github.com/apache/cloudstack/pull/14160#discussion_r3995283962
##########
plugins/pom.xml:
##########
@@ -142,6 +142,7 @@
<module>storage/object/minio</module>
<module>storage/object/ceph</module>
<module>storage/object/cloudian</module>
+ <module>storage/object/seaweedfs</module>
Review Comment:
Adding the module only to the plugins reactor does not put it in the
management-server/client artifact. `client/pom.xml` explicitly lists the
object-storage providers, but has no SeaweedFS dependency, so this provider's
classes and Spring metadata will not be shipped or loaded in normal builds. Add
the matching dependency to the client packaging POM as well.
##########
plugins/storage/object/seaweedfs/src/main/java/org/apache/cloudstack/storage/datastore/util/SeaweedFSObjectStoreUtil.java:
##########
@@ -0,0 +1,285 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+// SPDX-License-Identifier: Apache-2.0
+package org.apache.cloudstack.storage.datastore.util;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.amazonaws.AmazonServiceException;
+import com.amazonaws.auth.AWSStaticCredentialsProvider;
+import com.amazonaws.auth.BasicAWSCredentials;
+import com.amazonaws.client.builder.AwsClientBuilder;
+import com.amazonaws.services.identitymanagement.AmazonIdentityManagement;
+import
com.amazonaws.services.identitymanagement.AmazonIdentityManagementClientBuilder;
+import com.amazonaws.services.s3.AmazonS3;
+import com.amazonaws.services.s3.AmazonS3ClientBuilder;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+/**
+ * Utility class for the SeaweedFS object storage provider.
+ *
+ * SeaweedFS exposes both an S3-compatible API and an AWS IAM-compatible API,
+ * so this provider needs no proprietary admin client — only the AWS S3 and IAM
+ * SDKs, the same pair Cloudian HyperStore already uses in this tree.
+ */
+public class SeaweedFSObjectStoreUtil {
+
+ /** The name of our Object Store Provider */
+ public static final String OBJECT_STORE_PROVIDER_NAME = "SeaweedFS";
+
+ public static final String STORE_KEY_PROVIDER_NAME = "providerName";
+ public static final String STORE_KEY_URL = "url";
+ public static final String STORE_KEY_NAME = "name";
+ public static final String STORE_KEY_DETAILS = "details";
+
+ // Store Details Map key names - managed outside of plugin
+ public static final String STORE_DETAILS_KEY_ACCESS_KEY = "accesskey";
// admin/root access key
+ public static final String STORE_DETAILS_KEY_SECRET_KEY = "secretkey";
// admin/root secret key
+ public static final String STORE_DETAILS_KEY_S3_URL = "s3Url";
// S3 endpoint URL
+ public static final String STORE_DETAILS_KEY_IAM_URL = "iamUrl";
// IAM endpoint URL
+
+ // Account Detail Map key names - credentials created per CloudStack
account
+ public static final String KEY_ACCESS_KEY = "swfs_AccessKey";
+ public static final String KEY_SECRET_KEY = "swfs_SecretKey";
+
+ /**
+ * IAM user policy applied to each per-account IAM user. Grants full S3
+ * access except bucket creation/deletion, so CloudStack retains control of
+ * bucket lifecycle while the account's IAM credentials can manage objects.
+ */
+ public static final String IAM_USER_POLICY = "{\n" +
+ " \"Version\": \"2012-10-17\",\n" +
+ " \"Statement\": [\n" +
+ " {\n" +
+ " \"Sid\": \"AllowFullS3Access\",\n" +
+ " \"Effect\": \"Allow\",\n" +
+ " \"Action\": [\n" +
+ " \"s3:*\"\n" +
+ " ],\n" +
+ " \"Resource\": \"*\"\n" +
Review Comment:
This policy is attached to every account-specific IAM user, but `s3:*` on
`*` permits reading, modifying, deleting, and changing policies/ACLs for every
bucket in the store; only bucket creation and deletion are denied. Without a
SeaweedFS-side group boundary, one CloudStack account can operate on another
account's data. Scope the policy to that account's bucket ARNs and update it as
buckets are created.
##########
plugins/storage/object/seaweedfs/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SeaweedFSObjectStoreLifeCycleImpl.java:
##########
@@ -0,0 +1,160 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+// SPDX-License-Identifier: Apache-2.0
+package org.apache.cloudstack.storage.datastore.lifecycle;
+
+import com.cloud.agent.api.StoragePoolInfo;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
+import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO;
+import org.apache.cloudstack.storage.datastore.util.SeaweedFSObjectStoreUtil;
+import org.apache.cloudstack.storage.object.datastore.ObjectStoreHelper;
+import
org.apache.cloudstack.storage.object.datastore.ObjectStoreProviderManager;
+import
org.apache.cloudstack.storage.object.store.lifecycle.ObjectStoreLifeCycle;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import javax.inject.Inject;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class SeaweedFSObjectStoreLifeCycleImpl implements ObjectStoreLifeCycle
{
+
+ protected Logger logger =
LogManager.getLogger(SeaweedFSObjectStoreLifeCycleImpl.class);
+
+ @Inject
+ ObjectStoreHelper objectStoreHelper;
+ @Inject
+ ObjectStoreProviderManager objectStoreMgr;
+
+ public SeaweedFSObjectStoreLifeCycleImpl() {
+ }
+
+ @Override
+ public DataStore initialize(Map<String, Object> dsInfos) {
+
+ String name =
(String)dsInfos.get(SeaweedFSObjectStoreUtil.STORE_KEY_NAME);
+ String url =
(String)dsInfos.get(SeaweedFSObjectStoreUtil.STORE_KEY_URL);
+ String providerName =
(String)dsInfos.get(SeaweedFSObjectStoreUtil.STORE_KEY_PROVIDER_NAME);
+
+ // Check the providerName is what we expect
+ if (! StringUtils.equalsIgnoreCase(providerName,
SeaweedFSObjectStoreUtil.OBJECT_STORE_PROVIDER_NAME)) {
+ String msg = String.format("Unexpected providerName \"%s\".
Expected \"%s\"", providerName,
SeaweedFSObjectStoreUtil.OBJECT_STORE_PROVIDER_NAME);
+ logger.error(msg);
+ throw new CloudRuntimeException(msg);
+ }
+
+ Map<String, Object> objectStoreParameters = new HashMap<String,
Object>();
+ objectStoreParameters.put(SeaweedFSObjectStoreUtil.STORE_KEY_NAME,
name);
+ objectStoreParameters.put(SeaweedFSObjectStoreUtil.STORE_KEY_URL, url);
+
objectStoreParameters.put(SeaweedFSObjectStoreUtil.STORE_KEY_PROVIDER_NAME,
providerName);
Review Comment:
`ObjectStoreHelper.createObjectStore` unconditionally unboxes
`params.get("size")`. This map never copies `dsInfos.get("size")`, so every
`addObjectStoragePool` for SeaweedFS reaches persistence with a null size and
fails before the store is created. Copy the size parameter as the other
object-store lifecycles do.
##########
plugins/storage/object/seaweedfs/src/main/java/org/apache/cloudstack/storage/datastore/driver/SeaweedFSObjectStoreDriverImpl.java:
##########
@@ -0,0 +1,478 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+// SPDX-License-Identifier: Apache-2.0
+package org.apache.cloudstack.storage.datastore.driver;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+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.datastore.util.SeaweedFSObjectStoreUtil;
+import org.apache.cloudstack.storage.object.BaseObjectStoreDriverImpl;
+import org.apache.cloudstack.storage.object.Bucket;
+import org.apache.cloudstack.storage.object.BucketObject;
+
+import com.amazonaws.AmazonClientException;
+import com.amazonaws.services.identitymanagement.AmazonIdentityManagement;
+import com.amazonaws.services.identitymanagement.model.AccessKey;
+import com.amazonaws.services.identitymanagement.model.CreateAccessKeyRequest;
+import com.amazonaws.services.identitymanagement.model.CreateAccessKeyResult;
+import com.amazonaws.services.identitymanagement.model.CreateUserRequest;
+import
com.amazonaws.services.identitymanagement.model.EntityAlreadyExistsException;
+import com.amazonaws.services.identitymanagement.model.PutUserPolicyRequest;
+import com.amazonaws.services.s3.AmazonS3;
+import com.amazonaws.services.s3.model.AccessControlList;
+import com.amazonaws.services.s3.model.BucketPolicy;
+import com.amazonaws.services.s3.model.BucketVersioningConfiguration;
+import com.amazonaws.services.s3.model.CreateBucketRequest;
+import com.amazonaws.services.s3.model.DeleteBucketPolicyRequest;
+import com.amazonaws.services.s3.model.GetBucketPolicyRequest;
+import com.amazonaws.services.s3.model.SSEAlgorithm;
+import com.amazonaws.services.s3.model.ServerSideEncryptionByDefault;
+import com.amazonaws.services.s3.model.ServerSideEncryptionConfiguration;
+import com.amazonaws.services.s3.model.ServerSideEncryptionRule;
+import com.amazonaws.services.s3.model.SetBucketEncryptionRequest;
+import com.amazonaws.services.s3.model.SetBucketVersioningConfigurationRequest;
+import com.cloud.agent.api.to.BucketTO;
+import com.cloud.agent.api.to.DataStoreTO;
+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;
+
+/**
+ * SeaweedFS object store driver.
+ *
+ * Bucket operations use the AWS S3 SDK v1 (path-style access,
endpoint-pinned).
+ * User/credential management uses the AWS IAM SDK v1, since SeaweedFS exposes
a
+ * standard AWS IAM-compatible API. No proprietary admin client is needed.
+ *
+ * Modeled on CloudianHyperStoreObjectStoreDriverImpl, which uses the same
+ * S3 + IAM SDK pair.
+ */
+public class SeaweedFSObjectStoreDriverImpl extends BaseObjectStoreDriverImpl {
+
+ @Inject
+ AccountDao _accountDao;
+
+ @Inject
+ AccountDetailsDao _accountDetailsDao;
+
+ @Inject
+ ObjectStoreDao _storeDao;
+
+ @Inject
+ BucketDao _bucketDao;
+
+ @Inject
+ ObjectStoreDetailsDao _storeDetailsDao;
+
+ private static final String ACS_PREFIX = "acs";
+
+ @Override
+ public DataStoreTO getStoreTO(DataStore store) {
+ return null;
+ }
+
+ /**
+ * Get the SeaweedFS IAM user name for the given CloudStack account.
+ * Uses the account UUID prefixed with "acs-" for namespacing.
+ */
+ protected String getUserNameForAccount(Account account) {
+ return String.format("%s-%s", ACS_PREFIX, account.getUuid());
+ }
+
+ /**
+ * Create the IAM user for the CloudStack account if it doesn't exist,
+ * attach the restricted S3 policy, create an access key, and persist the
+ * credentials in the account details.
+ *
+ * @return true if the user exists or was created, false on failure.
+ */
+ @Override
+ public boolean createUser(long accountId, long storeId) {
+ Account account = _accountDao.findById(accountId);
+ if (account == null) {
+ logger.error("Account {} not found", accountId);
+ return false;
+ }
+ String userName = getUserNameForAccount(account);
+ AmazonIdentityManagement iamClient = getIAMClient(storeId);
+
+ // Create the IAM user if it doesn't already exist
+ try {
+ iamClient.createUser(new CreateUserRequest(userName));
+ logger.info("Created IAM user {} for account {}", userName,
account.getAccountName());
+ } catch (EntityAlreadyExistsException e) {
+ logger.debug("IAM user {} already exists", userName);
+ }
+
+ // Attach the restricted S3 policy (idempotent — overwrites if present)
+ iamClient.putUserPolicy(new PutUserPolicyRequest(userName,
+ "CloudStackPolicy", SeaweedFSObjectStoreUtil.IAM_USER_POLICY));
+
+ // Create a new access key for this user
+ CreateAccessKeyResult result = iamClient.createAccessKey(
+ new CreateAccessKeyRequest().withUserName(userName));
+ AccessKey key = result.getAccessKey();
Review Comment:
`createUser` is called by `allocBucket` before each bucket creation, but
this path creates a new access key even after `EntityAlreadyExistsException`
and overwrites the account's stored key. Repeated bucket requests therefore
rotate credentials, leave earlier bucket records with invalid credentials, and
eventually hit IAM access-key limits. Reuse and validate the stored key,
cleaning up unmanaged keys before creating a replacement.
This issue also appears on line 428 of the same file.
##########
plugins/storage/object/seaweedfs/src/test/java/org/apache/cloudstack/storage/datastore/driver/SeaweedFSObjectStoreDriverImplTest.java:
##########
@@ -0,0 +1,390 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+// SPDX-License-Identifier: Apache-2.0
+package org.apache.cloudstack.storage.datastore.driver;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+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.List;
+import java.util.Map;
+
+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.datastore.util.SeaweedFSObjectStoreUtil;
+import org.apache.cloudstack.storage.object.Bucket;
+
+import com.amazonaws.services.identitymanagement.AmazonIdentityManagement;
+import com.amazonaws.services.identitymanagement.model.AccessKey;
+import com.amazonaws.services.identitymanagement.model.CreateAccessKeyRequest;
+import com.amazonaws.services.identitymanagement.model.CreateAccessKeyResult;
+import com.amazonaws.services.identitymanagement.model.CreateUserRequest;
+import
com.amazonaws.services.identitymanagement.model.EntityAlreadyExistsException;
+import com.amazonaws.services.identitymanagement.model.PutUserPolicyRequest;
+import com.amazonaws.services.s3.AmazonS3;
+import com.amazonaws.services.s3.model.BucketVersioningConfiguration;
+import com.amazonaws.services.s3.model.CreateBucketRequest;
+import com.amazonaws.services.s3.model.ListObjectsV2Request;
+import com.amazonaws.services.s3.model.ListObjectsV2Result;
+import com.amazonaws.services.s3.model.S3ObjectSummary;
+import com.amazonaws.services.s3.model.SetBucketVersioningConfigurationRequest;
+import com.cloud.agent.api.to.BucketTO;
+import com.cloud.storage.BucketVO;
+import com.cloud.storage.dao.BucketDao;
+import com.cloud.user.AccountDetailsDao;
+import com.cloud.user.AccountVO;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.Silent.class)
+public class SeaweedFSObjectStoreDriverImplTest {
+
+ @Spy
+ SeaweedFSObjectStoreDriverImpl driver = new
SeaweedFSObjectStoreDriverImpl();
+
+ @Mock
+ AmazonS3 s3Client;
+ @Mock
+ AmazonIdentityManagement iamClient;
+ @Mock
+ ObjectStoreDao objectStoreDao;
+ @Mock
+ ObjectStoreVO objectStoreVO;
+ @Mock
+ ObjectStoreDetailsDao objectStoreDetailsDao;
+ @Mock
+ AccountDao accountDao;
+ @Mock
+ BucketDao bucketDao;
+ @Mock
+ AccountDetailsDao accountDetailsDao;
+ @Mock
+ AccountVO account;
+
+ BucketVO bucketVo;
+ Map<String, String> storeDetailsMap;
+ Map<String, String> accountDetailsMap;
+
+ static long TEST_STORE_ID = 1010L;
+ static long TEST_ACCOUNT_ID = 2010L;
+ static long TEST_DOMAIN_ID = 3010L;
+ static String TEST_ACCESS_KEY = "test_access_key";
+ static String TEST_SECRET_KEY = "test_secret_key";
+ static String TEST_BUCKET_NAME = "testbucketname";
+ static String TEST_S3_URL = "http://s3-endpoint";;
+ static String TEST_IAM_URL = "http://iam-endpoint";;
+ static String TEST_AK = "user_access_key";
+ static String TEST_SK = "user_secret_key";
+ static String TEST_BUCKET_URL = TEST_S3_URL + "/" + TEST_BUCKET_NAME;
+ static String TEST_ACCOUNT_UUID = "account-uuid-1234";
+
+ private AutoCloseable closeable;
+
+ @Before
+ public void setUp() {
+ closeable = MockitoAnnotations.openMocks(this);
+ driver._storeDao = objectStoreDao;
+ driver._storeDetailsDao = objectStoreDetailsDao;
+ driver._accountDao = accountDao;
+ driver._bucketDao = bucketDao;
+ driver._accountDetailsDao = accountDetailsDao;
+
+
lenient().when(objectStoreDao.findById(TEST_STORE_ID)).thenReturn(objectStoreVO);
+ lenient().when(objectStoreVO.getUrl()).thenReturn(TEST_S3_URL);
+
+ storeDetailsMap = new HashMap<>();
+
storeDetailsMap.put(SeaweedFSObjectStoreUtil.STORE_DETAILS_KEY_ACCESS_KEY,
TEST_ACCESS_KEY);
+
storeDetailsMap.put(SeaweedFSObjectStoreUtil.STORE_DETAILS_KEY_SECRET_KEY,
TEST_SECRET_KEY);
+ storeDetailsMap.put(SeaweedFSObjectStoreUtil.STORE_DETAILS_KEY_S3_URL,
TEST_S3_URL);
+
storeDetailsMap.put(SeaweedFSObjectStoreUtil.STORE_DETAILS_KEY_IAM_URL,
TEST_IAM_URL);
+
lenient().when(objectStoreDetailsDao.getDetails(TEST_STORE_ID)).thenReturn(storeDetailsMap);
+
+ accountDetailsMap = new HashMap<>();
+ accountDetailsMap.put(SeaweedFSObjectStoreUtil.KEY_ACCESS_KEY,
TEST_AK);
+ accountDetailsMap.put(SeaweedFSObjectStoreUtil.KEY_SECRET_KEY,
TEST_SK);
+
lenient().when(accountDetailsDao.findDetails(TEST_ACCOUNT_ID)).thenReturn(accountDetailsMap);
+
+ bucketVo = new BucketVO(TEST_ACCOUNT_ID, TEST_DOMAIN_ID,
TEST_STORE_ID, TEST_BUCKET_NAME, null, false, false, false, null);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ closeable.close();
+ }
+
+ @Test
+ public void testGetStoreTO() {
+ assertNull(driver.getStoreTO(null));
+ }
+
+ @Test
+ public void testCreateBucket() throws Exception {
+ doReturn(s3Client).when(driver).getS3ClientByStoreId(TEST_STORE_ID);
+ when(s3Client.doesBucketExistV2(TEST_BUCKET_NAME)).thenReturn(false);
+ when(bucketDao.findById(anyLong())).thenReturn(bucketVo);
+
+ Bucket result = driver.createBucket(bucketVo, false);
+
+ assertEquals(TEST_BUCKET_NAME, result.getName());
+
+ ArgumentCaptor<BucketVO> captor =
ArgumentCaptor.forClass(BucketVO.class);
+ verify(bucketDao, times(1)).update(any(), captor.capture());
+ BucketVO updated = captor.getValue();
+ assertEquals(TEST_AK, updated.getAccessKey());
+ assertEquals(TEST_SK, updated.getSecretKey());
+ assertEquals(TEST_BUCKET_URL, updated.getBucketURL());
+
+ verify(s3Client,
times(1)).createBucket(any(CreateBucketRequest.class));
+ }
+
+ @Test
+ public void testCreateBucketAlreadyExists() throws Exception {
+ doReturn(s3Client).when(driver).getS3ClientByStoreId(TEST_STORE_ID);
+ when(s3Client.doesBucketExistV2(TEST_BUCKET_NAME)).thenReturn(true);
+
+ assertThrows(CloudRuntimeException.class, () ->
driver.createBucket(bucketVo, false));
+ verify(s3Client, never()).createBucket(any(CreateBucketRequest.class));
+ }
+
+ @Test
+ public void testListBuckets() throws Exception {
+ doReturn(s3Client).when(driver).getS3ClientByStoreId(TEST_STORE_ID);
+ List<com.amazonaws.services.s3.model.Bucket> s3Buckets = new
ArrayList<>();
+ s3Buckets.add(new com.amazonaws.services.s3.model.Bucket("bucket1"));
+ s3Buckets.add(new com.amazonaws.services.s3.model.Bucket("bucket2"));
+ when(s3Client.listBuckets()).thenReturn(s3Buckets);
+
+ List<Bucket> result = driver.listBuckets(TEST_STORE_ID);
+
+ assertEquals(2, result.size());
+ assertEquals("bucket1", result.get(0).getName());
+ assertEquals("bucket2", result.get(1).getName());
+ }
+
+ @Test
+ public void testDeleteBucket() throws Exception {
+ doReturn(s3Client).when(driver).getS3ClientByStoreId(TEST_STORE_ID);
+ BucketTO bucketTO = mock(BucketTO.class);
+ when(bucketTO.getName()).thenReturn(TEST_BUCKET_NAME);
+ when(s3Client.doesBucketExistV2(TEST_BUCKET_NAME)).thenReturn(true);
+
+ assertTrue(driver.deleteBucket(bucketTO, TEST_STORE_ID));
+ verify(s3Client, times(1)).deleteBucket(TEST_BUCKET_NAME);
+ }
+
+ @Test
+ public void testDeleteBucketNotFound() throws Exception {
+ doReturn(s3Client).when(driver).getS3ClientByStoreId(TEST_STORE_ID);
+ BucketTO bucketTO = mock(BucketTO.class);
+ when(bucketTO.getName()).thenReturn(TEST_BUCKET_NAME);
+ when(s3Client.doesBucketExistV2(TEST_BUCKET_NAME)).thenReturn(false);
+
+ assertThrows(CloudRuntimeException.class, () ->
driver.deleteBucket(bucketTO, TEST_STORE_ID));
+ }
+
+ @Test
+ public void testSetBucketVersioning() throws Exception {
+ doReturn(s3Client).when(driver).getS3ClientByStoreId(TEST_STORE_ID);
+ BucketTO bucketTO = mock(BucketTO.class);
+ when(bucketTO.getName()).thenReturn(TEST_BUCKET_NAME);
+
+ assertTrue(driver.setBucketVersioning(bucketTO, TEST_STORE_ID));
+ verify(s3Client,
times(1)).setBucketVersioningConfiguration(any(SetBucketVersioningConfigurationRequest.class));
+ }
+
+ @Test
+ public void testDeleteBucketVersioning() throws Exception {
+ doReturn(s3Client).when(driver).getS3ClientByStoreId(TEST_STORE_ID);
+ BucketTO bucketTO = mock(BucketTO.class);
+ when(bucketTO.getName()).thenReturn(TEST_BUCKET_NAME);
+
+ assertTrue(driver.deleteBucketVersioning(bucketTO, TEST_STORE_ID));
+ ArgumentCaptor<SetBucketVersioningConfigurationRequest> captor =
+
ArgumentCaptor.forClass(SetBucketVersioningConfigurationRequest.class);
+ verify(s3Client,
times(1)).setBucketVersioningConfiguration(captor.capture());
+ assertEquals(BucketVersioningConfiguration.SUSPENDED,
captor.getValue().getVersioningConfiguration().getStatus());
+ }
+
+ @Test
+ public void testSetBucketQuotaZero() throws Exception {
+ BucketTO bucketTO = mock(BucketTO.class);
+ when(bucketTO.getName()).thenReturn(TEST_BUCKET_NAME);
+ // Mock the S3 helpers to return valid values
+ doReturn("http://s3-endpoint";).when(driver).getS3Url(TEST_STORE_ID);
+ doReturn("access-key").when(driver).getAccessKey(TEST_STORE_ID);
+ doReturn("secret-key").when(driver).getSecretKey(TEST_STORE_ID);
+ // Should not throw for 0 — uses static method, can't easily mock, but
+ // the test validates the code path doesn't throw before the HTTP call
+ // Since we can't mock the static HTTP call, we expect a
CloudRuntimeException
+ // from the HTTP call failing (no real server). That's acceptable — it
proves
+ // the code path reaches the S3 extension rather than throwing "not
supported".
+ assertThrows(CloudRuntimeException.class, () ->
driver.setBucketQuota(bucketTO, TEST_STORE_ID, 0));
Review Comment:
These quota tests call the real `java.net.http.HttpClient` against
`http://s3-endpoint` and merely assert `CloudRuntimeException`. They are
network/DNS-dependent and would still pass if signing, the request body, or the
response handling were broken. Use WireMock or inject the HTTP sender and
assert the path, signed headers, and JSON body.
This issue also appears on line 279 of the same file.
##########
plugins/storage/object/seaweedfs/src/main/java/org/apache/cloudstack/storage/datastore/util/SeaweedFSObjectStoreUtil.java:
##########
@@ -0,0 +1,285 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+// SPDX-License-Identifier: Apache-2.0
+package org.apache.cloudstack.storage.datastore.util;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.amazonaws.AmazonServiceException;
+import com.amazonaws.auth.AWSStaticCredentialsProvider;
+import com.amazonaws.auth.BasicAWSCredentials;
+import com.amazonaws.client.builder.AwsClientBuilder;
+import com.amazonaws.services.identitymanagement.AmazonIdentityManagement;
+import
com.amazonaws.services.identitymanagement.AmazonIdentityManagementClientBuilder;
+import com.amazonaws.services.s3.AmazonS3;
+import com.amazonaws.services.s3.AmazonS3ClientBuilder;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+/**
+ * Utility class for the SeaweedFS object storage provider.
+ *
+ * SeaweedFS exposes both an S3-compatible API and an AWS IAM-compatible API,
+ * so this provider needs no proprietary admin client — only the AWS S3 and IAM
+ * SDKs, the same pair Cloudian HyperStore already uses in this tree.
+ */
+public class SeaweedFSObjectStoreUtil {
+
+ /** The name of our Object Store Provider */
+ public static final String OBJECT_STORE_PROVIDER_NAME = "SeaweedFS";
+
+ public static final String STORE_KEY_PROVIDER_NAME = "providerName";
+ public static final String STORE_KEY_URL = "url";
+ public static final String STORE_KEY_NAME = "name";
+ public static final String STORE_KEY_DETAILS = "details";
+
+ // Store Details Map key names - managed outside of plugin
+ public static final String STORE_DETAILS_KEY_ACCESS_KEY = "accesskey";
// admin/root access key
+ public static final String STORE_DETAILS_KEY_SECRET_KEY = "secretkey";
// admin/root secret key
+ public static final String STORE_DETAILS_KEY_S3_URL = "s3Url";
// S3 endpoint URL
+ public static final String STORE_DETAILS_KEY_IAM_URL = "iamUrl";
// IAM endpoint URL
+
+ // Account Detail Map key names - credentials created per CloudStack
account
+ public static final String KEY_ACCESS_KEY = "swfs_AccessKey";
+ public static final String KEY_SECRET_KEY = "swfs_SecretKey";
+
+ /**
+ * IAM user policy applied to each per-account IAM user. Grants full S3
+ * access except bucket creation/deletion, so CloudStack retains control of
+ * bucket lifecycle while the account's IAM credentials can manage objects.
+ */
+ public static final String IAM_USER_POLICY = "{\n" +
+ " \"Version\": \"2012-10-17\",\n" +
+ " \"Statement\": [\n" +
+ " {\n" +
+ " \"Sid\": \"AllowFullS3Access\",\n" +
+ " \"Effect\": \"Allow\",\n" +
+ " \"Action\": [\n" +
+ " \"s3:*\"\n" +
+ " ],\n" +
+ " \"Resource\": \"*\"\n" +
+ " },\n" +
+ " {\n" +
+ " \"Sid\": \"ExceptBucketCreationOrDeletion\",\n" +
+ " \"Effect\": \"Deny\",\n" +
+ " \"Action\": [\n" +
+ " \"s3:CreateBucket\",\n" +
+ " \"s3:DeleteBucket\"\n" +
+ " ],\n" +
+ " \"Resource\": \"*\"\n" +
+ " }\n" +
+ " ]\n" +
+ "}\n";
+
+ // The CloudStack service credential (the accesskey/secretkey configured on
+ // the object store) is the admin credential used for ALL driver
operations:
+ // - AmazonS3 client: bucket CRUD, policy, versioning, encryption,
listing
+ // - AmazonIdentityManagement client: per-account IAM user provisioning
+ // - setBucketQuotaViaS3Extension: PUT /{bucket}?seaweedfs-quota
+ // It must therefore have broad S3 and IAM permissions. It is NOT scoped
+ // down to only s3:PutBucketQuota/s3:GetBucketQuota — that was an earlier
+ // design idea that does not match the implementation. The per-account IAM
+ // users (created by createUser) are the ones with restricted permissions
+ // (see IAM_USER_POLICY above).
+
+ /**
+ * Returns an S3 connection for the given endpoint and credentials.
+ * Uses path-style access, which SeaweedFS requires.
+ *
+ * @param url the url of the S3 service
+ * @param accessKey the credentials to use for the S3 connection.
+ * @param secretKey the matching secret key.
+ * @return an S3 connection (never null)
+ * @throws CloudRuntimeException on failure.
+ */
+ public static AmazonS3 getS3Client(String url, String accessKey, String
secretKey) {
+ AmazonS3 client = AmazonS3ClientBuilder.standard()
+ .enablePathStyleAccess()
+ .withCredentials(new AWSStaticCredentialsProvider(new
BasicAWSCredentials(accessKey, secretKey)))
+ .withEndpointConfiguration(new
AwsClientBuilder.EndpointConfiguration(url, "us-east-1"))
+ .build();
+ if (client == null) {
+ throw new CloudRuntimeException("Error while creating SeaweedFS S3
client");
+ }
+ return client;
+ }
+
+ /**
+ * Returns an IAM connection for the given endpoint and credentials.
+ *
+ * @param url the url of the IAM service
+ * @param accessKey the credentials to use for the iam connection.
+ * @param secretKey the matching secret key.
+ * @return an IAM connection (never null)
+ * @throws CloudRuntimeException on failure.
+ */
+ public static AmazonIdentityManagement getIAMClient(String url, String
accessKey, String secretKey) {
+ AmazonIdentityManagement iamClient =
AmazonIdentityManagementClientBuilder.standard()
+ .withCredentials(new AWSStaticCredentialsProvider(new
BasicAWSCredentials(accessKey, secretKey)))
+ .withEndpointConfiguration(new
AwsClientBuilder.EndpointConfiguration(url, "us-east-1"))
+ .build();
+ if (iamClient == null) {
+ throw new CloudRuntimeException("Error while creating SeaweedFS
IAM client");
+ }
+ return iamClient;
+ }
+
+ /**
+ * Test the S3Url to confirm it behaves like an S3 Service.
+ *
+ * Uses bad credentials and looks for the particular error from S3 that
says
+ * InvalidAccessKeyId was used. Quietly returns if we connect and get the
+ * expected error back.
+ *
+ * @param s3Url the url to check
+ * @throws CloudRuntimeException if there is any unexpected issue.
+ */
+ public static void validateS3Url(String s3Url) {
+ try {
+ AmazonS3 s3Client = SeaweedFSObjectStoreUtil.getS3Client(s3Url,
"unknown", "unknown");
+ s3Client.listBuckets();
+ } catch (AmazonServiceException e) {
+ if (StringUtils.compareIgnoreCase(e.getErrorCode(),
"InvalidAccessKeyId") != 0
+ && StringUtils.compareIgnoreCase(e.getErrorCode(),
"SignatureDoesNotMatch") != 0) {
+ throw new CloudRuntimeException("Unexpected response from S3
Endpoint.", e);
+ }
+ }
+ }
+
+ /**
+ * Test the IAMUrl to confirm it behaves like an IAM Service.
+ *
+ * Uses bad credentials and looks for the particular error from IAM that
says
+ * InvalidAccessKeyId or InvalidClientTokenId was used. Quietly returns if
we
+ * connect and get the expected error back.
+ *
+ * @param iamUrl the url to check
+ * @throws CloudRuntimeException if there is any unexpected issue.
+ */
+ public static void validateIAMUrl(String iamUrl) {
+ try {
+ AmazonIdentityManagement iamClient =
SeaweedFSObjectStoreUtil.getIAMClient(iamUrl, "unknown", "unknown");
+ iamClient.listAccessKeys();
+ } catch (AmazonServiceException e) {
+ if (! StringUtils.equalsAnyIgnoreCase(e.getErrorCode(),
"InvalidAccessKeyId", "InvalidClientTokenId", "SignatureDoesNotMatch")) {
+ throw new CloudRuntimeException("Unexpected response from IAM
Endpoint.", e);
+ }
+ }
+ }
+
+ /**
+ * Set bucket quota via the SeaweedFS S3 extension endpoint.
+ *
+ * SeaweedFS exposes a custom S3 subresource at
+ * PUT /{bucket}?seaweedfs-quota
+ * authenticated via standard S3 SigV4 and authorized via the
+ * s3:PutBucketQuota IAM permission. This avoids the need for a
+ * separate admin API credential.
+ *
+ * The request body is JSON:
+ * {"quota_size": <n>, "quota_unit": "GB", "quota_enabled": true}
+ *
+ * @param s3Url the S3 endpoint URL (e.g. http://host:8333)
+ * @param accessKey the S3 access key (must have s3:PutBucketQuota
permission)
+ * @param secretKey the S3 secret key
+ * @param bucketName the bucket name
+ * @param sizeGiB the quota size in GiB (0 to disable quota)
+ * @throws CloudRuntimeException on any failure
+ */
+ public static void setBucketQuotaViaS3Extension(String s3Url, String
accessKey, String secretKey, String bucketName, long sizeGiB) {
+ String body;
+ if (sizeGiB <= 0) {
+ body =
"{\"quota_size\":0,\"quota_unit\":\"B\",\"quota_enabled\":false}";
+ } else {
+ body =
String.format("{\"quota_size\":%d,\"quota_unit\":\"GB\",\"quota_enabled\":true}",
sizeGiB);
+ }
+ executeSignedS3Request("PUT", s3Url, "/" + bucketName +
"?seaweedfs-quota", accessKey, secretKey, body);
+ }
+
+ /**
+ * Execute a custom S3 request with SigV4 signing.
+ *
+ * Uses the AWS SDK v1 Aws4Signer to sign the request, then sends it via
+ * java.net.http.HttpClient. This allows calling SeaweedFS-specific S3
+ * extensions (like ?seaweedfs-quota) that the AWS SDK doesn't natively
+ * support.
+ *
+ * @param method HTTP method (PUT, GET, etc.)
+ * @param s3Url the S3 endpoint base URL
+ * @param resourcePath the path + query string (e.g.
/bucket?seaweedfs-quota)
+ * @param accessKey S3 access key
+ * @param secretKey S3 secret key
+ * @param body the request body (null for GET)
+ * @return the response body as a string
+ * @throws CloudRuntimeException on any failure
+ */
+ private static String executeSignedS3Request(String method, String s3Url,
String resourcePath,
+ String accessKey, String
secretKey, String body) {
+ try {
+ java.net.URI endpointUri = java.net.URI.create(s3Url);
+ java.net.URL endpointUrl = endpointUri.toURL();
+
+ // Build AWS SDK v1 Request for SigV4 signing
+ com.amazonaws.DefaultRequest<?> request = new
com.amazonaws.DefaultRequest<>("s3");
+ request.setEndpoint(endpointUri);
+
request.setHttpMethod(com.amazonaws.http.HttpMethodName.valueOf(method));
+ request.setResourcePath(resourcePath);
Review Comment:
`DefaultRequest` keeps query parameters separately; `setResourcePath` does
not parse the `?seaweedfs-quota` suffix. The signer therefore does not sign
this as a canonical query parameter, while `fullUri` sends it as one, so
SeaweedFS can reject the request with a signature mismatch. Split the path and
add `seaweedfs-quota` through `request.addParameter(...)` before signing.
This issue also appears in the following locations of the same file:
- line 258
- line 269
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
