Possible regression in samba-tool provision

[John Mulligan]

Hi samba team,

One of our projects consumes container images based on nightly rpm builds of 
samba master. Over the last day or two one of our test jobs has been failing 
and I think it is due to recent changes.  This might only affect mit krb5 based 
builds.

When provisioning a domain we see a traceback like so:

INFO 2023-03-23 21:22:50,399 pid:6 /usr/lib64/python3.10/site-packages/samba/
provision/__init__.py #2021: Fixing provision GUIDs
ERROR(<class 'AttributeError'>): uncaught exception - 'DomainUpdate' object 
has no attribute 'upper'
  File "/usr/lib64/python3.10/site-packages/samba/netcmd/__init__.py", line 
230, in _run
    return self.run(*args, **kwargs)
  File "/usr/lib64/python3.10/site-packages/samba/netcmd/domain.py", line 555, 
in run
    result = provision(self.logger,
  File "/usr/lib64/python3.10/site-packages/samba/provision/__init__.py", line 
2408, in provision
    create_kdc_conf(paths.kdcconf, realm, domain, os.path.dirname(lp.get("log 
file")))
  File "/usr/lib64/python3.10/site-packages/samba/provision/kerberos.py", line 
43, in create_kdc_conf
    domain = domain.upper()
Temporarily overriding 'dsdb:schema update allowed' setting

This is followed by some other logging output but the overall command fails.

I tracked this down to change 4bba26579d124af6c0767bb98bee67357001e1e7 which 
adds some code to `python/samba/provision/__init__.py`. Part of the diff:

> +                try:
> +                    from samba.domain_update import DomainUpdate
> +
> +                    domain = DomainUpdate(samdb, fix=True)
> +                    domain.check_updates_functional_level(adprep_level,
> +                                                         
> DS_DOMAIN_FUNCTION_2008, +                                                 
>         update_revision=True) +
> +                    samdb.transaction_commit()
> +                except Exception as e:
> +                    samdb.transaction_cancel()
> +                    raise e

This block uses the variable domain that gets assigned a DomainUpdate object, 
but the lines below:

>          if not is_heimdal_built():
>              create_kdc_conf(paths.kdcconf, realm, domain,
>              os.path.dirname(lp.get("log file"))) logger.info("The Kerberos            
KDC configuration for Samba AD is "

pass domain to create_kdc_conf which expect the value in domain to be a 
string.  Skimming the code I think this block is the last to use domain 
variable, and the only one to use it after it gets reassigned to a 
DomainUpdate object, so it's probably only the mit krb5 build that will hit 
this error.
A fix might be to just rename `domain` variable in the new block or even remove 
it and chain the 
`DomainUpdate(...).domain.check_updates_functional_level(...)` together.

In the mean time, we think we may be able to work around this issue by using 
the `--adprep-level` option, but we are still investigating.

If you have any other questions, comments, or would prefer I report this to 
bugzilla please just ask.  Thanks!

--John M.


PS: As a python coder, the samdb transaction handling could be written in a 
neater way using a context manager. ;-) ;-)