Copilot commented on code in PR #14160: URL: https://github.com/apache/cloudstack/pull/14160#discussion_r4283950572
########## plugins/storage/object/seaweedfs/src/main/java/org/apache/cloudstack/storage/datastore/driver/SeaweedFSObjectStoreDriverImpl.java: ########## @@ -0,0 +1,1125 @@ +/* + * 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.AccessKeyMetadata; +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.DeleteAccessKeyRequest; +import com.amazonaws.services.identitymanagement.model.EntityAlreadyExistsException; +import com.amazonaws.services.identitymanagement.model.ListAccessKeysRequest; +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.BucketCrossOriginConfiguration; +import com.amazonaws.services.s3.model.CORSRule; +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.SetBucketCrossOriginConfigurationRequest; +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.db.GlobalLock; +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"; + + /** + * DB-backed global lock name prefix for serializing IAM provisioning and + * policy refreshes per store+account. Uses {@link GlobalLock} so the + * critical section is serialized across management servers in a + * clustered deployment, not just within a single JVM. + */ + private static final String IAM_LOCK_PREFIX = "seaweedfs.iam."; + private static final String BUCKET_NAME_LOCK_PREFIX = "seaweedfs.bucket."; + + private static String getIamLockName(long storeId, long accountId) { + return IAM_LOCK_PREFIX + storeId + "." + accountId; + } + + /** + * Acquire a DB-backed global lock for IAM operations on the given + * store+account. Returns a {@link GlobalLock} that the caller must + * {@link GlobalLock#unlock()} in a {@code finally} block, or {@code null} + * if the lock could not be acquired within the timeout. + * + * <p>Protected so tests can override with a no-op lock (the DB-backed + * {@link GlobalLock} requires a real transaction context). + */ + protected GlobalLock acquireIamLock(long storeId, long accountId) { + GlobalLock lock = GlobalLock.getInternLock(getIamLockName(storeId, accountId)); + if (!lock.lock(300)) { + logger.warn("Failed to acquire IAM lock for store {} account {}", storeId, accountId); + lock.releaseRef(); + return null; + } + return lock; + } + + private static String getBucketNameLockName(long storeId, String bucketName) { + return BUCKET_NAME_LOCK_PREFIX + storeId + "." + bucketName; + } + + /** + * Acquire a DB-backed global lock covering a bucket <em>name</em> on a store, + * independent of the owning account. + * + * S3 bucket names are globally unique within a store and are reusable after + * deletion, while the IAM lock is scoped to (store, account). Without this + * lock, once a delete removes the remote bucket but before the old owner's + * IAM policy is refreshed, a different account can recreate the same name and + * the old owner's credentials would still grant that ARN. Both createBucket + * and deleteBucket take this lock so the two never interleave for a name. + * + * <p>Protected so tests can override with a no-op lock (the DB-backed + * {@link GlobalLock} requires a real transaction context). + * + * @return the held lock, which the caller must {@link GlobalLock#unlock()} + * and {@link GlobalLock#releaseRef()}, or {@code null} on timeout + */ + protected GlobalLock acquireBucketNameLock(long storeId, String bucketName) { + GlobalLock lock = GlobalLock.getInternLock(getBucketNameLockName(storeId, bucketName)); + if (!lock.lock(300)) { + logger.warn("Failed to acquire bucket name lock for store {} bucket {}", storeId, bucketName); + lock.releaseRef(); + return null; + } + return lock; + } + + @Override + public DataStoreTO getStoreTO(DataStore store) { + return null; + } + + /** + * Get the SeaweedFS IAM user name for the given CloudStack account and + * store. The store ID is included so that two CloudStack pools pointing + * at the same SeaweedFS IAM service do not collide on the same + * {@code acs-<uuid>} user and overwrite each other's policy and access + * keys. + */ + protected String getUserNameForAccount(Account account, long storeId) { + return String.format("%s-%d-%s", ACS_PREFIX, storeId, account.getUuid()); + } + + /** + * Create the IAM user for the CloudStack account if it doesn't exist, + * attach the restricted S3 policy, and ensure the account has a usable + * IAM access key persisted in its account details. + * + * <p>If a previously stored access key is still present in IAM, it is + * reused rather than rotated. A new key is only created when no stored + * key exists or the stored key is no longer found in IAM; in the latter + * case any unmanaged (leftover) keys for the user are deleted first to + * avoid hitting IAM access-key limits. This keeps bucket records that + * reference the stored credentials valid across repeated calls. + * + * @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, storeId); + AmazonIdentityManagement iamClient = getIAMClient(storeId); + + // Serialize per store+account across management servers so two + // concurrent bucket requests do not both rotate credentials and leave + // bucket rows with mismatched key pairs. + GlobalLock lock = acquireIamLock(storeId, accountId); + if (lock == null) { + return false; + } + try { + + // 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 a scoped IAM policy that allows access only to this + // account's own buckets (the tenant boundary). Refreshed whenever + // buckets are created or deleted. Use the lock-free variant since + // createUser already holds the IAM lock. + updateAccountIAMPolicyLocked(iamClient, storeId, accountId, null); + + // Reuse the stored access key only if both the access key id and the + // secret key are present and the key is still Active in IAM; otherwise + // create a replacement. + Map<String, String> details = _accountDetailsDao.findDetails(accountId); + String accessKeyDetailKey = SeaweedFSObjectStoreUtil.keyAccessKey(storeId); + String secretKeyDetailKey = SeaweedFSObjectStoreUtil.keySecretKey(storeId); + String storedAccessKeyId = details.get(accessKeyDetailKey); + String storedSecretKey = details.get(secretKeyDetailKey); + if (storedAccessKeyId != null && storedSecretKey != null + && iamAccessKeyExists(iamClient, userName, storedAccessKeyId)) { + logger.debug("Reusing existing IAM access key {} for user {}", storedAccessKeyId, userName); + updateAccountBucketCredentials(storeId, accountId, storedAccessKeyId, storedSecretKey); + return true; + } + + // The stored key is missing, inactive, or no longer in IAM. Clean up + // ALL keys (including the inactive stored one) before creating a + // replacement so we do not accumulate keys and hit IAM limits. + deleteUnmanagedAccessKeys(iamClient, userName, null); + + CreateAccessKeyResult result = iamClient.createAccessKey( + new CreateAccessKeyRequest().withUserName(userName)); + AccessKey key = result.getAccessKey(); + + // Persist the credential pair in account details (namespaced by storeId) + // before updating BucketVO rows. The reuse path above reconciles bucket + // rows every time, so a later bucket update failure is repairable on + // retry. AccountDetailsDao.persist(accountId, map) is deliberately not + // used: it expunges every detail for the account and can clobber another + // store's namespaced credentials. + try { + persistAccountCredentialsOrRollback(accountId, accessKeyDetailKey, secretKeyDetailKey, + storedAccessKeyId, storedSecretKey, key); + } catch (RuntimeException e) { + deleteAccessKeyAfterCredentialPersistenceFailure(iamClient, userName, key.getAccessKeyId(), e); + throw e; + } + + updateAccountBucketCredentials(storeId, accountId, key); + + logger.info("Created IAM credentials {} for user {}", key.getAccessKeyId(), userName); + return true; + } finally { + lock.unlock(); + lock.releaseRef(); + } + } + + /** + * Persist a SeaweedFS IAM credential pair without using + * AccountDetailsDao.persist(accountId, map), and restore the previous pair if + * either single-key write fails so callers never observe a half-new pair. + */ + private void persistAccountCredentialsOrRollback(long accountId, String accessKeyDetailKey, String secretKeyDetailKey, + String previousAccessKey, String previousSecretKey, AccessKey key) { + try { + _accountDetailsDao.addDetail(accountId, accessKeyDetailKey, key.getAccessKeyId(), false); + _accountDetailsDao.addDetail(accountId, secretKeyDetailKey, key.getSecretAccessKey(), false); + } catch (RuntimeException e) { + CloudRuntimeException wrapped = new CloudRuntimeException("Failed to persist SeaweedFS IAM credential pair for account " + accountId, e); + restoreAccountCredentialDetail(accountId, accessKeyDetailKey, previousAccessKey, wrapped); + restoreAccountCredentialDetail(accountId, secretKeyDetailKey, previousSecretKey, wrapped); + throw wrapped; + } + } + + private void restoreAccountCredentialDetail(long accountId, String detailKey, String previousValue, RuntimeException cause) { + try { + if (previousValue == null) { + _accountDetailsDao.removeDetail(accountId, detailKey); + } else { + _accountDetailsDao.addDetail(accountId, detailKey, previousValue, false); + } + } catch (RuntimeException rollbackEx) { + logger.error("Failed to restore account detail {} for account {} after SeaweedFS credential persistence failed", + detailKey, accountId, rollbackEx); + cause.addSuppressed(rollbackEx); + } + } + + private void deleteAccessKeyAfterCredentialPersistenceFailure(AmazonIdentityManagement iamClient, String userName, + String accessKeyId, RuntimeException cause) { + try { + iamClient.deleteAccessKey(new DeleteAccessKeyRequest() + .withUserName(userName) + .withAccessKeyId(accessKeyId)); + } catch (AmazonClientException cleanupEx) { + logger.error("Failed to delete IAM access key {} for user {} after account credential persistence failed", + accessKeyId, userName, cleanupEx); + cause.addSuppressed(cleanupEx); + } + } + + private void updateAccountBucketCredentials(long storeId, long accountId, AccessKey iamCredential) { + updateAccountBucketCredentials(storeId, accountId, iamCredential.getAccessKeyId(), iamCredential.getSecretAccessKey()); + } + + /** + * Update the IAM credentials on all BucketVO rows for this store/account so + * previously created buckets reflect the current key pair. + */ + private void updateAccountBucketCredentials(long storeId, long accountId, String accessKeyId, String secretAccessKey) { + List<BucketVO> bucketList = _bucketDao.listByObjectStoreIdAndAccountId(storeId, accountId); + for (BucketVO bucketVO : bucketList) { + if (accessKeyId.equals(bucketVO.getAccessKey()) && secretAccessKey.equals(bucketVO.getSecretKey())) { + continue; + } + logger.info("Updating accountId={} bucket {} with new IAM credentials", accountId, bucketVO.getName()); + bucketVO.setAccessKey(accessKeyId); + bucketVO.setSecretKey(secretAccessKey); + if (!_bucketDao.update(bucketVO.getId(), bucketVO)) { + throw new CloudRuntimeException("Failed to update IAM credentials on bucket " + bucketVO.getName()); + } + } + } + + /** + * Refresh the per-account IAM user policy so it grants S3 access only to + * the account's current buckets (optionally excluding one, e.g. a bucket + * being deleted). This is the tenant boundary: each account's IAM + * credentials can only operate on that account's own buckets. + * + * Acquires the per-store/account IAM lock. Callers that already hold the + * lock (e.g. createUser, createBucket post-create) should call + * {@link #updateAccountIAMPolicyLocked} instead to avoid re-entrant lock + * acquisition warnings from GlobalLock. + * + * @param iamClient the IAM client + * @param storeId the object store + * @param accountId the CloudStack account + * @param excludeBucket a bucket name to omit (e.g. a bucket being deleted), + * or null to include all of the account's buckets + */ + protected void updateAccountIAMPolicy(AmazonIdentityManagement iamClient, long storeId, long accountId, String excludeBucket) { + GlobalLock lock = acquireIamLock(storeId, accountId); + if (lock == null) { + throw new CloudRuntimeException("Failed to acquire IAM lock for store " + storeId + " account " + accountId); + } + try { + updateAccountIAMPolicyLocked(iamClient, storeId, accountId, excludeBucket); + } finally { + lock.unlock(); + lock.releaseRef(); + } + } + + /** + * Lock-free variant of {@link #updateAccountIAMPolicy} for callers that + * already hold the per-store/account IAM lock. Performs the policy refresh + * without reacquiring the lock, avoiding the GlobalLock re-entrant + * acquisition warning. + */ + protected void updateAccountIAMPolicyLocked(AmazonIdentityManagement iamClient, long storeId, long accountId, String excludeBucket) { + Account account = _accountDao.findById(accountId); + if (account == null) { + return; + } + String userName = getUserNameForAccount(account, storeId); + List<BucketVO> buckets = _bucketDao.listByObjectStoreIdAndAccountId(storeId, accountId); + List<String> bucketNames = new ArrayList<>(); + for (BucketVO bvo : buckets) { + if (excludeBucket != null && excludeBucket.equals(bvo.getName())) { + continue; + } + // Skip buckets whose remote counterpart has been deleted but whose + // row is still present for resource accounting. Including them + // would re-grant a bucket name that is now free for another account + // to claim. + if (Bucket.State.Destroyed.equals(bvo.getState())) { + continue; + } + bucketNames.add(bvo.getName()); + } + String policy = SeaweedFSObjectStoreUtil.buildAccountIAMPolicy(bucketNames); + iamClient.putUserPolicy(new PutUserPolicyRequest(userName, + SeaweedFSObjectStoreUtil.IAM_USER_POLICY_NAME, policy)); + } + + /** + * Check whether the given access key id is still listed and Active in IAM + * for the user. Listing failures are propagated rather than swallowed so + * a transient IAM outage does not send createUser into the replacement + * path (which would overwrite stored credentials and invalidate bucket + * records). + */ + private boolean iamAccessKeyExists(AmazonIdentityManagement iamClient, String userName, String accessKeyId) { + for (AccessKeyMetadata metadata : + iamClient.listAccessKeys(new ListAccessKeysRequest() + .withUserName(userName)).getAccessKeyMetadata()) { + if (accessKeyId.equals(metadata.getAccessKeyId())) { + return "Active".equalsIgnoreCase(metadata.getStatus()); + } + } + return false; + } + + /** + * Delete access keys for the user other than the (optionally) preserved + * key id. Used to clean up unmanaged leftover keys before creating a + * replacement so repeated calls do not hit IAM access-key limits. + */ + private void deleteUnmanagedAccessKeys(AmazonIdentityManagement iamClient, String userName, String preserveAccessKeyId) { + try { + for (AccessKeyMetadata metadata : + iamClient.listAccessKeys(new ListAccessKeysRequest() + .withUserName(userName)).getAccessKeyMetadata()) { + String keyId = metadata.getAccessKeyId(); + if (preserveAccessKeyId != null && preserveAccessKeyId.equals(keyId)) { + continue; + } + DeleteAccessKeyRequest deleteReq = + new DeleteAccessKeyRequest() + .withUserName(userName) + .withAccessKeyId(keyId); + logger.info("Deleting un-managed IAM access key {} for user {}", keyId, userName); + iamClient.deleteAccessKey(deleteReq); + } + } catch (AmazonClientException e) { + // Propagate so the caller does not proceed to create a replacement + // key while stale unmanaged keys remain (which could hit IAM key + // limits or leave orphaned credentials). + throw new CloudRuntimeException("Failed to clean up IAM access keys for user " + userName, e); + } + } + + @Override + public Bucket createBucket(Bucket bucket, boolean objectLock) { + String bucketName = bucket.getName(); + long storeId = bucket.getObjectStoreId(); + + // Serialize against a concurrent deleteBucket of the same name by any + // account on this store. Bucket names are globally unique per store and + // reusable, so without this an account could claim a name while the + // previous owner's IAM policy still granted that ARN. + GlobalLock nameLock = acquireBucketNameLock(storeId, bucketName); + if (nameLock == null) { + throw new CloudRuntimeException("Failed to acquire bucket name lock for store " + storeId + " bucket " + bucketName); + } + try { + return createBucketLocked(bucket, objectLock); + } finally { + nameLock.unlock(); + nameLock.releaseRef(); + } + } + + private Bucket createBucketLocked(Bucket bucket, boolean objectLock) { + String bucketName = bucket.getName(); + long storeId = bucket.getObjectStoreId(); + long accountId = bucket.getAccountId(); + + // Use the store's admin credentials to create the bucket + AmazonS3 s3client = getS3ClientByStoreId(storeId); + + // Check if the bucket already exists + try { + if (s3client.doesBucketExistV2(bucketName)) { + throw new CloudRuntimeException("Bucket already exists with name " + bucketName); + } + } catch (AmazonClientException e) { + throw new CloudRuntimeException(e); + } + + // Create the bucket + try { + CreateBucketRequest request = new CreateBucketRequest(bucketName); + if (objectLock) { + request.setObjectLockEnabledForBucket(true); + } + s3client.createBucket(request); + } catch (AmazonClientException e) { + logger.error("Create bucket failed", e); + throw new CloudRuntimeException(e); + } + + // Step 2: update the bucket record with the account's IAM credentials, + // configure CORS, and refresh the IAM policy. If any of these fail, + // clean up the remote bucket so a retry does not find it already + // existing — mirroring the Cloudian createBucket pattern. + // + // Hold the IAM lock for the account so a concurrent createUser key + // rotation does not change the account credentials between reading + // them and writing the BucketVO, which would leave the bucket with + // a stale key pair. + GlobalLock iamLock = acquireIamLock(storeId, accountId); + if (iamLock == null) { + // The S3 bucket has already been created. Clean it up so a + // retry does not find it already existing, then throw. + try { + s3client.deleteBucket(bucketName); + } catch (AmazonClientException cleanupEx) { + logger.error("Failed to clean up bucket {} after IAM lock timeout", bucketName, cleanupEx); + } + throw new CloudRuntimeException("Failed to acquire IAM lock for store " + storeId + " account " + accountId); + } + try { + // Configure permissive CORS so the CloudStack S3 bucket browser + // (which performs list/upload/delete from the browser) can function. + // SeaweedFS supports the standard PutBucketCors operation. + configureBucketCORS(s3client, bucketName); + + Map<String, String> accountDetails = _accountDetailsDao.findDetails(accountId); + String accessKey = accountDetails.get(SeaweedFSObjectStoreUtil.keyAccessKey(storeId)); + String secretKey = accountDetails.get(SeaweedFSObjectStoreUtil.keySecretKey(storeId)); + if (accessKey == null || secretKey == null) { + throw new CloudRuntimeException("No IAM credentials found for account " + accountId + + " on store " + storeId + ". Run createUser before creating a bucket."); + } + + String s3Url = getS3Url(storeId); + BucketVO bucketVO = _bucketDao.findById(bucket.getId()); + if (bucketVO == null) { + throw new CloudRuntimeException("Bucket record " + bucket.getId() + + " not found for bucket " + bucketName); + } + bucketVO.setAccessKey(accessKey); + bucketVO.setSecretKey(secretKey); + // Normalize the endpoint: s3Url is operator-supplied and may carry a + // trailing slash, which would persist a broken "...//bucket" URL + // that BucketResponse and the object store browser both use. + bucketVO.setBucketURL(SeaweedFSObjectStoreUtil.stripTrailingSlashes(s3Url) + "/" + bucketName); + if (!_bucketDao.update(bucket.getId(), bucketVO)) { + // A failed row update must fail the create: otherwise the IAM + // refresh below would grant the new bucket ARN while the + // returned BucketVO carries credentials/URL that were never + // persisted. The catch block revokes the policy grant and + // removes the remote bucket. + throw new CloudRuntimeException("Failed to persist credentials and URL for bucket " + bucketName); + } + + // Refresh the account's IAM policy to include the new bucket. + // Use the lock-free variant since createBucket already holds the + // IAM lock for the post-create section. + AmazonIdentityManagement iamClient = getIAMClient(storeId); + updateAccountIAMPolicyLocked(iamClient, storeId, accountId, null); + + // Return the updated BucketVO (not the stale input bucket) so + // BucketApiServiceImpl.createBucket does not overwrite the + // persisted credentials with the stale values. + return bucketVO; + } catch (Exception e) { + logger.error("Post-create bucket record update failed for {}; cleaning up", bucketName, e); + CloudRuntimeException primary = new CloudRuntimeException(e); + // Revoke the IAM policy grant for the new bucket BEFORE deleting + // the remote S3 bucket. If the policy PUT succeeded before the DB + // update failed, the grant would otherwise persist and could be + // reused if another account later creates the same bucket name. + // Use the lock-free variant since createBucket already holds the + // IAM lock. + // + // If the policy revocation fails, the remote S3 bucket is LEFT IN + // PLACE: deleting it would free the bucket name for another account + // to claim while this account's policy still grants the ARN, + // letting the old credentials reach the new tenant's bucket. The + // leftover remote bucket keeps the name occupied until an operator + // reconciles; the row is removed by the caller so a retry reports + // "bucket already exists" rather than silently re-granting access. + try { + AmazonIdentityManagement iamClient = getIAMClient(storeId); + updateAccountIAMPolicyLocked(iamClient, storeId, accountId, bucketName); + } catch (Exception policyEx) { + logger.error("Failed to revoke IAM policy for bucket {} after create failure; " + + "leaving the remote bucket in place to prevent name reuse", bucketName, policyEx); + primary.addSuppressed(policyEx); + throw primary; + } + // Policy revocation succeeded: the stale grant is gone, so it is + // safe to delete the remote bucket and free the name. + try { + s3client.deleteBucket(bucketName); + logger.info("Cleanup of bucket {} succeeded", bucketName); + } catch (AmazonClientException cleanupEx) { + logger.error("Cleanup of bucket {} also failed", bucketName, cleanupEx); + primary.addSuppressed(cleanupEx); + } + throw primary; + } finally { + iamLock.unlock(); + iamLock.releaseRef(); + } + } + + /** + * Configure a permissive CORS policy on the bucket so the CloudStack + * S3 bucket browser (which performs list/upload/delete from the + * browser) can function. Mirrors the Cloudian configureBucketCORS. + */ + private void configureBucketCORS(AmazonS3 s3client, String bucketName) { + logger.debug("Configuring CORS for bucket {}", bucketName); + List<CORSRule> corsRules = new ArrayList<>(); + CORSRule allowAnyRule = new CORSRule().withId("AllowAny"); + allowAnyRule.setAllowedOrigins("*"); + allowAnyRule.setAllowedHeaders("*"); + allowAnyRule.setAllowedMethods( + CORSRule.AllowedMethods.HEAD, + CORSRule.AllowedMethods.GET, + CORSRule.AllowedMethods.PUT, + CORSRule.AllowedMethods.POST, + CORSRule.AllowedMethods.DELETE); Review Comment: CORSRule#setAllowedOrigins`, `setAllowedHeaders`, and `setAllowedMethods` in the AWS SDK v1 APIs are list-based setters (and typically paired with `withAllowedOrigins(...)`, `withAllowedHeaders(...)`, `withAllowedMethods(...)`). As written, these calls are very likely to be a compile-time error (wrong argument types). Update this to use the SDK’s `withAllowedOrigins("*")`, `withAllowedHeaders("*")`, and `withAllowedMethods(...)` (or set list values explicitly) so the code compiles and the CORS config is applied correctly. ########## server/src/main/java/org/apache/cloudstack/storage/object/BucketApiServiceImpl.java: ########## @@ -290,61 +341,238 @@ public boolean updateBucket(UpdateBucketCmd cmd, Account caller) throws Resource objectStore.deleteBucketVersioning(bucketTO); } bucket.setVersioning(cmd.getVersioning()); + versioningApplied = true; } if (cmd.getPolicy() != null) { objectStore.setBucketPolicy(bucketTO, cmd.getPolicy()); bucket.setPolicy(cmd.getPolicy()); + policyApplied = true; } - updateBucketQuota(cmd, bucket, objectStore, objectStoreVO, bucketTO); + boolean bucketPersisted = updateBucketQuota(cmd, bucket, objectStore, objectStoreVO, bucketTO); - _bucketDao.update(bucket.getId(), bucket); + if (!bucketPersisted && !_bucketDao.update(bucket.getId(), bucket)) { + throw new CloudRuntimeException("Failed to update bucket " + bucket.getName()); + } } catch (Exception e) { - throw new CloudRuntimeException("Error while updating bucket: " +bucket.getName() +". "+e.getMessage()); + // Roll back the remote encryption/versioning/policy mutations that + // were applied before the failure (e.g. a quota reservation + // rejection) so the remote bucket and the BucketVO stay consistent. + // The quota path unwinds its own mutations internally. + rollbackBucketUpdateMutations(objectStore, bucketTO, bucket, + previousEncryption, encryptionApplied, + previousVersioning, versioningApplied, + previousPolicy, policyApplied, e); + if (e instanceof CloudRuntimeException) { + throw (CloudRuntimeException) e; + } + throw new CloudRuntimeException("Error while updating bucket: " + bucket.getName() + ". " + e.getMessage(), e); } return true; } - private void updateBucketQuota(UpdateBucketCmd cmd, BucketVO bucket, ObjectStoreEntity objectStore, ObjectStoreVO objectStoreVO, BucketTO bucketTO) throws ResourceAllocationException { + /** + * Best-effort compensation for the remote encryption, versioning, and + * policy mutations applied during {@link #updateBucket} when a later step + * fails. Each remote mutation is reverted to its pre-update value in + * reverse order; the in-memory {@link BucketVO} fields are restored too so + * a retry starts from the same state the backend is in. Failures are + * logged and attached to the original exception rather than masking it. + */ + private void rollbackBucketUpdateMutations(ObjectStoreEntity objectStore, BucketTO bucketTO, BucketVO bucket, + Boolean previousEncryption, boolean encryptionApplied, + Boolean previousVersioning, boolean versioningApplied, + String previousPolicy, boolean policyApplied, Exception cause) { + if (policyApplied) { + try { + if (previousPolicy == null || "private".equalsIgnoreCase(previousPolicy)) { + objectStore.setBucketPolicy(bucketTO, "private"); + } else { + objectStore.setBucketPolicy(bucketTO, previousPolicy); + } + bucket.setPolicy(previousPolicy); + } catch (Exception ex) { + logger.error("Failed to roll back bucket policy for {} while compensating a failed update", + bucket.getName(), ex); + cause.addSuppressed(ex); + } + } + if (versioningApplied) { + try { + if (Boolean.TRUE.equals(previousVersioning)) { + objectStore.setBucketVersioning(bucketTO); + } else { + objectStore.deleteBucketVersioning(bucketTO); + } + bucket.setVersioning(previousVersioning); + } catch (Exception ex) { + logger.error("Failed to roll back bucket versioning for {} while compensating a failed update", + bucket.getName(), ex); + cause.addSuppressed(ex); + } + } + if (encryptionApplied) { + try { + if (Boolean.TRUE.equals(previousEncryption)) { + objectStore.setBucketEncryption(bucketTO); + } else { + objectStore.deleteBucketEncryption(bucketTO); + } + bucket.setEncryption(previousEncryption); + } catch (Exception ex) { + logger.error("Failed to roll back bucket encryption for {} while compensating a failed update", + bucket.getName(), ex); + cause.addSuppressed(ex); + } + } + } + + private boolean updateBucketQuota(UpdateBucketCmd cmd, BucketVO bucket, ObjectStoreEntity objectStore, ObjectStoreVO objectStoreVO, BucketTO bucketTO) throws ResourceAllocationException { Integer quota = cmd.getQuota(); if (quota == null) { - return; + return false; } - int quotaDelta = quota - bucket.getQuota(); - objectStore.setQuota(bucketTO, quota); - bucket.setQuota(quota); - + Integer previousQuota = bucket.getQuota(); + int previousQuotaValue = ObjectUtils.defaultIfNull(previousQuota, 0); + long quotaDelta = (long) quota - previousQuotaValue; long diff = quotaDelta * Resource.ResourceType.bytesToGiB; - if (quotaDelta < 0) { - resourceLimitManager.decrementResourceCount(bucket.getAccountId(), Resource.ResourceType.object_storage, Math.abs(diff)); - _objectStoreDao.updateAllocatedSize(objectStoreVO, diff); - return; + if (diff <= 0) { + // A decrease (or no change) cannot exceed a limit, so no reservation + // is needed. + applyQuotaChange(bucket, objectStore, objectStoreVO, bucketTO, quota, previousQuota, previousQuotaValue, diff); + return true; } Review Comment: diff <= 0` treats “no change” (`diff == 0`) the same as a decrease and still calls `applyQuotaChange`, which triggers a remote `setQuota(...)` call and a DAO update even though nothing changed. That increases failure surface area and can make an otherwise no-op update fail. Consider adding an early return when `quota == previousQuotaValue` (skip remote call and persistence; let the caller persist other bucket changes as usual). ########## plugins/storage/object/seaweedfs/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/SeaweedFSObjectStoreLifeCycleImplTest.java: ########## @@ -0,0 +1,306 @@ +// 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 static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO; +import org.apache.cloudstack.storage.datastore.util.SeaweedFSObjectStoreUtil; +import org.apache.cloudstack.storage.object.ObjectStoreEntity; +import org.apache.cloudstack.storage.object.datastore.ObjectStoreHelper; +import org.apache.cloudstack.storage.object.datastore.ObjectStoreProviderManager; +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.MockedStatic; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.exception.CloudRuntimeException; + +@RunWith(MockitoJUnitRunner.class) +public class SeaweedFSObjectStoreLifeCycleImplTest { + + @Spy + SeaweedFSObjectStoreLifeCycleImpl lifecycle = new SeaweedFSObjectStoreLifeCycleImpl(); + + @Mock + ObjectStoreHelper objectStoreHelper; + @Mock + ObjectStoreProviderManager objectStoreMgr; + @Mock + ObjectStoreVO objectStoreVo; + @Mock + ObjectStoreEntity objectStoreEntity; + + static String TEST_STORE_NAME = "testStore"; + static String TEST_URL = "http://s3-endpoint";; + static String TEST_PROVIDER_NAME = "SeaweedFS"; + static String TEST_ACCESS_KEY = "admin-access-key"; + static String TEST_SECRET_KEY = "admin-secret-key"; + static String TEST_S3_URL_OVERRIDE = "http://s3-override:8333";; + static String TEST_IAM_URL_OVERRIDE = "http://iam-override:8111";; + static String TEST_METRICS_URL = "http://seaweedfs-s3:9327";; + + Map<String, String> detailsMap; + Map<String, Object> dsInfos; + + MockedStatic<SeaweedFSObjectStoreUtil> mockStatic; + + private AutoCloseable closeable; + + @Before + public void setUp() { + closeable = MockitoAnnotations.openMocks(this); + + mockStatic = Mockito.mockStatic(SeaweedFSObjectStoreUtil.class); Review Comment: This test class uses `@RunWith(MockitoJUnitRunner.class)` and also calls `MockitoAnnotations.openMocks(this)` in `setUp()`. The runner already initializes `@Mock`/`@Spy` fields; double-initialization is redundant and can lead to confusing behavior. Prefer removing `openMocks` (and the associated `closeable`) when using the Mockito JUnit runner. ########## plugins/storage/object/seaweedfs/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/SeaweedFSObjectStoreLifeCycleImplTest.java: ########## @@ -0,0 +1,306 @@ +// 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 static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO; +import org.apache.cloudstack.storage.datastore.util.SeaweedFSObjectStoreUtil; +import org.apache.cloudstack.storage.object.ObjectStoreEntity; +import org.apache.cloudstack.storage.object.datastore.ObjectStoreHelper; +import org.apache.cloudstack.storage.object.datastore.ObjectStoreProviderManager; +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.MockedStatic; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.exception.CloudRuntimeException; + +@RunWith(MockitoJUnitRunner.class) +public class SeaweedFSObjectStoreLifeCycleImplTest { + + @Spy + SeaweedFSObjectStoreLifeCycleImpl lifecycle = new SeaweedFSObjectStoreLifeCycleImpl(); Review Comment: This test class uses `@RunWith(MockitoJUnitRunner.class)` and also calls `MockitoAnnotations.openMocks(this)` in `setUp()`. The runner already initializes `@Mock`/`@Spy` fields; double-initialization is redundant and can lead to confusing behavior. Prefer removing `openMocks` (and the associated `closeable`) when using the Mockito JUnit runner. ########## plugins/storage/object/seaweedfs/src/main/java/org/apache/cloudstack/storage/datastore/util/SeaweedFSObjectStoreUtil.java: ########## @@ -0,0 +1,715 @@ +// 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_SIZE = "size"; + 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 + public static final String STORE_DETAILS_KEY_METRICS_URL = "metricsUrl"; // Prometheus metrics endpoint URL (optional, for scalable usage reporting) + + // Account Detail Map key names - credentials created per CloudStack account. + // Namespaced by store ID so one account can use multiple SeaweedFS pools + // without the second pool overwriting the first pool's credentials. + public static final String KEY_ACCESS_KEY_PREFIX = "swfs_AccessKey_"; + public static final String KEY_SECRET_KEY_PREFIX = "swfs_SecretKey_"; + + /** + * Build the account-detail key for the IAM access key of a given store. + */ + public static String keyAccessKey(long storeId) { + return KEY_ACCESS_KEY_PREFIX + storeId; + } + + /** + * Build the account-detail key for the IAM secret key of a given store. + */ + public static String keySecretKey(long storeId) { + return KEY_SECRET_KEY_PREFIX + storeId; + } + + /** + * Strip trailing slashes from a configured endpoint URL so callers can + * append a {@code "/" + path} suffix without producing a double slash. + * The S3 and IAM endpoint URLs are operator-supplied and are accepted with + * or without a trailing slash. + * + * @param url the URL to normalize, may be null + * @return the URL without trailing slashes, or the input if null/empty + */ + public static String stripTrailingSlashes(String url) { + if (url == null) { + return null; + } + String normalized = url; + while (normalized.endsWith("/")) { + normalized = normalized.substring(0, normalized.length() - 1); + } + return normalized; + } + + /** + * Returns true when the URL carries a non-empty path component beyond the + * authority (e.g. {@code http://host:8333/s3} has the path {@code /s3}). + * + * The CloudStack object-store browser builds its MinIO client from only the + * host and port of the stored bucket URL and drops any path prefix (see + * {@code ObjectStoreBrowser.vue#initMinioClient}). The MinIO JS client has + * no basePath option, so a SeaweedFS deployment behind a path-prefixed + * reverse proxy (e.g. {@code /s3}) works for server-side calls but the + * browser's list/upload requests target the wrong endpoint. Rejecting + * path-prefixed endpoints at registration prevents a configuration that + * would silently break the browser. + * + * @param url the endpoint URL to check, may be null + * @return true if the URL has a path component other than empty or "/" + */ + public static boolean hasPathPrefix(String url) { + if (url == null) { + return false; + } + try { + java.net.URI uri = java.net.URI.create(url); + String path = uri.getPath(); + return path != null && !path.isEmpty() && !"/".equals(path); + } catch (IllegalArgumentException e) { + return false; + } + } + + /** + * Connect timeout for the S3 extension HTTP client, in seconds. + */ + public static final int S3_EXTENSION_CONNECT_TIMEOUT_SECONDS = 10; + /** + * Per-request timeout for the S3 extension HTTP request, in seconds. + */ + public static final int S3_EXTENSION_REQUEST_TIMEOUT_SECONDS = 30; + + /** + * IAM user policy name applied to each per-account IAM user. + */ + public static final String IAM_USER_POLICY_NAME = "CloudStackPolicy"; + + /** + * Build an IAM user policy that grants full S3 access only to the given + * buckets (both the bucket and its contents), while denying bucket + * creation and deletion everywhere so CloudStack retains control of the + * bucket lifecycle. When no buckets are provided, all S3 access is denied. + * + * <p>This is the tenant boundary: each account's IAM credentials can only + * operate on that account's own buckets, not on every bucket in the + * SeaweedFS pool. The policy is refreshed whenever buckets are created or + * deleted (see + * {@code SeaweedFSObjectStoreDriverImpl.updateAccountIAMPolicy}). + * + * @param bucketNames the bucket names the account is allowed to access + * @return a JSON IAM policy document + */ + public static String buildAccountIAMPolicy(java.util.List<String> bucketNames) { + StringBuilder sb = new StringBuilder(); + sb.append("{\n"); + sb.append(" \"Version\": \"2012-10-17\",\n"); + sb.append(" \"Statement\": [\n"); + if (bucketNames == null || bucketNames.isEmpty()) { + // No buckets: deny all S3 access. A Resource cannot be empty in + // an IAM policy, so deny everything explicitly. + sb.append(" {\n"); + sb.append(" \"Sid\": \"DenyAllS3\",\n"); + sb.append(" \"Effect\": \"Deny\",\n"); + sb.append(" \"Action\": [\"s3:*\"],\n"); + sb.append(" \"Resource\": [\"arn:aws:s3:::*\", \"arn:aws:s3:::*/*\"]\n"); + sb.append(" }\n"); + } else { + sb.append(" {\n"); + sb.append(" \"Sid\": \"AllowAccountBuckets\",\n"); + sb.append(" \"Effect\": \"Allow\",\n"); + sb.append(" \"Action\": [\"s3:*\"],\n"); + sb.append(" \"Resource\": [\n"); + for (int i = 0; i < bucketNames.size(); i++) { + String name = bucketNames.get(i); + sb.append(" \"arn:aws:s3:::").append(name).append("\",\n"); + sb.append(" \"arn:aws:s3:::").append(name).append("/*\""); + if (i < bucketNames.size() - 1) { + sb.append(","); + } + sb.append("\n"); + } + sb.append(" ]\n"); + sb.append(" }\n"); + } + // Always deny bucket creation/deletion and quota mutation — + // CloudStack controls lifecycle and resource accounting. Denying + // s3:PutBucketQuota prevents a tenant from using the credentials + // returned in BucketResponse to call the SeaweedFS quota extension + // directly and bypass CloudStack's resource accounting. + sb.append(" ,{\n"); + sb.append(" \"Sid\": \"DenyBucketLifecycleAndQuota\",\n"); + sb.append(" \"Effect\": \"Deny\",\n"); + sb.append(" \"Action\": [\"s3:CreateBucket\", \"s3:DeleteBucket\", \"s3:PutBucketQuota\"],\n"); + sb.append(" \"Resource\": \"*\"\n"); + sb.append(" }\n"); + sb.append(" ]\n"); + sb.append("}\n"); + return sb.toString(); + } + + /** + * 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); + } + } + } Review Comment: validateS3Url` (and similarly `validateIAMUrl` / `validateCredentials`) only catches `AmazonServiceException`. Connection failures, DNS errors, TLS failures, etc. typically throw `AmazonClientException` and will currently escape without being wrapped in a `CloudRuntimeException`, despite the method contract/comments implying CloudStack exceptions. Catch `AmazonClientException` and rethrow as `CloudRuntimeException` with a helpful message so registration errors are consistent and user-facing. -- 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]
