From 3dbb19d624a3eb4ed874d2fbc37b15dfaadbcd55 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Mon, 6 Jun 2022 14:24:52 +0300 Subject: [PATCH] lib/promscrape/discovery/kubernetes: follow-up after 006b8c7534f97cf5379c9cea8292f9ca0c0b32d6 - make more clear error logs - simplify testing for newKubeConfig by passing only the path to kube_config file instead of SDConfig struct --- docs/CHANGELOG.md | 8 +- lib/promscrape/discovery/kubernetes/api.go | 17 ++-- .../discovery/kubernetes/kubeconfig.go | 95 +++++++++---------- .../discovery/kubernetes/kubeconfig_test.go | 31 +++--- .../discovery/kubernetes/kubernetes.go | 5 +- 5 files changed, 75 insertions(+), 81 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a876a54fbd..9d1bdb9b8d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -16,14 +16,14 @@ The following tip changes can be tested by building VictoriaMetrics components f ## tip * FEATURE: adds service discovery visualisation tab for `/targets` page. It simplifies service discovery debugging. See [this PR](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/2675). -* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): Allows using kubeconfig file within `kubernetes_sd_configs`. It may be useful for kubernetes cluster monitoring by `vmagent` outside kubernetes cluster. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1464). * FEATURE: allow overriding default limits for in-memory cache `indexdb/tagFilters` via flag `-storage.cacheSizeIndexDBTagFilters`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2663). * FEATURE: add support of `lowercase` and `uppercase` relabeling actions in the same way as [Prometheus 2.36.0 does](https://github.com/prometheus/prometheus/releases/tag/v2.36.0). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2664). * FEATURE: support query tracing, which allows determining bottlenecks during query processing. See [these docs](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html#query-tracing) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1403). -* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): remove dependency on Internet access in `http://vmagent:8429/targets` page. Previously the page layout was broken without Internet access. See [shis issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2594). -* FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): remove dependency on Internet access in [web API pages](https://docs.victoriametrics.com/vmalert.html#web). Previously the functionality and the layout of these pages was broken without Internet access. See [shis issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2594). -* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): expose `/api/v1/status/config` endpoint in the same way as Prometheus does. See [these docs](https://prometheus.io/docs/prometheus/latest/querying/api/#config). * FEATURE: add ability to change the `indexdb` rotation timezone offset via `-retentionTimezoneOffset` command-line flag. Previously it was performed at 4am UTC time. This could lead to performance degradation in the middle of the day when VictoriaMetrics runs in time zones located too far from UTC. Thanks to @cnych for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/2574). +* FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): remove dependency on Internet access at [web API pages](https://docs.victoriametrics.com/vmalert.html#web). Previously the functionality and the layout of these pages was broken without Internet access. See [shis issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2594). +* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): remove dependency on Internet access at `http://vmagent:8429/targets` page. Previously the page layout was broken without Internet access. See [shis issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2594). +* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): add support for `kubeconfig_file` option at [kubernetes_sd_configs](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#kubernetes_sd_config). It may be useful for Kubernetes monitoring by `vmagent` outside Kubernetes cluster. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1464). +* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): expose `/api/v1/status/config` endpoint in the same way as Prometheus does. See [these docs](https://prometheus.io/docs/prometheus/latest/querying/api/#config). * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): add `-promscrape.suppressScrapeErrorsDelay` command-line flag, which can be used for delaying and aggregating the logging of per-target scrape errors. This may reduce the amounts of logs when `vmagent` scrapes many unreliable targets. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2575). Thanks to @jelmd for [the initial implementation](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/2576). * FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent.html): add `-promscrape.cluster.name` command-line flag, which allows proper data de-duplication when the same target is scraped from multiple [vmagent clusters](https://docs.victoriametrics.com/vmagent.html#scraping-big-number-of-targets). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2679). diff --git a/lib/promscrape/discovery/kubernetes/api.go b/lib/promscrape/discovery/kubernetes/api.go index 36ffa67be8..8950e623d8 100644 --- a/lib/promscrape/discovery/kubernetes/api.go +++ b/lib/promscrape/discovery/kubernetes/api.go @@ -22,16 +22,19 @@ func newAPIConfig(sdc *SDConfig, baseDir string, swcFunc ScrapeWorkConstructorFu } apiServer := sdc.APIServer - if len(sdc.KubeConfig) > 0 { - fmt.Println("building") - kc, err := buildConfig(sdc) - if err != nil { - return nil, fmt.Errorf("cannot build kube config: %w", err) + if len(sdc.KubeConfigFile) > 0 { + if len(apiServer) > 0 { + return nil, fmt.Errorf("`api_server: %q` and `kubeconfig_file: %q` options cannot be set simultaneously", apiServer, sdc.KubeConfigFile) } - ac, err = promauth.NewConfig(".", nil, kc.basicAuth, kc.token, kc.tokenFile, nil, kc.tlsConfig) + kc, err := newKubeConfig(sdc.KubeConfigFile) if err != nil { - return nil, fmt.Errorf("cannot initialize service account auth: %w; probably, `kubernetes_sd_config->api_server` is missing in Prometheus configs?", err) + return nil, fmt.Errorf("cannot build kube config from the specified `kubeconfig_file` config option: %w", err) } + acNew, err := promauth.NewConfig(".", nil, kc.basicAuth, kc.token, kc.tokenFile, nil, kc.tlsConfig) + if err != nil { + return nil, fmt.Errorf("cannot initialize auth config from `kubeconfig_file: %q`: %w", sdc.KubeConfigFile, err) + } + ac = acNew apiServer = kc.server sdc.ProxyURL = kc.proxyURL } diff --git a/lib/promscrape/discovery/kubernetes/kubeconfig.go b/lib/promscrape/discovery/kubernetes/kubeconfig.go index 0971640fab..e63a1573b5 100644 --- a/lib/promscrape/discovery/kubernetes/kubeconfig.go +++ b/lib/promscrape/discovery/kubernetes/kubeconfig.go @@ -66,7 +66,7 @@ type AuthInfo struct { } func (au *AuthInfo) validate() error { - errContext := "field: %s is not supported currently, open an issue with feature request for it" + errContext := "field %q is not supported yet; if you feel it is needed please open a feature request at https://github.com/VictoriaMetrics/VictoriaMetrics/issues/new" if au.Exec != nil { return fmt.Errorf(errContext, "exec") } @@ -150,94 +150,95 @@ type kubeConfig struct { proxyURL *proxy.URL } -func buildConfig(sdc *SDConfig) (*kubeConfig, error) { - - data, err := fs.ReadFileOrHTTP(sdc.KubeConfig) +func newKubeConfig(kubeConfigFile string) (*kubeConfig, error) { + data, err := fs.ReadFileOrHTTP(kubeConfigFile) if err != nil { - return nil, fmt.Errorf("cannot read kubeConfig from %q: %w", sdc.KubeConfig, err) + return nil, fmt.Errorf("cannot read %q: %w", kubeConfigFile, err) } - var config Config - if err = yaml.Unmarshal(data, &config); err != nil { - return nil, fmt.Errorf("cannot parse %q: %w", sdc.KubeConfig, err) + var cfg Config + if err = yaml.Unmarshal(data, &cfg); err != nil { + return nil, fmt.Errorf("cannot parse %q: %w", kubeConfigFile, err) } + kc, err := cfg.buildKubeConfig() + if err != nil { + return nil, fmt.Errorf("cannot build kubeConfig from %q: %w", kubeConfigFile, err) + } + return kc, nil +} +func (cfg *Config) buildKubeConfig() (*kubeConfig, error) { authInfos := make(map[string]*AuthInfo) - for _, obj := range config.AuthInfos { + for _, obj := range cfg.AuthInfos { authInfos[obj.Name] = obj.AuthInfo } clusterInfos := make(map[string]*Cluster) - for _, obj := range config.Clusters { + for _, obj := range cfg.Clusters { clusterInfos[obj.Name] = obj.Cluster } contexts := make(map[string]*Context) - for _, obj := range config.Contexts { + for _, obj := range cfg.Contexts { contexts[obj.Name] = obj.Context } - contextName := config.CurrentContext + contextName := cfg.CurrentContext configContext := contexts[contextName] if configContext == nil { - return nil, fmt.Errorf("context %q does not exist", contextName) + return nil, fmt.Errorf("missing context %q", contextName) } clusterInfoName := configContext.Cluster configClusterInfo := clusterInfos[clusterInfoName] if configClusterInfo == nil { - return nil, fmt.Errorf("cluster %q does not exist", clusterInfoName) + return nil, fmt.Errorf("missing cluster config %q at context %q", clusterInfoName, contextName) } - - if len(configClusterInfo.Server) == 0 { - return nil, fmt.Errorf("kubernetes server address cannot be empty, define it for context: %s", contextName) + server := configClusterInfo.Server + if len(server) == 0 { + return nil, fmt.Errorf("missing kubernetes server address for config %q at context %q", clusterInfoName, contextName) } authInfoName := configContext.AuthInfo configAuthInfo := authInfos[authInfoName] if authInfoName != "" && configAuthInfo == nil { - return nil, fmt.Errorf("auth info %q does not exist", authInfoName) + return nil, fmt.Errorf("missing auth config %q", authInfoName) } - var tlsConfig *promauth.TLSConfig var basicAuth *promauth.BasicAuthConfig var token, tokenFile string - isHTTPS := strings.HasPrefix(configClusterInfo.Server, "https://") - - if isHTTPS { - tlsConfig = &promauth.TLSConfig{ - CAFile: configClusterInfo.CertificateAuthority, - ServerName: configClusterInfo.TLSServerName, - InsecureSkipVerify: configClusterInfo.InsecureSkipTLSVerify, - } - } - - if len(configClusterInfo.CertificateAuthorityData) > 0 && isHTTPS { - tlsConfig.CA, err = base64.StdEncoding.DecodeString(configClusterInfo.CertificateAuthorityData) - if err != nil { - return nil, fmt.Errorf("cannot base64-decode configClusterInfo.CertificateAuthorityData %q: %w", configClusterInfo.CertificateAuthorityData, err) - } - } - if configAuthInfo != nil { if err := configAuthInfo.validate(); err != nil { - return nil, fmt.Errorf("invalid user auth configuration for context: %s, err: %w", contextName, err) + return nil, fmt.Errorf("invalid auth config %q: %w", authInfoName, err) } - if isHTTPS { + if strings.HasPrefix(configClusterInfo.Server, "https://") { + tlsConfig = &promauth.TLSConfig{ + CAFile: configClusterInfo.CertificateAuthority, + ServerName: configClusterInfo.TLSServerName, + InsecureSkipVerify: configClusterInfo.InsecureSkipTLSVerify, + } + if len(configClusterInfo.CertificateAuthorityData) > 0 { + ca, err := base64.StdEncoding.DecodeString(configClusterInfo.CertificateAuthorityData) + if err != nil { + return nil, fmt.Errorf("cannot base64-decode certificate-authority-data from config %q at context %q: %w", clusterInfoName, contextName, err) + } + tlsConfig.CA = ca + } tlsConfig.CertFile = configAuthInfo.ClientCertificate tlsConfig.KeyFile = configAuthInfo.ClientKey if len(configAuthInfo.ClientCertificateData) > 0 { - tlsConfig.Cert, err = base64.StdEncoding.DecodeString(configAuthInfo.ClientCertificateData) + cert, err := base64.StdEncoding.DecodeString(configAuthInfo.ClientCertificateData) if err != nil { - return nil, fmt.Errorf("cannot base64-decode configAuthInfo.ClientCertificateData %q: %w", configClusterInfo.CertificateAuthorityData, err) + return nil, fmt.Errorf("cannot base64-decode client-certificate-data from %q: %w", authInfoName, err) } + tlsConfig.Cert = cert } if len(configAuthInfo.ClientKeyData) > 0 { - tlsConfig.Key, err = base64.StdEncoding.DecodeString(configAuthInfo.ClientKeyData) + key, err := base64.StdEncoding.DecodeString(configAuthInfo.ClientKeyData) if err != nil { - return nil, fmt.Errorf("cannot base64-decode configAuthInfo.ClientKeyData %q: %w", configClusterInfo.CertificateAuthorityData, err) + return nil, fmt.Errorf("cannot base64-decode client-key-data from %q: %w", authInfoName, err) } + tlsConfig.Key = key } } - if len(configAuthInfo.Username) > 0 || len(configAuthInfo.Password) > 0 { basicAuth = &promauth.BasicAuthConfig{ Username: configAuthInfo.Username, @@ -247,15 +248,13 @@ func buildConfig(sdc *SDConfig) (*kubeConfig, error) { token = configAuthInfo.Token tokenFile = configAuthInfo.TokenFile } - - kc := kubeConfig{ + kc := &kubeConfig{ basicAuth: basicAuth, - server: configClusterInfo.Server, + server: server, token: token, tokenFile: tokenFile, tlsConfig: tlsConfig, proxyURL: configClusterInfo.ProxyURL, } - - return &kc, nil + return kc, nil } diff --git a/lib/promscrape/discovery/kubernetes/kubeconfig_test.go b/lib/promscrape/discovery/kubernetes/kubeconfig_test.go index cb50ff94c1..fc0c7334ef 100644 --- a/lib/promscrape/discovery/kubernetes/kubeconfig_test.go +++ b/lib/promscrape/discovery/kubernetes/kubeconfig_test.go @@ -11,26 +11,22 @@ func TestParseKubeConfigSuccess(t *testing.T) { type testCase struct { name string - sdc *SDConfig + kubeConfigFile string expectedConfig *kubeConfig } var cases = []testCase{ { - name: "token", - sdc: &SDConfig{ - KubeConfig: "testdata/good_kubeconfig/with_token.yaml", - }, + name: "token", + kubeConfigFile: "testdata/good_kubeconfig/with_token.yaml", expectedConfig: &kubeConfig{ server: "http://some-server:8080", token: "abc", }, }, { - name: "cert", - sdc: &SDConfig{ - KubeConfig: "testdata/good_kubeconfig/with_tls.yaml", - }, + name: "cert", + kubeConfigFile: "testdata/good_kubeconfig/with_tls.yaml", expectedConfig: &kubeConfig{ server: "https://localhost:6443", tlsConfig: &promauth.TLSConfig{ @@ -41,10 +37,8 @@ func TestParseKubeConfigSuccess(t *testing.T) { }, }, { - name: "basic", - sdc: &SDConfig{ - KubeConfig: "testdata/good_kubeconfig/with_basic.yaml", - }, + name: "basic", + kubeConfigFile: "testdata/good_kubeconfig/with_basic.yaml", expectedConfig: &kubeConfig{ server: "http://some-server:8080", basicAuth: &promauth.BasicAuthConfig{ @@ -56,7 +50,7 @@ func TestParseKubeConfigSuccess(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - ac, err := buildConfig(tc.sdc) + ac, err := newKubeConfig(tc.kubeConfigFile) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -68,14 +62,11 @@ func TestParseKubeConfigSuccess(t *testing.T) { } func TestParseKubeConfigFail(t *testing.T) { - f := func(name, kubeConfigPath string) { + f := func(name, kubeConfigFile string) { t.Helper() t.Run(name, func(t *testing.T) { - sdc := &SDConfig{ - KubeConfig: kubeConfigPath, - } - if _, err := buildConfig(sdc); err == nil { - t.Fatalf("unexpected result for config file: %s, must return error", kubeConfigPath) + if _, err := newKubeConfig(kubeConfigFile); err == nil { + t.Fatalf("unexpected result for config file: %s, must return error", kubeConfigFile) } }) } diff --git a/lib/promscrape/discovery/kubernetes/kubernetes.go b/lib/promscrape/discovery/kubernetes/kubernetes.go index d5ed48f3ab..5652ec8004 100644 --- a/lib/promscrape/discovery/kubernetes/kubernetes.go +++ b/lib/promscrape/discovery/kubernetes/kubernetes.go @@ -22,8 +22,9 @@ type SDConfig struct { // Use role() function for accessing the Role field Role string `yaml:"role"` - // if defined any cluster connection information from HTTPClientConfig will be ignored - KubeConfig string `yaml:"kubeconfig_file"` + // The filepath to kube config. + // If defined any cluster connection information from HTTPClientConfig is ignored. + KubeConfigFile string `yaml:"kubeconfig_file"` HTTPClientConfig promauth.HTTPClientConfig `yaml:",inline"` ProxyURL *proxy.URL `yaml:"proxy_url,omitempty"`