From 3fc5f4552ac3f9439c3115999358b563581340eb Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 22 Nov 2019 14:21:23 +0100 Subject: [PATCH] CloudWatch: Fix high CPU load (#20579) * Cache decrypted securejsondata * Models: Add datasource cache tests --- pkg/api/pluginproxy/ds_proxy_test.go | 1 + pkg/models/datasource.go | 2 +- pkg/models/datasource_cache.go | 46 +++++++++++++++++ pkg/models/datasource_cache_test.go | 76 ++++++++++++++++++++++++---- pkg/tsdb/cloudwatch/credentials.go | 13 ++--- 5 files changed, 118 insertions(+), 20 deletions(-) diff --git a/pkg/api/pluginproxy/ds_proxy_test.go b/pkg/api/pluginproxy/ds_proxy_test.go index 2a6cc375dc8..9a562fb40fc 100644 --- a/pkg/api/pluginproxy/ds_proxy_test.go +++ b/pkg/api/pluginproxy/ds_proxy_test.go @@ -463,6 +463,7 @@ func TestDSRouteRule(t *testing.T) { createAuthTest(m.DS_ES, AUTHTYPE_BASIC, AUTHCHECK_HEADER, true), } for _, test := range tests { + m.ClearDSDecryptionCache() runDatasourceAuthTest(test) } }) diff --git a/pkg/models/datasource.go b/pkg/models/datasource.go index 9c8156f2a3a..780ab801042 100644 --- a/pkg/models/datasource.go +++ b/pkg/models/datasource.go @@ -76,7 +76,7 @@ func (ds *DataSource) DecryptedPassword() string { // decryptedValue returns decrypted value from secureJsonData func (ds *DataSource) decryptedValue(field string, fallback string) string { - if value, ok := ds.SecureJsonData.DecryptedValue(field); ok { + if value, ok := ds.DecryptedValue(field); ok { return value } return fallback diff --git a/pkg/models/datasource_cache.go b/pkg/models/datasource_cache.go index 0806de49083..af80d1d50e7 100644 --- a/pkg/models/datasource_cache.go +++ b/pkg/models/datasource_cache.go @@ -162,3 +162,49 @@ func (ds *DataSource) getCustomHeaders() map[string]string { return headers } + +type cachedDecryptedJSON struct { + updated time.Time + json map[string]string +} + +type secureJSONDecryptionCache struct { + cache map[int64]cachedDecryptedJSON + sync.Mutex +} + +var dsDecryptionCache = secureJSONDecryptionCache{ + cache: make(map[int64]cachedDecryptedJSON), +} + +// DecryptedValues returns cached decrypted values from secureJsonData. +func (ds *DataSource) DecryptedValues() map[string]string { + dsDecryptionCache.Lock() + defer dsDecryptionCache.Unlock() + + if item, present := dsDecryptionCache.cache[ds.Id]; present && ds.Updated.Equal(item.updated) { + return item.json + } + + json := ds.SecureJsonData.Decrypt() + dsDecryptionCache.cache[ds.Id] = cachedDecryptedJSON{ + updated: ds.Updated, + json: json, + } + + return json +} + +// DecryptedValue returns cached decrypted value from cached secureJsonData. +func (ds *DataSource) DecryptedValue(key string) (string, bool) { + value, exists := ds.DecryptedValues()[key] + return value, exists +} + +// ClearDSDecryptionCache clears the datasource decryption cache. +func ClearDSDecryptionCache() { + dsDecryptionCache.Lock() + defer dsDecryptionCache.Unlock() + + dsDecryptionCache.cache = make(map[int64]cachedDecryptedJSON) +} diff --git a/pkg/models/datasource_cache_test.go b/pkg/models/datasource_cache_test.go index 7dd80ce96b9..cb13965e894 100644 --- a/pkg/models/datasource_cache_test.go +++ b/pkg/models/datasource_cache_test.go @@ -11,15 +11,16 @@ import ( . "github.com/smartystreets/goconvey/convey" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/components/securejsondata" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) //nolint:goconst -func TestDataSourceCache(t *testing.T) { +func TestDataSourceProxyCache(t *testing.T) { Convey("When caching a datasource proxy", t, func() { - clearCache() + clearDSProxyCache() ds := DataSource{ Id: 1, Url: "http://k8s:8001", @@ -41,13 +42,13 @@ func TestDataSourceCache(t *testing.T) { Convey("Should have no TLS client certificate configured", func() { So(len(t1.transport.TLSClientConfig.Certificates), ShouldEqual, 0) }) - Convey("Should have no user-supplied TLS CA onfigured", func() { + Convey("Should have no user-supplied TLS CA configured", func() { So(t1.transport.TLSClientConfig.RootCAs, ShouldBeNil) }) }) Convey("When caching a datasource proxy then updating it", t, func() { - clearCache() + clearDSProxyCache() setting.SecretKey = "password" json := simplejson.New() @@ -89,7 +90,7 @@ func TestDataSourceCache(t *testing.T) { }) Convey("When caching a datasource proxy with TLS client authentication enabled", t, func() { - clearCache() + clearDSProxyCache() setting.SecretKey = "password" json := simplejson.New() @@ -123,7 +124,7 @@ func TestDataSourceCache(t *testing.T) { }) Convey("When caching a datasource proxy with a user-supplied TLS CA", t, func() { - clearCache() + clearDSProxyCache() setting.SecretKey = "password" json := simplejson.New() @@ -152,7 +153,7 @@ func TestDataSourceCache(t *testing.T) { }) Convey("When caching a datasource proxy when user skips TLS verification", t, func() { - clearCache() + clearDSProxyCache() json := simplejson.New() json.Set("tlsSkipVerify", true) @@ -173,7 +174,7 @@ func TestDataSourceCache(t *testing.T) { }) Convey("When caching a datasource proxy with custom headers specified", t, func() { - clearCache() + clearDSProxyCache() json := simplejson.NewFromAny(map[string]interface{}{ "httpHeaderName1": "Authorization", @@ -236,7 +237,64 @@ func TestDataSourceCache(t *testing.T) { }) } -func clearCache() { +func TestDataSourceDecryptionCache(t *testing.T) { + Convey("When datasource hasn't been updated, encrypted JSON should be fetched from cache", t, func() { + ClearDSDecryptionCache() + + ds := DataSource{ + Id: 1, + Type: DS_INFLUXDB_08, + JsonData: simplejson.New(), + User: "user", + SecureJsonData: securejsondata.GetEncryptedJsonData(map[string]string{ + "password": "password", + }), + } + + // Populate cache + password, ok := ds.DecryptedValue("password") + So(password, ShouldEqual, "password") + So(ok, ShouldBeTrue) + + ds.SecureJsonData = securejsondata.GetEncryptedJsonData(map[string]string{ + "password": "", + }) + + password, ok = ds.DecryptedValue("password") + So(password, ShouldEqual, "password") + So(ok, ShouldBeTrue) + }) + + Convey("When datasource is updated, encrypted JSON should not be fetched from cache", t, func() { + ClearDSDecryptionCache() + + ds := DataSource{ + Id: 1, + Type: DS_INFLUXDB_08, + JsonData: simplejson.New(), + User: "user", + SecureJsonData: securejsondata.GetEncryptedJsonData(map[string]string{ + "password": "password", + }), + } + + // Populate cache + password, ok := ds.DecryptedValue("password") + So(password, ShouldEqual, "password") + So(ok, ShouldBeTrue) + + ds.SecureJsonData = securejsondata.GetEncryptedJsonData(map[string]string{ + "password": "", + }) + ds.Updated = time.Now() + + password, ok = ds.DecryptedValue("password") + So(password, ShouldEqual, "") + So(ok, ShouldBeTrue) + }) +} + +func clearDSProxyCache() { ptc.Lock() defer ptc.Unlock() diff --git a/pkg/tsdb/cloudwatch/credentials.go b/pkg/tsdb/cloudwatch/credentials.go index 414d1995b7b..7d3c7d6c80c 100644 --- a/pkg/tsdb/cloudwatch/credentials.go +++ b/pkg/tsdb/cloudwatch/credentials.go @@ -149,16 +149,9 @@ func (e *CloudWatchExecutor) getDsInfo(region string) *DatasourceInfo { authType := e.DataSource.JsonData.Get("authType").MustString() assumeRoleArn := e.DataSource.JsonData.Get("assumeRoleArn").MustString() - accessKey := "" - secretKey := "" - for key, value := range e.DataSource.SecureJsonData.Decrypt() { - if key == "accessKey" { - accessKey = value - } - if key == "secretKey" { - secretKey = value - } - } + decrypted := e.DataSource.DecryptedValues() + accessKey := decrypted["accessKey"] + secretKey := decrypted["secretKey"] datasourceInfo := &DatasourceInfo{ Region: region,