diff --git a/pkg/loki/config_wrapper_test.go b/pkg/loki/config_wrapper_test.go index 49dbdbec6553..7c65b0f6c9d0 100644 --- a/pkg/loki/config_wrapper_test.go +++ b/pkg/loki/config_wrapper_test.go @@ -270,8 +270,8 @@ memberlist: assert.Equal(t, false, actual.S3ForcePathStyle) assert.Equal(t, "s3://foo-bucket", actual.Endpoint) assert.Equal(t, "us-east1", actual.Region) - assert.Equal(t, "abc123", actual.AccessKeyID.Value) - assert.Equal(t, "def789", actual.SecretAccessKey.Value) + assert.Equal(t, "abc123", actual.AccessKeyID) + assert.Equal(t, "def789", actual.SecretAccessKey.String()) assert.Equal(t, true, actual.Insecure) assert.Equal(t, false, actual.SSEEncryption) assert.Equal(t, 5*time.Minute, actual.HTTPConfig.ResponseHeaderTimeout) @@ -406,7 +406,7 @@ memberlist: assert.Equal(t, "arcosx", actual.BucketName) assert.Equal(t, "bj.bcebos.com", actual.Endpoint) assert.Equal(t, "baidu", actual.AccessKeyID) - assert.Equal(t, "bce", actual.SecretAccessKey) + assert.Equal(t, "bce", actual.SecretAccessKey.String()) } // should remain empty @@ -538,8 +538,8 @@ ruler: assert.Equal(t, "s3", config.Ruler.StoreConfig.Type) assert.Equal(t, "s3://foo-bucket", config.Ruler.StoreConfig.S3.Endpoint) assert.Equal(t, "us-east1", config.Ruler.StoreConfig.S3.Region) - assert.Equal(t, "abc123", config.Ruler.StoreConfig.S3.AccessKeyID.Value) - assert.Equal(t, "def789", config.Ruler.StoreConfig.S3.SecretAccessKey.Value) + assert.Equal(t, "abc123", config.Ruler.StoreConfig.S3.AccessKeyID) + assert.Equal(t, "def789", config.Ruler.StoreConfig.S3.SecretAccessKey.String()) // should be set by common config assert.EqualValues(t, "foobar", config.StorageConfig.GCSConfig.BucketName) @@ -568,8 +568,8 @@ storage_config: assert.Equal(t, "s3://foo-bucket", config.StorageConfig.AWSStorageConfig.S3Config.Endpoint) assert.Equal(t, "us-east1", config.StorageConfig.AWSStorageConfig.S3Config.Region) - assert.Equal(t, "abc123", config.StorageConfig.AWSStorageConfig.S3Config.AccessKeyID.Value) - assert.Equal(t, "def789", config.StorageConfig.AWSStorageConfig.S3Config.SecretAccessKey.Value) + assert.Equal(t, "abc123", config.StorageConfig.AWSStorageConfig.S3Config.AccessKeyID) + assert.Equal(t, "def789", config.StorageConfig.AWSStorageConfig.S3Config.SecretAccessKey.String()) // should be set by common config assert.EqualValues(t, "foobar", config.Ruler.StoreConfig.GCS.BucketName) diff --git a/pkg/storage/chunk/client/aws/s3_storage_client.go b/pkg/storage/chunk/client/aws/s3_storage_client.go index badf31bb59e9..bc6d5f12fe03 100644 --- a/pkg/storage/chunk/client/aws/s3_storage_client.go +++ b/pkg/storage/chunk/client/aws/s3_storage_client.go @@ -14,8 +14,6 @@ import ( "strings" "time" - "gopkg.in/yaml.v2" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" @@ -63,20 +61,6 @@ func init() { s3RequestDuration.Register() } -type secret struct { - Value string -} - -func (secret) MarshalYAML() (interface{}, error) { - return "redacted", nil -} - -func (s *secret) UnmarshalYAML(unmarshal func(interface{}) error) error { - return unmarshal(&s.Value) -} - -var _ yaml.Marshaler = secret{} - // S3Config specifies config for storing chunks on AWS S3. type S3Config struct { S3 flagext.URLValue @@ -85,8 +69,8 @@ type S3Config struct { BucketNames string Endpoint string `yaml:"endpoint"` Region string `yaml:"region"` - AccessKeyID secret `yaml:"access_key_id"` - SecretAccessKey secret `yaml:"secret_access_key"` + AccessKeyID string `yaml:"access_key_id"` + SecretAccessKey flagext.Secret `yaml:"secret_access_key"` Insecure bool `yaml:"insecure"` SSEEncryption bool `yaml:"sse_encryption"` HTTPConfig HTTPConfig `yaml:"http_config"` @@ -119,8 +103,8 @@ func (cfg *S3Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { f.StringVar(&cfg.Endpoint, prefix+"s3.endpoint", "", "S3 Endpoint to connect to.") f.StringVar(&cfg.Region, prefix+"s3.region", "", "AWS region to use.") - f.StringVar(&cfg.AccessKeyID.Value, prefix+"s3.access-key-id", "", "AWS Access Key ID") - f.StringVar(&cfg.SecretAccessKey.Value, prefix+"s3.secret-access-key", "", "AWS Secret Access Key") + f.StringVar(&cfg.AccessKeyID, prefix+"s3.access-key-id", "", "AWS Access Key ID") + f.Var(&cfg.SecretAccessKey, prefix+"s3.secret-access-key", "AWS Secret Access Key") f.BoolVar(&cfg.Insecure, prefix+"s3.insecure", false, "Disable https on s3 connection.") // TODO Remove in Cortex 1.10.0 @@ -253,13 +237,13 @@ func buildS3Client(cfg S3Config, hedgingCfg hedging.Config, hedging bool) (*s3.S s3Config = s3Config.WithRegion(cfg.Region) } - if cfg.AccessKeyID.Value != "" && cfg.SecretAccessKey.Value == "" || - cfg.AccessKeyID.Value == "" && cfg.SecretAccessKey.Value != "" { + if cfg.AccessKeyID != "" && cfg.SecretAccessKey.String() == "" || + cfg.AccessKeyID == "" && cfg.SecretAccessKey.String() != "" { return nil, errors.New("must supply both an Access Key ID and Secret Access Key or neither") } - if cfg.AccessKeyID.Value != "" && cfg.SecretAccessKey.Value != "" { - creds := credentials.NewStaticCredentials(cfg.AccessKeyID.Value, cfg.SecretAccessKey.Value, "") + if cfg.AccessKeyID != "" && cfg.SecretAccessKey.String() != "" { + creds := credentials.NewStaticCredentials(cfg.AccessKeyID, cfg.SecretAccessKey.String(), "") s3Config = s3Config.WithCredentials(creds) } diff --git a/pkg/storage/chunk/client/aws/s3_storage_client_test.go b/pkg/storage/chunk/client/aws/s3_storage_client_test.go index f61cbeeb5019..2e2193702c6c 100644 --- a/pkg/storage/chunk/client/aws/s3_storage_client_test.go +++ b/pkg/storage/chunk/client/aws/s3_storage_client_test.go @@ -15,6 +15,7 @@ import ( "gopkg.in/yaml.v2" "github.com/grafana/dskit/backoff" + "github.com/grafana/dskit/flagext" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/atomic" @@ -39,8 +40,8 @@ func TestRequestMiddleware(t *testing.T) { BucketNames: "buck-o", S3ForcePathStyle: true, Insecure: true, - AccessKeyID: secret{"key"}, - SecretAccessKey: secret{"secret"}, + AccessKeyID: "key", + SecretAccessKey: flagext.SecretWithValue("secret"), } tests := []struct { @@ -128,8 +129,8 @@ func Test_Hedging(t *testing.T) { count := atomic.NewInt32(0) c, err := NewS3ObjectClient(S3Config{ - AccessKeyID: secret{"foo"}, - SecretAccessKey: secret{"bar"}, + AccessKeyID: "foo", + SecretAccessKey: flagext.SecretWithValue("bar"), BackoffConfig: backoff.Config{MaxRetries: 1}, BucketNames: "foo", Inject: func(next http.RoundTripper) http.RoundTripper { @@ -153,14 +154,14 @@ func Test_Hedging(t *testing.T) { func Test_ConfigRedactsCredentials(t *testing.T) { underTest := S3Config{ - AccessKeyID: secret{"secret key id"}, - SecretAccessKey: secret{"secret access key"}, + AccessKeyID: "access key id", + SecretAccessKey: flagext.SecretWithValue("secret access key"), } output, err := yaml.Marshal(underTest) require.NoError(t, err) - require.False(t, bytes.Contains(output, []byte("secret key id"))) + require.True(t, bytes.Contains(output, []byte("access key id"))) require.False(t, bytes.Contains(output, []byte("secret access id"))) } @@ -173,7 +174,7 @@ secret_access_key: secret access key err := yaml.Unmarshal([]byte(yamlCfg), &underTest) require.NoError(t, err) - require.Equal(t, underTest.AccessKeyID.Value, "access key id") - require.Equal(t, underTest.SecretAccessKey.Value, "secret access key") + require.Equal(t, underTest.AccessKeyID, "access key id") + require.Equal(t, underTest.SecretAccessKey.String(), "secret access key") } diff --git a/pkg/storage/chunk/client/baidubce/bos_storage_client.go b/pkg/storage/chunk/client/baidubce/bos_storage_client.go index 4b79aaf2854a..d44bc004e52b 100644 --- a/pkg/storage/chunk/client/baidubce/bos_storage_client.go +++ b/pkg/storage/chunk/client/baidubce/bos_storage_client.go @@ -10,6 +10,7 @@ import ( "github.com/baidubce/bce-sdk-go/bce" "github.com/baidubce/bce-sdk-go/services/bos" "github.com/baidubce/bce-sdk-go/services/bos/api" + "github.com/grafana/dskit/flagext" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/weaveworks/common/instrument" @@ -36,10 +37,10 @@ func init() { } type BOSStorageConfig struct { - BucketName string `yaml:"bucket_name"` - Endpoint string `yaml:"endpoint"` - AccessKeyID string `yaml:"access_key_id"` - SecretAccessKey string `yaml:"secret_access_key"` + BucketName string `yaml:"bucket_name"` + Endpoint string `yaml:"endpoint"` + AccessKeyID string `yaml:"access_key_id"` + SecretAccessKey flagext.Secret `yaml:"secret_access_key"` } // RegisterFlags adds the flags required to config this to the given FlagSet @@ -52,7 +53,7 @@ func (cfg *BOSStorageConfig) RegisterFlagsWithPrefix(prefix string, f *flag.Flag f.StringVar(&cfg.BucketName, prefix+"baidubce.bucket-name", "", "Name of BOS bucket.") f.StringVar(&cfg.Endpoint, prefix+"baidubce.endpoint", DefaultEndpoint, "BOS endpoint to connect to.") f.StringVar(&cfg.AccessKeyID, prefix+"baidubce.access-key-id", "", "Baidu Cloud Engine (BCE) Access Key ID.") - f.StringVar(&cfg.SecretAccessKey, prefix+"baidubce.secret-access-key", "", "Baidu Cloud Engine (BCE) Secret Access Key.") + f.Var(&cfg.SecretAccessKey, prefix+"baidubce.secret-access-key", "Baidu Cloud Engine (BCE) Secret Access Key.") } type BOSObjectStorage struct { @@ -63,7 +64,7 @@ type BOSObjectStorage struct { func NewBOSObjectStorage(cfg *BOSStorageConfig) (*BOSObjectStorage, error) { clientConfig := bos.BosClientConfiguration{ Ak: cfg.AccessKeyID, - Sk: cfg.SecretAccessKey, + Sk: cfg.SecretAccessKey.String(), Endpoint: cfg.Endpoint, RedirectDisabled: false, } diff --git a/pkg/storage/chunk/client/baidubce/bos_storage_client_test.go b/pkg/storage/chunk/client/baidubce/bos_storage_client_test.go new file mode 100644 index 000000000000..8e68d9a1f2d4 --- /dev/null +++ b/pkg/storage/chunk/client/baidubce/bos_storage_client_test.go @@ -0,0 +1,24 @@ +package baidubce + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v2" + + "github.com/grafana/dskit/flagext" +) + +func Test_ConfigRedactsCredentials(t *testing.T) { + underTest := BOSStorageConfig{ + AccessKeyID: "access key id", + SecretAccessKey: flagext.SecretWithValue("secret access key"), + } + + output, err := yaml.Marshal(underTest) + require.NoError(t, err) + + require.True(t, bytes.Contains(output, []byte("access key id"))) + require.False(t, bytes.Contains(output, []byte("secret access id"))) +}