nixos/gitea: add nginx configuration #218020

Izorkin · 2023-02-24T13:09:49Z

Description of changes

Add nginx configuration.

Things done

nixos/modules/services/misc/gitea.nix

Izorkin · 2023-03-08T18:03:48Z

Don't need access to /var/empty.

emilylange · 2023-03-09T23:56:57Z

nixos/modules/services/misc/gitea.nix

+        locations."^~ /avatars/" = {
+          alias = "${cfg.stateDir}/data/avatars/";
+          tryFiles = "$uri =404";
+
+          extraConfig = ''
+            default_type "image";
+          '' + nginxCommonHeaders;
+        };
+
+        locations."^~ ${cfg.settings.server.STATIC_URL_PREFIX}/assets/" = {
+          alias = "${cfg.package.data}/public/";
+          tryFiles = "$uri =404";


I thought about it a few days, and I really do not see that much value in letting nginx serve static files directly.
As mentioned in #218020 (comment), even big instances don't seem to enable this feature.
And in the context of nixpkgs, it just makes this config unnecessarily complex.

If possible, I always prefer to serve static files directly through nginx. In this case the file will be transferred a little faster.

/avatars/ (and /repo-avatars/, for that matter) isn't static, though.
It dynamically resizes the image based on the ?size uri parameter.

And can be overwritten via picture.AVATAR_UPLOAD_PATH (picture.REPOSITORY_AVATAR_UPLOAD_PATH), which would break "${cfg.stateDir}/data/avatars/".

Furthermore, those could be stored remotely using an entirely different storage driver using AVATAR_STORAGE_TYPE (REPOSITORY_AVATAR_STORAGE_TYPE).

I stand by what I said and do not understand why this nixos/gitea module needs to be made more complex than it needs to be.

I can later try to test the operation with these parameters.

/avatars/ (and /repo-avatars/, for that matter) isn't static, though.
It dynamically resizes the image based on the ?size uri parameter.

Image processing is done by the browser, the gitea service is not involved.

And can be overwritten via picture.AVATAR_UPLOAD_PATH (picture.REPOSITORY_AVATAR_UPLOAD_PATH), which would break "${cfg.stateDir}/data/avatars/".

Furthermore, those could be stored remotely using an entirely different storage driver using AVATAR_STORAGE_TYPE (REPOSITORY_AVATAR_STORAGE_TYPE).

Fixed!

If Gravatar is enabled, then /avatar/<email-hash>?size=96 redirects (http/303) to https://secure.gravatar.com/avatar/<email-hash>?d=identicon&s=96

You are, however, correct, that the local storage does not resize the images server-side (yet?¹)

Your avatar-serving breaks if

Gravatar isn't disabled (DISABLE_GRAVATAR), which as of v1.18 moved to the database, meaning you can no longer assert it from the nixos config. OR

OFFLINE_MODE is enabled, which also disables Gravatar as side effect. OR

AVATAR_STORAGE_TYPE/REPOSITORY_AVATAR_STORAGE_TYPE is to something custom/not local

Plus, the docs for STATIC_URL_PREFIX explicitly state

[...] Avatar images are dynamic resources and still served by Gitea. [...]

Footnotes

Could change in the future, see https://github.com/go-gitea/gitea/issues/20918 ↩

If gravatar is used, the link will have an address like this - https://www.gravatar.com/avatar/... As a result, the rule location ^~ /avatars/ { will not work for them.

Plus, the docs for STATIC_URL_PREFIX explicitly state

If avatar handling in gitea changes in the future, we can fix the nginx configuration.

Added small fix.

oh lol sorry -- my bad, yes, you are right.
I somehow overlooked that additional s in /avatars/ compared to /avatar/ 😅

I would still prefer this to be dropped altogether.
But I also don't want to annoy you anymore regarding this.
Just keep it however you imagined it originally and let the other reviewers decide.
Sorry for the noise (and the whole back and forth)

Your comments helped improve PR. I didn't take into account possible changes to the defaults when I created the PR.

nixos/modules/services/misc/gitea.nix

Ma27 · 2023-03-11T19:45:32Z

Just two minor things. I guess it'd be nice to extend the VM test to check if e.g. avatars are served correctly, but other than that it looks good to me, thanks 👍

Izorkin · 2023-03-11T19:47:50Z

Just two minor things. I guess it'd be nice to extend the VM test to check if e.g. avatars are served correctly, but other than that it looks good to me, thanks 👍

Tested on a test virtual machine, no problems with avatars

SuperSandro2000 · 2023-03-19T21:33:40Z

nixos/modules/services/misc/gitea.nix

+        root = "/var/empty";
+
+        locations."/" = {
+          proxyPass = (if cfg.enableUnixSocket then "http://unix:/run/gitea/gitea.sock" else "http://127.0.0.1:${toString cfg.httpPort}");


Suggested change

proxyPass = (if cfg.enableUnixSocket then "http://unix:/run/gitea/gitea.sock" else "http://127.0.0.1:${toString cfg.httpPort}");

proxyPass = if cfg.enableUnixSocket then "http://unix:/run/gitea/gitea.sock" else "http://127.0.0.1:${toString cfg.httpPort}";

One could also use "http://unix:${cfg.settings.server.HTTP_ADDR}" instead of
"http://unix:/run/gitea/gitea.sock".

And if we go down that route, it might also be worth setting cfg.httpAddress = "127.0.0.1" (which in turn sets cfg.settings.server.HTTP_ADDR) when cfg.configureNginx is enabled to further deduplicate the addresses used and allow something like:

proxyPass = "http://" + (lib.optionalString cfg.enableUnixSocket "unix:") + cfg.settings.server.HTTP_ADDR + (lib.optionalString (!cfg.enableUnixSocket) ":${toString cfg.httpPort}");

Or assert cfg.enableUnixSocket = true when cfg.configureNginx = true instead of just setting the default for it.
That would at least simplify the logic /shrug

Update to:

proxyPass = if cfg.enableUnixSocket then "http://unix:${cfg.settings.server.HTTP_ADD}" else "http://127.0.0.1:${toString cfg.httpPort}";

And if we go down that route, it might also be worth setting cfg.httpAddress = "127.0.0.1" (which in turn sets cfg.settings.server.HTTP_ADDR) when cfg.configureNginx is enabled to further deduplicate the addresses used and allow something like:

By default HTTP_ADDR = "0.0.0.0";, not 127.0.0.1.

Or assert cfg.enableUnixSocket = true when cfg.configureNginx = true instead of just setting the default for it.
That would at least simplify the logic /shrug

Change to:

enableUnixSocket = mkOption { type = types.bool; default = cfg.configureNginx;

or:

enableUnixSocket = mkOption { type = types.bool; default = true;

?

Izorkin · 2023-12-24T22:18:30Z

Remove includeSubDomains from Strict-Transport-Security header.

RaitoBezarius · 2024-01-12T19:18:42Z

@emilylange @Ma27 Do you think we can come to a conclusion on this change? Either, we decrease the scope into smaller PRs so that authors can at least dispatch some useful changes and then for the more controversial stuff, we can keep this PR.

emilylange · 2024-01-12T20:28:34Z

@RaitoBezarius see #218020 (comment).
I dismissed my review with "I'm no longer maintaining gitea" roughly 2 months ago.

I guess I could have been more explicit, but back then I figured this wording alone with the context surrounding it is clear enough.

My bad, let me give it another go:

I completely withdrew my code ownership of gitea and no longer intent to contribute to gitea (in nixpkgs).

Any remaining reviews, objections, whatever, of open or not-yet-merged PRs can and should be ignored.
I don't want to hinder the actual gitea maintainers (anymore?). And I also don't intend to spend any more time on this.
Especially given this simply isn't my realm anymore.

I maintain forgejo, not gitea -- and while forgejo initially did use almost everything from gitea in nixpkgs (drv, module, VM test), this is no longer the case.

RaitoBezarius · 2024-01-12T20:58:16Z

@RaitoBezarius see #218020 (comment). I dismissed my review with "I'm no longer maintaining gitea" roughly 2 months ago.

I guess I could have been more explicit, but back then I figured this wording alone with the context surrounding it is clear enough.

My bad, let me give it another go:

I completely withdrew my code ownership of gitea and no longer intent to contribute to gitea (in nixpkgs).

Any remaining reviews, objections, whatever, of open or not-yet-merged PRs can and should be ignored. I don't want to hinder the actual gitea maintainers (anymore?). And I also don't intend to spend any more time on this. Especially given this simply isn't my realm anymore.

I maintain forgejo, not gitea -- and while forgejo initially did use almost everything from gitea in nixpkgs (drv, module, VM test), this is no longer the case.

My apologies, I will not do this mistake again.

Izorkin · 2024-01-17T12:55:49Z

Update PR.

Izorkin · 2024-02-16T06:28:28Z

cc @Ma27 @SuperSandro2000 @RaitoBezarius

github-actions bot added 6.topic: nixos 8.has: module (update) labels Feb 24, 2023

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Feb 24, 2023

Izorkin requested review from SuperSandro2000 and Ma27 February 24, 2023 13:42

Ma27 reviewed Mar 4, 2023

View reviewed changes

nixos/modules/services/misc/gitea.nix Outdated Show resolved Hide resolved

nixos/modules/services/misc/gitea.nix Show resolved Hide resolved

Izorkin force-pushed the add-gitea-nginx branch from 70cf6c8 to 4f84313 Compare March 5, 2023 09:54

Izorkin requested a review from Ma27 March 5, 2023 09:55

SuperSandro2000 reviewed Mar 6, 2023

View reviewed changes

nixos/modules/services/misc/gitea.nix Outdated Show resolved Hide resolved

SuperSandro2000 reviewed Mar 6, 2023

View reviewed changes

nixos/modules/services/misc/gitea.nix Outdated Show resolved Hide resolved

nixos/modules/services/misc/gitea.nix Show resolved Hide resolved

nixos/modules/services/misc/gitea.nix Outdated Show resolved Hide resolved

nixos/modules/services/misc/gitea.nix Outdated Show resolved Hide resolved

Izorkin force-pushed the add-gitea-nginx branch from 4f84313 to 9066b2a Compare March 6, 2023 18:12

Izorkin marked this pull request as draft March 8, 2023 16:25

Izorkin force-pushed the add-gitea-nginx branch from 9066b2a to 5de9737 Compare March 8, 2023 18:02

Izorkin marked this pull request as ready for review March 8, 2023 18:03

emilylange requested changes Mar 10, 2023

View reviewed changes

Izorkin force-pushed the add-gitea-nginx branch 2 times, most recently from 9e825b3 to 907ff8f Compare March 10, 2023 20:46

Izorkin requested a review from SuperSandro2000 March 10, 2023 20:52

Ma27 reviewed Mar 11, 2023

View reviewed changes

nixos/modules/services/misc/gitea.nix Outdated Show resolved Hide resolved

Izorkin force-pushed the add-gitea-nginx branch from 907ff8f to 343df9d Compare March 11, 2023 19:46

Izorkin force-pushed the add-gitea-nginx branch from 343df9d to dc916f9 Compare March 16, 2023 17:57

Izorkin requested a review from Ma27 March 16, 2023 17:59

Izorkin force-pushed the add-gitea-nginx branch from dc916f9 to 2e30023 Compare March 17, 2023 06:38

SuperSandro2000 reviewed Mar 19, 2023

View reviewed changes

Izorkin force-pushed the add-gitea-nginx branch 2 times, most recently from b1d4414 to 3e6c62b Compare March 20, 2023 12:08

Izorkin force-pushed the add-gitea-nginx branch from e576aff to 06f6404 Compare January 17, 2024 12:55

Izorkin force-pushed the add-gitea-nginx branch 5 times, most recently from 3f79474 to 8da45ee Compare January 20, 2024 10:33

Izorkin force-pushed the add-gitea-nginx branch from 8da45ee to 1a26488 Compare January 27, 2024 10:56

Izorkin force-pushed the add-gitea-nginx branch from 1a26488 to 7ef308a Compare February 16, 2024 06:27

wegank added the 2.status: merge conflict label May 3, 2024

Izorkin force-pushed the add-gitea-nginx branch from 7ef308a to a6de654 Compare May 8, 2024 12:16

ofborg bot removed the 2.status: merge conflict label May 8, 2024

Izorkin force-pushed the add-gitea-nginx branch 2 times, most recently from 4511526 to 52f37bb Compare May 12, 2024 10:40

Izorkin requested review from Ma27 and SuperSandro2000 and removed request for Ma27 and SuperSandro2000 May 12, 2024 10:45

Izorkin added 2 commits October 4, 2024 23:36

nixos/gitea: add nginx configuration

bf8cd00

nixos/tests/gitea: check gitea with use nginx web-server

9e3e8ae

Izorkin force-pushed the add-gitea-nginx branch from 52f37bb to 9e3e8ae Compare October 4, 2024 20:38

Izorkin requested review from Ma27 and SuperSandro2000 and removed request for Ma27 and SuperSandro2000 October 4, 2024 20:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/gitea: add nginx configuration #218020

nixos/gitea: add nginx configuration #218020

Izorkin commented Feb 24, 2023

Izorkin commented Mar 8, 2023

emilylange Mar 9, 2023

Izorkin Mar 10, 2023

emilylange Mar 11, 2023

Izorkin Mar 12, 2023

Izorkin Mar 16, 2023

emilylange Mar 17, 2023 •

edited

Loading

Izorkin Mar 17, 2023

emilylange Mar 17, 2023

Izorkin Mar 17, 2023

Ma27 commented Mar 11, 2023

Izorkin commented Mar 11, 2023

SuperSandro2000 Mar 19, 2023

emilylange Mar 20, 2023

Izorkin Mar 20, 2023

Izorkin commented Dec 24, 2023

RaitoBezarius commented Jan 12, 2024

emilylange commented Jan 12, 2024

RaitoBezarius commented Jan 12, 2024

Izorkin commented Jan 17, 2024

Izorkin commented Feb 16, 2024

	proxyPass = (if cfg.enableUnixSocket then "http://unix:/run/gitea/gitea.sock" else "http://127.0.0.1:${toString cfg.httpPort}");
	proxyPass = if cfg.enableUnixSocket then "http://unix:/run/gitea/gitea.sock" else "http://127.0.0.1:${toString cfg.httpPort}";

nixos/gitea: add nginx configuration #218020

Are you sure you want to change the base?

nixos/gitea: add nginx configuration #218020

Conversation

Izorkin commented Feb 24, 2023

Description of changes

Things done

Izorkin commented Mar 8, 2023

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

emilylange Mar 17, 2023 • edited Loading

Choose a reason for hiding this comment

Footnotes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ma27 commented Mar 11, 2023

Izorkin commented Mar 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Izorkin commented Dec 24, 2023

RaitoBezarius commented Jan 12, 2024

emilylange commented Jan 12, 2024

RaitoBezarius commented Jan 12, 2024

Izorkin commented Jan 17, 2024

Izorkin commented Feb 16, 2024

emilylange Mar 17, 2023 •

edited

Loading