Skip to content

Fix network name lookups on older CloudStack servers - #164

Open
1fanwang wants to merge 2 commits into
apache:mainfrom
1fanwang:1fannnw/fix-legacy-network-lookups
Open

Fix network name lookups on older CloudStack servers#164
1fanwang wants to merge 2 commits into
apache:mainfrom
1fanwang:1fannnw/fix-legacy-network-lookups

Conversation

@1fanwang

@1fanwang 1fanwang commented Sep 12, 2026

Copy link
Copy Markdown

Summary

On CloudStack before 4.22, looking up a missing network name can return an unrelated network's ID. Those servers ignore the name filter introduced in SDK 2.19.0.

The generator now sends keyword alongside name for network lookups. Both GetNetworkID and GetNetworkByName require an exact name, even when the keyword search returns only one candidate.

Fixes: #140

Testing Done

I ran the committed CI subtest against CloudStack 4.21.0.0 with real MySQL. It creates a network and VMs through the API and exercises exact, absent, and partial names. HTTP regressions also cover the newer server's exact-name filter.

# Scenario Command Result
1 Single partial match, before fix CI command below Returned another network's ID
2 Same lookup after fix Same command Exact-name error; no ID
3 Exact and absent names Same command Correct ID or no match
Raw logs

With CLOUDSTACK_API_KEY and CLOUDSTACK_SECRET_KEY set to the simulator credentials used by the existing CI workflow:

export CLOUDSTACK_API_URL=http://127.0.0.1:18172/client/api
go test -mod=readonly -count=1 -v ./ci -run '^TestCloudstackAPI$/^NetworkNameLookups$'

Before, on ad15f21 with the added regression:

    ci_test.go:129: GetNetworkID("test-network-1789189235570606000"): id="2d1b2698-a301-4523-a32e-2d968116f391" count=1 error=<nil>
    ci_test.go:138: GetNetworkByName("test-network-1789189235570606000"): id="2d1b2698-a301-4523-a32e-2d968116f391" count=1 error=<nil>

After, on 4.21:

    ci_test.go:129: GetNetworkID("test-network-1789189315495373000"): id="" count=1 error=Could not find an exact match for test-network-1789189315495373000: &{Count:1 Networks:[0x5bac5a6cc908]}
    ci_test.go:138: GetNetworkByName("test-network-1789189315495373000"): id="" count=1 error=Could not find an exact match for test-network-1789189315495373000: &{Count:1 Networks:[0x5bac5a789208]}

The HTTP regression also covers multiple matches and option forwarding.

Send keyword alongside name so older servers filter network lookups while newer servers retain exact-name matching.

Fixes: apache#140
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Copilot AI lite review requested due to automatic review settings September 12, 2026 00:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Validate the returned name for single partial matches and add regression coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes CloudStack pre-4.22 network name lookups by sending keyword alongside name.

Changes:

  • Updates generated and network service lookup code.
  • Adds HTTP regression coverage.
  • Adds simulator-backed CI scenarios.

Review finding: single partial keyword matches still require exact-name validation.

File summaries
File Description
test/GetNetworkByNameRegression_test.go Tests legacy and current lookup behavior.
generate/generate.go Generates the legacy-compatible keyword fallback.
cloudstack/NetworkService.go Applies keyword to network ID lookups.
ci/ci_test.go Adds simulator-backed lookup coverage.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread generate/generate.go
Require the requested name to match the sole keyword candidate before returning its ID.

apache#164 (comment)
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Copilot AI review requested due to automatic review settings September 12, 2026 05:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The reviewed changes address the compatibility issue with regression coverage and have no unresolved blocking concerns.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

Breaking change in v2.19.0 in GetNetworkID for CloudStack versions < 4.22

2 participants