[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Mon Sep 19 07:15:02 UTC 2022


The branch, master has been updated
       via  063976fca37 WHATSNEW: samba-tool: fewer tracebacks, more colour
       via  dad0c9a52eb docs/man/samba-tool explain --color
       via  98c7af03945 py/dbcheck: improve 'please --fix' message
       via  10bcf2bb08e dbcheck: don't recommend --fix for errors we can't fix
       via  d71258b4550 dbcheck: do not crash on empty DN
       via  2b039eb8c52 samba-tool dbcheck: use colour if wanted
       via  318eb65cb8d py/dbchecker: dbcheck prints bits of colour if asked
      from  6e5d79ff408 shadow_copy2: Remove an intermediate if-statement

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 063976fca375be367fa6b471389a3d7258b73460
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Sep 15 16:48:31 2022 +1200

    WHATSNEW: samba-tool: fewer tracebacks, more colour
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Mon Sep 19 07:14:31 UTC 2022 on sn-devel-184

commit dad0c9a52eb142ea105231ab1e8df75ff00da210
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Sep 15 12:41:13 2022 +1200

    docs/man/samba-tool explain --color
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 98c7af03945e9af7fa032dc2d8682838b0b2d5fc
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Sat Sep 17 18:18:25 2022 +1200

    py/dbcheck: improve 'please --fix' message
    
    The dbcheck module is used in places other than samba-tool (backup,
    provision) where the old 'use --fix' message made no sense. Also,
    now that we're not necessarily claiming to fix all errors, we say
    how many we think we can.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 10bcf2bb08ee742023325bcbb3005d6a9e8295b6
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Sep 16 16:26:41 2022 +1200

    dbcheck: don't recommend --fix for errors we can't fix
    
    and/or won't fix.
    
    I think there are others that should be here.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit d71258b45502a5552cf3540c854b925be3194b8c
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Sep 15 11:20:25 2022 +1200

    dbcheck: do not crash on empty DN
    
    we had
    
    $ bin/samba-tool dbcheck -H st/rpc_proxy/private/sam.ldb
    Checking 202 objects
    ERROR(<class 'ValueError'>): uncaught exception - unable to parse dn string
      File "/home/douglasb/src/samba/bin/python/samba/netcmd/__init__.py", line 230, in _run
        return self.run(*args, **kwargs)
      File "/home/douglasb/src/samba/bin/python/samba/netcmd/dbcheck.py", line 173, in run
        error_count = chk.check_database(DN=DN, scope=search_scope,
      File "/home/douglasb/src/samba/bin/python/samba/dbchecker.py", line 255, in check_database
        error_count += self.check_object(object.dn, requested_attrs=attrs)
      File "/home/douglasb/src/samba/bin/python/samba/dbchecker.py", line 2616, in check_object
        expected_dn = ldb.Dn(self.samdb, "RDN=RDN,%s" % (parent_dn))
    
    Now we have:
    
    $ bin/samba-tool dbcheck -H st/rpc_proxy/private/sam.ldb
    Checking 202 objects
    ERROR: could not handle parent DN '': skipping RDN checks
    Please use --fix to fix these errors
    Checked 202 objects (1 errors)
    
    which is still not really right, since --fix won't help.
    
    (same with st/s4member/private/sam.ldb).
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 2b039eb8c52a491c3d7b5bcae952e826b3ac1b21
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Sep 15 10:17:16 2022 +1200

    samba-tool dbcheck: use colour if wanted
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 318eb65cb8d777651861266818c646246f82e1a1
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Sep 15 11:13:30 2022 +1200

    py/dbchecker: dbcheck prints bits of colour if asked
    
    Prefixes like ERROR, WARNING, and INFO are given interpretive colours.
    
    This won't change anything until samba-tool decides to ask for colour,
    which, who knows, might even be in the next commit.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 WHATSNEW.txt                       | 60 ++++++++++++++++++++++++++
 docs-xml/manpages/samba-tool.8.xml | 23 ++++++++++
 python/samba/dbchecker.py          | 86 ++++++++++++++++++++++++++------------
 python/samba/netcmd/dbcheck.py     |  9 +++-
 4 files changed, 151 insertions(+), 27 deletions(-)


Changeset truncated at 500 lines:

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index c9cd84faa26..94ced206dbb 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -16,6 +16,66 @@ UPGRADING
 NEW FEATURES/CHANGES
 ====================
 
+More succinct samba-tool error messages
+---------------------------------------
+
+Historically samba-tool has reported user error or misconfiguration by
+means of a Python traceback, showing you where in its code it noticed
+something was wrong, but not always exactly what is amiss. Now it
+tries harder to identify the true cause and restrict its output to
+describing that. Particular cases include:
+
+ * a username or password is incorrect
+ * an ldb database filename is wrong (including in smb.conf)
+ * samba-tool dns: various zones or records do not exist
+ * samba-tool ntacl: certain files are missing
+ * the network seems to be down
+ * bad --realm or --debug arguments
+
+Accessing the old samba-tool messages
+-------------------------------------
+
+This is not new, but users are reminded they can get the full Python
+stack trace, along with other noise, by using the argument '-d3'.
+This may be useful when searching the web.
+
+The intention is that when samba-tool encounters an unrecognised
+problem (especially a bug), it will still output a Python traceback.
+If you encounter a problem that has been incorrectly identified by
+samba-tool, please report it on https://bugzilla.samba.org.
+
+Colour output with samba-tool --color
+-------------------------------------
+
+For some time a few samba-tool commands have had a --color=yes|no|auto
+option, which determines whether the command outputs ANSI colour
+codes. Now all samba-tool commands support this option, which now also
+accepts 'always' and 'force' for 'yes', 'never' and 'none' for 'no',
+and 'tty' and 'if-tty' for 'auto' (this more closely matches
+convention). With --color=auto, or when --color is omitted, colour
+codes are only used when output is directed to a terminal.
+
+Most commands have very little colour in any case. For those that
+already used it, the defaults have changed slightly.
+
+ * samba-tool drs showrepl: default is now 'auto', not 'no'
+ 
+ * samba-tool visualize: the interactions between --color-scheme,
+   --color, and --output have changed slightly. When --color-scheme is
+   set it overrides --color for the purpose of the output diagram, but
+   not for other output like error messages.
+
+No colour with NO_COLOR environment variable
+--------------------------------------------
+
+With both samba-tool --color=auto (see above) and some other places
+where we use ANSI colour codes, the NO_COLOR environment variable will
+disable colour output. See https://no-color.org/ for a description of
+this variable. `samba-tool --color=always` will use colour regardless
+of NO_COLOR.
+
+
+
 
 REMOVED FEATURES
 ================
diff --git a/docs-xml/manpages/samba-tool.8.xml b/docs-xml/manpages/samba-tool.8.xml
index 9a40bb1bec4..10ffa8a4d5f 100644
--- a/docs-xml/manpages/samba-tool.8.xml
+++ b/docs-xml/manpages/samba-tool.8.xml
@@ -69,6 +69,29 @@
 	</para></listitem>
 	</varlistentry>
 
+	<varlistentry>
+	<term>--color=always|never|auto</term>
+	<listitem>
+          <para>
+	    Indicate whether samba-tool should use ANSI colour codes
+	    in its output. If 'auto' (the default), samba-tool will
+	    use colour when its output is directed toward a terminal,
+	    unless the NO_COLOR environment variable is set and
+	    non-empty.
+	  </para>
+          <para>
+            The values 'yes' and 'force' are accepted as synonyms for
+            'always'; 'no' and 'none' for 'never'; and 'tty' and
+            'if-tty' for 'auto'.
+          </para>
+          <para>
+	    Note that asking for colour doesn't mean samba-tool will
+	    necessarily be very colourful. Many commands are very
+	    monochrome, particularly when successful.
+	</para>
+        </listitem>
+	</varlistentry>
+
 	&cmdline.common.debug.client;
 
 	</variablelist>
diff --git a/python/samba/dbchecker.py b/python/samba/dbchecker.py
index 449b0a7d985..c3485292f04 100644
--- a/python/samba/dbchecker.py
+++ b/python/samba/dbchecker.py
@@ -33,7 +33,7 @@ from samba.descriptor import get_wellknown_sds, get_diff_sds
 from samba.auth import system_session, admin_session
 from samba.netcmd import CommandError
 from samba.netcmd.fsmo import get_fsmo_roleowner
-
+from samba.colour import c_RED, c_DARK_YELLOW, c_DARK_CYAN, c_DARK_GREEN
 
 def dump_attr_values(vals):
     """Stringify a value list, using utf-8 if possible (which some tests
@@ -56,7 +56,8 @@ class dbcheck(object):
                  yes=False, quiet=False, in_transaction=False,
                  quick_membership_checks=False,
                  reset_well_known_acls=False,
-                 check_expired_tombstones=False):
+                 check_expired_tombstones=False,
+                 colour=False):
         self.samdb = samdb
         self.dict_oid_name = None
         self.samdb_schema = (samdb_schema or samdb)
@@ -64,6 +65,7 @@ class dbcheck(object):
         self.fix = fix
         self.yes = yes
         self.quiet = quiet
+        self.colour = colour
         self.remove_all_unknown_attributes = False
         self.remove_all_empty_attributes = False
         self.fix_all_normalisation = False
@@ -243,6 +245,7 @@ class dbcheck(object):
         res = self.samdb.search(base=DN, scope=scope, attrs=['dn'], controls=controls)
         self.report('Checking %u objects' % len(res))
         error_count = 0
+        self.unfixable_errors = 0
 
         error_count += self.check_deleted_objects_containers()
 
@@ -262,10 +265,17 @@ class dbcheck(object):
                         "would do that immediately." % (
                         self.expired_tombstones))
 
+        self.report('Checked %u objects (%u errors)' %
+                    (len(res), error_count + self.unfixable_errors))
+
+        if self.unfixable_errors != 0:
+            self.report(f"WARNING: {self.unfixable_errors} "
+                        "of these errors cannot be automatically fixed.")
+
         if error_count != 0 and not self.fix:
-            self.report("Please use --fix to fix these errors")
+            self.report("Please use 'samba-tool dbcheck --fix' to fix "
+                        f"{error_count} errors")
 
-        self.report('Checked %u objects (%u errors)' % (len(res), error_count))
         return error_count
 
     def check_deleted_objects_containers(self):
@@ -370,8 +380,23 @@ systemFlags: -1946157056%s""" % (dn, guid_suffix),
 
     def report(self, msg):
         '''print a message unless quiet is set'''
-        if not self.quiet:
-            print(msg)
+        if self.quiet:
+            return
+        if self.colour:
+            if msg.startswith('ERROR'):
+                msg = c_RED('ERROR') + msg[5:]
+            elif msg.startswith('WARNING'):
+                msg = c_DARK_YELLOW('WARNING') + msg[8:]
+            elif msg.startswith('INFO'):
+                msg = c_DARK_CYAN('INFO') + msg[4:]
+            elif msg.startswith('NOTICE'):
+                msg = c_DARK_CYAN('NOTICE') + msg[6:]
+            elif msg.startswith('NOTE'):
+                msg = c_DARK_CYAN('NOTE') + msg[4:]
+            elif msg.startswith('SKIPPING'):
+                msg = c_DARK_GREEN('SKIPPING') + msg[8:]
+
+        print(msg)
 
     def confirm(self, msg, allow_all=False, forced=False):
         '''confirm a change'''
@@ -2375,7 +2400,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
 
             if attrname.lower() == "name":
                 if len(obj[attrname]) != 1:
-                    error_count += 1
+                    self.unfixable_errors += 1
                     self.report("ERROR: Not fixing num_values(%d) for '%s' on '%s'" %
                                 (len(obj[attrname]), attrname, str(obj.dn)))
                 else:
@@ -2384,7 +2409,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
             if attrname.lower() == str(obj.dn.get_rdn_name()).lower():
                 object_rdn_attr = attrname
                 if len(obj[attrname]) != 1:
-                    error_count += 1
+                    self.unfixable_errors += 1
                     self.report("ERROR: Not fixing num_values(%d) for '%s' on '%s'" %
                                 (len(obj[attrname]), attrname, str(obj.dn)))
                 else:
@@ -2415,7 +2440,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                     # Here we check that the first attid is 0
                     # (objectClass).
                     if list_attid_from_md[0] != 0:
-                        error_count += 1
+                        self.unfixable_errors += 1
                         self.report("ERROR: Not fixing incorrect initial attributeID in '%s' on '%s', it should be objectClass" %
                                     (attrname, str(dn)))
 
@@ -2517,7 +2542,7 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
 
             if attrname.lower() == 'attributeid' or attrname.lower() == 'governsid':
                 if obj[attrname][0] in self.attribute_or_class_ids:
-                    error_count += 1
+                    self.unfixable_errors += 1
                     self.report('Error: %s %s on %s already exists as an attributeId or governsId'
                                 % (attrname, obj.dn, obj[attrname][0]))
                 else:
@@ -2581,10 +2606,10 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
 
         if ("*" in lc_attrs or "name" in lc_attrs):
             if name_val is None:
-                error_count += 1
+                self.unfixable_errors += 1
                 self.report("ERROR: Not fixing missing 'name' on '%s'" % (str(obj.dn)))
             if object_rdn_attr is None:
-                error_count += 1
+                self.unfixable_errors += 1
                 self.report("ERROR: Not fixing missing '%s' on '%s'" % (obj.dn.get_rdn_name(), str(obj.dn)))
 
         if name_val is not None:
@@ -2596,19 +2621,28 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 controls += ["local_oid:%s:1" % dsdb.DSDB_CONTROL_DBCHECK_FIX_LINK_DN_NAME]
             if parent_dn is None:
                 parent_dn = obj.dn.parent()
-            expected_dn = ldb.Dn(self.samdb, "RDN=RDN,%s" % (parent_dn))
-            expected_dn.set_component(0, obj.dn.get_rdn_name(), name_val)
 
-            if obj.dn == deleted_objects_dn:
-                expected_dn = obj.dn
+            try:
+                expected_dn = ldb.Dn(self.samdb, "RDN=RDN,%s" % (parent_dn))
+            except ValueError as e:
+                self.unfixable_errors += 1
+                self.report(f"ERROR: could not handle parent DN '{parent_dn}': "
+                            "skipping RDN checks")
+            else:
+                expected_dn.set_component(0, obj.dn.get_rdn_name(), name_val)
 
-            if expected_dn != obj.dn:
-                error_count += 1
-                self.err_wrong_dn(obj, expected_dn, object_rdn_attr,
-                        object_rdn_val, name_val, controls)
-            elif obj.dn.get_rdn_value() != object_rdn_val:
-                error_count += 1
-                self.report("ERROR: Not fixing %s=%r on '%s'" % (object_rdn_attr, object_rdn_val, str(obj.dn)))
+                if obj.dn == deleted_objects_dn:
+                    expected_dn = obj.dn
+
+                if expected_dn != obj.dn:
+                    error_count += 1
+                    self.err_wrong_dn(obj, expected_dn, object_rdn_attr,
+                            object_rdn_val, name_val, controls)
+                elif obj.dn.get_rdn_value() != object_rdn_val:
+                    self.unfixable_errors += 1
+                    self.report("ERROR: Not fixing %s=%r on '%s'" % (object_rdn_attr,
+                                                                     object_rdn_val,
+                                                                     obj.dn))
 
         show_dn = True
         if repl_meta_data_val:
@@ -2763,18 +2797,18 @@ newSuperior: %s""" % (str(from_dn), str(to_rdn), str(to_base)))
                 if pool != 0 and low >= high:
                     self.report("Invalid RID pool %d-%d, %d >= %d!" %
                                 (low, high, low, high))
-                    error_count += 1
+                    self.unfixable_errors += 1
 
             if "rIDAllocationPool" not in res[0]:
                 self.report("No rIDAllocationPool found in %s" % dn)
-                error_count += 1
+                self.unfixable_errors += 1
 
             try:
                 next_free_rid, high = self.samdb.free_rid_bounds()
             except ldb.LdbError as err:
                 enum, estr = err.args
                 self.report("Couldn't get available RIDs: %s" % estr)
-                error_count += 1
+                self.unfixable_errors += 1
             else:
                 # Check the remainder of this pool for conflicts.  If
                 # ridalloc_allocate_rid() moves to a new pool, this
diff --git a/python/samba/netcmd/dbcheck.py b/python/samba/netcmd/dbcheck.py
index 8827ee35d63..215e8072f14 100644
--- a/python/samba/netcmd/dbcheck.py
+++ b/python/samba/netcmd/dbcheck.py
@@ -27,6 +27,7 @@ from samba.netcmd import (
     Option
 )
 from samba.dbchecker import dbcheck
+from samba import colour
 
 
 class cmd_dbcheck(Command):
@@ -139,6 +140,11 @@ class cmd_dbcheck(Command):
         else:
             attrs = attrs.split()
 
+        # The dbcheck module always prints to stdout, not our self.outf
+        # (yes, maybe FIXME).
+        stdout_colour = colour.colour_if_wanted(sys.stdout,
+                                                hint=self.requested_colour)
+
         started_transaction = False
         if yes and fix:
             samdb.transaction_start()
@@ -149,7 +155,8 @@ class cmd_dbcheck(Command):
                           in_transaction=started_transaction,
                           quick_membership_checks=quick_membership_checks,
                           reset_well_known_acls=reset_well_known_acls,
-                          check_expired_tombstones=selftest_check_expired_tombstones)
+                          check_expired_tombstones=selftest_check_expired_tombstones,
+                          colour=stdout_colour)
 
             for option in yes_rules:
                 if hasattr(chk, option):


-- 
Samba Shared Repository



More information about the samba-cvs mailing list