ESD-1638: Validate VXC update JSON with checked helpers and a field gate#526
ESD-1638: Validate VXC update JSON with checked helpers and a field gate#526Phil-Browne wants to merge 10 commits into
Conversation
buildUpdateVXCRequestFromJSON used raw type assertions, so a misspelled key or wrong-typed value was silently dropped and an empty update was sent to the API. Switch to the checked utils.JSON* helpers used by the buy path, and reject a payload that sets no recognized field before any API call.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #526 +/- ##
==========================================
+ Coverage 78.87% 79.30% +0.42%
==========================================
Files 193 193
Lines 18574 18683 +109
==========================================
+ Hits 14651 14817 +166
+ Misses 2857 2818 -39
+ Partials 1066 1048 -18
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves the safety of the VXC update JSON input path by making JSON parsing type-safe (using shared utils.JSON* helpers) and by rejecting “empty/no-op” updates before any API call, with new tests covering the new validation behavior.
Changes:
- Replaced raw JSON type assertions in
buildUpdateVXCRequestFromJSONwith checkedutils.JSONString/JSONNumber/JSONBool/JSONObjecthelpers to surface key typos and wrong JSON types as errors. - Added an “at least one field must be updated” gate to prevent silent no-op updates.
- Added unit tests covering typo keys, wrong-typed values (flat/nested/partner config), empty object, VLAN fallback behavior, and a valid multi-field update.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| internal/commands/vxc/vxc_inputs_update.go | Uses checked JSON helpers across update fields and adds an at-least-one-field gate. |
| internal/commands/vxc/vxc_inputs_test.go | Adds test coverage for typo/wrong-type/empty JSON update payloads and valid multi-field updates. |
…date JSON Copilot flagged that VLAN, inner VLAN, and vNIC-related fields silently truncated fractional JSON numbers (e.g. 100.9 -> 100) before validation. Add the same whole-number check already used for rateLimit/term to all six affected fields. Also add tests for the remaining untested error branches and valid-value paths in buildUpdateVXCRequestFromJSON.
…missing Copilot noted that an absent connectType fell through to the generic "only VRouter" message instead of naming the missing field.
Wrap buildUpdateVXCRequestFromJSON's returned errors with exitcodes.NewUsageError to match the buy JSON builder's convention, so malformed or wrongly typed update payloads exit with the usage code instead of the general code.
Guards against a regression where an error message stays the same but silently stops being classified as a usage error.
|
The checked-helper swap is right, but the unknown-key rejection is only partial. The unrecognised-key scan runs only in the |
…d is set The unrecognized-key scan only ran inside the fieldSet == false branch, so a typo alongside a valid field (e.g. rateLimit + rateLimt) set fieldSet and skipped the scan, silently dropping the typo. Compute the unrecognized set unconditionally and reject whenever it's non-empty.
|
Good catch, fixed in eccef2d: the unrecognized-key scan now runs unconditionally over rawData (and the nested aEndConfiguration/bEndConfiguration maps) instead of only when fieldSet is false, so |
…son-checked # Conflicts: # internal/commands/vxc/vxc_inputs_test.go # internal/commands/vxc/vxc_inputs_update.go
The VXC update JSON builder used raw type assertions, so a misspelled key or a wrong-typed value (e.g.
--json '{"rateLimt": 500}'or{"rateLimit": "500"}') was silently dropped and the CLI happily reported success on an empty update.rawData[...].(type)assertion inbuildUpdateVXCRequestFromJSONfor the checkedutils.JSON*helpers already used by the buy path, so a typo or wrong type now returns a clear error.{}or only unknown keys) is rejected before any API call.aEndVlan/bEndVlanfallback, and a valid multi-field payload.