Skip to content

build: add Debian packaging#120

Merged
GabrielNagy merged 6 commits intomainfrom
debian-packaging
Dec 5, 2023
Merged

build: add Debian packaging#120
GabrielNagy merged 6 commits intomainfrom
debian-packaging

Conversation

@GabrielNagy
Copy link
Copy Markdown
Contributor

@GabrielNagy GabrielNagy commented Nov 29, 2023

Build the authd package in a single binary package containing the daemon, PAM & NSS modules.

Add hooks to ensure PAM & NSS are properly configured and un-configured.

Most of this was inspired by adsys and aad-auth packaging where we also ship PAM (and NSS) modules.

d/copyright is still incomplete but it's an arduous task we can handle afterwards.

Fixes UDENG-1781

A passing LP build can be seen here: https://launchpad.net/~gabuscus/+archive/ubuntu/ppa/+sourcepub/15403265/+listing-archive-extra

I've also ran the autopkgtests for amd64 locally with qemu.

I haven't added systemd units yet because I did some tests and something is afoot causing the entire boot to hang. It looks like if authd starts right after dbus, both services will remain in an "activating" state. I'm not sure why this happens, but my assumption is that we are connecting to the system bus before it's actually ready to accept connections. I'll open a card and we can investigate the issue separately without needing to block this one.

@GabrielNagy GabrielNagy requested a review from a team as a code owner November 29, 2023 22:21
denisonbarbosa
denisonbarbosa reviewed Nov 30, 2023
View reviewed changes
didrocks
didrocks requested changes Nov 30, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@didrocks didrocks left a comment

Choose a reason for hiding this comment

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

Excellent work! I have one real required change (leftover of other project copy/paste I think), and 2 questions!

Also, along those line, can you ensure that current copy of: pam/go-loader/libpam-authd.pam-auth-update allows to have our PAM module being called before pam_unix.so as otherwise, this latter will always request a password for us?

3v1n0
3v1n0 reviewed Nov 30, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

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

Few comments on my side :)

@GabrielNagy
Copy link
Copy Markdown
Contributor Author

GabrielNagy commented Dec 4, 2023

Also, along those line, can you ensure that current copy of: pam/go-loader/libpam-authd.pam-auth-update allows to have our PAM module being called before pam_unix.so as otherwise, this latter will always request a password for us?

Yes, pam_unix has a priority of 256 and pam_authd has 280 so the latter wins.

@GabrielNagy GabrielNagy force-pushed the debian-packaging branch 2 times, most recently from 460fb8c to 433bbcd Compare December 4, 2023 11:48
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 4, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.98%. Comparing base (bf88ae2) to head (aef2d42).
Report is 1887 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
- Coverage   88.14%   87.98%   -0.17%     
==========================================
  Files          32       32              
  Lines        2430     2430              
==========================================
- Hits         2142     2138       -4     
- Misses        221      224       +3     
- Partials       67       68       +1

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@GabrielNagy
Copy link
Copy Markdown
Contributor Author

GabrielNagy commented Dec 4, 2023

So I believe I made all the requested changes, but the package doesn't build anymore with vendored dependencies due to https://github.com/ubuntu/authd/blob/bf88ae20223faf371eb743ee385b08e786462f22/pam/pam.go#L1

 go run github.com/msteinert/pam/cmd/pam-moduler -libname pam_authd.so -type pamModule -tags !pam_binary_cli
cannot find module providing package github.com/msteinert/pam/cmd/pam-moduler: import lookup disabled by -mod=vendor
	(Go version in go.mod is at least 1.14 and vendor directory exists.)
pam/pam.go:1: running "go": exit status 1

@GabrielNagy GabrielNagy force-pushed the debian-packaging branch 3 times, most recently from d103435 to 9097f56 Compare December 4, 2023 16:32
@GabrielNagy
Copy link
Copy Markdown
Contributor Author

GabrielNagy commented Dec 4, 2023

Addressed the issue above in the last commit. Confirmed the package builds and test pass on all avenues.

@GabrielNagy GabrielNagy force-pushed the debian-packaging branch 3 times, most recently from c48ff9d to 7f4520e Compare December 5, 2023 10:30
didrocks
didrocks approved these changes Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@didrocks didrocks left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Everything’s addressed from my side, thanks!

denisonbarbosa
denisonbarbosa approved these changes Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@denisonbarbosa denisonbarbosa left a comment

Choose a reason for hiding this comment

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

LGTM. Nice job!

3v1n0
3v1n0 requested changes Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

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

I think we're almost there, but I noticed few more things 👼

3v1n0
3v1n0 reviewed Dec 5, 2023
View reviewed changes
3v1n0
3v1n0 reviewed Dec 5, 2023
View reviewed changes
3v1n0
3v1n0 reviewed Dec 5, 2023
View reviewed changes
3v1n0
3v1n0 reviewed Dec 5, 2023
View reviewed changes
3v1n0
3v1n0 reviewed Dec 5, 2023
View reviewed changes
3v1n0
3v1n0 reviewed Dec 5, 2023
View reviewed changes
@GabrielNagy GabrielNagy force-pushed the debian-packaging branch 4 times, most recently from a8088ec to e408ced Compare December 5, 2023 18:41
@GabrielNagy GabrielNagy requested a review from 3v1n0 December 5, 2023 18:54
@GabrielNagy
Copy link
Copy Markdown
Contributor Author

GabrielNagy commented Dec 5, 2023

@3v1n0 I'd appreciate if you could go over your first round of review comments and mark everything applicable as resolved, otherwise it's hard to follow what was done and what was not 🙏🏼

@3v1n0
Copy link
Copy Markdown
Contributor

3v1n0 commented Dec 5, 2023

@3v1n0 I'd appreciate if you could go over your first round of review comments and mark everything applicable as resolved, otherwise it's hard to follow what was done and what was not 🙏🏼

I would love to... But I can't :(

I think i've not the permissions in this repo, as I was trying to this the whole time and I couldn't find a way.

3v1n0
3v1n0 approved these changes Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

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

I think we're good... Please just drop that ls subshell in the vendor script and we're good to go from my POV.

@GabrielNagy
Add workflow to auto-update Rust vendored sources
719ff22 
Similar to what we have in aad-auth, this helps keep the
XS-Vendored-Sources-Rust field in debian/changelog up-to-date.
@GabrielNagy
Add Debian packaging
fcf3b71 
Build the authd package in a single binary package containing the
daemon, PAM & NSS modules.

Add hooks to ensure PAM & NSS are properly configured and un-configured.

Most of this was inspired by adsys and aad-auth packaging where we also
ship PAM (and NSS) modules.

Fixes UDENG-1781
@GabrielNagy
Add jq as a build dependency
486a1c8 
On the account of it being needed when building the source package.
@GabrielNagy
Export variables used multiple times
559299f
@GabrielNagy
Skip pam-moduler call if dependencies are vendored
36d2ea8 
We depend on github.com/msteinert/pam/cmd/pam-moduler to generate the
pam_module.go file for the PAM module. This poses an issue if we are
trying to run it with vendored dependencies, as we cannot vendor the cmd
package.

This can be fixed multiple ways, but in the interest of keeping the fix
close to the problem, and avoid moving the generate statement to a
separate package/file, simply skip the call if we detect a vendor
directory at the root of the project.
@GabrielNagy
Add ldconfig trigger
aef2d42 
This is not added automatically, presumably because the name of the
packge does not contain "lib". Add it using Debian's guidelines[1].

[1] https://www.debian.org/doc/debian-policy/ch-sharedlibs.html#ldconfig
@GabrielNagy GabrielNagy merged commit c03337a into main Dec 5, 2023
@GabrielNagy GabrielNagy deleted the debian-packaging branch December 5, 2023 22:23
@3v1n0 3v1n0 mentioned this pull request Dec 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@denisonbarbosa denisonbarbosa denisonbarbosa approved these changes

@3v1n0 3v1n0 3v1n0 approved these changes

@didrocks didrocks didrocks approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@GabrielNagy @codecov-commenter @3v1n0 @didrocks @denisonbarbosa