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.

Sign up for GitHub

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

Jump to bottom

PR to support modular interface? #22

Open
tomeon opened this issue Jan 22, 2023 · 7 comments · May be fixed by #48
Open

PR to support modular interface? #22

tomeon opened this issue Jan 22, 2023 · 7 comments · May be fixed by #48

Comments

@tomeon
Copy link

tomeon commented Jan 22, 2023

Hello, and thanks for this neat and useful toolchain. I'm trying to do a better job of managing my OpenWRT-based router and APs, and as a confirmed Nixaholic this repository is just what I needed.

Would you be interested in a PR introducing a modular interface to the OpenWRT image builder code? I have a (very) rough cut at such an interface up on SourceHut here.

The high-level view:

{
  openwrtImages.default = self.lib.openwrtImage {
    modules = [
      {
        image = {
          profileName = "avm_fritz7412";
          packages.include = ["tcpdump"];
          packages.exclude = ["ppp"];
          disabledServices = ["dnsmasq"];
          snippets."etc/uci-defaults/99-custom".text = ''
            uci -q batch << EOI
            set system.@system[0].hostname='testap'
            commit
            EOI
          '';
        };
      }
    ];
  };
}

If specified as a flake output, the image and associated assets could then be built with nix build '.#openwrtImages.default.config.system.build.image' . I plan to add a wrapper command for this that would permit doing, say, nix-openwrt-imagebuild '.#default'.

Any interest in integrating something like this in your project?

Thank you!
@astro
Copy link
Owner

astro commented Jan 22, 2023

In general, great idea! I already use the NixOS module system for networking configuration.

However, this piece of code already has a few ways of calling it. I prefer the managability and flexibility of putting lib.evalModules in my own code, passing the nix-openwrt-imagebuilder parameters together in one place. Are there use-cases where doing so would be inconvenient?

To me it starts to make sense if we start defining all the OpenWRT options as sensible NixOS module options. @ehmry launched a fork into this direction starting in #9. Although it would be nice to have, I am not sure I am ready to maintain and update such a bulk of declarations. Can it be automated just like I've done with the download hashes?

@ehmry
Copy link
Contributor

ehmry commented Jan 22, 2023

I'm not working with OpenWRT anymore so I'm not maintain my module fork. As I recall @Mic92 has similar configuration work that is relevant.

@Mic92
Copy link

Mic92 commented Jan 22, 2023

https://github.com/Mic92/dotfiles/blob/main/openwrt/example.nix

@astro
Copy link
Owner

astro commented Nov 9, 2023

There is also https://git.eno.space/dewclaw.git/tree/ now.

@pedorich-n
Copy link
Contributor

Hey, @tomeon! I noticed that your sourcehut repository is active and that you have implemented a very comprehensive module system, including the flake-parts module ❤️. Are you still interested in creating a PR to port it to this repository? 👀

I believe everyone would benefit from a more modular system that allows for atomic changes, better type-safety, and auto-generated documentation similar to NixOS modules.

Regarding @astro's comment:

Although it would be nice to have, I am not sure I am ready to maintain and update such a bulk of declarations.

I think @tomeon's approach to modules is simpler and easier to maintain than what was proposed in #9. While it would be great to have many individual services wrapped into Nix expressions like NixOS service modules, I agree that it would be hard to maintain.

In my opinion, wrapping UCI expressions into NIX and exposing packages, disabledServices, and files makes more sense, as it aligns more closely with what is already implemented in this repository and with ImageBuilder's interface.

@pedorich-n pedorich-n linked a pull request Dec 1, 2024 that will close this issue
Draft
@tomeon
Copy link
Author

tomeon commented Dec 20, 2024

@astro -- apologies for taking so long to reply here. Maybe it is superfluous now that #48 appears to have traction, but I'd like to put my thoughts down anyway.

I agree with @pedorich-n's remark that

everyone would benefit from a more modular system that allows for atomic changes, better type-safety, and auto-generated documentation similar to NixOS modules.

However, since it's likely everyone here is already convinced of the benefits of modular systems in general and the NixOS module system in particular, let's talk about the benefits specific to my implementation of a modular interface to this project.

I prefer the managability and flexibility of putting lib.evalModules in my own code, passing the nix-openwrt-imagebuilder parameters together in one place. Are there use-cases where doing so would be inconvenient?

It's not inconvenient for me personally, or (as far as I can guess) for any single person. However, there is the potential for the collective inconvenience of repeated wheel-reinvention. One of my goals in implementing a modular interface is to expose interfaces at various levels of abstraction, reducing the likelihood of needing to re-implement low-level plumbing while still making it possible to futz with the plumbing when necessary.

A case in point here is the layering of options that eventually get composed into the derivation that's passed as the files argument to the image builder function. From highest- to lowest-level:

  1. The (semi-)declarative UCI defaults options, which compose into
  2. UCI instructions, which compose into
  3. Text snippets representing the bodies of scripts to be installed in /etc/uci-defaults, which compose into
  4. Derivations built with pkgs.writeTextDir, which compose into
  5. A final, single derivation passed as the files argument to the image builder function.

At each of these levels, barring the lowest,1 it is possible to take the reins directly:

  1. The (semi-)declarative UCI defaults options are where I'd hope the bulk of the configuration will go; they are at least intended for that purpose.
  2. UCI instructions can be specified explicitly if (for instance) the user needs fine-grained control over instruction sequencing.
  3. Text snippets can be defined directly if (for instance) the user needs to do something other than run uci batch in an /etc/uci-defaults script.
  4. Derivations can be added directly to the list composed into the files argument.

I believe this represents a reasonably thorough hierarchy that balances power and convenience.

Regarding

To me it starts to make sense if we start defining all the OpenWRT options as sensible NixOS module options. [...] Although it would be nice to have, I am not sure I am ready to maintain and update such a bulk of declarations. Can it be automated just like I've done with the download hashes?

I again concur with @pedorich-n's remarks.

I do not think that it would be straightforward or even possible to implement a comprehensive modeling of "all the OpenWRT options". For one, I don't think there is any canonical representation of all available options, other than their implementation in the OpenWRT codebase. I certainly have not found any API that can be queried to get all of these options and their default values for a given OpenWRT-supported device.

Instead, I've focused on building a more-or-less comprehensive interface to the UCI defaults system.2 This way, consumers of the modular interface can declare and define just the settings that they need for the target device. It might not be as polished an experience as working with NixOS itself -- however, if the modeling of the UCI defaults system is reasonably comprehensive, then it could be used as the basis for implementing even-higher-level interfaces if and when it looks like it would be useful to do so.

Footnotes

  1. This is probably a mistake; users should be able to directly control the contents of the files argument. I'll update the module accordingly. ↩

  2. I am not entirely satisfied with the implementation, particularly the interface to "unnamed" options. I've represented sparse lists as Nix attribute sets whose keys are list indices, and provided library functions to help with defining such sparse lists, but the experience is clumsier than I'd like. ↩

@astro
Copy link
Owner

astro commented Dec 27, 2024 •
edited
Loading

I like the module system, and there seem to be even more gems in your fork. I'm happy to merge them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add module builder pedorich-n/nix-openwrt-imagebuilder
5 participants
@astro @Mic92 @ehmry @tomeon @pedorich-n