JoaoJandre commented on code in PR #9748:
URL: https://github.com/apache/cloudstack/pull/9748#discussion_r1805263925
##########
plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/client/CloudianClient.java:
##########
@@ -287,33 +492,42 @@ public CloudianGroup listGroup(final String groupId) {
try {
final HttpResponse response =
get(String.format("/group?groupId=%s", groupId));
checkResponseOK(response);
- if (checkEmptyResponse(response)) {
- return null;
+ if (noResponseEntity(response)) {
+ return null; // Group Not Found
}
final ObjectMapper mapper = new ObjectMapper();
return mapper.readValue(response.getEntity().getContent(),
CloudianGroup.class);
} catch (final IOException e) {
logger.error("Failed to list Cloudian group due to:", e);
- checkResponseTimeOut(e);
+ throwTimeoutOrServerException(e);
+ return null; // never reached
}
- return null;
}
public List<CloudianGroup> listGroups() {
logger.debug("Trying to list Cloudian groups");
try {
final HttpResponse response = get("/group/list");
checkResponseOK(response);
- if (checkEmptyResponse(response)) {
- return new ArrayList<>();
+ if (noResponseEntity(response)) {
+ throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "API
error");
+ }
+ // The empty list case is badly behaved and returns as 200 with an
empty body.
+ // We'll try detect this by reading the first byte and then
putting it back if required.
+ PushbackInputStream iStream = new
PushbackInputStream(response.getEntity().getContent());
+ int firstByte=iStream.read();
+ if (firstByte == -1) {
+ return new ArrayList<>(); // EOF => empty list
}
+ // unread that first byte and process the JSON
+ iStream.unread(firstByte);
Review Comment:
Duplicated code, we should extract it to a method.
##########
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);
Review Comment:
The class being extended already has a protected logger, there is no need to
define another one.
##########
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:
Is it really ok to delete the credential in this method? If some error
created a lot of unmanaged credentials, shouldn't the admin delete them
manually? (asking as a layman on Cloudian)
##########
plugins/integrations/cloudian/src/main/java/org/apache/cloudstack/cloudian/client/CloudianClient.java:
##########
@@ -159,22 +180,108 @@ private HttpResponse post(final String path, final
Object item) throws IOExcepti
return response;
}
+ /**
+ * Perform a HTTP PUT operation using the path and optional JSON body item.
+ * @param path the http path to use
+ * @param item optional object to send in the body payload. Set to null if
no body.
+ * @return the HttpResponse object
+ * @throws IOException if the request cannot be executed completely.
+ * @throws ServerApiException if the request meets 401 unauthorized.
+ */
private HttpResponse put(final String path, final Object item) throws
IOException {
- final ObjectMapper mapper = new ObjectMapper();
- final String json = mapper.writeValueAsString(item);
- final StringEntity entity = new StringEntity(json);
final HttpPut request = new HttpPut(adminApiUrl + path);
- request.setHeader("content-type", "application/json");
- request.setEntity(entity);
+ if (item != null) {
+ final ObjectMapper mapper = new ObjectMapper();
+ final String json = mapper.writeValueAsString(item);
+ final StringEntity entity = new StringEntity(json);
+ request.setHeader("content-type", "application/json");
+ request.setEntity(entity);
+ }
final HttpResponse response = httpClient.execute(request, httpContext);
checkAuthFailure(response);
return response;
}
+ ////////////////////////////////////////////////////////
+ //////////////// Public APIs: Misc /////////////////////
+ ////////////////////////////////////////////////////////
+
+ /**
+ * Get the HyperStore Server Version number.
+ *
+ * @return version number
+ * @throws ServerApiException on non-200 response or timeout
+ */
+ public String getServerVersion() {
+ logger.debug("Getting server version");
+ try {
+ final HttpResponse response = get("/system/version");
+ checkResponseOK(response);
+ HttpEntity entity = response.getEntity();
+ return EntityUtils.toString(entity, "UTF-8");
+ } catch (final IOException e) {
+ logger.error("Failed to get HyperStore system version:", e);
+ throwTimeoutOrServerException(e);
+ }
+ return null;
+ }
+
+ /**
+ * Get bucket usage information for a group, a user or a particular bucket.
+ *
+ * Note: Bucket Usage Statistics in HyperStore are disabled by default.
They
+ * can be enabled by the HyperStore Administrator by setting of the
configuration
+ * 's3.qos.bucketLevel=true'.
+ *
+ * @param groupId the groupId is required (and must exist)
+ * @param userId the userId is optional (null) and if not set all group
users are returned.
+ * @param bucket the bucket is optional (null). If set the userId must
also be set.
+ * @return a list of bucket usages (possibly empty).
+ * @throws ServerApiException on non-200 response such as unknown groupId
etc or response issue.
+ */
+ public List<CloudianUserBucketUsage> getUserBucketUsages(final String
groupId, final String userId, final String bucket) {
+ if (StringUtils.isBlank(groupId)) {
+ return new ArrayList<>();
+ }
+ logger.debug("Getting bucket usages for groupId={} userId={}
bucket={}", groupId, userId, bucket);
+ StringBuilder cmd = new StringBuilder("/system/bucketusage?groupId=");
+ cmd.append(groupId);
+ if (! StringUtils.isBlank(userId)) {
+ cmd.append("&userId=");
+ cmd.append(userId);
+ }
+ if (! StringUtils.isBlank(bucket)) {
+ // Assume userId is also set (or request fails).
Review Comment:
If request fails when userId is not set, why not stop the process here
instead of doing the request?
##########
plugins/storage/object/cloudian/src/main/java/org/apache/cloudstack/storage/datastore/util/CloudianHyperStoreUtil.java:
##########
@@ -0,0 +1,218 @@
+// 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 java.net.MalformedURLException;
+import java.net.URL;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+
+import org.apache.cloudstack.cloudian.client.CloudianClient;
+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;
+
+public class CloudianHyperStoreUtil {
+
+ /** The name of our Object Store Provider */
+ public static final String OBJECT_STORE_PROVIDER_NAME = "Cloudian
HyperStore";
+
+ 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_USER_NAME = "accesskey";
// admin user name
+ public static final String STORE_DETAILS_KEY_PASSWORD = "secretkey";
// admin password
+ public static final String STORE_DETAILS_KEY_VALIDATE_SSL = "validateSSL";
+ public static final String STORE_DETAILS_KEY_S3_URL = "s3Url";
+ public static final String STORE_DETAILS_KEY_IAM_URL = "iamUrl";
+
+ // Account Detail Map key names
+ public static final String KEY_ROOT_ACCESS_KEY = "hs_AccessKey";
+ public static final String KEY_ROOT_SECRET_KEY = "hs_SecretKey";
+ public static final String KEY_IAM_ACCESS_KEY = "hs_IAMAccessKey";
+ public static final String KEY_IAM_SECRET_KEY = "hs_IAMSecretKey";
+
+ public static final int DEFAULT_ADMIN_PORT = 19443;
+ public static final int DEFAULT_ADMIN_TIMEOUT_SECONDS = 10;
+
+ public static final String IAM_USER_USERNAME = "CloudStack";
+ public static final String IAM_USER_POLICY_NAME = "CloudStackPolicy";
+ public static final String IAM_USER_POLICY;
+ static {
+ StringBuilder sb = new StringBuilder();
+ sb.append("{\n");
+ sb.append(" \"Version\": \"2012-10-17\",\n");
+ sb.append(" \"Statement\": [\n");
+ sb.append(" {\n");
+ sb.append(" \"Sid\": \"AllowFullS3Access\",\n");
+ sb.append(" \"Effect\": \"Allow\",\n");
+ sb.append(" \"Action\": [\n");
+ sb.append(" \"s3:*\"\n");
+ sb.append(" ],\n");
+ sb.append(" \"Resource\": \"*\"\n");
+ sb.append(" },\n");
+ sb.append(" {\n");
+ sb.append(" \"Sid\": \"ExceptBucketCreationOrDeletion\",\n");
+ sb.append(" \"Effect\": \"Deny\",\n");
+ sb.append(" \"Action\": [\n");
+ sb.append(" \"s3:createBucket\",\n");
+ sb.append(" \"s3:deleteBucket\"\n");
+ sb.append(" ],\n");
+ sb.append(" \"Resource\": \"*\"\n");
+ sb.append(" }\n");
+ sb.append(" ]\n");
+ sb.append("}\n");
+ IAM_USER_POLICY = sb.toString();
+ }
Review Comment:
Why not simply define the variable with the plain string value? Is there any
reason to use StringBuilder here?
##########
plugins/storage/object/cloudian/src/main/java/org/apache/cloudstack/storage/datastore/util/CloudianHyperStoreUtil.java:
##########
@@ -0,0 +1,218 @@
+// 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 java.net.MalformedURLException;
+import java.net.URL;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+
+import org.apache.cloudstack.cloudian.client.CloudianClient;
+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;
+
+public class CloudianHyperStoreUtil {
+
+ /** The name of our Object Store Provider */
+ public static final String OBJECT_STORE_PROVIDER_NAME = "Cloudian
HyperStore";
+
+ 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_USER_NAME = "accesskey";
// admin user name
+ public static final String STORE_DETAILS_KEY_PASSWORD = "secretkey";
// admin password
+ public static final String STORE_DETAILS_KEY_VALIDATE_SSL = "validateSSL";
+ public static final String STORE_DETAILS_KEY_S3_URL = "s3Url";
+ public static final String STORE_DETAILS_KEY_IAM_URL = "iamUrl";
+
+ // Account Detail Map key names
+ public static final String KEY_ROOT_ACCESS_KEY = "hs_AccessKey";
+ public static final String KEY_ROOT_SECRET_KEY = "hs_SecretKey";
+ public static final String KEY_IAM_ACCESS_KEY = "hs_IAMAccessKey";
+ public static final String KEY_IAM_SECRET_KEY = "hs_IAMSecretKey";
+
+ public static final int DEFAULT_ADMIN_PORT = 19443;
+ public static final int DEFAULT_ADMIN_TIMEOUT_SECONDS = 10;
+
+ public static final String IAM_USER_USERNAME = "CloudStack";
+ public static final String IAM_USER_POLICY_NAME = "CloudStackPolicy";
+ public static final String IAM_USER_POLICY;
+ static {
+ StringBuilder sb = new StringBuilder();
+ sb.append("{\n");
+ sb.append(" \"Version\": \"2012-10-17\",\n");
+ sb.append(" \"Statement\": [\n");
+ sb.append(" {\n");
+ sb.append(" \"Sid\": \"AllowFullS3Access\",\n");
+ sb.append(" \"Effect\": \"Allow\",\n");
+ sb.append(" \"Action\": [\n");
+ sb.append(" \"s3:*\"\n");
+ sb.append(" ],\n");
+ sb.append(" \"Resource\": \"*\"\n");
+ sb.append(" },\n");
+ sb.append(" {\n");
+ sb.append(" \"Sid\": \"ExceptBucketCreationOrDeletion\",\n");
+ sb.append(" \"Effect\": \"Deny\",\n");
+ sb.append(" \"Action\": [\n");
+ sb.append(" \"s3:createBucket\",\n");
+ sb.append(" \"s3:deleteBucket\"\n");
+ sb.append(" ],\n");
+ sb.append(" \"Resource\": \"*\"\n");
+ sb.append(" }\n");
+ sb.append(" ]\n");
+ sb.append("}\n");
+ IAM_USER_POLICY = sb.toString();
+ }
+
+ /**
+ * This method is solely for test purposes so that we can mock the timeout.
+ *
+ * @returns the timeout in seconds
+ */
+ protected static int getAdminTimeoutSeconds() {
+ return DEFAULT_ADMIN_TIMEOUT_SECONDS;
+ }
+
+ /**
+ * Get a connection to the Cloudian HyperStore ADMIN API Service.
+ * @param url the url of the ADMIN API service
+ * @param user the admin username to connect as
+ * @param pass the matching admin password
+ * @param validateSSL validate the SSL Certificate (when using https://)
+ * @return a connection object (never null)
+ * @throws CloudRuntimeException if the connection fails for any reason
+ */
+ public static CloudianClient getCloudianClient(String url, String user,
String pass, boolean validateSSL) {
+ try {
+ URL parsedURL = new URL(url);
+ String scheme = parsedURL.getProtocol();
+ String host = parsedURL.getHost();
+ int port = parsedURL.getPort();
+ if (port == -1) {
+ port = DEFAULT_ADMIN_PORT;
+ }
+ return new CloudianClient(host, port, scheme, user, pass,
validateSSL, getAdminTimeoutSeconds());
+ } catch (MalformedURLException e) {
+ throw new CloudRuntimeException(e);
+ } catch (KeyStoreException | NoSuchAlgorithmException |
KeyManagementException e) {
+ throw new CloudRuntimeException(e);
+ }
Review Comment:
We could join these catches
##########
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);
+ }
+ } catch (NoSuchEntityException e) {
+ // No IAM User. Ignore and fix this below.
+ }
+ }
+
+ // If we get here, a usable credential does not yet exist so create it.
+ // Before creating it, we also need to ensure the IAM User that will
own it exists.
+ boolean createdUser = false;
+ try {
+ iamClient.createUser(new CreateUserRequest(iamUser));
+ logger.info("Created IAM user {} for account", iamUser);
+ createdUser = true;
+ } catch (EntityAlreadyExistsException e) {
+ // User already exists. Ignore and continue.
+ }
+
+ // Always Add or Update the IAM policy
+ iamClient.putUserPolicy(new PutUserPolicyRequest(iamUser,
CloudianHyperStoreUtil.IAM_USER_POLICY_NAME,
CloudianHyperStoreUtil.IAM_USER_POLICY));
+
+ if (! createdUser && iamAccessKeyId == null) {
+ // User already exists but we never saved any access key before.
We should try clean up
+ logger.debug("Looking for any un-managed IAM credentials for IAM
User {}", iamUser);
+ ListAccessKeysResult listRes = iamClient.listAccessKeys(new
ListAccessKeysRequest().withUserName(iamUser));
+ for (AccessKeyMetadata accessKeyMetadata :
listRes.getAccessKeyMetadata()) {
+ 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:
Same here, should we be cleaning up stuff on a method used to create
credentials?
--
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
