This commit fixes two bugs:
1) When starting a github-runner for the very first time, the
unconfigure script did not copy the `tokenFile` to the state
directory. This case just was not handled so far. As a result, the
runner could not configure. The unit did, however, fail even before
as the state token file is configured as inaccessible for the service
through `InaccessiblePaths=`. As the given path did not exist in the
described case, setting up the unit's namespacing failed.
2) Similarly, the `tokenFile` is also marked as not accessible to the
service user. There are, however, cases where other namespacing
options make the files inaccessible even before `InaccessiblePaths=`
kicks in; thus, they appear as non existing and cause the namespacing
to fail yet again. Prefixing the entry with a `-` causes Systemd to
ignore the entry if it cannot find it. This is the behavior we want.
I also took fixing those bugs as a chance to refactor the unconfigure
script to make it easier to follow.
conversions were done using https://github.com/pennae/nix-doc-munge
using (probably) rev f34e145 running
nix-doc-munge nixos/**/*.nix
nix-doc-munge --import nixos/**/*.nix
the tool ensures that only changes that could affect the generated
manual *but don't* are committed, other changes require manual review
and are discarded.
using regular strings works well for docbook because docbook is not as
whitespace-sensitive as markdown. markdown would render all of these as
code blocks when given the chance.
The `runsvc.sh` script wraps a JavaScript script which starts
`Runner.Listener` and also handles failures. This has the downside that
the service _always_ exits with status code 0, i.e., success. This
causes frequent service restarts when running in ephemeral mode with a
faulty config as Systemd always sees a success exit status. To prevent
this, this commit changes the service config to call `Runner.Listener`
directly. The JavaScript wrapper stops the process with a SIGINT, hence,
the Systemd unit now sends a SIGINT to stop the service.
Adds the module option `ephemeral`. If set to true, configures the
runner registration with the `--ephemeral` option. This causes the
runner to exit after processing a single job, to de-register itself, and
to delete its configuration. Afterward, systemd restarts the service
which triggers a new ephemeral registration with a clean state.
This commit introduces support for runner registrations through a
personal access token (PAT). To use a PAT instead of a registration
token, place an appropriately scoped PAT in `tokenFile`. If the file
contains a PAT, the configuration script queries a new runner
registration token. Using a runner registration token directly continues
to work as before.
Using the runtime directory as `RUNNER_ROOT` is wrong. We should always
use the state directory like we already do when invoking the runner
configure script. Otherwise, the runner constructs the wrong path for
some files (.credentials, .runner, ...).
make (almost) all links appear on only a single line, with no
unnecessary whitespace, using double quotes for attributes. this lets us
automatically convert them to markdown easily.
the few remaining links are extremely long link in a gnome module, we'll
come back to those at a later date.
the conversion procedure is simple:
- find all things that look like options, ie calls to either `mkOption`
or `lib.mkOption` that take an attrset. remember the attrset as the
option
- for all options, find a `description` attribute who's value is not a
call to `mdDoc` or `lib.mdDoc`
- textually convert the entire value of the attribute to MD with a few
simple regexes (the set from mdize-module.sh)
- if the change produced a change in the manual output, discard
- if the change kept the manual unchanged, add some text to the
description to make sure we've actually found an option. if the
manual changes this time, keep the converted description
this procedure converts 80% of nixos options to markdown. around 2000
options remain to be inspected, but most of those fail the "does not
change the manual output check": currently the MD conversion process
does not faithfully convert docbook tags like <code> and <package>, so
any option using such tags will not be converted at all.
This change allows detecting configuration errors during
switch-to-configuration instead of them being reported asynchronously
*after* switch-to-configuration has exited.
(And update the NixOS test accordingly.)
The current authentication code is broken against newer jenkins:
jenkins-job-builder-start[1257]: Asking Jenkins to reload config
jenkins-start[789]: 2022-07-12 14:34:31.148+0000 [id=17] WARNING hudson.security.csrf.CrumbFilter#doFilter: Found invalid crumb 31e96e52938b51f099a61df9505a4427cb9dca7e35192216755659032a4151df. If you are calling this URL with a script, please use the API Token instead. More information: https://www.jenkins.io/redirect/crumb-cannot-be-used-for-script
jenkins-start[789]: 2022-07-12 14:34:31.160+0000 [id=17] WARNING hudson.security.csrf.CrumbFilter#doFilter: No valid crumb was included in request for /reload by admin. Returning 403.
jenkins-job-builder-start[1357]: curl: (22) The requested URL returned error: 403
Fix it by using `jenkins-cli` instead of messing with `curl`.
This rewrite also prevents leaking the password in process listings. (We
could probably do it without `replace-secret`, assuming `printf` is a
shell built-in, but this implementation should be safe even with shells
not having a built-in `printf`.)
Ref https://github.com/NixOS/nixpkgs/issues/156400.
The description for the runner in the UI is by default sthg like
"npm_nixos_d0544ed48909" i.e., the name of the attribute.
I wanted to have a more user-friendly description and added a
description to the service.
Seems like gitlab-runner doesn't like having both fields set:
"Cannot use two forms of the same flag: description name"
so I used one or the other.
Installs Java into the Jenkins agent and allows specifying the JDK/JRE package to use. This is necessary as Jenkins verifies if the agent contains Java installed through the java -fullversion command, which if not, the connection will fail.
We spent a whole afternoon debugging this, because upstream has very
bad software quality and the error messages were incredibly
misleading.
So let’s document it for the sanity of other people.
Btw, I think the implementation of our module is pretty brittle,
especially the part about diffing tokens to check whether they
changed. We should rather just request a new builder registration
every time, it’s not that much overhead, and always set `replace` so
it is idempotent.
The `nix.*` options, apart from options for setting up the
daemon itself, currently provide a lot of setting mappings
for the Nix daemon configuration. The scope of the mapping yields
convience, but the line where an option is considered essential
is blurry. For instance, the `extra-sandbox-paths` mapping is
provided without its primary consumer, and the corresponding
`sandbox-paths` option is also not mapped.
The current system increases the maintenance burden as maintainers have to
closely follow upstream changes. In this case, there are two state versions
of Nix which have to be maintained collectively, with different options
avaliable.
This commit aims to following the standard outlined in RFC 42[1] to
implement a structural setting pattern. The Nix configuration is encoded
at its core as key-value pairs which maps nicely to attribute sets, making
it feasible to express in the Nix language itself. Some existing options are
kept such as `buildMachines` and `registry` which present a simplified interface
to managing the respective settings. The interface is exposed as `nix.settings`.
Legacy configurations are mapped to their corresponding options under `nix.settings`
for backwards compatibility.
Various options settings in other nixos modules and relevant tests have been
updated to use structural setting for consistency.
The generation and validation of the configration file has been modified to
use `writeTextFile` instead of `runCommand` for clarity. Note that validation
is now mandatory as strict checking of options has been pushed down to the
derivation level due to freeformType consuming unmatched options. Furthermore,
validation can not occur when cross-compiling due to current limitations.
A new option `publicHostKey` was added to the `buildMachines`
submodule corresponding to the base64 encoded public host key settings
exposed in the builder syntax. The build machine generation was subsequently
rewritten to use `concatStringsSep` for better performance by grouping
concatenations.
[1] - https://github.com/NixOS/rfcs/blob/master/rfcs/0042-config-option.md
adds defaultText for all options that use `cfg.*` values in their
defaults, but only for interpolations with no extra processing (other
than toString where necessary)
This commit changes how we deal with the current token, i.e., the token
which may exist from a previous runner registration, and the configured
token, i.e., the path set for the respective NixOS configuration option.
Until now, we copied the configured and the current token (if any) to
the runtime directory to compare them. The path of the current token may
reference a file which is only accessible to specific users (even only
root). Therefore, we ran the copying of credentials with elevated
privileges by prefixing the `ExecStartPre=` script with a `+` (see
systemd.service(5)). In this script, we also changed the owner of the
files to the service user. Apparently, however, the user/group pair
sometimes did not exist because we use `DynamicUser=`.
To address this issue, we no longer change the owner of the file.
Instead, we change the file permissions to 0666 to allow the runner
configuration script (runs with full sandboxing) to read-write the file.
Due to the current permissions of the runtime directory (0755), this
would expose the token. Therefore, we process the tokens in the state
directory, which is only accessible to the service user.
If a new token file exists in the state directory, the configuration
script should trigger a new runner registration. Afterward, it deletes
the new token file. The token is still available using the path of the
current token which is inaccessible within the service's sandbox.
This addresses #120263 in part, by allowing users to override the
github-runner derivation that is bound to turn non-functional via the
self-update mechanism. (And it'll allow using a buildFHSUserEnv-based
derivation, if someone ends up building that!)
Add support for folder jobs
(https://plugins.jenkins.io/cloudbees-folder/) by reworking the service
to support nested jobs.
This also fixes this deprecation warning (as a happy side effect):
WARNING:jenkins_jobs.cli.subcommand.test:(Deprecated) The default output behavior of `jenkins-jobs test` when given the --output flag will change in JJB 3.0. Instead of writing jobs to OUTPUT/jobname; they will be written to OUTPUT/jobname/config.xml. The new behavior can be enabled by the passing `--config-xml` parameter
While looking at the sphinx package I noticed it was heavily
undermaintained, which is when we noticed nand0p has been inactive for
roughly 18 months. It is therefore prudent to assume they will not be
maintaining their packages, modules and tests.
- Their last contribution to nixpkgs was in 2019/12
- On 2021/05/08 I wrote them an email to the address listed in the
maintainer-list, which they didn't reply to.
The buildkite agent supports multiple tags with the same key. This
functionality is used to have a [single agent listen on multiple
queues](https://buildkite.com/docs/agent/v3/queues#setting-an-agents-queue).
However, having the tags be of type `attrsOf str` means that
we cannot suport this use case. This commit modifies the type
of tags to be `attrsOf (either str (listOf str))` where the list
is expanded into multiple tags with the same key.
Example:
```
{tags = {queue = ["default", "testing"];};}
```
generates
```
tags="queue=default,queue=testing"
```
in the buildkite agent configuration.
* github-runner: init at 2.277.1
* nixos/github-runner: initial version
* nixos/github-runner: add warning if tokenFile in Nix store
* github-runner: don't accept unexpected attrs
* github-runner: formatting nits
* github-runner: add pre and post hooks to checkPhase
* nixos/github-runner: update ExecStartPre= comment
* nixos/github-runner: adapt tokenFile option description
Also note that not only a change to the option value will trigger a
reconfiguration but also modifications to the file's content.
* nixos/github-runner: remove mkDefault for DynamicUser=
* nixos/github-runner: create a parent for systemd dirs
Adds a parent directory "github-runner/" to all of the systemd lifecycle
directories StateDirectory=, RuntimeDirectory= and LogDirectory=.
Doing this has two motivations:
1. Something like this would required if we want to support multiple
runners configurations. Please note that this is already possible
using NixOS containers.
2. Having an additional parent directory makes it easier to remap
any of the directories. Without a parent, systemd is going to
complain if, for example, the given StateDirectory= is a symlink.
* nixos/github-runner: use specifier to get abs runtime path
* nixos/github-runner: use hostname as default for option `name`
Until now, the runner registration did not set the `--name` argument if
the configuration option was `null`, the default for the option.
According to GitHub's documentation, this instructs the registration
script to use the machine's hostname.
This commit causes the registration to always pass the `--name` argument
to the runner configuration script. The option now defaults to
`networking.hostName` which should be always set on NixOS.
This change becomes necessary as the systemd service name includes the
name of the runner since fcfa809 and, hence, expects it to be set. Thus,
an unset `name` option leads to an error.
* nixos/github-runner: use types.str for `name` option
Forcing a `name` option to comply with a pattern which could also be
used as a hostname is probably not required by GitHub.
* nixos/github-runner: pass dir paths explicitly for ExecStartPre=
* nixos/github-runner: update variable and script naming
* nixos/github-runner: let systemd choose the user/group
User and group naming restrictions are a complex topic [1] that I don't
even want to touch. Let systemd figure out the username and group and
reference it in our scripts through the USER environment variable.
[1] https://systemd.io/USER_NAMES/
* Revert "nixos/github-runner: use types.str for `name` option"
The escaping applied to the subdirectory paths given to StateDirectory=,
RuntimeDirectory= and LogsDirectory= apparently doesn't use the same
strategy that is used to escape unit names (cf. systemd-escape(1)). This
makes it unreasonably hard to construct reliable paths which work for
StateDirectory=/RuntimeDirectory=/LogsDirectory= and ExecStartPre=.
Against this background, I decided to (re-)apply restrictions to the
name a user might give for the GitHub runner. The pattern for
`networking.hostName` seems like a reasonable choice, also as its value
is the default if the `name` option isn't set.
This reverts commit 193ac67ba337990c22126da24a775c497dbc7e7d.
* nixos/github-runner: use types.path for `tokenFile` option
* nixos/github-runner: escape options used as shell arguments
* nixos/github-runner: wait for network-online.target
* github-runner: ignore additional online tests
This will make it easier to track specifically where queries are being
made from (assuming a `log_line_prefix` that includes `%a` in the
postgres configuration).
These were broken since 2016:
f0367da7d1
since StartLimitIntervalSec got moved into [Unit] from [Service].
StartLimitBurst has also been moved accordingly, so let's fix that one
too.
NixOS systems have been producing logs such as:
/nix/store/wf98r55aszi1bkmln1lvdbp7znsfr70i-unit-caddy.service/caddy.service:31:
Unknown key name 'StartLimitIntervalSec' in section 'Service', ignoring.
I have also removed some unnecessary duplication in units disabling
rate limiting since setting either interval or burst to zero disables it
(ad16158c10/src/basic/ratelimit.c (L16))
This should NOT be backported to 20.09!
When 21.03 is released, the DB changes are about a year old and
operators had two release cycles for the upgrade. At this point it
should be fair to remove the compat layer to reduce the complexity of
the module itself.
In the current implementation, there's no possibility to modify the default
parameter for keepalive. This is a number that indicates how frequently
keepalive messages should be sent from the worker to the buildmaster,
expressed in seconds. The default (600) causes a message to be sent to
the buildmaster at least once every 10 minutes.
If the worker is behind a NAT box or stateful firewall, these messages
may help to keep the connection alive: some NAT boxes tend to forget about
a connection if it has not been used in a while. When this happens, the
buildmaster will think that the worker has disappeared, and builds will
time out. Meanwhile the worker will not realize than anything is wrong.
Since Buildbot 0.9.0, status targets were deprecated and ignored.
There's a very small line on startup explaining that, and status simply
isn't reported. Avoid others the same headaches, and do it right in the
NixOS module.
As there might have been changes in the way reporters are organized, and
configuration might need to be migrated remove the old option, and not
just provide an alias.
It's pbPort, and it's also a connection string, meaning
listen-on-localhost is also possible. Provide an alias for the old
option name, so old configs still work.
We already set the relevant env vars in the systemd services. That does
not help one when executing any of the executables outside a service,
e.g. when creating a new user.
Also removed `pkgs.hydra-flakes` since flake-support has been merged
into master[1]. Because of that, `pkgs.hydra-unstable` is now compiled
against `pkgs.nixFlakes` and currently requires a patch since Hydra's
master doesn't compile[2] atm.
[1] https://github.com/NixOS/hydra/pull/730
[2] https://github.com/NixOS/hydra/pull/732
Upgrades Hydra to the latest master/flake branch. To perform this
upgrade, it's needed to do a non-trivial db-migration which provides a
massive performance-improvement[1].
The basic ideas behind multi-step upgrades of services between NixOS versions
have been gathered already[2]. For further context it's recommended to
read this first.
Basically, the following steps are needed:
* Upgrade to a non-breaking version of Hydra with the db-changes
(columns are still nullable here). If `system.stateVersion` is set to
something older than 20.03, the package will be selected
automatically, otherwise `pkgs.hydra-migration` needs to be used.
* Run `hydra-backfill-ids` on the server.
* Deploy either `pkgs.hydra-unstable` (for Hydra master) or
`pkgs.hydra-flakes` (for flakes-support) to activate the optimization.
The steps are also documented in the release-notes and in the module
using `warnings`.
`pkgs.hydra` has been removed as latest Hydra doesn't compile with
`pkgs.nixStable` and to ensure a graceful migration using the newly
introduced packages.
To verify the approach, a simple vm-test has been added which verifies
the migration steps.
[1] https://github.com/NixOS/hydra/pull/711
[2] https://github.com/NixOS/nixpkgs/pull/82353#issuecomment-598269471
* nixos/buildkite: drop user option
This reverts 8c6b1c3eaa.
Turns out, buildkite-agent has logic to write .ssh/known_hosts files and
only really works when $HOME and the user homedir are in sync.
On top of that, we provision ssh keys in /var/lib/buildkite-agent, which
doesn't work if that other users' homedir points elsewhere (we can cheat
by setting $HOME, but then getent and $HOME provide conflicting
results).
So after all, it's better to only run the system-wide buildkite agent as
the "buildkite-agent" user only - if one wants to run buildkite as
different users, systemd user services might be a better fit.
* nixosTests.buildkite-agent: add node with separate user and no ssh key
This gets passed to BUILDKITE_SHELL, which will specify the shell being
used to executes script in.
Defaults to `${pkgs.bash}/bin/bash -e -c`, matching how buildkite
behaves on other distros.
SSH public keys aren't needed to clone private repos, and if we only
need to configure a single attribute, there's no need for the "openssh"
attrset anymore.
This applies [hydra PR #432](https://github.com/NixOS/hydra/pull/432)
to the NixOS module in nixpkgs:
```
commit 4efd078977e5ea20e1104783efc324cba11690bc
Author: Bas van Dijk <v.dijk.bas@gmail.com>
Date: Sun Dec 11 15:35:38 2016 +0100
Only set buildMachinesFiles when nix.buildMachines is defined
```
The following commit from 2016 in hydra removed the `--option
build-use-substitutes` from the hydra-queue-runner service:
```
commit ee2e9f5335c8c0288c102975b506f6b275793cfe
Author: Eelco Dolstra <edolstra@gmail.com>
Date: Fri Oct 7 20:23:05 2016 +0200
Update to reflect BinaryCacheStore changes
BinaryCacheStore no longer implements buildPaths() and ensurePath(),
so we need to use copyPath() / copyClosure().
```
It would be better if the hydra module in NixOS matches the upstream
module.
During the last update, `hydra-notify` was rewritten as a daemon which
listens to postgresql notifications for each build[1]. The module
uses the `hydra-notify.service` unit from upstream's Hydra module and
the VM test ensures that email notifications are sent properly.
Also updated `hydra-init.service` to install `pg_trgm` on a local
database if needed[2].
[1] c7861b85c4
[2] 8a0a5ec3a3
Currently there are two calls to curl in the reloadScript, neither which
check for errors. If something is misconfigured (like wrong authToken),
the only trace that something wrong happened is this log message:
Asking Jenkins to reload config
<h1>Bad Message 400</h1><pre>reason: Illegal character VCHAR='<'</pre>
The service isn't marked as failed, so it's easy to miss.
Fix it by passing --fail to curl.
While at it:
* Add $curl_opts and $jenkins_url variables to keep the curl command
lines DRY.
* Add --show-error to curl to show short error message explanation when
things go wrong (like HTTP 401 error).
* Lower-case the $CRUMB variable as upper case is for exported environment
variables.
The new behaviour, when having wrong accessToken:
Asking Jenkins to reload config
curl: (22) The requested URL returned error: 401
And the service is clearly marked as failed in `systemctl --failed`.
@cleverca found this bug in the declarative hooks config. Any shell
variables referenced in a hook script would get expanded by the hooks
directory builder.
Prevent variable expansion by quoting the here doc limit string.
