[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/1485


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[swill]

Github user swill commented on the pull request:

https://github.com/apache/cloudstack/pull/1485#issuecomment-216578573
  
My CI does not have the capability of testing this, so even if I ran CI it 
would not cover this.  I tend to agree that his is fine as is.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[kiwiflyer]

Github user kiwiflyer commented on the pull request:

https://github.com/apache/cloudstack/pull/1485#issuecomment-216516629
  
@swill This has 2 x LGTM. Simple change and probably doesn't require any 
additional CI IMO.
This is Ready to Merge


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[nlivens]

Github user nlivens commented on the pull request:

https://github.com/apache/cloudstack/pull/1485#issuecomment-216513556
  
According to the explanation of @remibergsma, and the code changes : LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1485#issuecomment-216435060
  
tag:easypr


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1485#issuecomment-216435050
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[imduffy15]

Github user imduffy15 commented on the pull request:

https://github.com/apache/cloudstack/pull/1485#issuecomment-216353475
  
I'm not familiar with that block of code, remi's changes and justification 
seem fine though.
Some very oddly/badly worded variable names. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[rhtyd]

Github user rhtyd commented on the pull request:

https://github.com/apache/cloudstack/pull/1485#issuecomment-216227853
  
@karuturi @imduffy15 comments on the change, thanks

tag:needlove


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Set default networkDomain to empty instea...

[remibergsma]

GitHub user remibergsma opened a pull request:

https://github.com/apache/cloudstack/pull/1485

Set default networkDomain to empty instead of username

The 10th field of `createUserAccount` is `networkDomain` (See 
`AccountService.java`) and it is set to a var named `admin`, which is the user 
name.
So, the first user that is created in a domain that links to LDAP, creates 
the account within the domain, and sets the `networkDomain` field to the 
username. All next users are created in the same account.

Then we have the situation that in domain SBP we have a user `rbergsma` 
that logs in first, gets an account created and then (unless you override) all 
VMs started in the SBP domain will have network domain `rbergsma`. That is 
highly confusing and not what is should be.

The `linkDomainToLdap` api call has no `networkDomain` field, so I propose 
to make this field empty (set it to null). It's a sting and null / empty is 
allowed.

One can also specify the networkDomain when creating a VPC and also there 
it is allowed to be null.

When te networkDomain is needed (and is not set in the domain and not in 
the VPC) it is constructed by using `guest.domain.suffix` so there always is a 
networkDomain to be used.

It makes more sense to manually set it on a domain level, or specify it on 
the VPC and in the final case end up with something that is clearly generated 
(like cs342cloud.local) rather than the username of someone else.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/remibergsma/cloudstack fix-ldap-default-domain

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/1485.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1485


commit 9e1859ee2bbe82ad742c30cd9ca9aa7393d34f36
Author: Remi Bergsma 
Date:   2016-04-10T17:50:32Z

Set default networkDomain to empty instead of username

The 10th field of createUserAccount is 'networkDomain' 
(AccountService.java) and it is set to a var named 'admin', which is the user 
name.
So, the first user that is created in a domain that links to LDAP, creates 
the account within the domain, and sets the 'networkDomain' field to the 
username. All next users are created in the same account.

Then we have the situation that in domain SBP we have a user 'rbergsma' 
that logs in first, gets an account created and then (unless you override) all 
VMs started in the SBP domain will have network domain 'rbergsma'. That is 
highly confusing and not what is should be.

linkDomainToLdap api call has no 'networkDomain' field, so I propose to 
make this field empty (set it to null). It's a sting and null / empty is 
allowed.

One can also specify the networkDomain when creating a VPC and also there 
it is allowed to be null.

When te networkDomain is needed (and is not set in the domain and not in 
the VPC) it is constructed by using guest.domain.suffix so there always is a 
netWork domain to be used.

It makes more sense to manually set it on a domain level, or specify it on 
the VPC and in the final case end up with something that is clearly generated 
(like cs342cloud.local) rather than the username of someone else.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---