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.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Andrew Bartlett <abart...@samba.org> Autobuild-Date(master): Mon Sep 19 07:14:31 UTC 2022 on sn-devel-184 commit dad0c9a52eb142ea105231ab1e8df75ff00da210 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Sep 15 12:41:13 2022 +1200 docs/man/samba-tool explain --color Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 98c7af03945e9af7fa032dc2d8682838b0b2d5fc Author: Douglas Bagnall <douglas.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 10bcf2bb08ee742023325bcbb3005d6a9e8295b6 Author: Douglas Bagnall <douglas.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit d71258b45502a5552cf3540c854b925be3194b8c Author: Douglas Bagnall <douglas.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 2b039eb8c52a491c3d7b5bcae952e826b3ac1b21 Author: Douglas Bagnall <douglas.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 318eb65cb8d777651861266818c646246f82e1a1 Author: Douglas Bagnall <douglas.bagn...@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.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@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