From d979e9ac2e974e49d1952d81e4145c44d39122ad Mon Sep 17 00:00:00 2001 From: Soshi Katsuta Date: Sun, 13 Mar 2022 23:45:23 +0900 Subject: [PATCH 1/5] Add SecretFetcher interface and secretsManagerSecretFether type --- internal/metrics/secret_fetcher.go | 61 +++++++++++++++++++++++++ internal/metrics/secret_fetcher_test.go | 58 +++++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 internal/metrics/secret_fetcher.go create mode 100644 internal/metrics/secret_fetcher_test.go diff --git a/internal/metrics/secret_fetcher.go b/internal/metrics/secret_fetcher.go new file mode 100644 index 00000000..84689b10 --- /dev/null +++ b/internal/metrics/secret_fetcher.go @@ -0,0 +1,61 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. +package metrics + +import ( + "errors" + "fmt" + + "github.com/DataDog/datadog-lambda-go/internal/logger" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface" +) + +// SecretFetcher attempts to fetch a secret. +type SecretFetcher interface { + FetchSecret(secretID string) (string, error) +} + +// secretsManagerSecretFether fetches a secret from AWS Secrets Manager. +type secretsManagerSecretFether struct { + client secretsmanageriface.SecretsManagerAPI +} + +// MakeSecretsManagerSecretFetcher creates a new SecretFetcher which uses the AWS +// Secrets Manager service to fetch a secret. +func MakeSecretsManagerSecretFetcher() SecretFetcher { + return &secretsManagerSecretFether{ + client: secretsmanager.New(session.Must(session.NewSession())), + } +} + +// FetchSecret fetches and returns a secret for a given secret ID. +func (sf *secretsManagerSecretFether) FetchSecret(secretID string) (string, error) { + logger.Debug("Fetching Secrets Manager secret " + secretID) + output, err := sf.client.GetSecretValue(&secretsmanager.GetSecretValueInput{ + SecretId: &secretID, + }) + if err != nil { + return "", fmt.Errorf( + "could not retrieve Secrets Manager secret %q: %s", secretID, err, + ) + } + + if s := output.SecretString; s != nil { + return *s, nil + } + + if b := output.SecretBinary; b != nil { + // SecretBinary field has been automatically base64 decoded by the AWS SDK + return string(b), nil + } + + // Should not happen but let's handle this gracefully + logger.Error(errors.New( + "Secrets Manager returned something but there seems to be no data available", + )) + return "", nil +} diff --git a/internal/metrics/secret_fetcher_test.go b/internal/metrics/secret_fetcher_test.go new file mode 100644 index 00000000..1fb77ac7 --- /dev/null +++ b/internal/metrics/secret_fetcher_test.go @@ -0,0 +1,58 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. +package metrics + +import ( + "testing" + + "github.com/aws/aws-sdk-go/service/secretsmanager" + "github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface" + "github.com/stretchr/testify/assert" +) + +type mockSecretsManagerClient struct { + secretsmanageriface.SecretsManagerAPI + + secretString string + secretBinary []byte +} + +func (c mockSecretsManagerClient) GetSecretValue( + input *secretsmanager.GetSecretValueInput, +) (*secretsmanager.GetSecretValueOutput, error) { + out := &secretsmanager.GetSecretValueOutput{SecretBinary: c.secretBinary} + if c.secretString != "" { + out.SecretString = &c.secretString + } + return out, nil +} + +func TestSecretsManagerSecretsFetcher(t *testing.T) { + tests := map[string]struct { + client mockSecretsManagerClient + want string + }{ + "secret is string": { + client: mockSecretsManagerClient{secretString: "3333333333"}, + want: "3333333333", + }, + "secret is binary": { + client: mockSecretsManagerClient{secretBinary: []byte("4444444444")}, + want: "4444444444", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + fetcher := &secretsManagerSecretFether{client: tt.client} + got, err := fetcher.FetchSecret( + "arn:aws:secretsmanager:us-east-1:123456789012:secret:test-secret", + ) + + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} From ec92a007c9f504b06dde5deb4d83fec23ce92421 Mon Sep 17 00:00:00 2001 From: Soshi Katsuta Date: Sun, 13 Mar 2022 23:46:26 +0900 Subject: [PATCH 2/5] Support the method of giving an API key with an AWS Secrets Manager secret --- internal/metrics/api.go | 75 +++++++++++++++++++++----------- internal/metrics/api_test.go | 84 +++++++++++++++++++++++++----------- 2 files changed, 109 insertions(+), 50 deletions(-) diff --git a/internal/metrics/api.go b/internal/metrics/api.go index f394946d..16765316 100644 --- a/internal/metrics/api.go +++ b/internal/metrics/api.go @@ -28,11 +28,11 @@ type ( // APIClient send metrics to Datadog, via the Datadog API APIClient struct { - apiKey string - apiKeyDecryptChan <-chan string - baseAPIURL string - httpClient *http.Client - context context.Context + apiKey string + apiKeyLazyLoadChan <-chan string + baseAPIURL string + httpClient *http.Client + context context.Context } // APIClientOptions contains instantiation options from creating an APIClient. @@ -41,6 +41,8 @@ type ( apiKey string kmsAPIKey string decrypter Decrypter + apiKeySecretARN string + secretFetcher SecretFetcher httpClientTimeout time.Duration } @@ -60,20 +62,58 @@ func MakeAPIClient(ctx context.Context, options APIClientOptions) *APIClient { httpClient: httpClient, context: ctx, } - if len(options.apiKey) == 0 && len(options.kmsAPIKey) != 0 { - client.apiKeyDecryptChan = client.decryptAPIKey(options.decrypter, options.kmsAPIKey) + if options.apiKey != "" { + return client + } + + if options.kmsAPIKey != "" { + client.apiKeyLazyLoadChan = decryptAPIKey(options.decrypter, options.kmsAPIKey) + } else if options.apiKeySecretARN != "" { + client.apiKeyLazyLoadChan = fetchAPIKey(options.secretFetcher, options.apiKeySecretARN) } return client } +func decryptAPIKey(decrypter Decrypter, kmsAPIKey string) <-chan string { + ch := make(chan string) + + go func() { + result, err := decrypter.Decrypt(kmsAPIKey) + if err != nil { + logger.Error(fmt.Errorf("Couldn't decrypt api kms key: %s", err)) + } + + ch <- result + close(ch) + }() + + return ch +} + +func fetchAPIKey(fetcher SecretFetcher, apiKeySecretARN string) <-chan string { + ch := make(chan string) + + go func() { + result, err := fetcher.FetchSecret(apiKeySecretARN) + if err != nil { + logger.Error(fmt.Errorf("Couldn't retrieve api key secret: %s", err)) + } + + ch <- result + close(ch) + }() + + return ch +} + // SendMetrics posts a batch metrics payload to the Datadog API func (cl *APIClient) SendMetrics(metrics []APIMetric) error { // If the api key was provided as a kms key, wait for it to finish decrypting - if cl.apiKeyDecryptChan != nil { - cl.apiKey = <-cl.apiKeyDecryptChan - cl.apiKeyDecryptChan = nil + if cl.apiKeyLazyLoadChan != nil { + cl.apiKey = <-cl.apiKeyLazyLoadChan + cl.apiKeyLazyLoadChan = nil } content, err := marshalAPIMetricsModel(metrics) @@ -118,21 +158,6 @@ func (cl *APIClient) SendMetrics(metrics []APIMetric) error { return err } -func (cl *APIClient) decryptAPIKey(decrypter Decrypter, kmsAPIKey string) <-chan string { - - ch := make(chan string) - - go func() { - result, err := decrypter.Decrypt(kmsAPIKey) - if err != nil { - logger.Error(fmt.Errorf("Couldn't decrypt api kms key %s", err)) - } - ch <- result - close(ch) - }() - return ch -} - func (cl *APIClient) addAPICredentials(req *http.Request) { query := req.URL.Query() query.Add(apiKeyParam, cl.apiKey) diff --git a/internal/metrics/api_test.go b/internal/metrics/api_test.go index bcefa0d5..09ae0a0d 100644 --- a/internal/metrics/api_test.go +++ b/internal/metrics/api_test.go @@ -22,6 +22,7 @@ const ( mockAPIKey = "12345" mockEncryptedAPIKey = "mockEncrypted" mockDecryptedAPIKey = "mockDecrypted" + mockAPIKeySecretARN = "arn:aws:secretsmanager:us-east-1:123456789012:secret:apiKey" ) type ( @@ -35,6 +36,15 @@ func (md *mockDecrypter) Decrypt(cipherText string) (string, error) { return md.returnValue, md.returnError } +type mockSecretFetcher struct { + returnValue string + returnError error +} + +func (m *mockSecretFetcher) FetchSecret(secretID string) (string, error) { + return m.returnValue, m.returnError +} + func TestAddAPICredentials(t *testing.T) { cl := MakeAPIClient(context.Background(), APIClientOptions{baseAPIURL: "", apiKey: mockAPIKey}) req, _ := http.NewRequest("GET", "http://some-api.com/endpoint", nil) @@ -140,33 +150,57 @@ func TestSendMetricsCantReachServer(t *testing.T) { assert.False(t, called) } -func TestDecryptsUsingKMSKey(t *testing.T) { - called := false - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - called = true - assert.Equal(t, "/distribution_points?api_key=mockDecrypted", r.URL.String()) - })) - defer server.Close() - - am := []APIMetric{ - { - Name: "metric-1", - Host: nil, - Tags: []string{"a", "b", "c"}, - MetricType: DistributionType, - Points: []interface{}{ - []interface{}{float64(1), []interface{}{float64(2)}}, - []interface{}{float64(3), []interface{}{float64(4)}}, - []interface{}{float64(5), []interface{}{float64(6)}}, +func TestLazyLoadAPIKey(t *testing.T) { + tests := map[string]struct { + clientOptions APIClientOptions + expectedAPIKey string + }{ + "decrypt using KMS key": { + clientOptions: APIClientOptions{ + kmsAPIKey: mockEncryptedAPIKey, + decrypter: &mockDecrypter{returnValue: mockDecryptedAPIKey}, }, + expectedAPIKey: mockDecryptedAPIKey, + }, + "fetch from secret ARN": { + clientOptions: APIClientOptions{ + apiKeySecretARN: mockAPIKeySecretARN, + secretFetcher: &mockSecretFetcher{returnValue: mockAPIKey}, + }, + expectedAPIKey: mockAPIKey, }, } - md := mockDecrypter{} - md.returnValue = mockDecryptedAPIKey - - cl := MakeAPIClient(context.Background(), APIClientOptions{baseAPIURL: server.URL, apiKey: "", kmsAPIKey: mockEncryptedAPIKey, decrypter: &md}) - err := cl.SendMetrics(am) - assert.NoError(t, err) - assert.True(t, called) + for name, tt := range tests { + tt := tt // https://go.dev/doc/faq#closures_and_goroutines + t.Run(name, func(t *testing.T) { + t.Parallel() + + called := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + assert.Equal(t, "/distribution_points?api_key="+tt.expectedAPIKey, r.URL.String()) + })) + defer server.Close() + + am := []APIMetric{ + { + Name: "metric-1", + Host: nil, + Tags: []string{"a", "b", "c"}, + MetricType: DistributionType, + Points: []interface{}{ + []interface{}{float64(1), []interface{}{float64(2)}}, + []interface{}{float64(3), []interface{}{float64(4)}}, + []interface{}{float64(5), []interface{}{float64(6)}}, + }, + }, + } + tt.clientOptions.baseAPIURL = server.URL + err := MakeAPIClient(context.Background(), tt.clientOptions).SendMetrics(am) + + assert.NoError(t, err) + assert.True(t, called) + }) + } } From 55930b423dfb35588374b503c0650140441e3128 Mon Sep 17 00:00:00 2001 From: Soshi Katsuta Date: Sun, 13 Mar 2022 23:58:17 +0900 Subject: [PATCH 3/5] Add support for DD_API_KEY_SECRET_ARN environment variable --- ddlambda.go | 16 +++++++++++++--- internal/metrics/listener.go | 3 +++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/ddlambda.go b/ddlambda.go index 1111b7f5..977cf220 100644 --- a/ddlambda.go +++ b/ddlambda.go @@ -33,6 +33,9 @@ type ( APIKey string // KMSAPIKey is your Datadog API key, encrypted using the AWS KMS service. This is used for sending metrics. KMSAPIKey string + // APIKeySecretARN is the ARN of an AWS Secrets Manager secret where your Datadog API key is stored. + // This is used for sending metrics. + APIKeySecretARN string // ShouldRetryOnFailure is used to turn on retry logic when sending metrics via the API. This can negatively effect the performance of your lambda, // and should only be turned on if you can't afford to lose metrics data under poor network conditions. ShouldRetryOnFailure bool @@ -79,6 +82,8 @@ const ( DatadogAPIKeyEnvVar = "DD_API_KEY" // DatadogKMSAPIKeyEnvVar is the environment variable that will be sent to KMS for decryption, then used as an API key. DatadogKMSAPIKeyEnvVar = "DD_KMS_API_KEY" + // DatadogAPIKeySecretARNEnvVar is the environment variable that will be used to retreive the API key from AWS Secrets Manager. + DatadogAPIKeySecretARNEnvVar = "DD_API_KEY_SECRET_ARN" // DatadogSiteEnvVar is the environment variable that will be used as the API host. DatadogSiteEnvVar = "DD_SITE" // LogLevelEnvVar is the environment variable that will be used to set the log level. @@ -234,6 +239,7 @@ func (cfg *Config) toMetricsConfig() metrics.Config { mc.ShouldRetryOnFailure = cfg.ShouldRetryOnFailure mc.APIKey = cfg.APIKey mc.KMSAPIKey = cfg.KMSAPIKey + mc.APIKeySecretARN = cfg.APIKeySecretARN mc.Site = cfg.Site mc.ShouldUseLogForwarder = cfg.ShouldUseLogForwarder mc.HttpClientTimeout = cfg.HttpClientTimeout @@ -258,13 +264,17 @@ func (cfg *Config) toMetricsConfig() metrics.Config { if mc.APIKey == "" { mc.APIKey = os.Getenv(DatadogAPIKeyEnvVar) - } if mc.KMSAPIKey == "" { mc.KMSAPIKey = os.Getenv(DatadogKMSAPIKeyEnvVar) } - if mc.APIKey == "" && mc.KMSAPIKey == "" && !mc.ShouldUseLogForwarder { - logger.Error(fmt.Errorf("couldn't read DD_API_KEY or DD_KMS_API_KEY from environment")) + if mc.APIKeySecretARN == "" { + mc.APIKeySecretARN = os.Getenv(DatadogAPIKeySecretARNEnvVar) + } + if mc.APIKey == "" && mc.KMSAPIKey == "" && mc.APIKeySecretARN == "" && !mc.ShouldUseLogForwarder { + logger.Error(fmt.Errorf( + "couldn't read DD_API_KEY, DD_KMS_API_KEY or DD_API_KEY_SECRET_ARN from environment", + )) } enhancedMetrics := os.Getenv("DD_ENHANCED_METRICS") diff --git a/internal/metrics/listener.go b/internal/metrics/listener.go index eee6942c..647dd8c3 100644 --- a/internal/metrics/listener.go +++ b/internal/metrics/listener.go @@ -40,6 +40,7 @@ type ( Config struct { APIKey string KMSAPIKey string + APIKeySecretARN string Site string ShouldRetryOnFailure bool ShouldUseLogForwarder bool @@ -67,6 +68,8 @@ func MakeListener(config Config, extensionManager *extension.ExtensionManager) L apiKey: config.APIKey, decrypter: MakeKMSDecrypter(), kmsAPIKey: config.KMSAPIKey, + secretFetcher: MakeSecretsManagerSecretFetcher(), + apiKeySecretARN: config.APIKeySecretARN, httpClientTimeout: config.HttpClientTimeout, }) if config.HttpClientTimeout <= 0 { From 7287e24f12fa08cc1d551821298a97fa82efadf8 Mon Sep 17 00:00:00 2001 From: Soshi Katsuta Date: Mon, 14 Mar 2022 00:27:32 +0900 Subject: [PATCH 4/5] Make MakeKMSDecrypter and MakeSecretsManagerSecretFetcher take client.ConfigProvider --- internal/metrics/kms_decrypter.go | 8 ++++---- internal/metrics/listener.go | 7 ++++--- internal/metrics/secret_fetcher.go | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/metrics/kms_decrypter.go b/internal/metrics/kms_decrypter.go index efe98acf..b2a30c9f 100644 --- a/internal/metrics/kms_decrypter.go +++ b/internal/metrics/kms_decrypter.go @@ -13,7 +13,7 @@ import ( "os" "github.com/DataDog/datadog-lambda-go/internal/logger" - "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/aws/client" "github.com/aws/aws-sdk-go/service/kms" "github.com/aws/aws-sdk-go/service/kms/kmsiface" ) @@ -35,10 +35,10 @@ const functionNameEnvVar = "AWS_LAMBDA_FUNCTION_NAME" // encryptionContextKey is the key added to the encryption context by the Lambda console UI const encryptionContextKey = "LambdaFunctionName" -// MakeKMSDecrypter creates a new decrypter which uses the AWS KMS service to decrypt variables -func MakeKMSDecrypter() Decrypter { +// MakeKMSDecrypter creates a new decrypter which uses the AWS KMS service to decrypt variables. +func MakeKMSDecrypter(p client.ConfigProvider) Decrypter { return &kmsDecrypter{ - kmsClient: kms.New(session.New(nil)), + kmsClient: kms.New(p), } } diff --git a/internal/metrics/listener.go b/internal/metrics/listener.go index 647dd8c3..de079663 100644 --- a/internal/metrics/listener.go +++ b/internal/metrics/listener.go @@ -18,6 +18,7 @@ import ( "time" "github.com/aws/aws-lambda-go/lambdacontext" + "github.com/aws/aws-sdk-go/aws/session" "github.com/DataDog/datadog-go/statsd" "github.com/DataDog/datadog-lambda-go/internal/extension" @@ -62,13 +63,13 @@ type ( // MakeListener initializes a new metrics lambda listener func MakeListener(config Config, extensionManager *extension.ExtensionManager) Listener { - + sess := session.Must(session.NewSession()) apiClient := MakeAPIClient(context.Background(), APIClientOptions{ baseAPIURL: config.Site, apiKey: config.APIKey, - decrypter: MakeKMSDecrypter(), + decrypter: MakeKMSDecrypter(sess), kmsAPIKey: config.KMSAPIKey, - secretFetcher: MakeSecretsManagerSecretFetcher(), + secretFetcher: MakeSecretsManagerSecretFetcher(sess), apiKeySecretARN: config.APIKeySecretARN, httpClientTimeout: config.HttpClientTimeout, }) diff --git a/internal/metrics/secret_fetcher.go b/internal/metrics/secret_fetcher.go index 84689b10..525f09ee 100644 --- a/internal/metrics/secret_fetcher.go +++ b/internal/metrics/secret_fetcher.go @@ -9,7 +9,7 @@ import ( "fmt" "github.com/DataDog/datadog-lambda-go/internal/logger" - "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/aws/client" "github.com/aws/aws-sdk-go/service/secretsmanager" "github.com/aws/aws-sdk-go/service/secretsmanager/secretsmanageriface" ) @@ -26,9 +26,9 @@ type secretsManagerSecretFether struct { // MakeSecretsManagerSecretFetcher creates a new SecretFetcher which uses the AWS // Secrets Manager service to fetch a secret. -func MakeSecretsManagerSecretFetcher() SecretFetcher { +func MakeSecretsManagerSecretFetcher(p client.ConfigProvider) SecretFetcher { return &secretsManagerSecretFether{ - client: secretsmanager.New(session.Must(session.NewSession())), + client: secretsmanager.New(p), } } From c7bb9414328dff0d035746d3fde524add79b7cc1 Mon Sep 17 00:00:00 2001 From: Soshi Katsuta Date: Tue, 15 Mar 2022 01:39:04 +0900 Subject: [PATCH 5/5] README: add description of DD_KMS_API_KEY and DD_API_KEY_SECRET_ARN envivorment variables --- README.md | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index fa43ae08..2e287995 100644 --- a/README.md +++ b/README.md @@ -83,11 +83,28 @@ A more complete example can be found in the `ddlambda_example_test.go` file. ### DD_FLUSH_TO_LOG -Set to `true` (recommended) to send custom metrics asynchronously (with no added latency to your Lambda function executions) through CloudWatch Logs with the help of [Datadog Forwarder](https://github.com/DataDog/datadog-serverless-functions/tree/master/aws/logs_monitoring). Defaults to `false`. If set to `false`, you also need to set `DD_API_KEY` and `DD_SITE`. +Set to `true` (recommended) to send custom metrics asynchronously (with no added latency to your Lambda function executions) through CloudWatch Logs with the help of [Datadog Forwarder](https://github.com/DataDog/datadog-serverless-functions/tree/master/aws/logs_monitoring). Defaults to `false`. If set to `false`, you also need to set `DD_SITE` and one of the +following `DD_API_KEY`, `DD_KMS_API_KEY` or `DD_API_KEY_SECRET_ARN`. ### DD_API_KEY -If `DD_FLUSH_TO_LOG` is set to `false` (not recommended), the Datadog API Key must be defined. +If `DD_FLUSH_TO_LOG` is set to `false` (not recommended), the Datadog API key must be +set to `DD_API_KEY`, or either `DD_KMS_API_KEY` or `DD_API_KEY_SECRET_ARN` below must +be defined instead. + +### DD_KMS_API_KEY + +If `DD_FLUSH_TO_LOG` is set to `false` (not recommended) and `DD_API_KEY` is not set, +the Datadog API key encrypted using the AWS Key Management Service (KMS) must be set to +`DD_KMS_API_KEY`, or the following `DD_API_KEY_SECRET_ARN` must be defined instead. +The encryption key used to encrypt the API key must be a symmetric KMS key. + +### DD_API_KEY_SECRET_ARN + +If `DD_FLUSH_TO_LOG` is set to `false` (not recommended) and neither `DD_API_KEY` nor +`DD_KMS_API_KEY` is set, the ARN of an AWS Secrets Manager secret where the Datadog API +key is stored must be set to `DD_API_KEY_SECRET_ARN`. The secret value must be just the +API key string itself (no double quotes), not a JSON object. ### DD_SITE