diff --git a/.drone.star b/.drone.star index c00c11ac5bf..e741fa72f67 100644 --- a/.drone.star +++ b/.drone.star @@ -23,8 +23,6 @@ def main(ctx): + release_pipelines() + publish_image_pipelines_public() + publish_image_pipelines_security() - + publish_github_pipeline('public') - + publish_github_pipeline('security') + publish_artifacts_pipelines('security') + publish_artifacts_pipelines('public') + publish_npm_pipelines('public') diff --git a/.drone.yml b/.drone.yml index 845339558b6..131ef5c1da6 100644 --- a/.drone.yml +++ b/.drone.yml @@ -3545,128 +3545,6 @@ volumes: clone: retries: 3 depends_on: [] -environment: - EDITION: enterprise2 -image_pull_secrets: -- dockerconfigjson -kind: pipeline -name: publish-github-public -node: - type: no-parallel -platform: - arch: amd64 - os: linux -services: [] -steps: -- commands: - - go build -o ./bin/build -ldflags '-extldflags -static' ./pkg/build/cmd - depends_on: [] - environment: - CGO_ENABLED: 0 - image: golang:1.19.4 - name: compile-build-cmd -- commands: - - ./bin/build artifacts docker fetch --edition enterprise2 - depends_on: - - compile-build-cmd - environment: - DOCKER_PASSWORD: - from_secret: docker_password - DOCKER_USER: - from_secret: docker_username - GCP_KEY: - from_secret: gcp_key - image: google/cloud-sdk - name: fetch-images-enterprise2 - volumes: - - name: docker - path: /var/run/docker.sock -- commands: - - ./bin/build publish github --repo $${GH_REGISTRY} --create - depends_on: - - fetch-images-enterprise2 - environment: - GH_REGISTRY: - from_secret: gh_registry - GH_TOKEN: - from_secret: github_token - image: grafana/grafana-ci-deploy:1.3.3 - name: publish-github -trigger: - event: - - promote - target: - - public -type: docker -volumes: -- host: - path: /var/run/docker.sock - name: docker ---- -clone: - retries: 3 -depends_on: [] -environment: - EDITION: enterprise2 -image_pull_secrets: -- dockerconfigjson -kind: pipeline -name: publish-github-security -node: - type: no-parallel -platform: - arch: amd64 - os: linux -services: [] -steps: -- commands: - - go build -o ./bin/build -ldflags '-extldflags -static' ./pkg/build/cmd - depends_on: [] - environment: - CGO_ENABLED: 0 - image: golang:1.19.4 - name: compile-build-cmd -- commands: - - ./bin/build artifacts docker fetch --edition enterprise2 - depends_on: - - compile-build-cmd - environment: - DOCKER_PASSWORD: - from_secret: docker_password - DOCKER_USER: - from_secret: docker_username - GCP_KEY: - from_secret: gcp_key - image: google/cloud-sdk - name: fetch-images-enterprise2 - volumes: - - name: docker - path: /var/run/docker.sock -- commands: - - ./bin/build publish github --repo $${GH_REGISTRY} --create - depends_on: - - fetch-images-enterprise2 - environment: - GH_REGISTRY: - from_secret: gh_registry - GH_TOKEN: - from_secret: github_token - image: grafana/grafana-ci-deploy:1.3.3 - name: publish-github -trigger: - event: - - promote - target: - - security -type: docker -volumes: -- host: - path: /var/run/docker.sock - name: docker ---- -clone: - retries: 3 -depends_on: [] image_pull_secrets: - dockerconfigjson kind: pipeline @@ -5643,6 +5521,6 @@ kind: secret name: aws_secret_access_key --- kind: signature -hmac: 6941895851aab38b5181aaed1ee43f9caeebc4c33fb3666762e2b5ca60c8d35d +hmac: 631d09c1517d776c809ee05208df63c9eb23279a5408f207162ea706c4e2072f ... diff --git a/CHANGELOG.md b/CHANGELOG.md index c3e61f94d8d..d3827883fca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,30 @@ + + +# 9.3.4 (2023-01-25) + +### Features and enhancements + +- **Prometheus:** Add default editor configuration. [#61510](https://github.com/grafana/grafana/pull/61510), [@itsmylife](https://github.com/itsmylife) +- **TextPanel:** Refactor to functional component (#60885). [#61937](https://github.com/grafana/grafana/pull/61937), [@ryantxu](https://github.com/ryantxu) + +### Bug fixes + +- **Alerting:** Fix webhook to use correct key for decrypting token. [#61717](https://github.com/grafana/grafana/pull/61717), [@yuri-tceretian](https://github.com/yuri-tceretian) +- **Alerting:** Set error annotation on EvaluationError regardless of underlying error type. [#61506](https://github.com/grafana/grafana/pull/61506), [@alexweav](https://github.com/alexweav) +- **Datasources:** Fix Proxy by UID Failing for UIDs with a Hyphen. [#61723](https://github.com/grafana/grafana/pull/61723), [@csmarchbanks](https://github.com/csmarchbanks) +- **Elasticsearch:** Fix creating of span link with no tags. [#61753](https://github.com/grafana/grafana/pull/61753), [@ivanahuckova](https://github.com/ivanahuckova) +- **Elasticsearch:** Fix failing requests when using SigV4. [#61923](https://github.com/grafana/grafana/pull/61923), [@svennergr](https://github.com/svennergr) +- **Elasticsearch:** Fix toggle-settings are not shown correctly. [#61751](https://github.com/grafana/grafana/pull/61751), [@svennergr](https://github.com/svennergr) +- **Explore:** Be sure time range key bindings are mounted after clear. [#61892](https://github.com/grafana/grafana/pull/61892), [@gelicia](https://github.com/gelicia) +- **Explore:** Unsync time ranges when a pane is closed. [#61369](https://github.com/grafana/grafana/pull/61369), [@Elfo404](https://github.com/Elfo404) +- **Logs:** Lines with long words do not break properly. [#61707](https://github.com/grafana/grafana/pull/61707), [@svennergr](https://github.com/svennergr) +- **Loki:** Fix misaligned derived fields settings. [#61475](https://github.com/grafana/grafana/pull/61475), [@svennergr](https://github.com/svennergr) +- **Query Builder:** Fix max width of input component to prevent overflows. [#61798](https://github.com/grafana/grafana/pull/61798), [@matyax](https://github.com/matyax) +- **Search:** Auto focus input elements. [#61443](https://github.com/grafana/grafana/pull/61443), [@ryantxu](https://github.com/ryantxu) +- **Search:** Fix empty folder message showing when by starred dashboards. [#61610](https://github.com/grafana/grafana/pull/61610), [@eledobleefe](https://github.com/eledobleefe) +- **Table Panel:** Fix image of image cell overflowing table cell and cells ignoring text alignment setting when a data link is added. [#59392](https://github.com/grafana/grafana/pull/59392), [@oscarkilhed](https://github.com/oscarkilhed) + + # 9.3.2 (unreleased) diff --git a/docs/sources/datasources/azure-monitor/_index.md b/docs/sources/datasources/azure-monitor/_index.md index ea4ba6753f2..cc5a98c11d0 100644 --- a/docs/sources/datasources/azure-monitor/_index.md +++ b/docs/sources/datasources/azure-monitor/_index.md @@ -157,4 +157,4 @@ Until Grafana v8.0, you could query the same Azure Application Insights data usi These queries were deprecated in Grafana v7.5. In Grafana v8.0, Application Insights and Insights Analytics were made read-only in favor of querying this data through Metrics and Logs. These query methods were completely removed in Grafana v9.0. -If you're upgrading from a Grafana version prior to v9.0 and relied on Application Insights and Analytics queries, refer to the [Grafana v9.0 documentation](/v9.0/datasources/azuremonitor/deprecated-application-insights/) for help migrating these queries to Metrics and Logs queries. +If you're upgrading from a Grafana version prior to v9.0 and relied on Application Insights and Analytics queries, refer to the [Grafana v9.0 documentation](/docs/grafana/v9.0/datasources/azuremonitor/deprecated-application-insights/) for help migrating these queries to Metrics and Logs queries. diff --git a/docs/sources/developers/plugins/package-a-plugin.md b/docs/sources/developers/plugins/package-a-plugin.md index c37835d9943..885ed3fbd15 100644 --- a/docs/sources/developers/plugins/package-a-plugin.md +++ b/docs/sources/developers/plugins/package-a-plugin.md @@ -98,8 +98,28 @@ To speed up the time it takes to review your plugin: - We're not able to give an estimate at this time, though we're constantly working on improving the time it takes to review a plugin. +**Can I see metrics of my plugin installs, downloads or usage?** + +- No. We don't offer this information at the moment to plugin authors. + +**How can I update my plugin's catalog page?** + +- The plugin's catalog page content is extracted from the plugin README file. To update the plugin's catalog page, it is necessary to submit an updated plugin with the new content included in the README file. + +**Can I unlist my plugin from the Grafana Plugin's Catalog in case of a bug?** + +- In the event of a bug, unlisting the plugin from the Grafana Plugin's Catalog may be possible in exceptional cases, such as security concerns. However, we do not have control over the specific instances where the plugin is installed. + +**Can I distribute my plugin somewhere else than the Grafana Catalog?** + +- The official method for distributing Grafana plugins is through the Grafana Catalog. Alternative methods, such as installing private or development plugins on local Grafana instances, are available as per the guidelines provided in [this guide](https://grafana.com/docs/grafana/latest/administration/plugin-management/#install-plugin-on-local-grafana). + ## Publishing your plugin for the first time +**Do plugin signatures expire?** + +- Plugin signatures do not currently expire. + {{< figure src="/static/img/docs/plugins/plugins-submission-create2.png" class="docs-image--no-shadow" max-width="650px" >}} 1. [Sign in](https://grafana.com/auth/sign-in) to your Grafana Cloud account. diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md index 60a4210c942..c14268ffcd8 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md @@ -39,6 +39,7 @@ auth_url = https://accounts.google.com/o/oauth2/auth token_url = https://accounts.google.com/o/oauth2/token allowed_domains = mycompany.com mycompany.org allow_sign_up = true +hosted_domain = mycompany.com ``` You may have to set the `root_url` option of `[server]` for the callback URL to be @@ -53,6 +54,9 @@ You may allow users to sign-up via Google authentication by setting the user successfully authenticating via Google authentication will be automatically signed up. +You may specify a domain to be passed as `hd` query parameter accepted by Google's +OAuth 2.0 authentication API. Refer to Google's OAuth [documentation](https://developers.google.com/identity/openid-connect/openid-connect#hd-param). + ### Configure refresh token > Available in Grafana v9.3 and later versions. diff --git a/docs/sources/setup-grafana/upgrade-grafana.md b/docs/sources/setup-grafana/upgrade-grafana.md index a379fd75a64..40297fc2d7e 100644 --- a/docs/sources/setup-grafana/upgrade-grafana.md +++ b/docs/sources/setup-grafana/upgrade-grafana.md @@ -13,33 +13,33 @@ weight: 500 # Upgrade Grafana -We recommend that you upgrade Grafana often to stay up to date with the latest fixes and enhancements. -In order to make this a reality, Grafana upgrades are backward compatible and the upgrade process is simple and quick. +We recommend that you upgrade Grafana often to stay current with the latest fixes and enhancements. +Because Grafana upgrades are backward compatible, the upgrade process is straightforward. Upgrading between many minor versions and one major version is generally safe, and dashboards and graphs will not change. -Upgrading between many minor versions and one major version is generally safe, and dashboards and graphs will look the same. -There might be minor breaking changes in some releases. -We outline these in the [What's New overviews]({{< relref "../whatsnew/" >}}) for each release. -For versions of Grafana prior to v9.2, we also published additional information in the [Release Notes]({{< relref "../release-notes/" >}}). -We also list all changes, with links to pull requests or issues when available, in the [Changelog](https://github.com/grafana/grafana/blob/main/CHANGELOG.md). +In addition to common tasks you should complete for all versions of Grafana, there might be additional upgrade tasks to complete for a version. -## Backup +> **Note:** There might be minor breaking changes in some releases. We outline these in the [What's New ]({{< relref "../whatsnew/" >}}) document for each release. -We recommend that you backup a few things in case you have to rollback the upgrade. +For versions of Grafana prior to v9.2, we published additional information in the [Release Notes]({{< relref "../release-notes/" >}}). -- Installed plugins - Back them up before you upgrade them in case you want to rollback the Grafana version and want to get the exact same versions you were running before the upgrade. -- Configuration files do not need to be backed up. However, you might want to in case you add new configuration options after upgrade and then rollback. +When available, we list all changes with links to pull requests or issues in the [Changelog](https://github.com/grafana/grafana/blob/main/CHANGELOG.md). -### Database backup +> **Note:** When possible, we recommend that you test the Grafana upgrade process in a test or development environment. -Before upgrading it can be a good idea to backup your Grafana database. This will ensure that you can always rollback to your previous version. During startup, Grafana will automatically migrate the database schema (if there are changes or new tables). Sometimes this can cause issues if you later want to downgrade. +## Back up the Grafana database -#### sqlite +Although Grafana automatically upgrades the database on startup, we recommend that you back up your Grafana database so that you can roll back to a previous version, if required. -If you use sqlite you only need to make a backup of your `grafana.db` file. This is usually located at `/var/lib/grafana/grafana.db` on Unix systems. -If you are unsure what database you use and where it is stored check you grafana configuration file. If you -installed grafana to custom location using a binary tar/zip it is usually in `/data`. +### sqlite -#### mysql +If you use sqlite, you only need to back up the `grafana.db` file. On Unix systems, the database file is usually located in `/var/lib/grafana/`. + +If you are unsure which database you use and where it is stored, check the Grafana configuration file. If you +installed Grafana to a custom location using a binary tar/zip, the database is usually located in `/data`. + +### mysql + +To back up or restore a mysql Grafana database, run the following commands: ```bash backup: @@ -49,7 +49,9 @@ restore: > mysql -u root -p grafana < grafana_backup.sql ``` -#### postgres +### postgres + +To back up or restore a postgres Grafana database, run the following commands: ```bash backup: @@ -59,70 +61,117 @@ restore: > psql grafana < grafana_backup ``` -### Ubuntu or Debian +## Backup plugins -You can upgrade Grafana by following the same procedure as when you installed it. +We recommend that you back up installed plugins before you upgrade Grafana so that you can roll back to a previous version of Grafana, if necessary. -#### Upgrade Debian package +## Upgrade Grafana -If you installed Grafana by downloading a Debian package (`.deb`), then you can execute the same `dpkg -i` command but with the new package. It will upgrade your Grafana installation. +The following sections provide instructions for how to upgrade Grafana based on your installation method. -Go to the [download page](https://grafana.com/grafana/download?platform=linux) for the latest download -links. +### Debian -```bash -wget -sudo apt-get install -y adduser -sudo dpkg -i grafana__amd64.deb -``` +To upgrade Grafana installed from a Debian package (`.deb`), complete the following steps: -#### Upgrade from APT repository +1. In your current installation of Grafana, save your custom configuration changes to a file named `/conf/custom.ini`. -If you installed Grafana from our APT repository, then Grafana will automatically update when you run apt-get upgrade to upgrade all system packages. + This enables you to upgrade Grafana without the risk of losing your configuration changes. -```bash -sudo apt-get update -sudo apt-get upgrade -``` +1. [Download](https://grafana.com/grafana/download?platform=linux) the latest version of Grafana. -#### Upgrade from binary .tar file +1. Execute the `dpkg -i` command. -If you downloaded the binary `.tar.gz` package, then you can just download and extract the new package and overwrite all your existing files. However, this might overwrite your config changes. + ```bash + wget + sudo apt-get install -y adduser + sudo dpkg -i grafana__amd64.deb + ``` -We recommend that you save your custom configuration changes in a file named `/conf/custom.ini`. -This allows you to upgrade Grafana without risking losing your configuration changes. +### APT repository -### Centos / RHEL +To upgrade Grafana installed from the Grafana Labs APT repository, complete the following steps: -If you installed Grafana by downloading an RPM package you can just follow the same installation guide and execute the same `yum install` or `rpm -i` command but with the new package. It will upgrade your Grafana installation. +1. In your current installation of Grafana, save your custom configuration changes to a file named `/conf/custom.ini`. -If you used our YUM repository: + This enables you to upgrade Grafana without the risk of losing your configuration changes. -```bash -sudo yum update grafana -``` +1. Run the following command. + + ```bash + sudo apt-get update + sudo apt-get upgrade + ``` + +Grafana automatically updates when you run `apt-get upgrade`. + +### Binary .tar file + +To upgrade Grafana installed from the binary `.tar.gz` package, complete the following steps: + +1. In your current installation of Grafana, save your custom configuration changes to a file named `/conf/custom.ini`. + + This enables you to upgrade Grafana without the risk of losing your configuration changes. + +1. [Download](https://grafana.com/grafana/download) the binary `.tar.gz` package. + +1. Extract the downloaded package and overwrite the existing files. + +### Centos or RHEL + +To upgrade Grafana running on Centos or RHEL, complete the following steps: + +1. In your current installation of Grafana, save your custom configuration changes to a file named `/conf/custom.ini`. + + This enables you to upgrade Grafana without the risk of losing your configuration changes. + +1. Perform one of the following steps based on your installation. + + - If you [downloaded an RPM package](https://grafana.com/grafana/download) to install Grafana, then complete the steps documented in [Install on RPM-based Linux]({{< relref "./installation/rpm" >}}) to upgrade Grafana. + - If you used the Grafana YUM repository, execute the following command: + + ```bash + sudo yum update grafana + ``` ### Docker -This just an example, details depend on how you configured your grafana container. +To upgrade Grafana running in a Docker container, complete the following steps: -```bash -docker pull grafana/grafana -docker stop my-grafana-container -docker rm my-grafana-container -docker run -d --name=my-grafana-container --restart=always -v /var/lib/grafana:/var/lib/grafana grafana/grafana -``` +1. In your current installation of Grafana, save your custom configuration changes to a file named `/conf/custom.ini`. + + This enables you to upgrade Grafana without the risk of losing your configuration changes. + +1. Run a command similar to the following command. + + > **Note:** This is an example. The parameters you enter depend on how you configured your Grafana container. + + ```bash + docker pull grafana/grafana + docker stop my-grafana-container + docker rm my-grafana-container + docker run -d --name=my-grafana-container --restart=always -v /var/lib/grafana:/var/lib/grafana grafana/grafana + ``` ### Windows -If you downloaded the Windows binary package you can just download a newer package and extract to the same location (and overwrite the existing files). This might overwrite your configuration changes. We recommend that you save your configuration changes in a file named `/conf/custom.ini` as this will make upgrades easier without risking losing your configuration changes. +To upgrade Grafana installed on Windows, complete the following steps: -## Update plugins +1. In your current installation of Grafana, save your custom configuration changes to a file named `/conf/custom.ini`. -After you have upgraded, we strongly recommend that you update all your plugins as a new version of Grafana + This enables you to upgrade Grafana without the risk of losing your configuration changes. + +1. [Download](https://grafana.com/grafana/download) the Windows binary package. + +1. Extract the contents of the package to the location in which you installed Grafana. + + You can overwrite existing files and folders, when prompted. + +## Update Grafana plugins + +After you upgrade Grafana, we recommend that you update all plugins because a new version of Grafana can make older plugins stop working properly. -You can update all plugins using +Run the following command to update plugins: ```bash grafana-cli plugins update-all diff --git a/packages/grafana-ui/src/components/index.ts b/packages/grafana-ui/src/components/index.ts index be1987ed037..aae60abf206 100644 --- a/packages/grafana-ui/src/components/index.ts +++ b/packages/grafana-ui/src/components/index.ts @@ -176,6 +176,7 @@ export { } from './ErrorBoundary/ErrorBoundary'; export { ErrorWithStack } from './ErrorBoundary/ErrorWithStack'; export { DataSourceHttpSettings } from './DataSourceSettings/DataSourceHttpSettings'; +export { CustomHeadersSettings } from './DataSourceSettings/CustomHeadersSettings'; export { AlertingSettings } from './DataSourceSettings/AlertingSettings'; export { TLSAuthSettings } from './DataSourceSettings/TLSAuthSettings'; export { CertificationKey } from './DataSourceSettings/CertificationKey'; diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 603f158611a..84bb08d7fe4 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "net/http" - "net/url" "golang.org/x/oauth2" @@ -258,7 +257,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) { hs.HooksService.RunLoginHook(&loginInfo, ctx) metrics.MApiLoginOAuth.Inc() - if redirectTo, err := url.QueryUnescape(ctx.GetCookie("redirect_to")); err == nil && len(redirectTo) > 0 { + if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 { if err := hs.ValidateRedirectTo(redirectTo); err == nil { cookies.DeleteCookie(ctx.Resp, "redirect_to", hs.CookieOptionsFromCfg) ctx.Redirect(redirectTo) diff --git a/pkg/services/datasourceproxy/datasourceproxy.go b/pkg/services/datasourceproxy/datasourceproxy.go index 0590376c741..b6680121801 100644 --- a/pkg/services/datasourceproxy/datasourceproxy.go +++ b/pkg/services/datasourceproxy/datasourceproxy.go @@ -131,7 +131,7 @@ func (p *DataSourceProxyService) proxyDatasourceRequest(c *models.ReqContext, ds proxy.HandleRequest() } -var proxyPathRegexp = regexp.MustCompile(`^\/api\/datasources\/proxy\/([\d]+|uid\/[\w]+)\/?`) +var proxyPathRegexp = regexp.MustCompile(`^\/api\/datasources\/proxy\/([\d]+|uid\/[\w-]+)\/?`) func extractProxyPath(originalRawPath string) string { return proxyPathRegexp.ReplaceAllString(originalRawPath, "") diff --git a/pkg/services/datasourceproxy/datasourceproxy_test.go b/pkg/services/datasourceproxy/datasourceproxy_test.go index 6567ba579c5..57da4aa33c0 100644 --- a/pkg/services/datasourceproxy/datasourceproxy_test.go +++ b/pkg/services/datasourceproxy/datasourceproxy_test.go @@ -32,6 +32,14 @@ func TestDataProxy(t *testing.T) { "/api/datasources/proxy/uid/26MI0wZ7k/some/thing", "some/thing", }, + { + "/api/datasources/proxy/uid/pUWo-no4k/search", + "search", + }, + { + "/api/datasources/proxy/uid/pUWo_no4k/search", + "search", + }, { "/api/datasources/proxy/uid/26MI0wZ7k/api/services/afsd%2Fafsd/operations", "api/services/afsd%2Fafsd/operations", diff --git a/pkg/services/secrets/database/database.go b/pkg/services/secrets/database/database.go index 74cf5475fcd..5511d4832c7 100644 --- a/pkg/services/secrets/database/database.go +++ b/pkg/services/secrets/database/database.go @@ -5,8 +5,6 @@ import ( "fmt" "time" - "xorm.io/xorm" - "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/kmsproviders" @@ -16,22 +14,24 @@ import ( const dataKeysTable = "data_keys" type SecretsStoreImpl struct { - sqlStore db.DB - log log.Logger + db db.DB + log log.Logger } -func ProvideSecretsStore(sqlStore db.DB) *SecretsStoreImpl { - return &SecretsStoreImpl{ - sqlStore: sqlStore, - log: log.New("secrets.store"), +func ProvideSecretsStore(db db.DB) *SecretsStoreImpl { + store := &SecretsStoreImpl{ + db: db, + log: log.New("secrets.store"), } + + return store } func (ss *SecretsStoreImpl) GetDataKey(ctx context.Context, id string) (*secrets.DataKey, error) { dataKey := &secrets.DataKey{} var exists bool - err := ss.sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { var err error exists, err = sess.Table(dataKeysTable). Where("name = ?", id). @@ -54,10 +54,10 @@ func (ss *SecretsStoreImpl) GetCurrentDataKey(ctx context.Context, label string) dataKey := &secrets.DataKey{} var exists bool - err := ss.sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { var err error exists, err = sess.Table(dataKeysTable). - Where("label = ? AND active = ?", label, ss.sqlStore.GetDialect().BooleanStr(true)). + Where("label = ? AND active = ?", label, ss.db.GetDialect().BooleanStr(true)). Get(dataKey) return err }) @@ -75,7 +75,7 @@ func (ss *SecretsStoreImpl) GetCurrentDataKey(ctx context.Context, label string) func (ss *SecretsStoreImpl) GetAllDataKeys(ctx context.Context) ([]*secrets.DataKey, error) { result := make([]*secrets.DataKey, 0) - err := ss.sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { err := sess.Table(dataKeysTable).Find(&result) return err }) @@ -83,12 +83,6 @@ func (ss *SecretsStoreImpl) GetAllDataKeys(ctx context.Context) ([]*secrets.Data } func (ss *SecretsStoreImpl) CreateDataKey(ctx context.Context, dataKey *secrets.DataKey) error { - return ss.sqlStore.WithDbSession(ctx, func(sess *db.Session) error { - return ss.CreateDataKeyWithDBSession(ctx, dataKey, sess.Session) - }) -} - -func (ss *SecretsStoreImpl) CreateDataKeyWithDBSession(_ context.Context, dataKey *secrets.DataKey, sess *xorm.Session) error { if !dataKey.Active { return fmt.Errorf("cannot insert deactivated data keys") } @@ -96,14 +90,20 @@ func (ss *SecretsStoreImpl) CreateDataKeyWithDBSession(_ context.Context, dataKe dataKey.Created = time.Now() dataKey.Updated = dataKey.Created - _, err := sess.Table(dataKeysTable).Insert(dataKey) - return err + return ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + _, err := sess.Table(dataKeysTable).Insert(dataKey) + if err != nil { + return err + } + + return nil + }) } func (ss *SecretsStoreImpl) DisableDataKeys(ctx context.Context) error { - return ss.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { _, err := sess.Table(dataKeysTable). - Where("active = ?", ss.sqlStore.GetDialect().BooleanStr(true)). + Where("active = ?", ss.db.GetDialect().BooleanStr(true)). UseBool("active").Update(&secrets.DataKey{Active: false}) return err }) @@ -114,7 +114,7 @@ func (ss *SecretsStoreImpl) DeleteDataKey(ctx context.Context, id string) error return fmt.Errorf("data key id is missing") } - return ss.sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + return ss.db.WithDbSession(ctx, func(sess *db.Session) error { _, err := sess.Table(dataKeysTable).Delete(&secrets.DataKey{Id: id}) return err @@ -127,14 +127,14 @@ func (ss *SecretsStoreImpl) ReEncryptDataKeys( currProvider secrets.ProviderID, ) error { keys := make([]*secrets.DataKey, 0) - if err := ss.sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + if err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { return sess.Table(dataKeysTable).Find(&keys) }); err != nil { return err } for _, k := range keys { - err := ss.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + err := ss.db.WithTransactionalDbSession(ctx, func(sess *db.Session) error { provider, ok := providers[kmsproviders.NormalizeProviderID(k.Provider)] if !ok { ss.log.Warn( diff --git a/pkg/services/secrets/fakes/fake_store.go b/pkg/services/secrets/fakes/fake_store.go index 12650a87615..5b4080c3d50 100644 --- a/pkg/services/secrets/fakes/fake_store.go +++ b/pkg/services/secrets/fakes/fake_store.go @@ -4,7 +4,6 @@ import ( "context" "github.com/grafana/grafana/pkg/services/secrets" - "xorm.io/xorm" ) type FakeSecretsStore struct { @@ -47,11 +46,6 @@ func (f FakeSecretsStore) CreateDataKey(_ context.Context, dataKey *secrets.Data return nil } -func (f FakeSecretsStore) CreateDataKeyWithDBSession(_ context.Context, dataKey *secrets.DataKey, _ *xorm.Session) error { - f.store[dataKey.Id] = dataKey - return nil -} - func (f FakeSecretsStore) DisableDataKeys(_ context.Context) error { for id := range f.store { f.store[id].Active = false diff --git a/pkg/services/secrets/manager/cache.go b/pkg/services/secrets/manager/cache.go index 7ea65d11ec6..1aca3a36113 100644 --- a/pkg/services/secrets/manager/cache.go +++ b/pkg/services/secrets/manager/cache.go @@ -8,10 +8,6 @@ import ( "github.com/prometheus/client_golang/prometheus" ) -var ( - now = time.Now -) - type dataKeyCacheEntry struct { id string label string @@ -75,13 +71,21 @@ func (c *dataKeyCache) getByLabel(label string) (*dataKeyCacheEntry, bool) { return entry, true } -func (c *dataKeyCache) add(entry *dataKeyCacheEntry) { +func (c *dataKeyCache) addById(entry *dataKeyCacheEntry) { c.mtx.Lock() defer c.mtx.Unlock() entry.expiration = now().Add(c.cacheTTL) c.byId[entry.id] = entry +} + +func (c *dataKeyCache) addByLabel(entry *dataKeyCacheEntry) { + c.mtx.Lock() + defer c.mtx.Unlock() + + entry.expiration = now().Add(c.cacheTTL) + c.byLabel[entry.label] = entry } diff --git a/pkg/services/secrets/manager/manager.go b/pkg/services/secrets/manager/manager.go index 5d7581a9da2..c9c5c2b9967 100644 --- a/pkg/services/secrets/manager/manager.go +++ b/pkg/services/secrets/manager/manager.go @@ -19,15 +19,21 @@ import ( "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" + "github.com/prometheus/client_golang/prometheus" "golang.org/x/sync/errgroup" - "xorm.io/xorm" ) const ( keyIdDelimiter = '#' ) +var ( + // now is used for testing purposes, + // as a way to fake time.Now function. + now = time.Now +) + type SecretsService struct { store secrets.Store enc encryption.Internal @@ -152,10 +158,6 @@ func (s *SecretsService) encryptedWithEnvelopeEncryption(payload []byte) bool { var b64 = base64.RawStdEncoding func (s *SecretsService) Encrypt(ctx context.Context, payload []byte, opt secrets.EncryptionOptions) ([]byte, error) { - return s.EncryptWithDBSession(ctx, payload, opt, nil) -} - -func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byte, opt secrets.EncryptionOptions, sess *xorm.Session) ([]byte, error) { // Use legacy encryption service if featuremgmt.FlagDisableEnvelopeEncryption toggle is on if s.features.IsEnabled(featuremgmt.FlagDisableEnvelopeEncryption) { return s.enc.Encrypt(ctx, payload, setting.SecretKey) @@ -175,7 +177,7 @@ func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byt var id string var dataKey []byte - id, dataKey, err = s.currentDataKey(ctx, label, scope, sess) + id, dataKey, err = s.currentDataKey(ctx, label, scope) if err != nil { s.log.Error("Failed to get current data key", "error", err, "label", label) return nil, err @@ -203,7 +205,7 @@ func (s *SecretsService) EncryptWithDBSession(ctx context.Context, payload []byt // currentDataKey looks up for current data key in cache or database by name, and decrypts it. // If there's no current data key in cache nor in database it generates a new random data key, // and stores it into both the in-memory cache and database (encrypted by the encryption provider). -func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope string, sess *xorm.Session) (string, []byte, error) { +func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope string) (string, []byte, error) { // We want only one request fetching current data key at time to // avoid the creation of multiple ones in case there's no one existing. s.mtx.Lock() @@ -217,7 +219,7 @@ func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope // If no existing data key was found, create a new one if dataKey == nil { - id, dataKey, err = s.newDataKey(ctx, label, scope, sess) + id, dataKey, err = s.newDataKey(ctx, label, scope) if err != nil { return "", nil, err } @@ -226,7 +228,7 @@ func (s *SecretsService) currentDataKey(ctx context.Context, label string, scope return id, dataKey, nil } -// dataKeyByLabel looks up for data key in cache. +// dataKeyByLabel looks up for data key in cache by label. // Otherwise, it fetches it from database, decrypts it and caches it decrypted. func (s *SecretsService) dataKeyByLabel(ctx context.Context, label string) (string, []byte, error) { // 0. Get data key from in-memory cache. @@ -256,18 +258,13 @@ func (s *SecretsService) dataKeyByLabel(ctx context.Context, label string) (stri } // 3. Store the decrypted data key into the in-memory cache. - s.dataKeyCache.add(&dataKeyCacheEntry{ - id: dataKey.Id, - label: dataKey.Label, - dataKey: decrypted, - active: dataKey.Active, - }) + s.cacheDataKey(dataKey, decrypted) return dataKey.Id, decrypted, nil } // newDataKey creates a new random data key, encrypts it and stores it into the database and cache. -func (s *SecretsService) newDataKey(ctx context.Context, label string, scope string, sess *xorm.Session) (string, []byte, error) { +func (s *SecretsService) newDataKey(ctx context.Context, label string, scope string) (string, []byte, error) { // 1. Create new data key. dataKey, err := newRandomDataKey() if err != nil { @@ -288,6 +285,7 @@ func (s *SecretsService) newDataKey(ctx context.Context, label string, scope str // 3. Store its encrypted value into the DB. id := util.GenerateShortUID() + dbDataKey := secrets.DataKey{ Active: true, Id: id, @@ -297,24 +295,11 @@ func (s *SecretsService) newDataKey(ctx context.Context, label string, scope str Scope: scope, } - if sess == nil { - err = s.store.CreateDataKey(ctx, &dbDataKey) - } else { - err = s.store.CreateDataKeyWithDBSession(ctx, &dbDataKey, sess) - } - + err = s.store.CreateDataKey(ctx, &dbDataKey) if err != nil { return "", nil, err } - // 4. Store the decrypted data key into the in-memory cache. - s.dataKeyCache.add(&dataKeyCacheEntry{ - id: id, - label: label, - dataKey: dataKey, - active: true, - }) - return id, dataKey, nil } @@ -388,13 +373,9 @@ func (s *SecretsService) Decrypt(ctx context.Context, payload []byte) ([]byte, e } func (s *SecretsService) EncryptJsonData(ctx context.Context, kv map[string]string, opt secrets.EncryptionOptions) (map[string][]byte, error) { - return s.EncryptJsonDataWithDBSession(ctx, kv, opt, nil) -} - -func (s *SecretsService) EncryptJsonDataWithDBSession(ctx context.Context, kv map[string]string, opt secrets.EncryptionOptions, sess *xorm.Session) (map[string][]byte, error) { encrypted := make(map[string][]byte) for key, value := range kv { - encryptedData, err := s.EncryptWithDBSession(ctx, []byte(value), opt, sess) + encryptedData, err := s.Encrypt(ctx, []byte(value), opt) if err != nil { return nil, err } @@ -457,12 +438,7 @@ func (s *SecretsService) dataKeyById(ctx context.Context, id string) ([]byte, er } // 3. Store the decrypted data key into the in-memory cache. - s.dataKeyCache.add(&dataKeyCacheEntry{ - id: dataKey.Id, - label: dataKey.Label, - dataKey: decrypted, - active: dataKey.Active, - }) + s.cacheDataKey(dataKey, decrypted) return decrypted, nil } @@ -547,3 +523,51 @@ func (s *SecretsService) Run(ctx context.Context) error { } } } + +// Caching a data key is tricky, because at SecretsService level we cannot guarantee +// that a newly created data key has actually been persisted, depending on the different +// use cases that rely on SecretsService encryption and different database engines that +// we have support for, because the data key creation may have happened within a DB TX, +// that may fail afterwards. +// +// Therefore, if we cache a data key that hasn't been persisted with success (and won't), +// and later that one is used for a encryption operation (aside from the DB TX that created +// it), we may end up with data encrypted by a non-persisted data key, which could end up +// in (unrecoverable) data corruption. +// +// So, we cache the data key by id and/or by label, depending on the data key's lifetime, +// assuming that a data key older than a "caution period" should have been persisted. +// +// Look at the comments inline for further details. +// You can also take a look at the issue below for more context: +// https://github.com/grafana/grafana-enterprise/issues/4252 +func (s *SecretsService) cacheDataKey(dataKey *secrets.DataKey, decrypted []byte) { + // First, we cache the data key by id, because cache "by id" is + // only used by decrypt operations, so no risk of corrupting data. + entry := &dataKeyCacheEntry{ + id: dataKey.Id, + label: dataKey.Label, + dataKey: decrypted, + active: dataKey.Active, + } + + s.dataKeyCache.addById(entry) + + // Then, we cache the data key by label, ONLY if data key's lifetime + // is longer than a certain "caution period", because cache "by label" + // is used (only) by encrypt operations, and we want to ensure that + // no data key is cached for encryption ops before being persisted. + + const cautionPeriod = 10 * time.Minute + // We consider a "caution period" of 10m to be long enough for any database + // transaction that implied a data key creation to have finished successfully. + // + // Therefore, we consider that if we fetch a data key from the database, + // more than 10m later than its creation, it should have been actually + // persisted - i.e. the transaction that created it is no longer running. + + nowMinusCautionPeriod := now().Add(-cautionPeriod) + if dataKey.Created.Before(nowMinusCautionPeriod) { + s.dataKeyCache.addByLabel(entry) + } +} diff --git a/pkg/services/secrets/manager/manager_test.go b/pkg/services/secrets/manager/manager_test.go index 098cfa13135..a08065826e7 100644 --- a/pkg/services/secrets/manager/manager_test.go +++ b/pkg/services/secrets/manager/manager_test.go @@ -2,6 +2,7 @@ package manager import ( "context" + "errors" "testing" "time" @@ -17,12 +18,14 @@ import ( "github.com/grafana/grafana/pkg/services/kmsproviders/osskmsproviders" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/secrets/database" + "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) func TestSecretsService_EnvelopeEncryption(t *testing.T) { - store := database.ProvideSecretsStore(db.InitTestDB(t)) + testDB := db.InitTestDB(t) + store := database.ProvideSecretsStore(testDB) svc := SetupTestService(t, store) ctx := context.Background() @@ -82,7 +85,8 @@ func TestSecretsService_EnvelopeEncryption(t *testing.T) { } func TestSecretsService_DataKeys(t *testing.T) { - store := database.ProvideSecretsStore(db.InitTestDB(t)) + testDB := db.InitTestDB(t) + store := database.ProvideSecretsStore(testDB) ctx := context.Background() dataKey := &secrets.DataKey{ @@ -160,7 +164,8 @@ func TestSecretsService_DataKeys(t *testing.T) { func TestSecretsService_UseCurrentProvider(t *testing.T) { t.Run("When encryption_provider is not specified explicitly, should use 'secretKey' as a current provider", func(t *testing.T) { - svc := SetupTestService(t, database.ProvideSecretsStore(db.InitTestDB(t))) + testDB := db.InitTestDB(t) + svc := SetupTestService(t, database.ProvideSecretsStore(testDB)) assert.Equal(t, secrets.ProviderID("secretKey.v1"), svc.currentProviderID) }) @@ -187,7 +192,8 @@ func TestSecretsService_UseCurrentProvider(t *testing.T) { features := featuremgmt.WithFeatures() kms := newFakeKMS(osskmsproviders.ProvideService(encryptionService, settings, features)) - secretStore := database.ProvideSecretsStore(db.InitTestDB(t)) + testDB := db.InitTestDB(t) + secretStore := database.ProvideSecretsStore(testDB) secretsService, err := ProvideSecretsService( secretStore, @@ -261,8 +267,8 @@ func (f *fakeKMS) Provide() (map[secrets.ProviderID]secrets.Provider, error) { func TestSecretsService_Run(t *testing.T) { ctx := context.Background() - sql := db.InitTestDB(t) - store := database.ProvideSecretsStore(sql) + testDB := db.InitTestDB(t) + store := database.ProvideSecretsStore(testDB) svc := SetupTestService(t, store) t.Run("should stop with no error once the context's finished", func(t *testing.T) { @@ -274,16 +280,26 @@ func TestSecretsService_Run(t *testing.T) { }) t.Run("should trigger cache clean up", func(t *testing.T) { - // Encrypt to ensure there's a data encryption key generated - _, err := svc.Encrypt(ctx, []byte("grafana"), secrets.WithoutScope()) + restoreTimeNowAfterTestExec(t) + + // Encrypt to force data encryption key generation + encrypted, err := svc.Encrypt(ctx, []byte("grafana"), secrets.WithoutScope()) + require.NoError(t, err) + + // Ten minutes later (after caution period) + // Look SecretsService.cacheDataKey for more details. + now = func() time.Time { return time.Now().Add(10 * time.Minute) } + + // Decrypt to ensure data encryption key is cached + _, err = svc.Decrypt(ctx, encrypted) require.NoError(t, err) // Data encryption key cache should contain one element require.Len(t, svc.dataKeyCache.byId, 1) require.Len(t, svc.dataKeyCache.byLabel, 1) - t.Cleanup(func() { now = time.Now }) - now = func() time.Time { return time.Now().Add(10 * time.Minute) } + // Twenty minutes later (after caution period + cache ttl) + now = func() time.Time { return time.Now().Add(20 * time.Minute) } ctx, cancel := context.WithTimeout(ctx, 1*time.Second) defer cancel() @@ -301,8 +317,8 @@ func TestSecretsService_Run(t *testing.T) { func TestSecretsService_ReEncryptDataKeys(t *testing.T) { ctx := context.Background() - sql := db.InitTestDB(t) - store := database.ProvideSecretsStore(sql) + testDB := db.InitTestDB(t) + store := database.ProvideSecretsStore(testDB) svc := SetupTestService(t, store) // Encrypt to generate data encryption key @@ -326,6 +342,12 @@ func TestSecretsService_ReEncryptDataKeys(t *testing.T) { }) t.Run("data keys cache should be invalidated", func(t *testing.T) { + restoreTimeNowAfterTestExec(t) + + // Ten minutes later (after caution period) + // Look SecretsService.cacheDataKey for more details. + now = func() time.Time { return time.Now().Add(10 * time.Minute) } + // Decrypt to ensure data key is cached _, err := svc.Decrypt(ctx, ciphertext) require.NoError(t, err) @@ -342,7 +364,8 @@ func TestSecretsService_ReEncryptDataKeys(t *testing.T) { func TestSecretsService_Decrypt(t *testing.T) { ctx := context.Background() - store := database.ProvideSecretsStore(db.InitTestDB(t)) + testDB := db.InitTestDB(t) + store := database.ProvideSecretsStore(testDB) t.Run("empty payload should fail", func(t *testing.T) { svc := SetupTestService(t, store) @@ -401,3 +424,137 @@ func TestSecretsService_Decrypt(t *testing.T) { assert.Equal(t, []byte("grafana"), decrypted) }) } + +func TestIntegration_SecretsService(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + ctx := context.Background() + someData := []byte(`some-data`) + + tcs := map[string]func(*testing.T, *sqlstore.SQLStore, *SecretsService){ + "regular": func(t *testing.T, _ *sqlstore.SQLStore, svc *SecretsService) { + // We encrypt some data normally, no transactions implied. + _, err := svc.Encrypt(ctx, someData, secrets.WithoutScope()) + require.NoError(t, err) + }, + "within successful InTransaction": func(t *testing.T, store *sqlstore.SQLStore, svc *SecretsService) { + require.NoError(t, store.InTransaction(ctx, func(ctx context.Context) error { + // We encrypt some data within a transaction that shares the db session. + _, err := svc.Encrypt(ctx, someData, secrets.WithoutScope()) + require.NoError(t, err) + + // And the transition succeeds. + return nil + })) + }, + "within unsuccessful InTransaction": func(t *testing.T, store *sqlstore.SQLStore, svc *SecretsService) { + require.NotNil(t, store.InTransaction(ctx, func(ctx context.Context) error { + // We encrypt some data within a transaction that shares the db session. + _, err := svc.Encrypt(ctx, someData, secrets.WithoutScope()) + require.NoError(t, err) + + // But the transaction fails. + return errors.New("error") + })) + }, + "within unsuccessful InTransaction (plus forced db fetch)": func(t *testing.T, store *sqlstore.SQLStore, svc *SecretsService) { + require.NotNil(t, store.InTransaction(ctx, func(ctx context.Context) error { + // We encrypt some data within a transaction that shares the db session. + encrypted, err := svc.Encrypt(ctx, someData, secrets.WithoutScope()) + require.NoError(t, err) + + // At this point the data key is not cached yet because + // the transaction haven't been committed yet, + // and won't, so we do a decrypt operation within the + // transaction to force the data key to be + // (potentially) cached (it shouldn't to prevent issues). + decrypted, err := svc.Decrypt(ctx, encrypted) + require.NoError(t, err) + assert.Equal(t, someData, decrypted) + + // But the transaction fails. + return errors.New("error") + })) + }, + "within successful WithTransactionalDbSession": func(t *testing.T, store *sqlstore.SQLStore, svc *SecretsService) { + require.NoError(t, store.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + // We encrypt some data within a transaction that does not share the db session. + _, err := svc.Encrypt(ctx, someData, secrets.WithoutScope()) + require.NoError(t, err) + + // And the transition succeeds. + return nil + })) + }, + "within unsuccessful WithTransactionalDbSession": func(t *testing.T, store *sqlstore.SQLStore, svc *SecretsService) { + require.NotNil(t, store.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + // We encrypt some data within a transaction that does not share the db session. + _, err := svc.Encrypt(ctx, someData, secrets.WithoutScope()) + require.NoError(t, err) + + // But the transaction fails. + return errors.New("error") + })) + }, + "within unsuccessful WithTransactionalDbSession (plus forced db fetch)": func(t *testing.T, store *sqlstore.SQLStore, svc *SecretsService) { + require.NotNil(t, store.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + // We encrypt some data within a transaction that does not share the db session. + encrypted, err := svc.Encrypt(ctx, someData, secrets.WithoutScope()) + require.NoError(t, err) + + // At this point the data key is not cached yet because + // the transaction haven't been committed yet, + // and won't, so we do a decrypt operation within the + // transaction to force the data key to be + // (potentially) cached (it shouldn't to prevent issues). + decrypted, err := svc.Decrypt(ctx, encrypted) + require.NoError(t, err) + assert.Equal(t, someData, decrypted) + + // But the transaction fails. + return errors.New("error") + })) + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + testDB := db.InitTestDB(t) + svc := SetupTestService(t, database.ProvideSecretsStore(testDB)) + + // Here's what actually matters and varies on each test: look at the test case name. + // + // For historical reasons, and in an old implementation, when a successful encryption + // operation happened within an unsuccessful transaction, the data key was used to be + // cached in memory for the next encryption operations, which caused some data to be + // encrypted with a data key that haven't actually been persisted into the database. + tc(t, testDB, svc) + // Therefore, the data encrypted after this point, become unrecoverable after a restart. + // So, the different test cases here are there to prevent that from happening again + // in the future, whatever it is what happens. + + // So, we proceed with an encryption operation: + toEncrypt := []byte(`data-to-encrypt`) + encrypted, err := svc.Encrypt(ctx, toEncrypt, secrets.WithoutScope()) + require.NoError(t, err) + + // We simulate an instance restart. So, there's no data in the in-memory cache. + svc.dataKeyCache.flush() + + // And then, we MUST still be able to decrypt the previously encrypted data: + decrypted, err := svc.Decrypt(ctx, encrypted) + require.NoError(t, err) + assert.Equal(t, toEncrypt, decrypted) + }) + } +} + +// Use this function at the beginning of those tests +// that manipulates 'now', so it'll leave it in a +// correct state once test execution finishes. +func restoreTimeNowAfterTestExec(t *testing.T) { + t.Helper() + t.Cleanup(func() { now = time.Now }) +} diff --git a/pkg/services/secrets/migrator/reencrypt.go b/pkg/services/secrets/migrator/reencrypt.go index abc465e6bf6..c976253ff23 100644 --- a/pkg/services/secrets/migrator/reencrypt.go +++ b/pkg/services/secrets/migrator/reencrypt.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/notifier" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/services/secrets/manager" + "github.com/grafana/grafana/pkg/services/sqlstore" ) func (s simpleSecret) reencrypt(ctx context.Context, secretsSrv *manager.SecretsService, sqlStore db.DB) bool { @@ -32,21 +33,24 @@ func (s simpleSecret) reencrypt(ctx context.Context, secretsSrv *manager.Secrets continue } - err := sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + err := sqlStore.InTransaction(ctx, func(ctx context.Context) error { decrypted, err := secretsSrv.Decrypt(ctx, row.Secret) if err != nil { logger.Warn("Could not decrypt secret while re-encrypting it", "table", s.tableName, "id", row.Id, "error", err) return err } - encrypted, err := secretsSrv.EncryptWithDBSession(ctx, decrypted, secrets.WithoutScope(), sess.Session) + encrypted, err := secretsSrv.Encrypt(ctx, decrypted, secrets.WithoutScope()) if err != nil { logger.Warn("Could not encrypt secret while re-encrypting it", "table", s.tableName, "id", row.Id, "error", err) return err } updateSQL := fmt.Sprintf("UPDATE %s SET %s = ?, updated = ? WHERE id = ?", s.tableName, s.columnName) - if _, err = sess.Exec(updateSQL, encrypted, nowInUTC(), row.Id); err != nil { + if err = sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + _, err := sess.Exec(updateSQL, encrypted, nowInUTC(), row.Id) + return err + }); err != nil { logger.Warn("Could not update secret while re-encrypting it", "table", s.tableName, "id", row.Id, "error", err) return err } @@ -88,7 +92,7 @@ func (s b64Secret) reencrypt(ctx context.Context, secretsSrv *manager.SecretsSer continue } - err := sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + err := sqlStore.InTransaction(ctx, func(ctx context.Context) error { decoded, err := s.encoding.DecodeString(row.Secret) if err != nil { logger.Warn("Could not decode base64-encoded secret while re-encrypting it", "table", s.tableName, "id", row.Id, "error", err) @@ -101,22 +105,23 @@ func (s b64Secret) reencrypt(ctx context.Context, secretsSrv *manager.SecretsSer return err } - encrypted, err := secretsSrv.EncryptWithDBSession(ctx, decrypted, secrets.WithoutScope(), sess.Session) + encrypted, err := secretsSrv.Encrypt(ctx, decrypted, secrets.WithoutScope()) if err != nil { logger.Warn("Could not encrypt secret while re-encrypting it", "table", s.tableName, "id", row.Id, "error", err) return err } - encoded := s.encoding.EncodeToString(encrypted) - if s.hasUpdatedColumn { - updateSQL := fmt.Sprintf("UPDATE %s SET %s = ?, updated = ? WHERE id = ?", s.tableName, s.columnName) - _, err = sess.Exec(updateSQL, encoded, nowInUTC(), row.Id) - } else { - updateSQL := fmt.Sprintf("UPDATE %s SET %s = ? WHERE id = ?", s.tableName, s.columnName) - _, err = sess.Exec(updateSQL, encoded, row.Id) - } - - if err != nil { + if err = sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) (err error) { + encoded := s.encoding.EncodeToString(encrypted) + if s.hasUpdatedColumn { + updateSQL := fmt.Sprintf("UPDATE %s SET %s = ?, updated = ? WHERE id = ?", s.tableName, s.columnName) + _, err = sess.Exec(updateSQL, encoded, nowInUTC(), row.Id) + } else { + updateSQL := fmt.Sprintf("UPDATE %s SET %s = ? WHERE id = ?", s.tableName, s.columnName) + _, err = sess.Exec(updateSQL, encoded, row.Id) + } + return + }); err != nil { logger.Warn("Could not update secret while re-encrypting it", "table", s.tableName, "id", row.Id, "error", err) return err } @@ -158,7 +163,7 @@ func (s jsonSecret) reencrypt(ctx context.Context, secretsSrv *manager.SecretsSe continue } - err := sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + err := sqlStore.InTransaction(ctx, func(ctx context.Context) error { decrypted, err := secretsSrv.DecryptJsonData(ctx, row.SecureJsonData) if err != nil { logger.Warn("Could not decrypt secrets while re-encrypting them", "table", s.tableName, "id", row.Id, "error", err) @@ -170,13 +175,16 @@ func (s jsonSecret) reencrypt(ctx context.Context, secretsSrv *manager.SecretsSe Updated string }{Updated: nowInUTC()} - toUpdate.SecureJsonData, err = secretsSrv.EncryptJsonDataWithDBSession(ctx, decrypted, secrets.WithoutScope(), sess.Session) + toUpdate.SecureJsonData, err = secretsSrv.EncryptJsonData(ctx, decrypted, secrets.WithoutScope()) if err != nil { logger.Warn("Could not re-encrypt secrets", "table", s.tableName, "id", row.Id, "error", err) return err } - if _, err := sess.Table(s.tableName).Where("id = ?", row.Id).Update(toUpdate); err != nil { + if err := sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + _, err := sess.Table(s.tableName).Where("id = ?", row.Id).Update(toUpdate) + return err + }); err != nil { logger.Warn("Could not update secrets while re-encrypting them", "table", s.tableName, "id", row.Id, "error", err) return err } @@ -217,7 +225,7 @@ func (s alertingSecret) reencrypt(ctx context.Context, secretsSrv *manager.Secre for _, result := range results { result := result - err := sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error { + err := sqlStore.InTransaction(ctx, func(ctx context.Context) error { postableUserConfig, err := notifier.Load([]byte(result.AlertmanagerConfiguration)) if err != nil { logger.Warn("Could not load alert_configuration while re-encrypting it", "id", result.Id, "error", err) @@ -239,7 +247,7 @@ func (s alertingSecret) reencrypt(ctx context.Context, secretsSrv *manager.Secre return err } - reencrypted, err := secretsSrv.EncryptWithDBSession(ctx, decrypted, secrets.WithoutScope(), sess.Session) + reencrypted, err := secretsSrv.Encrypt(ctx, decrypted, secrets.WithoutScope()) if err != nil { logger.Warn("Could not re-encrypt alert_configuration secret", "id", result.Id, "key", k, "error", err) return err @@ -257,7 +265,10 @@ func (s alertingSecret) reencrypt(ctx context.Context, secretsSrv *manager.Secre } result.AlertmanagerConfiguration = string(marshalled) - if _, err := sess.Table("alert_configuration").Where("id = ?", result.Id).Update(&result); err != nil { + if err := sqlStore.WithDbSession(ctx, func(sess *db.Session) error { + _, err := sess.Table("alert_configuration").Where("id = ?", result.Id).Update(&result) + return err + }); err != nil { logger.Warn("Could not update alert_configuration secret while re-encrypting it", "id", result.Id, "error", err) return err } diff --git a/pkg/services/secrets/secrets.go b/pkg/services/secrets/secrets.go index 4b4a554406f..e3fb098732e 100644 --- a/pkg/services/secrets/secrets.go +++ b/pkg/services/secrets/secrets.go @@ -5,8 +5,6 @@ import ( "fmt" "strings" "time" - - "xorm.io/xorm" ) // Service is an envelope encryption service in charge of encrypting/decrypting secrets. @@ -36,7 +34,6 @@ type Store interface { GetCurrentDataKey(ctx context.Context, label string) (*DataKey, error) GetAllDataKeys(ctx context.Context) ([]*DataKey, error) CreateDataKey(ctx context.Context, dataKey *DataKey) error - CreateDataKeyWithDBSession(ctx context.Context, dataKey *DataKey, sess *xorm.Session) error DisableDataKeys(ctx context.Context) error DeleteDataKey(ctx context.Context, id string) error ReEncryptDataKeys(ctx context.Context, providers map[ProviderID]Provider, currProvider ProviderID) error diff --git a/public/app/core/navigation/GrafanaRoute.tsx b/public/app/core/navigation/GrafanaRoute.tsx index 9cfa4f6e224..1bfe2765013 100644 --- a/public/app/core/navigation/GrafanaRoute.tsx +++ b/public/app/core/navigation/GrafanaRoute.tsx @@ -1,4 +1,4 @@ -import React, { Suspense, useEffect } from 'react'; +import React, { Suspense, useEffect, useLayoutEffect } from 'react'; // @ts-ignore import Drop from 'tether-drop'; @@ -18,9 +18,11 @@ export function GrafanaRoute(props: Props) { chrome.setMatchedRoute(props.route); - useEffect(() => { + useLayoutEffect(() => { keybindings.clearAndInitGlobalBindings(); + }, [keybindings]); + useEffect(() => { updateBodyClassNames(props.route); cleanupDOM(); navigationLogger('GrafanaRoute', false, 'Mounted', props.match); diff --git a/public/app/plugins/panel/barchart/BarChartPanel.tsx b/public/app/plugins/panel/barchart/BarChartPanel.tsx index bff59f36723..e2ef898e020 100644 --- a/public/app/plugins/panel/barchart/BarChartPanel.tsx +++ b/public/app/plugins/panel/barchart/BarChartPanel.tsx @@ -218,7 +218,7 @@ export const BarChartPanel: React.FunctionComponent = ({ } } - return ; + return ; }; const rawValue = (seriesIdx: number, valueIdx: number) => { diff --git a/public/app/plugins/panel/barchart/types.ts b/public/app/plugins/panel/barchart/types.ts index f694f369a19..347ae56edc0 100644 --- a/public/app/plugins/panel/barchart/types.ts +++ b/public/app/plugins/panel/barchart/types.ts @@ -10,6 +10,12 @@ export interface BarChartDisplayValues { */ viz: [DataFrame]; + /** + * The fields we can display, first field is X axis. + * Contains same data as viz, but without config modifications (e.g: unit override) + */ + legend: DataFrame; + /** Potentialy color by a field value */ colorByField?: Field; } diff --git a/public/app/plugins/panel/barchart/utils.test.ts b/public/app/plugins/panel/barchart/utils.test.ts index 8590256f7f1..ff029ba875c 100644 --- a/public/app/plugins/panel/barchart/utils.test.ts +++ b/public/app/plugins/panel/barchart/utils.test.ts @@ -19,7 +19,7 @@ import { SortOrder, } from '@grafana/schema'; -import { PanelFieldConfig } from './models.gen'; +import { PanelFieldConfig, PanelOptions } from './models.gen'; import { BarChartOptionsEX, prepareBarChartDisplayValues, preparePlotConfigBuilder } from './utils'; function mockDataFrame() { @@ -202,6 +202,19 @@ describe('BarChart utils', () => { const field = displayValues.viz[0].fields[1]; expect(field.values.toArray()).toMatchInlineSnapshot(` + Array [ + -10, + null, + 10, + null, + null, + ] + `); + + const displayLegendValuesAsc = assertIsDefined('legend' in result ? result : null).legend; + const legendField = displayLegendValuesAsc.fields[1]; + + expect(legendField.values.toArray()).toMatchInlineSnapshot(` Array [ -10, null, @@ -231,6 +244,12 @@ describe('BarChart utils', () => { expect(displayValuesAsc.fields[2].name).toBe('c'); expect(displayValuesAsc.fields[3].name).toBe('b'); + const displayLegendValuesAsc = assertIsDefined('legend' in resultAsc ? resultAsc : null).legend; + expect(displayLegendValuesAsc.fields[0].type).toBe(FieldType.string); + expect(displayLegendValuesAsc.fields[1].name).toBe('a'); + expect(displayLegendValuesAsc.fields[2].name).toBe('c'); + expect(displayLegendValuesAsc.fields[3].name).toBe('b'); + const resultDesc = prepareBarChartDisplayValues([frame], createTheme(), { legend: { sortBy: 'Min', sortDesc: true }, } as any); @@ -239,6 +258,32 @@ describe('BarChart utils', () => { expect(displayValuesDesc.fields[1].name).toBe('b'); expect(displayValuesDesc.fields[2].name).toBe('c'); expect(displayValuesDesc.fields[3].name).toBe('a'); + + const displayLegendValuesDesc = assertIsDefined('legend' in resultDesc ? resultDesc : null).legend; + expect(displayLegendValuesDesc.fields[0].type).toBe(FieldType.string); + expect(displayLegendValuesDesc.fields[1].name).toBe('b'); + expect(displayLegendValuesDesc.fields[2].name).toBe('c'); + expect(displayLegendValuesDesc.fields[3].name).toBe('a'); + }); + + it('should remove unit from legend values when stacking is percent', () => { + const frame = new MutableDataFrame({ + fields: [ + { name: 'string', type: FieldType.string, values: ['a', 'b', 'c'] }, + { name: 'a', values: [-10, 20, 10], state: { calcs: { min: -10 } } }, + { name: 'b', values: [20, 20, 20], state: { calcs: { min: 20 } } }, + { name: 'c', values: [10, 10, 10], state: { calcs: { min: 10 } } }, + ], + }); + + const resultAsc = prepareBarChartDisplayValues([frame], createTheme(), { + stacking: StackingMode.Percent, + } as PanelOptions); + const displayLegendValuesAsc = assertIsDefined('legend' in resultAsc ? resultAsc : null).legend; + + expect(displayLegendValuesAsc.fields[1].config.unit).toBeUndefined(); + expect(displayLegendValuesAsc.fields[2].config.unit).toBeUndefined(); + expect(displayLegendValuesAsc.fields[3].config.unit).toBeUndefined(); }); }); }); diff --git a/public/app/plugins/panel/barchart/utils.ts b/public/app/plugins/panel/barchart/utils.ts index 0e4d3b8b529..d25e23b4039 100644 --- a/public/app/plugins/panel/barchart/utils.ts +++ b/public/app/plugins/panel/barchart/utils.ts @@ -464,6 +464,27 @@ export function prepareBarChartDisplayValues( ); } + let legendFields: Field[] = fields; + if (options.stacking === StackingMode.Percent) { + legendFields = fields.map((field) => { + const alignedFrameField = frame.fields.find((f) => f.name === field.name)!; + + const copy = { + ...field, + config: { + ...alignedFrameField.config, + }, + values: field.values, + }; + + copy.display = getDisplayProcessor({ field: copy, theme }); + + return copy; + }); + + legendFields.unshift(firstField); + } + // String field is first fields.unshift(firstField); @@ -476,6 +497,10 @@ export function prepareBarChartDisplayValues( fields: fields, // ideally: fields.filter((f) => !Boolean(f.config.custom?.hideFrom?.viz)), }, ], + legend: { + fields: legendFields, + length: firstField.values.length, + }, }; } diff --git a/yarn.lock b/yarn.lock index 8b3c507a97f..17b7ac4d951 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1,6 +1,3 @@ -# This file is generated by running "yarn install" inside your project. -# Manual changes might be lost - proceed with caution! - __metadata: version: 6 cacheKey: 8 @@ -18187,9 +18184,9 @@ __metadata: linkType: hard "dompurify@npm:^2.4.1": - version: 2.4.1 - resolution: "dompurify@npm:2.4.1" - checksum: 1169177465b3cbb25a44322937fba549f6c4e1a91b83245d144471be26619c835cccf0f8e20aa78c25ac11a06efd17cc1b9db9cacadceb78a4c08a1029eafee5 + version: 2.4.3 + resolution: "dompurify@npm:2.4.3" + checksum: b440981f2a38cada2085759cc3d1e2f94571afc34343d011a8a6aa1ad91ae6abf651adbfa4994b0e2283f0ce81f7891cdb04b67d0b234c8d190cb70e9691f026 languageName: node linkType: hard