tpodowd commented on code in PR #9748: URL: https://github.com/apache/cloudstack/pull/9748#discussion_r1805928197
########## plugins/storage/object/cloudian/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudianHyperStoreObjectStoreDriverImpl.java: ########## @@ -0,0 +1,877 @@ +/* + * 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.api.ServerApiException; +import org.apache.cloudstack.cloudian.client.CloudianClient; +import org.apache.cloudstack.cloudian.client.CloudianCredential; +import org.apache.cloudstack.cloudian.client.CloudianGroup; +import org.apache.cloudstack.cloudian.client.CloudianUser; +import org.apache.cloudstack.cloudian.client.CloudianUserBucketUsage; +import org.apache.cloudstack.cloudian.client.CloudianUserBucketUsage.CloudianBucketUsage; +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.CloudianHyperStoreUtil; +import org.apache.cloudstack.storage.object.BaseObjectStoreDriverImpl; +import org.apache.cloudstack.storage.object.Bucket; +import org.apache.cloudstack.storage.object.BucketObject; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +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.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.ListAccessKeysResult; +import com.amazonaws.services.identitymanagement.model.NoSuchEntityException; +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.BucketCrossOriginConfiguration; +import com.amazonaws.services.s3.model.BucketPolicy; +import com.amazonaws.services.s3.model.BucketVersioningConfiguration; +import com.amazonaws.services.s3.model.CORSRule; +import com.amazonaws.services.s3.model.CreateBucketRequest; +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.domain.Domain; +import com.cloud.domain.dao.DomainDao; +import com.cloud.user.Account; +import com.cloud.user.AccountDetailsDao; +import com.cloud.user.dao.AccountDao; +import com.cloud.utils.exception.CloudRuntimeException; + +public class CloudianHyperStoreObjectStoreDriverImpl extends BaseObjectStoreDriverImpl { + protected Logger logger = LogManager.getLogger(CloudianHyperStoreObjectStoreDriverImpl.class); + + @Inject + AccountDao _accountDao; + + @Inject + AccountDetailsDao _accountDetailsDao; + + @Inject + DomainDao _domainDao; + + @Inject + ObjectStoreDao _storeDao; + + @Inject + BucketDao _bucketDao; + + @Inject + ObjectStoreDetailsDao _storeDetailsDao; + + @Override + public DataStoreTO getStoreTO(DataStore store) { + return null; + } + + /** + * Get the HyperStore user id for the current account. + * @param account the current account + * @return the userId based on the CloudStack account uuid. + */ + protected String getHyperStoreUserId(Account account) { + return account.getUuid(); + } + + /** + * Get the HyperStore tenant/group id for the current domain. + * @param domain the current domain + * @return the groupId based on the CloudStack domain uuid + */ + protected String getHyperStoreGroupId(Domain domain) { + return domain.getUuid(); + } + + /** + * Create the HyperStore user resources matching this account if it doesn't exist. + * + * The following resources are created for the account: + * - HyperStore Group to match the CloudStack Domain UUID + * - HyperStore User to match the CloudStack Account UUID + * - HyperStore Root User Credentials to manage Account Buckets etc (kept private to this plugin) + * - HyperStore IAM User with IAM policy granting all S3 actions except create/delete buckets. + * - HyperStore IAM User Credentials (visible to end user as part of Bucket Details) + * + * @param accountId the CloudStack account + * @param storeId the object store. + * + * @return true if user exists or was created, false if there was some issue creating it. + * @throws CloudRuntimeException on errors checking if the user exists or if the HyperStore user or group is disabled. + */ + @Override + public boolean createUser(long accountId, long storeId) { + Account account = _accountDao.findById(accountId); + Domain domain = _domainDao.findById(account.getDomainId()); + String hsUserId = getHyperStoreUserId(account); + String hsGroupId = getHyperStoreGroupId(domain); + + CloudianClient client = getCloudianClientByStoreId(storeId); + logger.debug("Checking if user id={} group id={} exists.", hsGroupId, hsUserId); + CloudianUser user = client.listUser(hsUserId, hsGroupId); + if (user == null) { + // Create the group if it doesn't already exist + createHSGroup(client, hsGroupId, domain); + // Create the user under the group. + user = createHSUser(client, hsUserId, hsGroupId, account); + if (user == null) { + return false; // already logged. + } + } else if (! user.getActive()) { + // Normally this would be true unless an administrator has explicitly disabled the user account. + String msg = String.format("The User id=%s group id=%s is Disabled. Consult your HyperStore Administrator.", hsUserId, hsGroupId); + logger.error(msg); + throw new CloudRuntimeException(msg); + } else { + // User exists and is active. We know that the group therefore exists but + // we should ensure that it is active or it will lead to unknown access key errors + // which might confuse the administrator. Checking is clearer. + CloudianGroup group = client.listGroup(hsGroupId); + if (group != null && ! group.getActive()) { + String msg = String.format("The group id=%s is Disabled. Consult your HyperStore Administrator.", hsGroupId); + logger.error(msg); + throw new CloudRuntimeException(msg); + } + } + + // We either created a new account or found an existing one. + CloudianCredential credential = createHSCredential(client, hsUserId, hsGroupId); + + // Next, ensure we the IAM User Credentials exist. These are available + // to the user as part of the bucket details instead of the Root credentials. + Map<String, String> details = _accountDetailsDao.findDetails(accountId); + AccessKey iamCredential = createIAMCredentials(storeId, details, credential); + + // persist the root and iam credentials in the database and update all bucket details. + persistCredentials(storeId, accountId, details, credential, iamCredential); + return true; + } + + /** + * Create IAM credentials if required. + * + * When the HyperStore user is first created, this method will create an IAM User with an appropriate + * permission policy and a set of credentials which will be returned. + * After the first run, the IAM resources should already be in place in which case we just ensure + * the credentials we are using are still available and if not, it tries to recreate the IAM resources. + * + * @param storeId the store + * @param details a map of existing account details that we know about including any saved IAM credentials. + * @param credential the account Root User credentials (to manage the IAM resources). + * @return an AccessKey object for newly created IAM credentials or null if existing credentials were ok + * and nothing was created. + */ + protected AccessKey createIAMCredentials(long storeId, Map<String, String> details, CloudianCredential credential) { + AmazonIdentityManagement iamClient = getIAMClientByStoreId(storeId, credential); + final String iamUser = CloudianHyperStoreUtil.IAM_USER_USERNAME; + + // If an accessKeyId is known to us, check IAM still has it. + String iamAccessKeyId = details.get(CloudianHyperStoreUtil.KEY_IAM_ACCESS_KEY); + if (iamAccessKeyId != null) { + try { + logger.debug("Looking for IAM credential {} for IAM User {}", iamAccessKeyId, iamUser); + ListAccessKeysResult listAccessKeyResult = iamClient.listAccessKeys(new ListAccessKeysRequest().withUserName(iamUser)); + for (AccessKeyMetadata accessKeyMetadata : listAccessKeyResult.getAccessKeyMetadata()) { + if (iamAccessKeyId.equals(accessKeyMetadata.getAccessKeyId())) { + return null; // The IAM AccessKeyId still exists (as expected). return null. + } + // Usually, there will only be 1 credential that we manage, but an error persisting + // credentials might leave an un-managed credential which we can just delete. It is better + // to delete as otherwise, we may hit a max credential limit for this IAM user. + DeleteAccessKeyRequest deleteAccessKeyRequest = new DeleteAccessKeyRequest(); + deleteAccessKeyRequest.setUserName(iamUser); + deleteAccessKeyRequest.setAccessKeyId(accessKeyMetadata.getAccessKeyId()); + logger.info("Deleting un-managed IAM AccessKeyId {} for IAM User {}", accessKeyMetadata.getAccessKeyId(), iamUser); + iamClient.deleteAccessKey(deleteAccessKeyRequest); Review Comment: Hi @JoaoJandre . Thanks for asking. Indeed it's a little strange at first but I think it is the right thing to do. This IAM user is owned by cloudstack. There should be nothing else creating IAM credentials for this user except for this plugin. IAM resources such as user/credentials/policies etc are the responsibility of the account so the administrator would not normally get involved with managing them. Also the account itself is owned by cloudstack and the idea is that you can only create buckets via the cloudstack interface and the credentials you have access to as a user also have no IAM access. That's basically why in this particular case, I think it is best to tidy up directly. Hope that answers your question. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
