Skip to content

Commit d5d8b98

Browse files
kiriwalawrenkiriwalawren
committed
fix config access race conditions
1 parent 68220ca commit d5d8b98

4 files changed

Lines changed: 162 additions & 40 deletions

File tree

‎modules/restish-wrapper/configGenerator.nix‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{lib}: let
2+
sanitizeVarName = name: lib.replaceStrings ["-"] ["_"] (lib.toUpper name);
3+
4+
generateConfig = services:
5+
lib.concatStringsSep ",\n" (lib.mapAttrsToList (name: svc: let
6+
headerPairs = lib.mapAttrsToList (k: v: ''"${k}": "$HEADER_${sanitizeVarName name}_${sanitizeVarName k}"'') (svc.headers or {});
7+
queryPairs = lib.mapAttrsToList (k: v: ''"${k}": "$QUERY_${sanitizeVarName name}_${sanitizeVarName k}"'') (svc.query or {});
8+
profileParts =
9+
[''"base": "${svc.baseUrl}"'']
10+
++ (lib.optionals ((svc.headers or {}) != {}) [''"headers": { ${lib.concatStringsSep ", " headerPairs} }''])
11+
++ (lib.optionals ((svc.query or {}) != {}) [''"query": { ${lib.concatStringsSep ", " queryPairs} }'']);
12+
in ''
13+
"${name}": {
14+
"base": "${svc.baseUrl}",
15+
"profiles": {
16+
"default": {
17+
${lib.concatStringsSep ",\n" profileParts}
18+
}
19+
}
20+
}'') services);
21+
in {
22+
inherit sanitizeVarName generateConfig;
23+
}

‎modules/restish-wrapper/default.nix‎

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99

1010
enabledServices = lib.attrValues cfg.services;
1111

12-
# Helper to convert header/query names to valid bash variable names
13-
# Converts to uppercase and replaces hyphens with underscores
14-
sanitizeVarName = name: lib.replaceStrings ["-"] ["_"] (lib.toUpper name);
12+
configGenerator = import ./configGenerator.nix {inherit lib;};
13+
inherit (configGenerator) sanitizeVarName generateConfig;
1514

1615
restishWrapper = pkgs.writeShellScriptBin "restish" ''
1716
set -euo pipefail
@@ -24,7 +23,6 @@
2423
mkdir -p "$RESTISH_CONFIG_DIR"
2524
2625
${lib.optionalString ((lib.length enabledServices) > 0) ''
27-
# Helper function to read value from file if it exists, otherwise use as-is
2826
read_value() {
2927
local value="$1"
3028
if [ -f "$value" ]; then
@@ -34,53 +32,34 @@
3432
fi
3533
}
3634
37-
# Build JSON config using jq with file locking to prevent concurrent writes
3835
CONFIG_FILE="$RESTISH_CONFIG_DIR/apis.json"
3936
LOCK_FILE="$RESTISH_CONFIG_DIR/apis.json.lock"
4037
41-
# Acquire exclusive lock (will wait if another process has it)
42-
exec 200>"$LOCK_FILE"
43-
${pkgs.util-linux}/bin/flock 200
38+
(
39+
${pkgs.util-linux}/bin/flock -x 200
40+
4441
${lib.concatStringsSep "\n" (lib.mapAttrsToList (name: svc: ''
45-
${lib.concatStringsSep "\n" (lib.mapAttrsToList (k: v: ''
46-
export HEADER_${sanitizeVarName name}_${sanitizeVarName k}=$(read_value "${v}")
47-
'')
48-
svc.headers)}
49-
${lib.concatStringsSep "\n" (lib.mapAttrsToList (k: v: ''
50-
export QUERY_${sanitizeVarName name}_${sanitizeVarName k}=$(read_value "${v}")
51-
'')
52-
svc.query)}
53-
'')
54-
cfg.services)}
42+
${lib.concatStringsSep "\n" (lib.mapAttrsToList (k: v: ''
43+
export HEADER_${sanitizeVarName name}_${sanitizeVarName k}=$(read_value "${v}")
44+
'')
45+
(svc.headers or {}))}
46+
${lib.concatStringsSep "\n" (lib.mapAttrsToList (k: v: ''
47+
export QUERY_${sanitizeVarName name}_${sanitizeVarName k}=$(read_value "${v}")
48+
'')
49+
(svc.query or {}))}
50+
'')
51+
cfg.services)}
5552
5653
cat > "$CONFIG_FILE" <<'EOF'
5754
{
58-
${lib.concatStringsSep ",\n" (lib.mapAttrsToList (name: svc: let
59-
headerPairs = lib.mapAttrsToList (k: v: ''"${k}": "$HEADER_${sanitizeVarName name}_${sanitizeVarName k}"'') svc.headers;
60-
queryPairs = lib.mapAttrsToList (k: v: ''"${k}": "$QUERY_${sanitizeVarName name}_${sanitizeVarName k}"'') svc.query;
61-
profileParts =
62-
(lib.optionals (svc.headers != {}) [''"headers": { ${lib.concatStringsSep ", " headerPairs} }''])
63-
++ (lib.optionals (svc.query != {}) [''"query": { ${lib.concatStringsSep ", " queryPairs} }'']);
64-
in ''
65-
"${name}": {
66-
"base": "${svc.baseUrl}",
67-
"profiles": {
68-
"default": {
69-
${lib.concatStringsSep ",\n" profileParts}
70-
}
71-
}
72-
}'')
73-
cfg.services)}
55+
${generateConfig cfg.services}
7456
}
75-
EOF
57+
EOF
7658
77-
# Substitute environment variables
7859
TEMP_CONFIG=$(mktemp)
7960
${pkgs.envsubst}/bin/envsubst < "$CONFIG_FILE" > "$TEMP_CONFIG"
8061
mv "$TEMP_CONFIG" "$CONFIG_FILE"
81-
82-
# Release lock
83-
exec 200>&-
62+
) 200>"$LOCK_FILE"
8463
''}
8564
8665
exec ${pkgs.restish}/bin/restish "$@"

‎tests/unit-tests/default.nix‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@
2020
++ modules;
2121
};
2222

23-
# Test helper to assert conditions
2423
assertTest = name: cond:
2524
pkgs.runCommand "unit-test-${name}" {} ''
2625
${lib.optionalString (!cond) "echo 'FAIL: ${name}' && exit 1"}
2726
echo 'PASS: ${name}' > $out
2827
'';
28+
29+
restishConfigTest = import ./restish-config.nix {inherit pkgs lib;};
2930
in {
31+
restish-config-generation = assertTest "restish-config-generation" restishConfigTest.allPass;
3032
# Test that nixflix.sonarr options generate correct systemd units
3133
sonarr-service-generation = let
3234
config = evalConfig [

‎tests/unit-tests/restish-config.nix‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
{
2+
pkgs ? import <nixpkgs> {},
3+
lib ? pkgs.lib,
4+
}: let
5+
configGenerator = import ../../modules/restish-wrapper/configGenerator.nix {inherit lib;};
6+
inherit (configGenerator) generateConfig;
7+
8+
testCases = {
9+
withHeadersOnly = {
10+
services = {
11+
sonarr = {
12+
baseUrl = "http://127.0.0.1:8989/api/v3";
13+
headers."X-Api-Key" = "/path/to/key";
14+
};
15+
};
16+
expected = builtins.fromJSON ''
17+
{
18+
"sonarr": {
19+
"base": "http://127.0.0.1:8989/api/v3",
20+
"profiles": {
21+
"default": {
22+
"base": "http://127.0.0.1:8989/api/v3",
23+
"headers": { "X-Api-Key": "$HEADER_SONARR_X_API_KEY" }
24+
}
25+
}
26+
}
27+
}
28+
'';
29+
};
30+
31+
withQueryOnly = {
32+
services = {
33+
sabnzbd = {
34+
baseUrl = "http://127.0.0.1:8080/api";
35+
query = {
36+
apikey = "/path/to/apikey";
37+
output = "json";
38+
};
39+
};
40+
};
41+
expected = builtins.fromJSON ''
42+
{
43+
"sabnzbd": {
44+
"base": "http://127.0.0.1:8080/api",
45+
"profiles": {
46+
"default": {
47+
"base": "http://127.0.0.1:8080/api",
48+
"query": { "apikey": "$QUERY_SABNZBD_APIKEY", "output": "$QUERY_SABNZBD_OUTPUT" }
49+
}
50+
}
51+
}
52+
}
53+
'';
54+
};
55+
56+
withBoth = {
57+
services = {
58+
test = {
59+
baseUrl = "http://example.com/api";
60+
headers."X-Api-Key" = "/path/to/key";
61+
query.format = "json";
62+
};
63+
};
64+
expected = builtins.fromJSON ''
65+
{
66+
"test": {
67+
"base": "http://example.com/api",
68+
"profiles": {
69+
"default": {
70+
"base": "http://example.com/api",
71+
"headers": { "X-Api-Key": "$HEADER_TEST_X_API_KEY" },
72+
"query": { "format": "$QUERY_TEST_FORMAT" }
73+
}
74+
}
75+
}
76+
}
77+
'';
78+
};
79+
80+
multipleServices = {
81+
services = {
82+
prowlarr = {
83+
baseUrl = "http://127.0.0.1:9696/api/v1";
84+
headers."X-Api-Key" = "/path/to/prowlarr-key";
85+
};
86+
sonarr = {
87+
baseUrl = "http://127.0.0.1:8989/api/v3";
88+
headers."X-Api-Key" = "/path/to/sonarr-key";
89+
};
90+
};
91+
};
92+
};
93+
94+
runTest = name: testCase: let
95+
generatedContent = generateConfig testCase.services;
96+
generated = ''
97+
{
98+
${generatedContent}
99+
}
100+
'';
101+
parsed = builtins.fromJSON generated;
102+
in {
103+
inherit name;
104+
generated = generated;
105+
parsed = parsed;
106+
success =
107+
if testCase ? expected
108+
then parsed == testCase.expected
109+
else true;
110+
};
111+
112+
results = lib.mapAttrs runTest testCases;
113+
in {
114+
inherit results;
115+
116+
allConfigs = lib.mapAttrs (name: test: test.generated) results;
117+
allPass = lib.all (test: test.success) (lib.attrValues results);
118+
}

0 commit comments

Comments
 (0)