Message ID | 20191108120520.1966-1-freifunk@adrianschmutzler.de |
---|---|
State | Rejected |
Delegated to: | Adrian Schmutzler |
Headers | show |
Series | [OpenWrt-Devel,1/2] base-files: indicate initial setup by uci system config option | expand |
Hi Adrian, On 08.11.2019 13:05, Adrian Schmutzler wrote: > This provides a uci system config setting that will be set only > during initial setup. This can be used by uci-defaults script to > determine whether they are run during initial setup or after a > sysupgrade. > > Since the setting is removed again after uci-defaults have been > processed, it won't be recognized by the user on the running device, > but can be exploited also for downstream setup tasks. This looks for me like a misuse of uci configuration and some kind of workaround for a missing feature, maybe in procd/ubus? NAK on this one from me.
Hi Piotr, > -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Piotr Dymacz > Sent: Samstag, 16. November 2019 16:50 > To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; openwrt- > devel@lists.openwrt.org > Subject: Re: [OpenWrt-Devel] [PATCH 1/2] base-files: indicate initial setup by > uci system config option > > Hi Adrian, > > On 08.11.2019 13:05, Adrian Schmutzler wrote: > > This provides a uci system config setting that will be set only during > > initial setup. This can be used by uci-defaults script to determine > > whether they are run during initial setup or after a sysupgrade. > > > > Since the setting is removed again after uci-defaults have been > > processed, it won't be recognized by the user on the running device, > > but can be exploited also for downstream setup tasks. > > This looks for me like a misuse of uci configuration and some kind of > workaround for a missing feature, maybe in procd/ubus? Maybe I'm not familiar enough with it, but I currently can't think of a method indicating a fresh installation with procd/ubus. > > NAK on this one from me. Similar to the label MAC hostname/SSID stuff, this was just a nice-to-have thing for me. I will mark both patches as rejected. Best Adrian > > -- > Cheers, > Piotr > > > > > Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de> > > --- > > package/base-files/files/bin/config_generate | 1 + > > .../base-files/files/etc/uci-defaults/90_end-initial-setup | 4 ++++ > > 2 files changed, 5 insertions(+) > > create mode 100644 > > package/base-files/files/etc/uci-defaults/90_end-initial-setup > > > > diff --git a/package/base-files/files/bin/config_generate > > b/package/base-files/files/bin/config_generate > > index b473eba9e9..273561229a 100755 > > --- a/package/base-files/files/bin/config_generate > > +++ b/package/base-files/files/bin/config_generate > > @@ -243,6 +243,7 @@ generate_static_system() { > > set system.@system[-1].ttylogin='0' > > set system.@system[-1].log_size='64' > > set system.@system[-1].urandom_seed='0' > > + set system.@system[-1].initial_setup='1' > > > > delete system.ntp > > set system.ntp='timeserver' > > diff --git > > a/package/base-files/files/etc/uci-defaults/90_end-initial-setup > > b/package/base-files/files/etc/uci-defaults/90_end-initial-setup > > new file mode 100644 > > index 0000000000..779d858d5f > > --- /dev/null > > +++ b/package/base-files/files/etc/uci-defaults/90_end-initial-setup > > @@ -0,0 +1,4 @@ > > +uci -q delete system.@system[0].initial_setup uci commit system > > + > > +exit 0 > > > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Hi Adrian, On 17.11.2019 00:36, mail@adrianschmutzler.de wrote: > Hi Piotr, > >> -----Original Message----- >> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] >> On Behalf Of Piotr Dymacz >> Sent: Samstag, 16. November 2019 16:50 >> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; openwrt- >> devel@lists.openwrt.org >> Subject: Re: [OpenWrt-Devel] [PATCH 1/2] base-files: indicate initial setup by >> uci system config option >> >> Hi Adrian, >> >> On 08.11.2019 13:05, Adrian Schmutzler wrote: >> > This provides a uci system config setting that will be set only during >> > initial setup. This can be used by uci-defaults script to determine >> > whether they are run during initial setup or after a sysupgrade. >> > >> > Since the setting is removed again after uci-defaults have been >> > processed, it won't be recognized by the user on the running device, >> > but can be exploited also for downstream setup tasks. >> >> This looks for me like a misuse of uci configuration and some kind of >> workaround for a missing feature, maybe in procd/ubus? > > Maybe I'm not familiar enough with it, but I currently can't think of a method indicating a fresh installation with procd/ubus. I just don't think uci is the right way for this job. It's used for configuration, not for storing and clearing some flags about system state. Your way of using uci for that could introduce false positive results. I would look at preinit stage, maybe checking if the writable filesystem (rootfs_data) is created/ready could be used for detecting the 'initial setup' state (I'm just not sure if that could work across all targets). Either way, maybe an env variable, like we have for initramfs images (INITRAMFS set to 1), would be better?
Adrian Schmutzler <freifunk@adrianschmutzler.de> [2019-11-08 13:05:19]: > This provides a uci system config setting that will be set only > during initial setup. This can be used by uci-defaults script to > determine whether they are run during initial setup or after a > sysupgrade. I use similar stuff in my config scripts, just differently. I do `set system.@system[-1].firstboot_done='1'` in order to know, that the system was already first time booted. So if the user performs sysupgrade but prefers to keep settings, I don't run uci-default scripts which performs one time setup stuff, like setting hostname, generating unique auth token etc. basically it boils down to `is_firstboot || exit 0` in those one time uci-default scripts. -- ynezz
diff --git a/package/base-files/files/bin/config_generate b/package/base-files/files/bin/config_generate index b473eba9e9..273561229a 100755 --- a/package/base-files/files/bin/config_generate +++ b/package/base-files/files/bin/config_generate @@ -243,6 +243,7 @@ generate_static_system() { set system.@system[-1].ttylogin='0' set system.@system[-1].log_size='64' set system.@system[-1].urandom_seed='0' + set system.@system[-1].initial_setup='1' delete system.ntp set system.ntp='timeserver' diff --git a/package/base-files/files/etc/uci-defaults/90_end-initial-setup b/package/base-files/files/etc/uci-defaults/90_end-initial-setup new file mode 100644 index 0000000000..779d858d5f --- /dev/null +++ b/package/base-files/files/etc/uci-defaults/90_end-initial-setup @@ -0,0 +1,4 @@ +uci -q delete system.@system[0].initial_setup +uci commit system + +exit 0
This provides a uci system config setting that will be set only during initial setup. This can be used by uci-defaults script to determine whether they are run during initial setup or after a sysupgrade. Since the setting is removed again after uci-defaults have been processed, it won't be recognized by the user on the running device, but can be exploited also for downstream setup tasks. Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de> --- package/base-files/files/bin/config_generate | 1 + .../base-files/files/etc/uci-defaults/90_end-initial-setup | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 package/base-files/files/etc/uci-defaults/90_end-initial-setup