#212 Initial support for BIND 9.18 support
Merged by abbra. Opened by pemensik.
pemensik/bind-dyndb-ldap bind-9.18-support  into  master

Some changes needed to build with the most recent BIND9 release. It does
not yet provide complete support for new release.

Detects version of libdns just from libdns.so symlink. It requires
--libdir= explicitly set for this part to work.

Leaves unsolved issue with dns_ssutable, which I am not sure which solution would be the best. dns_rdatatype_t is replaced by structure containing type and maximum count for it:

typedef struct dns_ssuruletype {
        dns_rdatatype_t type; /* type allowed */
        unsigned int    max;  /* maximum number of records allowed. */
} dns_ssuruletype_t;
/bin/sh ../libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I..    -Wall -Wextra -Werror -std=gnu99 -O2 -I/usr/include/bind9 -I/usr/include/bind9 -g -O2 -fvisibility=hidden -fno-delete-null-pointer-checks -std=gnu11 -fvisibility=hidden -fno-delete-null-pointer-checks -std=gnu11 -MT ldap_la-acl.lo -MD -MP -MF .deps/ldap_la-acl.Tpo -c -o ldap_la-acl.lo `test -f 'acl.c' || echo './'`acl.c
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I.. -Wall -Wextra -Werror -std=gnu99 -O2 -I/usr/include/bind9 -I/usr/include/bind9 -g -O2 -fvisibility=hidden -fno-delete-null-pointer-checks -std=gnu11 -fvisibility=hidden -fno-delete-null-pointer-checks -std=gnu11 -MT ldap_la-acl.lo -MD -MP -MF .deps/ldap_la-acl.Tpo -c acl.c  -fPIC -DPIC -o .libs/ldap_la-acl.o
acl.c: In function acl_configure_zone_ssutable:
acl.c:340:50: error: passing argument 7 of dns_ssutable_addrule from incompatible pointer type [-Werror=incompatible-pointer-types]
  340 |                                               n, types);
      |                                                  ^~~~~
      |                                                  |
      |                                                  dns_rdatatype_t * {aka short unsigned int *}
In file included from acl.c:22:
/usr/include/bind9/dns/ssu.h:114:41: note: expected dns_ssuruletype_t * {aka struct dns_ssuruletype *} but argument is of type dns_rdatatype_t * {aka short unsigned int *}
  114 |                      dns_ssuruletype_t *types);
      |                      ~~~~~~~~~~~~~~~~~~~^~~~~
acl.c:336:24: error: void value not ignored as it ought to be
  336 |                 result = dns_ssutable_addrule(table, grant,
      |                        ^
cc1: all warnings being treated as errors

Test build of bind 9.18 is available at: https://copr.fedorainfracloud.org/coprs/pemensik/bind/

It seems we may ask bind upstream to provide public method alternative of internal function, which does something similar to implemented again by bind-dyndb-ldap. It seems unnecessary to parse update-policy again in plugin. Do we want to limit supported update policies? Is it required to reimplement existing code in bind?

static isc_result_t
configure_zone_ssutable(const cfg_obj_t *zconfig, dns_zone_t *zone,         const char *zname);

This is implemented in bin/named/zoneconf.c, function configure_zone_ssutable.

I don't think we want to limit update-policy, indeed. The original issue was and still is that configure_zone_ssutable is static and is called from named_zone_configure which is called from a static configure_zone. This does not work well with updates coming from LDAP where we basically need to reconfigure the existing zone.

What's the planned schedule here? Debian moved to 9.18 already last month.

2 new commits added

  • Support for 9.18 and 9.17 support
  • Add basic support of dns_ssuruletype_t

Updated support, it already compiles under 9.18. Do not know how much it would work.

seems to build, although Debian/Ubuntu for some reason rename the libs to look like this:

bind9-libs: /usr/lib/x86_64-linux-gnu/libbind9-9.18.0-2ubuntu1-Ubuntu.so

so I've just hardcoded the version instead

It would be nice, if we found generally working way to detect correct names for libraries used. Working on all distributions. Different directory should not matter. This configure time check is intended to work with bind9-dev package, not with bind9-libs itself. I attempted to make it universal and it should work also on debian or ubuntu, but I did not test it myself.

It may work if ./configure --libdir=/usr/lib/x86_64-linux-gnu were used, can you @tjaalton test it with bind9-dev installed? I don't know Debian packaging enough to know whether some variable has correct libdir. I would expect that. I have used libdir this way, because our RPM %configure macro always passes it to built packages.

Is Debian bind-dyndb-ldap packaging setting libdir correctly? bind9 package uses different value, which might work better.

https://salsa.debian.org/freeipa-team/bind-dyndb-ldap/-/blob/master/debian/rules:
dh_auto_configure --    --libdir=/usr/lib

https://salsa.debian.org/dns-team/bind9/-/blob/debian/main/debian/rules:
dh_auto_configure --    --libdir=/usr/lib/$(DEB_HOST_MULTIARCH)

That libdir scan were intended to be used on development package, which provides symlink to actual library, whatever it is. I think you should replace bind9-libs with bind9-dev in your packaging and it might work the way I have prepared it.

that was hardcoded so that freeipa wouldn't have to worry about multiarch paths, but that can be changed now

but the thing is that bind9-dev doesn't ship headers that the maintainer thinks are non-public, so dns/version.h is not available:

conftest.c:38:10: fatal error: dns/version.h: No such file or directory

actually, I can't find dns/version.h in upstream sources either

Sure, separate versions of each libraries were removed. dns/version.h include is no longer available and version built into libdns binary as well. That is the reason I fiddle with symlink path of libdns.so library. I could think only of named -V first line to provide exact version of named, including any extra version used. That is where I attempt to extract version from libdns.so symlink of devel package. That should work with just bind9-dev package, without bind9 package itself. If you know a better way, please share.

But that requires ${libdir}/libdns.so symlink to exist. Which would not in case of libdir=/usr/lib. I am sure that detection can be adjusted by downstream patch however.

1 new commit added

  • Move common dns_name_copynf compatibility macros to header

Could be this change tested together with bind 9.18.0 build from COPR repository pemensik/bind? Basic test worked. It seems to be ready for new Rawhide. What do you think?

Are there any tasks needed to move this PR along that could be contributed to? This is a critical component of getting the FreeIPA server working on Debian, as @tjaalton previously mentioned.

I'd be happy to pitch in, provided I knew what needed to be done x)

We wanted to run the whole set of FreeIPA tests on both Fedora and RHEL before merging. @pemensik created a COPR repo but sadly it did not have consistent state when we tried couple weeks so packages weren't installable. We can retry next week.

Sounds good :smile: Give us a shout if we can do anything to help out from the Debian side of things!

Bind 9.17+ support was merged to FreeIPA. We are going to release FreeIPA 4.9.10 and 4.10.0 which will have this support, so I think we can merge this pull request and do a new bind-dyndb-ldap release.

@pemensik is this PR a final one?

@abbra Yes, I don't have anything more to add now.

Thanks. We tested this extensively as a part of FreeIPA 4.9.10 integration testing.

Pull-Request has been merged by abbra

Great! Glad to hear that

@pemensik @tjaalton I have released bind-dyndb-ldap-11.10 which supports bind 9.17+.

@pemensik I pushed the update to Rawhide dist-git but haven't built the package aside from building a scratch one: https://koji.fedoraproject.org/koji/taskinfo?taskID=88530266. Feel free to create a side-tag and rebuild bind and bind-dyndb-ldap there in Rawhide.