From ad15f21fd738b82b880ac4498268da19ea382261 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Fri, 11 Sep 2026 17:52:37 -0700 Subject: [PATCH 1/2] Fix network name lookups on older CloudStack servers Send keyword alongside name so older servers filter network lookups while newer servers retain exact-name matching. Fixes: https://github.com/apache/cloudstack-go/issues/140 Signed-off-by: 1fanwang <1fannnw@gmail.com> --- ci/ci_test.go | 38 +++++++- cloudstack/NetworkService.go | 1 + generate/generate.go | 4 + test/GetNetworkByNameRegression_test.go | 117 ++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 test/GetNetworkByNameRegression_test.go diff --git a/ci/ci_test.go b/ci/ci_test.go index f7a7d658..f8b3fe92 100644 --- a/ci/ci_test.go +++ b/ci/ci_test.go @@ -45,10 +45,12 @@ These tests validate that: package citest import ( + "fmt" "os" "slices" "strings" "testing" + "time" "github.com/apache/cloudstack-go/v2/cloudstack" ) @@ -61,6 +63,7 @@ type testResources struct { serviceOfferingID1 string serviceOfferingID2 string networkID string + networkName string templateID string networkOfferingID string } @@ -90,6 +93,38 @@ func TestCloudstackAPI(t *testing.T) { // Set up test environment setupTestEnvironment(t, client, resources) + t.Run("NetworkNameLookups", func(t *testing.T) { + for _, tc := range []struct { + name string + wantID string + }{ + {name: resources.networkName, wantID: resources.networkID}, + {name: "missing-" + resources.networkID}, + } { + t.Run(tc.name, func(t *testing.T) { + wantCount := 0 + if tc.wantID != "" { + wantCount = 1 + } + id, count, err := client.Network.GetNetworkID(tc.name) + t.Logf("GetNetworkID(%q): id=%q count=%d error=%v", tc.name, id, count, err) + if id != tc.wantID || count != wantCount || (err != nil) != (tc.wantID == "") { + t.Errorf("wanted network ID %q and count %d", tc.wantID, wantCount) + } + + network, count, err := client.Network.GetNetworkByName(tc.name) + id = "" + if network != nil { + id = network.Id + } + t.Logf("GetNetworkByName(%q): id=%q count=%d error=%v", tc.name, id, count, err) + if id != tc.wantID || count != wantCount || (err != nil) != (tc.wantID == "") { + t.Errorf("wanted network ID %q and count %d", tc.wantID, wantCount) + } + }) + } + }) + // Run the actual tests t.Run("BasicTypes", func(t *testing.T) { testBasicTypes(t, client, resources) @@ -114,7 +149,8 @@ func setupTestEnvironment(t *testing.T, client *cloudstack.CloudStackClient, res resources.networkOfferingID = getTestNetworkOffering(t, client) // Create test network - resources.networkID = createTestNetwork(t, client, "ci-test-network", resources.networkOfferingID, resources.zoneID) + resources.networkName = fmt.Sprintf("ci-test-network-%d", time.Now().UnixNano()) + resources.networkID = createTestNetwork(t, client, resources.networkName, resources.networkOfferingID, resources.zoneID) // Get template ID resources.templateID = getTestTemplate(t, client, resources.zoneID) diff --git a/cloudstack/NetworkService.go b/cloudstack/NetworkService.go index fe99e200..dcd4b183 100644 --- a/cloudstack/NetworkService.go +++ b/cloudstack/NetworkService.go @@ -4855,6 +4855,7 @@ func (s *NetworkService) GetNetworkID(name string, opts ...OptionFunc) (string, p.p = make(map[string]interface{}) p.p["name"] = name + p.p["keyword"] = name for _, fn := range append(s.cs.options, opts...) { if err := fn(s.cs, p); err != nil { diff --git a/generate/generate.go b/generate/generate.go index a0b24e4c..287eb694 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -1666,6 +1666,10 @@ func (s *service) generateHelperFuncs(a *API) { pn(" p.p = make(map[string]interface{})") pn("") pn(" p.p[\"%s\"] = %s", v, v) + // CloudStack < 4.22 ignores listNetworks' name filter: https://github.com/apache/cloudstack-go/issues/140. + if a.Name == "listNetworks" && v == "name" { + pn(" p.p[\"keyword\"] = name") + } for _, ap := range a.Params { if ap.Required || isRequiredParam(a, ap) { pn(" p.p[\"%s\"] = %s", ap.Name, s.parseParamName(ap.Name)) diff --git a/test/GetNetworkByNameRegression_test.go b/test/GetNetworkByNameRegression_test.go new file mode 100644 index 00000000..7b270307 --- /dev/null +++ b/test/GetNetworkByNameRegression_test.go @@ -0,0 +1,117 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/apache/cloudstack-go/v2/cloudstack" +) + +func TestNetworkLookupCompatibility(t *testing.T) { + const networkID = "11111111-2222-4333-8444-555555555555" + const zoneID = "22222222-3333-4444-8555-666666666666" + existing := &cloudstack.Network{Id: networkID, Name: "existing"} + multiple := []*cloudstack.Network{ + {Id: "33333333-4444-4555-8666-777777777777", Name: "existing-extra"}, + {Id: "44444444-5555-4666-8777-888888888888", Name: "other"}, + existing, + } + cases := []struct { + name string + lookup string + networks []*cloudstack.Network + wantID string + wantCount int + }{ + {"single match", "existing", []*cloudstack.Network{existing}, networkID, 1}, + {"missing with one network", "missing", []*cloudstack.Network{existing}, "", 0}, + {"exact match among partial matches", "existing", multiple, networkID, 2}, + {"missing with multiple networks", "missing", multiple, "", 0}, + } + + for _, supportsName := range []bool{false, true} { + version := "legacy" + if supportsName { + version = "current" + } + t.Run(version, func(t *testing.T) { + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + wantCount := tc.wantCount + if supportsName && wantCount > 1 { + wantCount = 1 + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + q := r.URL.Query() + if q.Get("command") != "listNetworks" || q.Get("zoneid") != zoneID { + t.Errorf("unexpected request parameters: %v", q) + http.Error(w, "unexpected request parameters", http.StatusBadRequest) + return + } + var matches []*cloudstack.Network + for _, network := range tc.networks { + if id := q.Get("id"); id != "" && network.Id != id { + continue + } + if keyword := q.Get("keyword"); !strings.Contains(network.Name, keyword) { + continue + } + if name := q.Get("name"); supportsName && name != "" && network.Name != name { + continue + } + matches = append(matches, network) + } + w.Header().Set("Content-Type", "application/json") + response := map[string]*cloudstack.ListNetworksResponse{ + "listnetworksresponse": {Count: len(matches), Networks: matches}, + } + if err := json.NewEncoder(w).Encode(response); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer server.Close() + client := cloudstack.NewClient(server.URL, "APIKEY", "SECRETKEY", true) + + id, count, err := client.Network.GetNetworkID(tc.lookup, cloudstack.WithZone(zoneID)) + if id != tc.wantID || count != wantCount || (err != nil) != (tc.wantID == "") { + t.Errorf("GetNetworkID(%q) = %q, %d, %v; want %q, %d", + tc.lookup, id, count, err, tc.wantID, wantCount) + } + + network, count, err := client.Network.GetNetworkByName(tc.lookup, cloudstack.WithZone(zoneID)) + if tc.wantID == "" { + if network != nil || count != 0 || err == nil { + t.Errorf("GetNetworkByName(%q) = %+v, %d, %v; want no match", + tc.lookup, network, count, err) + } + } else if err != nil || network == nil || network.Id != tc.wantID || count != 1 { + t.Errorf("GetNetworkByName(%q) = %+v, %d, %v; want network %s", + tc.lookup, network, count, err, tc.wantID) + } + }) + } + }) + } +} From f6d0190343219de6f7fbfbbb0dd71d8ade12a490 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Fri, 11 Sep 2026 22:03:51 -0700 Subject: [PATCH 2/2] Reject a single partial network name match Require the requested name to match the sole keyword candidate before returning its ID. https://github.com/apache/cloudstack-go/pull/164#discussion_r3994517197 Signed-off-by: 1fanwang <1fannnw@gmail.com> --- ci/ci_test.go | 17 +++++++++++++++++ cloudstack/NetworkService.go | 2 +- generate/generate.go | 6 +++++- test/GetNetworkByNameRegression_test.go | 6 +++++- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ci/ci_test.go b/ci/ci_test.go index f8b3fe92..4b157d88 100644 --- a/ci/ci_test.go +++ b/ci/ci_test.go @@ -123,6 +123,23 @@ func TestCloudstackAPI(t *testing.T) { } }) } + t.Run("PartialName", func(t *testing.T) { + name := strings.TrimPrefix(resources.networkName, "ci-") + id, count, err := client.Network.GetNetworkID(name) + t.Logf("GetNetworkID(%q): id=%q count=%d error=%v", name, id, count, err) + if id != "" || err == nil || count < 0 || count > 1 { + t.Error("partial name must not resolve to a network") + } + network, count, err := client.Network.GetNetworkByName(name) + id = "" + if network != nil { + id = network.Id + } + t.Logf("GetNetworkByName(%q): id=%q count=%d error=%v", name, id, count, err) + if network != nil || err == nil || count < 0 || count > 1 { + t.Error("partial name must not resolve to a network") + } + }) }) // Run the actual tests diff --git a/cloudstack/NetworkService.go b/cloudstack/NetworkService.go index dcd4b183..5854ac47 100644 --- a/cloudstack/NetworkService.go +++ b/cloudstack/NetworkService.go @@ -4872,7 +4872,7 @@ func (s *NetworkService) GetNetworkID(name string, opts ...OptionFunc) (string, return "", l.Count, fmt.Errorf("No match found for %s: %+v", name, l) } - if l.Count == 1 { + if l.Count == 1 && l.Networks[0].Name == name { return l.Networks[0].Id, l.Count, nil } diff --git a/generate/generate.go b/generate/generate.go index 287eb694..27f83c69 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -1703,7 +1703,11 @@ func (s *service) generateHelperFuncs(a *API) { pn(" return \"\", l.Count, fmt.Errorf(\"No match found for %%s: %%+v\", %s, l)", v) pn(" }") pn("") - pn(" if l.Count == 1 {") + if a.Name == "listNetworks" { + pn(" if l.Count == 1 && l.%s[0].Name == %s {", ln, v) + } else { + pn(" if l.Count == 1 {") + } pn(" return l.%s[0].Id, l.Count, nil", ln) pn(" }") pn("") diff --git a/test/GetNetworkByNameRegression_test.go b/test/GetNetworkByNameRegression_test.go index 7b270307..f6254cd0 100644 --- a/test/GetNetworkByNameRegression_test.go +++ b/test/GetNetworkByNameRegression_test.go @@ -46,6 +46,7 @@ func TestNetworkLookupCompatibility(t *testing.T) { wantCount int }{ {"single match", "existing", []*cloudstack.Network{existing}, networkID, 1}, + {"single partial match", "exist", []*cloudstack.Network{existing}, "", 1}, {"missing with one network", "missing", []*cloudstack.Network{existing}, "", 0}, {"exact match among partial matches", "existing", multiple, networkID, 2}, {"missing with multiple networks", "missing", multiple, "", 0}, @@ -63,6 +64,9 @@ func TestNetworkLookupCompatibility(t *testing.T) { if supportsName && wantCount > 1 { wantCount = 1 } + if supportsName && tc.wantID == "" { + wantCount = 0 + } server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { q := r.URL.Query() if q.Get("command") != "listNetworks" || q.Get("zoneid") != zoneID { @@ -102,7 +106,7 @@ func TestNetworkLookupCompatibility(t *testing.T) { network, count, err := client.Network.GetNetworkByName(tc.lookup, cloudstack.WithZone(zoneID)) if tc.wantID == "" { - if network != nil || count != 0 || err == nil { + if network != nil || count != wantCount || err == nil { t.Errorf("GetNetworkByName(%q) = %+v, %d, %v; want no match", tc.lookup, network, count, err) }