From be2fa54459bd6aa9305ec6ee83be8599370f9ffe Mon Sep 17 00:00:00 2001 From: Martin Molnar Date: Tue, 20 Feb 2018 11:15:31 +0100 Subject: [PATCH 01/84] feat(ldap): Allow use of DN in user attribute filter (#3132) --- pkg/login/ldap.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/login/ldap.go b/pkg/login/ldap.go index be3babac02e..12e10557ffc 100644 --- a/pkg/login/ldap.go +++ b/pkg/login/ldap.go @@ -408,6 +408,10 @@ func (a *ldapAuther) searchForUser(username string) (*LdapUserInfo, error) { if a.server.GroupSearchFilterUserAttribute == "" { filter_replace = getLdapAttr(a.server.Attr.Username, searchResult) } + if a.server.GroupSearchFilterUserAttribute == "dn" { + filter_replace = searchResult.Entries[0].DN + } + filter := strings.Replace(a.server.GroupSearchFilter, "%s", ldap.EscapeFilter(filter_replace), -1) a.log.Info("Searching for user's groups", "filter", filter) @@ -430,7 +434,11 @@ func (a *ldapAuther) searchForUser(username string) (*LdapUserInfo, error) { if len(groupSearchResult.Entries) > 0 { for i := range groupSearchResult.Entries { - memberOf = append(memberOf, getLdapAttrN(a.server.Attr.MemberOf, groupSearchResult, i)) + if a.server.Attr.MemberOf == "dn" { + memberOf = append(memberOf, groupSearchResult.Entries[i].DN) + } else { + memberOf = append(memberOf, getLdapAttrN(a.server.Attr.MemberOf, groupSearchResult, i)) + } } break } From e104e9b2c251d8c7f068fc8c109e49900e6eca0b Mon Sep 17 00:00:00 2001 From: Aleksei Magusev Date: Mon, 29 Jan 2018 16:46:51 +0100 Subject: [PATCH 02/84] Conditionally select a field to return in ResponseParser for InfluxDB This patch also fixes "value[1] || value[0]" to not ignore zeros. --- .../datasource/influxdb/response_parser.ts | 22 +++++- .../influxdb/specs/response_parser.jest.ts | 71 +++++++++++++------ 2 files changed, 69 insertions(+), 24 deletions(-) diff --git a/public/app/plugins/datasource/influxdb/response_parser.ts b/public/app/plugins/datasource/influxdb/response_parser.ts index 78ce67e7a37..ea7403f1909 100644 --- a/public/app/plugins/datasource/influxdb/response_parser.ts +++ b/public/app/plugins/datasource/influxdb/response_parser.ts @@ -11,14 +11,30 @@ export default class ResponseParser { return []; } - var influxdb11format = query.toLowerCase().indexOf('show tag values') >= 0; + var normalizedQuery = query.toLowerCase(); + var isValueFirst = + normalizedQuery.indexOf('show field keys') >= 0 || normalizedQuery.indexOf('show retention policies') >= 0; var res = {}; _.each(influxResults.series, serie => { _.each(serie.values, value => { if (_.isArray(value)) { - if (influxdb11format) { - addUnique(res, value[1] || value[0]); + // In general, there are 2 possible shapes for the returned value. + // The first one is a two-element array, + // where the first element is somewhat a metadata value: + // the tag name for SHOW TAG VALUES queries, + // the time field for SELECT queries, etc. + // The second shape is an one-element array, + // that is containing an immediate value. + // For example, SHOW FIELD KEYS queries return such shape. + // Note, pre-0.11 versions return + // the second shape for SHOW TAG VALUES queries + // (while the newer versions—first). + + if (isValueFirst) { + addUnique(res, value[0]); + } else if (value[1] !== undefined) { + addUnique(res, value[1]); } else { addUnique(res, value[0]); } diff --git a/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts b/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts index 8ddc0fcdaf1..2928b9728d3 100644 --- a/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts +++ b/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts @@ -85,30 +85,36 @@ describe('influxdb response parser', () => { }); }); + describe('SELECT response', () => { + var query = 'SELECT "usage_iowait" FROM "cpu" LIMIT 10'; + var response = { + results: [ + { + series: [ + { + name: 'cpu', + columns: ['time', 'usage_iowait'], + values: [[1488465190006040638, 0.0], [1488465190006040638, 15.0], [1488465190006040638, 20.2]], + }, + ], + }, + ], + }; + + var result = parser.parse(query, response); + + it('should return second column', () => { + expect(_.size(result)).toBe(3); + expect(result[0].text).toBe(0); + expect(result[1].text).toBe(15); + expect(result[2].text).toBe(20.2); + }); + }); + describe('SHOW FIELD response', () => { var query = 'SHOW FIELD KEYS FROM "cpu"'; - describe('response from 0.10.0', () => { - var response = { - results: [ - { - series: [ - { - name: 'measurements', - columns: ['name'], - values: [['cpu'], ['derivative'], ['logins.count'], ['logs'], ['payment.ended'], ['payment.started']], - }, - ], - }, - ], - }; - var result = parser.parse(query, response); - it('should get two responses', () => { - expect(_.size(result)).toBe(6); - }); - }); - - describe('response from 0.11.0', () => { + describe('response from pre-1.0', () => { var response = { results: [ { @@ -129,5 +135,28 @@ describe('influxdb response parser', () => { expect(_.size(result)).toBe(1); }); }); + + describe('response from 1.0', () => { + var response = { + results: [ + { + series: [ + { + name: 'cpu', + columns: ['fieldKey', 'fieldType'], + values: [['time', 'float']], + }, + ], + }, + ], + }; + + var result = parser.parse(query, response); + + it('should return first column', () => { + expect(_.size(result)).toBe(1); + expect(result[0].text).toBe('time'); + }); + }); }); }); From b7482ae8b784b9c5364229be0c86b8ec61074826 Mon Sep 17 00:00:00 2001 From: Aleksei Magusev Date: Thu, 1 Feb 2018 14:55:03 +0100 Subject: [PATCH 03/84] Fix ResponseParser for InfluxDB to return only string values --- public/app/plugins/datasource/influxdb/response_parser.ts | 2 +- .../datasource/influxdb/specs/response_parser.jest.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/public/app/plugins/datasource/influxdb/response_parser.ts b/public/app/plugins/datasource/influxdb/response_parser.ts index ea7403f1909..f1a46001d95 100644 --- a/public/app/plugins/datasource/influxdb/response_parser.ts +++ b/public/app/plugins/datasource/influxdb/response_parser.ts @@ -45,7 +45,7 @@ export default class ResponseParser { }); return _.map(res, value => { - return { text: value }; + return { text: value.toString() }; }); } } diff --git a/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts b/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts index 2928b9728d3..525508b2c1d 100644 --- a/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts +++ b/public/app/plugins/datasource/influxdb/specs/response_parser.jest.ts @@ -105,9 +105,9 @@ describe('influxdb response parser', () => { it('should return second column', () => { expect(_.size(result)).toBe(3); - expect(result[0].text).toBe(0); - expect(result[1].text).toBe(15); - expect(result[2].text).toBe(20.2); + expect(result[0].text).toBe('0'); + expect(result[1].text).toBe('15'); + expect(result[2].text).toBe('20.2'); }); }); From e65cf5cbb5c929c2d99f1562499c4d63903035e3 Mon Sep 17 00:00:00 2001 From: Mitsuhiro Tanda Date: Fri, 18 May 2018 13:02:08 +0900 Subject: [PATCH 04/84] fix directly specified variable rendering --- .../datasource/cloudwatch/datasource.ts | 44 +++++++++------- .../cloudwatch/specs/datasource_specs.ts | 51 +++++++++++++++++++ 2 files changed, 78 insertions(+), 17 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/datasource.ts b/public/app/plugins/datasource/cloudwatch/datasource.ts index a466e9b84d2..4101759ec1d 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.ts @@ -374,23 +374,33 @@ export default class CloudWatchDatasource { getExpandedVariables(target, dimensionKey, variable, templateSrv) { /* if the all checkbox is marked we should add all values to the targets */ var allSelected = _.find(variable.options, { selected: true, text: 'All' }); - return _.chain(variable.options) - .filter(v => { - if (allSelected) { - return v.text !== 'All'; - } else { - return v.selected; - } - }) - .map(v => { - var t = angular.copy(target); - var scopedVar = {}; - scopedVar[variable.name] = v; - t.refId = target.refId + '_' + v.value; - t.dimensions[dimensionKey] = templateSrv.replace(t.dimensions[dimensionKey], scopedVar); - return t; - }) - .value(); + var selectedVariables = _.filter(variable.options, v => { + if (allSelected) { + return v.text !== 'All'; + } else { + return v.selected; + } + }); + var currentVariables = !_.isArray(variable.current.value) + ? [variable.current] + : variable.current.value.map(v => { + return { + text: v, + value: v, + }; + }); + let useSelectedVariables = + selectedVariables.some(s => { + return s.value === currentVariables[0].value; + }) || currentVariables[0].value === '$__all'; + return (useSelectedVariables ? selectedVariables : currentVariables).map(v => { + var t = angular.copy(target); + var scopedVar = {}; + scopedVar[variable.name] = v; + t.refId = target.refId + '_' + v.value; + t.dimensions[dimensionKey] = templateSrv.replace(t.dimensions[dimensionKey], scopedVar); + return t; + }); } expandTemplateVariable(targets, scopedVars, templateSrv) { diff --git a/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts b/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts index cca14f84255..16265beddc1 100644 --- a/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts +++ b/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts @@ -133,6 +133,10 @@ describe('CloudWatchDatasource', function() { { text: 'i-23456789', value: 'i-23456789', selected: false }, { text: 'i-34567890', value: 'i-34567890', selected: true }, ], + current: { + text: 'i-34567890', + value: 'i-34567890', + }, }, ], replace: function(target, scopedVars) { @@ -169,6 +173,53 @@ describe('CloudWatchDatasource', function() { var result = ctx.ds.expandTemplateVariable(targets, {}, templateSrv); expect(result[0].dimensions.InstanceId).to.be('i-34567890'); }); + + it('should generate the correct targets by expanding template variables from url', function() { + var templateSrv = { + variables: [ + { + name: 'instance_id', + options: [ + { text: 'i-23456789', value: 'i-23456789', selected: false }, + { text: 'i-34567890', value: 'i-34567890', selected: false }, + ], + current: 'i-45678901', + }, + ], + replace: function(target, scopedVars) { + if (target === '$instance_id') { + return 'i-45678901'; + } else { + return ''; + } + }, + getVariableName: function(e) { + return 'instance_id'; + }, + variableExists: function(e) { + return true; + }, + containsVariable: function(str, variableName) { + return str.indexOf('$' + variableName) !== -1; + }, + }; + + var targets = [ + { + region: 'us-east-1', + namespace: 'AWS/EC2', + metricName: 'CPUUtilization', + dimensions: { + InstanceId: '$instance_id', + }, + statistics: ['Average'], + period: 300, + }, + ]; + + var result = ctx.ds.expandTemplateVariable(targets, {}, templateSrv); + expect(result[0].dimensions.InstanceId).to.be('i-45678901'); + }); }); describe('When query region is "default"', function() { From 24f6d34abd38eef8bafe3767c632d276159269c2 Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Thu, 31 May 2018 22:18:59 +0300 Subject: [PATCH 05/84] graph: fix legend decimals precision calculation --- public/app/core/specs/ticks.jest.ts | 25 +++++++++++ public/app/core/specs/time_series.jest.ts | 52 +++++++++++++++++++++++ public/app/core/time_series2.ts | 14 +++--- public/app/core/utils/ticks.ts | 18 +++----- public/app/plugins/panel/graph/graph.ts | 3 +- 5 files changed, 94 insertions(+), 18 deletions(-) create mode 100644 public/app/core/specs/ticks.jest.ts diff --git a/public/app/core/specs/ticks.jest.ts b/public/app/core/specs/ticks.jest.ts new file mode 100644 index 00000000000..8b7e0cd73b5 --- /dev/null +++ b/public/app/core/specs/ticks.jest.ts @@ -0,0 +1,25 @@ +import * as ticks from '../utils/ticks'; + +describe('ticks', () => { + describe('getFlotTickDecimals()', () => { + let ctx: any = {}; + + beforeEach(() => { + ctx.axis = {}; + }); + + it('should calculate decimals precision based on graph height', () => { + let dec = ticks.getFlotTickDecimals(0, 10, ctx.axis, 200); + expect(dec.tickDecimals).toBe(1); + expect(dec.scaledDecimals).toBe(1); + + dec = ticks.getFlotTickDecimals(0, 100, ctx.axis, 200); + expect(dec.tickDecimals).toBe(0); + expect(dec.scaledDecimals).toBe(-1); + + dec = ticks.getFlotTickDecimals(0, 1, ctx.axis, 200); + expect(dec.tickDecimals).toBe(2); + expect(dec.scaledDecimals).toBe(3); + }); + }); +}); diff --git a/public/app/core/specs/time_series.jest.ts b/public/app/core/specs/time_series.jest.ts index 6214c687add..f5245476218 100644 --- a/public/app/core/specs/time_series.jest.ts +++ b/public/app/core/specs/time_series.jest.ts @@ -1,4 +1,5 @@ import TimeSeries from 'app/core/time_series2'; +import { updateLegendValues } from 'app/core/time_series2'; describe('TimeSeries', function() { var points, series; @@ -311,4 +312,55 @@ describe('TimeSeries', function() { expect(series.formatValue(-Infinity)).toBe(''); }); }); + + describe('legend decimals', function() { + let series, panel; + let height = 200; + beforeEach(function() { + testData = { + alias: 'test', + datapoints: [[1, 2], [0, 3], [10, 4], [8, 5]], + }; + series = new TimeSeries(testData); + series.getFlotPairs(); + panel = { + decimals: null, + yaxes: [ + { + decimals: null, + }, + ], + }; + }); + + it('should set decimals based on Y axis (expect calculated decimals = 1)', function() { + let data = [series]; + // Expect ticks with this data will have decimals = 1 + updateLegendValues(data, panel, height); + expect(data[0].decimals).toBe(2); + }); + + it('should set decimals based on Y axis to 0 if calculated decimals = 0)', function() { + testData.datapoints = [[10, 2], [0, 3], [100, 4], [80, 5]]; + series = new TimeSeries(testData); + series.getFlotPairs(); + let data = [series]; + updateLegendValues(data, panel, height); + expect(data[0].decimals).toBe(0); + }); + + it('should set decimals to Y axis decimals + 1', function() { + panel.yaxes[0].decimals = 2; + let data = [series]; + updateLegendValues(data, panel, height); + expect(data[0].decimals).toBe(3); + }); + + it('should set decimals to legend decimals value if it was set explicitly', function() { + panel.decimals = 3; + let data = [series]; + updateLegendValues(data, panel, height); + expect(data[0].decimals).toBe(3); + }); + }); }); diff --git a/public/app/core/time_series2.ts b/public/app/core/time_series2.ts index 4da64850e59..59729ebc312 100644 --- a/public/app/core/time_series2.ts +++ b/public/app/core/time_series2.ts @@ -23,23 +23,27 @@ function translateFillOption(fill) { * Calculate decimals for legend and update values for each series. * @param data series data * @param panel + * @param height */ -export function updateLegendValues(data: TimeSeries[], panel) { +export function updateLegendValues(data: TimeSeries[], panel, height) { for (let i = 0; i < data.length; i++) { let series = data[i]; - let yaxes = panel.yaxes; + const yaxes = panel.yaxes; const seriesYAxis = series.yaxis || 1; - let axis = yaxes[seriesYAxis - 1]; - let { tickDecimals, scaledDecimals } = getFlotTickDecimals(data, axis); - let formater = kbn.valueFormats[panel.yaxes[seriesYAxis - 1].format]; + const axis = yaxes[seriesYAxis - 1]; + let formater = kbn.valueFormats[axis.format]; // decimal override if (_.isNumber(panel.decimals)) { series.updateLegendValues(formater, panel.decimals, null); + } else if (_.isNumber(axis.decimals)) { + series.updateLegendValues(formater, axis.decimals + 1, null); } else { // auto decimals // legend and tooltip gets one more decimal precision // than graph legend ticks + const { datamin, datamax } = getDataMinMax(data); + let { tickDecimals, scaledDecimals } = getFlotTickDecimals(datamin, datamax, axis, height); tickDecimals = (tickDecimals || -1) + 1; series.updateLegendValues(formater, tickDecimals, scaledDecimals + 2); } diff --git a/public/app/core/utils/ticks.ts b/public/app/core/utils/ticks.ts index db65104cfc0..66e6a7ce4fc 100644 --- a/public/app/core/utils/ticks.ts +++ b/public/app/core/utils/ticks.ts @@ -1,5 +1,3 @@ -import { getDataMinMax } from 'app/core/time_series2'; - /** * Calculate tick step. * Implementation from d3-array (ticks.js) @@ -121,12 +119,10 @@ export function getFlotRange(panelMin, panelMax, datamin, datamax) { * Calculate tick decimals. * Implementation from Flot. */ -export function getFlotTickDecimals(data, axis) { - let { datamin, datamax } = getDataMinMax(data); - let { min, max } = getFlotRange(axis.min, axis.max, datamin, datamax); - let noTicks = 3; - let tickDecimals, maxDec; - let delta = (max - min) / noTicks; +export function getFlotTickDecimals(datamin, datamax, axis, height) { + const { min, max } = getFlotRange(axis.min, axis.max, datamin, datamax); + const noTicks = 0.3 * Math.sqrt(height); + const delta = (max - min) / noTicks; let dec = -Math.floor(Math.log(delta) / Math.LN10); let magn = Math.pow(10, -dec); @@ -139,19 +135,17 @@ export function getFlotTickDecimals(data, axis) { } else if (norm < 3) { size = 2; // special case for 2.5, requires an extra decimal - if (norm > 2.25 && (maxDec == null || dec + 1 <= maxDec)) { + if (norm > 2.25) { size = 2.5; - ++dec; } } else if (norm < 7.5) { size = 5; } else { size = 10; } - size *= magn; - tickDecimals = Math.max(0, maxDec != null ? maxDec : dec); + const tickDecimals = Math.max(0, -Math.floor(Math.log(delta) / Math.LN10) + 1); // grafana addition const scaledDecimals = tickDecimals - Math.floor(Math.log(size) / Math.LN10); return { tickDecimals, scaledDecimals }; diff --git a/public/app/plugins/panel/graph/graph.ts b/public/app/plugins/panel/graph/graph.ts index 9e4fb42952e..9f216c12288 100755 --- a/public/app/plugins/panel/graph/graph.ts +++ b/public/app/plugins/panel/graph/graph.ts @@ -64,7 +64,8 @@ function graphDirective(timeSrv, popoverSrv, contextSrv) { } annotations = ctrl.annotations || []; buildFlotPairs(data); - updateLegendValues(data, panel); + const graphHeight = elem.height(); + updateLegendValues(data, panel, graphHeight); ctrl.events.emit('render-legend'); }); From 516839d7b22b5a1fc75c698d47f87e0d4a59ed44 Mon Sep 17 00:00:00 2001 From: Anton Sergeyev Date: Thu, 14 Jun 2018 12:35:22 +0500 Subject: [PATCH 06/84] #11607 Cleanup time of temporary files is now configurable --- conf/defaults.ini | 3 ++ conf/sample.ini | 3 ++ docs/sources/installation/configuration.md | 5 +++ pkg/services/cleanup/cleanup.go | 12 ++++++- pkg/services/cleanup/cleanup_test.go | 42 ++++++++++++++++++++++ pkg/setting/setting.go | 6 ++++ 6 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 pkg/services/cleanup/cleanup_test.go diff --git a/conf/defaults.ini b/conf/defaults.ini index 4ca993038f9..5faba3ea7bd 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -14,6 +14,9 @@ instance_name = ${HOSTNAME} # Path to where grafana can store temp files, sessions, and the sqlite3 db (if that is used) data = data +# Temporary files in `data` directory older than given duration will be removed +temp_data_lifetime = 24h + # Directory where grafana can store logs logs = data/log diff --git a/conf/sample.ini b/conf/sample.ini index 45888cbadd8..af4e78eed72 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -14,6 +14,9 @@ # Path to where grafana can store temp files, sessions, and the sqlite3 db (if that is used) ;data = /var/lib/grafana +# Temporary files in `data` directory older than given duration will be removed +temp_data_lifetime = 24h + # Directory where grafana can store logs ;logs = /var/log/grafana diff --git a/docs/sources/installation/configuration.md b/docs/sources/installation/configuration.md index c1d0f3da834..668a44fcb2b 100644 --- a/docs/sources/installation/configuration.md +++ b/docs/sources/installation/configuration.md @@ -80,6 +80,11 @@ Path to where Grafana stores the sqlite3 database (if used), file based sessions (if used), and other data. This path is usually specified via command line in the init.d script or the systemd service file. +### temp_data_lifetime + +How long temporary images in `data` directory should be kept. Defaults to: `24h`. Supported modifiers: `h` (hours), +`m` (minutes), for example: `168h`, `30m`, `10h30m`. Use `0` to never clean up temporary files. + ### logs Path to where Grafana will store logs. This path is usually specified via diff --git a/pkg/services/cleanup/cleanup.go b/pkg/services/cleanup/cleanup.go index 69bc7695dea..521601a358b 100644 --- a/pkg/services/cleanup/cleanup.go +++ b/pkg/services/cleanup/cleanup.go @@ -57,8 +57,10 @@ func (srv *CleanUpService) cleanUpTmpFiles() { } var toDelete []os.FileInfo + var now = time.Now() + for _, file := range files { - if file.ModTime().AddDate(0, 0, 1).Before(time.Now()) { + if srv.shouldCleanupTempFile(file.ModTime(), now) { toDelete = append(toDelete, file) } } @@ -74,6 +76,14 @@ func (srv *CleanUpService) cleanUpTmpFiles() { srv.log.Debug("Found old rendered image to delete", "deleted", len(toDelete), "keept", len(files)) } +func (srv *CleanUpService) shouldCleanupTempFile(filemtime time.Time, now time.Time) bool { + if srv.Cfg.TempDataLifetime == 0 { + return false + } + + return filemtime.Add(srv.Cfg.TempDataLifetime).Before(now) +} + func (srv *CleanUpService) deleteExpiredSnapshots() { cmd := m.DeleteExpiredSnapshotsCommand{} if err := bus.Dispatch(&cmd); err != nil { diff --git a/pkg/services/cleanup/cleanup_test.go b/pkg/services/cleanup/cleanup_test.go new file mode 100644 index 00000000000..a504e316e75 --- /dev/null +++ b/pkg/services/cleanup/cleanup_test.go @@ -0,0 +1,42 @@ +package cleanup + +import ( + "testing" + + "github.com/grafana/grafana/pkg/setting" + . "github.com/smartystreets/goconvey/convey" + "time" +) + +func TestCleanUpTmpFiles(t *testing.T) { + Convey("Cleanup service tests", t, func() { + cfg := setting.Cfg{} + cfg.TempDataLifetime, _ = time.ParseDuration("24h") + service := CleanUpService{ + Cfg: &cfg, + } + now := time.Now() + secondAgo := now.Add(-time.Second) + dayAgo := now.Add(-time.Second * 3600 * 24 * 7) + weekAgo := now.Add(-time.Second * 3600 * 24 * 7) + + Convey("Should not cleanup recent files", func() { + So(service.shouldCleanupTempFile(secondAgo, now), ShouldBeFalse); + }) + + Convey("Should cleanup older files", func() { + So(service.shouldCleanupTempFile(dayAgo, now), ShouldBeTrue); + }) + + Convey("After increasing temporary files lifetime, older files should be kept", func() { + cfg.TempDataLifetime, _ = time.ParseDuration("1000h") + So(service.shouldCleanupTempFile(weekAgo, now), ShouldBeFalse); + }) + + Convey("If lifetime is 0, files should never be cleaned up", func() { + cfg.TempDataLifetime = 0 + So(service.shouldCleanupTempFile(weekAgo, now), ShouldBeFalse); + }) + }) + +} diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 9bae4e35950..3cb7966d20c 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/util" + "time" ) type Scheme string @@ -195,6 +196,8 @@ type Cfg struct { PhantomDir string RendererUrl string DisableBruteForceLoginProtection bool + + TempDataLifetime time.Duration } type CommandLineArgs struct { @@ -637,6 +640,9 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.RendererUrl = renderSec.Key("server_url").String() cfg.ImagesDir = filepath.Join(DataPath, "png") cfg.PhantomDir = filepath.Join(HomePath, "tools/phantomjs") + cfg.TempDataLifetime = iniFile.Section("paths").Key("temp_data_lifetime").MustDuration( + time.Duration(time.Second*3600*24), + ) analytics := iniFile.Section("analytics") ReportingEnabled = analytics.Key("reporting_enabled").MustBool(true) From 2024cf4b5686302eca563a1a90c93dc2a1a67b6f Mon Sep 17 00:00:00 2001 From: Anton Sergeyev Date: Thu, 14 Jun 2018 12:46:29 +0500 Subject: [PATCH 07/84] #11607 fixed formatting --- conf/sample.ini | 2 +- pkg/services/cleanup/cleanup_test.go | 11 +++++------ pkg/setting/setting.go | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/conf/sample.ini b/conf/sample.ini index af4e78eed72..87544a5ac39 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -15,7 +15,7 @@ ;data = /var/lib/grafana # Temporary files in `data` directory older than given duration will be removed -temp_data_lifetime = 24h +;temp_data_lifetime = 24h # Directory where grafana can store logs ;logs = /var/log/grafana diff --git a/pkg/services/cleanup/cleanup_test.go b/pkg/services/cleanup/cleanup_test.go index a504e316e75..834fdc424fb 100644 --- a/pkg/services/cleanup/cleanup_test.go +++ b/pkg/services/cleanup/cleanup_test.go @@ -1,10 +1,9 @@ package cleanup import ( - "testing" - "github.com/grafana/grafana/pkg/setting" . "github.com/smartystreets/goconvey/convey" + "testing" "time" ) @@ -21,21 +20,21 @@ func TestCleanUpTmpFiles(t *testing.T) { weekAgo := now.Add(-time.Second * 3600 * 24 * 7) Convey("Should not cleanup recent files", func() { - So(service.shouldCleanupTempFile(secondAgo, now), ShouldBeFalse); + So(service.shouldCleanupTempFile(secondAgo, now), ShouldBeFalse) }) Convey("Should cleanup older files", func() { - So(service.shouldCleanupTempFile(dayAgo, now), ShouldBeTrue); + So(service.shouldCleanupTempFile(dayAgo, now), ShouldBeTrue) }) Convey("After increasing temporary files lifetime, older files should be kept", func() { cfg.TempDataLifetime, _ = time.ParseDuration("1000h") - So(service.shouldCleanupTempFile(weekAgo, now), ShouldBeFalse); + So(service.shouldCleanupTempFile(weekAgo, now), ShouldBeFalse) }) Convey("If lifetime is 0, files should never be cleaned up", func() { cfg.TempDataLifetime = 0 - So(service.shouldCleanupTempFile(weekAgo, now), ShouldBeFalse); + So(service.shouldCleanupTempFile(weekAgo, now), ShouldBeFalse) }) }) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 3cb7966d20c..99a2a3ce1c9 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -641,7 +641,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.ImagesDir = filepath.Join(DataPath, "png") cfg.PhantomDir = filepath.Join(HomePath, "tools/phantomjs") cfg.TempDataLifetime = iniFile.Section("paths").Key("temp_data_lifetime").MustDuration( - time.Duration(time.Second*3600*24), + time.Duration(time.Second * 3600 * 24), ) analytics := iniFile.Section("analytics") From 7818578d6a67c4e6920bcd33a664ba7d3bd43088 Mon Sep 17 00:00:00 2001 From: Anton Sergeyev Date: Thu, 14 Jun 2018 12:50:18 +0500 Subject: [PATCH 08/84] #11607 removed unnecessary conversion (from gometalinter) --- pkg/setting/setting.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 99a2a3ce1c9..e71a3619aa5 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -640,9 +640,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { cfg.RendererUrl = renderSec.Key("server_url").String() cfg.ImagesDir = filepath.Join(DataPath, "png") cfg.PhantomDir = filepath.Join(HomePath, "tools/phantomjs") - cfg.TempDataLifetime = iniFile.Section("paths").Key("temp_data_lifetime").MustDuration( - time.Duration(time.Second * 3600 * 24), - ) + cfg.TempDataLifetime = iniFile.Section("paths").Key("temp_data_lifetime").MustDuration(time.Second * 3600 * 24) analytics := iniFile.Section("analytics") ReportingEnabled = analytics.Key("reporting_enabled").MustBool(true) From 2565a8087cc1741fb170cf0e1eef6fe4696b9a85 Mon Sep 17 00:00:00 2001 From: Anton Sergeyev Date: Thu, 14 Jun 2018 13:00:07 +0500 Subject: [PATCH 09/84] #11607 corrected file cleanup test --- pkg/services/cleanup/cleanup_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/services/cleanup/cleanup_test.go b/pkg/services/cleanup/cleanup_test.go index 834fdc424fb..54d29e32bf1 100644 --- a/pkg/services/cleanup/cleanup_test.go +++ b/pkg/services/cleanup/cleanup_test.go @@ -16,7 +16,7 @@ func TestCleanUpTmpFiles(t *testing.T) { } now := time.Now() secondAgo := now.Add(-time.Second) - dayAgo := now.Add(-time.Second * 3600 * 24 * 7) + twoDaysAgo := now.Add(-time.Second * 3600 * 24 * 2) weekAgo := now.Add(-time.Second * 3600 * 24 * 7) Convey("Should not cleanup recent files", func() { @@ -24,7 +24,7 @@ func TestCleanUpTmpFiles(t *testing.T) { }) Convey("Should cleanup older files", func() { - So(service.shouldCleanupTempFile(dayAgo, now), ShouldBeTrue) + So(service.shouldCleanupTempFile(twoDaysAgo, now), ShouldBeTrue) }) Convey("After increasing temporary files lifetime, older files should be kept", func() { From 8143610024ef01729a850659072bab83fe5694c1 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 5 Jun 2018 21:13:53 +0200 Subject: [PATCH 10/84] bus: support multiple dispatch in one transaction this makes it possible to run multiple DispatchCtx in one transaction. The TransactionManager will start/end the transaction and pass the dbsession in the context.Context variable --- pkg/bus/bus.go | 50 +++++++++++++++++++++++++ pkg/services/alerting/notifiers/base.go | 1 + pkg/services/sqlstore/shared.go | 28 +++++++++++++- pkg/services/sqlstore/sqlstore.go | 49 +++++++++++++++++++++++- pkg/services/sqlstore/stats.go | 18 +++++++++ 5 files changed, 144 insertions(+), 2 deletions(-) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 32a591b6672..98279678777 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -12,21 +12,51 @@ type Msg interface{} var ErrHandlerNotFound = errors.New("handler not found") +type TransactionManager interface { + Begin(ctx context.Context) (context.Context, error) + End(ctx context.Context, err error) error +} + type Bus interface { Dispatch(msg Msg) error DispatchCtx(ctx context.Context, msg Msg) error Publish(msg Msg) error + // InTransaction starts a transaction and store it in the context. + // The caller can then pass a function with multiple DispatchCtx calls that + // all will be executed in the same transaction. InTransaction will rollback if the + // callback returns an error.s + InTransaction(ctx context.Context, fn func(ctx context.Context) error) error + AddHandler(handler HandlerFunc) AddCtxHandler(handler HandlerFunc) AddEventListener(handler HandlerFunc) AddWildcardListener(handler HandlerFunc) + + // SetTransactionManager allows the user to replace the internal + // noop TransactionManager that is responsible for manageing + // transactions in `InTransaction` + SetTransactionManager(tm TransactionManager) +} + +func (b *InProcBus) InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { + ctxWithTran, err := b.transactionManager.Begin(ctx) + if err != nil { + return err + } + + err = fn(ctxWithTran) + b.transactionManager.End(ctxWithTran, err) + + return err } type InProcBus struct { handlers map[string]HandlerFunc listeners map[string][]HandlerFunc wildcardListeners []HandlerFunc + + transactionManager TransactionManager } // temp stuff, not sure how to handle bus instance, and init yet @@ -37,6 +67,9 @@ func New() Bus { bus.handlers = make(map[string]HandlerFunc) bus.listeners = make(map[string][]HandlerFunc) bus.wildcardListeners = make([]HandlerFunc, 0) + + bus.transactionManager = &NoopTransactionManager{} + return bus } @@ -45,6 +78,14 @@ func GetBus() Bus { return globalBus } +func SetTransactionManager(tm TransactionManager) { + globalBus.SetTransactionManager(tm) +} + +func (b *InProcBus) SetTransactionManager(tm TransactionManager) { + b.transactionManager = tm +} + func (b *InProcBus) DispatchCtx(ctx context.Context, msg Msg) error { var msgName = reflect.TypeOf(msg).Elem().Name() @@ -167,6 +208,15 @@ func Publish(msg Msg) error { return globalBus.Publish(msg) } +func InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { + return globalBus.InTransaction(ctx, fn) +} + func ClearBusHandlers() { globalBus = New() } + +type NoopTransactionManager struct{} + +func (*NoopTransactionManager) Begin(ctx context.Context) (context.Context, error) { return ctx, nil } +func (*NoopTransactionManager) End(ctx context.Context, err error) error { return err } diff --git a/pkg/services/alerting/notifiers/base.go b/pkg/services/alerting/notifiers/base.go index 51676efdfd5..868db3aec79 100644 --- a/pkg/services/alerting/notifiers/base.go +++ b/pkg/services/alerting/notifiers/base.go @@ -3,6 +3,7 @@ package notifiers import ( "github.com/grafana/grafana/pkg/components/simplejson" m "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/alerting" ) diff --git a/pkg/services/sqlstore/shared.go b/pkg/services/sqlstore/shared.go index 9a24a513aad..3ccb92f010f 100644 --- a/pkg/services/sqlstore/shared.go +++ b/pkg/services/sqlstore/shared.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "reflect" "time" @@ -29,10 +30,35 @@ func inTransaction(callback dbTransactionFunc) error { return inTransactionWithRetry(callback, 0) } +func startSession(ctx context.Context) *DBSession { + value := ctx.Value(ContextSessionName) + var sess *xorm.Session + sess, ok := value.(*xorm.Session) + + if !ok { + return newSession() + } + + old := newSession() + old.Session = sess + + return old +} + +func withDbSession(ctx context.Context, callback dbTransactionFunc) error { + sess := startSession(ctx) + + return callback(sess) +} + func inTransactionWithRetry(callback dbTransactionFunc, retry int) error { + return inTransactionWithRetryCtx(context.Background(), callback, retry) +} + +func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, retry int) error { var err error - sess := newSession() + sess := startSession(ctx) defer sess.Close() if err = sess.Begin(); err != nil { diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index ed82829665f..bfe462f4d91 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -1,6 +1,8 @@ package sqlstore import ( + "context" + "errors" "fmt" "net/url" "os" @@ -35,6 +37,8 @@ var ( sqlog log.Logger = log.New("sqlstore") ) +const ContextSessionName = "db-session" + func init() { registry.Register(®istry.Descriptor{ Name: "SqlStore", @@ -45,6 +49,7 @@ func init() { type SqlStore struct { Cfg *setting.Cfg `inject:""` + Bus bus.Bus `inject:""` dbCfg DatabaseConfig engine *xorm.Engine @@ -77,6 +82,10 @@ func (ss *SqlStore) Init() error { // Init repo instances annotations.SetRepository(&SqlAnnotationRepo{}) + ss.Bus.SetTransactionManager(&SQLTransactionManager{ + engine: ss.engine, + }) + // ensure admin user if ss.skipEnsureAdmin { return nil @@ -85,10 +94,47 @@ func (ss *SqlStore) Init() error { return ss.ensureAdminUser() } +type SQLTransactionManager struct { + engine *xorm.Engine +} + +func (stm *SQLTransactionManager) Begin(ctx context.Context) (context.Context, error) { + sess := stm.engine.NewSession() + err := sess.Begin() + if err != nil { + return ctx, err + } + + withValue := context.WithValue(ctx, ContextSessionName, sess) + + return withValue, nil +} + +func (stm *SQLTransactionManager) End(ctx context.Context, err error) error { + value := ctx.Value(ContextSessionName) + sess, ok := value.(*xorm.Session) + if !ok { + return errors.New("context is missing transaction") + } + + if err != nil { + sess.Rollback() + return err + } + + defer sess.Close() + + return sess.Commit() +} + func (ss *SqlStore) ensureAdminUser() error { systemUserCountQuery := m.GetSystemUserCountStatsQuery{} - if err := bus.Dispatch(&systemUserCountQuery); err != nil { + err := bus.InTransaction(context.Background(), func(ctx context.Context) error { + return bus.DispatchCtx(ctx, &systemUserCountQuery) + }) + + if err != nil { return fmt.Errorf("Could not determine if admin user exists: %v", err) } @@ -240,6 +286,7 @@ func (ss *SqlStore) readConfig() { func InitTestDB(t *testing.T) *SqlStore { sqlstore := &SqlStore{} sqlstore.skipEnsureAdmin = true + sqlstore.Bus = bus.New() dbType := migrator.SQLITE diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index 3e3e83c4014..af4482d9e25 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "time" "github.com/grafana/grafana/pkg/bus" @@ -13,6 +14,7 @@ func init() { bus.AddHandler("sql", GetDataSourceAccessStats) bus.AddHandler("sql", GetAdminStats) bus.AddHandler("sql", GetSystemUserCountStats) + bus.AddCtxHandler("sql", GetSystemUserCountStatsCtx) } var activeUserTimeLimit = time.Hour * 24 * 30 @@ -133,6 +135,22 @@ func GetAdminStats(query *m.GetAdminStatsQuery) error { return err } +func GetSystemUserCountStatsCtx(ctx context.Context, query *m.GetSystemUserCountStatsQuery) error { + return withDbSession(ctx, func(sess *DBSession) error { + + var rawSql = `SELECT COUNT(id) AS Count FROM ` + dialect.Quote("user") + var stats m.SystemUserCountStats + _, err := sess.SQL(rawSql).Get(&stats) + if err != nil { + return err + } + + query.Result = &stats + + return err + }) +} + func GetSystemUserCountStats(query *m.GetSystemUserCountStatsQuery) error { var rawSql = `SELECT COUNT(id) AS Count FROM ` + dialect.Quote("user") var stats m.SystemUserCountStats From 263572813a493ac34618075bedee5c0bac92df19 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 7 Jun 2018 18:02:28 +0200 Subject: [PATCH 11/84] replace begin/end with wrapper function --- pkg/bus/bus.go | 40 +++++++++++++--------------- pkg/services/sqlstore/shared.go | 13 +++++---- pkg/services/sqlstore/sqlstore.go | 44 ++++++++++++++++++++----------- 3 files changed, 52 insertions(+), 45 deletions(-) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 98279678777..0f10dfd9b17 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -12,9 +12,8 @@ type Msg interface{} var ErrHandlerNotFound = errors.New("handler not found") -type TransactionManager interface { - Begin(ctx context.Context) (context.Context, error) - End(ctx context.Context, err error) error +type TransactionWrapper interface { + Wrapp(ctx context.Context, fn func(ctx context.Context) error) error } type Bus interface { @@ -25,7 +24,7 @@ type Bus interface { // InTransaction starts a transaction and store it in the context. // The caller can then pass a function with multiple DispatchCtx calls that // all will be executed in the same transaction. InTransaction will rollback if the - // callback returns an error.s + // callback returns an error. InTransaction(ctx context.Context, fn func(ctx context.Context) error) error AddHandler(handler HandlerFunc) @@ -36,19 +35,11 @@ type Bus interface { // SetTransactionManager allows the user to replace the internal // noop TransactionManager that is responsible for manageing // transactions in `InTransaction` - SetTransactionManager(tm TransactionManager) + SetTransactionManager(tm TransactionWrapper) } func (b *InProcBus) InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { - ctxWithTran, err := b.transactionManager.Begin(ctx) - if err != nil { - return err - } - - err = fn(ctxWithTran) - b.transactionManager.End(ctxWithTran, err) - - return err + return b.transactionWrapper.Wrapp(ctx, fn) } type InProcBus struct { @@ -56,7 +47,7 @@ type InProcBus struct { listeners map[string][]HandlerFunc wildcardListeners []HandlerFunc - transactionManager TransactionManager + transactionWrapper TransactionWrapper } // temp stuff, not sure how to handle bus instance, and init yet @@ -68,7 +59,7 @@ func New() Bus { bus.listeners = make(map[string][]HandlerFunc) bus.wildcardListeners = make([]HandlerFunc, 0) - bus.transactionManager = &NoopTransactionManager{} + bus.transactionWrapper = &noopTransactionManager{} return bus } @@ -78,12 +69,12 @@ func GetBus() Bus { return globalBus } -func SetTransactionManager(tm TransactionManager) { +func SetTransactionManager(tm TransactionWrapper) { globalBus.SetTransactionManager(tm) } -func (b *InProcBus) SetTransactionManager(tm TransactionManager) { - b.transactionManager = tm +func (b *InProcBus) SetTransactionManager(tm TransactionWrapper) { + b.transactionWrapper = tm } func (b *InProcBus) DispatchCtx(ctx context.Context, msg Msg) error { @@ -208,6 +199,10 @@ func Publish(msg Msg) error { return globalBus.Publish(msg) } +// InTransaction starts a transaction and store it in the context. +// The caller can then pass a function with multiple DispatchCtx calls that +// all will be executed in the same transaction. InTransaction will rollback if the +// callback returns an error. func InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { return globalBus.InTransaction(ctx, fn) } @@ -216,7 +211,8 @@ func ClearBusHandlers() { globalBus = New() } -type NoopTransactionManager struct{} +type noopTransactionManager struct{} -func (*NoopTransactionManager) Begin(ctx context.Context) (context.Context, error) { return ctx, nil } -func (*NoopTransactionManager) End(ctx context.Context, err error) error { return err } +func (*noopTransactionManager) Wrapp(ctx context.Context, fn func(ctx context.Context) error) error { + return nil +} diff --git a/pkg/services/sqlstore/shared.go b/pkg/services/sqlstore/shared.go index 3ccb92f010f..7928f22b9f3 100644 --- a/pkg/services/sqlstore/shared.go +++ b/pkg/services/sqlstore/shared.go @@ -32,17 +32,16 @@ func inTransaction(callback dbTransactionFunc) error { func startSession(ctx context.Context) *DBSession { value := ctx.Value(ContextSessionName) - var sess *xorm.Session - sess, ok := value.(*xorm.Session) + var sess *DBSession + sess, ok := value.(*DBSession) if !ok { - return newSession() + newSess := newSession() + newSess.Begin() + return newSess } - old := newSession() - old.Session = sess - - return old + return sess } func withDbSession(ctx context.Context, callback dbTransactionFunc) error { diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index bfe462f4d91..d6268f3a2aa 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -2,7 +2,6 @@ package sqlstore import ( "context" - "errors" "fmt" "net/url" "os" @@ -26,6 +25,7 @@ import ( "github.com/go-xorm/xorm" _ "github.com/lib/pq" _ "github.com/mattn/go-sqlite3" + sqlite3 "github.com/mattn/go-sqlite3" _ "github.com/grafana/grafana/pkg/tsdb/mssql" ) @@ -94,37 +94,49 @@ func (ss *SqlStore) Init() error { return ss.ensureAdminUser() } +// SQLTransactionManager begin/end transaction type SQLTransactionManager struct { engine *xorm.Engine } -func (stm *SQLTransactionManager) Begin(ctx context.Context) (context.Context, error) { - sess := stm.engine.NewSession() - err := sess.Begin() - if err != nil { - return ctx, err - } +func (stm *SQLTransactionManager) Wrapp(ctx context.Context, fn func(ctx context.Context) error) error { + return stm.wrappInternal(ctx, fn, 0) +} + +func (stm *SQLTransactionManager) wrappInternal(ctx context.Context, fn func(ctx context.Context) error, retry int) error { + sess := startSession(ctx) + defer sess.Close() withValue := context.WithValue(ctx, ContextSessionName, sess) - return withValue, nil -} + err := fn(withValue) -func (stm *SQLTransactionManager) End(ctx context.Context, err error) error { - value := ctx.Value(ContextSessionName) - sess, ok := value.(*xorm.Session) - if !ok { - return errors.New("context is missing transaction") + // special handling of database locked errors for sqlite, then we can retry 3 times + if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 { + if sqlError.Code == sqlite3.ErrLocked { + sess.Rollback() + time.Sleep(time.Millisecond * time.Duration(10)) + sqlog.Info("Database table locked, sleeping then retrying", "retry", retry) + return stm.wrappInternal(ctx, fn, retry+1) + } } if err != nil { sess.Rollback() return err + } else if err = sess.Commit(); err != nil { + return err } - defer sess.Close() + if len(sess.events) > 0 { + for _, e := range sess.events { + if err = bus.Publish(e); err != nil { + log.Error(3, "Failed to publish event after commit", err) + } + } + } - return sess.Commit() + return nil } func (ss *SqlStore) ensureAdminUser() error { From 1bd31aa313df95b37781f859fe7c0926ab70a9e4 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 7 Jun 2018 18:17:01 +0200 Subject: [PATCH 12/84] check if admin exists or create one in one transaction --- pkg/services/sqlstore/sqlstore.go | 46 ++++----- pkg/services/sqlstore/user.go | 151 ++++++++++++++++-------------- 2 files changed, 106 insertions(+), 91 deletions(-) diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index d6268f3a2aa..a401e8c8c83 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -143,30 +143,32 @@ func (ss *SqlStore) ensureAdminUser() error { systemUserCountQuery := m.GetSystemUserCountStatsQuery{} err := bus.InTransaction(context.Background(), func(ctx context.Context) error { - return bus.DispatchCtx(ctx, &systemUserCountQuery) + + err := bus.DispatchCtx(ctx, &systemUserCountQuery) + if err != nil { + return fmt.Errorf("Could not determine if admin user exists: %v", err) + } + + if systemUserCountQuery.Result.Count > 0 { + return nil + } + + cmd := m.CreateUserCommand{} + cmd.Login = setting.AdminUser + cmd.Email = setting.AdminUser + "@localhost" + cmd.Password = setting.AdminPassword + cmd.IsAdmin = true + + if err := bus.DispatchCtx(ctx, &cmd); err != nil { + return fmt.Errorf("Failed to create admin user: %v", err) + } + + ss.log.Info("Created default admin", "user", setting.AdminUser) + + return nil }) - if err != nil { - return fmt.Errorf("Could not determine if admin user exists: %v", err) - } - - if systemUserCountQuery.Result.Count > 0 { - return nil - } - - cmd := m.CreateUserCommand{} - cmd.Login = setting.AdminUser - cmd.Email = setting.AdminUser + "@localhost" - cmd.Password = setting.AdminPassword - cmd.IsAdmin = true - - if err := bus.Dispatch(&cmd); err != nil { - return fmt.Errorf("Failed to create admin user: %v", err) - } - - ss.log.Info("Created default admin user: %v", setting.AdminUser) - - return nil + return err } func (ss *SqlStore) buildConnectionString() (string, error) { diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index e7aa8da837a..befc3a08401 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "strconv" "strings" "time" @@ -30,6 +31,8 @@ func init() { bus.AddHandler("sql", DeleteUser) bus.AddHandler("sql", UpdateUserPermissions) bus.AddHandler("sql", SetUserHelpFlag) + + bus.AddCtxHandler("sql", CreateUserCtx) } func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error) { @@ -79,77 +82,87 @@ func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error return org.Id, nil } +func internalCreateUser(sess *DBSession, cmd *m.CreateUserCommand) error { + orgId, err := getOrgIdForNewUser(cmd, sess) + if err != nil { + return err + } + + if cmd.Email == "" { + cmd.Email = cmd.Login + } + + // create user + user := m.User{ + Email: cmd.Email, + Name: cmd.Name, + Login: cmd.Login, + Company: cmd.Company, + IsAdmin: cmd.IsAdmin, + OrgId: orgId, + EmailVerified: cmd.EmailVerified, + Created: time.Now(), + Updated: time.Now(), + LastSeenAt: time.Now().AddDate(-10, 0, 0), + } + + if len(cmd.Password) > 0 { + user.Salt = util.GetRandomString(10) + user.Rands = util.GetRandomString(10) + user.Password = util.EncodePassword(cmd.Password, user.Salt) + } + + sess.UseBool("is_admin") + + if _, err := sess.Insert(&user); err != nil { + return err + } + + sess.publishAfterCommit(&events.UserCreated{ + Timestamp: user.Created, + Id: user.Id, + Name: user.Name, + Login: user.Login, + Email: user.Email, + }) + + cmd.Result = user + + // create org user link + if !cmd.SkipOrgSetup { + orgUser := m.OrgUser{ + OrgId: orgId, + UserId: user.Id, + Role: m.ROLE_ADMIN, + Created: time.Now(), + Updated: time.Now(), + } + + if setting.AutoAssignOrg && !user.IsAdmin { + if len(cmd.DefaultOrgRole) > 0 { + orgUser.Role = m.RoleType(cmd.DefaultOrgRole) + } else { + orgUser.Role = m.RoleType(setting.AutoAssignOrgRole) + } + } + + if _, err = sess.Insert(&orgUser); err != nil { + return err + } + } + + return nil +} + +func CreateUserCtx(ctx context.Context, cmd *m.CreateUserCommand) error { + return inTransactionWithRetryCtx(ctx, func(sess *DBSession) error { + return internalCreateUser(sess, cmd) + }, 0) +} + func CreateUser(cmd *m.CreateUserCommand) error { return inTransaction(func(sess *DBSession) error { - orgId, err := getOrgIdForNewUser(cmd, sess) - if err != nil { - return err - } - - if cmd.Email == "" { - cmd.Email = cmd.Login - } - - // create user - user := m.User{ - Email: cmd.Email, - Name: cmd.Name, - Login: cmd.Login, - Company: cmd.Company, - IsAdmin: cmd.IsAdmin, - OrgId: orgId, - EmailVerified: cmd.EmailVerified, - Created: time.Now(), - Updated: time.Now(), - LastSeenAt: time.Now().AddDate(-10, 0, 0), - } - - if len(cmd.Password) > 0 { - user.Salt = util.GetRandomString(10) - user.Rands = util.GetRandomString(10) - user.Password = util.EncodePassword(cmd.Password, user.Salt) - } - - sess.UseBool("is_admin") - - if _, err := sess.Insert(&user); err != nil { - return err - } - - sess.publishAfterCommit(&events.UserCreated{ - Timestamp: user.Created, - Id: user.Id, - Name: user.Name, - Login: user.Login, - Email: user.Email, - }) - - cmd.Result = user - - // create org user link - if !cmd.SkipOrgSetup { - orgUser := m.OrgUser{ - OrgId: orgId, - UserId: user.Id, - Role: m.ROLE_ADMIN, - Created: time.Now(), - Updated: time.Now(), - } - - if setting.AutoAssignOrg && !user.IsAdmin { - if len(cmd.DefaultOrgRole) > 0 { - orgUser.Role = m.RoleType(cmd.DefaultOrgRole) - } else { - orgUser.Role = m.RoleType(setting.AutoAssignOrgRole) - } - } - - if _, err = sess.Insert(&orgUser); err != nil { - return err - } - } - - return nil + return internalCreateUser(sess, cmd) }) } From 6775a82c82be88394949cc3f1a417cad4cfd6c10 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 7 Jun 2018 18:22:06 +0200 Subject: [PATCH 13/84] fixes typo in code --- pkg/bus/bus.go | 6 +++--- pkg/services/sqlstore/sqlstore.go | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 0f10dfd9b17..2972c9c7614 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -13,7 +13,7 @@ type Msg interface{} var ErrHandlerNotFound = errors.New("handler not found") type TransactionWrapper interface { - Wrapp(ctx context.Context, fn func(ctx context.Context) error) error + Wrap(ctx context.Context, fn func(ctx context.Context) error) error } type Bus interface { @@ -39,7 +39,7 @@ type Bus interface { } func (b *InProcBus) InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { - return b.transactionWrapper.Wrapp(ctx, fn) + return b.transactionWrapper.Wrap(ctx, fn) } type InProcBus struct { @@ -213,6 +213,6 @@ func ClearBusHandlers() { type noopTransactionManager struct{} -func (*noopTransactionManager) Wrapp(ctx context.Context, fn func(ctx context.Context) error) error { +func (*noopTransactionManager) Wrap(ctx context.Context, fn func(ctx context.Context) error) error { return nil } diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index a401e8c8c83..a36e6cb15d1 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -99,11 +99,11 @@ type SQLTransactionManager struct { engine *xorm.Engine } -func (stm *SQLTransactionManager) Wrapp(ctx context.Context, fn func(ctx context.Context) error) error { - return stm.wrappInternal(ctx, fn, 0) +func (stm *SQLTransactionManager) Wrap(ctx context.Context, fn func(ctx context.Context) error) error { + return stm.wrapInternal(ctx, fn, 0) } -func (stm *SQLTransactionManager) wrappInternal(ctx context.Context, fn func(ctx context.Context) error, retry int) error { +func (stm *SQLTransactionManager) wrapInternal(ctx context.Context, fn func(ctx context.Context) error, retry int) error { sess := startSession(ctx) defer sess.Close() @@ -117,14 +117,16 @@ func (stm *SQLTransactionManager) wrappInternal(ctx context.Context, fn func(ctx sess.Rollback() time.Sleep(time.Millisecond * time.Duration(10)) sqlog.Info("Database table locked, sleeping then retrying", "retry", retry) - return stm.wrappInternal(ctx, fn, retry+1) + return stm.wrapInternal(ctx, fn, retry+1) } } if err != nil { sess.Rollback() return err - } else if err = sess.Commit(); err != nil { + } + + if err = sess.Commit(); err != nil { return err } From 442e0e437b0384c5a25595ef6dfc8cb136b07bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 7 Jun 2018 12:54:36 -0700 Subject: [PATCH 14/84] refactoring: transaction manager PR #12203 --- pkg/bus/bus.go | 24 +++--- pkg/models/transaction.go | 7 ++ pkg/services/sqlstore/session.go | 63 +++++++++++++++ pkg/services/sqlstore/sqlstore.go | 58 +------------ .../sqlstore/{shared.go => transactions.go} | 81 ++++++++----------- pkg/services/sqlstore/user.go | 1 - 6 files changed, 116 insertions(+), 118 deletions(-) create mode 100644 pkg/models/transaction.go create mode 100644 pkg/services/sqlstore/session.go rename pkg/services/sqlstore/{shared.go => transactions.go} (56%) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 2972c9c7614..18248d6667e 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -12,8 +12,8 @@ type Msg interface{} var ErrHandlerNotFound = errors.New("handler not found") -type TransactionWrapper interface { - Wrap(ctx context.Context, fn func(ctx context.Context) error) error +type TransactionManager interface { + InTransaction(ctx context.Context, fn func(ctx context.Context) error) error } type Bus interface { @@ -35,19 +35,18 @@ type Bus interface { // SetTransactionManager allows the user to replace the internal // noop TransactionManager that is responsible for manageing // transactions in `InTransaction` - SetTransactionManager(tm TransactionWrapper) + SetTransactionManager(tm TransactionManager) } func (b *InProcBus) InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { - return b.transactionWrapper.Wrap(ctx, fn) + return b.txMng.InTransaction(ctx, fn) } type InProcBus struct { handlers map[string]HandlerFunc listeners map[string][]HandlerFunc wildcardListeners []HandlerFunc - - transactionWrapper TransactionWrapper + txMng TransactionManager } // temp stuff, not sure how to handle bus instance, and init yet @@ -58,8 +57,7 @@ func New() Bus { bus.handlers = make(map[string]HandlerFunc) bus.listeners = make(map[string][]HandlerFunc) bus.wildcardListeners = make([]HandlerFunc, 0) - - bus.transactionWrapper = &noopTransactionManager{} + bus.txMng = &noopTransactionManager{} return bus } @@ -69,12 +67,8 @@ func GetBus() Bus { return globalBus } -func SetTransactionManager(tm TransactionWrapper) { - globalBus.SetTransactionManager(tm) -} - -func (b *InProcBus) SetTransactionManager(tm TransactionWrapper) { - b.transactionWrapper = tm +func (b *InProcBus) SetTransactionManager(tm TransactionManager) { + b.txMng = tm } func (b *InProcBus) DispatchCtx(ctx context.Context, msg Msg) error { @@ -213,6 +207,6 @@ func ClearBusHandlers() { type noopTransactionManager struct{} -func (*noopTransactionManager) Wrap(ctx context.Context, fn func(ctx context.Context) error) error { +func (*noopTransactionManager) InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { return nil } diff --git a/pkg/models/transaction.go b/pkg/models/transaction.go new file mode 100644 index 00000000000..e07b2a9e397 --- /dev/null +++ b/pkg/models/transaction.go @@ -0,0 +1,7 @@ +package models + +import "context" + +type TransactionManager interface { + InTransaction(ctx context.Context, fn func(ctx context.Context) error) error +} diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go new file mode 100644 index 00000000000..307d3ee1eeb --- /dev/null +++ b/pkg/services/sqlstore/session.go @@ -0,0 +1,63 @@ +package sqlstore + +import ( + "context" + "reflect" + + "github.com/go-xorm/xorm" +) + +type DBSession struct { + *xorm.Session + events []interface{} +} + +type dbTransactionFunc func(sess *DBSession) error + +func (sess *DBSession) publishAfterCommit(msg interface{}) { + sess.events = append(sess.events, msg) +} + +func newSession() *DBSession { + return &DBSession{Session: x.NewSession()} +} + +func startSession(ctx context.Context) *DBSession { + value := ctx.Value(ContextSessionName) + var sess *DBSession + sess, ok := value.(*DBSession) + + if !ok { + newSess := newSession() + newSess.Begin() + return newSess + } + + return sess +} + +func withDbSession(ctx context.Context, callback dbTransactionFunc) error { + sess := startSession(ctx) + + return callback(sess) +} + +func (sess *DBSession) InsertId(bean interface{}) (int64, error) { + table := sess.DB().Mapper.Obj2Table(getTypeName(bean)) + + dialect.PreInsertId(table, sess.Session) + + id, err := sess.Session.InsertOne(bean) + + dialect.PostInsertId(table, sess.Session) + + return id, err +} + +func getTypeName(bean interface{}) (res string) { + t := reflect.TypeOf(bean) + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + return t.Name() +} diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index a36e6cb15d1..f97134fd0d5 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -23,11 +23,10 @@ import ( "github.com/go-sql-driver/mysql" "github.com/go-xorm/xorm" - _ "github.com/lib/pq" - _ "github.com/mattn/go-sqlite3" - sqlite3 "github.com/mattn/go-sqlite3" _ "github.com/grafana/grafana/pkg/tsdb/mssql" + _ "github.com/lib/pq" + _ "github.com/mattn/go-sqlite3" ) var ( @@ -82,9 +81,7 @@ func (ss *SqlStore) Init() error { // Init repo instances annotations.SetRepository(&SqlAnnotationRepo{}) - ss.Bus.SetTransactionManager(&SQLTransactionManager{ - engine: ss.engine, - }) + ss.Bus.SetTransactionManager(ss) // ensure admin user if ss.skipEnsureAdmin { @@ -94,57 +91,10 @@ func (ss *SqlStore) Init() error { return ss.ensureAdminUser() } -// SQLTransactionManager begin/end transaction -type SQLTransactionManager struct { - engine *xorm.Engine -} - -func (stm *SQLTransactionManager) Wrap(ctx context.Context, fn func(ctx context.Context) error) error { - return stm.wrapInternal(ctx, fn, 0) -} - -func (stm *SQLTransactionManager) wrapInternal(ctx context.Context, fn func(ctx context.Context) error, retry int) error { - sess := startSession(ctx) - defer sess.Close() - - withValue := context.WithValue(ctx, ContextSessionName, sess) - - err := fn(withValue) - - // special handling of database locked errors for sqlite, then we can retry 3 times - if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 { - if sqlError.Code == sqlite3.ErrLocked { - sess.Rollback() - time.Sleep(time.Millisecond * time.Duration(10)) - sqlog.Info("Database table locked, sleeping then retrying", "retry", retry) - return stm.wrapInternal(ctx, fn, retry+1) - } - } - - if err != nil { - sess.Rollback() - return err - } - - if err = sess.Commit(); err != nil { - return err - } - - if len(sess.events) > 0 { - for _, e := range sess.events { - if err = bus.Publish(e); err != nil { - log.Error(3, "Failed to publish event after commit", err) - } - } - } - - return nil -} - func (ss *SqlStore) ensureAdminUser() error { systemUserCountQuery := m.GetSystemUserCountStatsQuery{} - err := bus.InTransaction(context.Background(), func(ctx context.Context) error { + err := ss.InTransaction(context.Background(), func(ctx context.Context) error { err := bus.DispatchCtx(ctx, &systemUserCountQuery) if err != nil { diff --git a/pkg/services/sqlstore/shared.go b/pkg/services/sqlstore/transactions.go similarity index 56% rename from pkg/services/sqlstore/shared.go rename to pkg/services/sqlstore/transactions.go index 7928f22b9f3..959d21c0bf1 100644 --- a/pkg/services/sqlstore/shared.go +++ b/pkg/services/sqlstore/transactions.go @@ -2,52 +2,53 @@ package sqlstore import ( "context" - "reflect" "time" - "github.com/go-xorm/xorm" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/log" sqlite3 "github.com/mattn/go-sqlite3" ) -type DBSession struct { - *xorm.Session - events []interface{} +func (ss *SqlStore) InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { + return ss.inTransactionWithRetry(ctx, fn, 0) } -type dbTransactionFunc func(sess *DBSession) error +func (ss *SqlStore) inTransactionWithRetry(ctx context.Context, fn func(ctx context.Context) error, retry int) error { + sess := startSession(ctx) + defer sess.Close() -func (sess *DBSession) publishAfterCommit(msg interface{}) { - sess.events = append(sess.events, msg) -} + withValue := context.WithValue(ctx, ContextSessionName, sess) -func newSession() *DBSession { - return &DBSession{Session: x.NewSession()} -} + err := fn(withValue) -func inTransaction(callback dbTransactionFunc) error { - return inTransactionWithRetry(callback, 0) -} - -func startSession(ctx context.Context) *DBSession { - value := ctx.Value(ContextSessionName) - var sess *DBSession - sess, ok := value.(*DBSession) - - if !ok { - newSess := newSession() - newSess.Begin() - return newSess + // special handling of database locked errors for sqlite, then we can retry 3 times + if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 { + if sqlError.Code == sqlite3.ErrLocked { + sess.Rollback() + time.Sleep(time.Millisecond * time.Duration(10)) + ss.log.Info("Database table locked, sleeping then retrying", "retry", retry) + return ss.inTransactionWithRetry(ctx, fn, retry+1) + } } - return sess -} + if err != nil { + sess.Rollback() + return err + } -func withDbSession(ctx context.Context, callback dbTransactionFunc) error { - sess := startSession(ctx) + if err = sess.Commit(); err != nil { + return err + } - return callback(sess) + if len(sess.events) > 0 { + for _, e := range sess.events { + if err = bus.Publish(e); err != nil { + ss.log.Error("Failed to publish event after commit", err) + } + } + } + + return nil } func inTransactionWithRetry(callback dbTransactionFunc, retry int) error { @@ -94,22 +95,6 @@ func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, return nil } -func (sess *DBSession) InsertId(bean interface{}) (int64, error) { - table := sess.DB().Mapper.Obj2Table(getTypeName(bean)) - - dialect.PreInsertId(table, sess.Session) - - id, err := sess.Session.InsertOne(bean) - - dialect.PostInsertId(table, sess.Session) - - return id, err -} - -func getTypeName(bean interface{}) (res string) { - t := reflect.TypeOf(bean) - for t.Kind() == reflect.Ptr { - t = t.Elem() - } - return t.Name() +func inTransaction(callback dbTransactionFunc) error { + return inTransactionWithRetry(callback, 0) } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index befc3a08401..f01cb84ad4f 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -31,7 +31,6 @@ func init() { bus.AddHandler("sql", DeleteUser) bus.AddHandler("sql", UpdateUserPermissions) bus.AddHandler("sql", SetUserHelpFlag) - bus.AddCtxHandler("sql", CreateUserCtx) } From 5af0b924ff47cdd5b5e1ef414d9dbad28c638913 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 7 Jun 2018 13:03:27 -0700 Subject: [PATCH 15/84] refactoring: renamed AddCtxHandler to AddHandlerCtx PR #12203 --- pkg/bus/bus.go | 8 ++++---- pkg/services/notifications/notifications.go | 4 ++-- pkg/services/sqlstore/stats.go | 2 +- pkg/services/sqlstore/user.go | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 18248d6667e..a7d580ef2b1 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -28,7 +28,7 @@ type Bus interface { InTransaction(ctx context.Context, fn func(ctx context.Context) error) error AddHandler(handler HandlerFunc) - AddCtxHandler(handler HandlerFunc) + AddHandlerCtx(handler HandlerFunc) AddEventListener(handler HandlerFunc) AddWildcardListener(handler HandlerFunc) @@ -146,7 +146,7 @@ func (b *InProcBus) AddHandler(handler HandlerFunc) { b.handlers[queryTypeName] = handler } -func (b *InProcBus) AddCtxHandler(handler HandlerFunc) { +func (b *InProcBus) AddHandlerCtx(handler HandlerFunc) { handlerType := reflect.TypeOf(handler) queryTypeName := handlerType.In(1).Elem().Name() b.handlers[queryTypeName] = handler @@ -168,8 +168,8 @@ func AddHandler(implName string, handler HandlerFunc) { } // Package level functions -func AddCtxHandler(implName string, handler HandlerFunc) { - globalBus.AddCtxHandler(handler) +func AddHandlerCtx(implName string, handler HandlerFunc) { + globalBus.AddHandlerCtx(handler) } // Package level functions diff --git a/pkg/services/notifications/notifications.go b/pkg/services/notifications/notifications.go index 14d362c5e1e..fcefa91243d 100644 --- a/pkg/services/notifications/notifications.go +++ b/pkg/services/notifications/notifications.go @@ -45,8 +45,8 @@ func (ns *NotificationService) Init() error { ns.Bus.AddHandler(ns.validateResetPasswordCode) ns.Bus.AddHandler(ns.sendEmailCommandHandler) - ns.Bus.AddCtxHandler(ns.sendEmailCommandHandlerSync) - ns.Bus.AddCtxHandler(ns.SendWebhookSync) + ns.Bus.AddHandlerCtx(ns.sendEmailCommandHandlerSync) + ns.Bus.AddHandlerCtx(ns.SendWebhookSync) ns.Bus.AddEventListener(ns.signUpStartedHandler) ns.Bus.AddEventListener(ns.signUpCompletedHandler) diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index af4482d9e25..ef0f148bb2a 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -14,7 +14,7 @@ func init() { bus.AddHandler("sql", GetDataSourceAccessStats) bus.AddHandler("sql", GetAdminStats) bus.AddHandler("sql", GetSystemUserCountStats) - bus.AddCtxHandler("sql", GetSystemUserCountStatsCtx) + bus.AddHandlerCtx("sql", GetSystemUserCountStatsCtx) } var activeUserTimeLimit = time.Hour * 24 * 30 diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index f01cb84ad4f..252499d5fdc 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -31,7 +31,7 @@ func init() { bus.AddHandler("sql", DeleteUser) bus.AddHandler("sql", UpdateUserPermissions) bus.AddHandler("sql", SetUserHelpFlag) - bus.AddCtxHandler("sql", CreateUserCtx) + bus.AddHandlerCtx("sql", CreateUserCtx) } func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error) { From e2275701d8a508454395dade814381b2e4f659ba Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 8 Jun 2018 10:51:27 +0200 Subject: [PATCH 16/84] bus: DispatchCtx can now invoke any handler --- pkg/api/dashboard.go | 2 +- pkg/bus/bus.go | 24 +++++++++++++++---- pkg/bus/bus_test.go | 57 +++++++++++++++++++++++++++++++++++++------- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index c2ab6dd9a1a..40d855773d0 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -103,7 +103,7 @@ func GetDashboard(c *m.ReqContext) Response { } isDashboardProvisioned := &m.IsDashboardProvisionedQuery{DashboardId: dash.Id} - err = bus.Dispatch(isDashboardProvisioned) + err = bus.DispatchCtx(c.Req.Context(), isDashboardProvisioned) if err != nil { return Error(500, "Error while checking if dashboard is provisioned", err) } diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index a7d580ef2b1..69f0b95e195 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -44,6 +44,7 @@ func (b *InProcBus) InTransaction(ctx context.Context, fn func(ctx context.Conte type InProcBus struct { handlers map[string]HandlerFunc + handlersWithCtx map[string]HandlerFunc listeners map[string][]HandlerFunc wildcardListeners []HandlerFunc txMng TransactionManager @@ -55,6 +56,7 @@ var globalBus = New() func New() Bus { bus := &InProcBus{} bus.handlers = make(map[string]HandlerFunc) + bus.handlersWithCtx = make(map[string]HandlerFunc) bus.listeners = make(map[string][]HandlerFunc) bus.wildcardListeners = make([]HandlerFunc, 0) bus.txMng = &noopTransactionManager{} @@ -74,14 +76,26 @@ func (b *InProcBus) SetTransactionManager(tm TransactionManager) { func (b *InProcBus) DispatchCtx(ctx context.Context, msg Msg) error { var msgName = reflect.TypeOf(msg).Elem().Name() - var handler = b.handlers[msgName] + // we prefer to use the handler that support context.Context + var handler = b.handlersWithCtx[msgName] + var withCtx = true + + // fallback to use classic handlers + if handler == nil { + withCtx = false + handler = b.handlers[msgName] + } + if handler == nil { return ErrHandlerNotFound } - var params = make([]reflect.Value, 2) - params[0] = reflect.ValueOf(ctx) - params[1] = reflect.ValueOf(msg) + var params = []reflect.Value{} + if withCtx { + params = append(params, reflect.ValueOf(ctx)) + } + + params = append(params, reflect.ValueOf(msg)) ret := reflect.ValueOf(handler).Call(params) err := ret[0].Interface() @@ -149,7 +163,7 @@ func (b *InProcBus) AddHandler(handler HandlerFunc) { func (b *InProcBus) AddHandlerCtx(handler HandlerFunc) { handlerType := reflect.TypeOf(handler) queryTypeName := handlerType.In(1).Elem().Name() - b.handlers[queryTypeName] = handler + b.handlersWithCtx[queryTypeName] = handler } func (b *InProcBus) AddEventListener(handler HandlerFunc) { diff --git a/pkg/bus/bus_test.go b/pkg/bus/bus_test.go index 62e72f18308..4c061900718 100644 --- a/pkg/bus/bus_test.go +++ b/pkg/bus/bus_test.go @@ -1,24 +1,65 @@ package bus import ( + "context" "errors" "fmt" "testing" ) -type TestQuery struct { +type testQuery struct { Id int64 Resp string } +func TestDispatchCtxCanUseNormalHandlers(t *testing.T) { + bus := New() + + handlerWithCtxCallCount := 0 + handlerCallCount := 0 + + handlerWithCtx := func(ctx context.Context, query *testQuery) error { + handlerWithCtxCallCount++ + return nil + } + + handler := func(query *testQuery) error { + handlerCallCount++ + return nil + } + + err := bus.DispatchCtx(context.Background(), &testQuery{}) + if err != ErrHandlerNotFound { + t.Errorf("expected bus to return HandlerNotFound is no handler is registered") + } + + t.Run("when a normal handler is registered", func(t *testing.T) { + bus.AddHandler(handler) + bus.DispatchCtx(context.Background(), &testQuery{}) + + if handlerCallCount != 1 { + t.Errorf("Expected normal handler to be called once") + } + + t.Run("when a ctx handler is registered", func(t *testing.T) { + bus.AddHandlerCtx(handlerWithCtx) + bus.DispatchCtx(context.Background(), &testQuery{}) + + if handlerWithCtxCallCount != 1 { + t.Errorf("Expected ctx handler to be called once") + } + }) + }) +} + func TestQueryHandlerReturnsError(t *testing.T) { bus := New() - bus.AddHandler(func(query *TestQuery) error { + bus.AddHandler(func(query *testQuery) error { return errors.New("handler error") }) - err := bus.Dispatch(&TestQuery{}) + err := bus.Dispatch(&testQuery{}) if err == nil { t.Fatal("Send query failed " + err.Error()) @@ -30,12 +71,12 @@ func TestQueryHandlerReturnsError(t *testing.T) { func TestQueryHandlerReturn(t *testing.T) { bus := New() - bus.AddHandler(func(q *TestQuery) error { + bus.AddHandler(func(q *testQuery) error { q.Resp = "hello from handler" return nil }) - query := &TestQuery{} + query := &testQuery{} err := bus.Dispatch(query) if err != nil { @@ -49,17 +90,17 @@ func TestEventListeners(t *testing.T) { bus := New() count := 0 - bus.AddEventListener(func(query *TestQuery) error { + bus.AddEventListener(func(query *testQuery) error { count += 1 return nil }) - bus.AddEventListener(func(query *TestQuery) error { + bus.AddEventListener(func(query *testQuery) error { count += 10 return nil }) - err := bus.Publish(&TestQuery{}) + err := bus.Publish(&testQuery{}) if err != nil { t.Fatal("Publish event failed " + err.Error()) From 629eab0b1edca6ca629077365832953e6e547dc9 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sun, 10 Jun 2018 23:17:18 +0200 Subject: [PATCH 17/84] bus: dont mix ctx/classic handlers --- pkg/api/dashboard.go | 2 +- pkg/bus/bus.go | 14 +------------- pkg/bus/bus_test.go | 21 +++++++++++---------- pkg/services/sqlstore/stats.go | 18 ++---------------- pkg/services/sqlstore/stats_test.go | 3 ++- 5 files changed, 17 insertions(+), 41 deletions(-) diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 40d855773d0..c2ab6dd9a1a 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -103,7 +103,7 @@ func GetDashboard(c *m.ReqContext) Response { } isDashboardProvisioned := &m.IsDashboardProvisionedQuery{DashboardId: dash.Id} - err = bus.DispatchCtx(c.Req.Context(), isDashboardProvisioned) + err = bus.Dispatch(isDashboardProvisioned) if err != nil { return Error(500, "Error while checking if dashboard is provisioned", err) } diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 69f0b95e195..1259211cddc 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -76,25 +76,13 @@ func (b *InProcBus) SetTransactionManager(tm TransactionManager) { func (b *InProcBus) DispatchCtx(ctx context.Context, msg Msg) error { var msgName = reflect.TypeOf(msg).Elem().Name() - // we prefer to use the handler that support context.Context var handler = b.handlersWithCtx[msgName] - var withCtx = true - - // fallback to use classic handlers - if handler == nil { - withCtx = false - handler = b.handlers[msgName] - } - if handler == nil { return ErrHandlerNotFound } var params = []reflect.Value{} - if withCtx { - params = append(params, reflect.ValueOf(ctx)) - } - + params = append(params, reflect.ValueOf(ctx)) params = append(params, reflect.ValueOf(msg)) ret := reflect.ValueOf(handler).Call(params) diff --git a/pkg/bus/bus_test.go b/pkg/bus/bus_test.go index 4c061900718..83a0d7190ea 100644 --- a/pkg/bus/bus_test.go +++ b/pkg/bus/bus_test.go @@ -33,22 +33,23 @@ func TestDispatchCtxCanUseNormalHandlers(t *testing.T) { t.Errorf("expected bus to return HandlerNotFound is no handler is registered") } + bus.AddHandler(handler) + bus.AddHandlerCtx(handlerWithCtx) + t.Run("when a normal handler is registered", func(t *testing.T) { - bus.AddHandler(handler) - bus.DispatchCtx(context.Background(), &testQuery{}) + bus.Dispatch(&testQuery{}) if handlerCallCount != 1 { - t.Errorf("Expected normal handler to be called once") + t.Errorf("Expected normal handler to be called 1 time. was called %d", handlerCallCount) } + }) - t.Run("when a ctx handler is registered", func(t *testing.T) { - bus.AddHandlerCtx(handlerWithCtx) - bus.DispatchCtx(context.Background(), &testQuery{}) + t.Run("when a ctx handler is registered", func(t *testing.T) { + bus.DispatchCtx(context.Background(), &testQuery{}) - if handlerWithCtxCallCount != 1 { - t.Errorf("Expected ctx handler to be called once") - } - }) + if handlerWithCtxCallCount != 1 { + t.Errorf("Expected ctx handler to be called 1 time. was called %d", handlerWithCtxCallCount) + } }) } diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index ef0f148bb2a..6db481bf06b 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -13,8 +13,7 @@ func init() { bus.AddHandler("sql", GetDataSourceStats) bus.AddHandler("sql", GetDataSourceAccessStats) bus.AddHandler("sql", GetAdminStats) - bus.AddHandler("sql", GetSystemUserCountStats) - bus.AddHandlerCtx("sql", GetSystemUserCountStatsCtx) + bus.AddHandlerCtx("sql", GetSystemUserCountStats) } var activeUserTimeLimit = time.Hour * 24 * 30 @@ -135,7 +134,7 @@ func GetAdminStats(query *m.GetAdminStatsQuery) error { return err } -func GetSystemUserCountStatsCtx(ctx context.Context, query *m.GetSystemUserCountStatsQuery) error { +func GetSystemUserCountStats(ctx context.Context, query *m.GetSystemUserCountStatsQuery) error { return withDbSession(ctx, func(sess *DBSession) error { var rawSql = `SELECT COUNT(id) AS Count FROM ` + dialect.Quote("user") @@ -150,16 +149,3 @@ func GetSystemUserCountStatsCtx(ctx context.Context, query *m.GetSystemUserCount return err }) } - -func GetSystemUserCountStats(query *m.GetSystemUserCountStatsQuery) error { - var rawSql = `SELECT COUNT(id) AS Count FROM ` + dialect.Quote("user") - var stats m.SystemUserCountStats - _, err := x.SQL(rawSql).Get(&stats) - if err != nil { - return err - } - - query.Result = &stats - - return err -} diff --git a/pkg/services/sqlstore/stats_test.go b/pkg/services/sqlstore/stats_test.go index 97f0ca0c43e..dae24952d17 100644 --- a/pkg/services/sqlstore/stats_test.go +++ b/pkg/services/sqlstore/stats_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "testing" m "github.com/grafana/grafana/pkg/models" @@ -20,7 +21,7 @@ func TestStatsDataAccess(t *testing.T) { Convey("Get system user count stats should not results in error", func() { query := m.GetSystemUserCountStatsQuery{} - err := GetSystemUserCountStats(&query) + err := GetSystemUserCountStats(context.Background(), &query) So(err, ShouldBeNil) }) From 9ca9a7c30299a5959056ea40d110ac50bd80d06b Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 12 Jun 2018 22:58:03 +0200 Subject: [PATCH 18/84] bus: dont start transaction when creating session --- pkg/services/sqlstore/session.go | 1 - pkg/services/sqlstore/transactions.go | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go index 307d3ee1eeb..fdee2c76b0c 100644 --- a/pkg/services/sqlstore/session.go +++ b/pkg/services/sqlstore/session.go @@ -29,7 +29,6 @@ func startSession(ctx context.Context) *DBSession { if !ok { newSess := newSession() - newSess.Begin() return newSess } diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go index 959d21c0bf1..f72b0bb8500 100644 --- a/pkg/services/sqlstore/transactions.go +++ b/pkg/services/sqlstore/transactions.go @@ -17,6 +17,10 @@ func (ss *SqlStore) inTransactionWithRetry(ctx context.Context, fn func(ctx cont sess := startSession(ctx) defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + withValue := context.WithValue(ctx, ContextSessionName, sess) err := fn(withValue) @@ -59,6 +63,7 @@ func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, var err error sess := startSession(ctx) + defer sess.Close() if err = sess.Begin(); err != nil { From 9c1758b5931f9d55ee3080d943018aee3adbb0e0 Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 13 Jun 2018 09:18:53 +0200 Subject: [PATCH 19/84] bus: Dispatch now passes empty ctx if handler require it --- pkg/bus/bus.go | 16 +++++++++++++--- pkg/bus/bus_test.go | 17 +++++++++-------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 1259211cddc..9cd3f116172 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -96,13 +96,23 @@ func (b *InProcBus) DispatchCtx(ctx context.Context, msg Msg) error { func (b *InProcBus) Dispatch(msg Msg) error { var msgName = reflect.TypeOf(msg).Elem().Name() - var handler = b.handlers[msgName] + var handler = b.handlersWithCtx[msgName] + withCtx := true + + if handler == nil { + withCtx = false + handler = b.handlers[msgName] + } + if handler == nil { return ErrHandlerNotFound } - var params = make([]reflect.Value, 1) - params[0] = reflect.ValueOf(msg) + var params = []reflect.Value{} + if withCtx { + params = append(params, reflect.ValueOf(context.Background())) + } + params = append(params, reflect.ValueOf(msg)) ret := reflect.ValueOf(handler).Call(params) err := ret[0].Interface() diff --git a/pkg/bus/bus_test.go b/pkg/bus/bus_test.go index 83a0d7190ea..9f41a5154df 100644 --- a/pkg/bus/bus_test.go +++ b/pkg/bus/bus_test.go @@ -34,7 +34,6 @@ func TestDispatchCtxCanUseNormalHandlers(t *testing.T) { } bus.AddHandler(handler) - bus.AddHandlerCtx(handlerWithCtx) t.Run("when a normal handler is registered", func(t *testing.T) { bus.Dispatch(&testQuery{}) @@ -42,15 +41,17 @@ func TestDispatchCtxCanUseNormalHandlers(t *testing.T) { if handlerCallCount != 1 { t.Errorf("Expected normal handler to be called 1 time. was called %d", handlerCallCount) } + + t.Run("when a ctx handler is registered", func(t *testing.T) { + bus.AddHandlerCtx(handlerWithCtx) + bus.Dispatch(&testQuery{}) + + if handlerWithCtxCallCount != 1 { + t.Errorf("Expected ctx handler to be called 1 time. was called %d", handlerWithCtxCallCount) + } + }) }) - t.Run("when a ctx handler is registered", func(t *testing.T) { - bus.DispatchCtx(context.Background(), &testQuery{}) - - if handlerWithCtxCallCount != 1 { - t.Errorf("Expected ctx handler to be called 1 time. was called %d", handlerWithCtxCallCount) - } - }) } func TestQueryHandlerReturnsError(t *testing.T) { From a3ee778ddbfb98a79ce970ec1dc49c4ea66a1adb Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Jun 2018 09:57:49 +0200 Subject: [PATCH 20/84] removes unused code --- pkg/models/transaction.go | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 pkg/models/transaction.go diff --git a/pkg/models/transaction.go b/pkg/models/transaction.go deleted file mode 100644 index e07b2a9e397..00000000000 --- a/pkg/models/transaction.go +++ /dev/null @@ -1,7 +0,0 @@ -package models - -import "context" - -type TransactionManager interface { - InTransaction(ctx context.Context, fn func(ctx context.Context) error) error -} From 03dae10e796cb8f412c386861e634d76555fba18 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Jun 2018 09:59:52 +0200 Subject: [PATCH 21/84] bus: noop should still execute fn --- pkg/bus/bus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/bus/bus.go b/pkg/bus/bus.go index 9cd3f116172..9cf930aeb82 100644 --- a/pkg/bus/bus.go +++ b/pkg/bus/bus.go @@ -220,5 +220,5 @@ func ClearBusHandlers() { type noopTransactionManager struct{} func (*noopTransactionManager) InTransaction(ctx context.Context, fn func(ctx context.Context) error) error { - return nil + return fn(ctx) } From 09e71e00a36f9a487df69facca139aab160d7f52 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 14 Jun 2018 19:07:33 +0200 Subject: [PATCH 22/84] sql: adds tests for InTransaction --- pkg/services/sqlstore/apikey.go | 7 ++- pkg/services/sqlstore/sqlstore.go | 1 + pkg/services/sqlstore/transactions_test.go | 64 ++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 pkg/services/sqlstore/transactions_test.go diff --git a/pkg/services/sqlstore/apikey.go b/pkg/services/sqlstore/apikey.go index 9d41b5c809e..775d4cf6447 100644 --- a/pkg/services/sqlstore/apikey.go +++ b/pkg/services/sqlstore/apikey.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "time" "github.com/grafana/grafana/pkg/bus" @@ -11,7 +12,7 @@ func init() { bus.AddHandler("sql", GetApiKeys) bus.AddHandler("sql", GetApiKeyById) bus.AddHandler("sql", GetApiKeyByName) - bus.AddHandler("sql", DeleteApiKey) + bus.AddHandlerCtx("sql", DeleteApiKeyCtx) bus.AddHandler("sql", AddApiKey) } @@ -22,8 +23,8 @@ func GetApiKeys(query *m.GetApiKeysQuery) error { return sess.Find(&query.Result) } -func DeleteApiKey(cmd *m.DeleteApiKeyCommand) error { - return inTransaction(func(sess *DBSession) error { +func DeleteApiKeyCtx(ctx context.Context, cmd *m.DeleteApiKeyCommand) error { + return withDbSession(ctx, func(sess *DBSession) error { var rawSql = "DELETE FROM api_key WHERE id=? and org_id=?" _, err := sess.Exec(rawSql, cmd.Id, cmd.OrgId) return err diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index f97134fd0d5..40101528df5 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -250,6 +250,7 @@ func (ss *SqlStore) readConfig() { } func InitTestDB(t *testing.T) *SqlStore { + t.Helper() sqlstore := &SqlStore{} sqlstore.skipEnsureAdmin = true sqlstore.Bus = bus.New() diff --git a/pkg/services/sqlstore/transactions_test.go b/pkg/services/sqlstore/transactions_test.go new file mode 100644 index 00000000000..2575229aad5 --- /dev/null +++ b/pkg/services/sqlstore/transactions_test.go @@ -0,0 +1,64 @@ +package sqlstore + +import ( + "context" + "errors" + "testing" + + "github.com/grafana/grafana/pkg/log" + "github.com/grafana/grafana/pkg/models" + + . "github.com/smartystreets/goconvey/convey" +) + +type testQuery struct { + result bool +} + +var ProvokedError = errors.New("testing error.") + +func TestTransaction(t *testing.T) { + InitTestDB(t) + + Convey("InTransaction asdf asdf", t, func() { + ss := SqlStore{log: log.New("test-logger")} + + cmd := &models.AddApiKeyCommand{Key: "secret-key", Name: "key", OrgId: 1} + + err := AddApiKey(cmd) + So(err, ShouldBeNil) + + deleteApiKeyCmd := &models.DeleteApiKeyCommand{Id: cmd.Result.Id, OrgId: 1} + + Convey("can update key", func() { + err := ss.InTransaction(context.Background(), func(ctx context.Context) error { + return DeleteApiKeyCtx(ctx, deleteApiKeyCmd) + }) + + So(err, ShouldBeNil) + + query := &models.GetApiKeyByIdQuery{ApiKeyId: cmd.Result.Id} + err = GetApiKeyById(query) + So(err, ShouldEqual, models.ErrInvalidApiKey) + }) + + Convey("wont update if one handler fails", func() { + err := ss.InTransaction(context.Background(), func(ctx context.Context) error { + err := DeleteApiKeyCtx(ctx, deleteApiKeyCmd) + if err != nil { + return err + } + + return ProvokedError + + }) + + So(err, ShouldEqual, ProvokedError) + + query := &models.GetApiKeyByIdQuery{ApiKeyId: cmd.Result.Id} + err = GetApiKeyById(query) + So(err, ShouldBeNil) + So(query.Result.Id, ShouldEqual, cmd.Result.Id) + }) + }) +} From b418e14bd975bca7d8afd8a236a909c8405a29f6 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 15 Jun 2018 13:40:42 +0200 Subject: [PATCH 23/84] make sure to use real ip when validating white listed ip's --- pkg/middleware/auth_proxy.go | 20 ++++++----- pkg/middleware/middleware_test.go | 55 +++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index 144a0ae3a69..eff532b0da2 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -2,7 +2,6 @@ package middleware import ( "fmt" - "net" "net/mail" "reflect" "strings" @@ -29,7 +28,7 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { } // if auth proxy ip(s) defined, check if request comes from one of those - if err := checkAuthenticationProxy(ctx.Req.RemoteAddr, proxyHeaderValue); err != nil { + if err := checkAuthenticationProxy(ctx.RemoteAddr(), proxyHeaderValue); err != nil { ctx.Handle(407, "Proxy authentication required", err) return true } @@ -197,18 +196,23 @@ func checkAuthenticationProxy(remoteAddr string, proxyHeaderValue string) error return nil } - proxies := strings.Split(setting.AuthProxyWhitelist, ",") - sourceIP, _, err := net.SplitHostPort(remoteAddr) - if err != nil { - return err + // Multiple ip addresses? Right-most IP address is the IP address of the most recent proxy + if strings.Contains(remoteAddr, ",") { + sourceIPs := strings.Split(remoteAddr, ",") + remoteAddr = strings.TrimSpace(sourceIPs[len(sourceIPs)-1]) } + remoteAddr = strings.TrimPrefix(remoteAddr, "[") + remoteAddr = strings.TrimSuffix(remoteAddr, "]") + + proxies := strings.Split(setting.AuthProxyWhitelist, ",") + // Compare allowed IP addresses to actual address for _, proxyIP := range proxies { - if sourceIP == strings.TrimSpace(proxyIP) { + if remoteAddr == strings.TrimSpace(proxyIP) { return nil } } - return fmt.Errorf("Request for user (%s) from %s is not from the authentication proxy", proxyHeaderValue, sourceIP) + return fmt.Errorf("Request for user (%s) from %s is not from the authentication proxy", proxyHeaderValue, remoteAddr) } diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index b827751b1a5..0b50358ad73 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -293,6 +293,61 @@ func TestMiddlewareContext(t *testing.T) { }) }) + middlewareScenario("When auth_proxy is enabled and request has X-Forwarded-For that is not trusted", func(sc *scenarioContext) { + setting.AuthProxyEnabled = true + setting.AuthProxyHeaderName = "X-WEBAUTH-USER" + setting.AuthProxyHeaderProperty = "username" + setting.AuthProxyWhitelist = "192.168.1.1, 2001::23" + + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{OrgId: 4, UserId: 33} + return nil + }) + + bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error { + cmd.Result = &m.User{Id: 33} + return nil + }) + + sc.fakeReq("GET", "/") + sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") + sc.req.Header.Add("X-Forwarded-For", "client-ip, 192.168.1.1, 192.168.1.2") + sc.exec() + + Convey("should return 407 status code", func() { + So(sc.resp.Code, ShouldEqual, 407) + So(sc.resp.Body.String(), ShouldContainSubstring, "Request for user (torkelo) from 192.168.1.2 is not from the authentication proxy") + }) + }) + + middlewareScenario("When auth_proxy is enabled and request has X-Forwarded-For that is trusted", func(sc *scenarioContext) { + setting.AuthProxyEnabled = true + setting.AuthProxyHeaderName = "X-WEBAUTH-USER" + setting.AuthProxyHeaderProperty = "username" + setting.AuthProxyWhitelist = "192.168.1.1, 2001::23" + + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { + query.Result = &m.SignedInUser{OrgId: 4, UserId: 33} + return nil + }) + + bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error { + cmd.Result = &m.User{Id: 33} + return nil + }) + + sc.fakeReq("GET", "/") + sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") + sc.req.Header.Add("X-Forwarded-For", "client-ip, 192.168.1.2, 192.168.1.1") + sc.exec() + + Convey("Should init context with user info", func() { + So(sc.context.IsSignedIn, ShouldBeTrue) + So(sc.context.UserId, ShouldEqual, 33) + So(sc.context.OrgId, ShouldEqual, 4) + }) + }) + middlewareScenario("When session exists for previous user, create a new session", func(sc *scenarioContext) { setting.AuthProxyEnabled = true setting.AuthProxyHeaderName = "X-WEBAUTH-USER" From c02dd7462a1f651b2cd92f4935872b9999f129a3 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 15 Jun 2018 15:48:25 +0200 Subject: [PATCH 24/84] cloudwatch: handle invalid time range --- pkg/tsdb/cloudwatch/cloudwatch.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/tsdb/cloudwatch/cloudwatch.go b/pkg/tsdb/cloudwatch/cloudwatch.go index 499a3ed6e03..8af97575ae9 100644 --- a/pkg/tsdb/cloudwatch/cloudwatch.go +++ b/pkg/tsdb/cloudwatch/cloudwatch.go @@ -3,6 +3,7 @@ package cloudwatch import ( "context" "errors" + "fmt" "regexp" "sort" "strconv" @@ -144,6 +145,10 @@ func (e *CloudWatchExecutor) executeQuery(ctx context.Context, parameters *simpl return nil, err } + if endTime.Before(startTime) { + return nil, fmt.Errorf("Invalid time range: End time can't be before start time") + } + params := &cloudwatch.GetMetricStatisticsInput{ Namespace: aws.String(query.Namespace), MetricName: aws.String(query.MetricName), From da91b91b4bf32efdfd1946c419578a767b9b2de8 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Jun 2018 20:49:14 +0200 Subject: [PATCH 25/84] transactions: start sessions and transactions at the same place this make it possible for handler to use `withSession` when transactions is not nedded and `inTransactionCtx` if its needed without knowing who owns the session/transaction --- pkg/services/sqlstore/session.go | 19 ++++++++++++++----- pkg/services/sqlstore/transactions.go | 21 +++++++++------------ pkg/services/sqlstore/transactions_test.go | 6 +----- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go index fdee2c76b0c..c85346231e4 100644 --- a/pkg/services/sqlstore/session.go +++ b/pkg/services/sqlstore/session.go @@ -22,21 +22,30 @@ func newSession() *DBSession { return &DBSession{Session: x.NewSession()} } -func startSession(ctx context.Context) *DBSession { +func startSession(ctx context.Context, engine *xorm.Engine, beginTran bool) (*DBSession, error) { value := ctx.Value(ContextSessionName) var sess *DBSession sess, ok := value.(*DBSession) if !ok { - newSess := newSession() - return newSess + newSess := &DBSession{Session: engine.NewSession()} + if beginTran { + err := newSess.Begin() + if err != nil { + return nil, err + } + } + return newSess, nil } - return sess + return sess, nil } func withDbSession(ctx context.Context, callback dbTransactionFunc) error { - sess := startSession(ctx) + sess, err := startSession(ctx, x, false) + if err != nil { + return err + } return callback(sess) } diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go index f72b0bb8500..3e7634dc196 100644 --- a/pkg/services/sqlstore/transactions.go +++ b/pkg/services/sqlstore/transactions.go @@ -14,16 +14,16 @@ func (ss *SqlStore) InTransaction(ctx context.Context, fn func(ctx context.Conte } func (ss *SqlStore) inTransactionWithRetry(ctx context.Context, fn func(ctx context.Context) error, retry int) error { - sess := startSession(ctx) - defer sess.Close() - - if err := sess.Begin(); err != nil { + sess, err := startSession(ctx, ss.engine, true) + if err != nil { return err } + defer sess.Close() + withValue := context.WithValue(ctx, ContextSessionName, sess) - err := fn(withValue) + err = fn(withValue) // special handling of database locked errors for sqlite, then we can retry 3 times if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 { @@ -60,16 +60,13 @@ func inTransactionWithRetry(callback dbTransactionFunc, retry int) error { } func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, retry int) error { - var err error - - sess := startSession(ctx) - - defer sess.Close() - - if err = sess.Begin(); err != nil { + sess, err := startSession(ctx, x, true) + if err != nil { return err } + defer sess.Close() + err = callback(sess) // special handling of database locked errors for sqlite, then we can retry 3 times diff --git a/pkg/services/sqlstore/transactions_test.go b/pkg/services/sqlstore/transactions_test.go index 2575229aad5..937649921ba 100644 --- a/pkg/services/sqlstore/transactions_test.go +++ b/pkg/services/sqlstore/transactions_test.go @@ -5,7 +5,6 @@ import ( "errors" "testing" - "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/models" . "github.com/smartystreets/goconvey/convey" @@ -18,11 +17,9 @@ type testQuery struct { var ProvokedError = errors.New("testing error.") func TestTransaction(t *testing.T) { - InitTestDB(t) + ss := InitTestDB(t) Convey("InTransaction asdf asdf", t, func() { - ss := SqlStore{log: log.New("test-logger")} - cmd := &models.AddApiKeyCommand{Key: "secret-key", Name: "key", OrgId: 1} err := AddApiKey(cmd) @@ -50,7 +47,6 @@ func TestTransaction(t *testing.T) { } return ProvokedError - }) So(err, ShouldEqual, ProvokedError) From 1181e967992990c119e0194c8e7c55dfed46b0d6 Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Jun 2018 21:23:57 +0200 Subject: [PATCH 26/84] merge create user handlers --- pkg/services/sqlstore/dashboard_test.go | 3 +- pkg/services/sqlstore/org_test.go | 11 +- pkg/services/sqlstore/team_test.go | 3 +- pkg/services/sqlstore/user.go | 146 +++++++++++------------- pkg/services/sqlstore/user_auth_test.go | 3 +- pkg/services/sqlstore/user_test.go | 3 +- 6 files changed, 82 insertions(+), 87 deletions(-) diff --git a/pkg/services/sqlstore/dashboard_test.go b/pkg/services/sqlstore/dashboard_test.go index 6d7c7a93e47..e4aecf0391d 100644 --- a/pkg/services/sqlstore/dashboard_test.go +++ b/pkg/services/sqlstore/dashboard_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" "time" @@ -389,7 +390,7 @@ func createUser(name string, role string, isAdmin bool) m.User { setting.AutoAssignOrgRole = role currentUserCmd := m.CreateUserCommand{Login: name, Email: name + "@test.com", Name: "a " + name, IsAdmin: isAdmin} - err := CreateUser(¤tUserCmd) + err := CreateUser(context.Background(), ¤tUserCmd) So(err, ShouldBeNil) q1 := m.GetUserOrgListQuery{UserId: currentUserCmd.Result.Id} diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 63b20aa6e86..f41b449de96 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "testing" "time" @@ -22,9 +23,9 @@ func TestAccountDataAccess(t *testing.T) { ac1cmd := m.CreateUserCommand{Login: "ac1", Email: "ac1@test.com", Name: "ac1 name"} ac2cmd := m.CreateUserCommand{Login: "ac2", Email: "ac2@test.com", Name: "ac2 name"} - err := CreateUser(&ac1cmd) + err := CreateUser(context.Background(), &ac1cmd) So(err, ShouldBeNil) - err = CreateUser(&ac2cmd) + err = CreateUser(context.Background(), &ac2cmd) So(err, ShouldBeNil) q1 := m.GetUserOrgListQuery{UserId: ac1cmd.Result.Id} @@ -43,8 +44,8 @@ func TestAccountDataAccess(t *testing.T) { ac1cmd := m.CreateUserCommand{Login: "ac1", Email: "ac1@test.com", Name: "ac1 name"} ac2cmd := m.CreateUserCommand{Login: "ac2", Email: "ac2@test.com", Name: "ac2 name", IsAdmin: true} - err := CreateUser(&ac1cmd) - err = CreateUser(&ac2cmd) + err := CreateUser(context.Background(), &ac1cmd) + err = CreateUser(context.Background(), &ac2cmd) So(err, ShouldBeNil) ac1 := ac1cmd.Result @@ -182,7 +183,7 @@ func TestAccountDataAccess(t *testing.T) { Convey("Given an org user with dashboard permissions", func() { ac3cmd := m.CreateUserCommand{Login: "ac3", Email: "ac3@test.com", Name: "ac3 name", IsAdmin: false} - err := CreateUser(&ac3cmd) + err := CreateUser(context.Background(), &ac3cmd) So(err, ShouldBeNil) ac3 := ac3cmd.Result diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index f4b022906da..abaa973957d 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" @@ -22,7 +23,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { Name: fmt.Sprint("user", i), Login: fmt.Sprint("loginuser", i), } - err := CreateUser(userCmd) + err := CreateUser(context.Background(), userCmd) So(err, ShouldBeNil) userIds = append(userIds, userCmd.Result.Id) } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 252499d5fdc..4448e973e99 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -16,7 +16,7 @@ import ( ) func init() { - bus.AddHandler("sql", CreateUser) + //bus.AddHandler("sql", CreateUser) bus.AddHandler("sql", GetUserById) bus.AddHandler("sql", UpdateUser) bus.AddHandler("sql", ChangeUserPassword) @@ -31,7 +31,7 @@ func init() { bus.AddHandler("sql", DeleteUser) bus.AddHandler("sql", UpdateUserPermissions) bus.AddHandler("sql", SetUserHelpFlag) - bus.AddHandlerCtx("sql", CreateUserCtx) + bus.AddHandlerCtx("sql", CreateUser) } func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error) { @@ -81,90 +81,80 @@ func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error return org.Id, nil } -func internalCreateUser(sess *DBSession, cmd *m.CreateUserCommand) error { - orgId, err := getOrgIdForNewUser(cmd, sess) - if err != nil { - return err - } - - if cmd.Email == "" { - cmd.Email = cmd.Login - } - - // create user - user := m.User{ - Email: cmd.Email, - Name: cmd.Name, - Login: cmd.Login, - Company: cmd.Company, - IsAdmin: cmd.IsAdmin, - OrgId: orgId, - EmailVerified: cmd.EmailVerified, - Created: time.Now(), - Updated: time.Now(), - LastSeenAt: time.Now().AddDate(-10, 0, 0), - } - - if len(cmd.Password) > 0 { - user.Salt = util.GetRandomString(10) - user.Rands = util.GetRandomString(10) - user.Password = util.EncodePassword(cmd.Password, user.Salt) - } - - sess.UseBool("is_admin") - - if _, err := sess.Insert(&user); err != nil { - return err - } - - sess.publishAfterCommit(&events.UserCreated{ - Timestamp: user.Created, - Id: user.Id, - Name: user.Name, - Login: user.Login, - Email: user.Email, - }) - - cmd.Result = user - - // create org user link - if !cmd.SkipOrgSetup { - orgUser := m.OrgUser{ - OrgId: orgId, - UserId: user.Id, - Role: m.ROLE_ADMIN, - Created: time.Now(), - Updated: time.Now(), +func CreateUser(ctx context.Context, cmd *m.CreateUserCommand) error { + return inTransactionWithRetryCtx(ctx, func(sess *DBSession) error { + orgId, err := getOrgIdForNewUser(cmd, sess) + if err != nil { + return err } - if setting.AutoAssignOrg && !user.IsAdmin { - if len(cmd.DefaultOrgRole) > 0 { - orgUser.Role = m.RoleType(cmd.DefaultOrgRole) - } else { - orgUser.Role = m.RoleType(setting.AutoAssignOrgRole) + if cmd.Email == "" { + cmd.Email = cmd.Login + } + + // create user + user := m.User{ + Email: cmd.Email, + Name: cmd.Name, + Login: cmd.Login, + Company: cmd.Company, + IsAdmin: cmd.IsAdmin, + OrgId: orgId, + EmailVerified: cmd.EmailVerified, + Created: time.Now(), + Updated: time.Now(), + LastSeenAt: time.Now().AddDate(-10, 0, 0), + } + + if len(cmd.Password) > 0 { + user.Salt = util.GetRandomString(10) + user.Rands = util.GetRandomString(10) + user.Password = util.EncodePassword(cmd.Password, user.Salt) + } + + sess.UseBool("is_admin") + + if _, err := sess.Insert(&user); err != nil { + return err + } + + sess.publishAfterCommit(&events.UserCreated{ + Timestamp: user.Created, + Id: user.Id, + Name: user.Name, + Login: user.Login, + Email: user.Email, + }) + + cmd.Result = user + + // create org user link + if !cmd.SkipOrgSetup { + orgUser := m.OrgUser{ + OrgId: orgId, + UserId: user.Id, + Role: m.ROLE_ADMIN, + Created: time.Now(), + Updated: time.Now(), + } + + if setting.AutoAssignOrg && !user.IsAdmin { + if len(cmd.DefaultOrgRole) > 0 { + orgUser.Role = m.RoleType(cmd.DefaultOrgRole) + } else { + orgUser.Role = m.RoleType(setting.AutoAssignOrgRole) + } + } + + if _, err = sess.Insert(&orgUser); err != nil { + return err } } - if _, err = sess.Insert(&orgUser); err != nil { - return err - } - } - - return nil -} - -func CreateUserCtx(ctx context.Context, cmd *m.CreateUserCommand) error { - return inTransactionWithRetryCtx(ctx, func(sess *DBSession) error { - return internalCreateUser(sess, cmd) + return nil }, 0) } -func CreateUser(cmd *m.CreateUserCommand) error { - return inTransaction(func(sess *DBSession) error { - return internalCreateUser(sess, cmd) - }) -} - func GetUserById(query *m.GetUserByIdQuery) error { user := new(m.User) has, err := x.Id(query.Id).Get(user) diff --git a/pkg/services/sqlstore/user_auth_test.go b/pkg/services/sqlstore/user_auth_test.go index 882e0c7afa5..5ad93dc7a3b 100644 --- a/pkg/services/sqlstore/user_auth_test.go +++ b/pkg/services/sqlstore/user_auth_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" @@ -22,7 +23,7 @@ func TestUserAuth(t *testing.T) { Name: fmt.Sprint("user", i), Login: fmt.Sprint("loginuser", i), } - err = CreateUser(cmd) + err = CreateUser(context.Background(), cmd) So(err, ShouldBeNil) users = append(users, cmd.Result) } diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 2830733c96a..3597b6ad0c1 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -1,6 +1,7 @@ package sqlstore import ( + "context" "fmt" "testing" @@ -24,7 +25,7 @@ func TestUserDataAccess(t *testing.T) { Name: fmt.Sprint("user", i), Login: fmt.Sprint("loginuser", i), } - err = CreateUser(cmd) + err = CreateUser(context.Background(), cmd) So(err, ShouldBeNil) users = append(users, cmd.Result) } From 4c5fe68e7ef8c78f1572cb30730317358390d2bb Mon Sep 17 00:00:00 2001 From: bergquist Date: Fri, 15 Jun 2018 21:57:13 +0200 Subject: [PATCH 27/84] adds inTransactionCtx that calls inTransactionWithRetryCtx --- pkg/services/sqlstore/transactions.go | 4 ++++ pkg/services/sqlstore/user.go | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go index 3e7634dc196..eccd37f9a43 100644 --- a/pkg/services/sqlstore/transactions.go +++ b/pkg/services/sqlstore/transactions.go @@ -100,3 +100,7 @@ func inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, func inTransaction(callback dbTransactionFunc) error { return inTransactionWithRetry(callback, 0) } + +func inTransactionCtx(ctx context.Context, callback dbTransactionFunc) error { + return inTransactionWithRetryCtx(ctx, callback, 0) +} diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 4448e973e99..d32d51e0d0c 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -82,7 +82,7 @@ func getOrgIdForNewUser(cmd *m.CreateUserCommand, sess *DBSession) (int64, error } func CreateUser(ctx context.Context, cmd *m.CreateUserCommand) error { - return inTransactionWithRetryCtx(ctx, func(sess *DBSession) error { + return inTransactionCtx(ctx, func(sess *DBSession) error { orgId, err := getOrgIdForNewUser(cmd, sess) if err != nil { return err @@ -152,7 +152,7 @@ func CreateUser(ctx context.Context, cmd *m.CreateUserCommand) error { } return nil - }, 0) + }) } func GetUserById(query *m.GetUserByIdQuery) error { From 6782be80fd5e4c0d0b2e435a266315fb940f30d1 Mon Sep 17 00:00:00 2001 From: bergquist Date: Sat, 16 Jun 2018 16:59:15 +0200 Subject: [PATCH 28/84] changelog: adds note about closing #12199 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12dcadc822e..7c5133c5485 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * **Units**: Litre/min (flow) and milliLitre/min (flow) [#12282](https://github.com/grafana/grafana/pull/12282), thx [@flopp999](https://github.com/flopp999) * **Alerting**: Fix mobile notifications for Microsoft Teams alert notifier [#11484](https://github.com/grafana/grafana/pull/11484), thx [@manacker](https://github.com/manacker) * **Influxdb**: Add support for mode function [#12286](https://github.com/grafana/grafana/issues/12286) +* **Cloudwatch**: Fixes panic caused by bad timerange settings [#12199](https://github.com/grafana/grafana/issues/12199) # 5.2.0-beta1 (2018-06-05) From a2ff7629e06979c5467d852c7ed2710db5e56572 Mon Sep 17 00:00:00 2001 From: Tim Heckman Date: Sun, 17 Jun 2018 22:38:37 -0700 Subject: [PATCH 29/84] Include the vendor directory when copying source in to Docker (#12305) This change updates the `.dockerignore` file to no longer contain the `vendor/` directory. When a Go project provides a `vendor/` directory within the repository, the best practice is to build that project using their vendored dependencies. By putting it in the `.dockerignore` file we prevent consumers from easily doing that. The `vendor/` directory is used to include all of the dependencies needed to build a project. This makes it so that we can reproducibly build the project, at any given commit, because the dependencies will always be present. Also, using the vendor directory avoids us needing to continually re-download all the dependencies, and it protects us from build failures if GitHub is down or a dependency gets removed or renamed. In addition to the change above, this also removes an extra `/tmp` entry from the `.dockerignore` file. Fixes #12304 Signed-off-by: Tim Heckman --- .dockerignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.dockerignore b/.dockerignore index c79fe777899..e50dfd86aa3 100644 --- a/.dockerignore +++ b/.dockerignore @@ -11,8 +11,5 @@ dump.rdb node_modules /local /tmp -/vendor *.yml *.md -/vendor -/tmp From ab9f0e8edda9c07be93ef904cc23ca2a17352a23 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 18 Jun 2018 09:03:30 +0200 Subject: [PATCH 30/84] changelog: add notes about closing #10707 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c5133c5485..7120fed47ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ * **Alerting**: Fix mobile notifications for Microsoft Teams alert notifier [#11484](https://github.com/grafana/grafana/pull/11484), thx [@manacker](https://github.com/manacker) * **Influxdb**: Add support for mode function [#12286](https://github.com/grafana/grafana/issues/12286) * **Cloudwatch**: Fixes panic caused by bad timerange settings [#12199](https://github.com/grafana/grafana/issues/12199) +* **Auth Proxy**: Whitelist proxy IP address instead of client IP address [#10707](https://github.com/grafana/grafana/issues/10707) # 5.2.0-beta1 (2018-06-05) From 6d48d0a80c8ce59b9dc782a623dab4e3fcefcbb4 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Wed, 13 Jun 2018 18:01:50 +0200 Subject: [PATCH 31/84] set current org when adding/removing user to org To not get into a situation where a user has a current organization assign which he is not a member of we try to always make sure that a user has a valid current organization assigned. --- pkg/services/sqlstore/org_test.go | 16 ++++- pkg/services/sqlstore/org_users.go | 64 ++++++++++++++++++- pkg/services/sqlstore/user.go | 18 ++++-- pkg/services/sqlstore/user_test.go | 14 ++-- .../features/admin/admin_edit_user_ctrl.ts | 2 + 5 files changed, 96 insertions(+), 18 deletions(-) diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 63b20aa6e86..dcf45032198 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -150,7 +150,7 @@ func TestAccountDataAccess(t *testing.T) { }) Convey("Can set using org", func() { - cmd := m.SetUsingOrgCommand{UserId: ac2.Id, OrgId: ac1.Id} + cmd := m.SetUsingOrgCommand{UserId: ac2.Id, OrgId: ac1.OrgId} err := SetUsingOrg(&cmd) So(err, ShouldBeNil) @@ -159,13 +159,25 @@ func TestAccountDataAccess(t *testing.T) { err := GetSignedInUser(&query) So(err, ShouldBeNil) - So(query.Result.OrgId, ShouldEqual, ac1.Id) + So(query.Result.OrgId, ShouldEqual, ac1.OrgId) So(query.Result.Email, ShouldEqual, "ac2@test.com") So(query.Result.Name, ShouldEqual, "ac2 name") So(query.Result.Login, ShouldEqual, "ac2") So(query.Result.OrgName, ShouldEqual, "ac1@test.com") So(query.Result.OrgRole, ShouldEqual, "Viewer") }) + + Convey("Should set last org as current when removing user from current", func() { + remCmd := m.RemoveOrgUserCommand{OrgId: ac1.OrgId, UserId: ac2.Id} + err := RemoveOrgUser(&remCmd) + So(err, ShouldBeNil) + + query := m.GetSignedInUserQuery{UserId: ac2.Id} + err = GetSignedInUser(&query) + + So(err, ShouldBeNil) + So(query.Result.OrgId, ShouldEqual, ac2.OrgId) + }) }) Convey("Cannot delete last admin org user", func() { diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index 0b991c73c55..aad72cdacb4 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -20,7 +20,14 @@ func init() { func AddOrgUser(cmd *m.AddOrgUserCommand) error { return inTransaction(func(sess *DBSession) error { // check if user exists - if res, err := sess.Query("SELECT 1 from org_user WHERE org_id=? and user_id=?", cmd.OrgId, cmd.UserId); err != nil { + var user m.User + if exists, err := sess.Id(cmd.UserId).Get(&user); err != nil { + return err + } else if !exists { + return m.ErrUserNotFound + } + + if res, err := sess.Query("SELECT 1 from org_user WHERE org_id=? and user_id=?", cmd.OrgId, user.Id); err != nil { return err } else if len(res) == 1 { return m.ErrOrgUserAlreadyAdded @@ -41,7 +48,26 @@ func AddOrgUser(cmd *m.AddOrgUserCommand) error { } _, err := sess.Insert(&entity) - return err + if err != nil { + return err + } + + var userOrgs []*m.UserOrgDTO + sess.Table("org_user") + sess.Join("INNER", "org", "org_user.org_id=org.id") + sess.Where("org_user.user_id=? AND org_user.org_id=?", user.Id, user.OrgId) + sess.Cols("org.name", "org_user.role", "org_user.org_id") + err = sess.Find(&userOrgs) + + if err != nil { + return err + } + + if len(userOrgs) == 0 { + return setUsingOrgInTransaction(sess, user.Id, cmd.OrgId) + } + + return nil }) } @@ -110,6 +136,14 @@ func GetOrgUsers(query *m.GetOrgUsersQuery) error { func RemoveOrgUser(cmd *m.RemoveOrgUserCommand) error { return inTransaction(func(sess *DBSession) error { + // check if user exists + var user m.User + if exists, err := sess.Id(cmd.UserId).Get(&user); err != nil { + return err + } else if !exists { + return m.ErrUserNotFound + } + deletes := []string{ "DELETE FROM org_user WHERE org_id=? and user_id=?", "DELETE FROM dashboard_acl WHERE org_id=? and user_id = ?", @@ -123,6 +157,32 @@ func RemoveOrgUser(cmd *m.RemoveOrgUserCommand) error { } } + var userOrgs []*m.UserOrgDTO + sess.Table("org_user") + sess.Join("INNER", "org", "org_user.org_id=org.id") + sess.Where("org_user.user_id=?", user.Id) + sess.Cols("org.name", "org_user.role", "org_user.org_id") + err := sess.Find(&userOrgs) + + if err != nil { + return err + } + + hasCurrentOrgSet := false + for _, userOrg := range userOrgs { + if user.OrgId == userOrg.OrgId { + hasCurrentOrgSet = true + break + } + } + + if !hasCurrentOrgSet && len(userOrgs) > 0 { + err = setUsingOrgInTransaction(sess, user.Id, userOrgs[0].OrgId) + if err != nil { + return err + } + } + return validateOneAdminLeftInOrg(cmd.OrgId, sess) }) } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index e7aa8da837a..ad86323c0d8 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -290,16 +290,20 @@ func SetUsingOrg(cmd *m.SetUsingOrgCommand) error { } return inTransaction(func(sess *DBSession) error { - user := m.User{ - Id: cmd.UserId, - OrgId: cmd.OrgId, - } - - _, err := sess.Id(cmd.UserId).Update(&user) - return err + return setUsingOrgInTransaction(sess, cmd.UserId, cmd.OrgId) }) } +func setUsingOrgInTransaction(sess *DBSession, userID int64, orgID int64) error { + user := m.User{ + Id: userID, + OrgId: orgID, + } + + _, err := sess.Id(userID).Update(&user) + return err +} + func GetUserProfile(query *m.GetUserProfileQuery) error { var user m.User has, err := x.Id(query.UserId).Get(&user) diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index 2830733c96a..076e88c2bb3 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -96,33 +96,33 @@ func TestUserDataAccess(t *testing.T) { }) Convey("when a user is an org member and has been assigned permissions", func() { - err = AddOrgUser(&m.AddOrgUserCommand{LoginOrEmail: users[0].Login, Role: m.ROLE_VIEWER, OrgId: users[0].OrgId}) + err = AddOrgUser(&m.AddOrgUserCommand{LoginOrEmail: users[1].Login, Role: m.ROLE_VIEWER, OrgId: users[0].OrgId, UserId: users[1].Id}) So(err, ShouldBeNil) - testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[0].Id, Permission: m.PERMISSION_EDIT}) + testHelperUpdateDashboardAcl(1, m.DashboardAcl{DashboardId: 1, OrgId: users[0].OrgId, UserId: users[1].Id, Permission: m.PERMISSION_EDIT}) So(err, ShouldBeNil) - err = SavePreferences(&m.SavePreferencesCommand{UserId: users[0].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"}) + err = SavePreferences(&m.SavePreferencesCommand{UserId: users[1].Id, OrgId: users[0].OrgId, HomeDashboardId: 1, Theme: "dark"}) So(err, ShouldBeNil) Convey("when the user is deleted", func() { - err = DeleteUser(&m.DeleteUserCommand{UserId: users[0].Id}) + err = DeleteUser(&m.DeleteUserCommand{UserId: users[1].Id}) So(err, ShouldBeNil) Convey("Should delete connected org users and permissions", func() { - query := &m.GetOrgUsersQuery{OrgId: 1} + query := &m.GetOrgUsersQuery{OrgId: users[0].OrgId} err = GetOrgUsersForTest(query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) - permQuery := &m.GetDashboardAclInfoListQuery{DashboardId: 1, OrgId: 1} + permQuery := &m.GetDashboardAclInfoListQuery{DashboardId: 1, OrgId: users[0].OrgId} err = GetDashboardAclInfoList(permQuery) So(err, ShouldBeNil) So(len(permQuery.Result), ShouldEqual, 0) - prefsQuery := &m.GetPreferencesQuery{OrgId: users[0].OrgId, UserId: users[0].Id} + prefsQuery := &m.GetPreferencesQuery{OrgId: users[0].OrgId, UserId: users[1].Id} err = GetPreferences(prefsQuery) So(err, ShouldBeNil) diff --git a/public/app/features/admin/admin_edit_user_ctrl.ts b/public/app/features/admin/admin_edit_user_ctrl.ts index 8203c7399c1..1d4fb9cf19a 100644 --- a/public/app/features/admin/admin_edit_user_ctrl.ts +++ b/public/app/features/admin/admin_edit_user_ctrl.ts @@ -75,6 +75,7 @@ export class AdminEditUserCtrl { $scope.removeOrgUser = function(orgUser) { backendSrv.delete('/api/orgs/' + orgUser.orgId + '/users/' + $scope.user_id).then(function() { + $scope.getUser($scope.user_id); $scope.getUserOrgs($scope.user_id); }); }; @@ -108,6 +109,7 @@ export class AdminEditUserCtrl { $scope.newOrg.loginOrEmail = $scope.user.login; backendSrv.post('/api/orgs/' + orgInfo.id + '/users/', $scope.newOrg).then(function() { + $scope.getUser($scope.user_id); $scope.getUserOrgs($scope.user_id); }); }; From a7383479574a73cf4c0d87658e36ae0fccf3ac9c Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 18 Jun 2018 11:04:16 +0200 Subject: [PATCH 32/84] snapshot: copy correct props when creating a snapshot --- public/app/features/dashboard/share_snapshot_ctrl.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/public/app/features/dashboard/share_snapshot_ctrl.ts b/public/app/features/dashboard/share_snapshot_ctrl.ts index aa146dcad63..7d5bd112dfd 100644 --- a/public/app/features/dashboard/share_snapshot_ctrl.ts +++ b/public/app/features/dashboard/share_snapshot_ctrl.ts @@ -123,6 +123,9 @@ export class ShareSnapshotCtrl { enable: annotation.enable, iconColor: annotation.iconColor, snapshotData: annotation.snapshotData, + type: annotation.type, + builtIn: annotation.builtIn, + hide: annotation.hide, }; }) .value(); From b72c45f7355152804e16fa93cd325ebb92b00595 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 18 Jun 2018 11:22:55 +0200 Subject: [PATCH 33/84] changelog: add notes about closing #11076 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7120fed47ad..ba1e81e946e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * **Influxdb**: Add support for mode function [#12286](https://github.com/grafana/grafana/issues/12286) * **Cloudwatch**: Fixes panic caused by bad timerange settings [#12199](https://github.com/grafana/grafana/issues/12199) * **Auth Proxy**: Whitelist proxy IP address instead of client IP address [#10707](https://github.com/grafana/grafana/issues/10707) +* **User Management**: Make sure that a user always has a current org assigned [#11076](https://github.com/grafana/grafana/issues/11076) # 5.2.0-beta1 (2018-06-05) From 9a4ccdf3882ccc910ecd0c8fdaa01f6569cbf0d2 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 18 Jun 2018 11:28:33 +0200 Subject: [PATCH 34/84] changelog: add notes about closing #12278 [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1e81e946e..3624e90d128 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * **Cloudwatch**: Fixes panic caused by bad timerange settings [#12199](https://github.com/grafana/grafana/issues/12199) * **Auth Proxy**: Whitelist proxy IP address instead of client IP address [#10707](https://github.com/grafana/grafana/issues/10707) * **User Management**: Make sure that a user always has a current org assigned [#11076](https://github.com/grafana/grafana/issues/11076) +* **Snapshots**: Fix: annotations not properly extracted leading to incorrect rendering of annotations [#12278](https://github.com/grafana/grafana/issues/12278) # 5.2.0-beta1 (2018-06-05) From dd7a185a91062c11a3c946adb205568c34e639cc Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 18 Jun 2018 11:40:17 +0200 Subject: [PATCH 35/84] test commit for checking github permissions [skip ci] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3624e90d128..19f0f007d78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1739,3 +1739,4 @@ Thanks to everyone who contributed fixes and provided feedback :+1: # 1.0.0 (2014-01-19) First public release + From 2b849086a14f2a877c2edbc65b5395f5736ed33d Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 18 Jun 2018 13:50:46 +0200 Subject: [PATCH 36/84] changelog: adds note about closing #11607 [skip ci] --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19f0f007d78..f1202688b96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 5.3.0 (unreleased) + +* **Cleanup**: Make temp file time to live configurable [#11607](https://github.com/grafana/grafana/issues/11607), thx [@xapon](https://github.com/xapon) + # 5.2.0 (unreleased) ### New Features From 3479cf4b396e84a13cd766a5adde14ba338da515 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 18 Jun 2018 14:50:36 +0200 Subject: [PATCH 37/84] expose functions to use sessions --- pkg/services/sqlstore/session.go | 20 +++++------ pkg/services/sqlstore/sqlstore.go | 60 ++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/pkg/services/sqlstore/session.go b/pkg/services/sqlstore/session.go index c85346231e4..29d7392678f 100644 --- a/pkg/services/sqlstore/session.go +++ b/pkg/services/sqlstore/session.go @@ -27,18 +27,18 @@ func startSession(ctx context.Context, engine *xorm.Engine, beginTran bool) (*DB var sess *DBSession sess, ok := value.(*DBSession) - if !ok { - newSess := &DBSession{Session: engine.NewSession()} - if beginTran { - err := newSess.Begin() - if err != nil { - return nil, err - } - } - return newSess, nil + if ok { + return sess, nil } - return sess, nil + newSess := &DBSession{Session: engine.NewSession()} + if beginTran { + err := newSess.Begin() + if err != nil { + return nil, err + } + } + return newSess, nil } func withDbSession(ctx context.Context, callback dbTransactionFunc) error { diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index 40101528df5..b0edc1676e0 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -26,7 +26,7 @@ import ( _ "github.com/grafana/grafana/pkg/tsdb/mssql" _ "github.com/lib/pq" - _ "github.com/mattn/go-sqlite3" + sqlite3 "github.com/mattn/go-sqlite3" ) var ( @@ -56,6 +56,64 @@ type SqlStore struct { skipEnsureAdmin bool } +// NewSession returns a new DBSession +func (ss *SqlStore) NewSession() *DBSession { + return &DBSession{Session: ss.engine.NewSession()} +} + +// WithDbSession calls the callback with an session attached to the context. +func (ss *SqlStore) WithDbSession(ctx context.Context, callback dbTransactionFunc) error { + sess, err := startSession(ctx, ss.engine, false) + if err != nil { + return err + } + + return callback(sess) +} + +// WithTransactionalDbSession calls the callback with an session within a transaction +func (ss *SqlStore) WithTransactionalDbSession(ctx context.Context, callback dbTransactionFunc) error { + return ss.inTransactionWithRetryCtx(ctx, callback, 0) +} + +func (ss *SqlStore) inTransactionWithRetryCtx(ctx context.Context, callback dbTransactionFunc, retry int) error { + sess, err := startSession(ctx, ss.engine, true) + if err != nil { + return err + } + + defer sess.Close() + + err = callback(sess) + + // special handling of database locked errors for sqlite, then we can retry 3 times + if sqlError, ok := err.(sqlite3.Error); ok && retry < 5 { + if sqlError.Code == sqlite3.ErrLocked { + sess.Rollback() + time.Sleep(time.Millisecond * time.Duration(10)) + sqlog.Info("Database table locked, sleeping then retrying", "retry", retry) + return ss.inTransactionWithRetryCtx(ctx, callback, retry+1) + } + } + + if err != nil { + sess.Rollback() + return err + } else if err = sess.Commit(); err != nil { + return err + } + + if len(sess.events) > 0 { + for _, e := range sess.events { + if err = bus.Publish(e); err != nil { + log.Error(3, "Failed to publish event after commit", err) + } + } + } + + return nil +} + func (ss *SqlStore) Init() error { ss.log = log.New("sqlstore") ss.readConfig() From 7b3652af6721b5a5f9011beaace9818a27592f5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Bellon-Gervais?= Date: Mon, 18 Jun 2018 16:10:40 +0200 Subject: [PATCH 38/84] Fix error in InfluxDB query Minor error in sql query to retrieve annotations --- docs/sources/features/datasources/influxdb.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/features/datasources/influxdb.md b/docs/sources/features/datasources/influxdb.md index 1426f55e40b..bc96190e9b1 100644 --- a/docs/sources/features/datasources/influxdb.md +++ b/docs/sources/features/datasources/influxdb.md @@ -188,7 +188,7 @@ queries via the Dashboard menu / Annotations view. An example query: ```SQL -SELECT title, description from events WHERE $timeFilter order asc +SELECT title, description from events WHERE $timeFilter ORDER BY time ASC ``` For InfluxDB you need to enter a query like in the above example. You need to have the ```where $timeFilter``` From 5bf72fc9f4ff4a68f33484abeef82a8ce78315ba Mon Sep 17 00:00:00 2001 From: Tobias Skarhed Date: Mon, 18 Jun 2018 16:54:56 +0200 Subject: [PATCH 39/84] Karma to Jest: team_details_ctrl (#12321) --- .../org/specs/team_details_ctrl.jest.ts | 42 ++++++++++++++++ .../org/specs/team_details_ctrl_specs.ts | 48 ------------------- 2 files changed, 42 insertions(+), 48 deletions(-) create mode 100644 public/app/features/org/specs/team_details_ctrl.jest.ts delete mode 100644 public/app/features/org/specs/team_details_ctrl_specs.ts diff --git a/public/app/features/org/specs/team_details_ctrl.jest.ts b/public/app/features/org/specs/team_details_ctrl.jest.ts new file mode 100644 index 00000000000..c636de7ec56 --- /dev/null +++ b/public/app/features/org/specs/team_details_ctrl.jest.ts @@ -0,0 +1,42 @@ +import '../team_details_ctrl'; +import TeamDetailsCtrl from '../team_details_ctrl'; + +describe('TeamDetailsCtrl', () => { + var backendSrv = { + searchUsers: jest.fn(() => Promise.resolve([])), + get: jest.fn(() => Promise.resolve([])), + post: jest.fn(() => Promise.resolve([])), + }; + + //Team id + var routeParams = { + id: 1, + }; + + var navModelSrv = { + getNav: jest.fn(), + }; + + var teamDetailsCtrl = new TeamDetailsCtrl({ $broadcast: jest.fn() }, backendSrv, routeParams, navModelSrv); + + describe('when user is chosen to be added to team', () => { + beforeEach(() => { + teamDetailsCtrl = new TeamDetailsCtrl({ $broadcast: jest.fn() }, backendSrv, routeParams, navModelSrv); + const userItem = { + id: 2, + login: 'user2', + }; + teamDetailsCtrl.userPicked(userItem); + }); + + it('should parse the result and save to db', () => { + expect(backendSrv.post.mock.calls[0][0]).toBe('/api/teams/1/members'); + expect(backendSrv.post.mock.calls[0][1].userId).toBe(2); + }); + + it('should refresh the list after saving.', () => { + expect(backendSrv.get.mock.calls[0][0]).toBe('/api/teams/1'); + expect(backendSrv.get.mock.calls[1][0]).toBe('/api/teams/1/members'); + }); + }); +}); diff --git a/public/app/features/org/specs/team_details_ctrl_specs.ts b/public/app/features/org/specs/team_details_ctrl_specs.ts deleted file mode 100644 index 347f3796170..00000000000 --- a/public/app/features/org/specs/team_details_ctrl_specs.ts +++ /dev/null @@ -1,48 +0,0 @@ -import '../team_details_ctrl'; -import { describe, beforeEach, it, expect, sinon, angularMocks } from 'test/lib/common'; -import TeamDetailsCtrl from '../team_details_ctrl'; - -describe('TeamDetailsCtrl', () => { - var ctx: any = {}; - var backendSrv = { - searchUsers: sinon.stub().returns(Promise.resolve([])), - get: sinon.stub().returns(Promise.resolve([])), - post: sinon.stub().returns(Promise.resolve([])), - }; - - beforeEach(angularMocks.module('grafana.core')); - beforeEach(angularMocks.module('grafana.controllers')); - - beforeEach( - angularMocks.inject(($rootScope, $controller, $q) => { - ctx.$q = $q; - ctx.scope = $rootScope.$new(); - ctx.ctrl = $controller(TeamDetailsCtrl, { - $scope: ctx.scope, - backendSrv: backendSrv, - $routeParams: { id: 1 }, - navModelSrv: { getNav: sinon.stub() }, - }); - }) - ); - - describe('when user is chosen to be added to team', () => { - beforeEach(() => { - const userItem = { - id: 2, - login: 'user2', - }; - ctx.ctrl.userPicked(userItem); - }); - - it('should parse the result and save to db', () => { - expect(backendSrv.post.getCall(0).args[0]).to.eql('/api/teams/1/members'); - expect(backendSrv.post.getCall(0).args[1].userId).to.eql(2); - }); - - it('should refresh the list after saving.', () => { - expect(backendSrv.get.getCall(0).args[0]).to.eql('/api/teams/1'); - expect(backendSrv.get.getCall(1).args[0]).to.eql('/api/teams/1/members'); - }); - }); -}); From 8666c77cc95dac1bc31c8e8b4a9191e27e10ea95 Mon Sep 17 00:00:00 2001 From: Tobias Skarhed Date: Mon, 18 Jun 2018 18:47:23 +0200 Subject: [PATCH 40/84] Karma to Jest: time_srv (#12303) * Begin Karma to Jest: time_srv * Remove experimental fix * location search stubs for each test * Fix issue where time service was not available for other tests --- .../specs/annotations_srv_specs.ts | 3 + .../features/dashboard/specs/time_srv.jest.ts | 163 ++++++++++++++++++ .../dashboard/specs/time_srv_specs.ts | 115 ------------ public/app/features/dashboard/time_srv.ts | 2 +- .../cloudwatch/specs/datasource_specs.ts | 2 + 5 files changed, 169 insertions(+), 116 deletions(-) create mode 100644 public/app/features/dashboard/specs/time_srv.jest.ts delete mode 100644 public/app/features/dashboard/specs/time_srv_specs.ts diff --git a/public/app/features/annotations/specs/annotations_srv_specs.ts b/public/app/features/annotations/specs/annotations_srv_specs.ts index c18638e3f12..932fcf9415c 100644 --- a/public/app/features/annotations/specs/annotations_srv_specs.ts +++ b/public/app/features/annotations/specs/annotations_srv_specs.ts @@ -1,15 +1,18 @@ import { describe, beforeEach, it, expect, angularMocks } from 'test/lib/common'; import '../annotations_srv'; import helpers from 'test/specs/helpers'; +import 'app/features/dashboard/time_srv'; describe('AnnotationsSrv', function() { var ctx = new helpers.ServiceTestContext(); beforeEach(angularMocks.module('grafana.core')); beforeEach(angularMocks.module('grafana.services')); + beforeEach(ctx.createService('timeSrv')); beforeEach(() => { ctx.createService('annotationsSrv'); }); + describe('When translating the query result', () => { const annotationSource = { datasource: '-- Grafana --', diff --git a/public/app/features/dashboard/specs/time_srv.jest.ts b/public/app/features/dashboard/specs/time_srv.jest.ts new file mode 100644 index 00000000000..f8d9e42cfd4 --- /dev/null +++ b/public/app/features/dashboard/specs/time_srv.jest.ts @@ -0,0 +1,163 @@ +import { TimeSrv } from '../time_srv'; +import '../time_srv'; +import moment from 'moment'; + +describe('timeSrv', function() { + var rootScope = { + $on: jest.fn(), + onAppEvent: jest.fn(), + appEvent: jest.fn(), + }; + + var timer = { + register: jest.fn(), + cancel: jest.fn(), + cancelAll: jest.fn(), + }; + + var location = { + search: jest.fn(() => ({})), + }; + + var timeSrv; + + var _dashboard: any = { + time: { from: 'now-6h', to: 'now' }, + getTimezone: jest.fn(() => 'browser'), + }; + + beforeEach(function() { + timeSrv = new TimeSrv(rootScope, jest.fn(), location, timer, { isGrafanaVisibile: jest.fn() }); + timeSrv.init(_dashboard); + }); + + describe('timeRange', function() { + it('should return unparsed when parse is false', function() { + timeSrv.setTime({ from: 'now', to: 'now-1h' }); + var time = timeSrv.timeRange(); + expect(time.raw.from).toBe('now'); + expect(time.raw.to).toBe('now-1h'); + }); + + it('should return parsed when parse is true', function() { + timeSrv.setTime({ from: 'now', to: 'now-1h' }); + var time = timeSrv.timeRange(); + expect(moment.isMoment(time.from)).toBe(true); + expect(moment.isMoment(time.to)).toBe(true); + }); + }); + + describe('init time from url', function() { + it('should handle relative times', function() { + location = { + search: jest.fn(() => ({ + from: 'now-2d', + to: 'now', + })), + }; + + timeSrv = new TimeSrv(rootScope, jest.fn(), location, timer, { isGrafanaVisibile: jest.fn() }); + timeSrv.init(_dashboard); + var time = timeSrv.timeRange(); + expect(time.raw.from).toBe('now-2d'); + expect(time.raw.to).toBe('now'); + }); + + it('should handle formatted dates', function() { + location = { + search: jest.fn(() => ({ + from: '20140410T052010', + to: '20140520T031022', + })), + }; + + timeSrv = new TimeSrv(rootScope, jest.fn(), location, timer, { isGrafanaVisibile: jest.fn() }); + + timeSrv.init(_dashboard); + var time = timeSrv.timeRange(); + expect(time.from.valueOf()).toEqual(new Date('2014-04-10T05:20:10Z').getTime()); + expect(time.to.valueOf()).toEqual(new Date('2014-05-20T03:10:22Z').getTime()); + }); + + it('should handle formatted dates without time', function() { + location = { + search: jest.fn(() => ({ + from: '20140410', + to: '20140520', + })), + }; + + timeSrv = new TimeSrv(rootScope, jest.fn(), location, timer, { isGrafanaVisibile: jest.fn() }); + + timeSrv.init(_dashboard); + var time = timeSrv.timeRange(); + expect(time.from.valueOf()).toEqual(new Date('2014-04-10T00:00:00Z').getTime()); + expect(time.to.valueOf()).toEqual(new Date('2014-05-20T00:00:00Z').getTime()); + }); + + it('should handle epochs', function() { + location = { + search: jest.fn(() => ({ + from: '1410337646373', + to: '1410337665699', + })), + }; + + timeSrv = new TimeSrv(rootScope, jest.fn(), location, timer, { isGrafanaVisibile: jest.fn() }); + + timeSrv.init(_dashboard); + var time = timeSrv.timeRange(); + expect(time.from.valueOf()).toEqual(1410337646373); + expect(time.to.valueOf()).toEqual(1410337665699); + }); + + it('should handle bad dates', function() { + location = { + search: jest.fn(() => ({ + from: '20151126T00010%3C%2Fp%3E%3Cspan%20class', + to: 'now', + })), + }; + + timeSrv = new TimeSrv(rootScope, jest.fn(), location, timer, { isGrafanaVisibile: jest.fn() }); + + _dashboard.time.from = 'now-6h'; + timeSrv.init(_dashboard); + expect(timeSrv.time.from).toEqual('now-6h'); + expect(timeSrv.time.to).toEqual('now'); + }); + }); + + describe('setTime', function() { + it('should return disable refresh if refresh is disabled for any range', function() { + _dashboard.refresh = false; + + timeSrv.setTime({ from: '2011-01-01', to: '2015-01-01' }); + expect(_dashboard.refresh).toBe(false); + }); + + it('should restore refresh for absolute time range', function() { + _dashboard.refresh = '30s'; + + timeSrv.setTime({ from: '2011-01-01', to: '2015-01-01' }); + expect(_dashboard.refresh).toBe('30s'); + }); + + it('should restore refresh after relative time range is set', function() { + _dashboard.refresh = '10s'; + timeSrv.setTime({ + from: moment([2011, 1, 1]), + to: moment([2015, 1, 1]), + }); + expect(_dashboard.refresh).toBe(false); + timeSrv.setTime({ from: '2011-01-01', to: 'now' }); + expect(_dashboard.refresh).toBe('10s'); + }); + + it('should keep refresh after relative time range is changed and now delay exists', function() { + _dashboard.refresh = '10s'; + timeSrv.setTime({ from: 'now-1h', to: 'now-10s' }); + expect(_dashboard.refresh).toBe('10s'); + }); + }); +}); diff --git a/public/app/features/dashboard/specs/time_srv_specs.ts b/public/app/features/dashboard/specs/time_srv_specs.ts deleted file mode 100644 index 6e180679ff2..00000000000 --- a/public/app/features/dashboard/specs/time_srv_specs.ts +++ /dev/null @@ -1,115 +0,0 @@ -import { describe, beforeEach, it, expect, sinon, angularMocks } from 'test/lib/common'; - -import helpers from 'test/specs/helpers'; -import '../time_srv'; -import moment from 'moment'; - -describe('timeSrv', function() { - var ctx = new helpers.ServiceTestContext(); - var _dashboard: any = { - time: { from: 'now-6h', to: 'now' }, - getTimezone: sinon.stub().returns('browser'), - }; - - beforeEach(angularMocks.module('grafana.core')); - beforeEach(angularMocks.module('grafana.services')); - beforeEach(ctx.createService('timeSrv')); - - beforeEach(function() { - ctx.service.init(_dashboard); - }); - - describe('timeRange', function() { - it('should return unparsed when parse is false', function() { - ctx.service.setTime({ from: 'now', to: 'now-1h' }); - var time = ctx.service.timeRange(); - expect(time.raw.from).to.be('now'); - expect(time.raw.to).to.be('now-1h'); - }); - - it('should return parsed when parse is true', function() { - ctx.service.setTime({ from: 'now', to: 'now-1h' }); - var time = ctx.service.timeRange(); - expect(moment.isMoment(time.from)).to.be(true); - expect(moment.isMoment(time.to)).to.be(true); - }); - }); - - describe('init time from url', function() { - it('should handle relative times', function() { - ctx.$location.search({ from: 'now-2d', to: 'now' }); - ctx.service.init(_dashboard); - var time = ctx.service.timeRange(); - expect(time.raw.from).to.be('now-2d'); - expect(time.raw.to).to.be('now'); - }); - - it('should handle formatted dates', function() { - ctx.$location.search({ from: '20140410T052010', to: '20140520T031022' }); - ctx.service.init(_dashboard); - var time = ctx.service.timeRange(true); - expect(time.from.valueOf()).to.equal(new Date('2014-04-10T05:20:10Z').getTime()); - expect(time.to.valueOf()).to.equal(new Date('2014-05-20T03:10:22Z').getTime()); - }); - - it('should handle formatted dates without time', function() { - ctx.$location.search({ from: '20140410', to: '20140520' }); - ctx.service.init(_dashboard); - var time = ctx.service.timeRange(true); - expect(time.from.valueOf()).to.equal(new Date('2014-04-10T00:00:00Z').getTime()); - expect(time.to.valueOf()).to.equal(new Date('2014-05-20T00:00:00Z').getTime()); - }); - - it('should handle epochs', function() { - ctx.$location.search({ from: '1410337646373', to: '1410337665699' }); - ctx.service.init(_dashboard); - var time = ctx.service.timeRange(true); - expect(time.from.valueOf()).to.equal(1410337646373); - expect(time.to.valueOf()).to.equal(1410337665699); - }); - - it('should handle bad dates', function() { - ctx.$location.search({ - from: '20151126T00010%3C%2Fp%3E%3Cspan%20class', - to: 'now', - }); - _dashboard.time.from = 'now-6h'; - ctx.service.init(_dashboard); - expect(ctx.service.time.from).to.equal('now-6h'); - expect(ctx.service.time.to).to.equal('now'); - }); - }); - - describe('setTime', function() { - it('should return disable refresh if refresh is disabled for any range', function() { - _dashboard.refresh = false; - - ctx.service.setTime({ from: '2011-01-01', to: '2015-01-01' }); - expect(_dashboard.refresh).to.be(false); - }); - - it('should restore refresh for absolute time range', function() { - _dashboard.refresh = '30s'; - - ctx.service.setTime({ from: '2011-01-01', to: '2015-01-01' }); - expect(_dashboard.refresh).to.be('30s'); - }); - - it('should restore refresh after relative time range is set', function() { - _dashboard.refresh = '10s'; - ctx.service.setTime({ - from: moment([2011, 1, 1]), - to: moment([2015, 1, 1]), - }); - expect(_dashboard.refresh).to.be(false); - ctx.service.setTime({ from: '2011-01-01', to: 'now' }); - expect(_dashboard.refresh).to.be('10s'); - }); - - it('should keep refresh after relative time range is changed and now delay exists', function() { - _dashboard.refresh = '10s'; - ctx.service.setTime({ from: 'now-1h', to: 'now-10s' }); - expect(_dashboard.refresh).to.be('10s'); - }); - }); -}); diff --git a/public/app/features/dashboard/time_srv.ts b/public/app/features/dashboard/time_srv.ts index 3f7b5836653..7fd5aed7847 100644 --- a/public/app/features/dashboard/time_srv.ts +++ b/public/app/features/dashboard/time_srv.ts @@ -4,7 +4,7 @@ import coreModule from 'app/core/core_module'; import kbn from 'app/core/utils/kbn'; import * as dateMath from 'app/core/utils/datemath'; -class TimeSrv { +export class TimeSrv { time: any; refreshTimer: any; refresh: boolean; diff --git a/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts b/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts index cca14f84255..7de59fb317d 100644 --- a/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts +++ b/public/app/plugins/datasource/cloudwatch/specs/datasource_specs.ts @@ -2,6 +2,7 @@ import '../datasource'; import { describe, beforeEach, it, expect, angularMocks } from 'test/lib/common'; import helpers from 'test/specs/helpers'; import CloudWatchDatasource from '../datasource'; +import 'app/features/dashboard/time_srv'; describe('CloudWatchDatasource', function() { var ctx = new helpers.ServiceTestContext(); @@ -13,6 +14,7 @@ describe('CloudWatchDatasource', function() { beforeEach(angularMocks.module('grafana.services')); beforeEach(angularMocks.module('grafana.controllers')); beforeEach(ctx.providePhase(['templateSrv', 'backendSrv'])); + beforeEach(ctx.createService('timeSrv')); beforeEach( angularMocks.inject(function($q, $rootScope, $httpBackend, $injector) { From 24d0b43e620d3a9d9e6b7b3df4eed64edfc2588a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 19 Jun 2018 11:10:17 +0200 Subject: [PATCH 41/84] fix: fixed permission issue with api key with viewer role in dashboards with default permissions --- pkg/services/guardian/guardian.go | 2 +- pkg/services/guardian/guardian_test.go | 7 ++++++- pkg/services/guardian/guardian_util_test.go | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index bf455adc7ca..cfd8f5c3a6e 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -83,7 +83,7 @@ func (g *dashboardGuardianImpl) checkAcl(permission m.PermissionType, acl []*m.D for _, p := range acl { // user match - if !g.user.IsAnonymous { + if !g.user.IsAnonymous && p.UserId > 0 { if p.UserId == g.user.UserId && p.Permission >= permission { return true, nil } diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index 5e56b1d88c3..bd257473feb 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -162,6 +162,11 @@ func TestGuardianViewer(t *testing.T) { sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_EDIT, EDITOR_ACCESS) sc.parentFolderPermissionScenario(VIEWER, m.PERMISSION_VIEW, VIEWER_ACCESS) }) + + apiKeyScenario("Given api key with viewer role", t, m.ROLE_VIEWER, func(sc *scenarioContext) { + // dashboard has default permissions + sc.defaultPermissionScenario(VIEWER, m.PERMISSION_EDIT, VIEWER_ACCESS) + }) }) } @@ -267,7 +272,7 @@ func (sc *scenarioContext) verifyExpectedPermissionsFlags() { actualFlag = NO_ACCESS } - if sc.expectedFlags&actualFlag != sc.expectedFlags { + if actualFlag&sc.expectedFlags != actualFlag { sc.reportFailure(tc, sc.expectedFlags.String(), actualFlag.String()) } diff --git a/pkg/services/guardian/guardian_util_test.go b/pkg/services/guardian/guardian_util_test.go index b065c4194ad..3d839e71b74 100644 --- a/pkg/services/guardian/guardian_util_test.go +++ b/pkg/services/guardian/guardian_util_test.go @@ -48,6 +48,27 @@ func orgRoleScenario(desc string, t *testing.T, role m.RoleType, fn scenarioFunc }) } +func apiKeyScenario(desc string, t *testing.T, role m.RoleType, fn scenarioFunc) { + user := &m.SignedInUser{ + UserId: 0, + OrgId: orgID, + OrgRole: role, + ApiKeyId: 10, + } + guard := New(dashboardID, orgID, user) + sc := &scenarioContext{ + t: t, + orgRoleScenario: desc, + givenUser: user, + givenDashboardID: dashboardID, + g: guard, + } + + Convey(desc, func() { + fn(sc) + }) +} + func permissionScenario(desc string, dashboardID int64, sc *scenarioContext, permissions []*m.DashboardAclInfoDTO, fn scenarioFunc) { bus.ClearBusHandlers() From 5377ad4e960d7cbc418327200ea4535ce20b71b2 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 19 Jun 2018 12:34:34 +0200 Subject: [PATCH 42/84] remove unused argument in default scenario of guardian test --- pkg/services/guardian/guardian_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index bd257473feb..4704519b38d 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -28,7 +28,7 @@ func TestGuardianAdmin(t *testing.T) { Convey("Guardian admin org role tests", t, func() { orgRoleScenario("Given user has admin org role", t, m.ROLE_ADMIN, func(sc *scenarioContext) { // dashboard has default permissions - sc.defaultPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) + sc.defaultPermissionScenario(USER, FULL_ACCESS) // dashboard has user with permission sc.dashboardPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) @@ -76,6 +76,9 @@ func TestGuardianAdmin(t *testing.T) { func TestGuardianEditor(t *testing.T) { Convey("Guardian editor org role tests", t, func() { orgRoleScenario("Given user has editor org role", t, m.ROLE_EDITOR, func(sc *scenarioContext) { + // dashboard has default permissions + sc.defaultPermissionScenario(USER, EDITOR_ACCESS) + // dashboard has user with permission sc.dashboardPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) sc.dashboardPermissionScenario(USER, m.PERMISSION_EDIT, EDITOR_ACCESS) @@ -122,6 +125,9 @@ func TestGuardianEditor(t *testing.T) { func TestGuardianViewer(t *testing.T) { Convey("Guardian viewer org role tests", t, func() { orgRoleScenario("Given user has viewer org role", t, m.ROLE_VIEWER, func(sc *scenarioContext) { + // dashboard has default permissions + sc.defaultPermissionScenario(USER, VIEWER_ACCESS) + // dashboard has user with permission sc.dashboardPermissionScenario(USER, m.PERMISSION_ADMIN, FULL_ACCESS) sc.dashboardPermissionScenario(USER, m.PERMISSION_EDIT, EDITOR_ACCESS) @@ -165,12 +171,12 @@ func TestGuardianViewer(t *testing.T) { apiKeyScenario("Given api key with viewer role", t, m.ROLE_VIEWER, func(sc *scenarioContext) { // dashboard has default permissions - sc.defaultPermissionScenario(VIEWER, m.PERMISSION_EDIT, VIEWER_ACCESS) + sc.defaultPermissionScenario(VIEWER, VIEWER_ACCESS) }) }) } -func (sc *scenarioContext) defaultPermissionScenario(pt permissionType, permission m.PermissionType, flag permissionFlags) { +func (sc *scenarioContext) defaultPermissionScenario(pt permissionType, flag permissionFlags) { _, callerFile, callerLine, _ := runtime.Caller(1) sc.callerFile = callerFile sc.callerLine = callerLine From 94f39cb7343b4dc691e00e452f649f44305e8839 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Tue, 19 Jun 2018 13:15:58 +0200 Subject: [PATCH 43/84] docs: Plugin review guidelines and datasource auth pages --- .../developing/auth-for-datasources.md | 99 ++++++++++ .../developing/plugin-review-guidelines.md | 175 ++++++++++++++++++ 2 files changed, 274 insertions(+) create mode 100644 docs/sources/plugins/developing/auth-for-datasources.md create mode 100644 docs/sources/plugins/developing/plugin-review-guidelines.md diff --git a/docs/sources/plugins/developing/auth-for-datasources.md b/docs/sources/plugins/developing/auth-for-datasources.md new file mode 100644 index 00000000000..c03793e745f --- /dev/null +++ b/docs/sources/plugins/developing/auth-for-datasources.md @@ -0,0 +1,99 @@ ++++ +title = "Authentication for Datasource Plugins" +type = "docs" +[menu.docs] +name = "Authentication for Datasource Plugins" +parent = "developing" +weight = 3 ++++ + +# Authentication for Datasource Plugins + +Grafana has a proxy feature that proxies all data requests through the Grafana backend. This is very useful when your datasource plugin calls an external/thirdy-party API. The Grafana proxy adds CORS headers and can authenticate against the external API. This means that a datasource plugin that proxies all requests via Grafana can enable token authentication and the token will be renewed automatically for the user when it expires. + +The plugin config page should save the API key/password to be encrypted (using the `secureJsonData` feature) and then when a request from the datasource is made, the Grafana Proxy will: + + 1. decrypt the API key/password on the backend. + 2. carry out authentication and generate an OAuth token that will be added as an `Authorization` HTTP header to all requests (or it will add a HTTP header with the API key). + 3. renew the token if it expires. + +This means that users that access the datasource config page cannot access the API key or password after is saved the first time and that no secret keys are sent in plain text through the browser where they can be spied on. + +For backend authentication to work, the external/third-party API must either have an OAuth endpoint or that the API accepts an API key as a HTTP header for authentication. + +## Plugin Routes + +You can specify routes in the `plugin.json` file for your datasource plugin. [Here is an example](https://github.com/grafana/azure-monitor-datasource/blob/d74c82145c0a4af07a7e96cc8dde231bfd449bd9/src/plugin.json#L30-L95) with lots of routes (though most plugins will just have one route). + +When you build your url to the third-party API in your datasource class, the url should start with the text specified in the path field for a route. The proxy will strip out the path text and replace it with the value in the url field. + +For example, if my code makes a call to url `azuremonitor/foo/bar` with this code: + +```js +this.backendSrv.datasourceRequest({ + url: url, + method: 'GET', +}) +``` + +and this route: + +```json +"routes": [{ + "path": "azuremonitor", + "method": "GET", + "url": "https://management.azure.com", + ... +}] +``` + +then the Grafana proxy will transform it into "https://management.azure.com/foo/bar" and add CORS headers. + +The `method` parameter is optional. It can be set to any HTTP verb to provide more fine-grained control. + +## Encrypting Sensitive Data + +When a user saves a password or secret with your datasource plugin's Config page, then you can save data to a column in the datasource table called `secureJsonData` that is an encrypted blob. Any data saved in the blob is encrypted by Grafana and can only be decrypted by the Grafana server on the backend. This means once a password is saved, no sensitive data is sent to the browser. If the password is saved in the `jsonData` blob or the `password` field then it is unencrypted and anyone with Admin access (with the help of Chrome Developer Tools) can read it. + +This is an example of using the `secureJsonData` blob to save a property called `password`: + +```html + +``` + +## API Key/HTTP Header Authentication + +Some third-party API's accept a HTTP Header for authentication. The [example](https://github.com/grafana/azure-monitor-datasource/blob/d74c82145c0a4af07a7e96cc8dde231bfd449bd9/src/plugin.json#L91-L93) below has a `headers` section that defines the name of the HTTP Header that the API expects and it uses the `SecureJSONData` blob to fetch an encrypted API key. The Grafana server proxy will decrypt the key, add the `X-API-Key` header to the request and forward it to the third-party API. + +```json +{ + "path": "appinsights", + "method": "GET", + "url": "https://api.applicationinsights.io", + "headers": [ + {"name": "X-API-Key", "content": "{{.SecureJsonData.appInsightsApiKey}}"} + ] +} +``` + +## How Token Authentication Works + +The token auth section in the `plugin.json` file looks like this: + +```json +"tokenAuth": { + "url": "https://login.microsoftonline.com/{{.JsonData.tenantId}}/oauth2/token", + "params": { + "grant_type": "client_credentials", + "client_id": "{{.JsonData.clientId}}", + "client_secret": "{{.SecureJsonData.clientSecret}}", + "resource": "https://management.azure.com/" + } +} +``` + +This interpolates in data from both `jsonData` and `secureJsonData` to generate the token request to the third-party API. It is common for tokens to have a short expiry period (30 minutes). The proxy in Grafana server will automatically renew the token if it has expired. + +## Always Restart the Grafana Server After Route Changes + +The plugin.json files are only loaded when the Grafana server starts so when a route is added or changed then the Grafana server has to be restarted for the changes to take effect. diff --git a/docs/sources/plugins/developing/plugin-review-guidelines.md b/docs/sources/plugins/developing/plugin-review-guidelines.md new file mode 100644 index 00000000000..8efb023cf64 --- /dev/null +++ b/docs/sources/plugins/developing/plugin-review-guidelines.md @@ -0,0 +1,175 @@ ++++ +title = "Plugin Review Guidelines" +type = "docs" +[menu.docs] +name = "Plugin Review Guidelines" +parent = "developing" +weight = 2 ++++ + +# Plugin Review Guidelines + +The Grafana team reviews all plugins that are published on Grafana.com. There are two areas we review, the metadata for the plugin and the plugin functionality. + +## Metadata + +The plugin metadata consists of a `plugin.json` file and the README.md file. These `plugin.json` file is used by Grafana to load the plugin and the README.md file is shown in the plugins section of Grafana and the plugins section of Grafana.com. + +### README.md + +The README.md file is shown on the plugins page in Grafana and the plugin page on Grafana.com. There are some differences between the GitHub markdown and the markdown allowed in Grafana/Grafana.com: + +- Cannot contain inline HTML. +- Any image links should be absolute links. For example: https://raw.githubusercontent.com/grafana/azure-monitor-datasource/master/dist/img/grafana_cloud_install.png + +The README should: + +- describe the purpose of the plugin. +- contain steps on how to get started. + +### Plugin.json + +The `plugin.json` file is the same concept as the `package.json` file for an npm package. When the Grafana server starts it will scan the plugin folders (all folders in the data/plugins subfolder) and load every folder that contains a `plugin.json` file unless the folder contains a subfolder named `dist`. In that case, the Grafana server will load the `dist` folder instead. + +A minimal `plugin.json` file: + +```json +{ + "type": "panel", + "name": "Clock", + "id": "yourorg-clock-panel", + + "info": { + "description": "Clock panel for grafana", + "author": { + "name": "Author Name", + "url": "http://yourwebsite.com" + }, + "keywords": ["clock", "panel"], + "version": "1.0.0", + "updated": "2018-03-24" + }, + + "dependencies": { + "grafanaVersion": "3.x.x", + "plugins": [ ] + } +} +``` + +- The convention for the plugin id is [github username/org]-[plugin name]-[datasource|app|panel] and it has to be unique. Although if org and plugin name are the same then [plugin name]-[datasource|app|panel] is also valid. The org **cannot** be `grafana` unless it is a plugin created by the Grafana core team. + + Examples: + + - raintank-worldping-app + - ryantxu-ajax-panel + - alexanderzobnin-zabbix-app + - hawkular-datasource + +- The `type` field should be either `datasource` `app` or `panel`. +- The `version` field should be in the form: x.x.x e.g. `1.0.0` or `0.4.1`. + +The full file format for the `plugin.json` file is described [here](http://docs.grafana.org/plugins/developing/plugin.json/). + +## Plugin Language + +JavaScript, TypeScript, ES6 (or any other language) are all fine as long as the contents of the `dist` subdirectory are transpiled to JavaScript (ES5). + +## File and Directory Structure Conventions + +Here is a typical directory structure for a plugin. + +```bash +johnnyb-awesome-datasource +|-- dist +|-- src +| |-- img +| | |-- logo.svg +| |-- partials +| | |-- annotations.editor.html +| | |-- config.html +| | |-- query.editor.html +| |-- datasource.js +| |-- module.js +| |-- plugin.json +| |-- query_ctrl.js +|-- Gruntfile.js +|-- LICENSE +|-- package.json +|-- README.md +``` + +Most JavaScript projects have a build step. The generated JavaScript should be placed in the `dist` directory and the source code in the `src` directory. We recommend that the plugin.json file be placed in the src directory and then copied over to the dist directory when building. The `README.md` can be placed in the root or in the dist directory. + +Directories: + +- `src/` contains plugin source files. +- `src/partials` contains html templates. +- `src/img` contains plugin logos and other images. +- `dist/` contains built content. + +## HTML and CSS + +For the HTML on editor tabs, we recommend using the inbuilt Grafana styles rather than defining your own. This makes plugins feel like a more natural part of Grafana. If done correctly, the html will also be responsive and adapt to smaller screens. The `gf-form` css classes should be used for labels and inputs. + +Below is a minimal example of an editor row with one form group and two fields, a dropdown and a text input: + +```html +
+
+
My Plugin Options
+
+ +
+ +
+
+ + +
+
+
+
+``` + +Use the `width-x` and `max-width-x` classes to control the width of your labels and input fields. Try to get labels and input fields to line up neatly by having the same width for all the labels in a group and the same width for all inputs in a group if possible. + +## Data Sources + +A basic guide for data sources can be found [here](http://docs.grafana.org/plugins/developing/datasources/). + +### Config Page Guidelines + +- It should be as easy as possible for a user to configure a url. If the data source is using the `datasource-http-settings` component, it should use the `suggest-url` attribute to suggest the default url or a url that is similar to what it should be (especially important if the url refers to a REST endpoint that is not common knowledge for most users e.g. `https://yourserver:4000/api/custom-endpoint`). + + ```html + + + ``` + +- The `testDatasource` function should make a query to the data source that will also test that the authentication details are correct. This is so the data source is correctly configured when the user tries to write a query in a new dashboard. + +#### Password Security + +If possible, any passwords or secrets should be be saved in the `secureJsonData` blob. To encrypt sensitive data, the Grafana server's proxy feature must be used. The Grafana server has support for token authentication (OAuth) and HTTP Header authentication. If the calls have to be sent directly from the browser to a third-party API then this will not be possible and sensitive data will not be encrypted. + +Read more here about how [Authentication for Datasources]({{< relref "auth-for-datasources.md" >}}) works. + +If using the proxy feature then the Config page should use the `secureJsonData` blob like this: + + - good: `` + - bad: `` + +### Query Editor + +Each query editor is unique and can have a unique style. It should be adapted to what the users of the data source are used to. + +- Should use the Grafana CSS `gf-form` classes. +- Should be neat and tidy. Labels and fields in columns should be aligned and should be the same width if possible. +- The datasource should be able to handle when a user toggles a query (by clicking on the eye icon) and not execute the query. This is done by checking the `hide` property - an [example](https://github.com/grafana/grafana/blob/master/public/app/plugins/datasource/postgres/datasource.ts#L35-L38). +- Should not execute queries if fields in the Query Editor are empty and the query will throw an exception (defensive programming). +- Should handle errors. There are two main ways to do this: + - use the notification system in Grafana to show a toaster popup with the error message. Example [here](https://github.com/alexanderzobnin/grafana-zabbix/blob/fdbbba2fb03f5f2a4b3b0715415e09d5a4cf6cde/src/panel-triggers/triggers_panel_ctrl.js#L467-L471). + - provide an error notification in the query editor like the MySQL/Postgres data sources do. Example code in the `query_ctrl` [here](https://github.com/grafana/azure-monitor-datasource/blob/b184d077f082a69f962120ef0d1f8296a0d46f03/src/query_ctrl.ts#L36-L51) and in the [html](https://github.com/grafana/azure-monitor-datasource/blob/b184d077f082a69f962120ef0d1f8296a0d46f03/src/partials/query.editor.html#L190-L193). From a60332d459f311015d3988700f9aaa89afabb6ef Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 19 Jun 2018 13:54:37 +0200 Subject: [PATCH 44/84] changelog: add notes about closing #12343 [skip ci] --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1202688b96..b353ebdd269 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,10 @@ * **Dashboard list panel**: Search dashboards by folder [#11525](https://github.com/grafana/grafana/issues/11525) * **Sidenav**: Always show server admin link in sidenav if grafana admin [#11657](https://github.com/grafana/grafana/issues/11657) +# 5.1.4 (2018-06-19) + +* **Permissions**: Important security fix for API keys with viewer role [#12343](https://github.com/grafana/grafana/issues/12343) + # 5.1.3 (2018-05-16) * **Scroll**: Graph panel / legend texts shifts on the left each time we move scrollbar on firefox [#11830](https://github.com/grafana/grafana/issues/11830) From 40d760622e3e8403d868d1b43c4259455d45b620 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Tue, 19 Jun 2018 14:30:10 +0200 Subject: [PATCH 45/84] ldap: add note to dockerfile --- docker/blocks/openldap/Dockerfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docker/blocks/openldap/Dockerfile b/docker/blocks/openldap/Dockerfile index 54e383a6a97..c9b928ad56a 100644 --- a/docker/blocks/openldap/Dockerfile +++ b/docker/blocks/openldap/Dockerfile @@ -1,3 +1,5 @@ +# Fork of https://github.com/dinkel/docker-openldap + FROM debian:jessie LABEL maintainer="Christian LuginbĂĽhl " From e6c5a5a905d086aae555cae8c8e24623445f260d Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Tue, 19 Jun 2018 14:40:29 +0200 Subject: [PATCH 46/84] ldap: add note about config in Grafana --- docker/blocks/openldap/notes.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docker/blocks/openldap/notes.md b/docker/blocks/openldap/notes.md index 71813c2899a..483266f0d88 100644 --- a/docker/blocks/openldap/notes.md +++ b/docker/blocks/openldap/notes.md @@ -11,3 +11,14 @@ After adding ldif files to `prepopulate`: 1. Remove your current docker image: `docker rm docker_openldap_1` 2. Build: `docker-compose build` 3. `docker-compose up` + +## Enabling LDAP in Grafana + +The default `ldap.toml` file in `conf` has host set to `127.0.0.1` and port to set to 389 so all you need to do is enable it in the .ini file to get Grafana to use this block: + +```ini +[auth.ldap] +enabled = true +config_file = conf/ldap.toml +; allow_sign_up = true +``` From f73c04086cfdbc32bc0aec23d83bcd92f1b4afb1 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 19 Jun 2018 15:00:45 +0200 Subject: [PATCH 47/84] docs: update installation instructions --- docs/sources/installation/debian.md | 6 +++--- docs/sources/installation/rpm.md | 10 +++++----- docs/sources/installation/windows.md | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/sources/installation/debian.md b/docs/sources/installation/debian.md index 3025b2384df..2c847b10471 100644 --- a/docs/sources/installation/debian.md +++ b/docs/sources/installation/debian.md @@ -15,7 +15,7 @@ weight = 1 Description | Download ------------ | ------------- -Stable for Debian-based Linux | [grafana_5.1.3_amd64.deb](https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana_5.1.3_amd64.deb) +Stable for Debian-based Linux | [grafana_5.1.4_amd64.deb](https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana_5.1.4_amd64.deb) @@ -27,9 +27,9 @@ installation. ```bash -wget https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana_5.1.3_amd64.deb +wget https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana_5.1.4_amd64.deb sudo apt-get install -y adduser libfontconfig -sudo dpkg -i grafana_5.1.3_amd64.deb +sudo dpkg -i grafana_5.1.4_amd64.deb ``` @@ -28,7 +28,7 @@ installation. You can install Grafana using Yum directly. ```bash -$ sudo yum install https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.1.3-1.x86_64.rpm +$ sudo yum install https://s3-us-west-2.amazonaws.com/grafana-releases/release/grafana-5.1.4-1.x86_64.rpm ```