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

debian: run as a daemon, ask informed consent #162

Merged
merged 16 commits into from
Dec 15, 2020
Merged

Conversation

FedericoCeratto
Copy link
Contributor

@FedericoCeratto FedericoCeratto commented Nov 13, 2020

The PR also add 2 different GH Action pipelines for tagged/public releases and for PRs using 2 different bintray archives

@hellais
Copy link
Member

hellais commented Nov 19, 2020

Should it perhaps be called ooniprobe like the old package or do we want to avoid conflicts with the two?

debian/ooniprobe-cli.service Outdated Show resolved Hide resolved
@FedericoCeratto FedericoCeratto marked this pull request as ready for review December 4, 2020 20:09
@FedericoCeratto FedericoCeratto changed the title Debian package: run as a daemon Debian package: run as a daemon, ask informed consent Dec 11, 2020
Copy link
Contributor

@bassosimone bassosimone left a comment

Choose a reason for hiding this comment

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

Thank you very much for your excellent work in making a Debian package! I am super happy that we can soon proceed with a new package 🥳!

I have a bunch of requests for changes, which are mainly focused around updating the package to the way in which ooniprobe 3.1.0 works.

In the meanwhile, I'll spin up a Vagrant box and test more thoroughly.

🎸

debian/ooniprobe-cli.service Outdated Show resolved Hide resolved
[Unit]
Description=OONI CLI Probe
Documentation=man:ooniprobe-cli
#Documentation=file:///usr/share/doc/ooniprobe-cli/html/index.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#Documentation=file:///usr/share/doc/ooniprobe-cli/html/index.html

debian/ooniprobe-cli.service Outdated Show resolved Hide resolved
WorkingDirectory=/var/lib/ooniprobe

# Sandboxing
CapabilityBoundingSet=CAP_NET_BIND_SERVICE
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be the empty set of capabilities? (AFAIK we don't bind any privileged port)

ReadWriteDirectories=-/proc
ReadWriteDirectories=-/var/log/ooniprobe
ReadWriteDirectories=-/var/lib/ooniprobe
ReadWriteDirectories=-/var/run
Copy link
Contributor

Choose a reason for hiding this comment

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

My systemd-fu is such that I don't understand the - here. The manual to which I have access reads:

Paths in ReadOnlyDirectories= and InaccessibleDirectories= may be prefixed with "-", in which case they will be ignored when they do not exist. Note that using this setting will disconnect propagation of mounts from the service to the host (propagation in the opposite direction continues to work). This means that this setting may not be used for services which shall be able to install mount points in the main mount namespace.

I'm clearly missing some knowledge here, please help :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If those directories are missing for any reason, the systemd unit file will not break. E.g. /var/log/ooniprobe is not being created and that's ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

What confuses me is that the manual page that I linked to explicitly states that this behaviour is only occurring for ReadOnlyDirectories and InaccessibleDirectories, but above it's used for ReadWriteDirectories. It seems the Debian manual page for that topic says that also ReadWriteDirectories can be used along with the - modifier.

debian/ooniprobe-cli.timer Show resolved Hide resolved
debian/ooniprobe.conf.disabled Outdated Show resolved Hide resolved
debian/rules Show resolved Hide resolved
Federico Ceratto and others added 3 commits December 11, 2020 16:46
Co-authored-by: Simone Basso <bassosimone@gmail.com>
Co-authored-by: Simone Basso <bassosimone@gmail.com>
Co-authored-by: Simone Basso <bassosimone@gmail.com>
@bassosimone bassosimone changed the title Debian package: run as a daemon, ask informed consent debian: run as a daemon, ask informed consent Dec 15, 2020
@bassosimone
Copy link
Contributor

Thanks again @FedericoCeratto! I am going to merge when all the CIs are green. There are future improvements we can work on, including maybe finding a way to unify the stable and the testing builds file, whose diff is currently very limited. This diff being so limited, though, gives me confidence that the stable build is okay. For the records this is the diff:

% diff -u .github/workflows/linux-debian-packages{,-release}.yml
--- .github/workflows/linux-debian-packages.yml	2020-12-15 12:23:21.000000000 +0100
+++ .github/workflows/linux-debian-packages-release.yml	2020-12-15 12:21:54.000000000 +0100
@@ -1,10 +1,10 @@
-# Build a Debian package and publish on a test/internal archive
+# Build a Debian package only when a relase tag is applied
+# and publish it on the public/release archive
 name: linux-debian-packages
 on:
-  pull_request:
   push:
-    branches:
-      - master
+    tags:
+      - '*'
 jobs:
   build:
     runs-on: "ubuntu-20.04"
@@ -18,11 +18,10 @@
       - run: find . -name ooniprobe -type f -executable
       - run: sudo apt-get update -q
       - run: sudo apt-get build-dep -y --no-install-recommends .
-      # Use <probe version>~<github build number> as package version
+      # Use probe version as package version
       - run: |
-          VER=$(./CLI/linux/amd64/ooniprobe version)
-          DVER="${VER}~${GITHUB_RUN_NUMBER}"
-          dch -v $DVER "test version"
+          DVER=$(./CLI/linux/amd64/ooniprobe version)
+          dch -v $DVER "New release"
           dpkg-buildpackage -us -uc -b
           find ../ -name "*.deb" -type f
           DEB="../ooniprobe-cli_${DVER}_amd64.deb"
@@ -35,4 +34,4 @@
           BT_APIUSER: federicoceratto
           BT_ORG: ooni
           BT_PKGNAME: ooniprobe
-          BT_REPO: ooniprobe-debian-test
+          BT_REPO: ooniprobe-debian

@bassosimone bassosimone merged commit 8df91ec into master Dec 15, 2020
@bassosimone bassosimone deleted the packaging2 branch December 15, 2020 12:05
ainghazal pushed a commit to ainghazal/probe-cli that referenced this pull request Mar 8, 2022
* Set verbose mode, depend on adduser

* Run as daemon

* Generate manpage

* Implement informed consent

* Set version

* Switch format to native

* Set environment

* Update packaging

* Create test and release pipelines

* Update debian/ooniprobe-cli.service

Co-authored-by: Simone Basso <bassosimone@gmail.com>

* Update debian/ooniprobe-cli.service

Co-authored-by: Simone Basso <bassosimone@gmail.com>

* Update debian/ooniprobe.conf.disabled

Co-authored-by: Simone Basso <bassosimone@gmail.com>

* fix(linux-debian-packages): build also on pull requests

Otherwise there's no way for us to test :^).

* fix(debian/control): ubuntu 20.04 has debhelper 12

See https://packages.ubuntu.com/focal/debhelper

* fix(debian/control): debhelper-compat relations doesn't work the way I thought

* Update debian/ooniprobe-cli.timer

Co-authored-by: Simone Basso <bassosimone@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants