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.

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

nixos/dendrite: remove #123753

Closed
wants to merge 1 commit into from
Closed

nixos/dendrite: remove #123753

wants to merge 1 commit into from

Conversation

mjlbach
Copy link
Contributor

@mjlbach mjlbach commented May 20, 2021

Motivation for this change

There are some usability problems around the module re: dynamic user's ability to access protected files (you currently need to copy these into the persistent state directory). I said I'd fix it before the 21.05 branch off, but I could not due to some issues I ran into with systemd.

@dotlambda I'm not going to be able to get LoadCredential working before systemd 248 likely. There are other workarounds to improve the ergonomics of DynamicUser with the TLS/federation keys but not sure if we could get those in before 21.05. I can make a PR un-reverting this which includes the tests/etc.

Things done

Remove dendrite, dendrite module, dendrite tests.

  • 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 nixpkgs-review --run "nixpkgs-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.

@github-actions 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 May 20, 2021
@ofborg ofborg bot added 8.has: clean-up 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels May 20, 2021
@dotlambda
Copy link
Member

That's sad :(
Did you come across any particular issues when trying systemd credentials?

I think there's no need to remove the package, just the module.

What's @NixOS/nixos-release-managers opinion on this? The rationale is that shipping a broken module is bad because we might have to introduce breaking changes to fix it. That said, the module is working except for tlsKey and tlsCert options.

@dotlambda dotlambda changed the title dendrite: remove nixos/dendrite: remove May 20, 2021
@dotlambda
Copy link
Member

@mjlbach Please add a short rationale to the commit message.

@mjlbach mjlbach force-pushed the dendrite_remove branch from 7f2ff60 to 00cbe8c Compare May 20, 2021 17:02
@mjlbach
Copy link
Contributor Author

mjlbach commented May 20, 2021

@dotlambda you should write whatever rationale you want here, and I'll add it to the commit message.

Re: LoadCredential, I'm running into the same issue as @happysalada does here systemd/systemd#19604

@mohe2015
Copy link
Contributor

I'm not using this currently but what about only removing on the release branch (which is not created yet afaik) and keeping in unstable?

@dotlambda
Copy link
Member

I'm not using this currently but what about only removing on the release branch (which is not created yet afaik) and keeping in unstable?

That sounds like a good option.

@dotlambda you should write whatever rationale you want here, and I'll add it to the commit message.

I guess something like "The options tlsKey and tlsCert are broken and fixing them might require breaking changes. Thus the module should not be included in a stable release."

Re: LoadCredential, I'm running into the same issue as @happysalada does here systemd/systemd#19604

You could try rebasing your changes on #123476 and see if that works.

@mjlbach
Copy link
Contributor Author

mjlbach commented May 21, 2021

That sounds like a good option.

Ok! I'll file this same PR with that message into 21.05 after branch-off.

Re: systemd 248, sg, I will try this weekend to get everything running and if it works we can fix the module on nixpkgs-unstable.

@mohe2015
Copy link
Contributor

@mjlbach branchoff has happened so you may want to do this now.

@mjlbach mjlbach closed this May 26, 2021
@mjlbach
Copy link
Contributor Author

mjlbach commented May 26, 2021

Just did :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: clean-up 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants