Auth: Support dynamic library backends using C ABI instead of C++ ABI

[darnuria]

• Design/Feature issue: https://github.com/PowerDNS/pdns/issues/11886
• Revival of https://github.com/PowerDNS/pdns/pull/5160

Code heavily used on pdns 4.3.x, updated on master to upstream it.

Citing original @baloo pr because it's a rebase on master + fix to make it compiling/testing again.

I have co-edited some patch to avoid awful commit due to clang-format rules on powerdns code base, that were not enforced at the time of the first-PR.

Short description

Powerdns will dlopen a lib to act as a backend.

Missing:

• [x] Api versioning. I intend to add an api version at the beginning of the struct lib_so_api.
• [x] Packing. A bunch of the structs needs packing.
• [x] Malloc responsibility. dlso_register_t should be the one doing the malloc of struct lib_so_api, not dlso its struct.
• [ ] Documentation
• [ ] Debugability: adds some logging to debug a lib.
• [ ] Maybe rephrasing+rollup my own commits

Checklist

I have:

• [x] Read the CONTRIBUTING.md document
• [x] clang-formated the code
• [x] Compiled and tested this code: part of the DLSO backend is heavily used at work.
• [ ] Included documentation (including possible behavior changes)
• [ ] Documented the code
• [ ] Added or modified regression test(s) (may have some issue in actual regression test of this PR some may have drifted (investigating))
• [ ] Added or modified unit test(s)
• [ ] clang-tidy: whopsy files added may need some tyding
• [x] Find a better solution to avoid const discard in API
• [x] Pass malloc invocation to vectors
• [ ] Pass some allocation to unique_ptr in reg-test
• [ ] change lock to use LockGuarded

I have not squashed yet 3cb7241cb6f3ea8ab9bb9bae02c5779d543c602c because before methods were not marked override and the original PR suffered some patch drift.

Edit history:

• Added checking clang-tidy for the patches.

[darnuria]

Summary: Little bit of noise on this PR due to high pertinant warning with clang-tidy both in test and implementation. Will work on it today and try to fix CI.

[darnuria]

CI failing: Some CI fail due to Unable to load module '../regression-tests/modules/libgsqlite3backend.so': ../regression-tests/modules/libgsqlite3backend.so: cannot open shared object file: No such file or directory

[darnuria]

Did a first pass of clang-tidying in regression test of the backend. Next task a pass in the implementation and header of the backend. EDIT: now ~this~ theses makefile.am is sorted :rofl:

[Habbie]

Echoing what Rémi said (we might not merge this), and not having looked at the code, my first question was: uh, but what is this for. Then I remembered #5160 had a few comments explaining this - the problem is not dlopening backends, loading .so, etc. - PowerDNS already does all that! The problem is C++ name mangling. Perhaps the backend could get a better name that clarifies that.

[darnuria]

Thanks for your time and the early review. Sorry for the late reply.

About the general feedback about merging or not, the name etc:

From @rgacogne

I added a few comments, but please be aware that we might decide not to merge a new backend, depending on the interest from the community, and the estimated burden to maintain it.

From @Habbie

Echoing what Rémi said (we might not merge this),

I totally understands, and people in my team also. It's opensource and free-software rules, at the end of the day it must be maintained, and must have some interest in the community.

I think it might help if you can commit to maintain it for a reasonable time, for example.

This is something we are considering in my team, cannot state before a team decision.

and not having looked at the code, my first question was: uh, but what is this for.

Opened an issue for discussing the design (it can change) and the intend and the possibles alternatives: #11886 (may change the name there too).

Then I remembered #5160 had a few comments explaining this - the problem is not dlopening backends, loading .so, etc. - PowerDNS already does all that! The problem is C++ name mangling. Perhaps the backend could get a better name that clarifies that.

I agree that the current name is maybe misleading, maybe FFIBackend, CFFIBackend, or CABIBackend? I am not great at picking names.

For the rest of the review I will take some time to process it. I might also document it, it's not only for up-streaming it, it already need docs as in.

[darnuria]

Update: it's quite stalled since I think until "safe closing and ressource release" of powerdns is implemented (5.0? I read it somewere in a milestone) it's dangerous to open the door to "arbitrary backend" imho.

Would lead to incorrect databases states if D-tor doesn't get called.

Please don't hesitate to correct me if I am wrong, but I have some case at work with this implementation that lead to resources badly released and need recovery tasks.

[Habbie]

Hmm, I'm not actually sure what problem you are referring to - but perhaps I forgot?

[darnuria]

Like in case of Signal received by Powerdns historically I had some issue with this backend by don't having releasing called. But I may have to catch-up with master if it's already fixed! :)

Edit: Second issue the current design is not ideal error in the underling c-abi lib get just "forgetted" and blend into true/false, same for logging can be added later but are worth spelled out as current issues.

[darnuria]

Hello, I no longer work for Gandi so I cannot commit time to this. Thanks for the review time and the help. :)