[pve-devel] [RFC PATCH installer 0/5] fix #5579: allow specifying optional first-boot script

[pve-devel] [RFC PATCH installer 0/5] fix #5579: allow specifying optional first-boot script

[Christoph Heiss]

This implements #5579 [0] as proposed by Thomas [1].

Sending it as an RFC as probably some bike-shedding has still to be done
anyway, in regards of naming/filesystem locations etc. -- but should
give an good idea how it can/should work in the end.

Adds a new (optional) section to the auto-installer answer file called
`first-boot`, which can be used to the configure a script/executable
file to run on the first boot after the installation.

To used the baked-in (via the `proxmox-auto-install-assistant prepare-iso
--on-first-boot`) file from the ISO:

  [first-boot]
  source = "from-iso"

Or fetching it from a URL:

  [first-boot]
  source = "from-url"
  url = "http://example.com/first-boot"
  cert_fingerprint = ".." # if needed

The structure the section is mostly taken from the `network` section to
provide consistency.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=5579
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=5579#c5

Inner workings
==============

This creates a new package `proxmox-first-boot`, which only has a
systemd .service file. The service is conditioned to start based on the 
existence of a flag file in /var/lib/proxmox-first-boot, which will
be created by the installer if such a script is supplied by the user.

Q: The condition could maybe be set directly on the script instead of a
   separate file, since the executable file is only present when
   supplied anyway?

Testing
=======

Tested this with the latest (8.2-2) PVE ISO, both baking it into the ISO
and fetching it from an URL. Did not test with PBS explicitly (yet), but
I see no reason why it shouldn't work, as it is completely
product-agnostic.

Diffstat
========

Christoph Heiss (5):
  common: add function for issuing HTTP GET requests
  fix #5579: first-boot: add initial service packaging
  fix #5579: auto-install-assistant: enable baking in first-boot script
  fix #5579: auto-installer: add optional first-boot hook script
  fix #5579: install: copy over `proxmox-first-boot` script if present

 Makefile                                      | 13 +++-
 Proxmox/Install.pm                            | 20 ++++++
 debian/control                                |  7 ++
 debian/proxmox-first-boot.install             |  1 +
 debian/rules.proxmox-first-boot               | 13 ++++
 proxmox-auto-install-assistant/Cargo.toml     |  1 +
 proxmox-auto-install-assistant/src/main.rs    | 17 +++++
 proxmox-auto-installer/Cargo.toml             |  2 +-
 proxmox-auto-installer/src/answer.rs          | 27 +++++++
 .../src/bin/proxmox-auto-installer.rs         | 42 +++++++++--
 proxmox-auto-installer/src/utils.rs           | 15 +++-
 proxmox-first-boot/Makefile                   | 11 +++
 .../etc/proxmox-first-boot.service            | 16 +++++
 proxmox-installer-common/src/http.rs          | 71 +++++++++++++------
 proxmox-installer-common/src/lib.rs           |  3 +
 15 files changed, 227 insertions(+), 32 deletions(-)
 create mode 100644 debian/proxmox-first-boot.install
 create mode 100644 debian/rules.proxmox-first-boot
 create mode 100644 proxmox-first-boot/Makefile
 create mode 100644 proxmox-first-boot/etc/proxmox-first-boot.service

-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC PATCH installer 1/5] common: add function for issuing HTTP GET requests

[Christoph Heiss]

Factors out the user-agent building into a separate function and then
re-uses that for get().

This has the side-effect that now for all requests issued by post() a
timeout of 60s is applied. Previously, this was only done when an
explicit fingerprint was given. Minute change and shouldn't effect
anything.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-installer-common/src/http.rs | 71 +++++++++++++++++++---------
 1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/proxmox-installer-common/src/http.rs b/proxmox-installer-common/src/http.rs
index b754ed8..f4afe14 100644
--- a/proxmox-installer-common/src/http.rs
+++ b/proxmox-installer-common/src/http.rs
@@ -4,33 +4,25 @@ use sha2::{Digest, Sha256};
 use std::sync::Arc;
 use ureq::{Agent, AgentBuilder};
 
-/// Issues a POST request with the payload (JSON). Optionally a SHA256 fingerprint can be used to
-/// check the cert against it, instead of the regular cert validation.
+/// Builds an [`Agent`] with TLS suitable set up, depending whether a custom fingerprint was
+/// supplied or not. If a fingerprint was supplied, only matching certificates will be accepted.
+/// Otherwise, the system certificate store is loaded.
+///
 /// To gather the sha256 fingerprint you can use the following command:
 /// ```no_compile
 /// openssl s_client -connect <host>:443 < /dev/null 2>/dev/null | openssl x509 -fingerprint -sha256  -noout -in /dev/stdin
 /// ```
 ///
 /// # Arguments
-/// * `url` - URL to call
 /// * `fingerprint` - SHA256 cert fingerprint if certificate pinning should be used. Optional.
-/// * `payload` - The payload to send to the server. Expected to be a JSON formatted string.
-pub fn post(url: &str, fingerprint: Option<&str>, payload: String) -> Result<String> {
-    let answer;
-
+fn build_agent(fingerprint: Option<&str>) -> Result<Agent> {
     if let Some(fingerprint) = fingerprint {
         let tls_config = ClientConfig::builder()
             .with_safe_defaults()
             .with_custom_certificate_verifier(VerifyCertFingerprint::new(fingerprint)?)
             .with_no_client_auth();
 
-        let agent: Agent = AgentBuilder::new().tls_config(Arc::new(tls_config)).build();
-
-        answer = agent
-            .post(url)
-            .set("Content-Type", "application/json; charset=utf-8")
-            .send_string(&payload)?
-            .into_string()?;
+        Ok(AgentBuilder::new().tls_config(Arc::new(tls_config)).build())
     } else {
         let mut roots = rustls::RootCertStore::empty();
         for cert in rustls_native_certs::load_native_certs()? {
@@ -42,18 +34,51 @@ pub fn post(url: &str, fingerprint: Option<&str>, payload: String) -> Result<Str
             .with_root_certificates(roots)
             .with_no_client_auth();
 
-        let agent = AgentBuilder::new()
+        Ok(AgentBuilder::new()
             .tls_connector(Arc::new(native_tls::TlsConnector::new()?))
             .tls_config(Arc::new(tls_config))
-            .build();
-        answer = agent
-            .post(url)
-            .set("Content-Type", "application/json; charset=utf-8")
-            .timeout(std::time::Duration::from_secs(60))
-            .send_string(&payload)?
-            .into_string()?;
+            .build())
     }
-    Ok(answer)
+}
+
+/// Issues a GET request to the specified URL and fetches the response. Optionally a SHA256
+/// fingerprint can be used to check the certificate against it, instead of the regular certificate
+/// validation.
+///
+/// To gather the sha256 fingerprint you can use the following command:
+/// ```no_compile
+/// openssl s_client -connect <host>:443 < /dev/null 2>/dev/null | openssl x509 -fingerprint -sha256  -noout -in /dev/stdin
+/// ```
+///
+/// # Arguments
+/// * `url` - URL to fetch
+/// * `fingerprint` - SHA256 cert fingerprint if certificate pinning should be used. Optional.
+pub fn get(url: &str, fingerprint: Option<&str>) -> Result<String> {
+    Ok(build_agent(fingerprint)?
+        .get(url)
+        .timeout(std::time::Duration::from_secs(60))
+        .call()?
+        .into_string()?)
+}
+
+/// Issues a POST request with the payload (JSON). Optionally a SHA256 fingerprint can be used to
+/// check the cert against it, instead of the regular cert validation.
+/// To gather the sha256 fingerprint you can use the following command:
+/// ```no_compile
+/// openssl s_client -connect <host>:443 < /dev/null 2>/dev/null | openssl x509 -fingerprint -sha256  -noout -in /dev/stdin
+/// ```
+///
+/// # Arguments
+/// * `url` - URL to call
+/// * `fingerprint` - SHA256 cert fingerprint if certificate pinning should be used. Optional.
+/// * `payload` - The payload to send to the server. Expected to be a JSON formatted string.
+pub fn post(url: &str, fingerprint: Option<&str>, payload: String) -> Result<String> {
+    Ok(build_agent(fingerprint)?
+        .post(url)
+        .set("Content-Type", "application/json; charset=utf-8")
+        .timeout(std::time::Duration::from_secs(60))
+        .send_string(&payload)?
+        .into_string()?)
 }
 
 struct VerifyCertFingerprint {
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [RFC PATCH installer 1/5] common: add function for issuing HTTP GET requests

[Thomas Lamprecht]

Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> Factors out the user-agent building into a separate function and then
> re-uses that for get().
> 
> This has the side-effect that now for all requests issued by post() a
> timeout of 60s is applied. Previously, this was only done when an
> explicit fingerprint was given. Minute change and shouldn't effect
> anything.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  proxmox-installer-common/src/http.rs | 71 +++++++++++++++++++---------
>  1 file changed, 48 insertions(+), 23 deletions(-)
> 
>

applied this one already, thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

While there is the `systemd-first-boot.service`, it uses the
non-existence of `/etc/machine-id` as condition to run. As we already
set up that file in the installer ourselves, we cannot use that.

Instead our service depends on a custom flag file in
/var/lib/proxmox-first-boot and will only run if that is present.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Makefile                                         | 13 ++++++++++---
 debian/control                                   |  7 +++++++
 debian/proxmox-first-boot.install                |  1 +
 debian/rules.proxmox-first-boot                  | 13 +++++++++++++
 proxmox-first-boot/Makefile                      | 11 +++++++++++
 .../etc/proxmox-first-boot.service               | 16 ++++++++++++++++
 6 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 debian/proxmox-first-boot.install
 create mode 100644 debian/rules.proxmox-first-boot
 create mode 100644 proxmox-first-boot/Makefile
 create mode 100644 proxmox-first-boot/etc/proxmox-first-boot.service

diff --git a/Makefile b/Makefile
index d85347f..a17f6c5 100644
--- a/Makefile
+++ b/Makefile
@@ -5,6 +5,10 @@ BUILDDIR ?= $(PACKAGE)-$(DEB_VERSION_UPSTREAM)
 
 DEB=$(PACKAGE)_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
 ASSISTANT_DEB=proxmox-auto-install-assistant_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
+FIRST_BOOT_DEB=proxmox-first-boot_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
+
+ALL_DEBS = $(DEB) $(ASSISTANT_DEB) $(FIRST_BOOT_DEB)
+
 DSC=$(PACKAGE)_$(DEB_VERSION).dsc
 
 CARGO ?= cargo
@@ -61,6 +65,7 @@ $(BUILDDIR):
 	  proxmox-tui-installer/ \
 	  proxmox-installer-common/ \
 	  proxmox-post-hook \
+	  proxmox-first-boot \
 	  test/ \
 	  $(SHELL_SCRIPTS) \
 	  $@.tmp
@@ -73,9 +78,10 @@ country.dat: country.pl
 
 deb: $(DEB)
 $(ASSISTANT_DEB): $(DEB)
+$(FIRST_BOOT_DEB): $(DEB)
 $(DEB): $(BUILDDIR)
 	cd $(BUILDDIR); dpkg-buildpackage -b -us -uc
-	lintian $(DEB) $(ASSISTANT_DEB)
+	lintian $(ALL_DEBS)
 
 test-$(DEB): $(INSTALLER_SOURCES)
 	rsync --exclude='test*.img' --exclude='*.deb' --exclude='build' -a * build
@@ -114,6 +120,7 @@ HTMLDIR=$(VARLIBDIR)/html/common
 install: $(INSTALLER_SOURCES) $(COMPILED_BINS)
 	$(MAKE) -C banner install
 	$(MAKE) -C Proxmox install
+	$(MAKE) -C proxmox-first-boot install
 	install -D -m 644 interfaces $(DESTDIR)/etc/network/interfaces
 	install -D -m 755 fake-start-stop-daemon $(VARLIBDIR)/fake-start-stop-daemon
 	install -D -m 755 policy-disable-rc.d $(VARLIBDIR)/policy-disable-rc.d
@@ -143,8 +150,8 @@ cargo-build:
 
 .PHONY: upload
 upload: UPLOAD_DIST ?= $(DEB_DISTRIBUTION)
-upload: $(DEB) $(ASSISTANT_DEB)
-	tar cf - $(DEB) $(ASSISTANT_DEB) | ssh -X repoman@repo.proxmox.com -- upload --product pve,pmg,pbs --dist $(UPLOAD_DIST)
+upload: $(ALL_DEBS)
+	tar cf - $(ALL_DEBS) | ssh -X repoman@repo.proxmox.com -- upload --product pve,pmg,pbs --dist $(UPLOAD_DIST)
 
 %.img:
 	truncate -s 2G $@
diff --git a/debian/control b/debian/control
index ff00cc2..fd0f4df 100644
--- a/debian/control
+++ b/debian/control
@@ -62,3 +62,10 @@ Description: Assistant to help with automated installations
  Provides a helper that can assist with creating an answer file for a automated
  installation of a Proxmox project, and preparing a official ISO image to use
  this answer file.
+
+Package: proxmox-first-boot
+Architecture: any
+Depends: ${misc:Depends},
+Description: Service which runs on the first system boot for additional setup
+ Provides a service which will run on the first boot if the a script was
+ configured through the auto-installer.
diff --git a/debian/proxmox-first-boot.install b/debian/proxmox-first-boot.install
new file mode 100644
index 0000000..52f25d2
--- /dev/null
+++ b/debian/proxmox-first-boot.install
@@ -0,0 +1 @@
+lib/systemd/system/proxmox-first-boot.service
diff --git a/debian/rules.proxmox-first-boot b/debian/rules.proxmox-first-boot
new file mode 100644
index 0000000..5b30d35
--- /dev/null
+++ b/debian/rules.proxmox-first-boot
@@ -0,0 +1,13 @@
+#!/usr/bin/make -f
+# See debhelper(7) (uncomment to enable)
+# output every command that modifies files on the build system.
+#DH_VERBOSE = 1
+
+%:
+	dh $@
+
+override_dh_missing:
+	dh_missing --fail-missing
+
+override_dh_installsystemd:
+	dh_installsystemd --no-stop-on-upgrade --no-start proxmox-first-boot.service
diff --git a/proxmox-first-boot/Makefile b/proxmox-first-boot/Makefile
new file mode 100644
index 0000000..137de23
--- /dev/null
+++ b/proxmox-first-boot/Makefile
@@ -0,0 +1,11 @@
+all:
+
+DESTDIR =
+LIBSYSTEMD_DIR = $(DESTDIR)/lib/systemd/system
+
+.PHONY: install
+install: etc/proxmox-first-boot.service
+	install -D -m 644 etc/proxmox-first-boot.service $(LIBSYSTEMD_DIR)/proxmox-first-boot.service
+
+.PHONY: clean
+clean:
diff --git a/proxmox-first-boot/etc/proxmox-first-boot.service b/proxmox-first-boot/etc/proxmox-first-boot.service
new file mode 100644
index 0000000..046bb24
--- /dev/null
+++ b/proxmox-first-boot/etc/proxmox-first-boot.service
@@ -0,0 +1,16 @@
+[Unit]
+Description=Proxmox First Boot Setup
+After=systemd-remount-fs.service
+Before=network-pre.target
+Wants=network-pre.target
+ConditionPathExists=/var/lib/proxmox-first-boot/pending-first-boot-setup
+ConditionPathIsReadWrite=/var/lib
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=/var/lib/proxmox-first-boot/proxmox-first-boot
+ExecStartPost=/usr/bin/rm -v /var/lib/proxmox-first-boot/pending-first-boot-setup
+
+[Install]
+WantedBy=network.target
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Thomas Lamprecht]

Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> diff --git a/proxmox-first-boot/etc/proxmox-first-boot.service b/proxmox-first-boot/etc/proxmox-first-boot.service
> new file mode 100644
> index 0000000..046bb24
> --- /dev/null
> +++ b/proxmox-first-boot/etc/proxmox-first-boot.service
> @@ -0,0 +1,16 @@
> +[Unit]
> +Description=Proxmox First Boot Setup
> +After=systemd-remount-fs.service
> +Before=network-pre.target
> +Wants=network-pre.target

I now I mentioned above ordering in our off-list chat, and it seems correct
for the usecase where one needs to configure networking itself here.
But, when summarizing our chat in the bug report, I re-read the use-cases
and saw that there might be also some users that require the first-boot
script to have the network available, e.g. to pull further automation stuff in.

So it really would be great to allow overriding that ordering.

Simplest way might be to leave it out here, or well go for the default we want
(in doubt -> dice roll), and write out a systemd unit snippet during installation
depending on a additional setting from the answer file.


> +ConditionPathExists=/var/lib/proxmox-first-boot/pending-first-boot-setup
> +ConditionPathIsReadWrite=/var/lib
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStart=/var/lib/proxmox-first-boot/proxmox-first-boot
> +ExecStartPost=/usr/bin/rm -v /var/lib/proxmox-first-boot/pending-first-boot-setup
> +
> +[Install]
> +WantedBy=network.target



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
> Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> > diff --git a/proxmox-first-boot/etc/proxmox-first-boot.service b/proxmox-first-boot/etc/proxmox-first-boot.service
> > new file mode 100644
> > index 0000000..046bb24
> > --- /dev/null
> > +++ b/proxmox-first-boot/etc/proxmox-first-boot.service
> > @@ -0,0 +1,16 @@
> > +[Unit]
> > +Description=Proxmox First Boot Setup
> > +After=systemd-remount-fs.service
> > +Before=network-pre.target
> > +Wants=network-pre.target
>
> I now I mentioned above ordering in our off-list chat, and it seems correct
> for the usecase where one needs to configure networking itself here.
> But, when summarizing our chat in the bug report, I re-read the use-cases
> and saw that there might be also some users that require the first-boot
> script to have the network available, e.g. to pull further automation stuff in.
>
> So it really would be great to allow overriding that ordering.

I see, so probably introduce a `first-boot.ordering` (or similar)
key, defaulting to "network-pre"?

Should it be an enum then? I.e. only allowing certain values such as
- network-pre.target
- network.target
- network-online.target
- multi-user.target

Further we could include {local,remote}-fs.target and maybe ceph.target?

(All available can be listed with `systemctl list-units --type target`,
for reference.)

Or just be a freeform text field and let the user decide entirely by
themselves?

If we allow configuring that though, we might need to change WantedBy=
depending on that too.

Not sure if we could just use multi-user.target as a default target, but
systemd *should* pull it in and run it in the right ordering too with
e.g. {Before,Wants}=network-pre.target ?

>
> Simplest way might be to leave it out here, or well go for the default we want
> (in doubt -> dice roll), and write out a systemd unit snippet during installation
> depending on a additional setting from the answer file.
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Thomas Lamprecht]

Am 15.11.24 um 10:34 schrieb Christoph Heiss:
> I see, so probably introduce a `first-boot.ordering` (or similar)
> key, defaulting to "network-pre"?
> 
> Should it be an enum then? I.e. only allowing certain values such as
> - network-pre.target
> - network.target
> - network-online.target
> - multi-user.target

Yeah, I would not make it generic, just the two or three most common
orderings, we can then extend it on potential future user demand.

I think before network, post network and finished boot, i.e. multi-user
target seem enough for now.

> Further we could include {local,remote}-fs.target and maybe ceph.target?

Maybe, but wouldn't do that for now, ceph.target on a freshly installed
node seems a bit odd, the fs targets might be sensible, but I would wait
for actual user demand.

> Or just be a freeform text field and let the user decide entirely by
> themselves?

No, that is harder to understand IMO than simple "needed for network
communication itself", "needs working network to run"  and "needs fully
booted system to run".

> If we allow configuring that though, we might need to change WantedBy=
> depending on that too.

Changing that might probably make sense.

> Not sure if we could just use multi-user.target as a default target, but
> systemd *should* pull it in and run it in the right ordering too with
> e.g. {Before,Wants}=network-pre.target ?

Isn't the WantedBy is more for defining the target the unit itself will
be part of, or? Adapting that might indeed make sense, but a bit to long
ago that I looked into systemd unit ordering/dependency semantics more
closely.



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
> [..]
> So it really would be great to allow overriding that ordering.
>
> Simplest way might be to leave it out here, or well go for the default we want
> (in doubt -> dice roll), and write out a systemd unit snippet during installation
> depending on a additional setting from the answer file.

Thinking about this again, while implementing - if we need to customize
the unit file, do we want to:

- create an `override.conf` file in
  `/etc/systemd/system/proxmox-first-boot.service.d/`, like systemd
  would do it when using `systemctl edit <service>`?
  Disadvantage is, that that file isn't removed when removing the
  `proxmox-first-boot` package from the system (although we could do it
  via postrm maybe?)

- edit the unit file directly? That would mean that e.g. `debsums -c`
  would complain.

- even bother with a separate package and instead just write the unit
  file directly to /etc/systemd/system?
  Disadvantage here is that it would be a "lost" file, not managed by
  dpkg, and administrators would have to remove the file directly, if
  they want/need to.

I have implemented the first variant for now, for testing. But not sure
if it's the best way to go.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Thomas Lamprecht]

Am 15.11.24 um 14:34 schrieb Christoph Heiss:
> On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
>> [..]
>> So it really would be great to allow overriding that ordering.
>>
>> Simplest way might be to leave it out here, or well go for the default we want
>> (in doubt -> dice roll), and write out a systemd unit snippet during installation
>> depending on a additional setting from the answer file.
> 
> Thinking about this again, while implementing - if we need to customize
> the unit file, do we want to:
> 
> - create an `override.conf` file in
>   `/etc/systemd/system/proxmox-first-boot.service.d/`, like systemd
>   would do it when using `systemctl edit <service>`?
>   Disadvantage is, that that file isn't removed when removing the
>   `proxmox-first-boot` package from the system (although we could do it
>   via postrm maybe?)

I'd either use above or as an additional alternative: ship the different
variants as separate complete unit files with a common Alias (to convey
that they're the "same" thing) and enable only the one (manually) that is
configured. That would keep every file fully covered by the package
system.

> 
> - edit the unit file directly? That would mean that e.g. `debsums -c`
>   would complain.
> 
> - even bother with a separate package and instead just write the unit
>   file directly to /etc/systemd/system?
>   Disadvantage here is that it would be a "lost" file, not managed by
>   dpkg, and administrators would have to remove the file directly, if
>   they want/need to.
> 
> I have implemented the first variant for now, for testing. But not sure
> if it's the best way to go.
> 

Fine by me, but maybe you find some benefits on the separate unit file
one, not to hard feelings here.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 2/5] fix #5579: first-boot: add initial service packaging

[Christoph Heiss]

On Fri, Nov 15, 2024 at 02:39:16PM +0100, Thomas Lamprecht wrote:
> Am 15.11.24 um 14:34 schrieb Christoph Heiss:
> > On Thu, Nov 14, 2024 at 09:23:48PM +0100, Thomas Lamprecht wrote:
> >> [..]
> >> So it really would be great to allow overriding that ordering.
> >>
> >> Simplest way might be to leave it out here, or well go for the default we want
> >> (in doubt -> dice roll), and write out a systemd unit snippet during installation
> >> depending on a additional setting from the answer file.
> >
> > Thinking about this again, while implementing - if we need to customize
> > the unit file, do we want to:
> >
> > - create an `override.conf` file in
> >   `/etc/systemd/system/proxmox-first-boot.service.d/`, like systemd
> >   would do it when using `systemctl edit <service>`?
> >   Disadvantage is, that that file isn't removed when removing the
> >   `proxmox-first-boot` package from the system (although we could do it
> >   via postrm maybe?)
>
> I'd either use above or as an additional alternative: ship the different
> variants as separate complete unit files with a common Alias (to convey
> that they're the "same" thing) and enable only the one (manually) that is
> configured. That would keep every file fully covered by the package
> system.

Didn't think of that variant, good idea! Thanks!
Should really cover all the cases I think and leave no files unmanaged.

(Also, TIL about Alias=)


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC PATCH installer 3/5] fix #5579: auto-install-assistant: enable baking in first-boot script

[Christoph Heiss]

Adds a new parameter `--on-first-boot` to the `prepare-iso` command, to
specify a file to bake into the ISO.

To later use it with the auto-installer, the following must be set in
the answer file:

  [first-boot]
  source = "from-iso"

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-auto-install-assistant/Cargo.toml  |  1 +
 proxmox-auto-install-assistant/src/main.rs | 17 +++++++++++++++++
 proxmox-installer-common/src/lib.rs        |  3 +++
 3 files changed, 21 insertions(+)

diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml
index c4486f8..07e6ffb 100644
--- a/proxmox-auto-install-assistant/Cargo.toml
+++ b/proxmox-auto-install-assistant/Cargo.toml
@@ -13,6 +13,7 @@ homepage = "https://www.proxmox.com"
 [dependencies]
 anyhow.workspace = true
 log.workspace = true
+proxmox-installer-common.workspace = true
 proxmox-auto-installer.workspace = true
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index bc7d5d8..2b9b736 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -20,6 +20,7 @@ use proxmox_auto_installer::{
         FetchAnswerFrom, HttpOptions,
     },
 };
+use proxmox_installer_common::FIRST_BOOT_EXEC_NAME;
 
 static PROXMOX_ISO_FLAG: &str = "/auto-installer-capable";
 
@@ -150,6 +151,13 @@ struct CommandPrepareISO {
     // so shorten "Automated Installer Source" to "AIS" to be safe.
     #[arg(long, default_value_t = { "proxmox-ais".to_owned() } )]
     partition_label: String,
+
+    /// Executable file to include, which should be run on the first system boot after the
+    /// installation. Can be used for further bootstrapping the new system.
+    ///
+    /// Must be appropriately enabled in the answer file.
+    #[arg(long)]
+    on_first_boot: Option<PathBuf>,
 }
 
 /// Show the system information that can be used to identify a host.
@@ -353,6 +361,15 @@ fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
         inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml", &uuid)?;
     }
 
+    if let Some(first_boot) = &args.on_first_boot {
+        inject_file_to_iso(
+            &tmp_iso,
+            first_boot,
+            &format!("/{FIRST_BOOT_EXEC_NAME}"),
+            &uuid,
+        )?;
+    }
+
     println!("Moving prepared ISO to target location...");
     fs::rename(&tmp_iso, &iso_target)?;
     println!("Final ISO is available at {iso_target:?}.");
diff --git a/proxmox-installer-common/src/lib.rs b/proxmox-installer-common/src/lib.rs
index 10b5940..c25f105 100644
--- a/proxmox-installer-common/src/lib.rs
+++ b/proxmox-installer-common/src/lib.rs
@@ -11,3 +11,6 @@ pub const RUNTIME_DIR: &str = "/run/proxmox-installer";
 
 /// Default placeholder value for the administrator email address.
 pub const EMAIL_DEFAULT_PLACEHOLDER: &str = "mail@example.invalid";
+
+/// Name of the executable for the first-boot hook.
+pub const FIRST_BOOT_EXEC_NAME: &str = "proxmox-first-boot";
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC PATCH installer 4/5] fix #5579: auto-installer: add optional first-boot hook script

[Christoph Heiss]

Users can specifying an optional file - either fetched from an URL or
backed into the ISO - to execute on the first boot after the
installation, using the 'proxmox-first-boot' oneshot service.

Essentially adds an (optional) `[first-boot]` section to the answer
file. If specified, the `source` key must be at least set, which gives
the location of the hook script.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-auto-installer/Cargo.toml             |  2 +-
 proxmox-auto-installer/src/answer.rs          | 27 ++++++++++++
 .../src/bin/proxmox-auto-installer.rs         | 42 +++++++++++++++++--
 proxmox-auto-installer/src/utils.rs           | 15 ++++++-
 4 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/proxmox-auto-installer/Cargo.toml b/proxmox-auto-installer/Cargo.toml
index 21ed538..7e3d90c 100644
--- a/proxmox-auto-installer/Cargo.toml
+++ b/proxmox-auto-installer/Cargo.toml
@@ -13,7 +13,7 @@ homepage = "https://www.proxmox.com"
 [dependencies]
 anyhow.workspace = true
 log.workspace = true
-proxmox-installer-common.workspace = true
+proxmox-installer-common = { workspace = true, features = ["http"] }
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
 serde_plain.workspace = true
diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
index c23f1f3..23d6878 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -22,6 +22,8 @@ pub struct Answer {
     pub disks: Disks,
     #[serde(default)]
     pub posthook: Option<PostNotificationHookInfo>,
+    #[serde(default)]
+    pub first_boot: Option<FirstBootHookInfo>,
 }
 
 impl Answer {
@@ -62,6 +64,31 @@ pub struct PostNotificationHookInfo {
     pub cert_fingerprint: Option<String>,
 }
 
+/// Possible sources for the optional first-boot hook script/executable file.
+#[derive(Clone, Deserialize, Debug, PartialEq)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub enum FirstBootHookSourceMode {
+    /// Fetch the executable file from an URL, specified in the parent.
+    FromUrl,
+    /// The executable file has been baked into the ISO at a known location,
+    /// and should be retrieved from there.
+    FromIso,
+}
+
+/// Describes from where to fetch the first-boot hook script, either being baked into the ISO or
+/// from a URL.
+#[derive(Clone, Deserialize, Debug)]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+pub struct FirstBootHookInfo {
+    /// Mode how to retrieve the first-boot executable file, either from an URL or from the ISO if
+    /// it has been baked-in.
+    pub source: FirstBootHookSourceMode,
+    /// Retrieve the post-install script from a URL, if source == "from-url".
+    pub url: Option<String>,
+    /// SHA256 cert fingerprint if certificate pinning should be used, if source == "from-url".
+    pub cert_fingerprint: Option<String>,
+}
+
 #[derive(Clone, Deserialize, Debug, Default, PartialEq)]
 #[serde(deny_unknown_fields)]
 enum NetworkConfigMode {
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index ea45c29..9d8a2e5 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -1,18 +1,22 @@
 use anyhow::{bail, format_err, Result};
 use log::{error, info, LevelFilter};
 use std::{
-    env,
+    env, fs,
     io::{BufRead, BufReader, Write},
     path::PathBuf,
     process::ExitCode,
 };
 
-use proxmox_installer_common::setup::{
-    installer_setup, read_json, spawn_low_level_installer, LocaleInfo, RuntimeInfo, SetupInfo,
+use proxmox_installer_common::{
+    http,
+    setup::{
+        installer_setup, read_json, spawn_low_level_installer, LocaleInfo, RuntimeInfo, SetupInfo,
+    },
+    FIRST_BOOT_EXEC_NAME, RUNTIME_DIR,
 };
 
 use proxmox_auto_installer::{
-    answer::Answer,
+    answer::{Answer, FirstBootHookInfo, FirstBootSourceMode},
     log::AutoInstLogger,
     udevinfo::UdevInfo,
     utils::{parse_answer, LowLevelMessage},
@@ -27,6 +31,31 @@ pub fn init_log() -> Result<()> {
         .map_err(|err| format_err!(err))
 }
 
+fn setup_first_boot_executable(first_boot: &FirstBootHookInfo) -> Result<()> {
+    let content = match first_boot.source {
+        FirstBootSourceMode::FromUrl => {
+            if let Some(url) = &first_boot.url {
+                info!("Fetching first-boot hook from {url} ..");
+                Some(http::get(url, first_boot.cert_fingerprint.as_deref())?)
+            } else {
+                bail!("first-boot hook source set to URL, but none specified!");
+            }
+        }
+        FirstBootSourceMode::FromIso => Some(fs::read_to_string(format!(
+            "/cdrom/{FIRST_BOOT_EXEC_NAME}"
+        ))?),
+    };
+
+    if let Some(content) = content {
+        Ok(fs::write(
+            format!("/{RUNTIME_DIR}/{FIRST_BOOT_EXEC_NAME}"),
+            content,
+        )?)
+    } else {
+        Ok(())
+    }
+}
+
 fn auto_installer_setup(in_test_mode: bool) -> Result<(Answer, UdevInfo)> {
     let base_path = if in_test_mode { "./testdir" } else { "/" };
     let mut path = PathBuf::from(base_path);
@@ -43,6 +72,11 @@ fn auto_installer_setup(in_test_mode: bool) -> Result<(Answer, UdevInfo)> {
     };
 
     let answer = Answer::try_from_reader(std::io::stdin().lock())?;
+
+    if let Some(first_boot) = &answer.first_boot {
+        setup_first_boot_executable(first_boot)?;
+    }
+
     Ok((answer, udev_info))
 }
 
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 83f3f12..25f537d 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -5,7 +5,7 @@ use log::info;
 use std::{collections::BTreeMap, process::Command};
 
 use crate::{
-    answer::{self, Answer},
+    answer::{self, Answer, FirstBootSourceMode},
     udevinfo::UdevInfo,
 };
 use proxmox_installer_common::{
@@ -320,6 +320,18 @@ fn verify_email_and_root_password_settings(answer: &Answer) -> Result<()> {
     }
 }
 
+fn verify_first_boot_settings(answer: &Answer) -> Result<()> {
+    info!("Verifying first boot settings");
+
+    if let Some(first_boot) = &answer.first_boot {
+        if first_boot.source == FirstBootSourceMode::FromUrl && first_boot.url.is_none() {
+            bail!("first-boot executable source set to URL, but none specified!");
+        }
+    }
+
+    Ok(())
+}
+
 pub fn parse_answer(
     answer: &Answer,
     udev_info: &UdevInfo,
@@ -336,6 +348,7 @@ pub fn parse_answer(
 
     verify_locale_settings(answer, locales)?;
     verify_email_and_root_password_settings(answer)?;
+    verify_first_boot_settings(answer)?;
 
     let mut config = InstallConfig {
         autoreboot: 1_usize,
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 4/5] fix #5579: auto-installer: add optional first-boot hook script

[Thomas Lamprecht]

Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> Users can specifying an optional file - either fetched from an URL or
> backed into the ISO - to execute on the first boot after the
> installation, using the 'proxmox-first-boot' oneshot service.
> 
> Essentially adds an (optional) `[first-boot]` section to the answer
> file. If specified, the `source` key must be at least set, which gives
> the location of the hook script.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  proxmox-auto-installer/Cargo.toml             |  2 +-
>  proxmox-auto-installer/src/answer.rs          | 27 ++++++++++++
>  .../src/bin/proxmox-auto-installer.rs         | 42 +++++++++++++++++--
>  proxmox-auto-installer/src/utils.rs           | 15 ++++++-
>  4 files changed, 80 insertions(+), 6 deletions(-)
> 

> +fn setup_first_boot_executable(first_boot: &FirstBootHookInfo) -> Result<()> {
> +    let content = match first_boot.source {
> +        FirstBootSourceMode::FromUrl => {
> +            if let Some(url) = &first_boot.url {
> +                info!("Fetching first-boot hook from {url} ..");
> +                Some(http::get(url, first_boot.cert_fingerprint.as_deref())?)
> +            } else {
> +                bail!("first-boot hook source set to URL, but none specified!");
> +            }
> +        }


I'd sleep slightly better if we size limit this to something around 1 MiB, or
at max 10 MiB if one really wants to allow a lot of convenience.
In that amount of space one can fit far more than enough stuff to bootstrap
oneself.

Same for when embedding this into the ISO for consistency.

Tangentially related: do we already support sending along some sort of
Authorization header? Definitively not a blocker for this, but if we do not
have that already it could be great to add for some basic form of authentication
so that one can limit their fetch-answer/post-hook server to not answer setup
details or, even worse, secrets to any unauthenticated client.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 4/5] fix #5579: auto-installer: add optional first-boot hook script

[Christoph Heiss]

On Thu, Nov 14, 2024 at 09:33:31PM +0100, Thomas Lamprecht wrote:
> Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> > [..]
>
> I'd sleep slightly better if we size limit this to something around 1 MiB, or
> at max 10 MiB if one really wants to allow a lot of convenience.
> In that amount of space one can fit far more than enough stuff to bootstrap
> oneself.

I think 1 MiB is quite a reasonable size limit for a bootstrapping
script. I'll implement it.

>
> Same for when embedding this into the ISO for consistency.
>
> Tangentially related: do we already support sending along some sort of
> Authorization header? Definitively not a blocker for this, but if we do not
> have that already it could be great to add for some basic form of authentication
> so that one can limit their fetch-answer/post-hook server to not answer setup
> details or, even worse, secrets to any unauthenticated client.

Not yet, but makes sense to implement! Will do it as a separate
patch/series thought.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [RFC PATCH installer 4/5] fix #5579: auto-installer: add optional first-boot hook script

[Thomas Lamprecht]

Am 13.11.24 um 14:59 schrieb Christoph Heiss:
> +/// Possible sources for the optional first-boot hook script/executable file.
> +#[derive(Clone, Deserialize, Debug, PartialEq)]
> +#[serde(rename_all = "kebab-case", deny_unknown_fields)]
> +pub enum FirstBootHookSourceMode {
> +    /// Fetch the executable file from an URL, specified in the parent.
> +    FromUrl,
> +    /// The executable file has been baked into the ISO at a known location,
> +    /// and should be retrieved from there.
> +    FromIso,
> +}

FWIW and probably for a separate series after this got merged but a third source
could be the post-installation-webhook itself. That's mostly equivalent with
during the fetch-answer phase, but at that stage one can be sure that installation
was completed successfully and thus some, e.g. network resource allocation,
might be better if done only once that is clear.

But I'm totally fine with waiting on user feedback on this one, it's probably
not the most obvious source.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [RFC PATCH installer 5/5] fix #5579: install: copy over `proxmox-first-boot` script if present

[Christoph Heiss]

The auto-installer will place an executable file named
`proxmox-first-boot` in the installer runtime-directory if the user set
up.

Based on the presence of this file, we copy it over to the target system
and set a flag file, indicating to the 'proxmox-first-boot' service that
it is indeed the very first boot of the new system and should run.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Proxmox/Install.pm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
index 5c64c3d..f46d86a 100644
--- a/Proxmox/Install.pm
+++ b/Proxmox/Install.pm
@@ -681,6 +681,7 @@ sub extract_data {
 
     my $proxmox_libdir = $iso_env->{locations}->{lib};
     my $proxmox_cddir = $iso_env->{locations}->{iso};
+    my $proxmox_rundir = $iso_env->{locations}->{run};
     my $proxmox_pkgdir = "${proxmox_cddir}/proxmox/packages/";
 
     my $targetdir = is_test_mode() ? "target" : "/target";
@@ -1241,6 +1242,25 @@ _EOD
 	    debconfig_set($targetdir, "pve-manager pve-manager/country string $ucc\n");
 	}
 
+	my $firstboot_exec_name = 'proxmox-first-boot';
+	if (-f "$proxmox_rundir/$firstboot_exec_name") {
+	    my $firstboot_pending_flagfile = "pending-first-boot-setup";
+	    my $targetpath = "$targetdir/var/lib/proxmox-first-boot";
+
+	    syscmd("mkdir -p $targetpath/") == 0
+		|| die "failed to create $targetpath directory\n";
+
+	    syscmd("cp $proxmox_rundir/$firstboot_exec_name $targetpath/") == 0
+		|| die "unable to copy $firstboot_exec_name executable\n";
+	    syscmd("touch $targetpath/$firstboot_pending_flagfile") == 0
+		|| die "unable to create $firstboot_pending_flagfile flag file\n";
+
+	    # Explicitly mark the entire directory only accessible, to prevent
+	    # possible secret leaks from the bootstrap script.
+	    syscmd("chmod -R 0700 $targetpath") == 0
+		|| warn "failed to set permissions for $targetpath\n";
+	}
+
 	update_progress(0.8, 0.95, 1, "make system bootable");
 	my $target_cmdline='';
 	if ($target_cmdline = Proxmox::Install::Config::get_target_cmdline()) {
-- 
2.47.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel