From 370941b60b6adce3bfee5afa1efa78b098111e93 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 28 Sep 2017 17:38:51 +0530 Subject: [PATCH] cloudian: code cleanup and last refactoring/changes Signed-off-by: Rohit Yadav --- plugins/integrations/cloudian/pom.xml | 13 +------ .../cloudian/spring-cloudian-context.xml | 4 +-- .../com/cloudian/client/CloudianClient.java | 36 +++++++++++++++++-- .../com/cloudian/client/CloudianGroup.java | 1 - .../src/com/cloudian/client/CloudianUser.java | 1 - .../cloudstack/CloudianConnectorImpl.java | 32 ++++++++++------- .../cloudian/cloudstack/CloudianUtils.java | 4 +-- .../response/CloudianEnabledResponse.java | 8 ----- .../cloudian/client/CloudianClientTest.java | 2 -- 9 files changed, 58 insertions(+), 43 deletions(-) diff --git a/plugins/integrations/cloudian/pom.xml b/plugins/integrations/cloudian/pom.xml index 867436788af..bd01f739884 100644 --- a/plugins/integrations/cloudian/pom.xml +++ b/plugins/integrations/cloudian/pom.xml @@ -27,7 +27,7 @@ org.apache.cloudstack cloudstack-plugins 4.9.4.0-SNAPSHOT - ../pom.xml + ../../pom.xml @@ -57,15 +57,4 @@ test - - - - org.apache.maven.plugins - maven-surefire-plugin - - -Xmx1024m - - - - diff --git a/plugins/integrations/cloudian/resources/META-INF/cloudstack/cloudian/spring-cloudian-context.xml b/plugins/integrations/cloudian/resources/META-INF/cloudstack/cloudian/spring-cloudian-context.xml index 3d62ac9f785..3c48885c1da 100644 --- a/plugins/integrations/cloudian/resources/META-INF/cloudstack/cloudian/spring-cloudian-context.xml +++ b/plugins/integrations/cloudian/resources/META-INF/cloudstack/cloudian/spring-cloudian-context.xml @@ -19,9 +19,7 @@ - + http://www.springframework.org/schema/beans/spring-beans.xsd"> diff --git a/plugins/integrations/cloudian/src/com/cloudian/client/CloudianClient.java b/plugins/integrations/cloudian/src/com/cloudian/client/CloudianClient.java index 4b21c1dfb10..6f569847620 100644 --- a/plugins/integrations/cloudian/src/com/cloudian/client/CloudianClient.java +++ b/plugins/integrations/cloudian/src/com/cloudian/client/CloudianClient.java @@ -58,9 +58,9 @@ import org.apache.log4j.Logger; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.nio.TrustAllManager; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Strings; public class CloudianClient { - private static final Logger LOG = Logger.getLogger(CloudianClient.class); private final HttpClient httpClient; @@ -135,6 +135,10 @@ public class CloudianClient { //////////////////////////////////////////////////////// public boolean addUser(final CloudianUser user) { + if (user == null) { + return false; + } + LOG.debug("Adding Cloudian user: " + user); try { final HttpResponse response = put("/user", user); return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK; @@ -148,6 +152,9 @@ public class CloudianClient { } public CloudianUser listUser(final String userId, final String groupId) { + if (Strings.isNullOrEmpty(userId) || Strings.isNullOrEmpty(groupId)) { + return null; + } try { final HttpResponse response = get(String.format("/user?userId=%s&groupId=%s", userId, groupId)); final ObjectMapper mapper = new ObjectMapper(); @@ -165,6 +172,9 @@ public class CloudianClient { } public List listUsers(final String groupId) { + if (Strings.isNullOrEmpty(groupId)) { + return new ArrayList<>(); + } try { final HttpResponse response = get(String.format("/user/list?groupId=%s&userType=all&userStatus=active", groupId)); final ObjectMapper mapper = new ObjectMapper(); @@ -182,6 +192,10 @@ public class CloudianClient { } public boolean updateUser(final CloudianUser user) { + if (user == null) { + return false; + } + LOG.debug("Updating Cloudian user: " + user); try { final HttpResponse response = post("/user", user); return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK; @@ -195,6 +209,10 @@ public class CloudianClient { } public boolean removeUser(final String userId, final String groupId) { + if (Strings.isNullOrEmpty(userId) || Strings.isNullOrEmpty(groupId)) { + return false; + } + LOG.debug("Removing Cloudian user with user id=" + userId + " in group id=" + groupId); try { final HttpResponse response = delete(String.format("/user?userId=%s&groupId=%s", userId, groupId)); return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK; @@ -212,6 +230,10 @@ public class CloudianClient { ///////////////////////////////////////////////////////// public boolean addGroup(final CloudianGroup group) { + if (group == null) { + return false; + } + LOG.debug("Adding Cloudian group: " + group); try { final HttpResponse response = put("/group", group); return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK; @@ -225,6 +247,9 @@ public class CloudianClient { } public CloudianGroup listGroup(final String groupId) { + if (Strings.isNullOrEmpty(groupId)) { + return null; + } try { final HttpResponse response = get(String.format("/group?groupId=%s", groupId)); final ObjectMapper mapper = new ObjectMapper(); @@ -259,6 +284,10 @@ public class CloudianClient { } public boolean updateGroup(final CloudianGroup group) { + if (group == null) { + return false; + } + LOG.debug("Updating Cloudian group: " + group); try { final HttpResponse response = post("/group", group); return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK; @@ -272,6 +301,10 @@ public class CloudianClient { } public boolean removeGroup(final String groupId) { + if (Strings.isNullOrEmpty(groupId)) { + return false; + } + LOG.debug("Removing Cloudian group id=" + groupId); try { final HttpResponse response = delete(String.format("/group?groupId=%s", groupId)); return response.getStatusLine().getStatusCode() == HttpStatus.SC_OK; @@ -283,5 +316,4 @@ public class CloudianClient { } return false; } - } diff --git a/plugins/integrations/cloudian/src/com/cloudian/client/CloudianGroup.java b/plugins/integrations/cloudian/src/com/cloudian/client/CloudianGroup.java index 09c0cf5db22..ffa1df4f168 100644 --- a/plugins/integrations/cloudian/src/com/cloudian/client/CloudianGroup.java +++ b/plugins/integrations/cloudian/src/com/cloudian/client/CloudianGroup.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; @JsonIgnoreProperties(ignoreUnknown = true) public class CloudianGroup { - String groupId; String groupName; Boolean active; diff --git a/plugins/integrations/cloudian/src/com/cloudian/client/CloudianUser.java b/plugins/integrations/cloudian/src/com/cloudian/client/CloudianUser.java index 64860271992..2eae37de4f6 100644 --- a/plugins/integrations/cloudian/src/com/cloudian/client/CloudianUser.java +++ b/plugins/integrations/cloudian/src/com/cloudian/client/CloudianUser.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; @JsonIgnoreProperties(ignoreUnknown = true) public class CloudianUser { - public static final String USER = "User"; String userId; diff --git a/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianConnectorImpl.java b/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianConnectorImpl.java index dd84702c279..4ff43f3a7b0 100644 --- a/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianConnectorImpl.java +++ b/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianConnectorImpl.java @@ -77,9 +77,9 @@ public class CloudianConnectorImpl extends ComponentLifecycleBase implements Clo CloudianAdminUser.value(), CloudianAdminPassword.value(), CloudianValidateSSLSecurity.value(), CloudianAdminApiRequestTimeout.value()); } catch (final KeyStoreException | NoSuchAlgorithmException | KeyManagementException e) { - LOG.error("Failed to create Cloudian client due to: ", e); + LOG.error("Failed to create Cloudian API client due to: ", e); } - throw new CloudRuntimeException("Failed to create and return Cloudian client instance"); + throw new CloudRuntimeException("Failed to create and return Cloudian API client instance"); } private boolean addOrUpdateGroup(final Domain domain) { @@ -90,15 +90,12 @@ public class CloudianConnectorImpl extends ComponentLifecycleBase implements Clo final CloudianGroup existingGroup = client.listGroup(domain.getUuid()); if (existingGroup != null) { if (!existingGroup.getActive() || !existingGroup.getGroupName().equals(domain.getPath())) { - LOG.debug("Updating Cloudian group for domain uuid=" + domain.getUuid() + " name=" + domain.getName() + " path=" + domain.getPath()); existingGroup.setActive(true); existingGroup.setGroupName(domain.getPath()); return client.updateGroup(existingGroup); } return true; } - - LOG.debug("Adding Cloudian group for domain uuid=" + domain.getUuid() + " name=" + domain.getName() + " path=" + domain.getPath()); final CloudianGroup group = new CloudianGroup(); group.setGroupId(domain.getUuid()); group.setGroupName(domain.getPath()); @@ -111,13 +108,20 @@ public class CloudianConnectorImpl extends ComponentLifecycleBase implements Clo return false; } final CloudianClient client = getClient(); - LOG.debug("Removing Cloudian group for domain uuid=" + domain.getUuid() + " name=" + domain.getName() + " path=" + domain.getPath()); for (final CloudianUser user: client.listUsers(domain.getUuid())) { if (client.removeUser(user.getUserId(), domain.getUuid())) { LOG.error(String.format("Failed to remove Cloudian user id=%s, while removing Cloudian group id=%s", user.getUserId(), domain.getUuid())); } } - return client.removeGroup(domain.getUuid()); + for (int retry = 0; retry < 3; retry++) { + if (client.removeGroup(domain.getUuid())) { + return true; + } else { + LOG.warn("Failed to remove Cloudian group id=" + domain.getUuid() + ", retrying count=" + retry+1); + } + } + LOG.warn("Failed to remove Cloudian group id=" + domain.getUuid() + ", please remove manually"); + return false; } private boolean addOrUpdateUserAccount(final Account account, final Domain domain) { @@ -130,7 +134,6 @@ public class CloudianConnectorImpl extends ComponentLifecycleBase implements Clo final CloudianUser existingUser = client.listUser(account.getUuid(), domain.getUuid()); if (existingUser != null) { if (!existingUser.getActive() || !existingUser.getFullName().equals(fullName)) { - LOG.debug("Updating Cloudian user for account with uuid=" + account.getUuid() + " name=" + account.getAccountName()); existingUser.setActive(true); existingUser.setEmailAddr(accountUser.getEmail()); existingUser.setFullName(fullName); @@ -138,8 +141,6 @@ public class CloudianConnectorImpl extends ComponentLifecycleBase implements Clo } return true; } - - LOG.debug("Adding Cloudian user for account with uuid=" + account.getUuid() + " name=" + account.getAccountName()); final CloudianUser user = new CloudianUser(); user.setUserId(account.getUuid()); user.setGroupId(domain.getUuid()); @@ -156,8 +157,15 @@ public class CloudianConnectorImpl extends ComponentLifecycleBase implements Clo } final CloudianClient client = getClient(); final Domain domain = domainDao.findById(account.getDomainId()); - LOG.debug("Removing Cloudian user for account with uuid=" + account.getUuid() + " name=" + account.getAccountName()); - return client.removeUser(account.getUuid(), domain.getUuid()); + for (int retry = 0; retry < 3; retry++) { + if (client.removeUser(account.getUuid(), domain.getUuid())) { + return true; + } else { + LOG.warn("Failed to remove Cloudian user id=" + account.getUuid() + " in group id=" + domain.getUuid() + ", retrying count=" + retry+1); + } + } + LOG.warn("Failed to remove Cloudian user id=" + account.getUuid() + " in group id=" + domain.getUuid() + ", please remove manually"); + return false; } ////////////////////////////////////////////////// diff --git a/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianUtils.java b/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianUtils.java index bffcf8f1c5c..b8aae27b697 100644 --- a/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianUtils.java +++ b/plugins/integrations/cloudian/src/com/cloudian/cloudstack/CloudianUtils.java @@ -45,8 +45,8 @@ public class CloudianUtils { return null; } try { - SecretKeySpec signingKey = new SecretKeySpec(key.getBytes(), HMAC_SHA1_ALGORITHM); - Mac mac = Mac.getInstance(HMAC_SHA1_ALGORITHM); + final SecretKeySpec signingKey = new SecretKeySpec(key.getBytes(), HMAC_SHA1_ALGORITHM); + final Mac mac = Mac.getInstance(HMAC_SHA1_ALGORITHM); mac.init(signingKey); byte[] rawHmac = mac.doFinal(data.getBytes()); return Base64.encodeBase64String(rawHmac); diff --git a/plugins/integrations/cloudian/src/com/cloudian/cloudstack/response/CloudianEnabledResponse.java b/plugins/integrations/cloudian/src/com/cloudian/cloudstack/response/CloudianEnabledResponse.java index c4fb8fdb587..ce935cb4b1b 100644 --- a/plugins/integrations/cloudian/src/com/cloudian/cloudstack/response/CloudianEnabledResponse.java +++ b/plugins/integrations/cloudian/src/com/cloudian/cloudstack/response/CloudianEnabledResponse.java @@ -32,18 +32,10 @@ public class CloudianEnabledResponse extends BaseResponse { @Param(description = "the Cloudian Management Console base URL") private String cmcUrl; - public Boolean getEnabled() { - return enabled; - } - public void setEnabled(Boolean enabled) { this.enabled = enabled; } - public String getCmcUrl() { - return cmcUrl; - } - public void setCmcUrl(String cmcUrl) { this.cmcUrl = cmcUrl; } diff --git a/plugins/integrations/cloudian/test/com/cloudian/client/CloudianClientTest.java b/plugins/integrations/cloudian/test/com/cloudian/client/CloudianClientTest.java index e4fc946937a..defcbcfc320 100644 --- a/plugins/integrations/cloudian/test/com/cloudian/client/CloudianClientTest.java +++ b/plugins/integrations/cloudian/test/com/cloudian/client/CloudianClientTest.java @@ -44,7 +44,6 @@ import com.github.tomakehurst.wiremock.client.BasicCredentials; import com.github.tomakehurst.wiremock.junit.WireMockRule; public class CloudianClientTest { - private final int port = 14333; private final int timeout = 2; private final String adminUsername = "admin"; @@ -365,5 +364,4 @@ public class CloudianClientTest { boolean result = client.removeGroup(group.getGroupId()); Assert.assertFalse(result); } - } \ No newline at end of file