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

Improve Tiny Tiny RSS service #33347

Merged
merged 6 commits into from Jan 5, 2018
Merged

Improve Tiny Tiny RSS service #33347

merged 6 commits into from Jan 5, 2018

Conversation

jluttine
Copy link
Member

@jluttine jluttine commented Jan 2, 2018

Motivation for this change

Tiny Tiny RSS service was broken, see: #27048

This pull request fixes that and also makes a few small improvements to the service, in my opinion. Comments and feedback welcome.

To activate Tiny Tiny RSS service, this suffices:

    services.tt-rss = {
      enable = true;
      virtualHost = "domain.com";
      selfUrlPath = "http://domain.com/";
      database.type = "mysql";
    };
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@jluttine jluttine mentioned this pull request Jan 2, 2018
Closed
@jluttine
Copy link
Member Author

jluttine commented Jan 2, 2018

cc: @zohl @Nadrieril @bjornfor

Mic92
Mic92 reviewed Jan 2, 2018
View reviewed changes
nixos/modules/misc/ids.nix Outdated
@@ -301,6 +301,7 @@
pykms = 282;
kodi = 283;
restya-board = 284;
tt_rss = 285;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application directory is chowned on startup. Therefor it is not necessary to allocate a static uid/gid.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But tt_rss must be given some uid/gid somewhere anyway, right? If not here, then the user must provide it. At least that's how it works with immutable users and I'd prefer supporting immutable users seamlessly.

Copy link
Member

@Mic92 Mic92 Jan 3, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the user must not provide it. It will be allocated on the first use.
Static uids/gids are limited. Therefore we only allocate them, if necessary (i.e. when we have a lot of files, where we cannot change owner on startup)

Copy link
Member Author

@jluttine jluttine Jan 3, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, will it be allocated automatically even with immutable users?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any example user ID's or services in your mind where no static uids/gids were used but automatically generated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is an issue that each webapp defines and uses its own uid, we could add a generic phpfpm uid which can be used by all webapps using php-fpm. Similarly, as uwsgi uid. But of course it's best if each webapp is "isolated" with its own uid.

@zohl
Copy link
Contributor

zohl commented Jan 2, 2018

FWIW, I upgraded nixos on my server few monthes ago and got tt-rss broken too. There were problems with php configuration and wrong interpretation of exit codes in preStart script. Unfortunately, I manually fixed the second part and later forgot what I did.
As for php/nginx config, mine looks now like this:

services.nginx.virtualHosts = mkIf (cfg.virtualHost != null) {
  "${cfg.virtualHost}" = {
    root = "${cfg.root}";

    locations."/" = {
      index = "index.php";
    };

    locations."~ [^/]\.php(/|$)" = {
      extraConfig = ''
        fastcgi_split_path_info ^(.+?\.php)(/.*)$;

        if (!-f $document_root$fastcgi_script_name) {
            return 404;
        }

        # Mitigate https://httpoxy.org/ vulnerabilities
        fastcgi_param HTTP_PROXY "";

        fastcgi_pass unix:${phpfpmSocketName};
        fastcgi_index index.php;

        include ${pkgs.nginx}/conf/fastcgi_params;
        fastcgi_param PATH_INFO       $fastcgi_path_info;
        fastcgi_param PATH_TRANSLATED $document_root$fastcgi_path_info;
        fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
        fastcgi_param HTTP_PROXY "";
      '';
    };
  };
};

As far as I remember, I got blank web page and no information in logs. Turned out, the problem was in php-variables.
However, some people told me, that they haven't encountered any problems during setting up tt-rss.
I had not enough time to reproduce, prepare patch and test this, so if you find this useful, feel free to include in your PR :)

@globin globin self-assigned this Jan 4, 2018
jluttine and others added 6 commits January 5, 2018 14:43
@jluttine @globin
nixos/service.tt-rss: fix NixOS#27048
379ef52
@jluttine @globin
nixos/service.tt-rss: use tt_rss user
89c2e8a 
- Add tt_rss system user.
- Use tt_rss as the user by default.
- Create tt_rss user and group automatically if used.
@jluttine @globin
nixos/service.tt-rss: enable nginx automatically
c362443
@jluttine @globin
nixos/service.tt-rss: improve mysql automatic setup
4d485fe 
If the user chooses MySQL, it is enabled by default. Also, the used database is
created automatically along with the user and permissions.
@globin
nixos/service.tt-rss: improve pgsql support, do not use static uid/gid
3759e9a
@globin
tt-rss: 17.4 -> 2018-01-05
bad8b65 
tt-rss switched to a rolling release model
@globin
Copy link
Member

globin commented Jan 5, 2018

I've cleaned this up and updated tt-rss, thank you very much!

@globin globin merged commit 944376b into NixOS:master Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

Assignees

@globin globin

Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jluttine @zohl @globin @Mic92 @GrahamcOfBorg