[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10205?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16310935#comment-16310935
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10205:
---------------------------------------------

rhtyd closed pull request #2378: CLOUDSTACK-10205 make LinkDomainToLdap return 
UUID instead of internal id
URL: https://github.com/apache/cloudstack/pull/2378
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java 
b/api/src/org/apache/cloudstack/api/ApiConstants.java
index 55e8c285e6b..98b5ccb99a1 100644
--- a/api/src/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/org/apache/cloudstack/api/ApiConstants.java
@@ -705,6 +705,7 @@
 
     public static final String HAS_ANNOTATION = "hasannotation";
     public static final String LAST_ANNOTATED = "lastannotated";
+    public static final String LDAP_DOMAIN = "ldapdomain";
 
     public enum HostDetails {
         all, capacity, events, stats, min;
diff --git 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java
 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java
index b0032b04b4d..050eb6c3eb5 100644
--- 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java
+++ 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/api/response/LinkDomainToLdapResponse.java
@@ -27,12 +27,17 @@
 
     @SerializedName(ApiConstants.DOMAIN_ID)
     @Param(description = "id of the Domain which is linked to LDAP")
-    private long domainId;
+    private String domainId;
 
+    @Deprecated
     @SerializedName(ApiConstants.NAME)
     @Param(description = "name of the group or OU in LDAP which is linked to 
the domain")
     private String name;
 
+    @SerializedName(ApiConstants.LDAP_DOMAIN)
+    @Param(description = "name of the group or OU in LDAP which is linked to 
the domain")
+    private String ldapDomain;
+
     @SerializedName(ApiConstants.TYPE)
     @Param(description = "type of the name in LDAP which is linke to the 
domain")
     private String type;
@@ -45,19 +50,25 @@
     @Param(description = "Domain Admin accountId that is created")
     private String adminId;
 
-    public LinkDomainToLdapResponse(long domainId, String type, String name, 
short accountType) {
+    public LinkDomainToLdapResponse(String domainId, String type, String 
ldapDomain, short accountType) {
         this.domainId = domainId;
-        this.name = name;
+        this.name = ldapDomain;
+        this.ldapDomain = ldapDomain;
         this.type = type;
         this.accountType = accountType;
     }
 
-    public long getDomainId() {
+    public String getDomainId() {
         return domainId;
     }
 
+    public String getLdapDomain() {
+        return ldapDomain == null ? name : ldapDomain;
+    }
+
+    @Deprecated
     public String getName() {
-        return name;
+        return ldapDomain == null ? name : ldapDomain;
     }
 
     public String getType() {
diff --git 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
index a4d340647f4..beb7a61cd70 100644
--- 
a/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
+++ 
b/plugins/user-authenticators/ldap/src/org/apache/cloudstack/ldap/LdapManagerImpl.java
@@ -44,6 +44,8 @@
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.utils.Pair;
 
@@ -54,6 +56,9 @@
     @Inject
     private LdapConfigurationDao _ldapConfigurationDao;
 
+    @Inject
+    private DomainDao domainDao;
+
     @Inject
     private LdapContextFactory _ldapContextFactory;
 
@@ -270,7 +275,14 @@ public LinkDomainToLdapResponse linkDomainToLdap(Long 
domainId, String type, Str
         Validate.isTrue(accountType==0 || accountType==2, "accountype should 
be either 0(normal user) or 2(domain admin)");
         LinkType linkType = LdapManager.LinkType.valueOf(type.toUpperCase());
         LdapTrustMapVO vo = _ldapTrustMapDao.persist(new 
LdapTrustMapVO(domainId, linkType, name, accountType));
-        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(vo.getDomainId(), vo.getType().toString(), 
vo.getName(), vo.getAccountType());
+        DomainVO domain = domainDao.findById(vo.getDomainId());
+        String domainUuid = "<unknown>";
+        if (domain == null) {
+            s_logger.error("no domain in database for id " + vo.getDomainId());
+        } else {
+            domainUuid = domain.getUuid();
+        }
+        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainUuid, vo.getType().toString(), vo.getName(), 
vo.getAccountType());
         return response;
     }
 
diff --git 
a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy
 
b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy
index 9d667bf4cfb..bad41e82805 100644
--- 
a/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy
+++ 
b/plugins/user-authenticators/ldap/test/groovy/org/apache/cloudstack/ldap/LinkDomainToLdapCmdSpec.groovy
@@ -60,7 +60,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
             thrown(ServerApiException)
     }
     def "test valid params without admin"(){
-        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse(1, 
"GROUP", "CN=test,DC=ccp,DC=citrix,DC=com", (short)2)
+        LinkDomainToLdapResponse response = new LinkDomainToLdapResponse("1", 
"GROUP", "CN=test,DC=ccp,DC=citrix,DC=com", (short)2)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         when:
             linkDomainToLdapCmd.execute()
@@ -71,7 +71,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
     }
 
     def "test with valid params and with disabled admin"() {
-        def domainId = 1;
+        def domainId = "1";
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
@@ -99,13 +99,13 @@ class LinkDomainToLdapCmdSpec extends Specification {
     }
 
     def "test with valid params and with admin who exist in cloudstack 
already"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
 
-        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> new LdapUser(username, 
"ad...@ccp.citrix.com", "Admin", "Admin", name, "ccp", false)
 
@@ -122,21 +122,21 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = 
(LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == null
     }
 
     def "test with valid params and with admin who doesnt exist in 
cloudstack"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
         def accountId = 24
 
-        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> new LdapUser(username, 
"ad...@ccp.citrix.com", "Admin", "Admin", name, "ccp", false)
 
@@ -157,20 +157,20 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = 
(LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == String.valueOf(accountId)
     }
 
     def "test when admin doesnt exist in ldap"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
 
-        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> {throw new 
NoLdapUserMatchingQueryException("get ldap user failed from mock")}
 
@@ -185,7 +185,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = 
(LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == null
@@ -195,14 +195,14 @@ class LinkDomainToLdapCmdSpec extends Specification {
      * api should not fail in this case as link domain to ldap is successful
      */
     def "test when create user account throws a run time exception"() {
-        def domainId = 1;
+        def domainId = 1L;
         def type = "GROUP";
         def name = "CN=test,DC=ccp,DC=Citrix,DC=com"
         def accountType = 2;
         def username = "admin"
         def accountId = 24
 
-        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId, type, name, (short)accountType)
+        LinkDomainToLdapResponse response = new 
LinkDomainToLdapResponse(domainId.toString(), type, name, (short)accountType)
         _ldapManager.linkDomainToLdap(_,_,_,_) >> response
         _ldapManager.getUser(username, type, name) >> new LdapUser(username, 
"ad...@ccp.citrix.com", "Admin", "Admin", name, "ccp", false)
 
@@ -223,7 +223,7 @@ class LinkDomainToLdapCmdSpec extends Specification {
         LinkDomainToLdapResponse result = 
(LinkDomainToLdapResponse)linkDomainToLdapCmd.getResponseObject()
         result.getObjectName() == "LinkDomainToLdap"
         result.getResponseName() == linkDomainToLdapCmd.getCommandName()
-        result.getDomainId() == domainId
+        result.getDomainId() == domainId.toString()
         result.getType() == type
         result.getName() == name
         result.getAdminId() == null


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> LinkDomainToLdap returns internal id
> ------------------------------------
>
>                 Key: CLOUDSTACK-10205
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10205
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>            Reporter: Daan Hoogland
>
> The response should contain the uuid for the domain and not the internal db 
> id.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to