Skip to content

dns: add master2.ldap.test alias for the LDAP server - #232

Merged
spoore1 merged 1 commit into
SSSD:masterfrom
aborah-sudo:module
Oct 15, 2026
Merged

spoore1 merged 1 commit into
SSSD:masterfrom
aborah-sudo:module

Conversation

@aborah-sudo

Copy link
Copy Markdown
Contributor

A second name for 172.16.100.20, so failover topologies can configure two distinct LDAP servers. SSSD keys its failover server list on the hostname, so the alias yields a genuinely separate server entry even though the address is shared:

Inserted primary server 'master.ldap.test:389' to service 'LDAP'
Inserted backup server 'master2.ldap.test:389' to service 'LDAP'

No PTR record, reverse lookup for the address must stay unambiguous, and no second _ldap._tcp SRV record, service discovery should keep pointing at master.ldap.test only.

@justin-stephenson

justin-stephenson commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

@aborah-sudo Do you think deploying another LDAP server instances would work here? Or do you need to have 2 different LDAP servers?

@aborah-sudo

Copy link
Copy Markdown
Contributor Author

@aborah-sudo Do you think deploying another LDAP server instances would work here? Or do you need to have 2 different LDAP servers?

A second instance is enough, and that is the plan. We do not need two separate LDAP servers or a second container.

The thing that actually matters is the hostname, not the machine. SSSD keys its failover server list on the server name: get_server_common() (src/providers/fail_over.c) looks up the shared server_common by hostname with strcasecmp. So two distinct names give two genuinely distinct server entries and real resolution-level failover, even when both names resolve to the same address. One hostname with two ports would only ever exercise port-level failover, which is not what the bash tests we are converting actually cover.

I verified this on the current containers, with no second instance at all, just this DNS alias pointed at the existing 389-ds:

ldap_uri        = ldap://master.ldap.test
ldap_backup_uri = ldap://master2.ldap.test
fo_add_server_to_list: Inserted primary server 'master.ldap.test:389' to service 'LDAP'
fo_add_server_to_list: Inserted backup server 'master2.ldap.test:389' to service 'LDAP'
set_server_common_status: Marking server 'master.ldap.test' as 'name resolved'

Two separate entries with independent status, which is what the failover tests need.

The full plan, agreed with @jakub-vavra-cz, is framework-level rather than container-level: a topology that is not provisioned by default, which spawns a second 389-ds instance during topology setup on its own port (master2.ldap.test:2389) and removes it during teardown. This PR is just the DNS prerequisite so that instance can be addressed under a distinct name.

One caveat worth flagging, since it affects the next PR: the alias is not usable over TLS yet. The LDAP certificate is issued CN=master.ldap.test with no subjectAltName (data/configs/openssl_sign_service.ext sets none), so under the framework default ldap_tls_reqcert: demand:

sdap_connect_done: START TLS result: Success(0), Start TLS request accepted.
sdap_connect_done: ldap_install_tls failed: [Connect error] [TLS: hostname does not match name in peer certificate]

So a follow-up is needed to add master2.ldap.test as a SAN in gen-certs.sh. ldap_tls_reqcert = never is not an acceptable substitute, since several of the remaining bash phases test TLS failover specifically.

This record is inert until something asks for it, so it is safe to land ahead of the cert change.

@jakub-vavra-cz jakub-vavra-cz left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will not be enough for idmci.
Change in https://github.com/SSSD/sssd-ci-containers/blob/master/src/ansible/roles/dns/templates/etc.dnsmasq.conf.j2 would be needed with something like this (untested):

# Add a second ldap (instance) for failover located on the same server
{% if hostvars['master.ldap.test'] is defined %}
address=/master2.ldap.test/{{ hostvars['master.ldap.test']['ansible_facts']['default_ipv4']['address'] }}
{% endif %}

Maybe even srv record with port can be added as well the same way.

@jakub-vavra-cz jakub-vavra-cz self-assigned this Oct 14, 2026
A second name for 172.16.100.20, so failover topologies can configure two
distinct LDAP servers. SSSD keys its failover server list on the hostname,
so the alias yields a genuinely separate server entry even though the
address is shared:

  Inserted primary server 'master.ldap.test:389' to service 'LDAP'
  Inserted backup server 'master2.ldap.test:389' to service 'LDAP'

No PTR record, reverse lookup for the address must stay unambiguous, and
no second _ldap._tcp SRV record, service discovery should keep pointing at
master.ldap.test only.
@aborah-sudo

Copy link
Copy Markdown
Contributor Author

This will not be enough for idmci. Change in https://github.com/SSSD/sssd-ci-containers/blob/master/src/ansible/roles/dns/templates/etc.dnsmasq.conf.j2 would be needed with something like this (untested):

# Add a second ldap (instance) for failover located on the same server
{% if hostvars['master.ldap.test'] is defined %}
address=/master2.ldap.test/{{ hostvars['master.ldap.test']['ansible_facts']['default_ipv4']['address'] }}
{% endif %}

Maybe even srv record with port can be added as well the same way.

Good catch, thanks, I had only covered the container path. Added your snippet to src/ansible/roles/dns/templates/etc.dnsmasq.conf.j2, in the A record section:

# Add a second ldap (instance) for failover located on the same server.
# SSSD keys its failover server list on the hostname, so the alias yields a
# separate server entry even though the address is shared.
{% if hostvars['master.ldap.test'] is defined %}
address=/master2.ldap.test/{{ hostvars['master.ldap.test']['ansible_facts']['default_ipv4']['address'] }}
{% endif %}

You mentioned it was untested, so I rendered the template both ways to be sure:

  • with master.ldap.test in the inventory: address=/master2.ldap.test/172.16.100.20, matching the real host's address
  • without it: the block is skipped entirely, no stray record

The PTR section is untouched, so reverse lookup for the address still resolves to master.ldap.test only.

On the SRV record, I would rather hold off on that one for now, and not because of the extra work. Adding a second target to _ldap._tcp.ldap.test changes discovery for every existing test that uses _srv_, since it would then return two servers instead of one. We just got bitten by exactly this during the review of SSSD/sssd#9303: service discovery silently succeeds in the LDAP topology, and two tests were passing via the SRV-discovered primary while their names claimed they were exercising the backup URI. Nobody noticed until @spoore1 asked how we knew _srv_ was failing. Making _srv_ nondeterministic between two targets would reintroduce that class of bug in a harder-to-spot form.

So my suggestion is to add it when a test actually needs discovery-based failover, as a deliberate change with the impact on existing _srv_ tests looked at then. The second instance will listen on 2389, so the record would be srv-host=_ldap._tcp.ldap.test,master2.ldap.test,2389 when we get there. Happy to include it in this PR instead if you would rather have the DNS side complete in one go.

@aborah-sudo

Copy link
Copy Markdown
Contributor Author

Tested on idm-ci now, using the fork without merging:

repo: "https://github.com/SSSD/sssd.git"
branch: "master"
repo_containers: "https://github.com/aborah-sudo/sssd-ci-containers.git"
branch_containers: "module"

The record renders correctly on real VMs. From the dns role in the prep phase:

address=/master.ldap.test/10.0.193.230
address=/master2.ldap.test/10.0.193.230

Same address, picked up from hostvars['master.ldap.test'], so the is defined guard matched. That was the one way this could have silently done nothing, so it was worth confirming rather than assuming.

The two things that should not appear are also absent, which was deliberate:

  • only one SRV record, srv-host=_ldap._tcp.ldap.test,master.ldap.test,389, so _srv_ discovery stays deterministic
  • no master2 PTR record, so reverse lookup for the address remains unambiguous

The failover suite passes against it:

23 passed, 1138 deselected, 2 warnings in 278.30s (0:04:38)

That is all 18 test_failover__* functions, 23 with parametrization, across the ldap, ldap_krb5 and ipa topologies. No failures or errors.

I used upstream SSSD/sssd master rather than a refactor branch on purpose, so that a red result could only be attributable to this change.

One limitation worth repeating: this proves the name is created, not that it is usable. Nothing resolves master2.ldap.test yet, and anything that tries still hits ldap_install_tls failed: [TLS: hostname does not match name in peer certificate] until the certificate carries it as a SAN. That follow-up is next.

@jakub-vavra-cz the template change you asked for is in, let me know on the SRV question whether you would rather have it in this PR or deferred until a test needs discovery-based failover.

@spoore1 spoore1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I ran a test in CI and I was able to resolve master2.ldap.test.

@spoore1
spoore1 merged commit fb34f14 into SSSD:master Oct 15, 2026
9 checks passed
@aborah-sudo
aborah-sudo deleted the module branch October 16, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants