mirror of
https://github.com/VictoriaMetrics/VictoriaMetrics.git
synced 2024-11-21 14:44:00 +00:00
lib/fs/fscore: do not trim content from path (#6503)
### Describe Your Changes Trimming content which is loaded from an external pass leads to obscure issues in case user-defined input contained trimmed chars. For example. user-defined password "foo\n" will become "foo" while user will expect it to contain a new line. --- For example, a user defines a password which ends with `\n`. This often happens when user Kubernetes secrets and manually encodes value as base64-encoded string. In this case vmauth configuration might look like: ``` users: - url_prefix: - http://vminsert:8480/insert/0/prometheus/api/v1/write name: foo username: foo password: "foobar\n" ``` vmagent configuration for this setup will use the following flags: ``` -remoteWrite.url=http://vmauth:8427/ -remoteWrite.basicAuth.passwordFile=/tmp/vmagent-password -remoteWrite.basicAuth.username="foo" ``` Where `/tmp/vmagent-password` is a file with `foobar\n` password. Before this change such configuration will result in `401 Unauthorized` response received by vmagent since after file content will become `foobar`. --- An example with Kubernetes operator which uses a secret to reference the same password in multiple configurations. <details> <summary>See full manifests</summary> `Secret`: ``` apiVersion: v1 data: name: Zm9v # foo password: Zm9vYmFy # foobar\n username: Zm9v= # foo kind: Secret metadata: name: vmuser ``` `VMUser`: ``` apiVersion: operator.victoriametrics.com/v1beta1 kind: VMUser metadata: name: vmagents spec: generatePassword: false name: vmagents targetRefs: - crd: kind: VMAgent name: some-other-agent namespace: example username: foo # note - the secret above is referenced to provide password passwordRef: name: vmagent key: password ``` `VMAgent`: ``` apiVersion: operator.victoriametrics.com/v1beta1 kind: VMAgent metadata: name: example spec: selectAllByDefault: true scrapeInterval: 5s replicaCount: 1 remoteWrite: - url: "http://vmauth-vmauth-example:8427/api/v1/write" # note - the secret above is referenced as well basicAuth: username: name: vmagent key: username password: name: vmagent key: password ``` </details> Since both config target exactly the same `Secret` object it is expected to work, but apparently the result will be `401 Unauthrized` error. ### Checklist The following checks are **mandatory**: - [x] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/). --------- Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com> Signed-off-by: hagen1778 <roman@victoriametrics.com> Co-authored-by: hagen1778 <roman@victoriametrics.com>
This commit is contained in:
parent
75ad6c1b49
commit
201fd6de1e
3 changed files with 6 additions and 6 deletions
|
@ -32,6 +32,8 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
|
|||
|
||||
**Update note 1: the `--vm-disable-progress-bar` command-line flag at `vmctl` was deprecated. Use `--disable-progress-bar` instead.**
|
||||
|
||||
**Update note 2: `*.passwordFile` and similar flags are no longer trimming trailing whitespaces at the end of content. Make sure to update the templating of password files or HTTP endpoints to not include trailing whitespaces before the upgrade. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6503) PR for the details.**
|
||||
|
||||
* FEATURE: all VictoriaMetrics components: use constant-time comparison for comparing HTTP basic auth credentials and auth keys. This should prevent timing attacks when comparing these credentials. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6392) for details. Thanks to @wasim-nihal for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6423).
|
||||
* FEATURE: [alerts-vmagent](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/alerts-vmagent.yml): add new alerting rules `StreamAggrFlushTimeout` and `StreamAggrDedupFlushTimeout` to notify about issues during stream aggregation.
|
||||
* FEATURE: [dashboards/vmagent](https://grafana.com/grafana/dashboards/12683): add row `Streaming aggregation` with panels related to [streaming aggregation](https://docs.victoriametrics.com/stream-aggregation/) process.
|
||||
|
@ -43,6 +45,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
|
|||
* FEATURE: [vmalert-tool](https://docs.victoriametrics.com/vmalert-tool/): support file path with hierarchical patterns and regexpes, and http url in unittest cmd-line flag `-files`, e.g. `-files="http://<some-server-addr>/path/to/rules"` or `-files="dir/**/*.yaml"`.
|
||||
|
||||
* BUGFIX: all VictoriaMetrics components: prioritize `-configAuthKey` and `-reloadAuthKey` over `-httpAuth.*` settings. This change aligns behavior of mentioned flags with other auth flags like `-metricsAuthKey`, `-flagsAuthKey`, `-pprofAuthKey`. Check [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6329).
|
||||
* BUGFIX: all VictoriaMetrics components: do not trim trailing spaces when reading content from `*.passwordFile` and similar flags. Previously, trailing spaces were trimmed from the content of the password file, which could lead to unexpected authentication errors.
|
||||
* BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl/): add `--disable-progress-bar` global command-line flag. It can be used for disabling dynamic progress bar for all migration modes. `--vm-disable-progress-bar` command-line flag is deprecated and will be removed in the future releases. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6367).
|
||||
* BUGFIX: [stream aggregation](https://docs.victoriametrics.com/stream-aggregation/): prevent [rate_sum](https://docs.victoriametrics.com/stream-aggregation/#rate_sum) and [rate_avg](https://docs.victoriametrics.com/stream-aggregation/#rate_avg) producing `NaN` results for stale time series. Before, when series matched for aggregation became stale or weren't updated during aggregation interval, the `rate_sum` or `rate_avg` could produce data point with `NaN` value. During visualization, such aggregation results would be displayed as gaps in time series.
|
||||
* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert/): fix path for system links printed on default vmalert's UI page when `-http.pathPrefix` is set.
|
||||
|
|
|
@ -32,7 +32,7 @@ func TestPassword(t *testing.T) {
|
|||
|
||||
// read the password from file by relative path
|
||||
localPassFile := "testdata/password.txt"
|
||||
expectedPassword = "foo-bar-baz"
|
||||
expectedPassword = "foo-bar-baz\n\n\n"
|
||||
path := "file://" + localPassFile
|
||||
if err := p.Set(path); err != nil {
|
||||
t.Fatalf("cannot set password to file: %s", err)
|
||||
|
@ -52,7 +52,7 @@ func TestPassword(t *testing.T) {
|
|||
if err != nil {
|
||||
t.Fatalf("unexpected error: %s", err)
|
||||
}
|
||||
expectedPassword = "foo-bar-baz"
|
||||
expectedPassword = "foo-bar-baz\n\n\n"
|
||||
path = "file://" + localPassFile
|
||||
if err := p.Set(path); err != nil {
|
||||
t.Fatalf("unexpected error: %s", err)
|
||||
|
|
|
@ -7,8 +7,6 @@ import (
|
|||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"unicode"
|
||||
)
|
||||
|
||||
// ReadPasswordFromFileOrHTTP reads password for the give path.
|
||||
|
@ -19,8 +17,7 @@ func ReadPasswordFromFileOrHTTP(path string) (string, error) {
|
|||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
pass := strings.TrimRightFunc(string(data), unicode.IsSpace)
|
||||
return pass, nil
|
||||
return string(data), nil
|
||||
}
|
||||
|
||||
// ReadFileOrHTTP reads path either from local filesystem or from http if path starts with http or https.
|
||||
|
|
Loading…
Reference in a new issue