GitHub runners: configurable user, environment, and service overrides + multiple runners

[thomasjm]

Description of changes

This PR contains 3 improvements to the github-runner NixOS module.

1. Add a user option so the systemd service can be run as specific user.
2. Add the extraEnvironment and serviceOverrides options, which allow you to augment the environment and the systemd service settings respectively. The latter is essential when you find the default sandboxing settings are too strict.
3. Add the services.github-runners configuration option, which allows you to start multiple runners.

All together, you can now write the following:

{
  services.github-runners = {
    runner1 = {
      enable = true;
      url = "https://github.com/repo/owner";
      name = "runner1";
      tokenFile = "/secrets/token1";
      user = "tester";
      extraEnvironment = {
        GIT_CONFIG = "/path/to/git/config";
      };
      serviceOverrides = {
        ProtectHome = false; # Allow the test user to access their home folder
      };
    };

    runner2 = {
      enable = true;
      url = "https://github.com/repo/owner";
      name = "runner2";
      tokenFile = "/secrets/token2";
      user = "tester";
    };
  };
}

Backwards compatibility is preserved with the old services.github-runner = {...} option which starts a single runner.

Things done

• Built on platform(s)
  • [X] x86_64-linux
  • [ ] aarch64-linux
  • [ ] x86_64-darwin
  • [ ] aarch64-darwin
• [ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• [ ] 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/)
• 22.11 Release Notes (or backporting 22.05 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
  • [ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• [X] Fits CONTRIBUTING.md.

[thomasjm]

Hi @grahamc -- any chance you know who might be good to review this? It's actually a small change, it just looks large because I had to move some file contents around to reuse in the new module. Thanks!

[veehaitch]

I wasn't aware of this PR. Very nice, @thomasjm. If you resolve the conflicts, I'm happy to review and test your changes!

[thomasjm]

Ah, this has become pretty messy to rebase. I'll try to get to it when I have some time.

[mkaito]

This looks nice. I was actually thinking of adding the ability to run multiple runners myself, as I had previously done the same for buildkite. I'll subscribe so I get notified when you get around to rebasing it, and I'll test it.

[thomasjm]

Okay @veehaitch @mkaito , this is rebased and seems to work. Have at it!

[thomasjm]

Hey guys, please review before more changes happen and I have to do that rebase again :)

[domenkozar]

It would be really amazing if there was a nix-darwin port of this module :)

[veehaitch]

It would be really amazing if there was a nix-darwin port of this module :)

Agree! We'd first need a port of the package though.

[veehaitch]

I'm sorry that this takes a while but I ran into a bug while testing your PR: #195003

[thomasjm]

Okay @veehaitch, I've updated this branch to reflect 499748bc0446f61be94b6946183e8895ca2be57b.

[Profpatsch]

This seems to have broken our github runner’s access to the docker daemon:

 Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post "http://%2Fvar%2Frun%2Fdocker.sock/v1.24/auth": dial unix /var/run/docker.sock: connect: permission denied

I haven’t yet checked, but on first look the github runner does not have a user id anymore.

Please, please be more careful with stuff like this. Look at this commit, how is anybody expected to review this? https://github.com/NixOS/nixpkgs/commit/998083f2adb123f4bad30309f0994b9db9190b3d

[mkaito]

They run as DynamicUser now. You can add extra groups to dynamic users:

services.github-runners.foo.serviceOverrides = {
 SupplementaryGroups = [ "docker" ];
};

[veehaitch]

They run as DynamicUser now.

Has been running with DynamicUser=true from the start:

https://github.com/NixOS/nixpkgs/blob/f4af2f267a4a2e47277078e498b69400b6859478/nixos/modules/services/continuous-integration/github-runner.nix#L260

[mkaito]

Huh, I guess that was never a problem for me before then. Good to know though, thanks.

[Profpatsch]

our config states:

  systemd.services.github-runner.serviceConfig.SupplementaryGroups = [
    "docker"
  ];

yet it stopped working

[mkaito]

It was working for me. I stopped using it due to other issues (the __e thing with nodejs), but it was spinning up containers just fine.

If you have the time, poke me on Matrix. Maybe we can pair on this for a few minutes? @chris:mkaito.net, I'm in most NixOS matrix channels.

[veehaitch]

That makes sense as the naming of the systemd service changed. It's github-runner-${name} now. Sorry about that. I asked about a convention here: https://github.com/NixOS/nixpkgs/pull/176691#discussion_r979393632

Anyways, @mkaito's suggestion should work for you.

[veehaitch]

our config states:

  systemd.services.github-runner.serviceConfig.SupplementaryGroups = [
    "docker"
  ];

yet it stopped working

... I also did not consider that the module system will just silently ignore your config here.

[Profpatsch]

I switched it to

  services.github-runner.serviceOverrides.SupplementaryGroups = [
    "docker"
  ];

and now it seems to work.

[Profpatsch]

Thanks for all the help :heart_decoration:

[winterqt]

Hey all, if this is a breaking change (as it seems it may be?), can this be added to the 21.11 changelog? (If you make a PR, please request my review on it.)

(And if it's not, please disregard. Though it may be useful to mention the service name change/how to configure it from the github-runner module.)

Thanks.

[veehaitch]

Happy to hear that! Maybe we should at least add a release notes entry pointing users to the new option?

[veehaitch]

Great minds think alike, @winterqt. I'll open a PR.

[mkaito]

It's probably worth using SupplementaryGroups in the example for serviceOverrides since that's likely a widespread usage of the option.