Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for custom global options in AD DC confgurations #85

Merged
merged 14 commits into from
Aug 23, 2023

Conversation

phlogistonjohn
Copy link
Collaborator

This revisits and reuses parts of the work done in #45
Fixes: #43

When replying to samba-in-kubernetes/samba-container#154 I realized that one of the workarounds I suggested could be implemented in sambacc without too many complications and without needing to change samba-tool. Also, all versions of samba used by samba-container images that provide AD DC images should be providing the smbconf python library now. This avoids us having to parse smb.conf-style-ini in python.

Briefly, when an AD DC configuration provides globals references the global options are passed directory to samba-tool commands and after samba-tool has generated an smb.conf, sambacc uses smbconf libraries to read the smb.conf, merge the settings from the sambacc globals and write out an updated smb.conf file.

An example YAML configuration file I manually tested with:

samba-container-config: v0
configs:
  demo:
    instance_features: [addc]
    domain_settings: sink
    instance_name: dc1
    globals: ["adglobals"]
domain_settings:
  sink:
    realm: DOMAIN2.SINK.TEST
    short_domain: DOMAIN2
    admin_password: Passw0rd
domain_groups:
  sink:
    - name: peeps
domain_users:
  sink:
    - name: bwayne
      password: "1115Rose."
      given_name: Bruce
      surname: Wayne
      member_of: ["peeps"]
globals:
  adglobals:
    options:
      "dns forwarder": "192.168.64.1"
      "ldap server require strong auth": "no"
      "ignore:me": "yes"

Results in an smb.conf like:

[global]
        dns forwarder = 192.168.64.1
        netbios name = dc1
        realm = DOMAIN2.SINK.TEST
        server role = active directory domain controller
        workgroup = DOMAIN2
        idmap_ldb:use rfc2307 = yes
        ignore:me = yes
        ldap server require strong auth = no

[sysvol]
        path = /var/lib/samba/sysvol
        read only = No

[netlogon]
        path = /var/lib/samba/sysvol/domain2.sink.test/scripts
        read only = No

Using dig in the container I was able to test that dns forwarder option did have an actual effect on the running samba server.


The PR starts off with a few random cleanups of stuff that I hit while working on these changes. If you'd rather see those as a separate PR, just ask.

This doesn't update any docs. I will update the documentation in a separate PR, perhaps after asking the issue reporters to test out the changes when they land in samba-container server & ad-server latest images on quay.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
This looks like it might have been experimental code that was
accidentally left in. Remove it.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
This will allow setting of smb.conf options at provision time.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Avoid clear code duplication.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Apparently, I'm lazy and had not done it before.


Signed-off-by: John Mulligan <jmulligan@redhat.com>
This allows the customization of samba DC configuration beyond
the core parameters needed for domain provision or join.


Signed-off-by: John Mulligan <jmulligan@redhat.com>
In the case of AD DC it's pretty normal for there to be no globals key
in the instance configuration. If we want to optionally support global
configuration params for addc it's convenient to simply return an empty
list rather than raise an exception in this case.  If the globals list
refers to global configs that don't exist exceptions will continue to be
raised which seem like the right thing to do.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Copy link
Collaborator

@synarete synarete left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, look fine. I would prefer modern pathlib.Path over traditional os APIs.

tests/test_addc.py Show resolved Hide resolved
sambacc/config.py Show resolved Hide resolved
tests/test_smbconf_samba.py Outdated Show resolved Hide resolved
tests/test_smbconf_samba.py Show resolved Hide resolved
sambacc/commands/addc.py Show resolved Hide resolved
The smbconf library provided by Samba allows python code to interact
with Samba's configuration backends. Before adding a module that
uses those libraries directly add a module that provides general
abstractions and interfaces in that vein.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add smbconf_samba.py a module that builds a somewhat more pythonic and
convenient interface around the SMBConf class and functions provided by samba
python libraries. Note that the import of samba modules is deferred
until the class needs them so that we can import smbconf_samba without
raising an ImportError even if samba modules are unavailable.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add unit(ish) tests for smbconf_samba. We have to work around an (IMO)
annoying behavior of samba's. When using the registry backend once the
registry tdb file is opened it is never closed and subsequent calls
to smbconf's init_reg or similar end up reusing whatever first tdb file
got opened. (I looked at samba master code, this is still the case
AFAICT.) So we do a bit of hackery with regards to test fixtures to
get a "clean" SMBConf for each test. This shouldn't be much of an
issue outside of tests as you typically only have one smb.conf and/or
one data dir with a registry.tdb in it.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Global options passed to the samba-tool commands do no persist in the
smb.conf generated by samba-tool domain {provision,join} commands.  In
order to get custom global configurations into the AD DC we can instead
use the smbconf configuration reader to read the current smb.conf and
merge in any options defined in the sambacc configuration.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
Copy link
Collaborator

@synarete synarete left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already support custom options in non AD DC setup, right?

sambacc/addc.py Show resolved Hide resolved
@phlogistonjohn
Copy link
Collaborator Author

We already support custom options in non AD DC setup, right?

If I understand the question right, then yes.

sambacc initially only knew how to provision the AD DC, which generates it's own smb.conf but still had a place in it's own config for specifying globals, they were just ignored. This PR aims to make it work - and it's one of the more requested things for users of samba-containers.

@mergify mergify bot merged commit 06ccd8e into samba-in-kubernetes:master Aug 23, 2023
9 checks passed
@phlogistonjohn phlogistonjohn deleted the jjm-addc-globals-23 branch August 23, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuring globals does not appear to work
3 participants