Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
only include named store in storage_config.object_stores
  • Loading branch information
ashwanthgoli committed Oct 30, 2024
commit cfceecbfe06aa3ace5bca8858259a644486e2ba2
4 changes: 2 additions & 2 deletions pkg/loki/config_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,9 @@ func applyStorageConfig(cfg, defaults *ConfigWrapper) error {
}
}

if !reflect.DeepEqual(cfg.Common.Storage.ObjectStore, defaults.StorageConfig.ObjectStore) {
if !reflect.DeepEqual(cfg.Common.Storage.ObjectStore, defaults.StorageConfig.ObjectStore.Config) {
applyConfig = func(r *ConfigWrapper) {
r.StorageConfig.ObjectStore = r.Common.Storage.ObjectStore
r.StorageConfig.ObjectStore.Config = r.Common.Storage.ObjectStore
}
}

Expand Down
134 changes: 134 additions & 0 deletions pkg/loki/config_wrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import (

"github.com/grafana/loki/v3/pkg/distributor"
"github.com/grafana/loki/v3/pkg/loki/common"
azurebucket "github.com/grafana/loki/v3/pkg/storage/bucket/azure"
"github.com/grafana/loki/v3/pkg/storage/bucket/filesystem"
"github.com/grafana/loki/v3/pkg/storage/bucket/gcs"
"github.com/grafana/loki/v3/pkg/storage/bucket/s3"
"github.com/grafana/loki/v3/pkg/storage/bucket/swift"
"github.com/grafana/loki/v3/pkg/storage/chunk/client/alibaba"
"github.com/grafana/loki/v3/pkg/storage/chunk/client/aws"
Expand Down Expand Up @@ -842,6 +846,48 @@ storage_config:
assert.Equal(t, "789abc", config.StorageConfig.NamedStores.AWS["store-2"].S3Config.SecretAccessKey.String())
})

t.Run("named storage config (thanos) provided via config file is preserved", func(t *testing.T) {
namedStoresConfig := `common:
storage:
object_store:
s3:
endpoint: s3://common-bucket
region: us-east1
access_key_id: abc123
secret_access_key: def789
storage_config:
object_store:
named_stores:
s3:
store-1:
endpoint: s3://foo-bucket
region: us-west1
access_key_id: 123abc
secret_access_key: 789def
store-2:
endpoint: s3://bar-bucket
region: us-west2
access_key_id: 456def
secret_access_key: 789abc`
config, _ := testContext(namedStoresConfig, nil)

// should be set by common config
assert.Equal(t, "s3://common-bucket", config.StorageConfig.ObjectStore.S3.Endpoint)
assert.Equal(t, "us-east1", config.StorageConfig.ObjectStore.S3.Region)
assert.Equal(t, "abc123", config.StorageConfig.ObjectStore.S3.AccessKeyID)
assert.Equal(t, "def789", config.StorageConfig.ObjectStore.S3.SecretAccessKey.String())

assert.Equal(t, "s3://foo-bucket", config.StorageConfig.ObjectStore.NamedStores.S3["store-1"].Endpoint)
assert.Equal(t, "us-west1", config.StorageConfig.ObjectStore.NamedStores.S3["store-1"].Region)
assert.Equal(t, "123abc", config.StorageConfig.ObjectStore.NamedStores.S3["store-1"].AccessKeyID)
assert.Equal(t, "789def", config.StorageConfig.ObjectStore.NamedStores.S3["store-1"].SecretAccessKey.String())

assert.Equal(t, "s3://bar-bucket", config.StorageConfig.ObjectStore.NamedStores.S3["store-2"].Endpoint)
assert.Equal(t, "us-west2", config.StorageConfig.ObjectStore.NamedStores.S3["store-2"].Region)
assert.Equal(t, "456def", config.StorageConfig.ObjectStore.NamedStores.S3["store-2"].AccessKeyID)
assert.Equal(t, "789abc", config.StorageConfig.ObjectStore.NamedStores.S3["store-2"].SecretAccessKey.String())
})

t.Run("partial ruler config from file is honored for overriding things like bucket names", func(t *testing.T) {
specificRulerConfig := `common:
storage:
Expand Down Expand Up @@ -2280,3 +2326,91 @@ func TestNamedStores_applyDefaults(t *testing.T) {
assert.Equal(t, expected, (alibaba.OssConfig)(nsCfg.AlibabaCloud["store-8"]))
})
}

func TestBucketNamedStores_applyDefaults(t *testing.T) {
namedStoresConfig := `storage_config:
object_store:
named_stores:
s3:
store-1:
endpoint: s3.test
bucket_name: foobar
dualstack_enabled: false
azure:
store-2:
account_name: foo
container_name: bar
max_retries: 3
gcs:
store-3:
bucket_name: foobar
filesystem:
store-4:
dir: foobar
swift:
store-5:
container_name: foobar
request_timeout: 30s
`
// make goconst happy
bucketName := "foobar"

config, defaults, err := configWrapperFromYAML(t, namedStoresConfig, nil)
require.NoError(t, err)

nsCfg := config.StorageConfig.ObjectStore.NamedStores

t.Run("s3", func(t *testing.T) {
assert.Len(t, nsCfg.S3, 1)

// expect the defaults to be set on named store config
expected := defaults.StorageConfig.ObjectStore.S3
expected.BucketName = bucketName
expected.Endpoint = "s3.test"
// override defaults
expected.DualstackEnabled = false

assert.Equal(t, expected, (s3.Config)(nsCfg.S3["store-1"]))
})

t.Run("azure", func(t *testing.T) {
assert.Len(t, nsCfg.Azure, 1)

expected := defaults.StorageConfig.ObjectStore.Azure
expected.StorageAccountName = "foo"
expected.ContainerName = "bar"
// overrides defaults
expected.MaxRetries = 3

assert.Equal(t, expected, (azurebucket.Config)(nsCfg.Azure["store-2"]))
})

t.Run("gcs", func(t *testing.T) {
assert.Len(t, nsCfg.GCS, 1)

expected := defaults.StorageConfig.ObjectStore.GCS
expected.BucketName = bucketName

assert.Equal(t, expected, (gcs.Config)(nsCfg.GCS["store-3"]))
})

t.Run("filesystem", func(t *testing.T) {
assert.Len(t, nsCfg.Filesystem, 1)

expected := defaults.StorageConfig.ObjectStore.Filesystem
expected.Directory = bucketName

assert.Equal(t, expected, (filesystem.Config)(nsCfg.Filesystem["store-4"]))
})

t.Run("swift", func(t *testing.T) {
assert.Len(t, nsCfg.Swift, 1)

expected := defaults.StorageConfig.ObjectStore.Swift
expected.ContainerName = bucketName
// override defaults
expected.RequestTimeout = 30 * time.Second

assert.Equal(t, expected, (swift.Config)(nsCfg.Swift["store-5"]))
})
}
2 changes: 1 addition & 1 deletion pkg/ruler/base/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func NewRuleStore(ctx context.Context, cfg rulestore.Config, cfgProvider bucket.
return local.NewLocalRulesClient(cfg.Local, loader)
}

bucketClient, err := bucket.NewClient(ctx, cfg.Backend, cfg.Config, "ruler-storage", logger)
bucketClient, err := bucket.NewClient(ctx, cfg.Backend, cfg.Config, "ruler-storage", logger, nil)
if err != nil {
return nil, err
}
Expand Down
13 changes: 2 additions & 11 deletions pkg/storage/bucket/azure/bucket_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ import (
"github.com/thanos-io/objstore/providers/azure"
)

func NewBucketClient(cfg Config, name string, logger log.Logger) (objstore.Bucket, error) {
return newBucketClient(cfg, name, logger, azure.NewBucketWithConfig)
}

func newBucketClient(cfg Config, name string, logger log.Logger, factory func(log.Logger, azure.Config, string, http.RoundTripper) (*azure.Bucket, error)) (objstore.Bucket, error) {
func NewBucketClient(cfg Config, name string, logger log.Logger, rt http.RoundTripper) (objstore.Bucket, error) {
// Start with default config to make sure that all parameters are set to sensible values, especially
// HTTP Config field.
bucketConfig := azure.DefaultConfig
Expand All @@ -28,10 +24,5 @@ func newBucketClient(cfg Config, name string, logger log.Logger, factory func(lo
bucketConfig.Endpoint = cfg.Endpoint
}

var rt http.RoundTripper
if cfg.Transport != nil {
rt = cfg.Transport
}

return factory(logger, bucketConfig, name, rt)
return azure.NewBucketWithConfig(logger, bucketConfig, name, rt)
}
4 changes: 0 additions & 4 deletions pkg/storage/bucket/azure/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package azure

import (
"flag"
"net/http"

"github.com/grafana/dskit/flagext"
)
Expand All @@ -16,9 +15,6 @@ type Config struct {
Endpoint string `yaml:"endpoint_suffix"`
MaxRetries int `yaml:"max_retries"`
UserAssignedID string `yaml:"user_assigned_id"`

// Allow upstream callers to inject a round tripper
Transport http.RoundTripper `yaml:"-"`
}

// RegisterFlags registers the flags for Azure storage
Expand Down
Loading
Loading