tailscale: fix tailscale ssh

[SuperSandro2000]

Closes #310950

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[endocrimes]

We should revert the combination PR rather than shipping an un-reviewed PR as a patch - We clearly don't have enough testing infrastructure in place here, and repeatedly breaking folks networking and security boundary software is painfully invasive.

[endocrimes]

(Reviewed the patch and there's technically also a slight regression).

[SuperSandro2000]

(Reviewed the patch and there's technically also a slight regression).

That was easy to fix by using the same method the tailscaled uses to detect if it should start the cli https://github.com/tailscale/tailscale/pull/5193/files

We should revert the combination PR rather than shipping an un-reviewed PR as a patch - We clearly don't have enough testing infrastructure in place here, and repeatedly breaking folks networking and security boundary software is painfully invasive.

"We" as in tailscale. I took this from their upstream documentation https://tailscale.com/kb/1207/small-tailscale and since there were no hints at all, I assumed this is straightforward and easy to use to reduce unnecessary binary size.

I think we understand the problem at hand pretty well and I have looked at all other usages of os.Executable() and there should not be affected by this, so a revert would create a noisy history.

[endocrimes]

We: Nixpkgs - There have been several ways that we've broken the tailscale package and module lately - including this. We should move back to a stable point and then only start making changes if they're going to be well tested first.

[SuperSandro2000]

Well, the other breakage was because a years old commit got reverted and that PR shouldn't have been merged in the first place and didn't receive a proper review from a maintainer.

We cannot realistically test everything, hence we always need to rely a bit on nixos-unstable to discover some issues.

I think we should just merge this and move forward and then we are at the stable point again.

[06kellyjac]

I agree that not every tailscale feature can be realistically tested every PR.
It could be argued that tailscale ssh should have been tested but it can also be equally argued that nixos-unstable has no formal stability guarantees and by having any integration testing via headscale is ahead of other distributions.

If this issue when using ts_include_cli, and symlinking as described in the official tailscale docs, is not exclusive to the nix built copy due to our wrapping, this request for more extensive testing could also be applied to tailscale rather than unpaid volunteers.

---

In order to focus on how we (as the nixpkgs maintainers for tailscale) can improve going forwards rather than focusing on past negatives here are some proposals:

• we could look to extend headscale integration tests to further tailscale features
• we could leave non-version-bump changes open longer for people who are interested in testing
  • duration to be determined, could have exceptions
  • I still think faster merging of updates should be fine, extension of headscale integration testing should assist, we could pick a duration but it must be quicker than non-version-bump changes IMO
• people concerned with the stability of tailscale could join in testing these PRs and post their experiences
  • PRs being open longer allows for more opertunity
  • individuals could be informal testers or officially sign up as maintainers
  • a method of notifying these individuals (including informal testers) would potentially be necessary

We will not catch all issues but these could help. I welcome thoughts on these proposals or additional ideas.

[github-actions]

Successfully created backport PR for release-24.05:

• [Backport release-24.05] tailscale: fix tailscale ssh #315301