feat: add support for JSON5/JSONC config files #2059

NathanFlurry · 2025-02-20T08:02:42Z

Changes

NathanFlurry · 2025-02-20T08:03:12Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

greptile-apps

PR Summary

This PR adds support for JSON5/JSONC configuration files across the Rivet platform, migrating from YAML while maintaining backward compatibility.

Added JSON5/JSONC parsing in packages/common/config/src/lib.rs and packages/edge/infra/client/manager/src/main.rs with proper error handling
Converted configuration files from YAML to JSONC in docker/dev-full/ directory while preserving functionality
Added json5 v0.4.1 dependency to workspace and relevant packages for parsing JSON5/JSONC formats
Documentation paths in site/src/content/docs/self-hosting/ show inconsistencies between docker/dev-full/ and docker/monolith/ directories
Security concerns in new JSONC configs include hardcoded JWT keys, passwords, and disabled SSL marked for development use

_{16 file(s) reviewed, 10 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-02-20T08:03:37Z

docker/dev-full/docker-compose.yml

@@ -76,7 +76,7 @@ services:
      vector-server:
        condition: service_started
    volumes:
-      - ./rivet-edge-server:/etc/rivet-server:ro
+      - ./rivet-edge-server/config.jsonc:/etc/rivet-server/config.jsonc:ro


logic: The volume mount path /etc/rivet-server/config.jsonc is inconsistent with the service name rivet-edge-server. This could cause configuration conflicts.

Suggested change

- ./rivet-edge-server/config.jsonc:/etc/rivet-server/config.jsonc:ro

- ./rivet-edge-server/config.jsonc:/etc/rivet-edge-server/config.jsonc:ro

greptile-apps · 2025-02-20T08:03:38Z

docker/dev-full/docker-compose.yml

@@ -153,7 +153,7 @@ services:
    # TODO(RVT-4168): Compile libfdb from scratch for ARM
    platform: linux/amd64
    restart: unless-stopped
-    command: -c /etc/rivet-client/config.yaml
+    command: -c /etc/rivet-client/config.jsonc


style: The command format differs from rivet-server (line 9). Consider using consistent command formats across services.

greptile-apps · 2025-02-20T08:03:58Z

docker/dev-full/rivet-edge-server/config.jsonc

+		"jwt": {
+			"public": "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAx7S9ab9ErA50y0tVfFro919+BBxFSuwMKmcJ5QI853Y=\n-----END PUBLIC KEY-----\n",
+			"private": "-----BEGIN PRIVATE KEY-----\nMC4CAQAwBQYDK2VwBCIEIDI+WHFytxvdtfGot36NMCI26s2Yp0+E5u9OiPf3NQX3\n-----END PRIVATE KEY-----\n"


logic: JWT private key should not be committed to the repository. Remove this and use environment variable as noted in the comment above.

Suggested change

"jwt": {

"public": "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAx7S9ab9ErA50y0tVfFro919+BBxFSuwMKmcJ5QI853Y=\n-----END PUBLIC KEY-----\n",

"private": "-----BEGIN PRIVATE KEY-----\nMC4CAQAwBQYDK2VwBCIEIDI+WHFytxvdtfGot36NMCI26s2Yp0+E5u9OiPf3NQX3\n-----END PRIVATE KEY-----\n"

"jwt": {

"public": "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAx7S9ab9ErA50y0tVfFro919+BBxFSuwMKmcJ5QI853Y=\n-----END PUBLIC KEY-----\n"

greptile-apps · 2025-02-20T08:03:59Z

docker/dev-full/rivet-edge-server/config.jsonc

+				"url": "redis://redis:6379",
+				"password": "password"
+			},
+			"persistent": {
+				"url": "redis://redis:6379",
+				"password": "password"
+			}


style: Both Redis instances use same credentials and URL. Consider using separate passwords for ephemeral vs persistent Redis in development to catch connection errors.

greptile-apps · 2025-02-20T08:04:02Z

docker/dev-full/rivet-server/config.jsonc

+		"jwt": {
+			"public": "-----BEGIN PUBLIC KEY-----\nMCowBQYDK2VwAyEAx7S9ab9ErA50y0tVfFro919+BBxFSuwMKmcJ5QI853Y=\n-----END PUBLIC KEY-----\n",
+			"private": "-----BEGIN PRIVATE KEY-----\nMC4CAQAwBQYDK2VwBCIEIDI+WHFytxvdtfGot36NMCI26s2Yp0+E5u9OiPf3NQX3\n-----END PRIVATE KEY-----\n"


logic: The private JWT key is included in the config file despite the comment above explicitly stating not to include it. This is a security risk and contradicts the documentation. Remove the private key and use the environment variable as recommended.

greptile-apps · 2025-02-20T08:04:52Z

packages/common/config/src/lib.rs

@@ -70,7 +70,7 @@ fn add_source<P: AsRef<Path>>(
 			let path = entry.path();
 			if path.is_file() {
 				if let Some(extension) = path.extension().and_then(std::ffi::OsStr::to_str) {
-					if ["json", "yaml", "yml"].contains(&extension) {
+					if ["json", "json5", "jsonc", "yaml", "yml"].contains(&extension) {


style: Consider using a const array or HashSet for better maintainability of supported extensions

greptile-apps · 2025-02-20T08:04:53Z

packages/common/config/src/lib.rs

+			let value: serde_json::Value = json5::from_str(&content)
+				.map_err(|e| global_error::GlobalError::new(e))?;
+			let json = serde_json::to_string(&value)
+				.map_err(|e| global_error::GlobalError::new(e))?;
+			return Ok(settings.add_source(config_loader::File::from_str(&json, config_loader::FileFormat::Json)))


style: Consider adding error context about which file failed to parse. The current error only includes the parsing error but not the file path.

Suggested change

let value: serde_json::Value = json5::from_str(&content)

.map_err(|e| global_error::GlobalError::new(e))?;

let json = serde_json::to_string(&value)

.map_err(|e| global_error::GlobalError::new(e))?;

return Ok(settings.add_source(config_loader::File::from_str(&json, config_loader::FileFormat::Json)))

let value: serde_json::Value = json5::from_str(&content)

.map_err(|e| global_error::GlobalError::new_with_context(e, format!("failed to parse {}", path.display())))?;

let json = serde_json::to_string(&value)

.map_err(|e| global_error::GlobalError::new_with_context(e, format!("failed to serialize {}", path.display())))?;

return Ok(settings.add_source(config_loader::File::from_str(&json, config_loader::FileFormat::Json)))

greptile-apps · 2025-02-20T08:04:53Z

packages/common/config/src/lib.rs

+		Some("json5") | Some("jsonc") => {
+			// Parse JSON5/JSONC and convert to regular JSON
+			let value: serde_json::Value = json5::from_str(&content)
+				.map_err(|e| global_error::GlobalError::new(e))?;
+			let json = serde_json::to_string(&value)
+				.map_err(|e| global_error::GlobalError::new(e))?;
+			return Ok(settings.add_source(config_loader::File::from_str(&json, config_loader::FileFormat::Json)))
+		}


style: Early return here creates inconsistent control flow compared to other format handlers. Consider restructuring to use the same pattern as JSON and YAML handlers.

greptile-apps · 2025-02-20T08:05:12Z

site/src/content/docs/self-hosting/client-config.mdx

@@ -4,7 +4,7 @@ import Link from 'next/link';

 # Client Config

-Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-client/config.yaml'>`docker/monolith/rivet-client/config.yaml`</Link>
+Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-client/config.jsonc'>`docker/monolith/rivet-client/config.jsonc`</Link>


logic: Link path and displayed path are inconsistent. Link points to 'dev-full' directory but text shows 'monolith' directory.

Suggested change

Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-client/config.jsonc'>`docker/monolith/rivet-client/config.jsonc`</Link>

Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-client/config.jsonc'>`docker/dev-full/rivet-client/config.jsonc`</Link>

greptile-apps · 2025-02-20T08:05:28Z

site/src/content/docs/self-hosting/server-config.mdx

@@ -4,7 +4,7 @@ import Link from 'next/link';

 # Server Config

-Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-server/config.yaml'>`docker/monolith/rivet-server/config.yaml`</Link>
+Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-server/config.jsonc'>`docker/monolith/rivet-server/config.jsonc`</Link>


logic: The href points to dev-full but the displayed path shows monolith - this inconsistency needs to be fixed

Suggested change

Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-server/config.jsonc'>`docker/monolith/rivet-server/config.jsonc`</Link>

Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-server/config.jsonc'>`docker/dev-full/rivet-server/config.jsonc`</Link>

cloudflare-workers-and-pages · 2025-02-20T22:51:04Z

Deploying rivet with Cloudflare Pages

Latest commit:	`4c23ff5`
Status:	🚫 Build failed.

View logs

graphite-app · 2025-03-06T03:33:10Z

Merge activity

Mar 5, 10:33 PM EST: A user added this pull request to the Graphite merge queue.
Mar 5, 10:35 PM EST: CI is running for this PR on a draft PR: #2125
Mar 5, 10:37 PM EST: A user merged this pull request with the Graphite merge queue via draft PR: #2125.

## Changes

This was referenced Feb 20, 2025

feat: fdb sqlite workflows driver #1850

Closed

fix(workflows): allow op ctx to do all the things #1938

Closed

chore: restructure server binaries #1941

Closed

This was referenced Feb 20, 2025

feat(workflows): implement metrics for fdb driver #2057

Closed

fix: move actor state to fdb #2056

Closed

NathanFlurry mentioned this pull request Feb 20, 2025

fix(docker): fix corepack not enabled error #2058

Closed

greptile-apps bot reviewed Feb 20, 2025

View reviewed changes

NathanFlurry mentioned this pull request Feb 20, 2025

chore: add basic load test #2060

Closed

MasterPtato force-pushed the 02-19-fix_docker_fix_corepack_not_enabled_error branch from 7f6cbbe to 22d17b8 Compare February 20, 2025 22:50

MasterPtato force-pushed the feat_add_support_for_JSON5/JSONC_config_files branch from 52486f4 to dc6cea5 Compare February 20, 2025 22:50

This was referenced Feb 20, 2025

fix(workflows): fix activity error backoff, in memory polling #2062

Closed

fix(fdb): conflict ranges #2063

Closed

NathanFlurry force-pushed the 02-19-fix_docker_fix_corepack_not_enabled_error branch from 22d17b8 to b2c46c3 Compare February 21, 2025 05:17

MasterPtato force-pushed the feat_add_support_for_JSON5/JSONC_config_files branch from 822cda3 to 7b83adc Compare March 5, 2025 20:55

feat: add support for JSON5/JSONC config files

4c23ff5

MasterPtato mentioned this pull request Mar 5, 2025

fix: fix system test #2121

Closed

NathanFlurry force-pushed the feat_add_support_for_JSON5/JSONC_config_files branch from 7b83adc to 4c23ff5 Compare March 5, 2025 23:56

NathanFlurry force-pushed the 02-19-fix_docker_fix_corepack_not_enabled_error branch from cf49442 to 2bc1dbc Compare March 5, 2025 23:56

NathanFlurry mentioned this pull request Mar 5, 2025

chore: bump version from 5.1.2 to 25.1.3 #2122

Closed

MasterPtato force-pushed the 02-19-fix_docker_fix_corepack_not_enabled_error branch from 2bc1dbc to cf49442 Compare March 6, 2025 00:50

MasterPtato force-pushed the feat_add_support_for_JSON5/JSONC_config_files branch from 4c23ff5 to 7b83adc Compare March 6, 2025 00:50

NathanFlurry force-pushed the feat_add_support_for_JSON5/JSONC_config_files branch from 7b83adc to 4c23ff5 Compare March 6, 2025 01:32

NathanFlurry force-pushed the 02-19-fix_docker_fix_corepack_not_enabled_error branch from cf49442 to 2bc1dbc Compare March 6, 2025 01:32

This was referenced Mar 6, 2025

docs: add helm rollback troubleshooting guide #2123

Closed

fix(system-test): fix isolates use of hono #2124

Closed

MasterPtato force-pushed the 02-19-fix_docker_fix_corepack_not_enabled_error branch from 2bc1dbc to cf49442 Compare March 6, 2025 02:38

MasterPtato force-pushed the feat_add_support_for_JSON5/JSONC_config_files branch from 4c23ff5 to 7b83adc Compare March 6, 2025 02:38

NathanFlurry force-pushed the feat_add_support_for_JSON5/JSONC_config_files branch from 7b83adc to 4c23ff5 Compare March 6, 2025 03:30

NathanFlurry force-pushed the 02-19-fix_docker_fix_corepack_not_enabled_error branch from cf49442 to 2bc1dbc Compare March 6, 2025 03:30

graphite-app bot closed this Mar 6, 2025

graphite-app bot deleted the feat_add_support_for_JSON5/JSONC_config_files branch March 6, 2025 03:37

github-actions bot mentioned this pull request Mar 6, 2025

chore(main): release 25.2.0 #2079

Merged

This was referenced Mar 6, 2025

chore: add some logs #2133

Closed

fix: fix wf change bug with client wf #2134

Closed

fix: fix topology units, core actor endpoints #2135

Closed

fix: fix pb draining #2136

Closed

fix(workflows): remove tagged signals from fdb driver #2137

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for JSON5/JSONC config files #2059

feat: add support for JSON5/JSONC config files #2059

NathanFlurry commented Feb 20, 2025

NathanFlurry commented Feb 20, 2025 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

greptile-apps bot Feb 20, 2025

cloudflare-workers-and-pages bot commented Feb 20, 2025 •

edited

Loading

graphite-app bot commented Mar 6, 2025 •

edited

Loading

	- ./rivet-edge-server/config.jsonc:/etc/rivet-server/config.jsonc:ro
	- ./rivet-edge-server/config.jsonc:/etc/rivet-edge-server/config.jsonc:ro

	Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-client/config.jsonc'>`docker/monolith/rivet-client/config.jsonc`</Link>
	Default location: <Link href='https://github.com/rivet-gg/rivet/blob/main/docker/dev-full/rivet-client/config.jsonc'>`docker/dev-full/rivet-client/config.jsonc`</Link>

feat: add support for JSON5/JSONC config files #2059

feat: add support for JSON5/JSONC config files #2059

Conversation

NathanFlurry commented Feb 20, 2025

Changes

NathanFlurry commented Feb 20, 2025 • edited Loading

How to use the Graphite Merge Queue

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 20, 2025

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Feb 20, 2025 • edited Loading

Deploying rivet with Cloudflare Pages

graphite-app bot commented Mar 6, 2025 • edited Loading

Merge activity

NathanFlurry commented Feb 20, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Feb 20, 2025 •

edited

Loading

graphite-app bot commented Mar 6, 2025 •

edited

Loading