ESD-1648: Tighten validation and interactive-prompt gaps by Phil-Browne · Pull Request #535 · megaport/cli

Phil-Browne · 2026-07-10T19:58:00Z

Closes gaps where interactive prompts and validators diverged from the flag/JSON input paths.

Validate vNIC VLAN input during interactive MVE ordering, matching the flag/JSON paths
Reject empty required credentials in VXC partner prompts (AWS ownerAccount and ASN, Azure serviceKey, Google pairingKey, Oracle virtualCircuitId, IBM accountID); connection name is now optional (the API defaults it to MEGAPORT); stop silently defaulting an unparseable Azure peering VLAN to 0, and range-validate the parsed VLAN
Nil-guard and type-check every partner-config validator (AWS, Azure, Google, Oracle, IBM, vRouter, Transit); add the missing Transit case; align required-field checks between the shared validators and the interactive prompts
Count runes instead of bytes in port/MVE/AWS name-length validators, so multibyte names are not under-counted
Validate MCR prefix-filter entries as CIDRs with GE/LE bound checks, and reject an unknown address family up front rather than treating it as IPv6

Two orthogonal fixes originally in this PR were split out to keep the scope focused:

GetIntFromInterface int-width overflow bound -> fix(validation): correct int-width bound in GetIntFromInterface #538
Interactive tag-editing empty-value handling -> fix(cli): handle empty-value tag edits in interactive tag prompt #539

Close six gaps where interactive prompts and validators diverged from the flag/JSON paths: - Validate vNIC VLAN input during interactive MVE ordering - Reject empty required credentials in VXC partner prompts (AWS, Azure, IBM) and stop silently defaulting an unparseable Azure peering VLAN to 0; validate the parsed VLAN range too - Count runes, not bytes, in port/MVE/AWS name-length validators - Validate MCR prefix-filter entries as CIDRs with address-family consistency checks - Reject a float value of exactly 2^63 in GetIntFromInterface - Allow removing a tag with an empty existing value during interactive tag editing, and handle a newly entered empty-value key deterministically

codecov · 2026-07-10T20:00:05Z

Codecov Report

❌ Patch coverage is 95.87629% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.63%. Comparing base (6737d3b) to head (6778eee).

Files with missing lines	Patch %	Lines
internal/commands/vxc/vxc_prompts_partner.go	86.20%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #535 +/- ##
==========================================
+ Coverage 79.48% 79.63% +0.14% 
==========================================
Files 193 193 Lines 18859 18920 +61 ==========================================
+ Hits 14991 15067 +76 + Misses 2820 2812 -8 + Partials 1048 1041 -7

Files with missing lines	Coverage Δ
internal/commands/mcr/mcr_prompts.go	`70.21% <100.00%> (+0.18%)`	⬆️
internal/commands/mve/mve_prompts.go	`47.23% <100.00%> (+0.29%)`	⬆️
internal/validation/mcr.go	`100.00% <100.00%> (+11.32%)`	⬆️
internal/validation/mve.go	`62.67% <100.00%> (+1.32%)`	⬆️
internal/validation/port.go	`94.44% <100.00%> (+11.11%)`	⬆️
internal/validation/vxc.go	`72.10% <100.00%> (+1.83%)`	⬆️
internal/commands/vxc/vxc_prompts_partner.go	`54.54% <86.20%> (+4.12%)`	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Codecov flagged three partial hits: ValidatePortRequest, ValidateLAGPortRequest, and ValidateBuyMVERequest never exercised the name-length-exceeded branch, only ValidatePortName and ValidateMVERequest did.

Copilot

Pull request overview

Tightens input validation and aligns interactive prompt behavior with the flag/JSON and validator paths across multiple Megaport CLI resources (VXC partners, MVE ordering, MCR prefix filters, and tag editing), with additional test coverage for the new validation behavior.

Changes:

Switched several name-length checks to count Unicode runes (not bytes) and added multibyte-focused tests.
Added stricter interactive prompt validation for partner credentials/VLANs and improved tag-editing behavior when values are empty.
Strengthened MCR prefix-filter entry validation (CIDR + address-family consistency) and tightened numeric conversion overflow handling.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
internal/validation/vxc.go	Counts runes for AWS/IBM name-length validation.
internal/validation/vxc_test.go	Adds multibyte AWS connection-name boundary tests.
internal/validation/port.go	Counts runes for port-name validation.
internal/validation/port_test.go	Adds multibyte port-name boundary tests.
internal/validation/mve.go	Counts runes for MVE-name validation.
internal/validation/mve_test.go	Adds multibyte MVE-name boundary tests.
internal/validation/mcr.go	Validates prefix-filter entries as CIDRs and enforces address-family consistency.
internal/validation/mcr_test.go	Adds CIDR/address-family test cases for prefix-filter updates.
internal/validation/conversion.go	Tightens float64→int conversion overflow rejection.
internal/validation/conversion_test.go	Adds regression tests for the 2^63 float64 rounding edge case.
internal/utils/prompts.go	Makes tag removal work even when existing tag values are empty; adds clearer messaging.
internal/utils/prompts_test.go	Adds coverage for empty-value tag removal and empty-value new keys.
internal/commands/vxc/vxc_prompts_test.go	Adds tests ensuring partner prompts reject empty required credentials and invalid VLAN input.
internal/commands/vxc/vxc_prompts_partner.go	Enforces required partner credentials in interactive prompts; validates Azure peering VLAN parsing/range.
internal/commands/mve/mve_prompts.go	Validates interactive vNIC VLAN input with the shared VLAN validator.
internal/commands/mve/mve_prompts_test.go	Adds out-of-range VLAN test coverage for interactive vNIC prompting.

Comparing against float64(math.MaxInt) directly wrongly rejects the true max int on platforms where MaxInt fits exactly in a float64 (it only rounds up to 2^63 when int is 64 bits). Add 1 to get the correct exclusive upper bound on either width, and make the boundary tests derive from math.MaxInt instead of a hardcoded 2^63 so they stay in range regardless of platform int width. Also update the ValidatePrefixFilterListRequest / ValidateUpdatePrefixFilterList doc comments to mention the CIDR and address-family checks they now perform.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

ValidateUpdatePrefixFilterList dispatched into the shared entry validator without checking AddressFamily was present and valid, unlike the create path. An empty or invalid value would silently fall into the IPv6 branch.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

…th native - Azure peering VLAN prompt now uses validation.ParseInt instead of a raw strconv.Atoi wrap, so a bad value gets the same friendly 'invalid VLAN' wording as other numeric prompts instead of a leaked Atoi error string. - The WASM interactive tag prompt still used the old value == "" && tags[key] != "" condition, so it refused to remove a tag with an empty current value and stayed silent on a key that didn't exist. Mirrored the native prompts.go fix: check key existence explicitly and report when there's nothing to remove.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

ValidateAWSPartnerConfig allowed an empty ConnectionName and only checked max length when non-empty, while the interactive prompt path already treats connection name as required. Since JSON and flag-built configs both funnel through this shared validator, the gap let those paths bypass a requirement the prompt enforces. Update the fixtures across three test functions that constructed VXCPartnerConfigAWS without a connection name.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

ValidateVXCPartnerConfig treated any *VXCPartnerConfigTransit as always valid, so a nil pointer or a wrong ConnectType would pass validation and only fail later against the API. Extracted a ValidateTransitPartnerConfig function that checks both, matching the nil and connect-type checks the other partner validators already do.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

…ries ValidateAWSPartnerConfig and ValidateIBMPartnerConfig could panic on a typed-nil pointer reaching the dispatch switch, unlike their Azure, vRouter, and Transit siblings which already nil-check. Also guard validatePrefixFilterEntries against a nil entry in the list, which would otherwise panic instead of returning a validation error.

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

 if entry.Action != "permit" && entry.Action != "deny" {
 return NewValidationError("entry action", entry.Action, "must be permit or deny")
 }


Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

 {"float64 above int range rejected", float64(math.MaxInt) * 2, 0, false},
+ {"float64 just above max int rejected", justAboveMaxInt, 0, false},
+ {"float64 just below max int accepted", nearMaxInt, int(nearMaxInt), true},


Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Google and Oracle partner config validators lacked the nil-guard that AWS, IBM, Azure, vRouter, and Transit already have, so a nil pointer would panic instead of returning a validation error. The IBM interactive prompt also required a customer ASN unconditionally, even though the prompt text and validator both treat it as optional when the opposite end is an MCR.

The IBM partner config name is optional in the API (it defaults to "MEGAPORT" server-side), and the SDK and Terraform provider both treat it that way. The validator was requiring it, which broke JSON and flag inputs that omitted the name, and the interactive prompt required it too. Length and character-set checks still apply when a name is given.

- validate interactive MCR prefix filter list creation before returning - require pairing key / virtual circuit ID in Google and Oracle prompts - enforce MVE name length on update requests - validate prefix filter entry GE/LE bounds per address family and GE <= LE

The GetIntFromInterface int-width bound fix and the interactive tag-editing empty-value fix are orthogonal to the prompt/validator parity work here, so they were pulled into their own PRs: - GetIntFromInterface overflow bound -> #538 - interactive tag-edit empty-value handling -> #539 Restores those six files to the main baseline; this PR now stays focused on partner-config and prefix-filter validation parity.

penzeliz-megaport · 2026-07-13T06:48:10Z

This makes the AWS connection name required on all input paths: ValidateAWSPartnerConfig (internal/validation/vxc.go:231) now errors when ConnectionName is empty. Correct only if the API mandates it. If the API auto-generates the connection name when it's omitted, this blocks a previously-valid AWS VXC order (flags and JSON) client-side, which is a regression. Confirm against the API/SDK contract before merge; if it's optional, keep the length check but drop the empty-string rejection.

The AWS partner config name is optional in the API (both AwsRequest and AwshcRequest schemas omit it from required fields and default it to "MEGAPORT" server-side), same as IBM. An earlier commit in this PR made it required in the shared validator and the interactive prompt, which blocked previously-valid AWS VXC orders via flags and JSON. Length check still applies when a name is given.

Phil-Browne · 2026-07-13T14:22:52Z

Confirmed against the OpenAPI spec: name is not in the required array for either AwsRequest (AWS VIF) or AwshcRequest (AWS Hosted Connection), and its description states it defaults to "MEGAPORT" server-side, same as IBM. This was a genuine regression from an earlier commit in this PR.

Fixed in a4f570a: ValidateAWSPartnerConfig no longer rejects an empty ConnectionName, only enforcing the length check when a name is provided. Reverted the matching change in the interactive prompt (promptAWSConfig) so all three input paths agree again. Updated the affected tests.

…rompt-gaps

…tion validatePrefixFilterEntries treated any non-IPv4 family as IPv6 and defaulted the max prefix length to 32 for unknown values. Replace the implicit branch with a switch that rejects unknown families up front, so the function is self-contained rather than relying on callers to pre-validate.

CopilotAI review requested due to automatic review settings July 10, 2026 19:58

Phil-Browne requested review from a team and penzeliz-megaport as code owners July 10, 2026 19:58

Phil-Browne requested a review from mp1-tomclancy July 10, 2026 19:58

Copilotstarted reviewing on behalf of Phil-Browne July 10, 2026 19:58 View session

test: cover name-too-long branch in port and MVE buy/LAG validators
8f6b2bb
Codecov flagged three partial hits: ValidatePortRequest, ValidateLAGPortRequest, and ValidateBuyMVERequest never exercised the name-length-exceeded branch, only ValidatePortName and ValidateMVERequest did.