lib/promscrape: properly implement ScrapeConfig.clone()

Previously ScrapeConfig.clone() was improperly copying promauth.Secret fields -
their contents was replaced with `<secret>` value.

This led to inability to use passwords and secrets in `-promscrape.config` file.
The bug has been introduced in v1.77.0 in the commit 67b10896d2

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2551

docs/CHANGELOG.md

@@ -19,11 +19,12 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): expose `vmagent_remotewrite_global_rows_pushed_before_relabel_total` and `vmagent_remotewrite_rows_pushed_after_relabel_total` metrics at `http://vmagent:8429/metrics`, which can be used for monitoring the rate of rows (aka samples) pushed to remote storage before and after the relabeling via `-remoteWrite.relabelConfig` and `-remoteWrite.urlRelabelConfig`. See [relabeling docs](https://docs.victoriametrics.com/vmagent.html#relabeling) for details.
 * FEATURE: [vmctl](https://docs.victoriametrics.com/vmctl.html): add ability to skip `db` label during InfluxDB data import when `influx-skip-database-label` option is used. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/2544). Thanks to @mback2k .
 
+* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly process passwords and secrets specified in the file pointed by `-promscrape.config` command-line flag. All the passwords and secrets were mistakenly replaced with `<secret>` string in `v1.77.0`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2551).
+* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): rename `vmagent_remote_write_rate_limit_reached_total` metric to `vmagent_remotewrite_rate_limit_reached_total`, so its name is consistent with the rest of `vmagent_remotewrite_` metrics.
+* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): rename `promscrape_stale_samples_created_total` metric to `vm_promscrape_stale_samples_created_total`, so its name is consistent with the rest of `vm_promscrape_` metrics.
 * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): properly import InfluxDB measurements if they contain `db` tag. Previously this could result in incomplete import of measurmenet tags. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/2536). Thanks to @mback2k for the bugfix.
 * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): do not reset the selected relative time range when entering new query. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2402#issuecomment-1115817302).
 * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): properly show the graph when clicking on `Prometheus` link in Grafana graph editor. Previously the graph wasn't shown until clicking on the `Graph` tab. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2402#issuecomment-1115830648).
-* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): rename `vmagent_remote_write_rate_limit_reached_total` metric to `vmagent_remotewrite_rate_limit_reached_total`, so its name is consistent with the rest of `vmagent_remotewrite_` metrics.
-* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): rename `promscrape_stale_samples_created_total` metric to `vm_promscrape_stale_samples_created_total`, so its name is consistent with the rest of `vm_promscrape_` metrics.
 * BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): disallow writing backups to `-storageDataPath` directory, since this directory is managed solely by VictoriaMetrics or `vmstorage`. Other apps shouldn't write into this directory. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2503).
 
 

lib/promauth/config.go

@@ -26,7 +26,7 @@ import (
 // This is needed for hiding secret strings in /config page output.
 // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1764
 type Secret struct {
-	s string
+	S string
 }
 
 // NewSecret returns new secret for s.
@@ -35,7 +35,7 @@ func NewSecret(s string) *Secret {
 		return nil
 	}
 	return &Secret{
-		s: s,
+		S: s,
 	}
 }
 
@@ -52,7 +52,7 @@ func (s *Secret) UnmarshalYAML(f func(interface{}) error) error {
 	if err := f(&secret); err != nil {
 		return fmt.Errorf("cannot parse secret: %w", err)
 	}
-	s.s = secret
+	s.S = secret
 	return nil
 }
 
@@ -61,7 +61,7 @@ func (s *Secret) String() string {
 	if s == nil {
 		return ""
 	}
-	return s.s
+	return s.S
 }
 
 // TLSConfig represents TLS config.

lib/promrelabel/config.go

@@ -38,7 +38,7 @@ type RelabelConfig struct {
 //
 // regex: "foo|bar"
 type MultiLineRegex struct {
-	s string
+	S string
 }
 
 // UnmarshalYAML unmarshals mlr from YAML passed to f.
@@ -51,7 +51,7 @@ func (mlr *MultiLineRegex) UnmarshalYAML(f func(interface{}) error) error {
 	if err != nil {
 		return err
 	}
-	mlr.s = s
+	mlr.S = s
 	return nil
 }
 
@@ -88,7 +88,7 @@ func stringValue(v interface{}) (string, error) {
 
 // MarshalYAML marshals mlr to YAML.
 func (mlr *MultiLineRegex) MarshalYAML() (interface{}, error) {
-	a := strings.Split(mlr.s, "|")
+	a := strings.Split(mlr.S, "|")
 	if len(a) == 1 {
 		return a[0], nil
 	}
@@ -179,7 +179,7 @@ func parseRelabelConfig(rc *RelabelConfig) (*parsedRelabelConfig, error) {
 	regexCompiled := defaultRegexForRelabelConfig
 	regexOriginalCompiled := defaultOriginalRegexForRelabelConfig
 	if rc.Regex != nil {
-		regex := rc.Regex.s
+		regex := rc.Regex.S
 		regexOrig := regex
 		if rc.Action != "replace_all" && rc.Action != "labelmap_all" {
 			regex = "^(?:" + regex + ")$"
@@ -243,7 +243,7 @@ func parseRelabelConfig(rc *RelabelConfig) (*parsedRelabelConfig, error) {
 			return nil, fmt.Errorf("unexpected `modulus` for `action=hashmod`: %d; must be greater than 0", modulus)
 		}
 	case "keep_metrics":
-		if (rc.Regex == nil || rc.Regex.s == "") && rc.If == nil {
+		if (rc.Regex == nil || rc.Regex.S == "") && rc.If == nil {
 			return nil, fmt.Errorf("`regex` must be non-empty for `action=keep_metrics`")
 		}
 		if len(sourceLabels) > 0 {
@@ -252,7 +252,7 @@ func parseRelabelConfig(rc *RelabelConfig) (*parsedRelabelConfig, error) {
 		sourceLabels = []string{"__name__"}
 		action = "keep"
 	case "drop_metrics":
-		if (rc.Regex == nil || rc.Regex.s == "") && rc.If == nil {
+		if (rc.Regex == nil || rc.Regex.S == "") && rc.If == nil {
 			return nil, fmt.Errorf("`regex` must be non-empty for `action=drop_metrics`")
 		}
 		if len(sourceLabels) > 0 {

lib/promrelabel/config_test.go

@@ -128,7 +128,7 @@ func TestParseRelabelConfigsFailure(t *testing.T) {
 				SourceLabels: []string{"aaa"},
 				TargetLabel:  "xxx",
 				Regex: &MultiLineRegex{
-					s: "foo[bar",
+					S: "foo[bar",
 				},
 			},
 		})
@@ -248,7 +248,7 @@ func TestParseRelabelConfigsFailure(t *testing.T) {
 				Action:       "drop_metrics",
 				SourceLabels: []string{"foo"},
 				Regex: &MultiLineRegex{
-					s: "bar",
+					S: "bar",
 				},
 			},
 		})
@@ -266,7 +266,7 @@ func TestParseRelabelConfigsFailure(t *testing.T) {
 				Action:       "keep_metrics",
 				SourceLabels: []string{"foo"},
 				Regex: &MultiLineRegex{
-					s: "bar",
+					S: "bar",
 				},
 			},
 		})

lib/promrelabel/if_expression.go

@@ -1,6 +1,7 @@
 package promrelabel
 
 import (
+	"encoding/json"
 	"fmt"
 	"regexp"
 
@@ -36,6 +37,20 @@ func (ie *IfExpression) Parse(s string) error {
 	return nil
 }
 
+// UnmarshalJSON unmarshals ie from JSON data.
+func (ie *IfExpression) UnmarshalJSON(data []byte) error {
+	var s string
+	if err := json.Unmarshal(data, &s); err != nil {
+		return err
+	}
+	return ie.Parse(s)
+}
+
+// MarshalJSON marshals ie to JSON.
+func (ie *IfExpression) MarshalJSON() ([]byte, error) {
+	return json.Marshal(ie.s)
+}
+
 // UnmarshalYAML unmarshals ie from YAML passed to f.
 func (ie *IfExpression) UnmarshalYAML(f func(interface{}) error) error {
 	var s string

lib/promscrape/client.go

@@ -71,7 +71,7 @@ func newClient(sw *ScrapeWork) *client {
 		// Send full sw.ScrapeURL in requests to a proxy host for non-TLS scrape targets
 		// like net/http package from Go does.
 		// See https://en.wikipedia.org/wiki/Proxy_server#Web_proxy_servers
-		pu := proxyURL.URL()
+		pu := proxyURL.GetURL()
 		host = pu.Host
 		requestURI = sw.ScrapeURL
 		isTLS = pu.Scheme == "https"
@@ -110,7 +110,7 @@ func newClient(sw *ScrapeWork) *client {
 	}
 	var sc *http.Client
 	var proxyURLFunc func(*http.Request) (*url.URL, error)
-	if pu := sw.ProxyURL.URL(); pu != nil {
+	if pu := sw.ProxyURL.GetURL(); pu != nil {
 		proxyURLFunc = http.ProxyURL(pu)
 	}
 	sc = &http.Client{

lib/promscrape/config.go

@@ -1,6 +1,7 @@
 package promscrape
 
 import (
+	"encoding/json"
 	"flag"
 	"fmt"
 	"net/url"
@@ -161,17 +162,17 @@ func (cfg *Config) mustRestart(prevCfg *Config) {
 }
 
 func areEqualScrapeConfigs(a, b *ScrapeConfig) bool {
-	sa := a.marshal()
-	sb := b.marshal()
+	sa := a.marshalJSON()
+	sb := b.marshalJSON()
 	return string(sa) == string(sb)
 }
 
-func (sc *ScrapeConfig) unmarshal(data []byte) error {
-	return yaml.UnmarshalStrict(data, sc)
+func (sc *ScrapeConfig) unmarshalJSON(data []byte) error {
+	return json.Unmarshal(data, sc)
 }
 
-func (sc *ScrapeConfig) marshal() []byte {
-	data, err := yaml.Marshal(sc)
+func (sc *ScrapeConfig) marshalJSON() []byte {
+	data, err := json.Marshal(sc)
 	if err != nil {
 		logger.Panicf("BUG: cannot marshal ScrapeConfig: %s", err)
 	}
@@ -430,9 +431,9 @@ func (cfg *Config) parseData(data []byte, path string) ([]byte, error) {
 }
 
 func (sc *ScrapeConfig) clone() *ScrapeConfig {
-	data := sc.marshal()
+	data := sc.marshalJSON()
 	var scCopy ScrapeConfig
-	if err := scCopy.unmarshal(data); err != nil {
+	if err := scCopy.unmarshalJSON(data); err != nil {
 		logger.Panicf("BUG: cannot unmarshal scrape config: %s", err)
 	}
 	return &scCopy

lib/promscrape/config_test.go

@@ -11,6 +11,9 @@ import (
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promauth"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promrelabel"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promscrape/discovery/gce"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/proxy"
 )
 
@@ -1853,3 +1856,69 @@ func equalStaticConfigForScrapeWorks(a, b []*ScrapeWork) bool {
 	}
 	return true
 }
+
+func TestScrapeConfigClone(t *testing.T) {
+	f := func(sc *ScrapeConfig) {
+		t.Helper()
+		scCopy := sc.clone()
+		if !reflect.DeepEqual(sc, scCopy) {
+			t.Fatalf("unexpected result after unmarshalJSON() for JSON:\n%s", sc.marshalJSON())
+		}
+	}
+
+	f(&ScrapeConfig{})
+
+	bFalse := false
+	var ie promrelabel.IfExpression
+	if err := ie.Parse(`{foo=~"bar",baz!="z"}`); err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	f(&ScrapeConfig{
+		JobName:         "foo",
+		ScrapeInterval:  promutils.NewDuration(time.Second * 47),
+		HonorLabels:     true,
+		HonorTimestamps: &bFalse,
+		Params: map[string][]string{
+			"foo": {"bar", "baz"},
+		},
+		HTTPClientConfig: promauth.HTTPClientConfig{
+			Authorization: &promauth.Authorization{
+				Credentials: promauth.NewSecret("foo"),
+			},
+			BasicAuth: &promauth.BasicAuthConfig{
+				Username: "user_x",
+				Password: promauth.NewSecret("pass_x"),
+			},
+			BearerToken: promauth.NewSecret("zx"),
+			OAuth2: &promauth.OAuth2Config{
+				ClientSecret: promauth.NewSecret("aa"),
+				Scopes:       []string{"foo", "bar"},
+				TLSConfig: &promauth.TLSConfig{
+					CertFile: "foo",
+				},
+			},
+			TLSConfig: &promauth.TLSConfig{
+				KeyFile: "aaa",
+			},
+		},
+		ProxyURL: proxy.MustNewURL("https://foo.bar:3434/assdf/dsfd?sdf=dsf"),
+		RelabelConfigs: []promrelabel.RelabelConfig{{
+			SourceLabels: []string{"foo", "aaa"},
+			Regex: &promrelabel.MultiLineRegex{
+				S: "foo\nbar",
+			},
+			If: &ie,
+		}},
+		SampleLimit: 10,
+		GCESDConfigs: []gce.SDConfig{{
+			Project: "foo",
+			Zone: gce.ZoneYAML{
+				Zones: []string{"a", "b"},
+			},
+		}},
+		StreamParse: true,
+		ProxyClientConfig: promauth.ProxyClientConfig{
+			BearerTokenFile: "foo",
+		},
+	})
+}

lib/promscrape/discovery/gce/api.go

@@ -47,7 +47,7 @@ func newAPIConfig(sdc *SDConfig) (*apiConfig, error) {
 		project = proj
 		logger.Infof("autodetected the current GCE project: %q", project)
 	}
-	zones := sdc.Zone.zones
+	zones := sdc.Zone.Zones
 	if len(zones) == 0 {
 		// Autodetect the current zone.
 		zone, err := getCurrentZone()

lib/promscrape/discovery/gce/gce.go

@@ -26,7 +26,7 @@ type SDConfig struct {
 
 // ZoneYAML holds info about zones.
 type ZoneYAML struct {
-	zones []string
+	Zones []string
 }
 
 // UnmarshalYAML implements yaml.Unmarshaler
@@ -50,13 +50,13 @@ func (z *ZoneYAML) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	default:
 		return fmt.Errorf("unexpected type unmarshaled for ZoneYAML: %T; contents: %#v", v, v)
 	}
-	z.zones = zones
+	z.Zones = zones
 	return nil
 }
 
 // MarshalYAML implements yaml.Marshaler
 func (z ZoneYAML) MarshalYAML() (interface{}, error) {
-	return z.zones, nil
+	return z.Zones, nil
 }
 
 // GetLabels returns gce labels according to sdc.

lib/promscrape/discovery/gce/gce_test.go

@@ -10,7 +10,7 @@ func TestMarshallingSDConfigWithZoneYAML(t *testing.T) {
 	sdConfig := SDConfig{
 		Project: "test-project",
 		Zone: ZoneYAML{
-			zones: []string{"zone-a", "zone-b"},
+			Zones: []string{"zone-a", "zone-b"},
 		},
 	}
 

lib/promscrape/discovery/kubernetes/api_watcher.go

@@ -75,7 +75,7 @@ func newAPIWatcher(apiServer string, ac *promauth.Config, sdc *SDConfig, swcFunc
 	}
 	selectors := sdc.Selectors
 	attachNodeMetadata := sdc.AttachMetadata.Node
-	proxyURL := sdc.ProxyURL.URL()
+	proxyURL := sdc.ProxyURL.GetURL()
 	gw := getGroupWatcher(apiServer, ac, namespaces, selectors, attachNodeMetadata, proxyURL)
 	role := sdc.role()
 	return &apiWatcher{

lib/promscrape/discoveryutils/client.go

@@ -75,7 +75,7 @@ func NewClient(apiServer string, ac *promauth.Config, proxyURL *proxy.URL, proxy
 		// Send full urls in requests to a proxy host for non-TLS apiServer
 		// like net/http package from Go does.
 		// See https://en.wikipedia.org/wiki/Proxy_server#Web_proxy_servers
-		pu := proxyURL.URL()
+		pu := proxyURL.GetURL()
 		hostPort = pu.Host
 		isTLS = pu.Scheme == "https"
 		if isTLS {

lib/promutils/duration.go

@@ -8,19 +8,19 @@ import (
 
 // Duration is duration, which must be used in Prometheus-compatible yaml configs.
 type Duration struct {
-	d time.Duration
+	D time.Duration
 }
 
 // NewDuration returns Duration for given d.
 func NewDuration(d time.Duration) *Duration {
 	return &Duration{
-		d: d,
+		D: d,
 	}
 }
 
 // MarshalYAML implements yaml.Marshaler interface.
 func (pd Duration) MarshalYAML() (interface{}, error) {
-	return pd.d.String(), nil
+	return pd.D.String(), nil
 }
 
 // UnmarshalYAML implements yaml.Unmarshaler interface.
@@ -33,7 +33,7 @@ func (pd *Duration) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if err != nil {
 		return err
 	}
-	pd.d = time.Duration(ms) * time.Millisecond
+	pd.D = time.Duration(ms) * time.Millisecond
 	return nil
 }
 
@@ -42,7 +42,7 @@ func (pd *Duration) Duration() time.Duration {
 	if pd == nil {
 		return 0
 	}
-	return pd.d
+	return pd.D
 }
 
 // ParseDuration parses duration string in Prometheus format

lib/proxy/proxy.go

@@ -19,7 +19,7 @@ import (
 
 // URL implements YAML.Marshaler and yaml.Unmarshaler interfaces for url.URL.
 type URL struct {
-	url *url.URL
+	URL *url.URL
 }
 
 // MustNewURL returns new URL for the given u.
@@ -29,31 +29,31 @@ func MustNewURL(u string) *URL {
 		logger.Panicf("BUG: cannot parse u=%q: %s", u, err)
 	}
 	return &URL{
-		url: pu,
+		URL: pu,
 	}
 }
 
-// URL return the underlying url.
-func (u *URL) URL() *url.URL {
-	if u == nil || u.url == nil {
+// GetURL return the underlying url.
+func (u *URL) GetURL() *url.URL {
+	if u == nil || u.URL == nil {
 		return nil
 	}
-	return u.url
+	return u.URL
 }
 
 // IsHTTPOrHTTPS returns true if u is http or https
 func (u *URL) IsHTTPOrHTTPS() bool {
-	pu := u.URL()
+	pu := u.GetURL()
 	if pu == nil {
 		return false
 	}
-	scheme := u.url.Scheme
+	scheme := u.URL.Scheme
 	return scheme == "http" || scheme == "https"
 }
 
 // String returns string representation of u.
 func (u *URL) String() string {
-	pu := u.URL()
+	pu := u.GetURL()
 	if pu == nil {
 		return ""
 	}
@@ -66,10 +66,10 @@ func (u *URL) GetAuthHeader(ac *promauth.Config) string {
 	if ac != nil {
 		authHeader = ac.GetAuthHeader()
 	}
-	if u == nil || u.url == nil {
+	if u == nil || u.URL == nil {
 		return authHeader
 	}
-	pu := u.url
+	pu := u.URL
 	if pu.User != nil && len(pu.User.Username()) > 0 {
 		userPasswordEncoded := base64.StdEncoding.EncodeToString([]byte(pu.User.String()))
 		authHeader = "Basic " + userPasswordEncoded
@@ -79,10 +79,10 @@ func (u *URL) GetAuthHeader(ac *promauth.Config) string {
 
 // MarshalYAML implements yaml.Marshaler interface.
 func (u *URL) MarshalYAML() (interface{}, error) {
-	if u.url == nil {
+	if u.URL == nil {
 		return nil, nil
 	}
-	return u.url.String(), nil
+	return u.URL.String(), nil
 }
 
 // UnmarshalYAML implements yaml.Unmarshaler interface.
@@ -95,16 +95,16 @@ func (u *URL) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if err != nil {
 		return fmt.Errorf("cannot parse proxy_url=%q as *url.URL: %w", s, err)
 	}
-	u.url = parsedURL
+	u.URL = parsedURL
 	return nil
 }
 
 // NewDialFunc returns dial func for the given u and ac.
 func (u *URL) NewDialFunc(ac *promauth.Config) (fasthttp.DialFunc, error) {
-	if u == nil || u.url == nil {
+	if u == nil || u.URL == nil {
 		return defaultDialFunc, nil
 	}
-	pu := u.url
+	pu := u.URL
 	switch pu.Scheme {
 	case "http", "https", "socks5", "tls+socks5":
 	default: