nixos/imaginary: init #215222

dotlambda · 2023-02-08T01:21:32Z

Tested using

services.nextcloud.config = {
  enabledPreviewProviders = [
    ''OC\Preview\Imaginary''
  ];
  preview_imaginary_url = "http://localhost:8088/";
};
services.imaginary = {
  enable = true;
  settings = {
    a = "localhost";
    p = 8088;
    return-size = true;
  };
};

Description of changes

Things done

urandom2

I am pleased with the derivation changes; still new to nixos modules, but nothing stands out as egregious

pkgs/servers/imaginary/default.nix

pennae · 2023-02-08T03:20:47Z

nixos/modules/services/networking/imaginary.nix

+        ]);
+
+        options = {
+          a = mkOption {


are these option names that good? we'd much prefer giving the descriptive names and amending the description with something like Corresponds to the `-a` parameter.. this wouldn't stop anyone using the original names, but it'd make module instances more readable.

(note: we're only taking issue with a and p, but return-size is totally good as is)

I'm also not super happy with it but I think this would be against the spirit of NixOS/rfcs#42.
However we could file a PR upstream that introduces e.g. -address as an alias for -a.

I guess we could just add these options outside of settings.

both sound reasonable, but adding them outside of settings is probably better. that more clearly communicates that there could be special handling going on.

nixos/modules/services/networking/imaginary.nix

dotlambda · 2023-02-08T04:48:22Z

I can't get ports < 1024 to work:

listen: listen tcp 10.0.0.23:99: bind: permission denied

Does someone have an idea?

pennae · 2023-02-08T05:05:51Z

I can't get ports < 1024 to work:
listen: listen tcp 10.0.0.23:99: bind: permission denied
Does someone have an idea?

PrivateUsers= causes the unit to be executed with no capabilities in the hosting namespace, so your capability sets do nothing. should work if you drop it.

dotlambda · 2023-02-08T06:11:00Z

I added TemporaryFileSystem and BindReadOnlyPaths.

pennae

haven't tested the module, but it looks good.

nixos/modules/services/networking/imaginary.nix

Apply h2non/imaginary#382 as patch. The -return-size flag is recommended by Nextcloud: https://docs.nextcloud.com/server/latest/admin_manual/installation/server_tuning.html#previews

SuperSandro2000 · 2023-02-13T00:48:03Z

nixos/modules/services/networking/imaginary.nix

+
+    address = mkOption {
+      type = types.str;
+      default = "";


Does emptry string equal to localhost? If I understand the nextcloud doc correct it sounds like you want to run imaginary always bound on localhost.

Yes, it does mean that according to https://pkg.go.dev/net#Dial. I went with upstream's default: https://github.com/h2non/imaginary/blob/master/imaginary.go#L20

Never mind, it means any address. That's also what it says in https://github.com/h2non/imaginary#command-line-usage.

I guess it should be changed to localhost by default. See #100192. But the problem is that localhost means 127.0.0.1 and doesn't include ::1.

Can we use [::1]?

oh, gotta love it. golang/go#9334

Should we set address = "localhost" by default anyway?

if it's primarily not meant to be accessed from other hosts, probably? don't know enough about the service to be sure. but then again, if it makes for such interesting complications it might just be better to rely on the firewall existing and a note that by default all addresses are bound? 😕

For the nextcloud use case it is only accessed through localhost.

dotlambda requested review from urandom2, jtrees and dit7ya February 8, 2023 01:21

github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 8, 2023

dotlambda force-pushed the nixos-imaginary-init branch 2 times, most recently from 7588230 to 468d34f Compare February 8, 2023 01:27

urandom2 approved these changes Feb 8, 2023

View reviewed changes

pkgs/servers/imaginary/default.nix Show resolved Hide resolved

pkgs/servers/imaginary/default.nix Outdated Show resolved Hide resolved

ofborg bot requested a review from urandom2 February 8, 2023 01:53

ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Feb 8, 2023

pennae reviewed Feb 8, 2023

View reviewed changes

dotlambda force-pushed the nixos-imaginary-init branch from 468d34f to bd062c7 Compare February 8, 2023 04:46

dotlambda force-pushed the nixos-imaginary-init branch from bd062c7 to a8b4e7d Compare February 8, 2023 06:10

dotlambda marked this pull request as ready for review February 8, 2023 06:10

dotlambda requested review from SuperSandro2000, onny and yayayayaka February 8, 2023 06:17

urandom2 added the 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package label Feb 8, 2023

pennae approved these changes Feb 9, 2023

View reviewed changes

pennae reviewed Feb 9, 2023

View reviewed changes

nixos/modules/services/networking/imaginary.nix Outdated Show resolved Hide resolved

dotlambda force-pushed the nixos-imaginary-init branch 2 times, most recently from 9e2235d to fbd5bec Compare February 10, 2023 02:47

github-actions bot added 8.has: changelog 8.has: documentation This PR adds or changes documentation labels Feb 10, 2023

dotlambda added 2 commits February 11, 2023 09:11

imaginary: add -return-size flag

78b83f6

Apply h2non/imaginary#382 as patch. The -return-size flag is recommended by Nextcloud: https://docs.nextcloud.com/server/latest/admin_manual/installation/server_tuning.html#previews

nixos/imaginary: init

efee1b5

dotlambda force-pushed the nixos-imaginary-init branch from fbd5bec to efee1b5 Compare February 11, 2023 17:18

dotlambda merged commit 7b60fce into NixOS:master Feb 12, 2023

dotlambda deleted the nixos-imaginary-init branch February 12, 2023 16:42

SuperSandro2000 reviewed Feb 13, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/imaginary: init #215222

nixos/imaginary: init #215222

dotlambda commented Feb 8, 2023 •

edited

Loading

urandom2 left a comment

pennae Feb 8, 2023

dotlambda Feb 8, 2023

dotlambda Feb 8, 2023

pennae Feb 8, 2023

dotlambda commented Feb 8, 2023

pennae commented Feb 8, 2023

dotlambda commented Feb 8, 2023

pennae left a comment

SuperSandro2000 Feb 13, 2023

dotlambda Feb 13, 2023

dotlambda Feb 13, 2023

dotlambda Feb 15, 2023

SuperSandro2000 Feb 17, 2023

pennae Feb 17, 2023

dotlambda Feb 17, 2023

pennae Feb 18, 2023

dotlambda Feb 20, 2023

SuperSandro2000 Feb 22, 2023

nixos/imaginary: init #215222

nixos/imaginary: init #215222

Conversation

dotlambda commented Feb 8, 2023 • edited Loading

Description of changes

Things done

urandom2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotlambda commented Feb 8, 2023

pennae commented Feb 8, 2023

dotlambda commented Feb 8, 2023

pennae left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotlambda commented Feb 8, 2023 •

edited

Loading