20190702 update kdb support for 389 #52
Conversation
This is a large rework of kdb for 389, fixing a large number of issues that prevented the kdb setup from operating correctly. It now is able to correctly configure and setup Kerberos against a 389 Directory Server instance.
|
As a follow up, we'd like this to get into LEAP 15.2, so is there anything else I need to do to make sure that happens? |
src/lib/authserver/dir/client.rb
Outdated
| @@ -23,6 +25,7 @@ def initialize(url, bind_dn, bind_pw) | |||
|
|
|||
| # modify invokes ldapmodify and returns tuple of command output and boolean (success or not). | |||
| def modify(ldif_input, ignore_existing) | |||
| log.info('modify: %s' % ldif_input) | |||
There was a problem hiding this comment.
just hint. Format ( aliases by %operator) is mainly useful for translations. For logging usually interpolation is easier. So it can look like log.info "modify: #{ldif_input}"
src/lib/authserver/dir/client.rb
Outdated
| @@ -49,10 +53,17 @@ def create_person(dn_prefix, cnsn, suffix) | |||
| sn: #{cnsn}", true) | |||
| end | |||
|
|
|||
| def create_container(container_dn) | |||
| return self.add("dn: #{container_dn} | |||
There was a problem hiding this comment.
I suggest to use explicit line ending and formatting like
def create_container(container_dn)
add(
"dn: #{container_dn}\n" \
"objectClass: top\n" \
"objectClass: nsContainer",
true
)
| return | ||
| end | ||
| if !MITKerberos.restart_kadmind | ||
| Popup.Error(_('Failed to start kadmind, please inspect the journal of kadmind.service')) | ||
| UI.ReplaceWidget(Id(:busy), Empty()) |
There was a problem hiding this comment.
hint: this pattern is repeated many times, maybe it can be in own method. At least something like:
def report_error(msg)
Popup.Error(msg)
UI.ReplaceWidget(Id(:busy), Empty())
end
# and use it like
if !MITKerberos.restart_kadmind
report_error(_('Failed to start kadmind, please inspect the journal of kadmind.service'))
return
end
There was a problem hiding this comment.
I might do this in a future refactor?
|
Thanks for the review, I've integrated most of you changes except making popup a method, because I think that cleanup/refactor can be done later? |
|
yes, feel free to merge. |
|
✔️ Public Jenkins job #8 successfully finished |
|
✔️ Internal Jenkins job #3 successfully finished |
20190702 update kdb support for 389 This is a large rework of kdb for 389, fixing a large number of issues that prevented the kdb setup from operating correctly. It now is able to correctly configure and setup Kerberos against a 389 Directory Server instance.
This is a large rework of kdb for 389, fixing a large number of issues
that prevented the kdb setup from operating correctly. It now is able
to correctly configure and setup Kerberos against a 389 Directory
Server instance.