Merge remote-tracking branch 'grafana/master' into rename-reducer

* grafana/master: CSV: escape quotes in toCSV (grafana#16874) Dashboard: Lazy load out of view panels (grafana#15554) LDAP: Added reload endpoint for LDAP config (grafana#15470) PluginsList: Removed icons and updated snapshots (grafana#16872) Panels: Fixed issue with panel type change and data updates (grafana#16871) Chore: fix modes for non-executable files (grafana#16864)
ryantxu · May 3, 2019 · 1bedc10 · 1bedc10
2 parents 4877128 + 2af69cc
commit 1bedc10
Show file tree

Hide file tree

Showing 28 changed files with 228 additions and 93 deletions.
diff --git a/packages/grafana-ui/src/utils/csv.test.ts b/packages/grafana-ui/src/utils/csv.test.ts
@@ -48,11 +48,13 @@ function norm(csv: string): string {
 
 describe('write csv', () => {
   it('should write the same CSV that we read', () => {
+    const firstRow = [10, 'this "has quotes" inside', true];
     const path = __dirname + '/testdata/roundtrip.csv';
     const csv = fs.readFileSync(path, 'utf8');
     const data = readCSV(csv);
     const out = toCSV(data, { headerStyle: CSVHeaderStyle.full });
     expect(data.length).toBe(1);
+    expect(data[0].rows[0]).toEqual(firstRow);
     expect(data[0].fields.length).toBe(3);
     expect(norm(out)).toBe(norm(csv));
 
@@ -63,6 +65,7 @@ describe('write csv', () => {
     const f = readCSV(shorter);
     const fields = f[0].fields;
     expect(fields.length).toBe(3);
+    expect(f[0].rows[0]).toEqual(firstRow);
     expect(fields.map(f => f.name).join(',')).toEqual('a,b,c'); // the names
   });
 });
diff --git a/packages/grafana-ui/src/utils/csv.ts b/packages/grafana-ui/src/utils/csv.ts
@@ -266,7 +266,7 @@ function writeValue(value: any, config: CSVConfig): string {
   const str = value.toString();
   if (str.includes('"')) {
     // Escape the double quote characters
-    return config.quoteChar + str.replace('"', '""') + config.quoteChar;
+    return config.quoteChar + str.replace(/"/gi, '""') + config.quoteChar;
   }
   if (str.includes('\n') || str.includes(config.delimiter)) {
     return config.quoteChar + str + config.quoteChar;

diff --git a/packages/grafana-ui/src/utils/testdata/roundtrip.csv b/packages/grafana-ui/src/utils/testdata/roundtrip.csv
@@ -1,7 +1,7 @@
 #name#a,b,c
 #type#number,string,boolean
 #unit#ms,,s
-10,AA,true
+10,"this ""has quotes"" inside",true
 20,XX,false
 30,YY,false
 40,ZZ,true

diff --git a/pkg/api/admin_ldap.go b/pkg/api/admin_ldap.go
@@ -0,0 +1,17 @@
+package api
+
+import (
+	"github.com/grafana/grafana/pkg/services/ldap"
+)
+
+func (server *HTTPServer) ReloadLdapCfg() Response {
+	if !ldap.IsEnabled() {
+		return Error(400, "LDAP is not enabled", nil)
+	}
+
+	err := ldap.ReloadConfig()
+	if err != nil {
+		return Error(500, "Failed to reload ldap config.", err)
+	}
+	return Success("Ldap config reloaded")
+}
diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -393,6 +393,7 @@ func (hs *HTTPServer) registerRoutes() {
 		adminRoute.Post("/provisioning/dashboards/reload", Wrap(hs.AdminProvisioningReloadDasboards))
 		adminRoute.Post("/provisioning/datasources/reload", Wrap(hs.AdminProvisioningReloadDatasources))
 		adminRoute.Post("/provisioning/notifications/reload", Wrap(hs.AdminProvisioningReloadNotifications))
+		adminRoute.Post("/ldap/reload", Wrap(hs.ReloadLdapCfg))
 	}, reqGrafanaAdmin)
 
 	// rendering

diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go
@@ -3,20 +3,26 @@ package login
 import (
 	"github.com/grafana/grafana/pkg/models"
 	LDAP "github.com/grafana/grafana/pkg/services/ldap"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 var newLDAP = LDAP.New
-var readLDAPConfig = LDAP.ReadConfig
+var getLDAPConfig = LDAP.GetConfig
 var isLDAPEnabled = LDAP.IsEnabled
 
+// loginUsingLdap logs in user using LDAP. It returns whether LDAP is enabled and optional error and query arg will be
+// populated with the logged in user if successful.
 var loginUsingLdap = func(query *models.LoginUserQuery) (bool, error) {
 	enabled := isLDAPEnabled()
 
 	if !enabled {
 		return false, nil
 	}
 
-	config := readLDAPConfig()
+	config, err := getLDAPConfig()
+	if err != nil {
+		return true, errutil.Wrap("Failed to get LDAP config", err)
+	}
 	if len(config.Servers) == 0 {
 		return true, ErrNoLDAPServers
 	}

diff --git a/pkg/login/ldap_login_test.go b/pkg/login/ldap_login_test.go
@@ -20,12 +20,12 @@ func TestLdapLogin(t *testing.T) {
 
 			ldapLoginScenario("When login", func(sc *ldapLoginScenarioContext) {
 				sc.withLoginResult(false)
-				readLDAPConfig = func() *LDAP.Config {
+				getLDAPConfig = func() (*LDAP.Config, error) {
 					config := &LDAP.Config{
 						Servers: []*LDAP.ServerConfig{},
 					}
 
-					return config
+					return config, nil
 				}
 
 				enabled, err := loginUsingLdap(sc.loginUserQuery)
@@ -129,7 +129,7 @@ func ldapLoginScenario(desc string, fn ldapLoginScenarioFunc) {
 			ldapAuthenticatorMock: mock,
 		}
 
-		readLDAPConfig = func() *LDAP.Config {
+		getLDAPConfig = func() (*LDAP.Config, error) {
 			config := &LDAP.Config{
 				Servers: []*LDAP.ServerConfig{
 					{
@@ -138,7 +138,7 @@ func ldapLoginScenario(desc string, fn ldapLoginScenarioFunc) {
 				},
 			}
 
-			return config
+			return config, nil
 		}
 
 		newLDAP = func(server *LDAP.ServerConfig) LDAP.IAuth {
@@ -147,7 +147,7 @@ func ldapLoginScenario(desc string, fn ldapLoginScenarioFunc) {
 
 		defer func() {
 			newLDAP = LDAP.New
-			readLDAPConfig = LDAP.ReadConfig
+			getLDAPConfig = LDAP.GetConfig
 		}()
 
 		fn(sc)

diff --git a/pkg/middleware/auth_proxy/auth_proxy.go b/pkg/middleware/auth_proxy/auth_proxy.go
@@ -22,8 +22,8 @@ const (
 )
 
 var (
-	readLDAPConfig = ldap.ReadConfig
-	isLDAPEnabled  = ldap.IsEnabled
+	getLDAPConfig = ldap.GetConfig
+	isLDAPEnabled = ldap.IsEnabled
 )
 
 // AuthProxy struct
@@ -219,7 +219,10 @@ func (auth *AuthProxy) GetUserIDViaLDAP() (int64, *Error) {
 		Username:   auth.header,
 	}
 
-	config := readLDAPConfig()
+	config, err := getLDAPConfig()
+	if err != nil {
+		return 0, newError("Failed to get LDAP config", nil)
+	}
 	if len(config.Servers) == 0 {
 		return 0, newError("No LDAP servers available", nil)
 	}

diff --git a/pkg/middleware/auth_proxy/auth_proxy_test.go b/pkg/middleware/auth_proxy/auth_proxy_test.go
@@ -67,18 +67,18 @@ func TestMiddlewareContext(t *testing.T) {
 					return true
 				}
 
-				readLDAPConfig = func() *ldap.Config {
+				getLDAPConfig = func() (*ldap.Config, error) {
 					config := &ldap.Config{
 						Servers: []*ldap.ServerConfig{
 							{},
 						},
 					}
-					return config
+					return config, nil
 				}
 
 				defer func() {
 					isLDAPEnabled = ldap.IsEnabled
-					readLDAPConfig = ldap.ReadConfig
+					getLDAPConfig = ldap.GetConfig
 				}()
 
 				store := remotecache.NewFakeStore(t)
@@ -109,16 +109,16 @@ func TestMiddlewareContext(t *testing.T) {
 					return true
 				}
 
-				readLDAPConfig = func() *ldap.Config {
+				getLDAPConfig = func() (*ldap.Config, error) {
 					config := &ldap.Config{
 						Servers: []*ldap.ServerConfig{},
 					}
-					return config
+					return config, nil
 				}
 
 				defer func() {
 					isLDAPEnabled = ldap.IsEnabled
-					readLDAPConfig = ldap.ReadConfig
+					getLDAPConfig = ldap.GetConfig
 				}()
 
 				store := remotecache.NewFakeStore(t)

diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go
@@ -124,7 +124,7 @@ func (pm *PluginManager) Run(ctx context.Context) error {
 		}
 	}
 
-	// kil backend plugins
+	// kill backend plugins
 	for _, p := range DataSources {
 		p.Kill()
 	}

diff --git a/pkg/services/ldap/settings.go b/pkg/services/ldap/settings.go
@@ -2,9 +2,11 @@ package ldap
 
 import (
 	"fmt"
-	"os"
+	"sync"
 
 	"github.com/BurntSushi/toml"
+	"github.com/grafana/grafana/pkg/util/errutil"
+	"golang.org/x/xerrors"
 
 	"github.com/grafana/grafana/pkg/log"
 	m "github.com/grafana/grafana/pkg/models"
@@ -56,47 +58,72 @@ type GroupToOrgRole struct {
 var config *Config
 var logger = log.New("ldap")
 
+// loadingMutex locks the reading of the config so multiple requests for reloading are sequential.
+var loadingMutex = &sync.Mutex{}
+
 // IsEnabled checks if ldap is enabled
 func IsEnabled() bool {
 	return setting.LdapEnabled
 }
 
-// ReadConfig reads the config if
-// ldap is enabled otherwise it will return nil
-func ReadConfig() *Config {
+// ReloadConfig reads the config from the disc and caches it.
+func ReloadConfig() error {
 	if IsEnabled() == false {
 		return nil
 	}
+	loadingMutex.Lock()
+	defer loadingMutex.Unlock()
+
+	var err error
+	config, err = readConfig(setting.LdapConfigFile)
+	return err
+}
+
+// GetConfig returns the LDAP config if LDAP is enabled otherwise it returns nil. It returns either cached value of
+// the config or it reads it and caches it first.
+func GetConfig() (*Config, error) {
+	if IsEnabled() == false {
+		return nil, nil
+	}
 
 	// Make it a singleton
 	if config != nil {
-		return config
+		return config, nil
 	}
 
-	config = getConfig(setting.LdapConfigFile)
+	loadingMutex.Lock()
+	defer loadingMutex.Unlock()
+
+	var err error
+	config, err = readConfig(setting.LdapConfigFile)
 
-	return config
+	return config, err
 }
-func getConfig(configFile string) *Config {
+
+func readConfig(configFile string) (*Config, error) {
 	result := &Config{}
 
 	logger.Info("Ldap enabled, reading config file", "file", configFile)
 
 	_, err := toml.DecodeFile(configFile, result)
 	if err != nil {
-		logger.Crit("Failed to load ldap config file", "error", err)
-		os.Exit(1)
+		return nil, errutil.Wrap("Failed to load ldap config file", err)
 	}
 
 	if len(result.Servers) == 0 {
-		logger.Crit("ldap enabled but no ldap servers defined in config file")
-		os.Exit(1)
+		return nil, xerrors.New("ldap enabled but no ldap servers defined in config file")
 	}
 
 	// set default org id
 	for _, server := range result.Servers {
-		assertNotEmptyCfg(server.SearchFilter, "search_filter")
-		assertNotEmptyCfg(server.SearchBaseDNs, "search_base_dns")
+		err = assertNotEmptyCfg(server.SearchFilter, "search_filter")
+		if err != nil {
+			return nil, errutil.Wrap("Failed to validate SearchFilter section", err)
+		}
+		err = assertNotEmptyCfg(server.SearchBaseDNs, "search_base_dns")
+		if err != nil {
+			return nil, errutil.Wrap("Failed to validate SearchBaseDNs section", err)
+		}
 
 		for _, groupMap := range server.Groups {
 			if groupMap.OrgId == 0 {
@@ -105,22 +132,21 @@ func getConfig(configFile string) *Config {
 		}
 	}
 
-	return result
+	return result, nil
 }
 
-func assertNotEmptyCfg(val interface{}, propName string) {
+func assertNotEmptyCfg(val interface{}, propName string) error {
 	switch v := val.(type) {
 	case string:
 		if v == "" {
-			logger.Crit("LDAP config file is missing option", "option", propName)
-			os.Exit(1)
+			return xerrors.Errorf("LDAP config file is missing option: %v", propName)
 		}
 	case []string:
 		if len(v) == 0 {
-			logger.Crit("LDAP config file is missing option", "option", propName)
-			os.Exit(1)
+			return xerrors.Errorf("LDAP config file is missing option: %v", propName)
 		}
 	default:
 		fmt.Println("unknown")
 	}
+	return nil
 }
diff --git a/public/app/features/dashboard/components/DashboardSettings/SettingsCtrl.ts b/public/app/features/dashboard/components/DashboardSettings/SettingsCtrl.ts
diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx
@@ -270,6 +270,9 @@ export class DashboardPage extends PureComponent<Props, State> {
       'dashboard-container--has-submenu': dashboard.meta.submenuEnabled,
     });
 
+    // Only trigger render when the scroll has moved by 25
+    const approximateScrollTop = Math.round(scrollTop / 25) * 25;
+
     return (
       <div className={classes}>
         <DashNav
@@ -294,7 +297,12 @@ export class DashboardPage extends PureComponent<Props, State> {
 
             <div className={gridWrapperClasses}>
               {dashboard.meta.submenuEnabled && <SubMenu dashboard={dashboard} />}
-              <DashboardGrid dashboard={dashboard} isEditing={isEditing} isFullscreen={isFullscreen} />
+              <DashboardGrid
+                dashboard={dashboard}
+                isEditing={isEditing}
+                isFullscreen={isFullscreen}
+                scrollTop={approximateScrollTop}
+              />
             </div>
           </CustomScrollbar>
         </div>

diff --git a/public/app/features/dashboard/containers/SoloPanelPage.tsx b/public/app/features/dashboard/containers/SoloPanelPage.tsx
@@ -89,7 +89,7 @@ export class SoloPanelPage extends Component<Props, State> {
 
     return (
       <div className="panel-solo">
-        <DashboardPanel dashboard={dashboard} panel={panel} isEditing={false} isFullscreen={false} />
+        <DashboardPanel dashboard={dashboard} panel={panel} isEditing={false} isFullscreen={false} isInView={true} />
       </div>
     );
   }

diff --git a/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap b/public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap
@@ -211,6 +211,7 @@ exports[`DashboardPage Dashboard init completed  Should render dashboard grid 1`
           }
           isEditing={false}
           isFullscreen={false}
+          scrollTop={0}
         />
       </div>
     </CustomScrollbar>
@@ -540,6 +541,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
           }
           isEditing={false}
           isFullscreen={false}
+          scrollTop={0}
         />
       </div>
     </CustomScrollbar>