Repository: VictoriaMetrics/VictoriaMetrics
Pull request #6787 (2024-08-09 04:05:11)

fix: [vmagent] fix service discovery of Azure response contains nextLink
### Describe Your Changes

Fix https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6784

The root cause is that when checking the host, `nextURL.Host` only contains the host, while `ac.c.APIServer()` is a user input and contains the schema (`https://`).

https://github.com/VictoriaMetrics/VictoriaMetrics/blob/0788be35eb5ccd659dc0bb7aa9d63ff2449f1c2f/lib/promscrape/discovery/azure/machine.go#L99

### Checklist

The following checks are **mandatory**:

- [x] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/).

### Note
The fix could also be simplified in the following 1-line hard-coded manner based on Azure SD context. Please let me know which approach you prefer.

```
		if nextURL.Host != "" && nextURL.Host != strings.TrimPrefix(ac.c.APIServer(), "https://") {
```


Patch:
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 5e0f3b2c3..5700f2bc2 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -35,6 +35,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
 * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent/): reduce memory usage when scraping targets with big response body. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6759).
 * FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup/), [vmrestore](https://docs.victoriametrics.com/vmrestore/), [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager/): use exponential backoff for retries when uploading or downloading data from S3. This should reduce the number of failed uploads and downloads when S3 is temporarily unavailable. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6732).
 
+* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent/) fix service discovery of Azure Virtual Machines for response contains `nextLink`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6784).
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert): respect HTTP headers defined in [notifier configuration file](https://docs.victoriametrics.com/vmalert/#notifier-configuration-file) for each request to notifiers. Previously, this param was ignored by mistake.
 * BUGFIX: [stream aggregation](https://docs.victoriametrics.com/stream-aggregation/): correctly apply `-streamAggr.dropInputLabels` when global stream deduplication is enabled without `-streamAggr.config`. Previously, `-remoteWrite.streamAggr.dropInputLabels` was used instead.
 * BUGFIX: [stream aggregation](https://docs.victoriametrics.com/stream-aggregation/): fix command-line flag `-remoteWrite.streamAggr.ignoreFirstIntervals` to accept multiple values and be applied per each corresponding `-remoteWrite.url`. Previously, this flag only could have been used globally for all URLs. 
diff --git a/lib/promscrape/discovery/azure/api.go b/lib/promscrape/discovery/azure/api.go
index 76eb2667d..e21257d69 100644
--- a/lib/promscrape/discovery/azure/api.go
+++ b/lib/promscrape/discovery/azure/api.go
@@ -67,6 +67,9 @@ type apiConfig struct {
 	tokenLock           sync.Mutex
 	token               string
 	tokenExpireDeadline time.Time
+
+	// apiServerHost is only used for verifying the `nextLink` in response of the list API.
+	apiServerHost string
 }
 
 type refreshTokenFunc func() (string, time.Duration, error)
@@ -114,8 +117,12 @@ func newAPIConfig(sdc *SDConfig, baseDir string) (*apiConfig, error) {
 	if err != nil {
 		return nil, fmt.Errorf("cannot create client for %q: %w", env.ResourceManagerEndpoint, err)
 	}
+	// It's already verified in discoveryutils.NewClient so no need to check err.
+	u, _ := url.Parse(c.APIServer())
+
 	cfg := &apiConfig{
 		c:              c,
+		apiServerHost:  u.Host,
 		port:           port,
 		resourceGroup:  sdc.ResourceGroup,
 		subscriptionID: sdc.SubscriptionID,
diff --git a/lib/promscrape/discovery/azure/machine.go b/lib/promscrape/discovery/azure/machine.go
index a9b86a8a7..491d80b36 100644
--- a/lib/promscrape/discovery/azure/machine.go
+++ b/lib/promscrape/discovery/azure/machine.go
@@ -96,8 +96,8 @@ func visitAllAPIObjects(ac *apiConfig, apiURL string, cb func(data json.RawMessa
 			return fmt.Errorf("cannot parse nextLink from response %q: %w", lar.NextLink, err)
 		}
 
-		if nextURL.Host != "" && nextURL.Host != ac.c.APIServer() {
-			return fmt.Errorf("unexpected nextLink host %q, expecting %q", nextURL.Host, ac.c.APIServer())
+		if nextURL.Host != "" && nextURL.Host != ac.apiServerHost {
+			return fmt.Errorf("unexpected nextLink host %q, expecting %q", nextURL.Host, ac.apiServerHost)
 		}
 
 		nextLinkURI = nextURL.RequestURI()


Test patch:
diff --git a/lib/promscrape/discovery/azure/machine_test.go b/lib/promscrape/discovery/azure/machine_test.go
index bb13a2f5a..06710c192 100644
--- a/lib/promscrape/discovery/azure/machine_test.go
+++ b/lib/promscrape/discovery/azure/machine_test.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"reflect"
 	"strings"
 	"testing"
@@ -13,6 +14,11 @@ import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promscrape/discoveryutils"
 )
 
+var (
+	testServerURL string
+	listAPICall   int
+)
+
 func TestGetVirtualMachinesSuccess(t *testing.T) {
 	prettifyVMs := func(src []virtualMachine) string {
 		var sb strings.Builder
@@ -41,39 +47,51 @@ func TestGetVirtualMachinesSuccess(t *testing.T) {
 		}
 		return sb.String()
 	}
-	f := func(name string, expectedVMs []virtualMachine, apiResponses [4]string) {
+	f := func(name string, expectedVMs []virtualMachine, apiResponses [5]string) {
 		t.Run(name, func(t *testing.T) {
 			testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				switch {
 				// list vms response
 				case strings.Contains(r.URL.Path, "/providers/Microsoft.Compute/virtualMachines"):
 					w.WriteHeader(http.StatusOK)
-					fmt.Fprintf(w, apiResponses[0])
+					if listAPICall == 0 {
+						// with nextLink
+						apiResponse := strings.Replace(apiResponses[0], "{nextLinkPlaceHolder}", testServerURL+"/providers/Microsoft.Compute/virtualMachines", 1)
+						fmt.Fprint(w, apiResponse)
+						listAPICall++
+					} else {
+						// without nextLink
+						fmt.Fprint(w, apiResponses[1])
+					}
 					// list scaleSets response
 				case strings.Contains(r.URL.RequestURI(), "/providers/Microsoft.Compute/virtualMachineScaleSets?api-version=2022-03-01"):
 					w.WriteHeader(http.StatusOK)
-					fmt.Fprintf(w, apiResponses[1])
+					fmt.Fprint(w, apiResponses[2])
 					// list scalesets vms response
 				case strings.Contains(r.URL.Path, "/providers/Microsoft.Compute/virtualMachineScaleSets/{virtualMachineScaleSetName}/virtualMach"):
 					w.WriteHeader(http.StatusOK)
-					fmt.Fprintf(w, apiResponses[2])
+					fmt.Fprint(w, apiResponses[3])
 					// nic response
 				case strings.Contains(r.URL.Path, "/networkInterfaces/"):
 					w.WriteHeader(http.StatusOK)
-					fmt.Fprintf(w, apiResponses[3])
+					fmt.Fprint(w, apiResponses[4])
 				default:
 					w.WriteHeader(http.StatusNotFound)
 					fmt.Fprintf(w, "API path not found: %s", r.URL.Path)
 				}
 			}))
 			defer testServer.Close()
+			testServerURL = testServer.URL
 			c, err := discoveryutils.NewClient(testServer.URL, nil, nil, nil, &promauth.HTTPClientConfig{})
 			if err != nil {
 				t.Fatalf("unexpected error at client create: %s", err)
 			}
+			u, _ := url.Parse(c.APIServer())
+
 			defer c.Stop()
 			ac := &apiConfig{
 				c:              c,
+				apiServerHost:  u.Host,
 				subscriptionID: "some-id",
 				refreshToken: func() (string, time.Duration, error) {
 					return "auth-token", 0, nil
@@ -102,7 +120,7 @@ func TestGetVirtualMachinesSuccess(t *testing.T) {
 			},
 			Tags: map[string]string{},
 		},
-	}, [4]string{
+	}, [5]string{
 		`
 {
   "value": [
@@ -193,6 +211,10 @@ func TestGetVirtualMachinesSuccess(t *testing.T) {
       "name": "{virtualMachineName}"
     }
   ],
+  "nextLink": "{nextLinkPlaceHolder}"
+}`,
+		`{
+  "value": [],
   "nextLink": ""
 }`,
 		`{}`,
@@ -255,7 +277,8 @@ func TestGetVirtualMachinesSuccess(t *testing.T) {
 			},
 			Tags: map[string]string{},
 		},
-	}, [4]string{
+	}, [5]string{
+		`{}`,
 		`{}`,
 		`{
   "value": [