revamp based on review feedback

grafana · Nov 14, 2024 · d1aa776 · d1aa776
1 parent e48ae89
commit d1aa776
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 104 deletions.
diff --git a/backend/config.go b/backend/config.go
@@ -48,15 +48,11 @@ func WithGrafanaConfig(ctx context.Context, cfg *GrafanaCfg) context.Context {
 	return ctx
 }
 
-// ProxyHashFromContext returns a hash of some of the socks proxy options,
-// if present, for use in datasource instance caching
+// ProxyHashFromContext returns the hash of the client cert contents, set on
+// the incoming GrafanaCfg, for use in datasource instance caching
 func ProxyHashFromContext(ctx context.Context) string {
 	cfg := GrafanaConfigFromContext(ctx)
-	p, err := cfg.proxy()
-	if err != nil {
-		return ""
-	}
-	return p.clientCfg.Hash()
+	return cfg.Get(proxy.PluginSecureSocksProxyClientCertContentsHash)
 }
 
 type GrafanaCfg struct {

diff --git a/backend/config_test.go b/backend/config_test.go
@@ -2,8 +2,6 @@ package backend
 
 import (
 	"context"
-	"crypto/rand"
-	"fmt"
 	"os"
 	"testing"
 
@@ -360,36 +358,3 @@ func TestPluginAppClientSecret(t *testing.T) {
 		require.Equal(t, "client-secret", v)
 	})
 }
-
-func randomProxyConfig() *GrafanaCfg {
-	key := make([]byte, 500)
-	cert := make([]byte, 500)
-	_, _ = rand.Read(key)
-	_, _ = rand.Read(cert)
-	for i := 0; i < 500; i++ {
-		key[i] = 32 + key[i]%64
-		cert[i] = 32 + cert[i]%64
-	}
-	return NewGrafanaCfg(map[string]string{
-		proxy.PluginSecureSocksProxyEnabled:            "true",
-		proxy.PluginSecureSocksProxyClientKeyContents:  string(key),
-		proxy.PluginSecureSocksProxyClientCertContents: string(cert),
-	})
-}
-
-func Benchmark_ClientCfg_Hash(b *testing.B) {
-	count := 0
-	for i := 0; i < b.N; i++ {
-		// randomize in here so we don't get tricked by any low-level caching
-		// This setup takes about half of the benchmark time. We could use
-		// b.StopTimer() / b.StartTimer() but that seems to slow the overall
-		// benchmark down massively.
-		cfg := randomProxyConfig()
-		ctx := WithGrafanaConfig(context.Background(), cfg)
-		hash := ProxyHashFromContext(ctx)
-		if hash[0] == 'a' {
-			count++
-		}
-	}
-	fmt.Printf("This should be about one in 16: %f\n", float64(count)/float64(b.N))
-}
diff --git a/backend/datasource/instance_provider_test.go b/backend/datasource/instance_provider_test.go
@@ -36,9 +36,7 @@ func TestInstanceProvider(t *testing.T) {
 
 	t.Run("When PDC is configured, datasource cache key should include its hash", func(t *testing.T) {
 		cfg := backend.NewGrafanaCfg(map[string]string{
-			proxy.PluginSecureSocksProxyEnabled:            "true",
-			proxy.PluginSecureSocksProxyClientKeyContents:  "clientKey",
-			proxy.PluginSecureSocksProxyClientCertContents: "clientCert",
+			proxy.PluginSecureSocksProxyClientCertContentsHash: "01234568",
 		})
 		ctx := backend.WithGrafanaConfig(context.Background(), cfg)
 		key, err := ip.GetKey(ctx, backend.PluginContext{
@@ -48,41 +46,7 @@ func TestInstanceProvider(t *testing.T) {
 			},
 		})
 		require.NoError(t, err)
-		require.Equal(t, "5##a52015d0f6cbf3f8", key)
-	})
-
-	t.Run("Datasource cache key is different with different proxy settings", func(t *testing.T) {
-		cfg := backend.NewGrafanaCfg(map[string]string{
-			proxy.PluginSecureSocksProxyEnabled:            "true",
-			proxy.PluginSecureSocksProxyClientKeyContents:  "clientKeyTwo",
-			proxy.PluginSecureSocksProxyClientCertContents: "clientCertTwo",
-		})
-		ctx := backend.WithGrafanaConfig(context.Background(), cfg)
-		key, err := ip.GetKey(ctx, backend.PluginContext{
-			DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{
-				ID:       6,
-				JSONData: []byte(`{"enableSecureSocksProxy": true}`),
-			},
-		})
-		require.NoError(t, err)
-		require.Equal(t, "6##831bfc2b12764aa6", key)
-	})
-
-	t.Run("Datasource cache key does not include empty proxy settings", func(t *testing.T) {
-		cfg := backend.NewGrafanaCfg(map[string]string{
-			proxy.PluginSecureSocksProxyEnabled:            "true",
-			proxy.PluginSecureSocksProxyClientKeyContents:  "",
-			proxy.PluginSecureSocksProxyClientCertContents: "",
-		})
-		ctx := backend.WithGrafanaConfig(context.Background(), cfg)
-		key, err := ip.GetKey(ctx, backend.PluginContext{
-			DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{
-				ID:       6,
-				JSONData: []byte(`{"enableSecureSocksProxy": true}`),
-			},
-		})
-		require.NoError(t, err)
-		require.Equal(t, "6##", key)
+		require.Equal(t, "5##01234568", key)
 	})
 
 	t.Run("When both the configuration and updated field of current data source instance settings are equal to the cache, should return false", func(t *testing.T) {

diff --git a/backend/proxy/secure_socks_proxy.go b/backend/proxy/secure_socks_proxy.go
@@ -7,7 +7,6 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
-	"hash/fnv"
 	"net"
 	"net/http"
 	"os"
@@ -22,16 +21,17 @@ import (
 )
 
 const (
-	PluginSecureSocksProxyEnabled            = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED"
-	PluginSecureSocksProxyClientCert         = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT"
-	PluginSecureSocksProxyClientCertContents = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT_VAL"
-	PluginSecureSocksProxyClientKey          = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY"
-	PluginSecureSocksProxyClientKeyContents  = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY_VAL"
-	PluginSecureSocksProxyRootCAs            = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT"
-	PluginSecureSocksProxyRootCAsContents    = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT_VALS"
-	PluginSecureSocksProxyProxyAddress       = "GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS"
-	PluginSecureSocksProxyServerName         = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME"
-	PluginSecureSocksProxyAllowInsecure      = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ALLOW_INSECURE"
+	PluginSecureSocksProxyEnabled                = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED"
+	PluginSecureSocksProxyClientCert             = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT"
+	PluginSecureSocksProxyClientCertContents     = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT_VAL"
+	PluginSecureSocksProxyClientCertContentsHash = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT_HASH"
+	PluginSecureSocksProxyClientKey              = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY"
+	PluginSecureSocksProxyClientKeyContents      = "GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY_VAL"
+	PluginSecureSocksProxyRootCAs                = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT"
+	PluginSecureSocksProxyRootCAsContents        = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT_VALS"
+	PluginSecureSocksProxyProxyAddress           = "GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS"
+	PluginSecureSocksProxyServerName             = "GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME"
+	PluginSecureSocksProxyAllowInsecure          = "GF_SECURE_SOCKS_DATASOURCE_PROXY_ALLOW_INSECURE"
 )
 
 var (
@@ -68,19 +68,6 @@ type ClientCfg struct {
 	AllowInsecure bool
 }
 
-// Hash is used as part of the plugin instance cache key.
-// The ClientKeyVal and ClientCertVal differ between different Hosted Grafana instances,
-// so we include them in the hash key to avoid false positives in `NeedsUpdate`.
-func (c *ClientCfg) Hash() string {
-	if c == nil || c.ClientKeyVal == "" || c.ClientCertVal == "" {
-		return ""
-	}
-	h := fnv.New64a()
-	_, _ = h.Write([]byte(c.ClientKeyVal))
-	_, _ = h.Write([]byte(c.ClientCertVal))
-	return fmt.Sprintf("%x", h.Sum64())
-}
-
 // New creates a new proxy client from a given config.
 func New(opts *Options) Client {
 	return &cfgProxyWrapper{