Review Request 203: Domain Controller: Use instance name for the domain

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/203/
-----------------------------------------------------------

Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
Gallagher, and Thomas Woerner.


Repository: rolekit


Description
-------

This patch changes the default behavior of the domain controller
role. After discussions with UXD experts, it was determined that
the domain should never be auto-detected from the environment,
since it is very difficult to change later. It should always come
from some user action.

This patch changes the default domain to use the role instance name
for the domain name, as long as the instance name contains valid
characters. It also changes the behavior of the host_name option to
set the hostname to dc-<16 characters> if it is currently set to
localhost.


Diffs
-----

  config/roles/domaincontroller/role.py 62d2bc64d6efb9c2b7e254c8d61c612f05e4c4be 
  doc/xml/rolekit.roles.domaincontroller.xml 2416c889ba0035dc6247b2eb209dbe13eb3b9708 

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/


Testing
-------

Deployed multiple domain controllers with varying initial hostnames.


Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/203/
-----------------------------------------------------------

(Updated Sept. 2, 2015, 2:14 p.m.)


Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
Gallagher, and Thomas Woerner.


Changes
-------

Link to the upstream bug


Bugs: 43
    http://github.com/libre-server/rolekit/issues#issue/43


Repository: rolekit


Description (updated)
-------

This patch changes the default behavior of the domain controller
role. After discussions with UXD experts, it was determined that
the domain should never be auto-detected from the environment,
since it is very difficult to change later. It should always come
from some user action.

This patch changes the default domain to use the role instance name
for the domain name, as long as the instance name contains valid
characters. It also changes the behavior of the host_name option to
set the hostname to dc-<16 characters> if it is currently set to
localhost.

Fixes https://github.com/libre-server/rolekit/issues/43


Diffs (updated)
-----

  config/roles/domaincontroller/role.py 62d2bc64d6efb9c2b7e254c8d61c612f05e4c4be 
  doc/xml/rolekit.roles.domaincontroller.xml 2416c889ba0035dc6247b2eb209dbe13eb3b9708 

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/


Testing
-------

Deployed multiple domain controllers with varying initial hostnames.


Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/203/#review506
-----------------------------------------------------------



config/roles/domaincontroller/role.py (lines 260 - 264)
<http://reviewboard-fedoraserver.rhcloud.com/r/203/#comment295>

    Do you want to resurrect this code later? If not, these lines should just go away.



config/roles/domaincontroller/role.py (lines 266 - 271)
<http://reviewboard-fedoraserver.rhcloud.com/r/203/#comment296>

    The `raise` in line 266 prevents any later lines from being executed, should they just
go away, or is this in the wrong place?



config/roles/domaincontroller/role.py (lines 476 - 477)
<http://reviewboard-fedoraserver.rhcloud.com/r/203/#comment297>

    The compiled regex object is used and immediately thrown away here -- which is not a
problem, just odd.
    
    Not quoting the regex string with `r"..."` isn't a problem right now
unless someone fiddles with it in the wrong way (i.e. when the backslash becomes a quote
for the next character of the string and won't be passed on to the regex compiler)...
but it's bad practice ;).
    
    You could just do this instead:
    
        return all(re.match(
            r"(?!-)[A-Z\d-]{1,63}(?<!-)$", x, re.IGNORECASE)
            for x in fqdn.split("."))
            
    Or alternatively compile the regex object once and store it as a class member, of
course.


- Nils Philippsen


On Sept. 2, 2015, 4:14 nachm., Stephen Gallagher wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-fedoraserver.rhcloud.com/r/203/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 2, 2015, 4:14 nachm.)
>  
>  
>  Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
> Gallagher, and Thomas Woerner.
>  
>  
>  Bugs: 43
>      http://github.com/libre-server/rolekit/issues#issue/43
>  
>  
>  Repository: rolekit
>  
>  
>  Description
>  -------
>  
>  This patch changes the default behavior of the domain controller
>  role. After discussions with UXD experts, it was determined that
>  the domain should never be auto-detected from the environment,
>  since it is very difficult to change later. It should always come
>  from some user action.
>  
>  This patch changes the default domain to use the role instance name
>  for the domain name, as long as the instance name contains valid
>  characters. It also changes the behavior of the host_name option to
>  set the hostname to dc-<16 characters> if it is currently set to
>  localhost.
>  
>  Fixes https://github.com/libre-server/rolekit/issues/43
>  
>  
>  Diffs
>  -----
>  
>    config/roles/domaincontroller/role.py 62d2bc64d6efb9c2b7e254c8d61c612f05e4c4be 
>    doc/xml/rolekit.roles.domaincontroller.xml 2416c889ba0035dc6247b2eb209dbe13eb3b9708 
>  
>  Diff: http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/
>  
>  
>  Testing
>  -------
>  
>  Deployed multiple domain controllers with varying initial hostnames.
>  
>  
>  Thanks,
>  
>  Stephen Gallagher
>  
>

[Stephen Gallagher]

>  On Sept. 2, 2015, 3:25 p.m., Nils Philippsen wrote:
>  > config/roles/domaincontroller/role.py, lines 273-277
>  >
> <http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/2/?file=1009#file1...
>  >
>  >     Do you want to resurrect this code later? If not, these lines should just go
> away. 
Oops, I commented those out for testing only.


>  On Sept. 2, 2015, 3:25 p.m., Nils Philippsen wrote:
>  > config/roles/domaincontroller/role.py, lines 279-286
>  >
> <http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/2/?file=1009#file1...
>  >
>  >     The `raise` in line 266 prevents any later lines from being executed, should
> they just go away, or is this in the wrong place? 
Ditto above. This was just in there to make testing this fast (by skipping the actual
deployment)


- Stephen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/203/#review506
-----------------------------------------------------------


On Sept. 2, 2015, 2:14 p.m., Stephen Gallagher wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-fedoraserver.rhcloud.com/r/203/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 2, 2015, 2:14 p.m.)
>  
>  
>  Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
> Gallagher, and Thomas Woerner.
>  
>  
>  Bugs: 43
>      http://github.com/libre-server/rolekit/issues#issue/43
>  
>  
>  Repository: rolekit
>  
>  
>  Description
>  -------
>  
>  This patch changes the default behavior of the domain controller
>  role. After discussions with UXD experts, it was determined that
>  the domain should never be auto-detected from the environment,
>  since it is very difficult to change later. It should always come
>  from some user action.
>  
>  This patch changes the default domain to use the role instance name
>  for the domain name, as long as the instance name contains valid
>  characters. It also changes the behavior of the host_name option to
>  set the hostname to dc-<16 characters> if it is currently set to
>  localhost.
>  
>  Fixes https://github.com/libre-server/rolekit/issues/43
>  
>  
>  Diffs
>  -----
>  
>    config/roles/domaincontroller/role.py 62d2bc64d6efb9c2b7e254c8d61c612f05e4c4be 
>    doc/xml/rolekit.roles.domaincontroller.xml 2416c889ba0035dc6247b2eb209dbe13eb3b9708 
>  
>  Diff: http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/
>  
>  
>  Testing
>  -------
>  
>  Deployed multiple domain controllers with varying initial hostnames.
>  
>  
>  Thanks,
>  
>  Stephen Gallagher
>  
>

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/203/
-----------------------------------------------------------

(Updated Sept. 2, 2015, 3:37 p.m.)


Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
Gallagher, and Thomas Woerner.


Changes
-------

Restored commented-out shortcut.

Moved compiled regular expression to the role object.


Bugs: 43
    http://github.com/libre-server/rolekit/issues#issue/43


Repository: rolekit


Description
-------

This patch changes the default behavior of the domain controller
role. After discussions with UXD experts, it was determined that
the domain should never be auto-detected from the environment,
since it is very difficult to change later. It should always come
from some user action.

This patch changes the default domain to use the role instance name
for the domain name, as long as the instance name contains valid
characters. It also changes the behavior of the host_name option to
set the hostname to dc-<16 characters> if it is currently set to
localhost.

Fixes https://github.com/libre-server/rolekit/issues/43


Diffs (updated)
-----

  doc/xml/rolekit.roles.domaincontroller.xml 2416c889ba0035dc6247b2eb209dbe13eb3b9708 
  config/roles/domaincontroller/role.py 62d2bc64d6efb9c2b7e254c8d61c612f05e4c4be 

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/


Testing
-------

Deployed multiple domain controllers with varying initial hostnames.


Thanks,

Stephen Gallagher

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/203/#review508
-----------------------------------------------------------

Ship it!



config/roles/domaincontroller/role.py (lines 133 - 138)
<http://reviewboard-fedoraserver.rhcloud.com/r/203/#comment300>

    I know this seems super-pedantic, but why not compile the regex once on the class
level (if we want to optimize this point ;).



config/roles/domaincontroller/role.py (line 329)
<http://reviewboard-fedoraserver.rhcloud.com/r/203/#comment301>

    Some day I'll replace all old style string formatting with new-style calls to
`.format()`, I swear. ;)


- Nils Philippsen


On Sept. 2, 2015, 5:37 nachm., Stephen Gallagher wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-fedoraserver.rhcloud.com/r/203/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 2, 2015, 5:37 nachm.)
>  
>  
>  Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
> Gallagher, and Thomas Woerner.
>  
>  
>  Bugs: 43
>      http://github.com/libre-server/rolekit/issues#issue/43
>  
>  
>  Repository: rolekit
>  
>  
>  Description
>  -------
>  
>  This patch changes the default behavior of the domain controller
>  role. After discussions with UXD experts, it was determined that
>  the domain should never be auto-detected from the environment,
>  since it is very difficult to change later. It should always come
>  from some user action.
>  
>  This patch changes the default domain to use the role instance name
>  for the domain name, as long as the instance name contains valid
>  characters. It also changes the behavior of the host_name option to
>  set the hostname to dc-<16 characters> if it is currently set to
>  localhost.
>  
>  Fixes https://github.com/libre-server/rolekit/issues/43
>  
>  
>  Diffs
>  -----
>  
>    doc/xml/rolekit.roles.domaincontroller.xml 2416c889ba0035dc6247b2eb209dbe13eb3b9708 
>    config/roles/domaincontroller/role.py 62d2bc64d6efb9c2b7e254c8d61c612f05e4c4be 
>  
>  Diff: http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/
>  
>  
>  Testing
>  -------
>  
>  Deployed multiple domain controllers with varying initial hostnames.
>  
>  
>  Thanks,
>  
>  Stephen Gallagher
>  
>

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/203/
-----------------------------------------------------------

(Updated Sept. 2, 2015, 4:01 p.m.)


Status
------

This change has been marked as submitted.


Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
Gallagher, and Thomas Woerner.


Bugs: 43
    http://github.com/libre-server/rolekit/issues#issue/43


Repository: rolekit


Description
-------

This patch changes the default behavior of the domain controller
role. After discussions with UXD experts, it was determined that
the domain should never be auto-detected from the environment,
since it is very difficult to change later. It should always come
from some user action.

This patch changes the default domain to use the role instance name
for the domain name, as long as the instance name contains valid
characters. It also changes the behavior of the host_name option to
set the hostname to dc-<16 characters> if it is currently set to
localhost.

Fixes https://github.com/libre-server/rolekit/issues/43


Diffs
-----

  doc/xml/rolekit.roles.domaincontroller.xml 2416c889ba0035dc6247b2eb209dbe13eb3b9708 
  config/roles/domaincontroller/role.py 62d2bc64d6efb9c2b7e254c8d61c612f05e4c4be 

Diff: http://reviewboard-fedoraserver.rhcloud.com/r/203/diff/


Testing
-------

Deployed multiple domain controllers with varying initial hostnames.


Thanks,

Stephen Gallagher