Plugins: Make sure feature toggles config value is deterministic (#75249)

* make sure feature toggle config vals are deterministic

* fix nil pointer

* undo changes

* fix condition

* add tests

pkg/plugins/envvars/envvars.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -91,10 +92,10 @@ func (s *Service) GetConfigMap(ctx context.Context, _ string, _ *oauth.ExternalS
 			for feat := range enabledFeatures {
 				features = append(features, feat)
 			}
+			sort.Strings(features)
 			m[featuretoggles.EnabledFeatures] = strings.Join(features, ",")
 		}
 	}
-
 	// TODO add support via plugin SDK
 	//if s.cfg.AWSAssumeRoleEnabled {
 	//	m[awsds.AssumeRoleEnabledEnvVarKeyName] = "true"
@@ -181,16 +182,17 @@ func (s *Service) tracingEnvVars(plugin *plugins.Plugin) []string {
 func (s *Service) featureToggleEnableVar(ctx context.Context) []string {
 	var variables []string // an array is used for consistency and keep the logic simpler for no features case
 
-	if s.cfg.Features != nil {
+	if s.cfg.Features == nil {
-		enabledFeatures := s.cfg.Features.GetEnabled(ctx)
+		return variables
+	}
 
-		if len(enabledFeatures) > 0 {
+	enabledFeatures := s.cfg.Features.GetEnabled(ctx)
-			features := make([]string, 0, len(enabledFeatures))
+	if len(enabledFeatures) > 0 {
-			for feat := range enabledFeatures {
+		features := make([]string, 0, len(enabledFeatures))
-				features = append(features, feat)
+		for feat := range enabledFeatures {
-			}
+			features = append(features, feat)
-			variables = append(variables, fmt.Sprintf("GF_INSTANCE_FEATURE_TOGGLES_ENABLE=%s", strings.Join(features, ",")))
 		}
+		variables = append(variables, fmt.Sprintf("GF_INSTANCE_FEATURE_TOGGLES_ENABLE=%s", strings.Join(features, ",")))
 	}
 
 	return variables

pkg/plugins/envvars/envvars_test.go

@@ -379,3 +379,125 @@ func TestInitializer_featureToggleEnvVar(t *testing.T) {
 		}
 	})
 }
+
+func TestService_GetConfigMap(t *testing.T) {
+	tcs := []struct {
+		name     string
+		cfg      *config.Cfg
+		expected map[string]string
+	}{
+		{
+			name: "Both features and proxy settings enabled",
+			cfg: &config.Cfg{
+				Features: featuremgmt.WithFeatures("feat-2", "feat-500", "feat-1"),
+				ProxySettings: setting.SecureSocksDSProxySettings{
+					Enabled:      true,
+					ShowUI:       true,
+					ClientCert:   "c3rt",
+					ClientKey:    "k3y",
+					RootCA:       "ca",
+					ProxyAddress: "https://proxy.grafana.com",
+					ServerName:   "secureProxy",
+				},
+			},
+			expected: map[string]string{
+				"GF_INSTANCE_FEATURE_TOGGLES_ENABLE":              "feat-1,feat-2,feat-500",
+				"GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_ENABLED": "true",
+				"GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_CERT":    "c3rt",
+				"GF_SECURE_SOCKS_DATASOURCE_PROXY_CLIENT_KEY":     "k3y",
+				"GF_SECURE_SOCKS_DATASOURCE_PROXY_ROOT_CA_CERT":   "ca",
+				"GF_SECURE_SOCKS_DATASOURCE_PROXY_PROXY_ADDRESS":  "https://proxy.grafana.com",
+				"GF_SECURE_SOCKS_DATASOURCE_PROXY_SERVER_NAME":    "secureProxy",
+			},
+		},
+		{
+			name: "Features enabled but proxy settings disabled",
+			cfg: &config.Cfg{
+				Features: featuremgmt.WithFeatures("feat-2", "feat-500", "feat-1"),
+				ProxySettings: setting.SecureSocksDSProxySettings{
+					Enabled:      false,
+					ShowUI:       true,
+					ClientCert:   "c3rt",
+					ClientKey:    "k3y",
+					RootCA:       "ca",
+					ProxyAddress: "https://proxy.grafana.com",
+					ServerName:   "secureProxy",
+				},
+			},
+			expected: map[string]string{
+				"GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "feat-1,feat-2,feat-500",
+			},
+		},
+		{
+			name: "Both features and proxy settings disabled",
+			cfg: &config.Cfg{
+				Features: featuremgmt.WithFeatures("feat-2", false),
+				ProxySettings: setting.SecureSocksDSProxySettings{
+					Enabled:      false,
+					ShowUI:       true,
+					ClientCert:   "c3rt",
+					ClientKey:    "k3y",
+					RootCA:       "ca",
+					ProxyAddress: "https://proxy.grafana.com",
+					ServerName:   "secureProxy",
+				},
+			},
+			expected: map[string]string{},
+		},
+		{
+			name: "Both features and proxy settings empty",
+			cfg: &config.Cfg{
+				Features:      nil,
+				ProxySettings: setting.SecureSocksDSProxySettings{},
+			},
+			expected: map[string]string{},
+		},
+	}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			s := &Service{
+				cfg: tc.cfg,
+			}
+			require.Equal(t, tc.expected, s.GetConfigMap(context.Background(), "", nil))
+		})
+	}
+}
+
+func TestService_GetConfigMap_featureToggles(t *testing.T) {
+	t.Run("Feature toggles list is deterministic", func(t *testing.T) {
+		tcs := []struct {
+			enabledFeatures []string
+			expectedConfig  map[string]string
+		}{
+			{
+				enabledFeatures: nil,
+				expectedConfig:  map[string]string{},
+			},
+			{
+				enabledFeatures: []string{},
+				expectedConfig:  map[string]string{},
+			},
+			{
+				enabledFeatures: []string{"A", "B", "C"},
+				expectedConfig:  map[string]string{"GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "A,B,C"},
+			},
+			{
+				enabledFeatures: []string{"C", "B", "A"},
+				expectedConfig:  map[string]string{"GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "A,B,C"},
+			},
+			{
+				enabledFeatures: []string{"b", "a", "c", "d"},
+				expectedConfig:  map[string]string{"GF_INSTANCE_FEATURE_TOGGLES_ENABLE": "a,b,c,d"},
+			},
+		}
+
+		for _, tc := range tcs {
+			s := &Service{
+				cfg: &config.Cfg{
+					Features: fakes.NewFakeFeatureToggles(tc.enabledFeatures...),
+				},
+			}
+			require.Equal(t, tc.expectedConfig, s.GetConfigMap(context.Background(), "", nil))
+		}
+	})
+}