redact s3 credential values when printing config to logs (#5961)

pkg/loki/config_wrapper_test.go

@@ -267,8 +267,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)
-				assert.Equal(t, "def789", actual.SecretAccessKey)
+				assert.Equal(t, "abc123", actual.AccessKeyID.Value)
+				assert.Equal(t, "def789", actual.SecretAccessKey.Value)
 				assert.Equal(t, true, actual.Insecure)
 				assert.Equal(t, false, actual.SSEEncryption)
 				assert.Equal(t, 5*time.Minute, actual.HTTPConfig.ResponseHeaderTimeout)
@@ -490,8 +490,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)
-			assert.Equal(t, "def789", config.Ruler.StoreConfig.S3.SecretAccessKey)
+			assert.Equal(t, "abc123", config.Ruler.StoreConfig.S3.AccessKeyID.Value)
+			assert.Equal(t, "def789", config.Ruler.StoreConfig.S3.SecretAccessKey.Value)
 
 			// should be set by common config
 			assert.EqualValues(t, "foobar", config.StorageConfig.GCSConfig.BucketName)
@@ -520,8 +520,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)
-			assert.Equal(t, "def789", config.StorageConfig.AWSStorageConfig.S3Config.SecretAccessKey)
+			assert.Equal(t, "abc123", config.StorageConfig.AWSStorageConfig.S3Config.AccessKeyID.Value)
+			assert.Equal(t, "def789", config.StorageConfig.AWSStorageConfig.S3Config.SecretAccessKey.Value)
 
 			// should be set by common config
 			assert.EqualValues(t, "foobar", config.Ruler.StoreConfig.GCS.BucketName)

pkg/storage/chunk/client/aws/s3_storage_client.go

@@ -14,6 +14,8 @@ 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"
@@ -61,6 +63,20 @@ 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
@@ -69,8 +85,8 @@ type S3Config struct {
 	BucketNames      string
 	Endpoint         string              `yaml:"endpoint"`
 	Region           string              `yaml:"region"`
-	AccessKeyID      string              `yaml:"access_key_id"`
-	SecretAccessKey  string              `yaml:"secret_access_key"`
+	AccessKeyID      secret              `yaml:"access_key_id"`
+	SecretAccessKey  secret              `yaml:"secret_access_key"`
 	Insecure         bool                `yaml:"insecure"`
 	SSEEncryption    bool                `yaml:"sse_encryption"`
 	HTTPConfig       HTTPConfig          `yaml:"http_config"`
@@ -103,8 +119,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, prefix+"s3.access-key-id", "", "AWS Access Key ID")
-	f.StringVar(&cfg.SecretAccessKey, prefix+"s3.secret-access-key", "", "AWS Secret Access Key")
+	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.BoolVar(&cfg.Insecure, prefix+"s3.insecure", false, "Disable https on s3 connection.")
 
 	// TODO Remove in Cortex 1.10.0
@@ -237,13 +253,13 @@ func buildS3Client(cfg S3Config, hedgingCfg hedging.Config, hedging bool) (*s3.S
 		s3Config = s3Config.WithRegion(cfg.Region)
 	}
 
-	if cfg.AccessKeyID != "" && cfg.SecretAccessKey == "" ||
-		cfg.AccessKeyID == "" && cfg.SecretAccessKey != "" {
+	if cfg.AccessKeyID.Value != "" && cfg.SecretAccessKey.Value == "" ||
+		cfg.AccessKeyID.Value == "" && cfg.SecretAccessKey.Value != "" {
 		return nil, errors.New("must supply both an Access Key ID and Secret Access Key or neither")
 	}
 
-	if cfg.AccessKeyID != "" && cfg.SecretAccessKey != "" {
-		creds := credentials.NewStaticCredentials(cfg.AccessKeyID, cfg.SecretAccessKey, "")
+	if cfg.AccessKeyID.Value != "" && cfg.SecretAccessKey.Value != "" {
+		creds := credentials.NewStaticCredentials(cfg.AccessKeyID.Value, cfg.SecretAccessKey.Value, "")
 		s3Config = s3Config.WithCredentials(creds)
 	}
 

pkg/storage/chunk/client/aws/s3_storage_client_test.go

@@ -12,6 +12,8 @@ import (
 	"testing"
 	"time"
 
+	"gopkg.in/yaml.v2"
+
 	"github.com/grafana/dskit/backoff"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -37,8 +39,8 @@ func TestRequestMiddleware(t *testing.T) {
 		BucketNames:      "buck-o",
 		S3ForcePathStyle: true,
 		Insecure:         true,
-		AccessKeyID:      "key",
-		SecretAccessKey:  "secret",
+		AccessKeyID:      secret{"key"},
+		SecretAccessKey:  secret{"secret"},
 	}
 
 	tests := []struct {
@@ -126,8 +128,8 @@ func Test_Hedging(t *testing.T) {
 			count := atomic.NewInt32(0)
 
 			c, err := NewS3ObjectClient(S3Config{
-				AccessKeyID:     "foo",
-				SecretAccessKey: "bar",
+				AccessKeyID:     secret{"foo"},
+				SecretAccessKey: secret{"bar"},
 				BackoffConfig:   backoff.Config{MaxRetries: 1},
 				BucketNames:     "foo",
 				Inject: func(next http.RoundTripper) http.RoundTripper {
@@ -148,3 +150,30 @@ func Test_Hedging(t *testing.T) {
 		})
 	}
 }
+
+func Test_ConfigRedactsCredentials(t *testing.T) {
+	underTest := S3Config{
+		AccessKeyID:     secret{"secret key id"},
+		SecretAccessKey: secret{"secret access key"},
+	}
+
+	output, err := yaml.Marshal(underTest)
+	require.NoError(t, err)
+
+	require.False(t, bytes.Contains(output, []byte("secret key id")))
+	require.False(t, bytes.Contains(output, []byte("secret access id")))
+}
+
+func Test_ConfigParsesCredentialsInline(t *testing.T) {
+	var underTest = S3Config{}
+	yamlCfg := `
+access_key_id: access key id
+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")
+
+}