debian: Cleanup build process using more debhelper features

[3v1n0]

refactor quite a bit the debian/* code so that we are less dependent on our scripts, but instead we use debian tooling more.

In particular:

• Ensure that generated go files are properly created when bootstrapping the repo and removing them
• Use dh-cargo tools more, reducing duplication
  • And when it's not possible, mimic its behavior more
• Address various lintian warnings
• Use install files to install the produced binaries
  • It has the benefit of fail if we miss something or if we're adding unexpected
• Install the daemons in /usr/libexec instead of /usr/sbin (there's no need for them to be in $PATH)

Closes: #218

UDENG-2342

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.01%. Comparing base (37e5125) to head (7758956).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #223      +/-   ##
==========================================
- Coverage   87.07%   87.01%   -0.07%     
==========================================
  Files          64       64              
  Lines        4891     4891              
==========================================
- Hits         4259     4256       -3     
- Misses        440      442       +2     
- Partials      192      193       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[didrocks]

I didn’t look at the content of the diff itself, only read the description. There are 2 things we shouldn’t do IMHO:

Remove generated files from the debian tarball.
I know this is the common thing in the go ecosystem, but ideally debian package sources should contain no generated code

This is a big no no. We tested with generated code, our all CI is relying on generator code and the go ecosystem and potential dependencies are relying on those generated code. We are not going to compile code with a different version of protoc and grpc bindings than what we validated before releasing in the package.
This is already what we do in other packages we maintain and what other debian packages like kubernetes are doing. Follow the language paradigm and on what upstream put their quality seal on, don’t ship code that were not acknowledge and qualified by them.

This is the same reason we are using vendored dependencies.

Add authd-example-broker binary package providing the example broker and related configurations (systemd service, dbus config...)

I don’t think we shouild ship it.This is really an example source code which doesn’t track state and options. This can open security issues and we don’t want to be responsible for this. Please let’s not ship that binary and keep it as it is, example code (and not even a reference implementation)

[3v1n0]

This is a big no no. We tested with generated code, our all CI is relying on generator code and the go ecosystem and potential dependencies are relying on those generated code. We are not going to compile code with a different version of protoc and grpc bindings than what we validated before releasing in the package.

Well, I can revert that part, it's not a the main point of the PR, but I indeed wanted to be sure that thing can be generated at build time.

Also, while I do understand that we trust such code upstream, at distro level that's not the best thing to do IMHO, a that's not the policy in debian (even though that's accepted when it's hard to do otherwise). Reason why DH_GOLANG_GO_GENERATE according to man dh-golang is something that should be default true, and it wasn't only because of backward compatibility.

However, I'll drop the changes related to this, but I still feel that it's not the best for having builds that are reproducible in the same environment (e.g. we do test things upstream now, but once we update our CI, then it's hard to know how things are generated at a specific point of the project history, when we target a specific ubuntu release).

[3v1n0]

Ah, regarding the example broker... How do you think we could handle that otherwise?

Ideally we want it having available for autopkgtests, IMHO, having a separate binary can't be a security issue as really if you can make it load through /etc/authd/borkers.d or dpkg -i/apt install it we defintely have a big problem already.

So per se I feel it's just a test tool that once enabled in specific environments will make integration testing easier that is quite common (for example, even the fingerprint daemon comes with test devices enabled by default, but unless you've the power to setup some env variables on the process, then there's no way to give you access to those).

[didrocks]

Ah, regarding the example broker... How do you think we could handle that otherwise?
Ideally we want it having available for autopkgtests, IMHO, having a separate binary can't be a security issue as really if you can make it load through /etc/authd/borkers.d or dpkg -i/apt install it we defintely have a big problem already.

Maybe we should revisit the approach and having it as a real separate binary that could be built into CI and is subprocessed. That way, this will be more an end to end test using the whole stack, including the config file discovery and external dbus communication.

[3v1n0]

Yeah, that's fine for upstream, but my point is more for autopkgtests, where I'd like to install the various components as in distro and test the outcome.

I don't think there's another option than this, or running the daemon in a special mode...

[didrocks]

I mean, in the autopkgtests, we build the example broker separately and prepare the "config" on disk with our packaged authd. Then, we run the integration tests with this setup.

[3v1n0]

Yeah, that seems a viable option.

[3v1n0]

What about for manual-testing? I would at least like to have a ppa where we have this setup so that once we've to verify some cases, we have the broker setup ready for (indeed it's not hard to just run manually authd, but it would not imply testing a next-to-real setup).

[didrocks]

What about for manual-testing? I would at least like to have a ppa where we have this setup so that once we've to verify some cases, we have the broker setup ready for (indeed it's not hard to just run manually authd, but it would not imply testing a next-to-real setup).

That one is a valid argument. I suggest then that if we ship the additional binary package, then, I suggest that we don’t ship a configuration file nor the systemd service to start it. Instead, we have a command to "set it up" in the CLI, which installs the config file. That will force to run the example broker manually. This one will create the file, and on teardown, will remove it. That way, it’s not autostarted and we are sure running it was intentional.

Then, in our manual (and even automated/autokgtests setup), then, we have first to run it explciitely.

What do you think?

[3v1n0]

That's good, but why not just relying on systemd for that?

I think we can trust it :)

So, we ship the service as masked by default, in this way there's no way to start it, unless you explicitly systemctl start it. We need to modify a bit the daemon to not list it either in such case but it's an easy change.

[3v1n0]

BTW... I've dropped the generation changes from this branch, and also the example broker for now, but given this discussion I see if re-adding them 😋

[didrocks]

Let’s split the concerns and see in a separate PR for the second binary package. I still think we shouldn’t have it as a service as we will never want it to autostart (or maybe for some integration tests with reboot, but we need to discuss this first).

Btw, please add the GOTOOLCHAIN exported variable with its comments to debian/rules: https://github.com/canonical/ubuntu-pro-for-wsl/blob/main/wsl-pro-service/debian/rules#L9

[3v1n0]

Let’s split the concerns and see in a separate PR for the second binary package

Ok fair enough!

Btw, please add the GOTOOLCHAIN exported variable with its comments to debian/rules: https://github.com/canonical/ubuntu-pro-for-wsl/blob/main/wsl-pro-service/debian/rules#L9

I did only that on tests, because it's already on by default in dh-golang.

[didrocks]

Btw, please add the GOTOOLCHAIN exported variable with its comments to debian/rules: https://github.com/canonical/ubuntu-pro-for-wsl/blob/main/wsl-pro-service/debian/rules#L9

I did only that on tests, because it's already on by default in dh-golang.

Excellent, if this is in noble! I tried it few months ago and it wasn’t!

[3v1n0]

It's on 1.60 so it should have been there also in mantic.. 🤔

[didrocks]

It was definitively broken, should we give it a try?

[3v1n0]

It was definitively broken, should we give it a try?

It works so far here... I actually would like to have the workflow for this setup too, but while my old pr #130 code still works fine, we can't build using the common ones yet.

Btw... Branch rebased, removing the extra stuff... And I'll open another PR for that. In theory I could split this even in 3 (one with fixes for the generation phase), but I'd say to keep it as it is :)

[didrocks]

It works so far here... I actually would like to have the workflow for this setup too, but while my old pr #130 code still works fine, we can't build using the common ones yet.

Try to set the toolchain stenza to 1.22.3 for instance and see if the package builds.

[3v1n0]

Try to set the toolchain stenza to 1.22.3

I feel it's more a problem of dh-cargo when building the source package

[didrocks]

I feel it's more a problem of dh-cargo when building the source package

Just please try it :) And no, this was not an issue when building, but in CI, using mantic at the time.

[3v1n0]

I feel it's more a problem of dh-cargo when building the source package

Just please try it :) And no, this was not an issue when building, but in CI, using mantic at the time.

Tested, it does fail:

Build debian source package
  dpkg-buildpackage: info: source package authd
  dpkg-buildpackage: info: source version 0.2.1~24.04+1+f5610801
  dpkg-buildpackage: info: source distribution UNRELEASED
  dpkg-buildpackage: info: source changed by Marco <[email protected]>
   dpkg-source --before-build .
   debian/rules clean
  dh clean --buildsystem=golang --with=golang,apport
     debian/rules override_dh_auto_clean
  make[1]: Entering directory '/home/runner/work/authd/authd'
  dh_auto_clean
  dh_auto_clean --buildsystem=cargo
  debian cargo wrapper: options, profiles, parallel, lto: ['parallel=4'] ['noudeb'] ['-j4'] 0
  debian cargo wrapper: rust_type, gnu_type: x86_64-unknown-linux-gnu, x86_64-linux-gnu
  debian cargo wrapper: running subprocess (['env', 'RUST_BACKTRACE=1', '/usr/bin/cargo', 'clean', '--verbose', '--verbose'],) {'check': True}
       Removed 0 files
  # Vendor Go dependencies when building the source package
  [ -d vendor/ ] || go mod vendor
  go: downloading go1.22.3 (linux/amd64)
  go: download go1.22.3 for linux/amd64: toolchain not available
  make[1]: *** [debian/rules:43: override_dh_auto_clean] Error 1
  make[1]: Leaving directory '/home/runner/work/authd/authd'
  make: *** [debian/rules:37: clean] Error 2
  dpkg-buildpackage: error: debian/rules clean subprocess returned exit status 2

Full logs_163.zip attached of this action running on https://github.com/3v1n0/authd/commits/f561080c3793af902f1fc983bcbbade5adc79b1f

---

And note that the builder had full internet access during the source build.

[didrocks]

see, so for some reason, the dh-cargo thingy doesn’t work. Try now to export the variable as we do in wsl-pro-service

[didrocks]

Thanks Denis for reviewing my comments and Marco for fixing! :)