Just check the extension

Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>

runtimeconfig/manager.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strings"
 	"sync"
 	"time"
 
@@ -30,7 +31,6 @@ type Loader func(r io.Reader) (interface{}, error)
 // It holds config related to loading per-tenant config.
 type Config struct {
 	ReloadPeriod time.Duration `yaml:"period" category:"advanced"`
-	AllowGzip    bool          `yaml:"allow_gzip" category:"advanced"`
 	// LoadPath contains the path to the runtime config files.
 	// Requires a non-empty value
 	LoadPath flagext.StringSliceCSV `yaml:"file"`
@@ -41,7 +41,6 @@ type Config struct {
 func (mc *Config) RegisterFlags(f *flag.FlagSet) {
 	f.Var(&mc.LoadPath, "runtime-config.file", "Comma separated list of yaml files with the configuration that can be updated at runtime. Runtime config files will be merged from left to right.")
 	f.DurationVar(&mc.ReloadPeriod, "runtime-config.reload-period", 10*time.Second, "How often to check runtime config files.")
-	f.BoolVar(&mc.AllowGzip, "runtime-config.allow-gzip", false, "Allow runtime config files to be gzipped.")
 }
 
 // Manager periodically reloads the configuration from specified files, and keeps this
@@ -187,7 +186,7 @@ func (om *Manager) loadConfig() error {
 	mergedConfig := map[string]interface{}{}
 	for _, f := range om.cfg.LoadPath {
 		data := rawData[f]
-		yamlFile, err := om.unmarshalMaybeGzipped(data)
+		yamlFile, err := om.unmarshalMaybeGzipped(f, data)
 		if err != nil {
 			om.configLoadSuccess.Set(0)
 			return errors.Wrapf(err, "unmarshal file %q", f)
@@ -221,23 +220,22 @@ func (om *Manager) loadConfig() error {
 	return nil
 }
 
-func (om *Manager) unmarshalMaybeGzipped(data []byte) (map[string]any, error) {
+func (om *Manager) unmarshalMaybeGzipped(filename string, data []byte) (map[string]any, error) {
 	yamlFile := map[string]any{}
-	if om.cfg.AllowGzip && isGzip(data) {
+	if strings.HasSuffix(filename, ".gz") {
 		r, err := gzip.NewReader(bytes.NewReader(data))
 		if err != nil {
-			return nil, err
+			return nil, errors.Wrap(err, "read gzipped file")
 		}
 		defer r.Close()
-
 		err = yaml.NewDecoder(r).Decode(&yamlFile)
 		return yamlFile, errors.Wrap(err, "uncompress/unmarshal gzipped file")
 	}
 
 	if err := yaml.Unmarshal(data, &yamlFile); err != nil {
 		// Give a hint if we think that file is gzipped.
-		if !om.cfg.AllowGzip && isGzip(data) {
-			return nil, errors.Wrap(err, "file looks gzipped but gzip is disabled")
+		if isGzip(data) {
+			return nil, errors.Wrap(err, "file looks gzipped but doesn't have a .gz extension")
 		}
 		return nil, err
 	}

runtimeconfig/manager_test.go

@@ -179,10 +179,10 @@ func TestNewOverridesManager(t *testing.T) {
 }
 
 func TestManagerGzip(t *testing.T) {
-	writeConfig := func(gzipped bool) string {
+	writeConfig := func(filename string, gzipped bool) string {
 		dir := t.TempDir()
-		filename := filepath.Join(dir, "overrides.yaml")
-		f, err := os.Create(filename)
+		filePath := filepath.Join(dir, filename)
+		f, err := os.Create(filePath)
 		require.NoError(t, err)
 		defer f.Close()
 		w := io.Writer(f)
@@ -198,43 +198,44 @@ func TestManagerGzip(t *testing.T) {
 				},
 			},
 		}))
-		return filename
+		return filePath
+	}
+
+	cfg := func(file string) Config {
+		return Config{
+			ReloadPeriod: time.Second,
+			LoadPath:     []string{file},
+			Loader:       testLoadOverrides,
+		}
 	}
 
 	defaultTestLimits = &TestLimits{Limit1: 100}
+	t.Run("gzipped with .gz extension should succeed", func(t *testing.T) {
+		file := writeConfig("overrides.yaml.gz", true)
+		manager, err := New(cfg(file), "overrides", nil, log.NewNopLogger())
+		require.NoError(t, err)
+		require.NoError(t, services.StartAndAwaitRunning(context.Background(), manager))
+		t.Cleanup(func() { require.NoError(t, services.StopAndAwaitTerminated(context.Background(), manager)) })
+
+		// Make sure test limits were loaded.
+		require.NotNil(t, manager.GetConfig())
+		conf := manager.GetConfig().(*testOverrides)
+		require.NotNil(t, conf)
+		require.Equal(t, 150, conf.Overrides["user1"].Limit2)
+	})
 
-	t.Run("allowed", func(t *testing.T) {
-		for _, gzipped := range []bool{true, false} {
-			t.Run(fmt.Sprintf("gzipped=%t", gzipped), func(t *testing.T) {
-				cfg := Config{
-					ReloadPeriod: time.Second,
-					LoadPath:     []string{writeConfig(gzipped)},
-					Loader:       testLoadOverrides,
-					AllowGzip:    true,
-				}
-				manager, err := New(cfg, "overrides", nil, log.NewNopLogger())
-				require.NoError(t, err)
-				require.NoError(t, services.StartAndAwaitRunning(context.Background(), manager))
-				t.Cleanup(func() { require.NoError(t, services.StopAndAwaitTerminated(context.Background(), manager)) })
-
-				// Make sure test limits were loaded.
-				require.NotNil(t, manager.GetConfig())
-				conf := manager.GetConfig().(*testOverrides)
-				require.NotNil(t, conf)
-				require.Equal(t, 150, conf.Overrides["user1"].Limit2)
-
-			})
-		}
+	t.Run("non-gzipped with .gz extension should fail", func(t *testing.T) {
+		file := writeConfig("overrides.yaml.gz", false)
+		manager, err := New(cfg(file), "overrides", nil, log.NewNopLogger())
+		require.NoError(t, err)
+		err = services.StartAndAwaitRunning(context.Background(), manager)
+		require.Error(t, err)
+		require.ErrorIs(t, err, gzip.ErrHeader)
 	})
 
-	t.Run("disallowed", func(t *testing.T) {
-		cfg := Config{
-			ReloadPeriod: time.Second,
-			LoadPath:     []string{writeConfig(true)},
-			Loader:       testLoadOverrides,
-			AllowGzip:    false,
-		}
-		manager, err := New(cfg, "overrides", nil, log.NewNopLogger())
+	t.Run("gzipped without .gz extension should mention that in the error", func(t *testing.T) {
+		file := writeConfig("overrides.yaml", true)
+		manager, err := New(cfg(file), "overrides", nil, log.NewNopLogger())
 		require.NoError(t, err)
 		err = services.StartAndAwaitRunning(context.Background(), manager)
 		require.Error(t, err)