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

nixos/gitea: Fix startup #74852

Merged
merged 2 commits into from Dec 5, 2019
Merged

nixos/gitea: Fix startup #74852

merged 2 commits into from Dec 5, 2019

Conversation

srhb
Copy link
Contributor

@srhb srhb commented Dec 2, 2019 •
edited

Motivation for this change

Fixes #74849 hopefully.

I'm tempted to revert to a much more lenient sandbox than what this gives us, but I'd like to discuss how we can test that these are sufficient permissions @dasJ

This is just what I needed to remove to restore enough functionality to make very basic testing succeed in a real setup with postgres as the database. It's hard to tell whether more things are broken.

Thoughts?

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@srhb srhb requested a review from dasJ December 2, 2019 20:47
@srhb srhb changed the title Gitea fix start nixos/gitea: Fix startup Dec 2, 2019
@flokli
Copy link
Contributor

flokli commented Dec 4, 2019

@srhb how should the "hopefully" be understood?

We already have some gitea tests, including one with postgres, so it should be easy to verify.

@srhb srhb mentioned this pull request Dec 4, 2019
Merged
10 tasks
@srhb
Copy link
Contributor Author

srhb commented Dec 5, 2019

@srhb how should the "hopefully" be understood?

As in I don't know whether something is subtly broken or just changed without documentation still, but I have not found anything yet. The test passed simply by fixing up the systemd unit, but the rest of the changes I made after noticing that my real setup was still broken, eg. with nginx being unable to talk to the socket. :)

If you have a better method to verify things, I'm all ears.

@flokli
Copy link
Contributor

flokli commented Dec 5, 2019

I don't operate a gitea installation currently, sorry. Because of #74849 (comment) I'd assume it improves things, but maybe @petabyteboy can comment here?

@ghost
Copy link

ghost commented Dec 5, 2019

Yes, the change of SystemCallFilter which is included here does fix the problem most people will have.
I am running into additional problems because I was using AmbientCapabilities to allow gitea to bind to port 22, and I can not make it work with these new settings (with and without this fix).

But this PR would definitely improve the situation and fix it for most people.

@kolaente kolaente mentioned this pull request Dec 5, 2019
Merged
10 tasks
@srhb
Copy link
Contributor Author

srhb commented Dec 5, 2019

OK, let's go for it and hopefully there are no more traps.

@flokli flokli merged commit 77f26cc into NixOS:master Dec 5, 2019
@srhb srhb deleted the gitea-fix-start branch December 5, 2019 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dasJ dasJ Awaiting requested review from dasJ

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

gitea startup is broken
2 participants
@srhb @flokli