From 100d30a77b9e0f835ca8cf802ad88eb50976e801 Mon Sep 17 00:00:00 2001 From: Slach Date: Thu, 27 Jun 2024 19:24:21 +0400 Subject: [PATCH] properly restore environment variables to avoid failures in config.ValidateConfig in REST API mode, fix https://github.com/Altinity/clickhouse-backup/issues/940 --- ChangeLog.md | 5 +++++ pkg/common/utils_test.go | 19 +++++++++++++++++++ pkg/config/config.go | 29 ++++++++++++++++++++++------- 3 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 pkg/common/utils_test.go diff --git a/ChangeLog.md b/ChangeLog.md index 11f8b209..51e2cba0 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,3 +1,8 @@ +# v2.5.16 +BUG FIXES +- allow backup/restore tables and databases which contains additional special characters set, fix [938](https://github.com/Altinity/clickhouse-backup/issues/938) +- properly restore environment variables to avoid failures in config.ValidateConfig in REST API mode, fix [940](https://github.com/Altinity/clickhouse-backup/issues/940) + # v2.5.15 IMPROVEMENTS - increase `s3_request_timeout_ms` (23.7+) and turn off `s3_use_adaptive_timeouts` (23.11+) when `use_embedded_backup_restore: true` diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go new file mode 100644 index 00000000..0f9336f0 --- /dev/null +++ b/pkg/common/utils_test.go @@ -0,0 +1,19 @@ +package common + +import ( + "github.com/stretchr/testify/require" + "net/url" + "testing" +) + +func TestTablePathEncode(t *testing.T) { + r := require.New(t) + str := `!@#$^&*()+-=[]{}|;':\",./<>?~` + expected := "%21%40%23%24%5E%26%2A%28%29%2B%2D%3D%5B%5D%7B%7D%7C%3B%27%3A%5C%22%2C%2E%2F%3C%3E%3F%7E" + + actual := TablePathEncode(str) + r.Equal(expected, actual) + decoded, err := url.PathUnescape(actual) + r.NoError(err) + r.Equal(str, decoded) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 198854aa..2fc9fb62 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -645,9 +645,14 @@ func GetConfigPath(ctx *cli.Context) string { return DefaultConfigPath } -func OverrideEnvVars(ctx *cli.Context) map[string]string { +type oldEnvValues struct { + OldValue string + WasPresent bool +} + +func OverrideEnvVars(ctx *cli.Context) map[string]oldEnvValues { env := ctx.StringSlice("env") - oldValues := map[string]string{} + oldValues := map[string]oldEnvValues{} if len(env) > 0 { for _, v := range env { envVariable := strings.SplitN(v, "=", 2) @@ -655,7 +660,11 @@ func OverrideEnvVars(ctx *cli.Context) map[string]string { envVariable = append(envVariable, "true") } log.Infof("override %s=%s", envVariable[0], envVariable[1]) - oldValues[envVariable[0]] = os.Getenv(envVariable[0]) + oldValue, wasPresent := os.LookupEnv(envVariable[0]) + oldValues[envVariable[0]] = oldEnvValues{ + OldValue: oldValue, + WasPresent: wasPresent, + } if err := os.Setenv(envVariable[0], envVariable[1]); err != nil { log.Warnf("can't override %s=%s, error: %v", envVariable[0], envVariable[1], err) } @@ -664,10 +673,16 @@ func OverrideEnvVars(ctx *cli.Context) map[string]string { return oldValues } -func RestoreEnvVars(envVars map[string]string) { - for name, value := range envVars { - if err := os.Setenv(name, value); err != nil { - log.Warnf("can't restore %s=%s, error: %v", name, value, err) +func RestoreEnvVars(envVars map[string]oldEnvValues) { + for name, oldEnv := range envVars { + if oldEnv.WasPresent { + if err := os.Setenv(name, oldEnv.OldValue); err != nil { + log.Warnf("RestoreEnvVars can't restore %s=%s, error: %v", name, oldEnv.OldValue, err) + } + } else { + if err := os.Unsetenv(name); err != nil { + log.Warnf("RestoreEnvVars can't delete %s, error: %v", name, err) + } } } }