Skip to content

wicket: Deny unknown fields when accepting RSS config TOML#7667

Merged
jgallagher merged 2 commits intomainfrom
john/wicket-deny-unknown-toml-fields
Feb 27, 2025
Merged

wicket: Deny unknown fields when accepting RSS config TOML#7667
jgallagher merged 2 commits intomainfrom
john/wicket-deny-unknown-toml-fields

Conversation

@jgallagher
Copy link
Copy Markdown
Contributor

From discussions on #7653: wicket does not and should not accept a [recovery_silo] section in the uploaded TOML, but on main it will silently discard it. This PR turns on serde(deny_unknown_fields); now, attempting to upload a config TOML with an unexpected section will fail:

Feb 27 16:44:21.805 INFO reading config from stdin..., file: wicket/src/cli/rack_setup.rs:110
Feb 27 16:44:21.805 INFO parsing config..., file: wicket/src/cli/rack_setup.rs:115
Error: failed to parse config TOML

Caused by:
    TOML parse error at line 93, column 2
       |
    93 | [recovery_silo]
       |  ^^^^^^^^^^^^^
    unknown field `recovery_silo`, expected one of `bootstrap_sleds`, `ntp_servers`, `dns_servers`, `internal_services_ip_pool_ranges`, `external_dns_ips`, `external_dns_zone_name`, `rack_network_config`, `allowed_source_ips`

@leftwo
Copy link
Copy Markdown
Contributor

leftwo commented Feb 27, 2025

Any chance we could test this on an a4x2 setup, just to be sure we don't break something there?

@jgallagher
Copy link
Copy Markdown
Contributor Author

I can give that a whirl before merging, sure, but I don't think a4x2 does (or maybe even can?) use wicket to run RSS.

@leftwo
Copy link
Copy Markdown
Contributor

leftwo commented Feb 27, 2025

I can give that a whirl before merging, sure, but I don't think a4x2 does (or maybe even can?) use wicket to run RSS.

Oh yeah, this is all deep in wicket. Feel free to ignore that request.

@jgallagher jgallagher merged commit 087c856 into main Feb 27, 2025
17 checks passed
@jgallagher jgallagher deleted the john/wicket-deny-unknown-toml-fields branch February 27, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants