Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestClear/Error_when_cache_dir_has_invalid_permissions is failing when building the deb #218

Closed
2 tasks done
3v1n0 opened this issue Feb 21, 2024 · 7 comments · Fixed by #223
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@3v1n0
Copy link
Collaborator

3v1n0 commented Feb 21, 2024

Is there an existing issue for this?

  • I have searched the existing issues and found none that matched mine

Describe the issue

=== NAME  TestClear/Error_when_cache_dir_has_invalid_permissions
    db_test.go:460: 
        	Error Trace:	/build/authd-8HIczg/authd-0.2.2~gdm1/obj-x86_64-linux-gnu/src/github.com/ubuntu/authd/internal/users/cache/db_test.go:460
        	Error:      	An error is expected but got nil.
        	Test:       	TestClear/Error_when_cache_dir_has_invalid_permissions
        	Messages:   	Clear should return an error but didn't

Steps to reproduce it

  1. Just build the deb

Ubuntu users: System information and logs

No response

Non Ubuntu users: System information and logs

No response

Relevant information

Eh... If we had #130 active we wouldn't have missed it... :)

Double check your logs

  • I have redacted any sensitive information from the logs
@3v1n0 3v1n0 added the bug Something isn't working label Feb 21, 2024
@3v1n0
Copy link
Collaborator Author

3v1n0 commented Feb 21, 2024

@denisonbarbosa I guess you can handle this :)

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Feb 21, 2024

At least one seems to be skippable:

diff --git a/debian/control b/debian/control
index f808322..a9fb004 100644
--- a/debian/control
+++ b/debian/control
@@ -452,6 +452,11 @@ func TestClear(t *testing.T) {
 				require.NoError(t, os.Remove(c.DbPath()), "Setup: should be able to remove database file")
 			}
 			if tc.readOnlyDir {
+				currentUser, err := user.Current()
+				require.NoError(t, err)
+				if currentUser.Username == "root" {
+					t.Skip("Can't do permission checks as root")
+				}
 				testutils.MakeReadOnly(t, filepath.Dir(c.DbPath()))
 			}
 

@denisonbarbosa
Copy link
Member

The package builds normally for me and on launchpad as well. My question is: why are you building the deb as sudo?

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Feb 21, 2024

No sudo of course, but this is the error that you get when building it via sbuild that is what most ubuntu/debian developers do to build packages, so it's something to address.

In any case all the packages are run as fakeroot anyways, but in this case it may be related to different permission issues.

@denisonbarbosa
Copy link
Member

this is the error that you get when building it via sbuild

That's the kind of thing that should be listed on Relevant Information... 😉

Also, most of the tutorials and packaging documentation suggest using debuild to build the packages, so I'm a bit sceptical about the most ubuntu/debian developers do to build packages sentence.

My point is that if we try to cover "all" of the possible tools/ways to build the package, we could end up with a lot of code only to support a specific occurrence, which is not great.

In the team, we use gbp to build the packages (I'm not sure it's documented anywhere, so that's our bad, we should have it written down somewhere) and the permissions tests are essential.

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Feb 21, 2024

I suggest you to document a bit better ;-) on packaging, but in short, you should never (unless for tiny packages or for frequent rebuilds), build a package in the local environment because that affects a lot the build process making the whole process flawed.

To prevent this, and to make sure that a package has all the required components in depends & friends, one should build the package in a clean environment, and that's where schroot helps you and sbuild does nothing but calling dpkg-buldpackage inside a such clean environment. So, please use it :).

In the team, we use gbp to build the packages (I'm not sure it's documented anywhere, so that's our bad, we should have it written down somewhere) and the permissions tests are essential.

We documented it various years ago. And I do use gbp to generate the source packages too, but eventually it's sbuild that is used to build it in a clean environment. You can also use lxd, but it's harder to maintain.

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Feb 21, 2024

This page has also some quick infos: https://debian-handbook.info/browse/stable/debian-packaging.html

denisonbarbosa added a commit that referenced this issue Feb 29, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants