From 62b4efb3e70fbfac6211ae5155e3f472e01db525 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 2 Dec 2021 23:32:03 +0200 Subject: [PATCH] app/vmauth: follow-up for 13368bed18c0630af58159f9d921894e07255726 * Document the ability to specify http or https urls in `-auth.config` at docs/CHANGELOG.md * Move the ReadFileOrHTTP to lib/fs, so it can be re-used in other places where a file should be read from the given path. For example, in `-promscrape.config` at `vmagent`. --- app/vmauth/auth_config.go | 26 +++----------------------- app/vmauth/auth_config_test.go | 16 ---------------- docs/CHANGELOG.md | 6 ++++-- lib/fs/fs.go | 31 +++++++++++++++++++++++++++++++ lib/fs/fs_test.go | 15 +++++++++++++++ 5 files changed, 53 insertions(+), 41 deletions(-) diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go index ff90f00a3..9004ae513 100644 --- a/app/vmauth/auth_config.go +++ b/app/vmauth/auth_config.go @@ -4,8 +4,6 @@ import ( "encoding/base64" "flag" "fmt" - "io/ioutil" - "net/http" "net/url" "os" "regexp" @@ -15,6 +13,7 @@ import ( "sync/atomic" "github.com/VictoriaMetrics/VictoriaMetrics/lib/envtemplate" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/fs" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "github.com/VictoriaMetrics/VictoriaMetrics/lib/procutil" "github.com/VictoriaMetrics/metrics" @@ -238,21 +237,9 @@ var authConfigWG sync.WaitGroup var stopCh chan struct{} func readAuthConfig(path string) (map[string]*UserInfo, error) { - var data []byte - var err error - // reads remote file via http, if url is given - if isHTTPURL(path) { - httpPath, err := http.Get(path) - if err != nil { - return nil, fmt.Errorf("cannot read %q: %w", path, err) - } - defer func() { _ = httpPath.Body.Close() }() - data, err = ioutil.ReadAll(httpPath.Body) - } else { - data, err = ioutil.ReadFile(path) - } + data, err := fs.ReadFileOrHTTP(path) if err != nil { - return nil, fmt.Errorf("cannot read %q: %w", path, err) + return nil, err } m, err := parseAuthConfig(data) if err != nil { @@ -385,10 +372,3 @@ func sanitizeURLPrefix(urlPrefix *url.URL) (*url.URL, error) { } return urlPrefix, nil } - -// isHTTPURL checks if a given targetURL is valid and contains a valid http scheme -func isHTTPURL(targetURL string) bool { - parsed, err := url.Parse(targetURL) - return err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") && parsed.Host != "" - -} diff --git a/app/vmauth/auth_config_test.go b/app/vmauth/auth_config_test.go index ef81606f7..a95a3a43a 100644 --- a/app/vmauth/auth_config_test.go +++ b/app/vmauth/auth_config_test.go @@ -368,19 +368,3 @@ func mustParseURLs(us []string) *URLPrefix { urls: pus, } } - -func TestIsHTTPURLSuccess(t *testing.T) { - assert := func(s string, expected bool) { - t.Helper() - res := isHTTPURL(s) - if res != expected { - t.Fatalf("expecting %t, got %t", expected, res) - } - } - - assert("http://isvalid:8000/filepath", true) // test http - assert("https://isvalid:8000/filepath", true) // test https - assert("tcp://notvalid:8000/filepath", false) // test tcp - assert("0/filepath", false) // something invalid - assert("filepath.extension", false) // something invalid -} diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9851e9653..7e4b7e923 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -6,6 +6,8 @@ sort: 15 ## tip +* FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth.html): allow specifying `http` and `https` urls in `-auth.config` command-line flag. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1898). Thanks for @TFM93 . + ## [v1.70.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.70.0) @@ -14,7 +16,7 @@ sort: 15 * FEATURE: vmauth: allow using optional `name` field in configs. This field is then used as `username` label value for `vmauth_user_requests_total` metric. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1805). * FEATURE: vmagent: export `vm_persistentqueue_read_duration_seconds_total` and `vm_persistentqueue_write_duration_seconds_total` metrics, which can be used for detecting persistent queue saturation with `rate(vm_persistentqueue_write_duration_seconds_total) > 0.9` alerting rule. * FEATURE: export `vm_filestream_read_duration_seconds_total` and `vm_filestream_write_duration_seconds_total` metrics, which can be used for detecting persistent disk saturation with `rate(vm_filestream_read_duration_seconds_total) > 0.9` alerting rule. -* FEATURE: export `vm_cache_size_max_bytes` metrics, which show capacity for various caches. These metrics can be used for determining caches reaches its capacity with `vm_cache_size_bytes / vm_cache_size_max_bytes > 0.9` query. +* FEATURE: export `vm_cache_size_max_bytes` metrics, which show capacity for various caches. These metrics can be used for determining caches with reach its capacity with `vm_cache_size_bytes / vm_cache_size_max_bytes > 0.9` query. * FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup.html), [vmrestore](https://docs.victoriametrics.com/vmrestore.html): add `-s3ForcePathStyle` command-line flag, which can be used for making backups to [Aliyun OSS](https://www.aliyun.com/product/oss). See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1802). * FEATURE: [vmctl](https://docs.victoriametrics.com/vmctl.html): improve data migration from OpenTSDB. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1809). Thanks to @johnseekins . * FEATURE: suppress `connection reset by peer` errors when remote client resets TCP connection to VictoriaMetrics / vmagent while ingesting the data via InfluxDB line protocol, Graphite protocol or OpenTSDB protocol. This error is expected, so there is no need in logging it. @@ -22,7 +24,7 @@ sort: 15 * FEATURE: vmalert: make `-notifier.url` command-line flag optional. This flag can be omitted if `vmalert` is used solely for recording rules and doesn't evaluate alerting rules. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1870). * FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup.html), [vmrestore](https://docs.victoriametrics.com/vmrestore.html): export internal metrics at `http://vmbackup:8420/metrics` and `http://vmrestore:8421/metrics` for better visibility of the backup/restore process. * FEATURE: allow trailing whitespace after the timestamp when [parsing Graphite plaintext lines](https://docs.victoriametrics.com/#how-to-send-data-from-graphite-compatible-agents-such-as-statsd). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1865). -* FEATURE: expose `/-/healthy` and `/-/ready` endpoints as Prometheus does. This is needed for improving integration with third-party solutions, which rely on these endpoints. See [tis issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1833). +* FEATURE: expose `/-/healthy` and `/-/ready` endpoints as Prometheus does. This is needed for improving integration with third-party solutions, which rely on these endpoints. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1833). * BUGFIX: vmagent: prevent from scraping duplicate targets if `-promscrape.dropOriginalLabels` command-line flag is set. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1830). Thanks to @guidao for the fix. * BUGFIX: vmstorage [enterprise](https://victoriametrics.com/enterprise.html): added missing `vm_tenant_used_tenant_bytes` metric, which shows the approximate per-tenant disk usage. See [these docs](https://docs.victoriametrics.com/PerTenantStatistic.html) and [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1605). diff --git a/lib/fs/fs.go b/lib/fs/fs.go index b7a50dd77..2da6f7ceb 100644 --- a/lib/fs/fs.go +++ b/lib/fs/fs.go @@ -4,6 +4,8 @@ import ( "fmt" "io" "io/ioutil" + "net/http" + "net/url" "os" "path/filepath" "regexp" @@ -372,3 +374,32 @@ type freeSpaceEntry struct { updateTime uint64 freeSpace uint64 } + +// ReadFileOrHTTP reads path either from local filesystem or from http if path starts with http or https. +func ReadFileOrHTTP(path string) ([]byte, error) { + if isHTTPURL(path) { + // reads remote file via http or https, if url is given + resp, err := http.Get(path) + if err != nil { + return nil, fmt.Errorf("cannot fetch %q: %w", path, err) + } + data, err := ioutil.ReadAll(resp.Body) + _ = resp.Body.Close() + if err != nil { + return nil, fmt.Errorf("cannot read %q: %s", path, err) + } + return data, nil + } + data, err := ioutil.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("cannot read %q: %w", path, err) + } + return data, nil +} + +// isHTTPURL checks if a given targetURL is valid and contains a valid http scheme +func isHTTPURL(targetURL string) bool { + parsed, err := url.Parse(targetURL) + return err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") && parsed.Host != "" + +} diff --git a/lib/fs/fs_test.go b/lib/fs/fs_test.go index 394cb4a98..ecd8114d2 100644 --- a/lib/fs/fs_test.go +++ b/lib/fs/fs_test.go @@ -22,3 +22,18 @@ func TestIsTemporaryFileName(t *testing.T) { f("asdf.sdfds.tmp.dfd", false) f("dfd.sdfds.dfds.1232", false) } + +func TestIsHTTPURLSuccess(t *testing.T) { + f := func(s string, expected bool) { + t.Helper() + res := isHTTPURL(s) + if res != expected { + t.Fatalf("expecting %t, got %t", expected, res) + } + } + f("http://isvalid:8000/filepath", true) // test http + f("https://isvalid:8000/filepath", true) // test https + f("tcp://notvalid:8000/filepath", false) // test tcp + f("0/filepath", false) // something invalid + f("filepath.extension", false) // something invalid +}