Skip to content

Commit e87198a

Browse files
gpandersgpanders
authored
Fix container diffs when using JSONC config (#10009)
The diff printing code contains a number of issues: 1. It is replicated between cloudchamber/apply.ts and containers/deploy.ts. * Move all of the diff printing code into cloudchamber/helpers/diff.ts. At the same time, we refactor the diff code to remove the singleton static "Diff" object and move methods related to generating the diff into the Diff class. The diff generation code comes from a 3rd party BSD-3 licensed repo, so we also include the license in the file to adhere to the license requirements. 2. It is tailored specifically for TOML files. At the time this was written, wrangler.toml was the primary/default config file format. That is no longer the case, and increasingly users are using wrangler.jsonc. JSONC files were not formatted correctly at all: they had no proper indentation and the diff printing code didn't properly account for the "context" around changed lines. * The solution here is to eliminate the TOML specific code completely and have the diffing code just diff plain strings. This means we lose the syntax highlighting for TOML, however I think this is acceptable since we do not also offer syntax highlighting for JSON, and providing the syntax highlighting requires a lot of code (it also only works in specific scenarios and hard codes color codes which may not match the user's terminal colorscheme, which is an accessibility issue). The diff code now just prints 3 lines of context before and after each changed chunk, without attempting to parse anything. 3. The helpers/diff.ts file contains some functions which are unrelated to generating diffs. * Create a new file `src/utils/sortObjectRecursive.ts` that contains these functions.
1 parent d82c8e8 commit e87198a

File tree

7 files changed

+646
-673
lines changed

7 files changed

+646
-673
lines changed

‎.changeset/sour-areas-lose.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
Fix containers diff output when using JSONC config files

‎packages/wrangler/src/__tests__/cloudchamber/apply.test.ts‎

Lines changed: 118 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,12 @@ describe("cloudchamber apply", () => {
130130
│ instances = 3
131131
│ scheduling_policy = \\"default\\"
132132
│
133-
│ [containers.constraints]
134-
│ tier = 2
133+
│ [containers.constraints]
134+
│ tier = 2
135135
│
136-
│ [containers.configuration]
137-
│ image = \\"./Dockerfile\\"
138-
│ instance_type = \\"dev\\"
136+
│ [containers.configuration]
137+
│ image = \\"./Dockerfile\\"
138+
│ instance_type = \\"dev\\"
139139
│
140140
│
141141
│  SUCCESS  Created application my-container-app (Application ID: abc)
@@ -199,10 +199,15 @@ describe("cloudchamber apply", () => {
199199
│ - instances = 3
200200
│ + instances = 4
201201
│ name = \\"my-container-app\\"
202+
│ scheduling_policy = \\"default\\"
203+
│
204+
│ ...
205+
│
206+
│ instance_type = \\"dev\\"
202207
│
203-
│ [containers.constraints]
204-
│ - tier = 3
205-
│ + tier = 2
208+
│ [containers.constraints]
209+
│ - tier = 3
210+
│ + tier = 2
206211
│
207212
│
208213
│  SUCCESS  Modified application my-container-app
@@ -278,6 +283,7 @@ describe("cloudchamber apply", () => {
278283
│ - max_instances = 4
279284
│ + max_instances = 3
280285
│ name = \\"my-container-app\\"
286+
│ scheduling_policy = \\"default\\"
281287
│
282288
├ NEW my-container-app-2
283289
│
@@ -286,17 +292,16 @@ describe("cloudchamber apply", () => {
286292
│ max_instances = 3
287293
│ scheduling_policy = \\"default\\"
288294
│
289-
│ [containers.configuration]
290-
│ image = \\"other-app/Dockerfile\\"
291-
│ instance_type = \\"dev\\"
295+
│ [containers.configuration]
296+
│ image = \\"other-app/Dockerfile\\"
297+
│ instance_type = \\"dev\\"
292298
│
293-
│ [containers.constraints]
294-
│ tier = 1
299+
│ [containers.constraints]
300+
│ tier = 1
295301
│
296302
│
297303
│  SUCCESS  Modified application my-container-app
298304
│
299-
│
300305
│  SUCCESS  Created application my-container-app-2 (Application ID: abc)
301306
│
302307
╰ Applied changes
@@ -364,6 +369,8 @@ describe("cloudchamber apply", () => {
364369
│ - instances = 3
365370
│ + instances = 4
366371
│ name = \\"my-container-app\\"
372+
│ scheduling_policy = \\"default\\"
373+
│
367374
│ Skipping application rollout
368375
│
369376
├ NEW my-container-app-2
@@ -373,12 +380,12 @@ describe("cloudchamber apply", () => {
373380
│ instances = 1
374381
│ scheduling_policy = \\"default\\"
375382
│
376-
│ [containers.configuration]
377-
│ image = \\"other-app/Dockerfile\\"
378-
│ instance_type = \\"dev\\"
383+
│ [containers.configuration]
384+
│ image = \\"other-app/Dockerfile\\"
385+
│ instance_type = \\"dev\\"
379386
│
380-
│ [containers.constraints]
381-
│ tier = 1
387+
│ [containers.constraints]
388+
│ tier = 1
382389
│
383390
│
384391
│  SUCCESS  Created application my-container-app-2 (Application ID: abc)
@@ -448,6 +455,7 @@ describe("cloudchamber apply", () => {
448455
│ - instances = 3
449456
│ + instances = 4
450457
│ name = \\"my-container-app\\"
458+
│ scheduling_policy = \\"default\\"
451459
│
452460
├ NEW my-container-app-2
453461
│
@@ -456,17 +464,16 @@ describe("cloudchamber apply", () => {
456464
│ instances = 1
457465
│ scheduling_policy = \\"default\\"
458466
│
459-
│ [containers.configuration]
460-
│ image = \\"other-app/Dockerfile\\"
461-
│ instance_type = \\"dev\\"
467+
│ [containers.configuration]
468+
│ image = \\"other-app/Dockerfile\\"
469+
│ instance_type = \\"dev\\"
462470
│
463-
│ [containers.constraints]
464-
│ tier = 1
471+
│ [containers.constraints]
472+
│ tier = 1
465473
│
466474
│
467475
│  SUCCESS  Modified application my-container-app
468476
│
469-
│
470477
│  SUCCESS  Created application my-container-app-2 (Application ID: abc)
471478
│
472479
╰ Applied changes
@@ -582,21 +589,35 @@ describe("cloudchamber apply", () => {
582589
│ - instances = 3
583590
│ + instances = 4
584591
│ name = \\"my-container-app\\"
592+
│ scheduling_policy = \\"default\\"
593+
│
594+
│ ...
595+
│
596+
│ value = \\"value\\"
585597
│
586-
│ [[containers.configuration.labels]]
587-
│ + name = \\"name-1\\"
588-
│ + value = \\"value-1\\"
598+
│ [[containers.configuration.labels]]
599+
│ + name = \\"name-1\\"
600+
│ + value = \\"value-1\\"
601+
│ +
602+
│ + [[containers.configuration.labels]]
603+
│ +
604+
│ name = \\"name-2\\"
605+
│ value = \\"value-2\\"
589606
│
590-
│ + [[containers.configuration.labels]]
591-
│ name = \\"name-2\\"
607+
│ ...
592608
│
593-
│ [[containers.configuration.secrets]]
594-
│ - name = \\"MY_SECRET_1\\"
595-
│ - secret = \\"SECRET_NAME_1\\"
596-
│ - type = \\"env\\"
609+
│ type = \\"env\\"
597610
│
598-
│ - [[containers.configuration.secrets]]
599-
│ name = \\"MY_SECRET_2\\"
611+
│ [[containers.configuration.secrets]]
612+
│ - name = \\"MY_SECRET_1\\"
613+
│ - secret = \\"SECRET_NAME_1\\"
614+
│ - type = \\"env\\"
615+
│ -
616+
│ - [[containers.configuration.secrets]]
617+
│ -
618+
│ name = \\"MY_SECRET_2\\"
619+
│ secret = \\"SECRET_NAME_2\\"
620+
│ type = \\"env\\"
600621
│
601622
│
602623
│  SUCCESS  Modified application my-container-app
@@ -1101,15 +1122,15 @@ describe("cloudchamber apply", () => {
11011122
│
11021123
├ EDIT my-container-app
11031124
│
1104-
│ [containers.configuration]
1105-
│ ...
1106-
│ instance_type = \\"dev\\"
1125+
│ image = \\"./Dockerfile\\"
1126+
│ instance_type = \\"dev\\"
11071127
│
11081128
│ + [containers.configuration.observability.logs]
11091129
│ + enabled = true
1110-
│
1111-
│ [containers.constraints]
1112-
│ ...
1130+
│ +
1131+
│ +
1132+
│ [containers.constraints]
1133+
│ tier = 1
11131134
│
11141135
│
11151136
│  SUCCESS  Modified application my-container-app
@@ -1171,15 +1192,15 @@ describe("cloudchamber apply", () => {
11711192
│
11721193
├ EDIT my-container-app
11731194
│
1174-
│ [containers.configuration]
1175-
│ ...
1176-
│ instance_type = \\"dev\\"
1195+
│ image = \\"./Dockerfile\\"
1196+
│ instance_type = \\"dev\\"
11771197
│
11781198
│ + [containers.configuration.observability.logs]
11791199
│ + enabled = true
1180-
│
1181-
│ [containers.constraints]
1182-
│ ...
1200+
│ +
1201+
│ +
1202+
│ [containers.constraints]
1203+
│ tier = 1
11831204
│
11841205
│
11851206
│  SUCCESS  Modified application my-container-app
@@ -1246,12 +1267,14 @@ describe("cloudchamber apply", () => {
12461267
│
12471268
├ EDIT my-container-app
12481269
│
1270+
│ instance_type = \\"dev\\"
1271+
│
12491272
│ [containers.configuration.observability.logs]
12501273
│ - enabled = true
12511274
│ + enabled = false
12521275
│
1253-
│ [containers.constraints]
1254-
│ ...
1276+
│ [containers.constraints]
1277+
│ tier = 1
12551278
│
12561279
│
12571280
│  SUCCESS  Modified application my-container-app
@@ -1318,12 +1341,14 @@ describe("cloudchamber apply", () => {
13181341
│
13191342
├ EDIT my-container-app
13201343
│
1344+
│ instance_type = \\"dev\\"
1345+
│
13211346
│ [containers.configuration.observability.logs]
13221347
│ - enabled = true
13231348
│ + enabled = false
13241349
│
1325-
│ [containers.constraints]
1326-
│ ...
1350+
│ [containers.constraints]
1351+
│ tier = 1
13271352
│
13281353
│
13291354
│  SUCCESS  Modified application my-container-app
@@ -1389,12 +1414,14 @@ describe("cloudchamber apply", () => {
13891414
│
13901415
├ EDIT my-container-app
13911416
│
1417+
│ instance_type = \\"dev\\"
1418+
│
13921419
│ [containers.configuration.observability.logs]
13931420
│ - enabled = true
13941421
│ + enabled = false
13951422
│
1396-
│ [containers.constraints]
1397-
│ ...
1423+
│ [containers.constraints]
1424+
│ tier = 1
13981425
│
13991426
│
14001427
│  SUCCESS  Modified application my-container-app
@@ -1463,12 +1490,14 @@ describe("cloudchamber apply", () => {
14631490
│
14641491
├ EDIT my-container-app
14651492
│
1493+
│ instance_type = \\"dev\\"
1494+
│
14661495
│ [containers.configuration.observability.logs]
14671496
│ - enabled = true
14681497
│ + enabled = false
14691498
│
1470-
│ [containers.constraints]
1471-
│ ...
1499+
│ [containers.constraints]
1500+
│ tier = 1
14721501
│
14731502
│
14741503
│  SUCCESS  Modified application my-container-app
@@ -1688,12 +1717,12 @@ describe("cloudchamber apply", () => {
16881717
│ instances = 3
16891718
│ scheduling_policy = \\"default\\"
16901719
│
1691-
│ [containers.constraints]
1692-
│ tier = 2
1720+
│ [containers.constraints]
1721+
│ tier = 2
16931722
│
1694-
│ [containers.configuration]
1695-
│ image = \\"./Dockerfile\\"
1696-
│ instance_type = \\"dev\\"
1723+
│ [containers.configuration]
1724+
│ image = \\"./Dockerfile\\"
1725+
│ instance_type = \\"dev\\"
16971726
│
16981727
│
16991728
│  SUCCESS  Created application my-container-app (Application ID: abc)
@@ -1759,16 +1788,16 @@ describe("cloudchamber apply", () => {
17591788
│ - instances = 3
17601789
│ + instances = 4
17611790
│ name = \\"my-container-app\\"
1791+
│ scheduling_policy = \\"regional\\"
17621792
│
1763-
│ [containers.configuration]
1764-
│ image = \\"./Dockerfile\\"
1765-
│ - instance_type = \\"dev\\"
1766-
│ + instance_type = \\"standard\\"
1793+
│ [containers.configuration]
1794+
│ image = \\"./Dockerfile\\"
1795+
│ - instance_type = \\"dev\\"
1796+
│ + instance_type = \\"standard\\"
17671797
│
1768-
│ [containers.constraints]
1769-
│ ...
1770-
│ - tier = 3
1771-
│ + tier = 2
1798+
│ [containers.constraints]
1799+
│ - tier = 3
1800+
│ + tier = 2
17721801
│
17731802
│
17741803
│  SUCCESS  Modified application my-container-app
@@ -1835,16 +1864,16 @@ describe("cloudchamber apply", () => {
18351864
│ - instances = 3
18361865
│ + instances = 4
18371866
│ name = \\"my-container-app\\"
1867+
│ scheduling_policy = \\"regional\\"
18381868
│
1839-
│ [containers.configuration]
1840-
│ image = \\"./Dockerfile\\"
1841-
│ - instance_type = \\"basic\\"
1842-
│ + instance_type = \\"dev\\"
1869+
│ [containers.configuration]
1870+
│ image = \\"./Dockerfile\\"
1871+
│ - instance_type = \\"basic\\"
1872+
│ + instance_type = \\"dev\\"
18431873
│
1844-
│ [containers.constraints]
1845-
│ ...
1846-
│ - tier = 3
1847-
│ + tier = 2
1874+
│ [containers.constraints]
1875+
│ - tier = 3
1876+
│ + tier = 2
18481877
│
18491878
│
18501879
│  SUCCESS  Modified application my-container-app
@@ -1900,12 +1929,12 @@ describe("cloudchamber apply", () => {
19001929
│ instances = 3
19011930
│ scheduling_policy = \\"default\\"
19021931
│
1903-
│ [containers.constraints]
1904-
│ tier = 2
1932+
│ [containers.constraints]
1933+
│ tier = 2
19051934
│
1906-
│ [containers.configuration]
1907-
│ image = \\"${registry}/some-account-id/hello:1.0\\"
1908-
│ instance_type = \\"dev\\"
1935+
│ [containers.configuration]
1936+
│ image = \\"${registry}/some-account-id/hello:1.0\\"
1937+
│ instance_type = \\"dev\\"
19091938
│
19101939
│
19111940
│  SUCCESS  Created application my-container-app (Application ID: abc)
@@ -1969,15 +1998,14 @@ describe("cloudchamber apply", () => {
19691998
│
19701999
├ EDIT my-container-app
19712000
│
1972-
│ [containers.configuration]
1973-
│ image = \\"${registry}/some-account-id/hello:1.0\\"
1974-
│ - instance_type = \\"dev\\"
1975-
│ + instance_type = \\"standard\\"
2001+
│ [containers.configuration]
2002+
│ image = \\"${registry}/some-account-id/hello:1.0\\"
2003+
│ - instance_type = \\"dev\\"
2004+
│ + instance_type = \\"standard\\"
19762005
│
1977-
│ [containers.constraints]
1978-
│ ...
1979-
│ - tier = 3
1980-
│ + tier = 2
2006+
│ [containers.constraints]
2007+
│ - tier = 3
2008+
│ + tier = 2
19812009
│
19822010
│
19832011
│  SUCCESS  Modified application my-container-app

0 commit comments

Comments
 (0)