----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/ -----------------------------------------------------------
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description -------
Domain Controller deployment
Diffs -----
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing -------
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review210 -----------------------------------------------------------
config/roles/domaincontroller/role.py http://reviewboard-fedoraserver.rhcloud.com/r/63/#comment119
Don't you need to list kerberos and https ports too ? (and ntp)
config/roles/domaincontroller/role.py http://reviewboard-fedoraserver.rhcloud.com/r/63/#comment120
Are we sure it is a good idea to default the DM password to the admin password ?
- Simo Sorce
On Aug. 1, 2014, 9:11 p.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 1, 2014, 9:11 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
On Aug. 2, 2014, 11:41 a.m., Simo Sorce wrote:
config/roles/domaincontroller/role.py, lines 47-48 http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/1/?file=254#file254line47
Don't you need to list kerberos and https ports too ? (and ntp)
The firewalld service objects contain the complete set of available ports. There are two flavors of them, one that has everything but ldaps and one that has everything but ldap-on-389. For the Fedora Server, we're defaulting to having both 389 and 636 open.
On Aug. 2, 2014, 11:41 a.m., Simo Sorce wrote:
config/roles/domaincontroller/role.py, line 157 http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/1/?file=254#file254line157
Are we sure it is a good idea to default the DM password to the admin password ?
We're trying to simplify things as much as is reasonably possible. You can *always* choose to specify it, but after talking to UXD folks, they mostly agree that it's acceptable for the initial passwords to be the same.
Another option would be to randomly-generate the admin password and return it, but that will require some consideration (since it will mean revising the API to expect return values).
- Stephen
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review210 -----------------------------------------------------------
On Aug. 1, 2014, 9:11 p.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 1, 2014, 9:11 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/ -----------------------------------------------------------
(Updated Aug. 20, 2014, 2:09 a.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Changes -------
New patches rebased atop the new async mechanism.
I converted dm_password over to generating and saving a random password rather than reusing the submitted admin_password.
I modified the parameters visible to DBUS to remove admin_password (as it's not directly related to the service).
This patch also implements start() and stop() for services.
Repository: rolekit
Description -------
Domain Controller deployment
Diffs (updated) -----
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing -------
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review240 -----------------------------------------------------------
config/roles/domaincontroller/role.py http://reviewboard-fedoraserver.rhcloud.com/r/63/#comment142
Please use self._settings and not self._DEFAULTS here.
- Thomas Woerner
On Aug. 20, 2014, 2:09 a.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 2:09 a.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
On Aug. 20, 2014, 5:16 p.m., Thomas Woerner wrote:
config/roles/domaincontroller/role.py, line 146 http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/2/?file=282#file282line146
Please use self._settings and not self._DEFAULTS here.
If the deploy method will be called later on again (maybe in redeploy) then the former settings will not be used.
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review240 -----------------------------------------------------------
On Aug. 20, 2014, 2:09 a.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 2:09 a.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
On Aug. 20, 2014, 5:16 p.m., Thomas Woerner wrote:
config/roles/domaincontroller/role.py, line 146 http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/2/?file=282#file282line146
Please use self._settings and not self._DEFAULTS here.
Thomas Woerner wrote: If the deploy method will be called later on again (maybe in redeploy) then the former settings will not be used.
Now self._settings and self._DEFAULTS is not used at all anymore. With this change there might be some settings missing for example in a (re)deploy call.
It might be good to call do_deploy in RoleBase with the a copy of _settings updated with values. RoleBase.deploy_async needs to be changed to apply this copy then after the do_deploy call to _settings again. RoleBase.apply_values would need to be changed to not complain on readonly settings but silently ignore them - which is not nice, but with the check_values call before do_deploy this should not lead into unexpected losses.
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review240 -----------------------------------------------------------
On Aug. 20, 2014, 8:59 p.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 8:59 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
On Aug. 20, 2014, 5:16 p.m., Thomas Woerner wrote:
config/roles/domaincontroller/role.py, line 146 http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/2/?file=282#file282line146
Please use self._settings and not self._DEFAULTS here.
Thomas Woerner wrote: If the deploy method will be called later on again (maybe in redeploy) then the former settings will not be used.
Thomas Woerner wrote: Now self._settings and self._DEFAULTS is not used at all anymore. With this change there might be some settings missing for example in a (re)deploy call.
It might be good to call do_deploy in RoleBase with the a copy of _settings updated with values. RoleBase.deploy_async needs to be changed to apply this copy then after the do_deploy call to _settings again. RoleBase.apply_values would need to be changed to not complain on readonly settings but silently ignore them - which is not nice, but with the check_values call before do_deploy this should not lead into unexpected losses.
Drop my last comment..
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review240 -----------------------------------------------------------
On Aug. 20, 2014, 8:59 p.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 8:59 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
On Aug. 20, 2014, 5:16 p.m., Thomas Woerner wrote:
config/roles/domaincontroller/role.py, line 146 http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/2/?file=282#file282line146
Please use self._settings and not self._DEFAULTS here.
Thomas Woerner wrote: If the deploy method will be called later on again (maybe in redeploy) then the former settings will not be used.
Thomas Woerner wrote: Now self._settings and self._DEFAULTS is not used at all anymore. With this change there might be some settings missing for example in a (re)deploy call.
It might be good to call do_deploy in RoleBase with the a copy of _settings updated with values. RoleBase.deploy_async needs to be changed to apply this copy then after the do_deploy call to _settings again. RoleBase.apply_values would need to be changed to not complain on readonly settings but silently ignore them - which is not nice, but with the check_values call before do_deploy this should not lead into unexpected losses.
Thomas Woerner wrote: Drop my last comment..
We discussed this on IRC and decided that at least for now it makes sense to require subsequent deploy() attempts to submit the full set of values, not just the delta from the last attempt.
- Stephen
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review240 -----------------------------------------------------------
On Aug. 20, 2014, 8:59 p.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 8:59 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review241 -----------------------------------------------------------
config/roles/domaincontroller/role.py http://reviewboard-fedoraserver.rhcloud.com/r/63/#comment143
if the admin_password may not be exported, then you should return an empty string. Ottherwise "return super(Role, x).get_dbus_property(x, prop)" further down will return it.
- Thomas Woerner
On Aug. 20, 2014, 2:09 a.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 2:09 a.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
On Aug. 20, 2014, 5:21 p.m., Thomas Woerner wrote:
config/roles/domaincontroller/role.py, line 297 http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/2/?file=282#file282line297
if the admin_password may not be exported, then you should return an empty string. Ottherwise "return super(Role, x).get_dbus_property(x, prop)" further down will return it.
I reworked how I'm handling the values here so that I update the 'values' variable so that it won't return the admin_password for saving. I also added an explicit UNKNOWN_SETTING exception if it is directly requested.
- Stephen
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review241 -----------------------------------------------------------
On Aug. 20, 2014, 2:09 a.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 2:09 a.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review242 -----------------------------------------------------------
config/roles/domaincontroller/role.py http://reviewboard-fedoraserver.rhcloud.com/r/63/#comment144
The admin_password will be saved in _settings. If this should not be done, then remove it from values.
- Thomas Woerner
On Aug. 20, 2014, 2:09 a.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 2:09 a.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/ -----------------------------------------------------------
(Updated Aug. 20, 2014, 8:59 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Changes -------
I reworked how the values were set and retrieved so that they are all saved to the json (and the admin_password is not).
Repository: rolekit
Description -------
Domain Controller deployment
Diffs (updated) -----
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing -------
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/#review265 -----------------------------------------------------------
Ship it!
Ship It!
- Thomas Woerner
On Aug. 20, 2014, 8:59 p.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/
(Updated Aug. 20, 2014, 8:59 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description
Domain Controller deployment
Diffs
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/ -----------------------------------------------------------
(Updated Aug. 21, 2014, 3:13 p.m.)
Status ------
This change has been marked as submitted.
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
Repository: rolekit
Description -------
Domain Controller deployment
Diffs -----
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
Testing -------
Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.
(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).
Thanks,
Stephen Gallagher
rolekit-commits@lists.fedorahosted.org