From d433e212759809e539c1358bdd7febc2f1ff45ad 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 | 74 +++++++++++++++++++++++++--- pkg/tsdb/cloudwatch/credentials.go | 13 ++--- 5 files changed, 117 insertions(+), 19 deletions(-) diff --git a/pkg/api/pluginproxy/ds_proxy_test.go b/pkg/api/pluginproxy/ds_proxy_test.go index 3a07ec860e0c5..cd97d6935bac8 100644 --- a/pkg/api/pluginproxy/ds_proxy_test.go +++ b/pkg/api/pluginproxy/ds_proxy_test.go @@ -495,6 +495,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 6df4dcb34573f..2702d8968db42 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 c1c29c2c834b9..661f5f2cddb00 100644 --- a/pkg/models/datasource_cache.go +++ b/pkg/models/datasource_cache.go @@ -110,3 +110,49 @@ func (ds *DataSource) GetTLSConfig() (*tls.Config, error) { return tlsConfig, nil } + +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 c3ce8f27e5179..595a9fed534ab 100644 --- a/pkg/models/datasource_cache_test.go +++ b/pkg/models/datasource_cache_test.go @@ -6,15 +6,16 @@ import ( . "github.com/smartystreets/goconvey/convey" + "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", @@ -36,13 +37,13 @@ func TestDataSourceCache(t *testing.T) { Convey("Should have no TLS client certificate configured", func() { So(len(t1.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.TLSClientConfig.RootCAs, ShouldBeNil) }) }) Convey("When caching a datasource proxy then updating it", t, func() { - clearCache() + clearDSProxyCache() setting.SecretKey = "password" json := simplejson.New() @@ -84,7 +85,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() @@ -118,7 +119,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() @@ -147,7 +148,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) @@ -168,7 +169,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 fb92c827f7c32..b9f2c1e620de9 100644 --- a/pkg/tsdb/cloudwatch/credentials.go +++ b/pkg/tsdb/cloudwatch/credentials.go @@ -147,16 +147,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,