Repository: VictoriaMetrics/VictoriaMetrics
Pull request #5735 (2024-01-31 06:48:48)

clean unused code

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5735?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) Report
Attention: `1 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`ec0ca8e`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/ec0ca8e7eb9b223fdeea23e794137a03f1a009a9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.30% compared to head [(`dbc8e4c`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5735?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.27%.
> Report is 1 commits behind head on master.

| [Files](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5735?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) | Patch % | Lines |
|---|---|---|
| [lib/promscrape/scraper.go](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5735?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics#diff-bGliL3Byb21zY3JhcGUvc2NyYXBlci5nbw==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5735?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5735      +/-   ##
==========================================
- Coverage   60.30%   60.27%   -0.04%     
==========================================
  Files         409      409              
  Lines       76292    76292              
==========================================
- Hits        46009    45984      -25     
- Misses      27739    27759      +20     
- Partials     2544     2549       +5     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5735?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).

Haleygo: @helen-frank 
Please sign this commit https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5735/commits/e9c8c2dd33b1c8e982d027546e69880a9b97abc0.
helen-frank: > @helen-frank Please sign this commit [e9c8c2d](https://github.com/VictoriaMetrics/VictoriaMetrics/commit/e9c8c2dd33b1c8e982d027546e69880a9b97abc0).

solved

Patch:
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index a7b8bdc56..c851e0061 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -28,6 +28,8 @@ The sandbox cluster installation is running under the constant load generated by
 
 ## tip
 
+* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): add support for `username_file` option at `basic_auth` section in [`scrape_configs`](https://docs.victoriametrics.com/sd_configs/#http-api-client-options). See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5511). Thanks to @wasim-nihal for [the initial implementation](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5720).
+
 * BUGFIX: fix `runtime error: slice bounds out of range` panic, which can occur during query execution. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5733). The bug has been introduced in `v1.97.0`.
 * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly handle `avg_over_time({some_filter}[d]) keep_metric_names` queries, where [`some_filter`](https://docs.victoriametrics.com/keyconcepts/#filtering) matches multiple time series with multiple names, while `d` is bigger or equal to `3h`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5556).
 * BUGFIX: dashboards/single: fix typo in query for `version` annotation which falsely produced many version change events.
diff --git a/docs/sd_configs.md b/docs/sd_configs.md
index cd6001c7f..ba879a776 100644
--- a/docs/sd_configs.md
+++ b/docs/sd_configs.md
@@ -821,6 +821,7 @@ scrape_configs:
     #
     # basic_auth:
     #  username: "..."
+    #  username_file: "..."  # is mutually-exclusive with username
     #  password: "..."
     #  password_file: "..."  # is mutually-exclusive with password
 
@@ -1625,8 +1626,9 @@ and in the majority of [supported service discovery configs](#supported-service-
     # basic_auth is an optional HTTP basic authentication configuration.
     # basic_auth:
     #   username: "..."
+    #   username_file: "..."  # is mutually-exclusive with username
     #   password: "..."
-    #   password_file: "..."
+    #   password_file: "..."  # is mutually-exclusive with password
 
     # bearer_token is an optional Bearer token to send in every HTTP API request during service discovery.
     # bearer_token: "..."
@@ -1663,8 +1665,9 @@ and in the majority of [supported service discovery configs](#supported-service-
     # proxy_basic_auth is an optional HTTP basic authentication configuration for the proxy_url.
     # proxy_basic_auth:
     #   username: "..."
+    #   username_file: "..."  # is mutually-exclusive with username
     #   password: "..."
-    #   password_file: "..."
+    #   password_file: "..."  # is mutually-exclusive with password
 
     # proxy_bearer_token is an optional Bearer token to send to proxy_url.
     # proxy_bearer_token: "..."
diff --git a/lib/promauth/config.go b/lib/promauth/config.go
index 6b95a8017..4fafb3d7a 100644
--- a/lib/promauth/config.go
+++ b/lib/promauth/config.go
@@ -92,7 +92,8 @@ type Authorization struct {
 
 // BasicAuthConfig represents basic auth config.
 type BasicAuthConfig struct {
-	Username     string  `yaml:"username"`
+	Username     string  `yaml:"username,omitempty"`
+	UsernameFile string  `yaml:"username_file,omitempty"`
 	Password     *Secret `yaml:"password,omitempty"`
 	PasswordFile string  `yaml:"password_file,omitempty"`
 }
@@ -643,35 +644,54 @@ func (actx *authContext) initFromAuthorization(baseDir string, az *Authorization
 }
 
 func (actx *authContext) initFromBasicAuthConfig(baseDir string, ba *BasicAuthConfig) error {
-	if ba.Username == "" {
-		return fmt.Errorf("missing `username` in `basic_auth` section")
+	username := ba.Username
+	usernameFile := ba.UsernameFile
+	password := ""
+	if ba.Password != nil {
+		password = ba.Password.S
 	}
-	if ba.PasswordFile == "" {
-		// See https://en.wikipedia.org/wiki/Basic_access_authentication
-		token := ba.Username + ":" + ba.Password.String()
-		token64 := base64.StdEncoding.EncodeToString([]byte(token))
-		ah := "Basic " + token64
-		actx.getAuthHeader = func() (string, error) {
-			return ah, nil
-		}
-		actx.authHeaderDigest = fmt.Sprintf("basic(username=%q, password=%q)", ba.Username, ba.Password)
-		return nil
+	passwordFile := ba.PasswordFile
+	if username == "" && usernameFile == "" {
+		return fmt.Errorf("missing `username` and `username_file` in `basic_auth` section; please specify one; " +
+			"see https://docs.victoriametrics.com/sd_configs.html#http-api-client-options")
 	}
-	if ba.Password != nil {
-		return fmt.Errorf("both `password`=%q and `password_file`=%q are set in `basic_auth` section", ba.Password, ba.PasswordFile)
+	if username != "" && usernameFile != "" {
+		return fmt.Errorf("both `username` and `username_file` are set in `basic_auth` section; please specify only one; " +
+			"see https://docs.victoriametrics.com/sd_configs.html#http-api-client-options")
+	}
+	if password != "" && passwordFile != "" {
+		return fmt.Errorf("both `password` and `password_file` are set in `basic_auth` section; please specify only one; " +
+			"see https://docs.victoriametrics.com/sd_configs.html#http-api-client-options")
+	}
+	if usernameFile != "" {
+		usernameFile = fscore.GetFilepath(baseDir, usernameFile)
+	}
+	if passwordFile != "" {
+		passwordFile = fscore.GetFilepath(baseDir, passwordFile)
 	}
-	filePath := fscore.GetFilepath(baseDir, ba.PasswordFile)
 	actx.getAuthHeader = func() (string, error) {
-		password, err := fscore.ReadPasswordFromFileOrHTTP(filePath)
-		if err != nil {
-			return "", fmt.Errorf("cannot read password from `password_file`=%q set in `basic_auth` section: %w", ba.PasswordFile, err)
+		usernameLocal := username
+		if usernameFile != "" {
+			s, err := fscore.ReadPasswordFromFileOrHTTP(usernameFile)
+			if err != nil {
+				return "", fmt.Errorf("cannot read username from `username_file`=%q: %w", usernameFile, err)
+			}
+			usernameLocal = s
+		}
+		passwordLocal := password
+		if passwordFile != "" {
+			s, err := fscore.ReadPasswordFromFileOrHTTP(passwordFile)
+			if err != nil {
+				return "", fmt.Errorf("cannot read password from `password_file`=%q: %w", passwordFile, err)
+			}
+			passwordLocal = s
 		}
 		// See https://en.wikipedia.org/wiki/Basic_access_authentication
-		token := ba.Username + ":" + password
+		token := usernameLocal + ":" + passwordLocal
 		token64 := base64.StdEncoding.EncodeToString([]byte(token))
 		return "Basic " + token64, nil
 	}
-	actx.authHeaderDigest = fmt.Sprintf("basic(username=%q, passwordFile=%q)", ba.Username, filePath)
+	actx.authHeaderDigest = fmt.Sprintf("basic(username=%q, usernameFile=%q, password=%q, passwordFile=%q)", username, usernameFile, password, passwordFile)
 	return nil
 }
 
diff --git a/lib/promscrape/scraper.go b/lib/promscrape/scraper.go
index ad6319203..183dcd0e1 100644
--- a/lib/promscrape/scraper.go
+++ b/lib/promscrape/scraper.go
@@ -410,7 +410,7 @@ func (sg *scraperGroup) update(sws []*ScrapeWork) {
 		sg.scrapersStarted.Inc()
 		sg.wg.Add(1)
 		tsmGlobal.Register(&sc.sw)
-		go func(sw *ScrapeWork) {
+		go func() {
 			defer func() {
 				sg.wg.Done()
 				close(sc.stoppedCh)
@@ -419,7 +419,7 @@ func (sg *scraperGroup) update(sws []*ScrapeWork) {
 			tsmGlobal.Unregister(&sc.sw)
 			sg.activeScrapers.Dec()
 			sg.scrapersStopped.Inc()
-		}(sw)
+		}()
 		key := sw.key()
 		sg.m[key] = sc
 		additionsCount++


Test patch:
diff --git a/lib/promauth/config_test.go b/lib/promauth/config_test.go
index 861ab3265..0f51a223c 100644
--- a/lib/promauth/config_test.go
+++ b/lib/promauth/config_test.go
@@ -47,7 +47,14 @@ basic_auth:
   password: pass
 `)
 
-	// basic_auth: password and password_file are set
+	// basic_auth: both username and username_file are set
+	f(`
+basic_auth:
+  username: foo
+  username_file: testdata/test_secretfile.txt
+`)
+
+	// basic_auth: both password and password_file are set
 	f(`
 basic_auth:
   username: user
@@ -345,7 +352,14 @@ oauth2:
     ca_file: non-existing-file
 `)
 
-	// basic auth via non-existing file
+	// basic auth via non-existing username file
+	f(`
+basic_auth:
+  username_file: non-existing-file
+  password: foobar
+`)
+
+	// basic auth via non-existing password file
 	f(`
 basic_auth:
   username: user
@@ -464,13 +478,26 @@ basic_auth:
   password: password
 `, "Basic dXNlcjpwYXNzd29yZA==")
 
-	// basic auth via file
+	// basic auth via username file
+	f(`
+basic_auth:
+  username_file: testdata/test_secretfile.txt
+`, "Basic c2VjcmV0LWNvbnRlbnQ6")
+
+	// basic auth via password file
 	f(`
 basic_auth:
   username: user
   password_file: testdata/test_secretfile.txt
 `, "Basic dXNlcjpzZWNyZXQtY29udGVudA==")
 
+	// basic auth via username file and password file
+	f(`
+basic_auth:
+  username_file: testdata/test_secretfile.txt
+  password_file: testdata/test_secretfile.txt
+`, "Basic c2VjcmV0LWNvbnRlbnQ6c2VjcmV0LWNvbnRlbnQ=")
+
 	// inline authorization config
 	f(`
 authorization: