From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 225AD9132A for ; Wed, 3 Apr 2024 15:14:39 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EFEE218A16 for ; Wed, 3 Apr 2024 15:14:08 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 3 Apr 2024 15:14:07 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 8D27B44A72 for ; Wed, 3 Apr 2024 15:14:07 +0200 (CEST) Date: Wed, 03 Apr 2024 15:14:00 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20240402171629.536804-1-s.hanreich@proxmox.com> <20240402171629.536804-34-s.hanreich@proxmox.com> In-Reply-To: <20240402171629.536804-34-s.hanreich@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1712146806.hnyjn2lrom.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.059 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [network.target, architecture.mk, shutdown.target, defines.mk, proxmox.com, pkg-info.mk, gnu.org, timers.target, multi-user.target] Subject: Re: [pve-devel] [PATCH proxmox-firewall 33/37] firewall: add files for debian packaging X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Apr 2024 13:14:39 -0000 just looked at the packaging, mostly related to clean building, but not only. On April 2, 2024 7:16 pm, Stefan Hanreich wrote: > Signed-off-by: Stefan Hanreich > --- > Makefile | 93 +++++++++++++++++++++++++++++++++ > debian/changelog | 5 ++ > debian/control | 31 +++++++++++ > debian/copyright | 16 ++++++ > debian/proxmox-firewall.service | 16 ++++++ > debian/proxmox-firewall.timer | 11 ++++ > debian/rules | 14 +++++ > debian/source/format | 1 + > defines.mk | 13 +++++ > 9 files changed, 200 insertions(+) > create mode 100644 Makefile > create mode 100644 debian/changelog > create mode 100644 debian/control > create mode 100644 debian/copyright > create mode 100644 debian/proxmox-firewall.service > create mode 100644 debian/proxmox-firewall.timer > create mode 100644 debian/rules > create mode 100644 debian/source/format > create mode 100644 defines.mk >=20 > diff --git a/Makefile b/Makefile > new file mode 100644 > index 0000000..984c318 > --- /dev/null > +++ b/Makefile > @@ -0,0 +1,93 @@ > +include /usr/share/dpkg/pkg-info.mk > +include /usr/share/dpkg/architecture.mk > +include defines.mk > + > +PACKAGE=3Dproxmox-firewall > +BUILDDIR ?=3D $(PACKAGE)-$(DEB_VERSION_UPSTREAM) > + > + > +DEB=3D$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_HOST_ARCH).deb > +DBG_DEB=3D$(PACKAGE)-dbgsym_$(DEB_VERSION_UPSTREAM_REVISION)_$(DEB_HOST_= ARCH).deb > +DSC=3Drust-$(PACKAGE)_$(DEB_VERSION_UPSTREAM_REVISION).dsc this doesn't match d/control ;) > + > +DEBS =3D $(DEB) $(DBG_DEB) > + > +ifeq ($(BUILD_MODE), release) you need to set/export this in d/rules, else the package will contain a debug build.. > +CARGO_BUILD_ARGS +=3D --release > +COMPILEDIR :=3D target/release > +else > +COMPILEDIR :=3D target/debug > +endif > + > +USR_BIN :=3D \ > + proxmox-firewall > + > +COMPILED_BINS :=3D \ > + $(addprefix $(COMPILEDIR)/,$(USR_BIN)) > + > +all: cargo-build > + > +.PHONY: cargo-build > +cargo-build: > + cargo build $(CARGO_BUILD_ARGS) > + > +$(COMPILED_BINS): cargo-build > + > +install: $(COMPILED_BINS) > + install -dm755 $(DESTDIR)$(SBINDIR) > + $(foreach i,$(USR_BIN), \ > + install -m755 $(COMPILEDIR)/$(i) $(DESTDIR)$(SBINDIR)/ ;) I am not sure this (and all the helper stuff above that only exists for it) make much sense. maybe we could simply get `$(CARGO) install` to work here, and then let the packging put it into /usr/sbin ? > + > +update-dcontrol: #$(BUILDDIR) > + debcargo package \ > + --config debian/debcargo.toml \ > + --changelog-ready \ > + --no-overlay-write-back \ > + --directory $(BUILDDIR) \ > + $(PACKAGE) \ > + $(shell dpkg-parsechangelog -l debian/changelog -SVersion | sed -e 's= /-.*//') > + cat $(BUILDDIR)/debian/control debian/control.extra > debian/control > + wrap-and-sort -t -k -f debian/control this doesn't work because there is no debian/debcargo.toml and also no debian/control.extra ;) since debcargo doesn't work with workspaces (yet), it probably doesn't make sense to leave this in.. one just has to remember to update d/control when updating any of the Cargo.toml files w.r.t. dependencies or features. > + > +.PHONY: build > +build: $(BUILDDIR) > +$(BUILDDIR): > + rm -rf $@ $@.tmp; mkdir $@.tmp > + cp -a proxmox-firewall proxmox-nftables proxmox-ve-config debian Cargo.= toml Makefile defines.mk $@.tmp/ this is missing the .cargo dir, which doesn't matter (much) for building directly via `make deb` (since cargo will look in parent dirs), but completely breaks for `make dsc` or `make sbuild`. but I suggest handling that in d/rules (see comment there) > + mv $@.tmp $@ > + > +.PHONY: deb > +deb: $(DEB) > +$(HELPER_DEB) $(DBG_DEB) $(HELPER_DBG_DEB) $(DOC_DEB): $(DEB) > +$(DEB): $(BUILDDIR) > + cd $(BUILDDIR); dpkg-buildpackage -b -us -uc --no-pre-clean > + lintian $(DEB) $(DOC_DEB) $(HELPER_DEB) > + > +.PHONY: test > +test: > + cargo test > + > +.PHONY: dsc > +dsc: > + rm -rf $(BUILDDIR) $(DSC) > + $(MAKE) $(DSC) > + lintian $(DSC) > +$(DSC): $(BUILDDIR) > + cd $(BUILDDIR); dpkg-buildpackage -S -us -uc -d -nc > + > +sbuild: $(DSC) > + sbuild $< > + > +.PHONY: dinstall > +dinstall: $(DEB) > + dpkg -i $(DEB) $(DBG_DEB) $(DOC_DEB) > + > +.PHONY: distclean > +distclean: clean > + > +.PHONY: clean > +clean: > + cargo clean > + rm -f *.deb *.build *.buildinfo *.changes *.dsc rust-$(PACKAGE)*.tar* > + rm -rf $(PACKAGE)-[0-9]*/ > + find . -name '*~' -exec rm {} ';' > diff --git a/debian/changelog b/debian/changelog > new file mode 100644 > index 0000000..7918ec9 > --- /dev/null > +++ b/debian/changelog > @@ -0,0 +1,5 @@ > +proxmox-firewall (0.1-1) UNRELEASED; urgency=3Dmedium the `-1` and `debian/source/format` disagree (again, only a problem when attempting building via sbuild) > + > + * Initial release. > + > + -- Stefan Hanreich Thu, 07 Mar 2024 10:15:10 = +0100 > diff --git a/debian/control b/debian/control > new file mode 100644 > index 0000000..e04ce68 > --- /dev/null > +++ b/debian/control > @@ -0,0 +1,31 @@ > +Source: proxmox-firewall > +Section: admin > +Priority: optional > +Maintainer: Proxmox Support Team > +Build-Depends: cargo:native, > + debhelper-compat (=3D 13), > + dh-cargo (>=3D 25), you aren't actually using that though? (and probably shouldn't, it's mainly for "simple" crate <-> package situations) > + librust-anyhow-1+default-dev, > + librust-env-logger-0.10+default-dev, > + librust-log-0.4+default-dev (>=3D 0.4.17-~~), > + librust-nix-0.26+default-dev (>=3D 0.26.1-~~), > + librust-serde-1+default-dev, > + librust-serde-1+derive-dev, > + librust-serde-json-1+default-dev, > + librust-serde-plain-1+default-dev, > + librust-serde-plain-1+default-dev, > + librust-serde-with+default-dev, > + librust-libc-0.2+default-dev, > + librust-proxmox-schema-3+default-dev, this is missing at least libnftables-dev and netbase > +Standards-Version: 4.6.2 > +Homepage: https://www.proxmox.com > + > +Package: proxmox-firewall > +Architecture: any > +Conflicts: ulogd, > +Depends: ${misc:Depends}, ${shlibs:Depends}, > + pve-firewall, > + nftables, this is missing at least netbase ;) > +Description: Proxmox VE nft Firewall > + This package contains a nftables-based implementation of the Proxmox VE > + Firewall > diff --git a/debian/copyright b/debian/copyright > new file mode 100644 > index 0000000..fe09a1b > --- /dev/null > +++ b/debian/copyright > @@ -0,0 +1,16 @@ > +Copyright (C) 2018-2024 Proxmox Server Solutions GmbH > + > +This software is written by Proxmox Server Solutions GmbH > + > +This program is free software: you can redistribute it and/or modify > +it under the terms of the GNU Affero General Public License as published= by > +the Free Software Foundation, either version 3 of the License, or > +(at your option) any later version. > + > +This program is distributed in the hope that it will be useful, > +but WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +GNU Affero General Public License for more details. > + > +You should have received a copy of the GNU Affero General Public License > +along with this program. If not, see . > diff --git a/debian/proxmox-firewall.service b/debian/proxmox-firewall.se= rvice > new file mode 100644 > index 0000000..5f9bf4b > --- /dev/null > +++ b/debian/proxmox-firewall.service > @@ -0,0 +1,16 @@ > +[Unit] > +Description=3DProxmox VE nftables firewall > +ConditionPathExists=3D/usr/sbin/proxmox-firewall when would this path not exist? > +Wants=3Dpve-cluster.service pvefw-logger.service > +After=3Dpvefw-logger.service pve-cluster.service network.target systemd-= modules-load.service should the timer also get this to avoid premature starting? or am I misremembering how timers and their services interact :) > +DefaultDependencies=3Dno > +Before=3Dshutdown.target > +Conflicts=3Dshutdown.target > + > +[Service] > +ExecStart=3D/usr/sbin/proxmox-firewall > +Type=3Doneshot > + > +[Install] > +WantedBy=3Dmulti-user.target > + so this is started by the timer below (on boot and then every 5s) but also enabled and thus started once on boot? that seems confusing ;) > diff --git a/debian/proxmox-firewall.timer b/debian/proxmox-firewall.time= r > new file mode 100644 > index 0000000..d051102 > --- /dev/null > +++ b/debian/proxmox-firewall.timer > @@ -0,0 +1,11 @@ > +[Unit] > +Description=3DProxmox VE nft Firewall timer capitalisation compared to the service above (and also, nft vs nftables) > + > +[Timer] > +OnBootSec=3D1s > +OnUnitInactiveSec=3D5s > +Unit=3Dproxmox-firewall.service > + > +[Install] > +WantedBy=3Dtimers.target > + > diff --git a/debian/rules b/debian/rules > new file mode 100644 d/rules should be executable :) > index 0000000..5539a00 > --- /dev/null > +++ b/debian/rules > @@ -0,0 +1,14 @@ > +#!/usr/bin/make -f > + > +# Uncomment this to turn on verbose mode. > +#export DH_VERBOSE=3D1 > + > +%: > + dh $@ > + I would suggest calling the cargo wrapper here to create a proper .cargo/config file for a clean build. you can look at proxmox-backup for inspiration ;) here's a starting point: diff --git a/debian/rules b/debian/rules index 5539a00..bbb4d0a 100644 --- a/debian/rules +++ b/debian/rules @@ -1,14 +1,31 @@ #!/usr/bin/make -f =20 # Uncomment this to turn on verbose mode. -#export DH_VERBOSE=3D1 +export DH_VERBOSE=3D1 + +include /usr/share/dpkg/pkg-info.mk +include /usr/share/rustc/architecture.mk + +export BUILD_MODE=3Drelease + +CARGO=3D/usr/share/cargo/bin/cargo + +export CFLAGS CXXFLAGS CPPFLAGS LDFLAGS +export DEB_HOST_RUST_TYPE DEB_HOST_GNU_TYPE +export CARGO_HOME =3D $(CURDIR)/debian/cargo_home + +export DEB_CARGO_CRATE=3Dproxmox-firewall_$(DEB_VERSION_UPSTREAM) +export DEB_CARGO_PACKAGE=3Dproxmox-firewall =20 %: dh $@ =20 +override_dh_auto_configure: + @perl -ne 'if (/^version\s*=3D\s*"(\d+(?:\.\d+)+)"/) { my $$v_cargo =3D $= $1; my $$v_deb =3D "$(DEB_VERSION_UPSTREAM)"; \ + die "ERROR: d/changelog <-> Cargo.toml version mismatch: $$v_cargo != =3D $$v_deb\n" if $$v_cargo ne $$v_deb; exit(0); }' Cargo.toml + $(CARGO) prepare-debian $(CURDIR)/debian/cargo_registry --link-from-syste= m + dh_auto_configure + override_dh_installsystemd: dh_installsystemd --no-start proxmox-firewall.service dh_installsystemd proxmox-firewall.timer we probably also want `cargo` in the Makefile to be $(CARGO) (defaulting to `cargo` there if not set). that way, `cargo build` and `cargo test` also go through the wrapper and pick up any special sauce > +override_dh_installsystemd: > + dh_installsystemd --no-start proxmox-firewall.service > + dh_installsystemd proxmox-firewall.timer > + > +override_dh_installinit: > + don't think this dh_installinit override is needed? > diff --git a/debian/source/format b/debian/source/format > new file mode 100644 > index 0000000..89ae9db > --- /dev/null > +++ b/debian/source/format > @@ -0,0 +1 @@ > +3.0 (native) native means no Debian revision in the version, see above w.r.t. d/changelog > diff --git a/defines.mk b/defines.mk > new file mode 100644 > index 0000000..e01164d > --- /dev/null > +++ b/defines.mk > @@ -0,0 +1,13 @@ > +PREFIX =3D /usr > +BINDIR =3D $(PREFIX)/bin > +SBINDIR =3D $(PREFIX)/sbin > +LIBDIR =3D $(PREFIX)/lib > +LIBEXECDIR =3D $(LIBDIR) > +DATAROOTDIR =3D $(PREFIX)/share > +MAN1DIR =3D $(PREFIX)/share/man/man1 > +MAN5DIR =3D $(PREFIX)/share/man/man5 > +SYSCONFDIR =3D /etc > + > +# For local overrides > +-include local.mak > + > --=20 > 2.39.2 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20