auc: preserve image's uci-defaults upon upgrade

[bam80]

uci-defaults scripts are essential for accessing device after firmware reset.

The device or user-facing PC might have no (easy accessible) Ethernet ports, or wired LAN might not be accessible for any other reason. In this case, uci-defaults script can contain code to setup WiFi AP on the very first boot, thus granting the access. Such uci-defaults script should survive the firmware upgrade with AUC, if it was in the firmware image initially. Otherwise, access to the device can be lost next time user performs firmware Reset.

This patch fixes it by passing the file /rom/etc/uci-defaults/99-asu-defaults (the path used by ASU server) for inclusion to a new firmware image using BuildRequest API's "defaults" member which allows specifying a content of that uci-defaults script: https://sysupgrade.openwrt.org/ui/

fixes https://github.com/openwrt/asu/issues/575

@hauke @ynezz @aparcar

[bam80]

Thanks for review Daniel!

[bam80]

@dangowrt could we proceed please?

[bam80]

Ping please?

[bam80]

@dangowrt ping please?

[bam80]

@efahl thank you for the review, could you merge it?

[efahl]

Not me, I'm just another person who submits PRs like you, without commit privileges.

[efahl]

@bam80, maybe you can get some ideas here to rewrite the commit message:

My use case is to define port mappings on my generic devices (x86), so I go to https://firmware-selector.openwrt.org/, add a script that does something like

lan="eth1 eth2 eth3"
wan="eth0"
ucidef_set_interfaces_lan_wan "$lan" "$wan"

If I do auc, that gets wiped out on upgrade, so I have to manually run the selector, paste that in again, download and so on... Would be very much more convenient if auc supported that defaults field in the asu server's API.

I do see others using the defaults mechanism to do file system resizing using the expand root script, here's a recent example: https://forum.openwrt.org/t/resize-emmc-partition-and-attended-sysupgrade/181712/4

[efahl]

How about?

Achieve feature parity with firmware selector, which allows inclusion of first boot uci-defaults files without rebuilding from scratch.

And now that I'm really thinking about it, it would be even nicer to expose the filename in a command line option, so that users could add or modify the existing 99-asu-defaults. Maybe something like this, which would read contents from the specified file:

auc -d ./my-uci-defaults ...

[bam80]

I think my commit message is good enough, if @dangowrt doesn't think so - I can make further adjustments.

Regarding the path, 99-asu-defaults is hard-coded here: https://github.com/openwrt/asu/blob/3ef4757b9fcc401b3d18ebb6954276b0210f07fb/asu/build.py#L202 that is why it's essential to preserve exactly that path, no need to complicate this here. Other expanded functionality beyond that could go to a separate PR.

[efahl]

I agree your commit message is fine, our paths crossed, I didn't see your new commit until after I had posted.

I think you misunderstand my -d file option. It would not replace 99-asu-defaults in the generated .img, but rather use a different source to read into the defaults blob when sending the request to the ASU build API. This would allow you to modify or even create 99-asu-defaults in the image without ever using the firmware selector in the first place.

But, yes, I agree, let's not complicate the issue now, leave things as-is and get the commit done then think about enhancements later.

[bam80]

Merry Christmas!

I would merge this once the commit description gives some information about what is being done and why and how one can use that feature and what it is typically used for.

@dangowrt would you make a present for us? :)

[bam80]

ping, please?

[bam80]

Please explain the use-case of this new feature. I can imagine it, but it should be written in the commit description

@dangowrt the review has been addressed. Could we progress please?

[bam80]

@dangowrt ping

[rmilecki]

I'm not any real maintainer of any project but I have a pretty solid experience of contributing to various open source projects and subsystems. Please let me share one or two thought.

If you want to get your changes reviewed/accepted smoothly it's important to describe them properly. I don't think it's easy task. Your perspective it quite different from other developers as you worked closely on some technical detail. It takes some thinking what and how to describe.

In this case I think it's safe to assume that reviewers/developers/maintainers know what ASU is. You should however describe better what API part you work with, what is 99-asu-defaults and what your change affects.

I think your commit should:

1. Describe what is 99-asu-defaults
2. Mention what API feature you use
3. Explain how it affect images
4. Describe final result

It took me half an hour to get familiar with API, defaults and what 99-asu-defaults is about.

I would go with something like:

auc: build images with a copy of local uci-defaults 99-asu-defaults file

Images generated using ASU server may contain user specific
customizations handled with a custom 99-asu-defaults script. Those
changes may be critical for given use case and device access may depend
on them.

It's important to re-apply those changes after Attended SysUpgrade. To
achieve that generate a new image image with a copy of local
99-asu-defaults included. This is handled using API's "defaults"
parameter which allows specifying a content of that uci-defaults script.

This may be not accurate (I'm not 100% sure I got idea right) but should be a good hint. I think it explains better the purpose of changes.

[rmilecki]

Now, getting to the point of this pull request. I've one basic question: wouldn't that be much simpler to simply preserve local 99-asu-defaults as a part of sysupgrade process? It may be non-trivial as you need to access /rom/etc/uci-defualts/99-asu-defaults (as /etc/uci-defualts/99-asu-defaults most likely doesn't exist anymore) but it should not require a custom image.

FWIW I'm working on two changes:

• [PATCH RFC] base-files: sysupgrade: always setup overlay when creating backup
• [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

that I think may make such feature much easier to implement.

[bam80]

If you want to get your changes reviewed/accepted smoothly it's important to describe them properly.

This was addressed once. I'm not against to put even more work on this if needed but on this stage I need confirmation from the reviewers they still follow, as the patch seem just doesn't get their attention any more.

wouldn't that be much simpler to simply preserve local 99-asu-defaults as a part of sysupgrade process? It may be non-trivial as you need to access /rom/etc/uci-defualts/99-asu-defaults (as /etc/uci-defualts/99-asu-defaults most likely doesn't exist anymore) but it should not require a custom image.

We need to keep this file in /rom, as otherwise it will be wiped on nearest factory reset. Thus, a custom image is inevitable.

[rmilecki]

Thanks for explaining @bam80 as I didn't initially understand it was about sysupgrade followed by factory reset. You seem to be correct here, we need sysupgrade to image with custom uci-defaults included in its rootfs (squashfs). My sysupgrade changes won't be of any use here.

[bam80]

@rmilecki What are your PRs, can you invite your reviewers here too?

[rmilecki]

Those mentioned in my comment on 17.02. Just pushed to the master:

• 4fa9aaf0bed9 ("base-files: sysupgrade: always setup overlay when creating backup")
• bf304d10e97c ("base-files: sysupgrade: include uci-defaults script disabling services")

Unrelated to your auc changes I believe.

[efahl]

My sysupgrade changes won't be of any use here.

Ok, so is it an order-of-events issue? It seemed to me that your first patch series would allow one to add /rom/etc/uci-default/blah to /etc/sysupgrade.conf (or some alternate mechanism allowed by your tooling), thus pushing the auc issue here down into sysupgrade.

(Of course, none of this addresses the missing uci-defaults issue with ext4; we can only avoid that by actually supplying an external script as I suggested on my 2023-12-23 comment.)

Thanks for looking at this!

[rmilecki]

We could make /rom/etc/uci-default/99-asu-defaults back of backup which would result in restoring that file at /etc/uci-default/99-asu-defaults path in sysupgraded image. That would NOT result in that file being part of RO squashfs partition. After factory reset there would be no sign of 99-asu-defaults. That is why I don't think my sysupgrade work can help here.

[efahl]

I'm still trying to get all the pieces into my head, as there are a lot of different moving parts here. The reason I keep thinking this is best solved at the sysupgrade level is because that is the common point for both auc and LuCI Attended Sysupgrade, both of which suffer from the issue that @bam80 is trying to solve here. Fixing it in auc still leaves the bug in LuCI ASU.

And then there's the issue with ext4, since there's no /rom partition at all from which to carry over any custom uci-defaults script, so it seems to me that sysupgrade would again be the proper place to detect this (maybe just before it does first boot, stashing the custom scripts somewhere???) allowing it to restore them at the next sysupgrade???

And then there is the issue of carrying over other custom uci-defaults scripts that may not be using the ASU server's convention of 99-asu-defaults (maybe you used a full build system or image builder for that initial installation instead of Firmware Selector)...

[bam80]

@efahl no need to put all the issues into the one bin, that would just make things more complicated. All off those you mentioned are not directly related with this one, and should be addressed separately.

Let's please keep this discussion dedicated to the issue here. If you care, you can open related issues for each of them you mentioned, with the reference link to this one. Thanks.

[bam80]

Those mentioned in my comment on 17.02. Just pushed to the master:

@rmilecki I see those commits were without PRs, does it mean you have the merge rights? We could tweak the commit message if you want then, and get things done.

[bam80]

I would go with something like:

@rmilecki I adopted your suggestion for the description, please let me know if you see it satisfiable, I'll transfer it to the commit message then. Thanks.

[rmilecki]

@efahl: quick summary:

1. We need images to contain 99-asu-defaults in their RO filesystem (squashfs)
2. There is no way for sysupgrade to modify downloaded image's RO filesystem (squashfs) and append 99-asu-defaults to it
3. The only solution is to have images generated with 99-asu-defaults already included which this Pull Request takes care of

@bam80: I mean to take care of this Pull Request, just give me few days to find time for it

[efahl]

no need to put all the issues into the one bin

On the contrary, there really is only one issue, namely "how do we maintain custom uci-defaults across sysupgrades." If there is a general solution to this in the infrastructure layer (i.e., sysupgrade) then various hacks to auc and LuCI Attended Sysupgrade are not needed.

After looking into how all of sysupgrade, firmware selector, LuCI ASU, auc, ASU server and the downloads site interact, I don't think this very specific patch to auc is appropriate, I think it should be addressed where all current and future clients of sysupgrade will benefit.

[rmilecki]

I can't imagine modifying squashfs of downloaded firmware to append uci-defaults script (99-asu-defaults) to it. There are many firmware formats: squashfs can be MTD partition or UBI volume to start with. You would need to extract squashfs, then unpack it, modify and repack. All of that on a target device with limited tools and memory. I don't see that happening.

Now I'm thinking about it there is probably another solution: preserving files during factory reset. The more I think of it the more sense it makes. It may be a bit counter-intuitive but I don't see why we couldn't keep a list of critical files and preserve them.

[efahl]

Now I'm thinking about it there is probably another solution: preserving files during factory reset. The more I think of it the more sense it makes. It may be a bit counter-intuitive but I don't see why we couldn't keep a list of critical files and preserve them.

Yes, this is the same route I've been thinking along. If there were an alternate mechanism that did something like 1) stashing these files somewhere rather than embedding the files in the .img; or 2) augmenting sysupgrade -b with maybe added syntax in /etc/sysupgrade.conf, then I think the clients could remove code and in general just work better.

On obvious advantage is that for ext4, this PR simply doesn't work, so something else needs to be done. (Simple test case: use firmware selector to build an ext4 image with a custom uci-defaults file; install that, then run auc with this PR's patch, see that your custom uci-defaults are silently gone.)