Skip to content

Commit b6e7583

Browse files
alxricalxric
committed
cmd: add --objstore.config-file flag
This to be more in line with how the other thanos components work, allowing you to specify the path to a yaml config file instead of passing the object storage config as individual parameters direcly.
1 parent dbee0d9 commit b6e7583

File tree

4 files changed

+276
-34
lines changed

4 files changed

+276
-34
lines changed

‎cmd/config.go‎

Lines changed: 59 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"log/slog"
1111
"net/http"
1212
"net/http/pprof"
13+
"os"
1314
"strings"
1415
"time"
1516

@@ -45,6 +46,11 @@ func setupInterrupt(ctx context.Context, g *run.Group, log *slog.Logger) {
4546
}
4647

4748
type bucketOpts struct {
49+
// New config file options (preferred)
50+
objStoreConfigFile string
51+
objStoreConfig string
52+
53+
// Legacy individual flags (for backwards compatibility)
4854
storage string
4955
prefix string
5056

@@ -62,46 +68,65 @@ type bucketOpts struct {
6268
}
6369

6470
func setupBucket(log *slog.Logger, opts bucketOpts) (objstore.Bucket, error) {
65-
prov := objstore.ObjProvider(strings.ToUpper(opts.storage))
66-
cfg := client.BucketConfig{
67-
Type: prov,
68-
Prefix: opts.prefix,
69-
}
70-
var subCfg any
71-
switch prov {
72-
case objstore.FILESYSTEM:
73-
subCfg = struct {
74-
Directory string `yaml:"directory"`
75-
}{
76-
Directory: opts.filesystemDirectory,
71+
var confContentYaml []byte
72+
var err error
73+
74+
// Prefer new config file approach if provided
75+
if opts.objStoreConfigFile != "" {
76+
confContentYaml, err = os.ReadFile(opts.objStoreConfigFile)
77+
if err != nil {
78+
return nil, fmt.Errorf("unable to read objstore config file: %w", err)
7779
}
78-
case objstore.S3:
79-
subCfg = struct {
80-
Bucket string `yaml:"bucket"`
81-
Endpoint string `yaml:"endpoint"`
82-
AccessKey string `yaml:"access_key"`
83-
SecretKey string `yaml:"secret_key"`
84-
MaxRetries int `yaml:"max_retries"`
85-
Insecure bool `yaml:"insecure"`
86-
}{
87-
Bucket: opts.s3Bucket,
88-
Endpoint: opts.s3Endpoint,
89-
AccessKey: opts.s3AccessKey,
90-
SecretKey: opts.s3SecretKey,
91-
Insecure: opts.s3Insecure,
92-
MaxRetries: opts.retries,
80+
} else if opts.objStoreConfig != "" {
81+
confContentYaml = []byte(opts.objStoreConfig)
82+
} else {
83+
// Fall back to legacy individual flags for backwards compatibility
84+
prov := objstore.ObjProvider(strings.ToUpper(opts.storage))
85+
cfg := client.BucketConfig{
86+
Type: prov,
87+
Prefix: opts.prefix,
88+
}
89+
var subCfg any
90+
switch prov {
91+
case objstore.FILESYSTEM:
92+
subCfg = struct {
93+
Directory string `yaml:"directory"`
94+
}{
95+
Directory: opts.filesystemDirectory,
96+
}
97+
case objstore.S3:
98+
subCfg = struct {
99+
Bucket string `yaml:"bucket"`
100+
Endpoint string `yaml:"endpoint"`
101+
AccessKey string `yaml:"access_key"`
102+
SecretKey string `yaml:"secret_key"`
103+
MaxRetries int `yaml:"max_retries"`
104+
Insecure bool `yaml:"insecure"`
105+
}{
106+
Bucket: opts.s3Bucket,
107+
Endpoint: opts.s3Endpoint,
108+
AccessKey: opts.s3AccessKey,
109+
SecretKey: opts.s3SecretKey,
110+
Insecure: opts.s3Insecure,
111+
MaxRetries: opts.retries,
112+
}
113+
default:
114+
return nil, fmt.Errorf("unknown bucket type: %s", prov)
115+
}
116+
117+
cfg.Config = subCfg
118+
confContentYaml, err = yaml.Marshal(cfg)
119+
if err != nil {
120+
return nil, fmt.Errorf("unable to marshal bucket config yaml: %w", err)
93121
}
94-
default:
95-
return nil, fmt.Errorf("unknown bucket type: %s", prov)
96122
}
97123

98-
cfg.Config = subCfg
99-
bytes, err := yaml.Marshal(cfg)
100-
if err != nil {
101-
return nil, fmt.Errorf("unable to marshal bucket config yaml: %w", err)
124+
// If config is empty, return error
125+
if len(confContentYaml) == 0 {
126+
return nil, fmt.Errorf("objstore config is required")
102127
}
103128

104-
bkt, err := client.NewBucket(slogAdapter{log}, bytes, "parquet-gateway", nil)
129+
bkt, err := client.NewBucket(slogAdapter{log}, confContentYaml, "parquet-gateway", nil)
105130
if err != nil {
106131
return nil, fmt.Errorf("unable to create bucket client: %w", err)
107132
}

‎cmd/config_test.go‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
// Copyright (c) The Thanos Authors.
2+
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
3+
// https://opensource.org/licenses/Apache-2.0
4+
5+
package main
6+
7+
import (
8+
"io"
9+
"log/slog"
10+
"os"
11+
"path/filepath"
12+
"testing"
13+
14+
"go.uber.org/goleak"
15+
)
16+
17+
func TestMain(m *testing.M) {
18+
goleak.VerifyTestMain(m, goleak.IgnoreTopFunction("github.com/baidubce/bce-sdk-go/util/log.NewLogger.func1"))
19+
}
20+
21+
func TestSetupBucketWithConfigFile(t *testing.T) {
22+
t.Run("filesystem config from file", func(tt *testing.T) {
23+
tmpDir := tt.TempDir()
24+
configFile := filepath.Join(tmpDir, "config.yaml")
25+
configContent := `type: FILESYSTEM
26+
config:
27+
directory: ` + tmpDir + `
28+
`
29+
if err := os.WriteFile(configFile, []byte(configContent), 0644); err != nil {
30+
tt.Fatalf("unable to write config file: %v", err)
31+
}
32+
33+
opts := bucketOpts{
34+
objStoreConfigFile: configFile,
35+
}
36+
37+
log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError}))
38+
bkt, err := setupBucket(log, opts)
39+
if err != nil {
40+
tt.Fatalf("unable to setup bucket: %v", err)
41+
}
42+
if bkt == nil {
43+
tt.Fatal("bucket is nil")
44+
}
45+
46+
// Verify it's a filesystem bucket by checking if we can list (empty bucket)
47+
ctx := tt.Context()
48+
if err := bkt.Iter(ctx, "", func(name string) error {
49+
return nil
50+
}); err != nil {
51+
tt.Fatalf("unable to iterate bucket: %v", err)
52+
}
53+
})
54+
55+
t.Run("filesystem config from inline yaml", func(tt *testing.T) {
56+
tmpDir := tt.TempDir()
57+
configContent := `type: FILESYSTEM
58+
config:
59+
directory: ` + tmpDir + `
60+
`
61+
62+
opts := bucketOpts{
63+
objStoreConfig: configContent,
64+
}
65+
66+
log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError}))
67+
bkt, err := setupBucket(log, opts)
68+
if err != nil {
69+
tt.Fatalf("unable to setup bucket: %v", err)
70+
}
71+
if bkt == nil {
72+
tt.Fatal("bucket is nil")
73+
}
74+
75+
ctx := tt.Context()
76+
if err := bkt.Iter(ctx, "", func(name string) error {
77+
return nil
78+
}); err != nil {
79+
tt.Fatalf("unable to iterate bucket: %v", err)
80+
}
81+
})
82+
83+
t.Run("empty config returns error", func(tt *testing.T) {
84+
opts := bucketOpts{}
85+
86+
log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError}))
87+
_, err := setupBucket(log, opts)
88+
if err == nil {
89+
tt.Fatal("expected error for empty config")
90+
}
91+
})
92+
93+
t.Run("invalid config returns error", func(tt *testing.T) {
94+
opts := bucketOpts{
95+
objStoreConfig: "invalid: yaml: content",
96+
}
97+
98+
log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError}))
99+
_, err := setupBucket(log, opts)
100+
if err == nil {
101+
tt.Fatal("expected error for invalid config")
102+
}
103+
})
104+
105+
t.Run("s3 config from file", func(tt *testing.T) {
106+
configFile := filepath.Join(tt.TempDir(), "config.yaml")
107+
configContent := `type: S3
108+
config:
109+
bucket: test-bucket
110+
endpoint: localhost:9000
111+
access_key: minioadmin
112+
secret_key: minioadmin
113+
insecure: true
114+
`
115+
if err := os.WriteFile(configFile, []byte(configContent), 0644); err != nil {
116+
tt.Fatalf("unable to write config file: %v", err)
117+
}
118+
119+
opts := bucketOpts{
120+
objStoreConfigFile: configFile,
121+
}
122+
123+
log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError}))
124+
bkt, err := setupBucket(log, opts)
125+
// S3 bucket creation might fail if minio is not running, but config parsing should work
126+
if err != nil && bkt == nil {
127+
// This is expected if S3 endpoint is not available
128+
return
129+
}
130+
if bkt != nil {
131+
// If bucket was created, verify it's the right type
132+
ctx := tt.Context()
133+
_ = bkt.Iter(ctx, "", func(name string) error {
134+
return nil
135+
})
136+
}
137+
})
138+
}
139+
140+
func TestSetupBucketWithLegacyFlags(t *testing.T) {
141+
t.Run("filesystem config from legacy flags", func(tt *testing.T) {
142+
tmpDir := tt.TempDir()
143+
opts := bucketOpts{
144+
storage: "filesystem",
145+
filesystemDirectory: tmpDir,
146+
}
147+
148+
log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError}))
149+
bkt, err := setupBucket(log, opts)
150+
if err != nil {
151+
tt.Fatalf("unable to setup bucket: %v", err)
152+
}
153+
if bkt == nil {
154+
tt.Fatal("bucket is nil")
155+
}
156+
157+
// Verify it's a filesystem bucket by checking if we can list (empty bucket)
158+
ctx := tt.Context()
159+
if err := bkt.Iter(ctx, "", func(name string) error {
160+
return nil
161+
}); err != nil {
162+
tt.Fatalf("unable to iterate bucket: %v", err)
163+
}
164+
})
165+
166+
t.Run("config file takes precedence over legacy flags", func(tt *testing.T) {
167+
tmpDir := tt.TempDir()
168+
configFile := filepath.Join(tmpDir, "config.yaml")
169+
configContent := `type: FILESYSTEM
170+
config:
171+
directory: ` + tmpDir + `
172+
`
173+
if err := os.WriteFile(configFile, []byte(configContent), 0644); err != nil {
174+
tt.Fatalf("unable to write config file: %v", err)
175+
}
176+
177+
// Provide both config file and legacy flags - config file should win
178+
opts := bucketOpts{
179+
objStoreConfigFile: configFile,
180+
storage: "s3", // This should be ignored
181+
s3Bucket: "should-be-ignored",
182+
}
183+
184+
log := slog.New(slog.NewTextHandler(io.Discard, &slog.HandlerOptions{Level: slog.LevelError}))
185+
bkt, err := setupBucket(log, opts)
186+
if err != nil {
187+
tt.Fatalf("unable to setup bucket: %v", err)
188+
}
189+
if bkt == nil {
190+
tt.Fatal("bucket is nil")
191+
}
192+
193+
// Verify it's a filesystem bucket (from config file, not the s3 legacy flags)
194+
ctx := tt.Context()
195+
if err := bkt.Iter(ctx, "", func(name string) error {
196+
return nil
197+
}); err != nil {
198+
tt.Fatalf("unable to iterate bucket: %v", err)
199+
}
200+
})
201+
}
202+

‎cmd/convert.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ func (opts *conversionOpts) registerFlags(cmd *kingpin.CmdClause) {
8585
}
8686

8787
func (opts *bucketOpts) registerConvertParquetFlags(cmd *kingpin.CmdClause) {
88+
// New config file options (preferred, like regular Thanos)
89+
cmd.Flag("objstore-parquet.config-file", "YAML file that contains object store configuration for parquet storage. See format details: https://thanos.io/tip/thanos/storage.md/#configuration").StringVar(&opts.objStoreConfigFile)
90+
cmd.Flag("objstore-parquet.config", "Alternative to 'objstore-parquet.config-file'. YAML content for parquet storage configuration.").StringVar(&opts.objStoreConfig)
91+
92+
// Legacy individual flags (for backwards compatibility)
8893
cmd.Flag("parquet.storage.type", "type of storage").Default("filesystem").EnumVar(&opts.storage, "filesystem", "s3")
8994
cmd.Flag("parquet.storage.prefix", "prefix for the storage").Default("").StringVar(&opts.prefix)
9095
cmd.Flag("parquet.storage.filesystem.directory", "directory for filesystem").Default(".data").StringVar(&opts.filesystemDirectory)
@@ -96,6 +101,11 @@ func (opts *bucketOpts) registerConvertParquetFlags(cmd *kingpin.CmdClause) {
96101
}
97102

98103
func (opts *bucketOpts) registerConvertTSDBFlags(cmd *kingpin.CmdClause) {
104+
// New config file options (preferred, like regular Thanos)
105+
cmd.Flag("objstore-tsdb.config-file", "YAML file that contains object store configuration for TSDB storage. See format details: https://thanos.io/tip/thanos/storage.md/#configuration").StringVar(&opts.objStoreConfigFile)
106+
cmd.Flag("objstore-tsdb.config", "Alternative to 'objstore-tsdb.config-file'. YAML content for TSDB storage configuration.").StringVar(&opts.objStoreConfig)
107+
108+
// Legacy individual flags (for backwards compatibility)
99109
cmd.Flag("tsdb.storage.type", "type of storage").Default("filesystem").EnumVar(&opts.storage, "filesystem", "s3")
100110
cmd.Flag("tsdb.storage.prefix", "prefix for the storage").Default("").StringVar(&opts.prefix)
101111
cmd.Flag("tsdb.storage.filesystem.directory", "directory for filesystem").Default(".data").StringVar(&opts.filesystemDirectory)

‎cmd/serve.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ func (opts *serveOpts) registerFlags(cmd *kingpin.CmdClause) {
6565
}
6666

6767
func (opts *bucketOpts) registerServeFlags(cmd *kingpin.CmdClause) {
68+
// New config file options (preferred, like regular Thanos)
69+
cmd.Flag("objstore.config-file", "YAML file that contains object store configuration. See format details: https://thanos.io/tip/thanos/storage.md/#configuration").StringVar(&opts.objStoreConfigFile)
70+
cmd.Flag("objstore.config", "Alternative to 'objstore.config-file'. YAML content for object store configuration.").StringVar(&opts.objStoreConfig)
71+
72+
// Legacy individual flags (for backwards compatibility)
6873
cmd.Flag("storage.type", "type of storage").Default("filesystem").EnumVar(&opts.storage, "filesystem", "s3")
6974
cmd.Flag("storage.prefix", "prefix for the storage").Default("").StringVar(&opts.prefix)
7075
cmd.Flag("storage.filesystem.directory", "directory for filesystem").Default(".data").StringVar(&opts.filesystemDirectory)

0 commit comments

Comments
 (0)