Skip to content

Commit ef5a903

Browse files
fix(zed): fix settings JSON parsing with base64 encoding (#604)
## Problem The Zed module's settings parsing was broken. The previous implementation used quote escaping: ```hcl SETTINGS_JSON='${replace(var.settings, "\"", "\\\"")}' ``` This produced invalid JSON like `{\"theme\":\"dark\"}` which **jq could not parse** because the backslash-escaped quotes are not valid JSON syntax. ## Solution Changed to use base64 encoding internally: ```hcl locals { settings_b64 = var.settings != "" ? base64encode(var.settings) : "" } # In the script: SETTINGS_B64='${local.settings_b64}' SETTINGS_JSON="$(echo -n "${SETTINGS_B64}" | base64 -d)" ``` **User interface remains the same** - users still provide plain JSON via `jsonencode()` or heredoc: ```hcl module "zed" { source = "..." agent_id = coder_agent.main.id settings = jsonencode({ theme = "dark" fontSize = 14 }) } ``` ## Testing Added comprehensive tests: **Terraform tests (5):** - URL generation (default, folder, agent_name) - Settings base64 encoding verification - Empty settings edge case **Container e2e tests (3):** - Creates settings file with correct JSON (including special chars: quotes, backslashes, URLs) - Merges with existing settings via jq - Exits early with empty settings Also fixed existing tests to use `override_data` for proper workspace mocking. --------- Signed-off-by: Muhammad Atif Ali <[email protected]> Co-authored-by: DevCats <[email protected]>
1 parent 4b7128b commit ef5a903

File tree

4 files changed

+181
-49
lines changed

4 files changed

+181
-49
lines changed

registry/coder/modules/zed/README.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ Zed is a high-performance, multiplayer code editor from the creators of Atom and
1919
module "zed" {
2020
count = data.coder_workspace.me.start_count
2121
source = "registry.coder.com/coder/zed/coder"
22-
version = "1.1.3"
22+
version = "1.1.4"
2323
agent_id = coder_agent.main.id
2424
}
2525
```
@@ -32,7 +32,7 @@ module "zed" {
3232
module "zed" {
3333
count = data.coder_workspace.me.start_count
3434
source = "registry.coder.com/coder/zed/coder"
35-
version = "1.1.3"
35+
version = "1.1.4"
3636
agent_id = coder_agent.main.id
3737
folder = "/home/coder/project"
3838
}
@@ -44,7 +44,7 @@ module "zed" {
4444
module "zed" {
4545
count = data.coder_workspace.me.start_count
4646
source = "registry.coder.com/coder/zed/coder"
47-
version = "1.1.3"
47+
version = "1.1.4"
4848
agent_id = coder_agent.main.id
4949
display_name = "Zed Editor"
5050
order = 1
@@ -57,7 +57,7 @@ module "zed" {
5757
module "zed" {
5858
count = data.coder_workspace.me.start_count
5959
source = "registry.coder.com/coder/zed/coder"
60-
version = "1.1.3"
60+
version = "1.1.4"
6161
agent_id = coder_agent.main.id
6262
agent_name = coder_agent.example.name
6363
}
@@ -73,7 +73,7 @@ You can declaratively set/merge settings with the `settings` input. Provide a JS
7373
module "zed" {
7474
count = data.coder_workspace.me.start_count
7575
source = "registry.coder.com/coder/zed/coder"
76-
version = "1.1.3"
76+
version = "1.1.4"
7777
agent_id = coder_agent.main.id
7878
7979
settings = jsonencode({
@@ -85,6 +85,7 @@ module "zed" {
8585
env = {}
8686
}
8787
88+
8889
}
8990
})
9091
}
Lines changed: 95 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, it } from "bun:test";
22
import {
3+
execContainer,
4+
findResourceInstance,
5+
removeContainer,
6+
runContainer,
37
runTerraformApply,
48
runTerraformInit,
59
testRequiredVariables,
@@ -12,66 +16,114 @@ describe("zed", async () => {
1216
agent_id: "foo",
1317
});
1418

15-
it("default output", async () => {
19+
it("creates settings file with correct JSON", async () => {
20+
const settings = {
21+
theme: "One Dark",
22+
buffer_font_size: 14,
23+
vim_mode: true,
24+
telemetry: {
25+
diagnostics: false,
26+
metrics: false,
27+
},
28+
// Test special characters: single quotes, backslashes, URLs
29+
message: "it's working",
30+
path: "C:\\Users\\test",
31+
api_url: "https://api.example.com/v1?token=abc&user=test",
32+
};
33+
1634
const state = await runTerraformApply(import.meta.dir, {
1735
agent_id: "foo",
36+
settings: JSON.stringify(settings),
1837
});
19-
expect(state.outputs.zed_url.value).toBe("zed://ssh/default.coder");
2038

21-
const coder_app = state.resources.find(
22-
(res) => res.type === "coder_app" && res.name === "zed",
23-
);
39+
const instance = findResourceInstance(state, "coder_script");
40+
const id = await runContainer("alpine:latest");
2441

25-
expect(coder_app).not.toBeNull();
26-
expect(coder_app?.instances.length).toBe(1);
27-
expect(coder_app?.instances[0].attributes.order).toBeNull();
28-
});
42+
try {
43+
const result = await execContainer(id, ["sh", "-c", instance.script]);
44+
expect(result.exitCode).toBe(0);
2945

30-
it("adds folder", async () => {
31-
const state = await runTerraformApply(import.meta.dir, {
32-
agent_id: "foo",
33-
folder: "/foo/bar",
34-
});
35-
expect(state.outputs.zed_url.value).toBe("zed://ssh/default.coder/foo/bar");
36-
});
46+
const catResult = await execContainer(id, [
47+
"cat",
48+
"/root/.config/zed/settings.json",
49+
]);
50+
expect(catResult.exitCode).toBe(0);
3751

38-
it("expect order to be set", async () => {
39-
const state = await runTerraformApply(import.meta.dir, {
40-
agent_id: "foo",
41-
order: "22",
42-
});
52+
const written = JSON.parse(catResult.stdout.trim());
53+
expect(written).toEqual(settings);
54+
} finally {
55+
await removeContainer(id);
56+
}
57+
}, 30000);
4358

44-
const coder_app = state.resources.find(
45-
(res) => res.type === "coder_app" && res.name === "zed",
46-
);
59+
it("merges settings with existing file when jq available", async () => {
60+
const existingSettings = {
61+
theme: "Solarized Dark",
62+
vim_mode: true,
63+
};
4764

48-
expect(coder_app).not.toBeNull();
49-
expect(coder_app?.instances.length).toBe(1);
50-
expect(coder_app?.instances[0].attributes.order).toBe(22);
51-
});
65+
const newSettings = {
66+
theme: "One Dark",
67+
buffer_font_size: 14,
68+
};
5269

53-
it("expect display_name to be set", async () => {
5470
const state = await runTerraformApply(import.meta.dir, {
5571
agent_id: "foo",
56-
display_name: "Custom Zed",
72+
settings: JSON.stringify(newSettings),
5773
});
5874

59-
const coder_app = state.resources.find(
60-
(res) => res.type === "coder_app" && res.name === "zed",
61-
);
75+
const instance = findResourceInstance(state, "coder_script");
76+
const id = await runContainer("alpine:latest");
6277

63-
expect(coder_app).not.toBeNull();
64-
expect(coder_app?.instances.length).toBe(1);
65-
expect(coder_app?.instances[0].attributes.display_name).toBe("Custom Zed");
66-
});
78+
try {
79+
// Install jq and create existing settings file
80+
await execContainer(id, ["apk", "add", "--no-cache", "jq"]);
81+
await execContainer(id, ["mkdir", "-p", "/root/.config/zed"]);
82+
await execContainer(id, [
83+
"sh",
84+
"-c",
85+
`echo '${JSON.stringify(existingSettings)}' > /root/.config/zed/settings.json`,
86+
]);
87+
88+
const result = await execContainer(id, ["sh", "-c", instance.script]);
89+
expect(result.exitCode).toBe(0);
90+
91+
const catResult = await execContainer(id, [
92+
"cat",
93+
"/root/.config/zed/settings.json",
94+
]);
95+
expect(catResult.exitCode).toBe(0);
96+
97+
const merged = JSON.parse(catResult.stdout.trim());
98+
expect(merged.theme).toBe("One Dark"); // overwritten
99+
expect(merged.buffer_font_size).toBe(14); // added
100+
expect(merged.vim_mode).toBe(true); // preserved
101+
} finally {
102+
await removeContainer(id);
103+
}
104+
}, 30000);
67105

68-
it("adds agent_name to hostname", async () => {
106+
it("exits early with empty settings", async () => {
69107
const state = await runTerraformApply(import.meta.dir, {
70108
agent_id: "foo",
71-
agent_name: "myagent",
109+
settings: "",
72110
});
73-
expect(state.outputs.zed_url.value).toBe(
74-
"zed://ssh/myagent.default.default.coder",
75-
);
76-
});
111+
112+
const instance = findResourceInstance(state, "coder_script");
113+
const id = await runContainer("alpine:latest");
114+
115+
try {
116+
const result = await execContainer(id, ["sh", "-c", instance.script]);
117+
expect(result.exitCode).toBe(0);
118+
119+
// Settings file should not be created
120+
const catResult = await execContainer(id, [
121+
"cat",
122+
"/root/.config/zed/settings.json",
123+
]);
124+
expect(catResult.exitCode).not.toBe(0);
125+
} finally {
126+
await removeContainer(id);
127+
}
128+
}, 30000);
77129
});

registry/coder/modules/zed/main.tf

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ locals {
6565
owner_name = lower(data.coder_workspace_owner.me.name)
6666
agent_name = lower(var.agent_name)
6767
hostname = var.agent_name != "" ? "${local.agent_name}.${local.workspace_name}.${local.owner_name}.coder" : "${local.workspace_name}.coder"
68+
settings_b64 = var.settings != "" ? base64encode(var.settings) : ""
6869
}
6970

7071
resource "coder_script" "zed_settings" {
@@ -75,7 +76,11 @@ resource "coder_script" "zed_settings" {
7576
script = <<-EOT
7677
#!/usr/bin/env bash
7778
set -eu
78-
SETTINGS_JSON='${replace(var.settings, "\"", "\\\"")}'
79+
SETTINGS_B64='${local.settings_b64}'
80+
if [ -z "$${SETTINGS_B64}" ]; then
81+
exit 0
82+
fi
83+
SETTINGS_JSON="$(echo -n "$${SETTINGS_B64}" | base64 -d)"
7984
if [ -z "$${SETTINGS_JSON}" ] || [ "$${SETTINGS_JSON}" = "{}" ]; then
8085
exit 0
8186
fi

registry/coder/modules/zed/zed.tftest.hcl

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,20 @@ run "default_output" {
55
agent_id = "foo"
66
}
77

8+
override_data {
9+
target = data.coder_workspace.me
10+
values = {
11+
name = "default"
12+
}
13+
}
14+
15+
override_data {
16+
target = data.coder_workspace_owner.me
17+
values = {
18+
name = "default"
19+
}
20+
}
21+
822
assert {
923
condition = output.zed_url == "zed://ssh/default.coder"
1024
error_message = "zed_url did not match expected default URL"
@@ -19,6 +33,20 @@ run "adds_folder" {
1933
folder = "/foo/bar"
2034
}
2135

36+
override_data {
37+
target = data.coder_workspace.me
38+
values = {
39+
name = "default"
40+
}
41+
}
42+
43+
override_data {
44+
target = data.coder_workspace_owner.me
45+
values = {
46+
name = "default"
47+
}
48+
}
49+
2250
assert {
2351
condition = output.zed_url == "zed://ssh/default.coder/foo/bar"
2452
error_message = "zed_url did not include provided folder path"
@@ -33,8 +61,54 @@ run "adds_agent_name" {
3361
agent_name = "myagent"
3462
}
3563

64+
override_data {
65+
target = data.coder_workspace.me
66+
values = {
67+
name = "default"
68+
}
69+
}
70+
71+
override_data {
72+
target = data.coder_workspace_owner.me
73+
values = {
74+
name = "default"
75+
}
76+
}
77+
3678
assert {
3779
condition = output.zed_url == "zed://ssh/myagent.default.default.coder"
3880
error_message = "zed_url did not include agent_name in hostname"
3981
}
4082
}
83+
84+
run "settings_base64_encoding" {
85+
command = apply
86+
87+
variables {
88+
agent_id = "foo"
89+
settings = jsonencode({
90+
theme = "dark"
91+
fontSize = 14
92+
})
93+
}
94+
95+
# Verify settings are base64 encoded (eyJ = base64 prefix for JSON starting with {")
96+
assert {
97+
condition = can(regex("SETTINGS_B64='eyJ", coder_script.zed_settings.script))
98+
error_message = "settings should be base64 encoded in the script"
99+
}
100+
}
101+
102+
run "empty_settings" {
103+
command = apply
104+
105+
variables {
106+
agent_id = "foo"
107+
settings = ""
108+
}
109+
110+
assert {
111+
condition = can(regex("SETTINGS_B64=''", coder_script.zed_settings.script))
112+
error_message = "empty settings should result in empty SETTINGS_B64"
113+
}
114+
}

0 commit comments

Comments
 (0)