#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

@@ -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])

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

  

              selinux_label_port(slapd['port'])

  

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))

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

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

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: ?

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

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

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

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).

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

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

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

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3226

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

2 years ago