From 2c72f8babc04cd6af7e63dae8220337cfbfff881 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 30 Mar 2021 10:54:57 +0200 Subject: [PATCH] MSSQL: By default let driver choose port (#32417) * MSSQL: By default let driver choose port Signed-off-by: Arve Knudsen --- docs/sources/datasources/mssql.md | 2 +- pkg/tsdb/mssql/mssql.go | 35 ++++++---- pkg/tsdb/mssql/mssql_test.go | 68 +++++++++++++++++-- .../datasource/mssql/partials/config.html | 2 +- 4 files changed, 88 insertions(+), 19 deletions(-) diff --git a/docs/sources/datasources/mssql.md b/docs/sources/datasources/mssql.md index 9c12cc13275..c4b64455466 100644 --- a/docs/sources/datasources/mssql.md +++ b/docs/sources/datasources/mssql.md @@ -16,7 +16,7 @@ Grafana ships with a built-in Microsoft SQL Server (MS SQL) data source plugin t | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------- | | `Name` | The data source name. This is how you refer to the data source in panels and queries. | | `Default` | Default data source means that it will be pre-selected for new panels. | -| `Host` | The IP address/hostname and optional port of your MS SQL instance. If port is omitted, default 1433 will be used. | +| `Host` | The IP address/hostname and optional port of your MS SQL instance. If the port is omitted, the driver default will be used (0). | | `Database` | Name of your MS SQL database. | | `Authentication` | Authentication mode. Either using SQL Server Authentication or Windows Authentication (single sign on for Windows users). | | `User` | Database user's login/username | diff --git a/pkg/tsdb/mssql/mssql.go b/pkg/tsdb/mssql/mssql.go index 8be66327c00..3e94b8c610c 100644 --- a/pkg/tsdb/mssql/mssql.go +++ b/pkg/tsdb/mssql/mssql.go @@ -66,33 +66,44 @@ func ParseURL(u string) (*url.URL, error) { }, nil } -func generateConnectionString(datasource *models.DataSource) (string, error) { +func generateConnectionString(dataSource *models.DataSource) (string, error) { + const dfltPort = "0" var addr util.NetworkAddress - if datasource.Url != "" { - u, err := ParseURL(datasource.Url) + if dataSource.Url != "" { + u, err := ParseURL(dataSource.Url) if err != nil { return "", err } - addr, err = util.SplitHostPortDefault(u.Host, "localhost", "1433") + addr, err = util.SplitHostPortDefault(u.Host, "localhost", dfltPort) if err != nil { return "", err } } else { addr = util.NetworkAddress{ Host: "localhost", - Port: "1433", + Port: dfltPort, } } - logger.Debug("Generating connection string", "url", datasource.Url, "host", addr.Host, "port", addr.Port) - encrypt := datasource.JsonData.Get("encrypt").MustString("false") - connStr := fmt.Sprintf("server=%s;port=%s;database=%s;user id=%s;password=%s;", + args := []interface{}{ + "url", dataSource.Url, "host", addr.Host, + } + if addr.Port != "0" { + args = append(args, "port", addr.Port) + } + + logger.Debug("Generating connection string", args...) + encrypt := dataSource.JsonData.Get("encrypt").MustString("false") + connStr := fmt.Sprintf("server=%s;database=%s;user id=%s;password=%s;", addr.Host, - addr.Port, - datasource.Database, - datasource.User, - datasource.DecryptedPassword(), + dataSource.Database, + dataSource.User, + dataSource.DecryptedPassword(), ) + // Port number 0 means to determine the port automatically, so we can let the driver choose + if addr.Port != "0" { + connStr += fmt.Sprintf("port=%s;", addr.Port) + } if encrypt != "false" { connStr += fmt.Sprintf("encrypt=%s;", encrypt) } diff --git a/pkg/tsdb/mssql/mssql_test.go b/pkg/tsdb/mssql/mssql_test.go index 9f8e6a770dd..784bf9622dc 100644 --- a/pkg/tsdb/mssql/mssql_test.go +++ b/pkg/tsdb/mssql/mssql_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/tsdb/sqleng" . "github.com/smartystreets/goconvey/convey" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "xorm.io/xorm" ) @@ -32,7 +33,7 @@ var serverIP = "localhost" func TestMSSQL(t *testing.T) { SkipConvey("MSSQL", t, func() { - x := InitMSSQLTestDB(t) + x := initMSSQLTestDB(t) origXormEngine := sqleng.NewXormEngine sqleng.NewXormEngine = func(d, c string) (*xorm.Engine, error) { @@ -1161,13 +1162,70 @@ func TestTransformQueryError(t *testing.T) { } } -func InitMSSQLTestDB(t *testing.T) *xorm.Engine { +func TestGenerateConnectionString(t *testing.T) { + testCases := []struct { + desc string + dataSource *models.DataSource + expConnStr string + }{ + { + desc: "From URL w/ port", + dataSource: &models.DataSource{ + Url: "localhost:1001", + Database: "database", + User: "user", + JsonData: simplejson.NewFromAny(map[string]interface{}{}), + }, + expConnStr: "server=localhost;database=database;user id=user;password=;port=1001;", + }, + // When no port is specified, the driver should be allowed to choose + { + desc: "From URL w/o port", + dataSource: &models.DataSource{ + Url: "localhost", + Database: "database", + User: "user", + JsonData: simplejson.NewFromAny(map[string]interface{}{}), + }, + expConnStr: "server=localhost;database=database;user id=user;password=;", + }, + // Port 0 should be equivalent to not specifying a port, i.e. let the driver choose + { + desc: "From URL w port 0", + dataSource: &models.DataSource{ + Url: "localhost:0", + Database: "database", + User: "user", + JsonData: simplejson.NewFromAny(map[string]interface{}{}), + }, + expConnStr: "server=localhost;database=database;user id=user;password=;", + }, + { + desc: "Defaults", + dataSource: &models.DataSource{ + Database: "database", + User: "user", + JsonData: simplejson.NewFromAny(map[string]interface{}{}), + }, + expConnStr: "server=localhost;database=database;user id=user;password=;", + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + connStr, err := generateConnectionString(tc.dataSource) + require.NoError(t, err) + assert.Equal(t, tc.expConnStr, connStr) + }) + } +} + +func initMSSQLTestDB(t *testing.T) *xorm.Engine { + t.Helper() + testDB := sqlutil.MSSQLTestDB() x, err := xorm.NewEngine(testDB.DriverName, strings.Replace(testDB.ConnStr, "localhost", serverIP, 1)) - if err != nil { - t.Fatalf("Failed to init mssql db %v", err) - } + require.NoError(t, err) x.DatabaseTZ = time.UTC x.TZLocation = time.UTC diff --git a/public/app/plugins/datasource/mssql/partials/config.html b/public/app/plugins/datasource/mssql/partials/config.html index 51ecbd7579f..af18c632ae0 100644 --- a/public/app/plugins/datasource/mssql/partials/config.html +++ b/public/app/plugins/datasource/mssql/partials/config.html @@ -4,7 +4,7 @@
Host - +