From c1b8a10f41b1bfdf238602f0eef3b4935883f933 Mon Sep 17 00:00:00 2001 From: Sergey Kostrukov Date: Tue, 18 May 2021 06:36:58 -0700 Subject: [PATCH] Plugins: Fix Azure token provider cache panic and auth param nil value (#34252) * More tests for token cache * Safeguarding from panic and concurrency fixes * Update Azure dependencies * Fix interpolation of empty plugin data --- go.mod | 8 +- go.sum | 13 ++ pkg/api/pluginproxy/ds_auth_provider_test.go | 55 ++++-- pkg/api/pluginproxy/token_cache.go | 30 ++- pkg/api/pluginproxy/token_cache_test.go | 184 ++++++++++++++++++ pkg/api/pluginproxy/token_provider_azure.go | 62 ++++-- pkg/api/pluginproxy/utils.go | 11 +- .../plugin.json | 80 ++++---- 8 files changed, 360 insertions(+), 83 deletions(-) diff --git a/go.mod b/go.mod index e42ffe48e75..8a3a0490474 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ replace k8s.io/client-go => k8s.io/client-go v0.18.8 require ( cloud.google.com/go/storage v1.14.0 cuelang.org/go v0.3.2 - github.com/Azure/azure-sdk-for-go/sdk/azcore v0.14.0 + github.com/Azure/azure-sdk-for-go/sdk/azcore v0.16.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/azidentity v0.8.0 github.com/BurntSushi/toml v0.3.1 github.com/Masterminds/semver v1.5.0 @@ -70,6 +70,7 @@ require ( github.com/mattn/go-sqlite3 v1.14.7 github.com/opentracing/opentracing-go v1.2.0 github.com/patrickmn/go-cache v2.1.0+incompatible + github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4 // indirect github.com/pkg/errors v0.9.1 github.com/prometheus/alertmanager v0.21.1-0.20210511232218-7301451eb94d github.com/prometheus/client_golang v1.10.0 @@ -91,11 +92,12 @@ require ( github.com/xorcare/pointer v1.1.0 github.com/yudai/gojsondiff v1.0.0 go.opentelemetry.io/collector v0.25.0 - golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 + golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a golang.org/x/exp v0.0.0-20210220032938-85be41e4509f // indirect - golang.org/x/net v0.0.0-20210421230115-4e50805a0758 + golang.org/x/net v0.0.0-20210510120150-4163338589ed golang.org/x/oauth2 v0.0.0-20210413134643-5e61552d6c78 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c + golang.org/x/sys v0.0.0-20210514084401-e8d321eab015 // indirect golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba golang.org/x/tools v0.1.0 gonum.org/v1/gonum v0.9.1 diff --git a/go.sum b/go.sum index 68fc0f838d4..9fc3ca2e530 100644 --- a/go.sum +++ b/go.sum @@ -79,10 +79,14 @@ github.com/Azure/azure-sdk-for-go v52.5.0+incompatible h1:/NLBWHCnIHtZyLPc1P7WIq github.com/Azure/azure-sdk-for-go v52.5.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc= github.com/Azure/azure-sdk-for-go/sdk/azcore v0.14.0 h1:4HBTI/9UDZN7tsXyB5TYP3xCv5xVHIUTbvHHH2HFxQY= github.com/Azure/azure-sdk-for-go/sdk/azcore v0.14.0/go.mod h1:pElNP+u99BvCZD+0jOlhI9OC/NB2IDTOTGZOZH0Qhq8= +github.com/Azure/azure-sdk-for-go/sdk/azcore v0.16.0 h1:ZsS7JltN+5D42mcU3Mb4lwVivlFL89v+FlXXMXE2YEM= +github.com/Azure/azure-sdk-for-go/sdk/azcore v0.16.0/go.mod h1:MVdrcUC4Hup35qHym3VdzoW+NBgBxrta9Vei97jRtM8= github.com/Azure/azure-sdk-for-go/sdk/azidentity v0.8.0 h1:wb00szFWtKeIef2Q5X8gdd0mYp8oSHmJOYUh/QXD8sw= github.com/Azure/azure-sdk-for-go/sdk/azidentity v0.8.0/go.mod h1:acANgl9stsT5xflESXKjZx4rhZJSr0TGgTDYY0xJPIE= github.com/Azure/azure-sdk-for-go/sdk/internal v0.5.0 h1:HG1ggl8L3ZkV/Ydanf7lKr5kkhhPGCpWdnr1J6v7cO4= github.com/Azure/azure-sdk-for-go/sdk/internal v0.5.0/go.mod h1:k4KbFSunV/+0hOHL1vyFaPsiYQ1Vmvy1TBpmtvCDLZM= +github.com/Azure/azure-sdk-for-go/sdk/internal v0.5.1 h1:vx8McI56N5oLSQu8xa+xdiE0fjQq8W8Zt49vHP8Rygw= +github.com/Azure/azure-sdk-for-go/sdk/internal v0.5.1/go.mod h1:k4KbFSunV/+0hOHL1vyFaPsiYQ1Vmvy1TBpmtvCDLZM= github.com/Azure/azure-storage-blob-go v0.6.0/go.mod h1:oGfmITT1V6x//CswqY2gtAHND+xIP64/qL7a5QJix0Y= github.com/Azure/azure-storage-blob-go v0.8.0/go.mod h1:lPI3aLPpuLTeUwh1sViKXFxwl2B6teiRqI0deQUvsw0= github.com/Azure/azure-storage-queue-go v0.0.0-20181215014128-6ed74e755687/go.mod h1:K6am8mT+5iFXgingS9LUc7TmbsW6XBw3nxaRyaMyWc8= @@ -1475,6 +1479,8 @@ github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4 github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4 h1:49lOXmGaUpV9Fz3gd7TFZY106KVlPVa5jcYD1gaQf98= github.com/pkg/browser v0.0.0-20180916011732-0a3d74bf9ce4/go.mod h1:4OwLy04Bl9Ef3GJJCoec+30X3LQs/0/m4HFRt/2LUSA= +github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4 h1:Qj1ukM4GlMWXNdMBuXcXfz/Kw9s1qm0CLY32QxuSImI= +github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4/go.mod h1:N6UoU20jOqggOuDwUaBQpluzLNDqif3kq9z2wpdYEfQ= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -1962,6 +1968,8 @@ golang.org/x/crypto v0.0.0-20201208171446-5f87f3452ae9/go.mod h1:jdWPYTVW3xRLrWP golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 h1:/ZScEX8SfEmUGRHs0gxpqteO5nfNW6axyZbBdw9A12g= golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= +golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a h1:kr2P4QFmQr29mSLA43kwrOcgcReGTfbE9N577tCTuBc= +golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a/go.mod h1:P+XmwS30IXTQdn5tA2iutPOUgjI07+tq3H3K9MVA1s8= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -2088,6 +2096,8 @@ golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLd golang.org/x/net v0.0.0-20210324051636-2c4c8ecb7826/go.mod h1:RBQZq4jEuRlivfhVLdyRGr576XBO4/greRjx4P4O3yc= golang.org/x/net v0.0.0-20210421230115-4e50805a0758 h1:aEpZnXcAmXkd6AvLb2OPt+EN1Zu/8Ne3pCqPjja5PXY= golang.org/x/net v0.0.0-20210421230115-4e50805a0758/go.mod h1:72T/g9IO56b78aLF+1Kcs5dz7/ng1VjMUvfKvpfy+jM= +golang.org/x/net v0.0.0-20210510120150-4163338589ed h1:p9UgmWI9wKpfYmgaV/IZKGdXc5qEK45tDwwwDyjS26I= +golang.org/x/net v0.0.0-20210510120150-4163338589ed/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -2235,6 +2245,9 @@ golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210412220455-f1c623a9e750/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210420072515-93ed5bcd2bfe h1:WdX7u8s3yOigWAhHEaDl8r9G+4XwFQEQFtBMYyN+kXQ= golang.org/x/sys v0.0.0-20210420072515-93ed5bcd2bfe/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210514084401-e8d321eab015 h1:hZR0X1kPW+nwyJ9xRxqZk1vx5RUObAPBdKVvXPDUH/E= +golang.org/x/sys v0.0.0-20210514084401-e8d321eab015/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/pkg/api/pluginproxy/ds_auth_provider_test.go b/pkg/api/pluginproxy/ds_auth_provider_test.go index e0f2b0f08cd..1aee6bf5da6 100644 --- a/pkg/api/pluginproxy/ds_auth_provider_test.go +++ b/pkg/api/pluginproxy/ds_auth_provider_test.go @@ -9,25 +9,20 @@ import ( ) func TestApplyRoute_interpolateAuthParams(t *testing.T) { - pluginRoute := &plugins.AppPluginRoute{ - Path: "pathwithjwttoken1", - URL: "https://api.jwt.io/some/path", - Method: "GET", - TokenAuth: &plugins.JwtTokenAuth{ - Url: "https://login.server.com/{{.JsonData.tenantId}}/oauth2/token", - Scopes: []string{ - "https://www.testapi.com/auth/Read.All", - "https://www.testapi.com/auth/Write.All", - }, - Params: map[string]string{ - "token_uri": "{{.JsonData.tokenUri}}", - "client_email": "{{.JsonData.clientEmail}}", - "private_key": "{{.SecureJsonData.privateKey}}", - }, + tokenAuth := &plugins.JwtTokenAuth{ + Url: "https://login.server.com/{{.JsonData.tenantId}}/oauth2/token", + Scopes: []string{ + "https://www.testapi.com/auth/Read.All", + "https://www.testapi.com/auth/Write.All", + }, + Params: map[string]string{ + "token_uri": "{{.JsonData.tokenUri}}", + "client_email": "{{.JsonData.clientEmail | orEmpty}}", + "private_key": "{{.SecureJsonData.privateKey | orEmpty}}", }, } - templateData := templateData{ + validData := templateData{ JsonData: map[string]interface{}{ "clientEmail": "test@test.com", "tokenUri": "login.url.com/token", @@ -38,8 +33,13 @@ func TestApplyRoute_interpolateAuthParams(t *testing.T) { }, } + emptyData := templateData{ + JsonData: map[string]interface{}{}, + SecureJsonData: map[string]string{}, + } + t.Run("should interpolate JwtTokenAuth struct using given JsonData", func(t *testing.T) { - interpolated, err := interpolateAuthParams(pluginRoute.TokenAuth, templateData) + interpolated, err := interpolateAuthParams(tokenAuth, validData) require.NoError(t, err) require.NotNil(t, interpolated) @@ -55,8 +55,27 @@ func TestApplyRoute_interpolateAuthParams(t *testing.T) { }) t.Run("should return Nil if given JwtTokenAuth is Nil", func(t *testing.T) { - interpolated, err := interpolateAuthParams(pluginRoute.JwtTokenAuth, templateData) + interpolated, err := interpolateAuthParams(nil, validData) require.NoError(t, err) require.Nil(t, interpolated) }) + + t.Run("when plugin data is empty", func(t *testing.T) { + interpolated, err := interpolateAuthParams(tokenAuth, emptyData) + require.NoError(t, err) + require.NotNil(t, interpolated) + + t.Run("template expressions in url should resolve to ", func(t *testing.T) { + assert.Equal(t, "https://login.server.com//oauth2/token", interpolated.Url) + }) + + t.Run("template expressions in params resolve to ", func(t *testing.T) { + assert.Equal(t, "", interpolated.Params["token_uri"]) + }) + + t.Run("template expressions with orEmpty should resolve to empty string", func(t *testing.T) { + assert.Equal(t, "", interpolated.Params["client_email"]) + assert.Equal(t, "", interpolated.Params["private_key"]) + }) + }) } diff --git a/pkg/api/pluginproxy/token_cache.go b/pkg/api/pluginproxy/token_cache.go index 45b79a02332..91a3d92d361 100644 --- a/pkg/api/pluginproxy/token_cache.go +++ b/pkg/api/pluginproxy/token_cache.go @@ -97,22 +97,38 @@ func (c *scopesCacheEntry) getAccessToken(ctx context.Context) (string, error) { c.cond.L.Unlock() if shouldRefresh { - accessToken, err = c.credential.GetAccessToken(ctx, c.scopes) + accessToken, err = c.refreshAccessToken(ctx) + if err != nil { + return "", err + } + } + + return accessToken.Token, nil +} + +func (c *scopesCacheEntry) refreshAccessToken(ctx context.Context) (*AccessToken, error) { + var accessToken *AccessToken + // Safeguarding from panic caused by credential implementation + defer func() { c.cond.L.Lock() c.refreshing = false - c.accessToken = accessToken + + if accessToken != nil { + c.accessToken = accessToken + } c.cond.Broadcast() c.cond.L.Unlock() + }() - if err != nil { - return "", err - } + token, err := c.credential.GetAccessToken(ctx, c.scopes) + if err != nil { + return nil, err } - - return accessToken.Token, nil + accessToken = token + return accessToken, nil } func getKeyForScopes(scopes []string) string { diff --git a/pkg/api/pluginproxy/token_cache_test.go b/pkg/api/pluginproxy/token_cache_test.go index 549548423bc..904e64d39b8 100644 --- a/pkg/api/pluginproxy/token_cache_test.go +++ b/pkg/api/pluginproxy/token_cache_test.go @@ -2,7 +2,9 @@ package pluginproxy import ( "context" + "errors" "fmt" + "sync" "testing" "time" @@ -100,3 +102,185 @@ func TestConcurrentTokenCache_GetAccessToken(t *testing.T) { assert.Equal(t, 1, credential2.calledTimes) }) } + +func TestScopesCacheEntry_GetAccessToken(t *testing.T) { + ctx := context.Background() + + scopes := []string{"Scope1"} + + t.Run("when credential returns error", func(t *testing.T) { + credential := &fakeCredential{ + getAccessTokenFunc: func(ctx context.Context, scopes []string) (*AccessToken, error) { + invalidToken := &AccessToken{Token: "invalid_token", ExpiresOn: timeNow().Add(time.Hour)} + return invalidToken, errors.New("unable to get access token") + }, + } + + t.Run("should return error", func(t *testing.T) { + cacheEntry := &scopesCacheEntry{ + credential: credential, + scopes: scopes, + cond: sync.NewCond(&sync.Mutex{}), + } + + accessToken, err := cacheEntry.getAccessToken(ctx) + + assert.Error(t, err) + assert.Equal(t, "", accessToken) + }) + + t.Run("should call credential again each time and return error", func(t *testing.T) { + credential.calledTimes = 0 + + cacheEntry := &scopesCacheEntry{ + credential: credential, + scopes: scopes, + cond: sync.NewCond(&sync.Mutex{}), + } + + var err error + _, err = cacheEntry.getAccessToken(ctx) + assert.Error(t, err) + + _, err = cacheEntry.getAccessToken(ctx) + assert.Error(t, err) + + _, err = cacheEntry.getAccessToken(ctx) + assert.Error(t, err) + + assert.Equal(t, 3, credential.calledTimes) + }) + }) + + t.Run("when credential returns error only once", func(t *testing.T) { + var times = 0 + credential := &fakeCredential{ + getAccessTokenFunc: func(ctx context.Context, scopes []string) (*AccessToken, error) { + times = times + 1 + if times == 1 { + invalidToken := &AccessToken{Token: "invalid_token", ExpiresOn: timeNow().Add(time.Hour)} + return invalidToken, errors.New("unable to get access token") + } + fakeAccessToken := &AccessToken{Token: fmt.Sprintf("token-%v", times), ExpiresOn: timeNow().Add(time.Hour)} + return fakeAccessToken, nil + }, + } + + t.Run("should call credential again only while it returns error", func(t *testing.T) { + cacheEntry := &scopesCacheEntry{ + credential: credential, + scopes: scopes, + cond: sync.NewCond(&sync.Mutex{}), + } + + var accessToken string + var err error + + _, err = cacheEntry.getAccessToken(ctx) + assert.Error(t, err) + + accessToken, err = cacheEntry.getAccessToken(ctx) + assert.NoError(t, err) + assert.Equal(t, "token-2", accessToken) + + accessToken, err = cacheEntry.getAccessToken(ctx) + assert.NoError(t, err) + assert.Equal(t, "token-2", accessToken) + + assert.Equal(t, 2, credential.calledTimes) + }) + }) + + t.Run("when credential panics", func(t *testing.T) { + credential := &fakeCredential{ + getAccessTokenFunc: func(ctx context.Context, scopes []string) (*AccessToken, error) { + panic(errors.New("unable to get access token")) + }, + } + + t.Run("should call credential again each time", func(t *testing.T) { + credential.calledTimes = 0 + + cacheEntry := &scopesCacheEntry{ + credential: credential, + scopes: scopes, + cond: sync.NewCond(&sync.Mutex{}), + } + + func() { + defer func() { + assert.NotNil(t, recover(), "credential expected to panic") + }() + _, _ = cacheEntry.getAccessToken(ctx) + }() + + func() { + defer func() { + assert.NotNil(t, recover(), "credential expected to panic") + }() + _, _ = cacheEntry.getAccessToken(ctx) + }() + + func() { + defer func() { + assert.NotNil(t, recover(), "credential expected to panic") + }() + _, _ = cacheEntry.getAccessToken(ctx) + }() + + assert.Equal(t, 3, credential.calledTimes) + }) + }) + + t.Run("when credential panics only once", func(t *testing.T) { + var times = 0 + credential := &fakeCredential{ + getAccessTokenFunc: func(ctx context.Context, scopes []string) (*AccessToken, error) { + times = times + 1 + if times == 1 { + panic(errors.New("unable to get access token")) + } + fakeAccessToken := &AccessToken{Token: fmt.Sprintf("token-%v", times), ExpiresOn: timeNow().Add(time.Hour)} + return fakeAccessToken, nil + }, + } + + t.Run("should call credential again only while it panics", func(t *testing.T) { + cacheEntry := &scopesCacheEntry{ + credential: credential, + scopes: scopes, + cond: sync.NewCond(&sync.Mutex{}), + } + + var accessToken string + var err error + + func() { + defer func() { + assert.NotNil(t, recover(), "credential expected to panic") + }() + _, _ = cacheEntry.getAccessToken(ctx) + }() + + func() { + defer func() { + assert.Nil(t, recover(), "credential not expected to panic") + }() + accessToken, err = cacheEntry.getAccessToken(ctx) + assert.NoError(t, err) + assert.Equal(t, "token-2", accessToken) + }() + + func() { + defer func() { + assert.Nil(t, recover(), "credential not expected to panic") + }() + accessToken, err = cacheEntry.getAccessToken(ctx) + assert.NoError(t, err) + assert.Equal(t, "token-2", accessToken) + }() + + assert.Equal(t, 2, credential.calledTimes) + }) + }) +} diff --git a/pkg/api/pluginproxy/token_provider_azure.go b/pkg/api/pluginproxy/token_provider_azure.go index 143e5919c03..a72d9355b52 100644 --- a/pkg/api/pluginproxy/token_provider_azure.go +++ b/pkg/api/pluginproxy/token_provider_azure.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" "strings" + "sync" + "sync/atomic" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -44,7 +46,7 @@ func (provider *azureAccessTokenProvider) getAccessToken() (string, error) { if provider.isManagedIdentityCredential() { if !provider.cfg.Azure.ManagedIdentityEnabled { - err := fmt.Errorf("managed identity authentication not enabled in Grafana config") + err := fmt.Errorf("managed identity authentication is not enabled in Grafana config") return "", err } else { credential = provider.getManagedIdentityCredential() @@ -106,8 +108,9 @@ func (provider *azureAccessTokenProvider) resolveAuthorityHost(cloudName string) } type managedIdentityCredential struct { - clientId string - credential azcore.TokenCredential + clientId string + credLock sync.Mutex + credValue atomic.Value // of azcore.TokenCredential } func (c *managedIdentityCredential) GetCacheKey() string { @@ -118,14 +121,29 @@ func (c *managedIdentityCredential) GetCacheKey() string { return fmt.Sprintf("azure|msi|%s", clientId) } -func (c *managedIdentityCredential) GetAccessToken(ctx context.Context, scopes []string) (*AccessToken, error) { - // No need to lock here because the caller is responsible for thread safety - if c.credential == nil { +func (c *managedIdentityCredential) getCredential() (azcore.TokenCredential, error) { + credential := c.credValue.Load() + + if credential == nil { + c.credLock.Lock() + defer c.credLock.Unlock() + var err error - c.credential, err = azidentity.NewManagedIdentityCredential(c.clientId, nil) + credential, err = azidentity.NewManagedIdentityCredential(c.clientId, nil) if err != nil { return nil, err } + + c.credValue.Store(credential) + } + + return credential.(azcore.TokenCredential), nil +} + +func (c *managedIdentityCredential) GetAccessToken(ctx context.Context, scopes []string) (*AccessToken, error) { + credential, err := c.getCredential() + if err != nil { + return nil, err } // Implementation of ManagedIdentityCredential doesn't support scopes, converting to resource @@ -135,7 +153,7 @@ func (c *managedIdentityCredential) GetAccessToken(ctx context.Context, scopes [ resource := strings.TrimSuffix(scopes[0], "/.default") scopes = []string{resource} - accessToken, err := c.credential.GetToken(ctx, azcore.TokenRequestOptions{Scopes: scopes}) + accessToken, err := credential.GetToken(ctx, azcore.TokenRequestOptions{Scopes: scopes}) if err != nil { return nil, err } @@ -148,24 +166,40 @@ type clientSecretCredential struct { tenantId string clientId string clientSecret string - credential azcore.TokenCredential + credLock sync.Mutex + credValue atomic.Value // of azcore.TokenCredential } func (c *clientSecretCredential) GetCacheKey() string { return fmt.Sprintf("azure|clientsecret|%s|%s|%s|%s", c.authority, c.tenantId, c.clientId, hashSecret(c.clientSecret)) } -func (c *clientSecretCredential) GetAccessToken(ctx context.Context, scopes []string) (*AccessToken, error) { - // No need to lock here because the caller is responsible for thread safety - if c.credential == nil { +func (c *clientSecretCredential) getCredential() (azcore.TokenCredential, error) { + credential := c.credValue.Load() + + if credential == nil { + c.credLock.Lock() + defer c.credLock.Unlock() + var err error - c.credential, err = azidentity.NewClientSecretCredential(c.tenantId, c.clientId, c.clientSecret, nil) + credential, err = azidentity.NewClientSecretCredential(c.tenantId, c.clientId, c.clientSecret, nil) if err != nil { return nil, err } + + c.credValue.Store(credential) + } + + return credential.(azcore.TokenCredential), nil +} + +func (c *clientSecretCredential) GetAccessToken(ctx context.Context, scopes []string) (*AccessToken, error) { + credential, err := c.getCredential() + if err != nil { + return nil, err } - accessToken, err := c.credential.GetToken(ctx, azcore.TokenRequestOptions{Scopes: scopes}) + accessToken, err := credential.GetToken(ctx, azcore.TokenRequestOptions{Scopes: scopes}) if err != nil { return nil, err } diff --git a/pkg/api/pluginproxy/utils.go b/pkg/api/pluginproxy/utils.go index a2f10dca434..bea476e2c7e 100644 --- a/pkg/api/pluginproxy/utils.go +++ b/pkg/api/pluginproxy/utils.go @@ -14,7 +14,16 @@ import ( // interpolateString accepts template data and return a string with substitutions func interpolateString(text string, data templateData) (string, error) { - t, err := template.New("content").Parse(text) + extraFuncs := map[string]interface{}{ + "orEmpty": func(v interface{}) interface{} { + if v == nil { + return "" + } + return v + }, + } + + t, err := template.New("content").Funcs(extraFuncs).Parse(text) if err != nil { return "", fmt.Errorf("could not parse template %s", text) } diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/plugin.json b/public/app/plugins/datasource/grafana-azure-monitor-datasource/plugin.json index b4244e62039..fb40c88f53f 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/plugin.json +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/plugin.json @@ -37,11 +37,11 @@ "tokenAuth": { "scopes": ["https://management.azure.com/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureCloud", - "tenant_id": "{{.JsonData.tenantId}}", - "client_id": "{{.JsonData.clientId}}", - "client_secret": "{{.SecureJsonData.clientSecret}}" + "tenant_id": "{{.JsonData.tenantId | orEmpty}}", + "client_id": "{{.JsonData.clientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.clientSecret | orEmpty}}" } }, "headers": [{ "name": "x-ms-app", "content": "Grafana" }] @@ -54,11 +54,11 @@ "tokenAuth": { "scopes": ["https://management.usgovcloudapi.net/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureUSGovernment", - "tenant_id": "{{.JsonData.tenantId}}", - "client_id": "{{.JsonData.clientId}}", - "client_secret": "{{.SecureJsonData.clientSecret}}" + "tenant_id": "{{.JsonData.tenantId | orEmpty}}", + "client_id": "{{.JsonData.clientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.clientSecret | orEmpty}}" } }, "headers": [{ "name": "x-ms-app", "content": "Grafana" }] @@ -71,11 +71,11 @@ "tokenAuth": { "scopes": ["https://management.microsoftazure.de/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureGermanCloud", - "tenant_id": "{{.JsonData.tenantId}}", - "client_id": "{{.JsonData.clientId}}", - "client_secret": "{{.SecureJsonData.clientSecret}}" + "tenant_id": "{{.JsonData.tenantId | orEmpty}}", + "client_id": "{{.JsonData.clientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.clientSecret | orEmpty}}" } }, "headers": [{ "name": "x-ms-app", "content": "Grafana" }] @@ -88,11 +88,11 @@ "tokenAuth": { "scopes": ["https://management.chinacloudapi.cn/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureChinaCloud", - "tenant_id": "{{.JsonData.tenantId}}", - "client_id": "{{.JsonData.clientId}}", - "client_secret": "{{.SecureJsonData.clientSecret}}" + "tenant_id": "{{.JsonData.tenantId | orEmpty}}", + "client_id": "{{.JsonData.clientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.clientSecret | orEmpty}}" } }, "headers": [{ "name": "x-ms-app", "content": "Grafana" }] @@ -123,11 +123,11 @@ "tokenAuth": { "scopes": ["https://management.azure.com/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureCloud", - "tenant_id": "{{.JsonData.logAnalyticsTenantId}}", - "client_id": "{{.JsonData.logAnalyticsClientId}}", - "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret}}" + "tenant_id": "{{.JsonData.logAnalyticsTenantId | orEmpty}}", + "client_id": "{{.JsonData.logAnalyticsClientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret | orEmpty}}" } }, "headers": [{ "name": "x-ms-app", "content": "Grafana" }] @@ -140,11 +140,11 @@ "tokenAuth": { "scopes": ["https://management.chinacloudapi.cn/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureChinaCloud", - "tenant_id": "{{.JsonData.logAnalyticsTenantId}}", - "client_id": "{{.JsonData.logAnalyticsClientId}}", - "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret}}" + "tenant_id": "{{.JsonData.logAnalyticsTenantId | orEmpty}}", + "client_id": "{{.JsonData.logAnalyticsClientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret | orEmpty}}" } }, "headers": [{ "name": "x-ms-app", "content": "Grafana" }] @@ -157,11 +157,11 @@ "tokenAuth": { "scopes": ["https://management.usgovcloudapi.net/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureUSGovernment", - "tenant_id": "{{.JsonData.logAnalyticsTenantId}}", - "client_id": "{{.JsonData.logAnalyticsClientId}}", - "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret}}" + "tenant_id": "{{.JsonData.logAnalyticsTenantId | orEmpty}}", + "client_id": "{{.JsonData.logAnalyticsClientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret | orEmpty}}" } }, "headers": [{ "name": "x-ms-app", "content": "Grafana" }] @@ -174,11 +174,11 @@ "tokenAuth": { "scopes": ["https://api.loganalytics.io/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureCloud", - "tenant_id": "{{.JsonData.logAnalyticsTenantId}}", - "client_id": "{{.JsonData.logAnalyticsClientId}}", - "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret}}" + "tenant_id": "{{.JsonData.logAnalyticsTenantId | orEmpty}}", + "client_id": "{{.JsonData.logAnalyticsClientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret | orEmpty}}" } }, "headers": [ @@ -194,11 +194,11 @@ "tokenAuth": { "scopes": ["https://api.loganalytics.azure.cn/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureChinaCloud", - "tenant_id": "{{.JsonData.logAnalyticsTenantId}}", - "client_id": "{{.JsonData.logAnalyticsClientId}}", - "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret}}" + "tenant_id": "{{.JsonData.logAnalyticsTenantId | orEmpty}}", + "client_id": "{{.JsonData.logAnalyticsClientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret | orEmpty}}" } }, "headers": [ @@ -214,11 +214,11 @@ "tokenAuth": { "scopes": ["https://api.loganalytics.us/.default"], "params": { - "azure_auth_type": "{{.JsonData.azureAuthType}}", + "azure_auth_type": "{{.JsonData.azureAuthType | orEmpty}}", "azure_cloud": "AzureUSGovernment", - "tenant_id": "{{.JsonData.logAnalyticsTenantId}}", - "client_id": "{{.JsonData.logAnalyticsClientId}}", - "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret}}" + "tenant_id": "{{.JsonData.logAnalyticsTenantId | orEmpty}}", + "client_id": "{{.JsonData.logAnalyticsClientId | orEmpty}}", + "client_secret": "{{.SecureJsonData.logAnalyticsClientSecret | orEmpty}}" } }, "headers": [