PR#50167: Ticket 50165 - Fix dscreate issues - 389-ds-base

389-ds-base

#50167 Ticket 50165 - Fix dscreate issues

Closed 2 years ago by spichugi. Opened 3 years ago by mreynolds.

mreynolds/389-ds-base ticket50165 into master

Ticket 50165 - Fix dscreate issues

Mark Reynolds • 3 years ago

68e0880

src/lib389/lib389/instance/setup.py

file modified

+1 -1

		`@@ -828,7 +828,7 @@`
		`selinux_paths = ('backup_dir', 'cert_dir', 'config_dir', 'db_dir', 'ldif_dir',`
		`'lock_dir', 'log_dir', 'run_dir', 'schema_dir', 'tmp_dir')`
		`for path in selinux_paths:`
		`- selinux_restorecon(path)`
		`+ selinux_restorecon(slapd[path])`
firstyear commented 3 years ago This is inside a loop, so path is correct here ....

		`selinux_label_port(slapd['port'])`

src/lib389/lib389/utils.py

file modified

+3 -1

		`@@ -172,6 +172,7 @@`
		`# Utilities`
		`#`

		`+`
		`def selinux_restorecon(path):`
		`"""`
		`Relabel a filesystem rooted at path.`
		`@@ -195,6 +196,7 @@`
		`except:`
		`log.debug("Failed to run restorecon on: " + path)`

		`+`
		`def selinux_label_port(port, remove_label=False):`
		`"""`
		`Either set or remove an SELinux label(ldap_port_t) for a TCP port`
		`@@ -225,7 +227,7 @@`
		`# a RH based system.`
		`selinux_default_ports = [389, 636, 3268, 3269, 7389]`
		`if port in selinux_default_ports:`
		`- log.error('port %s already in %s, skipping port relabel' % (port, selinux_default_ports))`
		`+ log.debug('port %s already in %s, skipping port relabel' % (port, selinux_default_ports))`
firstyear commented 3 years ago log.warning? Just to make it have every possible log level in this review :P
mhonek commented 3 years ago I vote for 'info' level then. No, really, it's good to know the utils double checked; which would be also nice - really check the ports are well labelled (thou not necessarily in this PR).
		`return`

		`label_set = False`

mreynolds commented 3 years ago

Description:

There were some recent regressions around selinux in dscreate.

          - When skipping labelling of default port an error message was incorrectly logged
          - restorecon was not using the correct path

https://pagure.io/389-ds-base/issue/50165

Reviewed by: ?

firstyear commented on line 6 of src/lib389/lib389/instance/setup.py 3 years ago

This is inside a loop, so path is correct here ....

firstyear commented on line 22 of src/lib389/lib389/utils.py 3 years ago

log.warning? Just to make it have every possible log level in this review :P

firstyear commented 3 years ago

My comment is incorrect, slapd[path] is right. My apologies. runs for another coffee :)

mhonek commented on line 22 of src/lib389/lib389/utils.py 3 years ago

I vote for 'info' level then. No, really, it's good to know the utils double checked; which would be also nice - really check the ports are well labelled (thou not necessarily in this PR).

mreynolds commented 3 years ago

Hey guys, here the thing, error, warning, or info will always log to dscreate's (or dsctl remove) output if you use port 389. It looks like an error but it's perfectly fine. I stand by my approach that this message should be hidden by default, or even removed completely (it's not really useful either way). I don't want a lot of busy/harmless output generated by dscreate by default

mhonek commented 3 years ago

Ok, double checking the behaviour, the 'debug' looks just fine. ACK from me then.

mreynolds commented 3 years ago

Ok, double checking the behaviour, the 'debug' looks just fine. ACK from me then.

Thanks, and just to be clear to everyone, I feel that the output generated by dscreate/dsctl remove should be as minimal as possible unless you use a verbose option or run into an error. If the installer is working fine then there is no need to log that "hey we don't need to label a default port". It's just so random and out of context of a successful installation. Now if we logged more information by default then I would vote to use a visible log level, but that is not the case at this time.

rebased onto 68e0880

3 years ago

Pull-Request has been merged by mreynolds

3 years ago