ESD-1647: Close input-mode divergences across buy and update commands#536
Open
Phil-Browne wants to merge 9 commits into
Open
ESD-1647: Close input-mode divergences across buy and update commands#536Phil-Browne wants to merge 9 commits into
Phil-Browne wants to merge 9 commits into
Conversation
Bring interactive, flag, and JSON input modes to parity across eight commands so each validates, resolves, and gates inputs the same way regardless of mode. - nat gateway: reject empty tag keys on flag paths (already done on JSON) - managed account: validate required fields in the JSON path - vxc buy: resolve partner-port UIDs in the JSON path like the flags path - mve update: gate the name update on an explicit flag/JSON change, not a non-empty check - ix update: require at least one field before issuing the update - vxc update prompts: reject an update where nothing changed or the new name is empty - nat gateway update: keep the current promo code and service level reference when left blank, instead of relying on server-side omitempty - partners search: normalize the interactive output-format input (case, whitespace, unsupported values falling back to table)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #536 +/- ##
==========================================
+ Coverage 78.57% 79.18% +0.61%
==========================================
Files 192 193 +1
Lines 18522 18647 +125
==========================================
+ Hits 14553 14765 +212
+ Misses 2904 2805 -99
- Partials 1065 1077 +12
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens input validation and behavior consistency across interactive, flag, and JSON input modes for several “buy”/“update” commands, aiming to ensure the same requests are accepted/rejected regardless of how the user supplies input.
Changes:
- Align VXC behavior across modes: interactive update now rejects no-op/empty-name updates, and JSON buy now resolves partner-port identifiers similarly to flags mode.
- Harden update/build inputs: require at least one field for IX updates; reject explicit empty names for MVE updates; require core fields for managed-account buy via JSON.
- Normalize interactive output/metadata inputs: partners search output-format prompt now supports more formats and normalizes user input; NAT gateway flags now reject empty tag keys and update defaults preserve promo/SLR when left blank.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/commands/vxc/vxc_prompts.go | Reject empty name and no-op updates in interactive VXC update flow. |
| internal/commands/vxc/vxc_prompts_test.go | Update prompt-builder tests to cover new error cases and no-op rejection. |
| internal/commands/vxc/vxc_inputs.go | Add JSON-path partner-port UID resolution and thread ctx/service into JSON builder. |
| internal/commands/vxc/vxc_inputs_test.go | Update JSON builder call sites and add partner-port resolution test coverage. |
| internal/commands/vxc/vxc_actions.go | Wire ctx/service through to the JSON VXC request builder. |
| internal/commands/partners/partners_actions.go | Expand/normalize interactive output-format selection for partner search. |
| internal/commands/partners/partners_actions_test.go | Add tests asserting output-format normalization behavior. |
| internal/commands/nat_gateway/nat_gateway_inputs.go | Reject empty resource-tag keys in NAT gateway flag paths. |
| internal/commands/nat_gateway/nat_gateway_additional_test.go | Add tests for promo/SLR default inheritance behavior in updates. |
| internal/commands/nat_gateway/nat_gateway_actions.go | Preserve promo code and service level reference when left blank during update merges. |
| internal/commands/mve/mve_inputs.go | Gate name updates on presence/Changed and reject explicit empty names in JSON/flags paths. |
| internal/commands/mve/mve_inputs_test.go | Add tests for explicit empty-name rejection in MVE updates. |
| internal/commands/managed_account/managed_account_inputs.go | Enforce required fields for managed-account buy via JSON. |
| internal/commands/managed_account/managed_account_inputs_test.go | Update JSON tests to expect required-field failures. |
| internal/commands/ix/ix_test.go | Update IX update JSON tests to require at least one recognized field. |
| internal/commands/ix/ix_inputs.go | Reject IX update JSON bodies that don’t change any supported field. |
…dd coverage Copilot review on PR #536 flagged that JSON-mode buy could send an empty B-End productUID when the partner config type doesn't support UID lookup (e.g. AWS), since ValidateVXCRequest only requires it when partnerConfig is nil. Match flags mode, which always requires a resolved UID. Also fills in patch coverage gaps flagged by Codecov: empty tag key rejection on both NAT gateway flag paths, the remaining VXC update prompt fields, and the new VXC JSON partner-port resolution branches.
Copilot review on PR #536 noted the JSON-mode required-fields error used flag names (account-name/account-ref) instead of the JSON keys (accountName/accountRef), which is confusing when the input is a JSON body or file rather than flags.
Copilot review on PR #536 noted that resolvePartnerPortUID could run before vxcName/rateLimit/term were even parsed, wasting an API call on malformed JSON that would fail validation anyway. Move the rateLimit/ term whole-number checks (and vxcName parsing) ahead of the lookup so malformed JSON fails fast without the network round-trip.
Copilot review on PR #536 pointed out that vxcName/rateLimit/term presence wasn't enforced before resolvePartnerPortUID, so a JSON body missing these fields but relying on a partner config for portUid or productUID would still trigger a network lookup and only fail later in ValidateVXCRequest. Added a requireBuyFields check called immediately before both the A-End and B-End lookups.
…e errors nat_gateway's flags create/update paths and its JSON update path returned plain errors for an empty resource tag key, exiting general (1) instead of usage, unlike the JSON create path and every other resource's tag validation. mve's update paths and vxc's update prompt returned a plain error for an explicitly empty name instead of a validation.ValidationError, so they also exited general instead of usage, unlike the vxc buy prompt's equivalent check.
buildUpdateIXRequestFromJSON returned utils.ReadJSONInput and json.Unmarshal errors unwrapped, so a missing --json-file got classified as an API error (the "failed to read" pattern) instead of usage, unlike buildIXRequestFromJSON.
"failed to parse resource tags JSON" doesn't match classifyError's exact "failed to parse JSON" usage pattern, so it fell through to the broader API-error pattern and returned the wrong exit code. Wrap the read and unmarshal errors explicitly, same as the empty-tag-key check right below them.
Collaborator
|
Two issues.
|
buildManagedAccountRequestFromFlags built the request with no validation while buildManagedAccountRequestFromJSON rejected empty accountName/accountRef, letting managed-account buy --account-name Foo (blank --account-ref) ship to the API. Add the same check to the flags builder.
Contributor
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings interactive, flag, and JSON input modes to parity across eight buy/update commands so each validates, resolves, and gates inputs the same way regardless of mode.
accountName,accountRef) in the JSON pathFull validation (
go build ./... && go test ./... && golangci-lint run) passes cleanly.