diff --git a/.betterer.eslint.config.js b/.betterer.eslint.config.js index 1be44601a3c..349ee612837 100644 --- a/.betterer.eslint.config.js +++ b/.betterer.eslint.config.js @@ -113,6 +113,7 @@ module.exports = [ ignores: ['public/app/plugins/**', '**/*.story.tsx', '**/*.{test,spec}.{ts,tsx}', '**/__mocks__/', 'public/test'], rules: { '@grafana/no-untranslated-strings': 'error', + '@grafana/no-translation-top-level': 'error', }, }, ]; diff --git a/.betterer.results b/.betterer.results index 09d20205df6..b889576f7fe 100644 --- a/.betterer.results +++ b/.betterer.results @@ -549,6 +549,11 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], + "packages/grafana-ui/src/components/DataSourceSettings/DataSourceHttpSettings.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"] + ], "packages/grafana-ui/src/components/DataSourceSettings/types.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] @@ -3047,7 +3052,8 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"] ], "public/app/features/connections/tabs/ConnectData/Search/Search.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"] ], "public/app/features/connections/tabs/ConnectData/Search/index.tsx:5381": [ [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"] @@ -4621,9 +4627,18 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] ], + "public/app/features/explore/RichHistory/RichHistorySettingsTab.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "3"] + ], "public/app/features/explore/RichHistory/RichHistoryStarredTab.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], + "public/app/features/explore/ShortLinkButtonMenu.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"] + ], "public/app/features/explore/SupplementaryResultError.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], @@ -4982,8 +4997,14 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], "public/app/features/inspector/InspectJSONTab.tsx:5381": [ - [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "3"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "4"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "5"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] ], "public/app/features/inspector/InspectStatsTab.tsx:5381": [ [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"] diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index e9d98be067b..ada306f734d 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -622,7 +622,7 @@ playwright.config.ts @grafana/plugins-platform-frontend /scripts/import_many_dashboards.sh @torkelo /scripts/mixin-check.sh @bergquist /scripts/openapi3/ @grafana/grafana-operator-experience-squad -/scripts/prepare-packagejson.js @grafana/frontend-ops +/scripts/prepare-npm-package.js @grafana/frontend-ops /scripts/protobuf-check.sh @grafana/plugins-platform-backend /scripts/stripnulls.sh @grafana/grafana-as-code /scripts/tag_release.sh @grafana/grafana-developer-enablement-squad @@ -820,6 +820,7 @@ embed.go @grafana/grafana-as-code /.github/workflows/changelog.yml @zserge /.github/workflows/actions/changelog @zserge /.github/workflows/frontend-unit-tests.yml @grafana/grafana-frontend-platform +/.github/workflows/frontend-lint.yml @grafana/grafana-frontend-platform # Generated files not requiring owner approval /packages/grafana-data/src/types/featureToggles.gen.ts @grafanabot diff --git a/.github/workflows/frontend-lint.yml b/.github/workflows/frontend-lint.yml new file mode 100644 index 00000000000..a9b65b247fb --- /dev/null +++ b/.github/workflows/frontend-lint.yml @@ -0,0 +1,55 @@ +name: Lint Frontend +on: + pull_request: + push: + branches: + - main + - release-*.*.* + +jobs: + verify-i18n: + name: Verify i18n + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + cache: 'yarn' + cache-dependency-path: 'yarn.lock' + - run: yarn install --immutable --check-cache + - run: | + extract_error_message='::error::Extraction failed. Make sure that you have no dynamic translation phrases, such as "t(`preferences.theme.{themeID}`, themeName)" and that no translation key is used twice. Search the output for '[warning]' to find the offending file.' + make i18n-extract || (echo "${extract_error_message}" && false) + - run: | + uncommited_error_message="::error::Translation extraction has not been committed. Please run 'make i18n-extract', commit the changes and push again." + file_diff=$(git diff --dirstat public/locales) + if [ -n "$file_diff" ]; then + echo $file_diff + echo "${uncommited_error_message}" + exit 1 + fi + prettier: + name: Prettier + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + cache: 'yarn' + cache-dependency-path: 'yarn.lock' + - run: yarn install --immutable --check-cache + - run: yarn run prettier:check + typecheck: + name: Typecheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + cache: 'yarn' + cache-dependency-path: 'yarn.lock' + - run: yarn install --immutable --check-cache + - run: yarn run typecheck diff --git a/.gitignore b/.gitignore index b8618dff2be..858a241fcf9 100644 --- a/.gitignore +++ b/.gitignore @@ -151,6 +151,7 @@ pkg/services/quota/quotaimpl/storage/storage.json # Ignoring frontend packages specifics /packages/grafana-ui/.yarn/.cache /packages/grafana-ui/.storybook/static +/packages/grafana-ui/unstable /packages/**/dist /packages/**/compiled /packages/**/.rpt2_cache diff --git a/.vale.ini b/.vale.ini index cf4c71392dc..37e764e6d0f 100644 --- a/.vale.ini +++ b/.vale.ini @@ -1,5 +1,5 @@ MinAlertLevel = warning -[*] +[*.md] BasedOnStyles = Grafana TokenIgnores = (+?), \*\*[^\n]+\*\* diff --git a/docs/sources/alerting/_index.md b/docs/sources/alerting/_index.md index 601ae34e4e1..8362360d188 100644 --- a/docs/sources/alerting/_index.md +++ b/docs/sources/alerting/_index.md @@ -25,7 +25,7 @@ cards: items: - title: Introduction href: ./fundamentals/ - description: Learn more about the fundamentals and available features that help you create, manage, and respond to alerts; and improve your team’s ability to resolve issues quickly. + description: Learn more about the fundamentals and available features that help you create, manage, and respond to alerts; and improve your team's ability to resolve issues quickly. height: 24 - title: Configure alert rules href: ./alerting-rules/ @@ -55,7 +55,8 @@ Monitor your incoming metrics data or log entries and set up your Grafana Alerti In this way, you eliminate the need for manual monitoring and provide a first line of defense against system outages or changes that could turn into major incidents. -Using Grafana Alerting, you create queries and expressions from multiple data sources — no matter where your data is stored — giving you the flexibility to combine your data and alert on your metrics and logs in new and unique ways. You can then create, manage, and take action on your alerts from a single, consolidated view, and improve your team’s ability to identify and resolve issues quickly. +Using Grafana Alerting, you create queries and expressions from multiple data sources, no matter where you store your data, giving you the flexibility to combine your data and alert on your metrics and logs in new and unique ways. +You can then create, manage, and take action on your alerts from a single, consolidated view, and improve your team's ability to identify and resolve issues quickly. ## Explore diff --git a/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md b/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md index 41c9150b52c..3b30819b328 100644 --- a/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md +++ b/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md @@ -118,15 +118,17 @@ refs: # Configure Grafana-managed alert rules -Grafana-managed rules can query data from multiple data sources in a single alert rule. They are the most flexible [alert rule type](ref:alert-rules). You can also add expressions to transform your data, set alert conditions, and images in alert notifications. +Grafana-managed rules can query data from multiple data sources in a single alert rule. +They're the most flexible [alert rule type](ref:alert-rules). +You can also add expressions to transform your data, set alert conditions, and images in alert notifications. -{{% admonition type="note" %}} +{{< admonition type="note" >}} In Grafana Cloud, the number of Grafana-managed alert rules you can create depends on your Grafana Cloud plan. - Free Forever plan: You can create up to 100 free alert rules, with each alert rule having a maximum of 1000 alert instances. - All paid plans (Pro and Advanced): They have a soft limit of 2000 alert rules and support unlimited alert instances. To increase the limit, open a support ticket from the [Cloud portal](/docs/grafana-cloud/account-management/support/). -{{% /admonition %}} +{{< /admonition >}} To create or edit Grafana-managed alert rules, follow the instructions below. For a practical example, check out our [tutorial on getting started with Grafana alerting](http://grafana.com/tutorials/alerting-get-started/). @@ -144,9 +146,11 @@ Only users with **Edit** permissions for the folder storing the rules can edit o You can use default or advanced options for Grafana-managed alert rule creation. The default options streamline rule creation with a cleaner header and a single query and condition. For more complex rules, use advanced options to add multiple queries and expressions. -You can toggle between the two options. Once you have created an alert rule, the system defaults to your previous choice for the next alert rule. +You can toggle between the two options. +After you have created an alert rule, the system defaults to your previous choice for the next alert rule. -Switching from advanced to default may result in queries and expressions that cannot be converted. In this case, a warning message asks if you want to continue to reset to default settings. +Switching from advanced to default may result in queries and expressions that can't be converted. +In this case, a warning message asks if you want to continue to reset to default settings. {{< docs/shared lookup="alerts/configure-alert-rule-name.md" source="grafana" version="" >}} @@ -169,9 +173,10 @@ You can toggle between **Default** and **Advanced** options. If the [Default vs. {{< collapse title="Advanced options" >}} 1. Select a data source. -1. From the **Options** dropdown, specify a [time range](ref:time-units-and-relative-ranges). +1. From the **Options** drop-down menu, specify a [time range](ref:time-units-and-relative-ranges). - Note that Grafana Alerting only supports fixed relative time ranges, for example, `now-24hr: now`. It does not support absolute time ranges: `2021-12-02 00:00:00 to 2021-12-05 23:59:592` or semi-relative time ranges: `now/d to: now`. + Note that Grafana Alerting only supports fixed relative time ranges, for example, `now-24hr: now`. + It doesn't support absolute time ranges: `2021-12-02 00:00:00 to 2021-12-05 23:59:592` or semi-relative time ranges: `now/d to: now`. 1. Add a query. @@ -222,7 +227,8 @@ To do this, you need to make sure that your alert rule is in the right evaluatio The pending period is the period in which an alert rule can be in breach of the condition until it fires. - Once a condition is met, the alert goes into the **Pending** state. If the condition remains active for the duration specified, the alert transitions to the **Firing** state, else it reverts to the **Normal** state. + After a condition is met, the alert goes into the **Pending** state. + If the condition remains active for the duration specified, the alert transitions to the **Firing** state, else it reverts to the **Normal** state. 1. Turn on pause alert notifications, if required. @@ -253,7 +259,7 @@ Complete the following steps to set up notifications. 1. Choose this option to select an existing [contact point](ref:contact-points). - All notifications for this alert rule are sent to this contact point automatically and notification policies are not used. + All notifications for this alert rule are sent to this contact point automatically and notification policies aren't used. 1. You can also optionally select a mute timing as well as groupings and timings to define when not to send notifications. @@ -261,7 +267,8 @@ Complete the following steps to set up notifications. 1. Choose this option to use the [notification policy tree](ref:notification-policies) to handle alert notifications. - All notifications for this alert rule are managed by the notification policy tree, which routes alerts based on their labels. If an alert does not match a specific policy, the default notification policy applies, ensuring all alerts are handled. + All notifications for this alert rule are managed by the notification policy tree, which routes alerts based on their labels. + If an alert doesn't match a specific policy, the default notification policy applies, ensuring all alerts are handled. 1. Preview your alert instance routing set up. diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index f40ad6a1766..7ceb0dc3460 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -226,6 +226,7 @@ Experimental features might be changed or removed without prior notice. | `datasourceConnectionsTab` | Shows defined connections for a data source in the plugins detail page | | `newLogsPanel` | Enables the new logs panel in Explore | | `pluginsCDNSyncLoader` | Load plugins from CDN synchronously | +| `assetSriChecks` | Enables SRI checks for Grafana JavaScript assets | ## Development feature toggles diff --git a/docs/sources/shared/plugins/rename-note.md b/docs/sources/shared/plugins/rename-note.md index 5eaa9a1adf4..4fec1abb499 100644 --- a/docs/sources/shared/plugins/rename-note.md +++ b/docs/sources/shared/plugins/rename-note.md @@ -15,5 +15,5 @@ labels: {{< admonition type="note" >}} The Grafana Explore apps have changed to Grafana Drilldown apps. For example, Explore Logs is now Logs Drilldown. -To learn more, read [Grafana Drilldown apps: the improved queryless experience known as the Explore apps](https://grafana.com/blog/2025/02/20/grafana-drilldown-apps-the-improved-queryless-experience-formerly-known-as-the-explore-apps/). +To learn more, read [Grafana Drilldown apps: the improved queryless experience formerly known as the Explore apps](https://grafana.com/blog/2025/02/20/grafana-drilldown-apps-the-improved-queryless-experience-formerly-known-as-the-explore-apps/). {{< /admonition >}} diff --git a/package.json b/package.json index 1660de6b891..3708b2b49ac 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "@grafana/plugin-e2e": "1.17.1", "@grafana/tsconfig": "^2.0.0", "@manypkg/get-packages": "^2.2.0", + "@npmcli/package-json": "^5.2.0", "@playwright/test": "1.50.1", "@pmmmwh/react-refresh-webpack-plugin": "0.5.15", "@react-types/button": "3.10.2", @@ -247,6 +248,7 @@ "webpack-livereload-plugin": "3.0.2", "webpack-manifest-plugin": "5.0.0", "webpack-merge": "6.0.1", + "webpack-subresource-integrity": "^5.2.0-rc.1", "webpackbar": "^7.0.0", "yaml": "^2.0.0", "yargs": "^17.5.1" diff --git a/packages/grafana-data/package.json b/packages/grafana-data/package.json index 61124640e72..c33053087f2 100644 --- a/packages/grafana-data/package.json +++ b/packages/grafana-data/package.json @@ -16,9 +16,9 @@ "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { - "main": "dist/index.js", - "module": "dist/esm/index.js", - "types": "dist/index.d.ts", + "main": "./dist/cjs/index.cjs", + "module": "./dist/esm/index.mjs", + "types": "./dist/cjs/index.d.cts", "access": "public" }, "files": [ @@ -29,10 +29,10 @@ ], "scripts": { "build": "tsc -p ./tsconfig.build.json && rollup -c rollup.config.ts --configPlugin esbuild", - "clean": "rimraf ./dist ./compiled ./package.tgz", + "clean": "rimraf ./dist ./compiled ./unstable ./package.tgz", "typecheck": "tsc --emitDeclarationOnly false --noEmit", - "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-packagejson.js", - "postpack": "mv package.json.bak package.json" + "prepack": "cp package.json package.json.bak && ALIAS_PACKAGE_NAME=unstable node ../../scripts/prepare-npm-package.js", + "postpack": "mv package.json.bak package.json && rimraf ./unstable" }, "dependencies": { "@braintree/sanitize-url": "7.0.1", diff --git a/packages/grafana-data/rollup.config.ts b/packages/grafana-data/rollup.config.ts index 5a913449466..f67f12e07d0 100644 --- a/packages/grafana-data/rollup.config.ts +++ b/packages/grafana-data/rollup.config.ts @@ -11,5 +11,23 @@ export default [ plugins, output: [cjsOutput(pkg), esmOutput(pkg, 'grafana-data')], }, + { + input: 'src/unstable.ts', + plugins, + output: [cjsOutput(pkg), esmOutput(pkg, 'grafana-data')], + }, tsDeclarationOutput(pkg), + tsDeclarationOutput(pkg, { + input: './compiled/unstable.d.ts', + output: [ + { + file: './dist/cjs/unstable.d.cts', + format: 'cjs', + }, + { + file: './dist/esm/unstable.d.mts', + format: 'es', + }, + ], + }), ]; diff --git a/packages/grafana-data/src/transformations/transformDataFrame.ts b/packages/grafana-data/src/transformations/transformDataFrame.ts index 1a923ec0d86..35951f9f5f6 100644 --- a/packages/grafana-data/src/transformations/transformDataFrame.ts +++ b/packages/grafana-data/src/transformations/transformDataFrame.ts @@ -1,3 +1,4 @@ +import { cloneDeep } from 'lodash'; import { MonoTypeOperatorFunction, Observable, of } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; @@ -12,9 +13,6 @@ import { import { getFrameMatchers } from './matchers'; import { standardTransformersRegistry, TransformerRegistryItem } from './standardTransformersRegistry'; -// when running within Scenes, we can skip var interpolation, since it's already handled upstream -const isScenes = window.__grafanaSceneContext != null; - const getOperator = (config: DataTransformerConfig, ctx: DataTransformContext): MonoTypeOperatorFunction => (source) => { @@ -27,9 +25,12 @@ const getOperator = const defaultOptions = info.transformation.defaultOptions ?? {}; const options = { ...defaultOptions, ...config.options }; + // when running within Scenes, we can skip var interpolation, since it's already handled upstream + const isScenes = window.__grafanaSceneContext != null; + const interpolated = isScenes ? options - : deepIterate(options, (v) => { + : deepIterate(cloneDeep(options), (v) => { if (typeof v === 'string') { return ctx.interpolate(v); } diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 87c5ff7c0d1..7e3ac0d6dba 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -254,4 +254,5 @@ export interface FeatureToggles { alertingRuleVersionHistoryRestore?: boolean; newShareReportDrawer?: boolean; rendererDisableAppPluginsPreload?: boolean; + assetSriChecks?: boolean; } diff --git a/packages/grafana-data/src/unstable.ts b/packages/grafana-data/src/unstable.ts new file mode 100644 index 00000000000..8a42447206f --- /dev/null +++ b/packages/grafana-data/src/unstable.ts @@ -0,0 +1,13 @@ +/** + * THESE APIS MUST NOT BE USED IN COMMUNITY PLUGINS. + * + * Unstable APIs are still in development and are subject to breaking changes + * at any point, like feature flags but for APIS. They must only be used in + * Grafana core and internal plugins where we can coordinate changes. + * + * Once mature, they will be moved to the main export, be available to plugins via the standard import path, + * and be subject to the standard policies + */ + +// This is a dummy export so typescript doesn't error importing an "empty module" +export const unstable = {}; diff --git a/packages/grafana-e2e-selectors/package.json b/packages/grafana-e2e-selectors/package.json index 739e36cfec8..dc8a0541bcf 100644 --- a/packages/grafana-e2e-selectors/package.json +++ b/packages/grafana-e2e-selectors/package.json @@ -19,9 +19,9 @@ "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { - "main": "dist/index.js", - "module": "dist/esm/index.js", - "types": "dist/index.d.ts", + "main": "./dist/cjs/index.cjs", + "module": "./dist/esm/index.mjs", + "types": "./dist/cjs/index.d.cts", "access": "public" }, "files": [ @@ -35,7 +35,7 @@ "bundle": "rollup -c rollup.config.ts --configPlugin esbuild", "clean": "rimraf ./dist ./compiled ./package.tgz", "typecheck": "tsc --emitDeclarationOnly false --noEmit", - "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-packagejson.js", + "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-npm-package.js", "postpack": "mv package.json.bak package.json" }, "devDependencies": { @@ -51,7 +51,7 @@ }, "dependencies": { "@grafana/tsconfig": "^2.0.0", - "semver": "7.7.0", + "semver": "^7.7.0", "tslib": "2.8.1", "typescript": "5.7.3" } diff --git a/packages/grafana-eslint-rules/README.md b/packages/grafana-eslint-rules/README.md index 2f73d46a529..e57e2ec09a1 100644 --- a/packages/grafana-eslint-rules/README.md +++ b/packages/grafana-eslint-rules/README.md @@ -156,7 +156,18 @@ const SearchTitle = ({ term }) => ( #### How to translate props or attributes -Right now, we only check if a string is wrapped up by the `Trans` tag. We currently do not apply this rule to props, attributes or similar, but we also ask for them to be translated with the `t()` function. +This rule checks if a string is wrapped up by the `Trans` tag, or if certain props contain untranslated strings. +We ask for such props to be translated with the `t()` function. + +The below props are checked for untranslated strings: + +- `label` +- `description` +- `placeholder` +- `aria-label` +- `title` +- `text` +- `tooltip` ```tsx // Bad ❌ @@ -168,3 +179,50 @@ return ; ``` Check more info about how translations work in Grafana in [Internationalization.md](https://github.com/grafana/grafana/blob/main/contribute/internationalization.md) + +### `no-translation-top-level` + +Ensure that `t()` translation method is not used at the top level of a file, outside of a component of method. +This is to prevent calling the translation method before it's been instantiated. + +This does not cause an error if a file is lazily loaded, but refactors can cause errors, and it can cause problems in tests. +Fix the + +```tsx +// Bad ❌ +const someTranslatedText = t('some.key', 'Some text'); +const SomeComponent = () => { + return
; +}; + +// Good ✅ +const SomeComponent = () => { + const someTranslatedText = t('some.key', 'Some text'); + return
; +}; + +// Bad ❌ +const someConfigThatHasToBeShared = [{ foo: t('some.key', 'Some text') }]; +const SomeComponent = () => { + return ( +
+ {someConfigThatHasToBeShared.map((cfg) => { + return
{cfg.foo}
; + })} +
+ ); +}; + +// Good ✅ +const someConfigThatHasToBeShared = () => [{ foo: t('some.key', 'Some text') }]; +const SomeComponent = () => { + const configs = someConfigThatHasToBeShared(); + return ( +
+ {configs.map((cfg) => { + return
{cfg.foo}
; + })} +
+ ); +}; +``` diff --git a/packages/grafana-eslint-rules/index.cjs b/packages/grafana-eslint-rules/index.cjs index 21c3bfa739e..11be6258993 100644 --- a/packages/grafana-eslint-rules/index.cjs +++ b/packages/grafana-eslint-rules/index.cjs @@ -2,6 +2,7 @@ const noAriaLabelSelectors = require('./rules/no-aria-label-e2e-selectors.cjs'); const noBorderRadiusLiteral = require('./rules/no-border-radius-literal.cjs'); const noUnreducedMotion = require('./rules/no-unreduced-motion.cjs'); const noUntranslatedStrings = require('./rules/no-untranslated-strings.cjs'); +const noTranslationTopLevel = require('./rules/no-translation-top-level.cjs'); const themeTokenUsage = require('./rules/theme-token-usage.cjs'); module.exports = { @@ -11,5 +12,6 @@ module.exports = { 'no-border-radius-literal': noBorderRadiusLiteral, 'theme-token-usage': themeTokenUsage, 'no-untranslated-strings': noUntranslatedStrings, + 'no-translation-top-level': noTranslationTopLevel, }, }; diff --git a/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs b/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs new file mode 100644 index 00000000000..2290a980cf1 --- /dev/null +++ b/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs @@ -0,0 +1,59 @@ +// @ts-check +const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/utils'); + +/** + * @typedef {import('@typescript-eslint/utils').TSESTree.Node} Node + * @typedef {import('@typescript-eslint/utils').TSESLint.RuleContext} RuleContext + */ + +const createRule = ESLintUtils.RuleCreator( + (name) => `https://github.com/grafana/grafana/blob/main/packages/grafana-eslint-rules/README.md#${name}` +); + +/** + * @param {RuleContext} context + * @param {Node} node + * @returns {boolean} + */ +const isInFunction = (context, node) => { + const ancestors = context.sourceCode.getAncestors(node); + return ancestors.some((anc) => { + return [ + AST_NODE_TYPES.ArrowFunctionExpression, + AST_NODE_TYPES.FunctionDeclaration, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.ClassDeclaration, + ].includes(anc.type); + }); +}; + +const noTranslationTopLevel = createRule({ + create(context) { + return { + CallExpression(node) { + if (node.callee.type === AST_NODE_TYPES.Identifier && node.callee.name === 't') { + if (!isInFunction(context, node)) { + context.report({ + node, + messageId: 'noMethodOutsideComponent', + }); + } + } + }, + }; + }, + name: 'no-translation-top-level', + meta: { + type: 'suggestion', + docs: { + description: 'Do not use translation functions outside of components', + }, + messages: { + noMethodOutsideComponent: 'Do not use the t() function outside of a component or function', + }, + schema: [], + }, + defaultOptions: [], +}); + +module.exports = noTranslationTopLevel; diff --git a/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js b/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js new file mode 100644 index 00000000000..88326b500d4 --- /dev/null +++ b/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js @@ -0,0 +1,58 @@ +import { RuleTester } from 'eslint'; + +import noTranslationTopLevel from '../rules/no-translation-top-level.cjs'; + +RuleTester.setDefaultConfig({ + languageOptions: { + ecmaVersion: 2018, + sourceType: 'module', + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + }, +}); + +const expectedErrorMessage = 'Do not use the t() function outside of a component or function'; + +const ruleTester = new RuleTester(); + +ruleTester.run('eslint no-translation-top-level', noTranslationTopLevel, { + valid: [ + { + code: ` +function Component() { + return
{t('some.key', 'Some text')}
; +} + `, + }, + { + code: `const foo = () => t('some.key', 'Some text');`, + }, + { + code: `const foo = ttt('some.key', 'Some text');`, + }, + { + code: `class Component { +render() { + return t('some.key', 'Some text'); +} +}`, + }, + ], + invalid: [ + { + code: `const thing = t('some.key', 'Some text');`, + errors: [{ message: expectedErrorMessage }], + }, + { + code: `const things = [t('some.key', 'Some text')];`, + errors: [{ message: expectedErrorMessage }], + }, + { + code: `const objectThings = [{foo: t('some.key', 'Some text')}];`, + errors: [{ message: expectedErrorMessage }], + }, + ], +}); diff --git a/packages/grafana-flamegraph/package.json b/packages/grafana-flamegraph/package.json index f47339c0aa0..9ce7c7e25c5 100644 --- a/packages/grafana-flamegraph/package.json +++ b/packages/grafana-flamegraph/package.json @@ -19,9 +19,9 @@ "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { - "main": "dist/index.js", - "module": "dist/esm/index.js", - "types": "dist/index.d.ts", + "main": "./dist/cjs/index.cjs", + "module": "./dist/esm/index.mjs", + "types": "./dist/cjs/index.d.cts", "access": "public" }, "files": [ @@ -35,7 +35,7 @@ "bundle": "rollup -c rollup.config.ts --configPlugin esbuild", "clean": "rimraf ./dist ./compiled ./package.tgz", "typecheck": "tsc --emitDeclarationOnly false --noEmit", - "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-packagejson.js", + "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-npm-package.js", "postpack": "mv package.json.bak package.json" }, "browserslist": [ diff --git a/packages/grafana-icons/package.json b/packages/grafana-icons/package.json index cb60d576dd5..08bb3641efb 100644 --- a/packages/grafana-icons/package.json +++ b/packages/grafana-icons/package.json @@ -15,9 +15,9 @@ "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { - "main": "dist/index.js", - "module": "dist/index.js", - "types": "dist/index.d.ts", + "main": "./dist/index.js", + "module": "./dist/index.js", + "types": "./dist/index.d.mts", "access": "public" }, "files": [ diff --git a/packages/grafana-prometheus/package.json b/packages/grafana-prometheus/package.json index 9cf1a0c1100..59690be5c05 100644 --- a/packages/grafana-prometheus/package.json +++ b/packages/grafana-prometheus/package.json @@ -22,9 +22,9 @@ "./LICENSE_AGPL" ], "publishConfig": { - "main": "dist/index.js", - "module": "dist/esm/index.js", - "types": "dist/index.d.ts", + "main": "./dist/cjs/index.cjs", + "module": "./dist/esm/index.mjs", + "types": "./dist/cjs/index.d.cts", "access": "public" }, "scripts": { @@ -32,7 +32,7 @@ "bundle": "rollup -c rollup.config.ts --configPlugin esbuild", "clean": "rimraf ./dist ./compiled ./package.tgz", "typecheck": "tsc --emitDeclarationOnly false --noEmit", - "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-packagejson.js", + "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-npm-package.js", "postpack": "mv package.json.bak package.json" }, "dependencies": { diff --git a/packages/grafana-runtime/package.json b/packages/grafana-runtime/package.json index 90ca6774646..4211e3d2387 100644 --- a/packages/grafana-runtime/package.json +++ b/packages/grafana-runtime/package.json @@ -17,9 +17,9 @@ "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { - "main": "dist/index.js", - "module": "dist/esm/index.js", - "types": "dist/index.d.ts", + "main": "./dist/cjs/index.cjs", + "module": "./dist/esm/index.mjs", + "types": "./dist/cjs/index.d.cts", "access": "public" }, "files": [ @@ -31,10 +31,10 @@ "scripts": { "build": "tsc -p ./tsconfig.build.json && rollup -c rollup.config.ts --configPlugin esbuild", "bundle": "rollup -c rollup.config.ts --configPlugin esbuild", - "clean": "rimraf ./dist ./compiled ./package.tgz", + "clean": "rimraf ./dist ./compiled ./unstable ./package.tgz", "typecheck": "tsc --emitDeclarationOnly false --noEmit", - "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-packagejson.js", - "postpack": "mv package.json.bak package.json" + "prepack": "cp package.json package.json.bak && ALIAS_PACKAGE_NAME=unstable node ../../scripts/prepare-npm-package.js", + "postpack": "mv package.json.bak package.json && rimraf ./unstable" }, "dependencies": { "@grafana/data": "11.6.0-pre", diff --git a/packages/grafana-runtime/rollup.config.ts b/packages/grafana-runtime/rollup.config.ts index 02f523deee1..dc9c05e1421 100644 --- a/packages/grafana-runtime/rollup.config.ts +++ b/packages/grafana-runtime/rollup.config.ts @@ -11,5 +11,23 @@ export default [ plugins, output: [cjsOutput(pkg), esmOutput(pkg, 'grafana-runtime')], }, + { + input: 'src/unstable.ts', + plugins, + output: [cjsOutput(pkg), esmOutput(pkg, 'grafana-runtime')], + }, tsDeclarationOutput(pkg), + tsDeclarationOutput(pkg, { + input: './compiled/unstable.d.ts', + output: [ + { + file: './dist/cjs/unstable.d.cts', + format: 'cjs', + }, + { + file: './dist/esm/unstable.d.mts', + format: 'es', + }, + ], + }), ]; diff --git a/packages/grafana-runtime/src/unstable.ts b/packages/grafana-runtime/src/unstable.ts new file mode 100644 index 00000000000..8a42447206f --- /dev/null +++ b/packages/grafana-runtime/src/unstable.ts @@ -0,0 +1,13 @@ +/** + * THESE APIS MUST NOT BE USED IN COMMUNITY PLUGINS. + * + * Unstable APIs are still in development and are subject to breaking changes + * at any point, like feature flags but for APIS. They must only be used in + * Grafana core and internal plugins where we can coordinate changes. + * + * Once mature, they will be moved to the main export, be available to plugins via the standard import path, + * and be subject to the standard policies + */ + +// This is a dummy export so typescript doesn't error importing an "empty module" +export const unstable = {}; diff --git a/packages/grafana-schema/package.json b/packages/grafana-schema/package.json index a4939ade8a1..3a97767a218 100644 --- a/packages/grafana-schema/package.json +++ b/packages/grafana-schema/package.json @@ -16,9 +16,9 @@ "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { - "main": "dist/index.js", - "module": "dist/esm/index.js", - "types": "dist/index.d.ts", + "main": "./dist/cjs/index.cjs", + "module": "./dist/esm/index.mjs", + "types": "./dist/cjs/index.d.cts", "access": "public" }, "files": [ @@ -32,7 +32,7 @@ "bundle": "rollup -c rollup.config.ts --configPlugin esbuild", "clean": "rimraf ./dist ./compiled ./package.tgz", "typecheck": "tsc --emitDeclarationOnly false --noEmit", - "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-packagejson.js", + "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-npm-package.js", "postpack": "mv package.json.bak package.json" }, "devDependencies": { diff --git a/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts b/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts index c12f1cc73f7..fe92defe024 100644 --- a/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts +++ b/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts @@ -18,6 +18,18 @@ export const handyTestingSchema: DashboardV2Spec = { timezone: 'UTC', to: 'now', weekStart: 'monday', + quickRanges: [ + { + display: 'Last 6 hours', + from: 'now-6h', + to: 'now', + }, + { + display: 'Last 3 days', + from: 'now-3d', + to: 'now', + }, + ], }, annotations: [ { diff --git a/packages/grafana-ui/package.json b/packages/grafana-ui/package.json index 1116e1b2980..4b51ff9386a 100644 --- a/packages/grafana-ui/package.json +++ b/packages/grafana-ui/package.json @@ -19,9 +19,9 @@ "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { - "main": "dist/index.js", - "module": "dist/esm/index.js", - "types": "dist/index.d.ts", + "main": "./dist/cjs/index.cjs", + "module": "./dist/esm/index.mjs", + "types": "./dist/cjs/index.d.cts", "access": "public" }, "files": [ @@ -34,12 +34,12 @@ "scripts": { "build": "tsc -p ./tsconfig.build.json && rollup -c rollup.config.ts --configPlugin esbuild", "bundle": "rollup -c rollup.config.ts --configPlugin esbuild", - "clean": "rimraf ./dist ./compiled ./package.tgz", + "clean": "rimraf ./dist ./compiled ./unstable ./package.tgz", "storybook": "storybook dev -p 9001 -c .storybook --no-open", "storybook:build": "storybook build -o ./dist/storybook -c .storybook", "typecheck": "tsc --emitDeclarationOnly false --noEmit", - "prepack": "cp package.json package.json.bak && node ../../scripts/prepare-packagejson.js", - "postpack": "mv package.json.bak package.json" + "prepack": "cp package.json package.json.bak && ALIAS_PACKAGE_NAME=unstable node ../../scripts/prepare-npm-package.js", + "postpack": "mv package.json.bak package.json && rimraf ./unstable" }, "browserslist": [ "defaults", diff --git a/packages/grafana-ui/rollup.config.ts b/packages/grafana-ui/rollup.config.ts index d3cb5c2d8b8..5dcad62f82f 100644 --- a/packages/grafana-ui/rollup.config.ts +++ b/packages/grafana-ui/rollup.config.ts @@ -25,5 +25,30 @@ export default [ ], output: [cjsOutput(pkg), esmOutput(pkg, 'grafana-ui')], }, + { + input: 'src/unstable.ts', + plugins: [ + ...plugins, + svg({ stringify: true }), + copy({ + targets: [{ src: iconSrcPaths, dest: './dist/public/' }], + flatten: false, + }), + ], + output: [cjsOutput(pkg), esmOutput(pkg, 'grafana-ui')], + }, tsDeclarationOutput(pkg), + tsDeclarationOutput(pkg, { + input: './compiled/unstable.d.ts', + output: [ + { + file: './dist/cjs/unstable.d.cts', + format: 'cjs', + }, + { + file: './dist/esm/unstable.d.mts', + format: 'es', + }, + ], + }), ]; diff --git a/packages/rollup.config.parts.ts b/packages/rollup.config.parts.ts index ca805b07ed9..41b760a3acf 100644 --- a/packages/rollup.config.parts.ts +++ b/packages/rollup.config.parts.ts @@ -1,6 +1,6 @@ // This file contains the common parts of the rollup configuration that are shared across multiple packages. import nodeResolve from '@rollup/plugin-node-resolve'; -import { dirname, resolve } from 'node:path'; +import { dirname, join, resolve } from 'node:path'; import dts from 'rollup-plugin-dts'; import esbuild from 'rollup-plugin-esbuild'; import { nodeExternals } from 'rollup-plugin-node-externals'; @@ -28,6 +28,7 @@ export function cjsOutput(pkg) { format: 'cjs', sourcemap: true, dir: dirname(pkg.publishConfig.main), + entryFileNames: '[name].cjs', esModule: true, interop: 'compat', }; @@ -39,6 +40,7 @@ export function esmOutput(pkg, pkgName) { format: 'esm', sourcemap: true, dir: dirname(pkg.publishConfig.module), + entryFileNames: '[name].mjs', preserveModules: true, preserveModulesRoot: resolve(projectCwd, `packages/${pkgName}/src`), }; @@ -49,10 +51,16 @@ export function tsDeclarationOutput(pkg, overrides = {}) { return { input: './compiled/index.d.ts', plugins: [dts()], - output: { - file: pkg.publishConfig.types, - format: 'es', - }, + output: [ + { + file: pkg.publishConfig.types, + format: 'cjs', + }, + { + file: join(dirname(pkg.publishConfig.module), 'index.d.mts'), + format: 'es', + }, + ], ...overrides, }; } diff --git a/pkg/api/apierrors/dashboard.go b/pkg/api/apierrors/dashboard.go index 6f740d80afd..e05c81ce329 100644 --- a/pkg/api/apierrors/dashboard.go +++ b/pkg/api/apierrors/dashboard.go @@ -7,9 +7,11 @@ import ( "net/http" "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/util" + apierrors "k8s.io/apimachinery/pkg/api/errors" ) // ToDashboardErrorResponse returns a different response status according to the dashboard error type @@ -39,5 +41,9 @@ func ToDashboardErrorResponse(ctx context.Context, pluginStore pluginstore.Store return response.JSON(http.StatusPreconditionFailed, util.DynMap{"status": "plugin-dashboard", "message": message}) } + if apierrors.IsRequestEntityTooLargeError(err) { + return response.Error(http.StatusRequestEntityTooLarge, fmt.Sprintf("Dashboard is too large, max is %d MB", apiserver.MaxRequestBodyBytes/1024/1024), err) + } + return response.Error(http.StatusInternalServerError, "Failed to save dashboard", err) } diff --git a/pkg/api/dtos/folder.go b/pkg/api/dtos/folder.go index 19df61f4f68..02187b73358 100644 --- a/pkg/api/dtos/folder.go +++ b/pkg/api/dtos/folder.go @@ -3,6 +3,7 @@ package dtos import ( "time" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/services/accesscontrol" ) @@ -31,7 +32,7 @@ type Folder struct { // When the folder belongs to a repository // NOTE: this is only populated when folders are managed by unified storage - Repository string `json:"repository,omitempty"` + ManagedBy utils.ManagerKind `json:"managedBy,omitempty"` } type FolderSearchHit struct { @@ -42,5 +43,5 @@ type FolderSearchHit struct { // When the folder belongs to a repository // NOTE: this is only populated when folders are managed by unified storage - Repository string `json:"repository,omitempty"` + ManagedBy utils.ManagerKind `json:"managedBy,omitempty"` } diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 185eb427d7d..b18db9c7850 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -94,11 +94,11 @@ func (hs *HTTPServer) GetFolders(c *contextmodel.ReqContext) response.Response { hits := make([]dtos.FolderSearchHit, 0) for _, f := range folders { hits = append(hits, dtos.FolderSearchHit{ - ID: f.ID, // nolint:staticcheck - UID: f.UID, - Title: f.Title, - ParentUID: f.ParentUID, - Repository: f.Repository, + ID: f.ID, // nolint:staticcheck + UID: f.UID, + Title: f.Title, + ParentUID: f.ParentUID, + ManagedBy: f.ManagedBy, }) metrics.MFolderIDsAPICount.WithLabelValues(metrics.GetFolders).Inc() } @@ -427,7 +427,7 @@ func (hs *HTTPServer) newToFolderDto(c *contextmodel.ReqContext, f *folder.Folde Version: f.Version, AccessControl: acMetadata, ParentUID: f.ParentUID, - Repository: f.Repository, + ManagedBy: f.ManagedBy, }, nil } diff --git a/pkg/apimachinery/utils/manager.go b/pkg/apimachinery/utils/manager.go index 76add8722c7..0b76cee2928 100644 --- a/pkg/apimachinery/utils/manager.go +++ b/pkg/apimachinery/utils/manager.go @@ -1,28 +1,27 @@ package utils -import "time" - // ManagerProperties is used to identify the manager of the resource. type ManagerProperties struct { // The kind of manager, which is responsible for managing the resource. // Examples include "git", "terraform", "kubectl", etc. - Kind ManagerKind + Kind ManagerKind `json:"kind,omitempty"` // The identity of the manager, which refers to a specific instance of the manager. // The format & the value depends on the manager kind. - Identity string + Identity string `json:"id,omitempty"` // AllowsEdits indicates whether the manager allows edits to the resource. // If set to true, it means that other requesters can edit the resource. - AllowsEdits bool + AllowsEdits bool `json:"allowEdits,omitempty"` // Suspended indicates whether the manager is suspended. // If set to true, then the manager skip updates to the resource. - Suspended bool + Suspended bool `json:"suspended,omitempty"` } // ManagerKind is the type of manager, which is responsible for managing the resource. // It can be a user or a tool or a generic API client. +// +enum type ManagerKind string // Known values for ManagerKind. @@ -31,6 +30,11 @@ const ( ManagerKindRepo ManagerKind = "repo" ManagerKindTerraform ManagerKind = "terraform" ManagerKindKubectl ManagerKind = "kubectl" + ManagerKindPlugin ManagerKind = "plugin" + + // Deprecated: this is used as a shim/migration path for legacy file provisioning + // Previously this was a "file:" prefix + ManagerKindClassicFP ManagerKind = "classic-file-provisioning" ) // ParseManagerKindString parses a string into a ManagerKind. @@ -44,6 +48,10 @@ func ParseManagerKindString(v string) ManagerKind { return ManagerKindTerraform case string(ManagerKindKubectl): return ManagerKindKubectl + case string(ManagerKindPlugin): + return ManagerKindPlugin + case string(ManagerKindClassicFP): // nolint:staticcheck + return ManagerKindClassicFP // nolint:staticcheck default: return ManagerKindUnknown } @@ -55,13 +63,13 @@ func ParseManagerKindString(v string) ManagerKind { type SourceProperties struct { // The path to the source of the resource. // Can be a file path, a URL, etc. - Path string + Path string `json:"path,omitempty"` // The checksum of the source of the resource. // An example could be a git commit hash. - Checksum string + Checksum string `json:"checksum,omitempty"` - // The timestamp of the source of the resource. + // The unix millis timestamp of the source of the resource. // An example could be the file modification time. - Timestamp time.Time + TimestampMillis int64 `json:"timestampMillis,omitempty"` } diff --git a/pkg/apimachinery/utils/meta.go b/pkg/apimachinery/utils/meta.go index 116fc4f37d9..49bbcd3eb02 100644 --- a/pkg/apimachinery/utils/meta.go +++ b/pkg/apimachinery/utils/meta.go @@ -41,10 +41,10 @@ const AnnoKeyMessage = "grafana.app/message" // Identify where values came from -const AnnoKeyRepoName = "grafana.app/repoName" -const AnnoKeyRepoPath = "grafana.app/repoPath" -const AnnoKeyRepoHash = "grafana.app/repoHash" -const AnnoKeyRepoTimestamp = "grafana.app/repoTimestamp" +const oldAnnoKeyRepoName = "grafana.app/repoName" +const oldAnnoKeyRepoPath = "grafana.app/repoPath" +const oldAnnoKeyRepoHash = "grafana.app/repoHash" +const oldAnnoKeyRepoTimestamp = "grafana.app/repoTimestamp" // Annotations used to store manager properties @@ -56,41 +56,13 @@ const AnnoKeyManagerSuspended = "grafana.app/managerSuspended" // Annotations used to store source properties const AnnoKeySourcePath = "grafana.app/sourcePath" -const AnnoKeySourceHash = "grafana.app/sourceHash" +const AnnoKeySourceChecksum = "grafana.app/sourceChecksum" const AnnoKeySourceTimestamp = "grafana.app/sourceTimestamp" // LabelKeyDeprecatedInternalID gives the deprecated internal ID of a resource // Deprecated: will be removed in grafana 13 const LabelKeyDeprecatedInternalID = "grafana.app/deprecatedInternalID" -// These can be removed once we verify that non of the dual-write sources -// (for dashboards/playlists/etc) depend on the saved internal ID in SQL -const oldAnnoKeyOriginName = "grafana.app/originName" -const oldAnnoKeyOriginPath = "grafana.app/originPath" -const oldAnnoKeyOriginHash = "grafana.app/originHash" -const oldAnnoKeyOriginTimestamp = "grafana.app/originTimestamp" - -// ResourceRepositoryInfo is encoded into kubernetes metadata annotations. -// This value identifies indicates the state of the resource in its provisioning source when -// the spec was last saved. Currently this is derived from the dashboards provisioning table. -type ResourceRepositoryInfo struct { - // Name of the repository/provisioning source - Name string `json:"name,omitempty"` - - // The path within the named repository above (external_id in the existing dashboard provisioning) - Path string `json:"path,omitempty"` - - // Verification/identification hash (check_sum in existing dashboard provisioning) - Hash string `json:"hash,omitempty"` - - // Origin modification timestamp when the resource was saved - // This will be before the resource updated time - Timestamp *time.Time `json:"time,omitempty"` - - // Avoid extending - _ any `json:"-"` -} - // Accessor functions for k8s objects type GrafanaMetaAccessor interface { metav1.Object @@ -128,13 +100,6 @@ type GrafanaMetaAccessor interface { // Deprecated: This will be removed in Grafana 13 SetDeprecatedInternalID(id int64) - GetRepositoryInfo() (*ResourceRepositoryInfo, error) - SetRepositoryInfo(info *ResourceRepositoryInfo) - GetRepositoryName() string - GetRepositoryPath() string - GetRepositoryHash() string - GetRepositoryTimestamp() (*time.Time, error) - GetSpec() (any, error) SetSpec(any) error @@ -351,90 +316,6 @@ func (m *grafanaMetaAccessor) SetDeprecatedInternalID(id int64) { m.obj.SetLabels(labels) } -// This allows looking up a primary and secondary key -- if either exist the value will be returned -func (m *grafanaMetaAccessor) getAnnoValue(primary, secondary string) (string, bool) { - v, ok := m.obj.GetAnnotations()[primary] - if !ok { - v, ok = m.obj.GetAnnotations()[secondary] - } - return v, ok -} - -func (m *grafanaMetaAccessor) SetRepositoryInfo(info *ResourceRepositoryInfo) { - anno := m.obj.GetAnnotations() - if anno == nil { - if info == nil { - return - } - anno = make(map[string]string, 0) - } - - // remove legacy values - delete(anno, oldAnnoKeyOriginHash) - delete(anno, oldAnnoKeyOriginPath) - delete(anno, oldAnnoKeyOriginHash) - delete(anno, oldAnnoKeyOriginTimestamp) - - delete(anno, AnnoKeyRepoName) - delete(anno, AnnoKeyRepoPath) - delete(anno, AnnoKeyRepoHash) - delete(anno, AnnoKeyRepoTimestamp) - if info != nil && info.Name != "" { - anno[AnnoKeyRepoName] = info.Name - if info.Path != "" { - anno[AnnoKeyRepoPath] = info.Path - } - if info.Hash != "" { - anno[AnnoKeyRepoHash] = info.Hash - } - if info.Timestamp != nil { - anno[AnnoKeyRepoTimestamp] = info.Timestamp.UTC().Format(time.RFC3339) - } - } - m.obj.SetAnnotations(anno) -} - -func (m *grafanaMetaAccessor) GetRepositoryInfo() (*ResourceRepositoryInfo, error) { - v, ok := m.getAnnoValue(AnnoKeyRepoName, oldAnnoKeyOriginName) - if !ok { - return nil, nil - } - t, err := m.GetRepositoryTimestamp() - return &ResourceRepositoryInfo{ - Name: v, - Path: m.GetRepositoryPath(), - Hash: m.GetRepositoryHash(), - Timestamp: t, - }, err -} - -func (m *grafanaMetaAccessor) GetRepositoryName() string { - v, _ := m.getAnnoValue(AnnoKeyRepoName, oldAnnoKeyOriginName) - return v // will be empty string -} - -func (m *grafanaMetaAccessor) GetRepositoryPath() string { - v, _ := m.getAnnoValue(AnnoKeyRepoPath, oldAnnoKeyOriginPath) - return v // will be empty string -} - -func (m *grafanaMetaAccessor) GetRepositoryHash() string { - v, _ := m.getAnnoValue(AnnoKeyRepoHash, oldAnnoKeyOriginHash) - return v // will be empty string -} - -func (m *grafanaMetaAccessor) GetRepositoryTimestamp() (*time.Time, error) { - v, ok := m.getAnnoValue(AnnoKeyRepoTimestamp, oldAnnoKeyOriginTimestamp) - if !ok || v == "" { - return nil, nil - } - t, err := time.Parse(time.RFC3339, v) - if err != nil { - return nil, fmt.Errorf("invalid origin timestamp: %s", err.Error()) - } - return &t, nil -} - // GetAnnotations implements GrafanaMetaAccessor. func (m *grafanaMetaAccessor) GetAnnotations() map[string]string { return m.obj.GetAnnotations() @@ -751,7 +632,7 @@ func (m *grafanaMetaAccessor) GetManagerProperties() (ManagerProperties, bool) { res := ManagerProperties{ Identity: "", Kind: ManagerKindUnknown, - AllowsEdits: true, + AllowsEdits: false, Suspended: false, } @@ -759,6 +640,15 @@ func (m *grafanaMetaAccessor) GetManagerProperties() (ManagerProperties, bool) { id, ok := annot[AnnoKeyManagerIdentity] if !ok || id == "" { + // Temporarily support the repo name annotation + repo := annot[oldAnnoKeyRepoName] + if repo != "" { + return ManagerProperties{ + Kind: ManagerKindRepo, + Identity: repo, + }, true + } + // If the identity is not set, we should ignore the other annotations and return the default values. // // This is to prevent inadvertently marking resources as managed, @@ -788,10 +678,31 @@ func (m *grafanaMetaAccessor) SetManagerProperties(v ManagerProperties) { annot = make(map[string]string, 4) } - annot[AnnoKeyManagerIdentity] = v.Identity - annot[AnnoKeyManagerKind] = string(v.Kind) - annot[AnnoKeyManagerAllowsEdits] = strconv.FormatBool(v.AllowsEdits) - annot[AnnoKeyManagerSuspended] = strconv.FormatBool(v.Suspended) + if v.Identity != "" { + annot[AnnoKeyManagerIdentity] = v.Identity + } else { + delete(annot, AnnoKeyManagerIdentity) + } + + if string(v.Kind) != "" { + annot[AnnoKeyManagerKind] = string(v.Kind) + } else { + delete(annot, AnnoKeyManagerKind) + } + + if v.AllowsEdits { + annot[AnnoKeyManagerAllowsEdits] = strconv.FormatBool(v.AllowsEdits) + } else { + delete(annot, AnnoKeyManagerAllowsEdits) + } + if v.Suspended { + annot[AnnoKeyManagerSuspended] = strconv.FormatBool(v.Suspended) + } else { + delete(annot, AnnoKeyManagerSuspended) + } + + // Clean up old annotation access + delete(annot, oldAnnoKeyRepoName) m.obj.SetAnnotations(annot) } @@ -810,16 +721,27 @@ func (m *grafanaMetaAccessor) GetSourceProperties() (SourceProperties, bool) { if path, ok := annot[AnnoKeySourcePath]; ok && path != "" { res.Path = path found = true + } else if path, ok := annot[oldAnnoKeyRepoPath]; ok && path != "" { + res.Path = path + found = true } - if hash, ok := annot[AnnoKeySourceHash]; ok && hash != "" { + if hash, ok := annot[AnnoKeySourceChecksum]; ok && hash != "" { + res.Checksum = hash + found = true + } else if hash, ok := annot[oldAnnoKeyRepoHash]; ok && hash != "" { res.Checksum = hash found = true } - if timestamp, ok := annot[AnnoKeySourceTimestamp]; ok && timestamp != "" { - if t, err := time.Parse(time.RFC3339, timestamp); err == nil { - res.Timestamp = t + t, ok := annot[AnnoKeySourceTimestamp] + if !ok { + t, ok = annot[oldAnnoKeyRepoTimestamp] + } + if ok && t != "" { + var err error + res.TimestampMillis, err = strconv.ParseInt(t, 10, 64) + if err != nil { found = true } } @@ -835,14 +757,20 @@ func (m *grafanaMetaAccessor) SetSourceProperties(v SourceProperties) { if v.Path != "" { annot[AnnoKeySourcePath] = v.Path + } else { + delete(annot, AnnoKeySourcePath) } if v.Checksum != "" { - annot[AnnoKeySourceHash] = v.Checksum + annot[AnnoKeySourceChecksum] = v.Checksum + } else { + delete(annot, AnnoKeySourceChecksum) } - if !v.Timestamp.IsZero() { - annot[AnnoKeySourceTimestamp] = v.Timestamp.Format(time.RFC3339) + if v.TimestampMillis > 0 { + annot[AnnoKeySourceTimestamp] = strconv.FormatInt(v.TimestampMillis, 10) + } else { + delete(annot, AnnoKeySourceTimestamp) } m.obj.SetAnnotations(annot) diff --git a/pkg/apimachinery/utils/meta_test.go b/pkg/apimachinery/utils/meta_test.go index 1796125048f..c6908adf2dd 100644 --- a/pkg/apimachinery/utils/meta_test.go +++ b/pkg/apimachinery/utils/meta_test.go @@ -131,10 +131,13 @@ func (in *Spec2) DeepCopy() *Spec2 { } func TestMetaAccessor(t *testing.T) { - repoInfo := &utils.ResourceRepositoryInfo{ - Name: "test", - Path: "a/b/c", - Hash: "kkk", + repoInfo := utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "test", + } + sourceInfo := utils.SourceProperties{ + Path: "a/b/c", + Checksum: "kkk", } t.Run("fails for non resource objects", func(t *testing.T) { @@ -202,14 +205,13 @@ func TestMetaAccessor(t *testing.T) { }, } - meta.SetRepositoryInfo(repoInfo) + meta.SetManagerProperties(repoInfo) meta.SetFolder("folderUID") require.Equal(t, map[string]string{ - "grafana.app/repoName": "test", - "grafana.app/repoPath": "a/b/c", - "grafana.app/repoHash": "kkk", - "grafana.app/folder": "folderUID", + "grafana.app/managedBy": "repo", + "grafana.app/managerId": "test", + "grafana.app/folder": "folderUID", }, res.GetAnnotations()) meta.SetNamespace("aaa") @@ -255,14 +257,16 @@ func TestMetaAccessor(t *testing.T) { meta, err := utils.MetaAccessor(res) require.NoError(t, err) - meta.SetRepositoryInfo(repoInfo) + meta.SetManagerProperties(repoInfo) + meta.SetSourceProperties(sourceInfo) meta.SetFolder("folderUID") require.Equal(t, map[string]string{ - "grafana.app/repoName": "test", - "grafana.app/repoPath": "a/b/c", - "grafana.app/repoHash": "kkk", - "grafana.app/folder": "folderUID", + "grafana.app/managedBy": "repo", + "grafana.app/managerId": "test", + "grafana.app/sourcePath": "a/b/c", + "grafana.app/sourceChecksum": "kkk", + "grafana.app/folder": "folderUID", }, res.GetAnnotations()) meta.SetNamespace("aaa") @@ -306,14 +310,13 @@ func TestMetaAccessor(t *testing.T) { meta, err := utils.MetaAccessor(res) require.NoError(t, err) - meta.SetRepositoryInfo(repoInfo) + meta.SetManagerProperties(repoInfo) meta.SetFolder("folderUID") require.Equal(t, map[string]string{ - "grafana.app/repoName": "test", - "grafana.app/repoPath": "a/b/c", - "grafana.app/repoHash": "kkk", - "grafana.app/folder": "folderUID", + "grafana.app/managedBy": "repo", + "grafana.app/managerId": "test", + "grafana.app/folder": "folderUID", }, res.GetAnnotations()) meta.SetNamespace("aaa") @@ -347,7 +350,7 @@ func TestMetaAccessor(t *testing.T) { require.Equal(t, "ZZ", res.Status.Title) }) - t.Run("test reading old originInfo (now repository)", func(t *testing.T) { + t.Run("test reading old repo fields (now manager+source)", func(t *testing.T) { res := &TestResource2{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ @@ -362,11 +365,15 @@ func TestMetaAccessor(t *testing.T) { meta, err := utils.MetaAccessor(res) require.NoError(t, err) - info, err := meta.GetRepositoryInfo() - require.NoError(t, err) - require.Equal(t, "test", info.Name) - require.Equal(t, "a/b/c", info.Path) - require.Equal(t, "zzz", info.Hash) + manager, ok := meta.GetManagerProperties() + require.True(t, ok) + require.Equal(t, utils.ManagerKindRepo, manager.Kind) + require.Equal(t, "test", manager.Identity) + + source, ok := meta.GetSourceProperties() + require.True(t, ok) + require.Equal(t, "a/b/c", source.Path) + require.Equal(t, "zzz", source.Checksum) }) t.Run("blob info", func(t *testing.T) { @@ -391,14 +398,16 @@ func TestMetaAccessor(t *testing.T) { meta, err := utils.MetaAccessor(obj) require.NoError(t, err) - meta.SetRepositoryInfo(repoInfo) + meta.SetManagerProperties(repoInfo) + meta.SetSourceProperties(sourceInfo) meta.SetFolder("folderUID") require.Equal(t, map[string]string{ - "grafana.app/repoName": "test", - "grafana.app/repoPath": "a/b/c", - "grafana.app/repoHash": "kkk", - "grafana.app/folder": "folderUID", + "grafana.app/managedBy": "repo", + "grafana.app/managerId": "test", + "grafana.app/sourcePath": "a/b/c", + "grafana.app/sourceChecksum": "kkk", + "grafana.app/folder": "folderUID", }, obj.GetAnnotations()) require.Equal(t, "HELLO", obj.Spec.Title) @@ -414,14 +423,13 @@ func TestMetaAccessor(t *testing.T) { meta, err = utils.MetaAccessor(obj2) require.NoError(t, err) - meta.SetRepositoryInfo(repoInfo) + meta.SetManagerProperties(repoInfo) meta.SetFolder("folderUID") require.Equal(t, map[string]string{ - "grafana.app/repoName": "test", - "grafana.app/repoPath": "a/b/c", - "grafana.app/repoHash": "kkk", - "grafana.app/folder": "folderUID", + "grafana.app/managedBy": "repo", + "grafana.app/managerId": "test", + "grafana.app/folder": "folderUID", }, obj2.GetAnnotations()) require.Equal(t, "xxx", meta.FindTitle("xxx")) @@ -447,7 +455,7 @@ func TestMetaAccessor(t *testing.T) { wantProperties: utils.ManagerProperties{ Identity: "", Kind: utils.ManagerKindUnknown, - AllowsEdits: true, + AllowsEdits: false, Suspended: false, }, wantOK: false, @@ -479,7 +487,7 @@ func TestMetaAccessor(t *testing.T) { wantProperties: utils.ManagerProperties{ Identity: "", Kind: utils.ManagerKindUnknown, - AllowsEdits: true, + AllowsEdits: false, Suspended: false, }, wantOK: false, @@ -534,14 +542,14 @@ func TestMetaAccessor(t *testing.T) { { name: "set and get valid values", setProperties: &utils.SourceProperties{ - Path: "path", - Checksum: "hash", - Timestamp: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC), + Path: "path", + Checksum: "hash", + TimestampMillis: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC).UnixMilli(), }, wantProperties: utils.SourceProperties{ - Path: "path", - Checksum: "hash", - Timestamp: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC), + Path: "path", + Checksum: "hash", + TimestampMillis: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC).UnixMilli(), }, wantOK: true, }, diff --git a/pkg/apis/dashboard/utils.go b/pkg/apis/dashboard/utils.go index a7fa10af2d0..a5979151a90 100644 --- a/pkg/apis/dashboard/utils.go +++ b/pkg/apis/dashboard/utils.go @@ -1,51 +1,31 @@ package dashboard import ( - "strings" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/grafana/grafana/pkg/apimachinery/utils" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -var PluginIDRepoName = "plugin" -var fileProvisionedRepoPrefix = "file:" - -// ProvisionedFileNameWithPrefix adds the `file:` prefix to the -// provisioner name, to be used as the annotation for dashboards -// provisioned from files -func ProvisionedFileNameWithPrefix(name string) string { - if name == "" { - return "" - } - - return fileProvisionedRepoPrefix + name -} - -// GetProvisionedFileNameFromMeta returns the provisioner name -// from a given annotation string, which is in the form file: -func GetProvisionedFileNameFromMeta(annotation string) (string, bool) { - return strings.CutPrefix(annotation, fileProvisionedRepoPrefix) -} - // SetPluginIDMeta sets the repo name to "plugin" and the path to the plugin ID -func SetPluginIDMeta(obj unstructured.Unstructured, pluginID string) { +func SetPluginIDMeta(obj *unstructured.Unstructured, pluginID string) { if pluginID == "" { return } - annotations := obj.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} + meta, err := utils.MetaAccessor(obj) + if err == nil { + meta.SetManagerProperties(utils.ManagerProperties{ + Kind: utils.ManagerKindPlugin, + Identity: pluginID, + }) } - annotations[utils.AnnoKeyRepoName] = PluginIDRepoName - annotations[utils.AnnoKeyRepoPath] = pluginID - obj.SetAnnotations(annotations) } // GetPluginIDFromMeta returns the plugin ID from the meta if the repo name is "plugin" func GetPluginIDFromMeta(obj utils.GrafanaMetaAccessor) string { - if obj.GetRepositoryName() == PluginIDRepoName { - return obj.GetRepositoryPath() + p, ok := obj.GetManagerProperties() + if ok && p.Kind == utils.ManagerKindPlugin { + return p.Identity } return "" } diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 08e8cc2a709..53adbb60f6a 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -317,13 +317,6 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows, history bool) (*dashboardRo } if origin_name.String != "" { - ts := time.Unix(origin_ts.Int64, 0) - - repo := &utils.ResourceRepositoryInfo{ - Name: dashboardOG.ProvisionedFileNameWithPrefix(origin_name.String), - Hash: origin_hash.String, - Timestamp: &ts, - } // if the reader cannot be found, it may be an orphaned provisioned dashboard resolvedPath := a.provisioning.GetDashboardProvisionerResolvedPath(origin_name.String) if resolvedPath != "" { @@ -334,13 +327,20 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows, history bool) (*dashboardRo if err != nil { return nil, err } - repo.Path = originPath + meta.SetSourceProperties(utils.SourceProperties{ + Path: originPath, // relative path within source + Checksum: origin_hash.String, + TimestampMillis: origin_ts.Int64, + }) + meta.SetManagerProperties(utils.ManagerProperties{ + Kind: utils.ManagerKindClassicFP, // nolint:staticcheck + Identity: origin_name.String, + }) } - meta.SetRepositoryInfo(repo) } else if plugin_id.String != "" { - meta.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: dashboardOG.PluginIDRepoName, - Path: plugin_id.String, + meta.SetManagerProperties(utils.ManagerProperties{ + Kind: utils.ManagerKindPlugin, + Identity: plugin_id.String, }) } diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go index 2436d5a8e73..692376ebb4b 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards_test.go @@ -83,12 +83,18 @@ func TestScanRow(t *testing.T) { meta, err := utils.MetaAccessor(row.Dash) require.NoError(t, err) - require.Equal(t, "file:provisioner", meta.GetRepositoryName()) // should be prefixed by file: - require.Equal(t, "../"+pathToFile, meta.GetRepositoryPath()) // relative to provisioner - require.Equal(t, "hashing", meta.GetRepositoryHash()) - ts, err := meta.GetRepositoryTimestamp() + m, ok := meta.GetManagerProperties() + require.True(t, ok) + + s, ok := meta.GetSourceProperties() + require.True(t, ok) + + require.Equal(t, utils.ManagerKindClassicFP, m.Kind) // nolint:staticcheck + require.Equal(t, "provisioner", m.Identity) + require.Equal(t, "../"+pathToFile, s.Path) // relative to provisioner + require.Equal(t, "hashing", s.Checksum) require.NoError(t, err) - require.Equal(t, int64(100000), ts.Unix()) + require.Equal(t, int64(100000), s.TimestampMillis) }) t.Run("Plugin provisioned dashboard should have annotations", func(t *testing.T) { @@ -105,8 +111,11 @@ func TestScanRow(t *testing.T) { meta, err := utils.MetaAccessor(row.Dash) require.NoError(t, err) - require.Equal(t, "plugin", meta.GetRepositoryName()) - require.Equal(t, "slo", meta.GetRepositoryPath()) // the ID of the plugin - require.Equal(t, "", meta.GetRepositoryHash()) // hash is not used on plugins + manager, ok := meta.GetManagerProperties() + require.True(t, ok) + + require.Equal(t, utils.ManagerKindPlugin, manager.Kind) + require.Equal(t, "slo", manager.Identity) // the ID of the plugin + require.Equal(t, "", meta.GetAnnotations()[utils.AnnoKeySourceChecksum]) // hash is not used on plugins }) } diff --git a/pkg/registry/apis/dashboard/legacysearcher/search_client.go b/pkg/registry/apis/dashboard/legacysearcher/search_client.go index 3947cecaf1e..1f98dc3a1e1 100644 --- a/pkg/registry/apis/dashboard/legacysearcher/search_client.go +++ b/pkg/registry/apis/dashboard/legacysearcher/search_client.go @@ -13,7 +13,6 @@ import ( claims "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" - dashboardOG "github.com/grafana/grafana/pkg/apis/dashboard" dashboard "github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1" folderv0alpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" "github.com/grafana/grafana/pkg/services/dashboards" @@ -181,18 +180,22 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resource.Resour } query.FolderUIDs = folders - case resource.SEARCH_FIELD_REPOSITORY_PATH: + case resource.SEARCH_FIELD_SOURCE_PATH: // only one value is supported in legacy search if len(vals) != 1 { return nil, fmt.Errorf("only one repo path query is supported") } - query.ProvisionedPath = vals[0] - case resource.SEARCH_FIELD_REPOSITORY_NAME: + query.SourcePath = vals[0] + + case resource.SEARCH_FIELD_MANAGER_KIND: + if len(vals) != 1 { + return nil, fmt.Errorf("only one manager kind supported") + } + query.ManagedBy = utils.ManagerKind(vals[0]) + + case resource.SEARCH_FIELD_MANAGER_ID: if field.Operator == string(selection.NotIn) { - for _, val := range vals { - name, _ := dashboardOG.GetProvisionedFileNameFromMeta(val) - query.ProvisionedReposNotIn = append(query.ProvisionedReposNotIn, name) - } + query.ManagerIdentityNotIn = vals continue } @@ -200,8 +203,7 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resource.Resour if len(vals) != 1 { return nil, fmt.Errorf("only one repo name is supported") } - - query.ProvisionedRepo, _ = dashboardOG.GetProvisionedFileNameFromMeta(vals[0]) + query.ManagerIdentity = vals[0] } } searchFields := resource.StandardSearchFields() @@ -221,17 +223,21 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resource.Resour // if we are querying for provisioning information, we need to use a different // legacy sql query, since legacy search does not support this - if query.ProvisionedRepo != "" || len(query.ProvisionedReposNotIn) > 0 { + if query.ManagerIdentity != "" || len(query.ManagerIdentityNotIn) > 0 { + if query.ManagedBy == utils.ManagerKindUnknown { + return nil, fmt.Errorf("query by manager identity also requires manager.kind parameter") + } + var dashes []*dashboards.Dashboard - if query.ProvisionedRepo == dashboardOG.PluginIDRepoName { + if query.ManagedBy == utils.ManagerKindPlugin { dashes, err = c.dashboardStore.GetDashboardsByPluginID(ctx, &dashboards.GetDashboardsByPluginIDQuery{ - PluginID: query.ProvisionedPath, + PluginID: query.ManagerIdentity, OrgID: user.GetOrgID(), }) - } else if query.ProvisionedRepo != "" { - dashes, err = c.dashboardStore.GetProvisionedDashboardsByName(ctx, query.ProvisionedRepo) - } else if len(query.ProvisionedReposNotIn) > 0 { - dashes, err = c.dashboardStore.GetOrphanedProvisionedDashboards(ctx, query.ProvisionedReposNotIn) + } else if query.ManagerIdentity != "" { + dashes, err = c.dashboardStore.GetProvisionedDashboardsByName(ctx, query.ManagerIdentity) + } else if len(query.ManagerIdentityNotIn) > 0 { + dashes, err = c.dashboardStore.GetOrphanedProvisionedDashboards(ctx, query.ManagerIdentityNotIn) } if err != nil { return nil, err diff --git a/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go b/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go index 5849f146184..4b56d2e9b6f 100644 --- a/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go +++ b/pkg/registry/apis/dashboard/legacysearcher/search_client_test.go @@ -373,12 +373,12 @@ func TestDashboardSearchClient_Search(t *testing.T) { Key: dashboardKey, Fields: []*resource.Requirement{ { - Key: resource.SEARCH_FIELD_REPOSITORY_PATH, + Key: resource.SEARCH_FIELD_MANAGER_ID, Operator: "in", Values: []string{"slo"}, }, { - Key: resource.SEARCH_FIELD_REPOSITORY_NAME, + Key: resource.SEARCH_FIELD_MANAGER_KIND, Operator: "in", Values: []string{"plugin"}, }, @@ -402,9 +402,14 @@ func TestDashboardSearchClient_Search(t *testing.T) { Key: dashboardKey, Fields: []*resource.Requirement{ { - Key: resource.SEARCH_FIELD_REPOSITORY_NAME, + Key: resource.SEARCH_FIELD_MANAGER_KIND, + Operator: "=", + Values: []string{string(utils.ManagerKindClassicFP)}, // nolint:staticcheck + }, + { + Key: resource.SEARCH_FIELD_MANAGER_ID, Operator: "in", - Values: []string{"file:test"}, // file prefix should be removed before going to legacy + Values: []string{"test"}, }, }, }, @@ -426,9 +431,14 @@ func TestDashboardSearchClient_Search(t *testing.T) { Key: dashboardKey, Fields: []*resource.Requirement{ { - Key: resource.SEARCH_FIELD_REPOSITORY_NAME, + Key: resource.SEARCH_FIELD_MANAGER_KIND, + Operator: "=", + Values: []string{string(utils.ManagerKindClassicFP)}, // nolint:staticcheck + }, + { + Key: resource.SEARCH_FIELD_MANAGER_ID, Operator: string(selection.NotIn), - Values: []string{"file:test", "file:test2"}, // file prefix should be removed before going to legacy + Values: []string{"test", "test2"}, }, }, }, diff --git a/pkg/registry/apis/dashboard/sub_dto.go b/pkg/registry/apis/dashboard/sub_dto.go index abc691e3517..33ffe2c8646 100644 --- a/pkg/registry/apis/dashboard/sub_dto.go +++ b/pkg/registry/apis/dashboard/sub_dto.go @@ -134,13 +134,9 @@ func (r *DTOConnector) Connect(ctx context.Context, name string, opts runtime.Ob OrgID: info.OrgID, ID: obj.GetDeprecatedInternalID(), // nolint:staticcheck } - repo, err := obj.GetRepositoryInfo() - if err != nil { - responder.Error(err) - return - } - if repo != nil && repo.Name == dashboard.PluginIDRepoName { - dto.PluginID = repo.Path + manager, ok := obj.GetManagerProperties() + if ok && manager.Kind == utils.ManagerKindPlugin { + dto.PluginID = manager.Identity } guardian, err := guardian.NewByDashboard(ctx, dto, info.OrgID, user) diff --git a/pkg/registry/apis/folders/legacy_storage.go b/pkg/registry/apis/folders/legacy_storage.go index 34dcaa9a73b..550331c855e 100644 --- a/pkg/registry/apis/folders/legacy_storage.go +++ b/pkg/registry/apis/folders/legacy_storage.go @@ -2,7 +2,6 @@ package folders import ( "context" - "errors" "fmt" "strings" @@ -138,13 +137,14 @@ func (s *legacyStorage) Get(ctx context.Context, name string, options *metav1.Ge UID: &name, OrgID: info.OrgID, }) - if err != nil || dto == nil { - if errors.Is(err, dashboards.ErrFolderNotFound) || err == nil { - err = resourceInfo.NewNotFound(name) - } + if err != nil { statusErr := apierrors.ToFolderStatusError(err) return nil, &statusErr } + if dto == nil { + statusErr := apierrors.ToFolderStatusError(dashboards.ErrFolderNotFound) + return nil, &statusErr + } r, err := convertToK8sResource(dto, s.namespacer) if err != nil { diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go index 4e5e144d50e..6d4d64f1d9a 100644 --- a/pkg/registry/apis/folders/register.go +++ b/pkg/registry/apis/folders/register.go @@ -297,6 +297,10 @@ func (b *FolderAPIBuilder) validateOnCreate(ctx context.Context, id string, obj return dashboards.ErrFolderTitleEmpty } + if f.Name == getParent(obj) { + return folder.ErrFolderCannotBeParentOfItself + } + _, err := b.checkFolderMaxDepth(ctx, obj) if err != nil { return err diff --git a/pkg/registry/apis/folders/register_test.go b/pkg/registry/apis/folders/register_test.go index 5d80a2c2369..17ba1f542e1 100644 --- a/pkg/registry/apis/folders/register_test.go +++ b/pkg/registry/apis/folders/register_test.go @@ -9,6 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -26,13 +27,22 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) { name string } - circularObj := &v0alpha1.Folder{ + initialMaxDepth := folderValidationRules.maxDepth + folderValidationRules.maxDepth = 2 + defer func() { folderValidationRules.maxDepth = initialMaxDepth }() + deepFolder := &v0alpha1.Folder{ Spec: v0alpha1.Spec{ Title: "foo", }, } - circularObj.Name = "valid-name" - circularObj.Annotations = map[string]string{"grafana.app/folder": "valid-name"} + deepFolder.Name = "valid-parent" + deepFolder.Annotations = map[string]string{"grafana.app/folder": "valid-grandparent"} + parentFolder := &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "foo-grandparent", + }, + } + deepFolder.Name = "valid-grandparent" tests := []struct { name string @@ -71,12 +81,15 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) { Title: "foo", }, }, - annotations: map[string]string{"grafana.app/folder": "valid-name"}, + annotations: map[string]string{"grafana.app/folder": "valid-parent"}, name: "valid-name", }, setupFn: func(m *mock.Mock) { - m.On("Get", mock.Anything, "valid-name", mock.Anything).Return( - circularObj, + m.On("Get", mock.Anything, "valid-parent", mock.Anything).Return( + deepFolder, + nil) + m.On("Get", mock.Anything, "valid-grandparent", mock.Anything).Return( + parentFolder, nil) }, err: folder.ErrMaximumDepthReached, @@ -93,6 +106,19 @@ func TestFolderAPIBuilder_Validate_Create(t *testing.T) { }, err: dashboards.ErrFolderTitleEmpty, }, + { + name: "should return error if folder is a parent of itself", + input: input{ + annotations: map[string]string{utils.AnnoKeyFolder: "myself"}, + obj: &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "title", + }, + }, + name: "myself", + }, + err: folder.ErrFolderCannotBeParentOfItself, + }, } s := (grafanarest.Storage)(nil) diff --git a/pkg/services/apiserver/service.go b/pkg/services/apiserver/service.go index 1105604aee1..4a4b5ac3ab0 100644 --- a/pkg/services/apiserver/service.go +++ b/pkg/services/apiserver/service.go @@ -80,6 +80,8 @@ var ( ready = make(chan struct{}) ) +const MaxRequestBodyBytes = 16 * 1024 * 1024 // 16MB - determined by the size of `mediumtext` on mysql, which is used to save dashboard data + func init() { // we need to add the options to empty v1 metav1.AddToGroupVersion(Scheme, schema.GroupVersion{Group: "", Version: "v1"}) @@ -335,6 +337,7 @@ func (s *service) start(ctx context.Context) error { transport := &roundTripperFunc{ready: make(chan struct{})} serverConfig.LoopbackClientConfig.Transport = transport serverConfig.LoopbackClientConfig.TLSClientConfig = clientrest.TLSClientConfig{} + serverConfig.MaxRequestBodyBytes = MaxRequestBodyBytes var optsregister apistore.StorageOptionsRegister diff --git a/pkg/services/dashboards/errors.go b/pkg/services/dashboards/errors.go index 93f6b1152e0..8665015c940 100644 --- a/pkg/services/dashboards/errors.go +++ b/pkg/services/dashboards/errors.go @@ -42,6 +42,11 @@ var ( StatusCode: 400, Status: "empty-name", } + ErrDashboardTitleTooLong = DashboardErr{ + Reason: "Dashboard title cannot contain more than 5 000 characters", + StatusCode: 400, + Status: "title-too-long", + } ErrDashboardFolderCannotHaveParent = DashboardErr{ Reason: "A Dashboard Folder cannot be added to another folder", StatusCode: 400, diff --git a/pkg/services/dashboards/models.go b/pkg/services/dashboards/models.go index 78f9f7dc827..0128d7d2e17 100644 --- a/pkg/services/dashboards/models.go +++ b/pkg/services/dashboards/models.go @@ -5,6 +5,7 @@ import ( "time" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/slugify" @@ -438,9 +439,10 @@ type FindPersistedDashboardsQuery struct { Sort model.SortOption IsDeleted bool - ProvisionedRepo string - ProvisionedPath string - ProvisionedReposNotIn []string + ManagedBy utils.ManagerKind + ManagerIdentity string + SourcePath string + ManagerIdentityNotIn []string Filters []any diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 2d3e7c8fb07..3b1f28764fd 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -237,7 +237,8 @@ func (dr *DashboardServiceImpl) GetProvisionedDashboardData(ctx context.Context, func(orgID int64) { g.Go(func() error { res, err := dr.searchProvisionedDashboardsThroughK8s(ctx, &dashboards.FindPersistedDashboardsQuery{ - ProvisionedRepo: name, + ManagedBy: utils.ManagerKindClassicFP, // nolint:staticcheck + ManagerIdentity: name, OrgId: orgID, }) if err != nil { @@ -341,6 +342,10 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d return nil, dashboards.ErrDashboardTitleEmpty } + if len(dash.Title) > 5000 { + return nil, dashboards.ErrDashboardTitleTooLong + } + if len(dto.Message) > 500 { return nil, dashboards.ErrDashboardMessageTooLong } @@ -564,8 +569,8 @@ func (dr *DashboardServiceImpl) DeleteOrphanedProvisionedDashboards(ctx context. ctx, _ := identity.WithServiceIdentity(ctx, org.ID) // find all dashboards in the org that have a file repo set that is not in the given readers list foundDashs, err := dr.searchProvisionedDashboardsThroughK8s(ctx, &dashboards.FindPersistedDashboardsQuery{ - ProvisionedReposNotIn: cmd.ReaderNames, - OrgId: org.ID, + ManagerIdentityNotIn: cmd.ReaderNames, + OrgId: org.ID, }) if err != nil { return err @@ -958,8 +963,8 @@ func (dr *DashboardServiceImpl) GetDashboardsByPluginID(ctx context.Context, que if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) { dashs, err := dr.searchDashboardsThroughK8s(ctx, &dashboards.FindPersistedDashboardsQuery{ OrgId: query.OrgID, - ProvisionedRepo: dashboard.PluginIDRepoName, - ProvisionedPath: query.PluginID, + ManagedBy: utils.ManagerKindPlugin, + ManagerIdentity: query.PluginID, }) if err != nil { return nil, err @@ -1549,22 +1554,22 @@ func (dr *DashboardServiceImpl) saveProvisionedDashboardThroughK8s(ctx context.C return nil, err } - annotations := obj.GetAnnotations() - if annotations == nil { - annotations = map[string]string{} + meta, err := utils.MetaAccessor(obj) + if err != nil { + return nil, err } - if unprovision { - delete(annotations, utils.AnnoKeyRepoName) - delete(annotations, utils.AnnoKeyRepoPath) - delete(annotations, utils.AnnoKeyRepoHash) - delete(annotations, utils.AnnoKeyRepoTimestamp) - } else { - annotations[utils.AnnoKeyRepoName] = dashboard.ProvisionedFileNameWithPrefix(provisioning.Name) - annotations[utils.AnnoKeyRepoPath] = provisioning.ExternalID - annotations[utils.AnnoKeyRepoHash] = provisioning.CheckSum - annotations[utils.AnnoKeyRepoTimestamp] = time.Unix(provisioning.Updated, 0).UTC().Format(time.RFC3339) + + m := utils.ManagerProperties{} + s := utils.SourceProperties{} + if !unprovision { + m.Kind = utils.ManagerKindClassicFP // nolint:staticcheck + m.Identity = provisioning.Name + s.Path = provisioning.ExternalID + s.Checksum = provisioning.CheckSum + s.TimestampMillis = time.Unix(provisioning.Updated, 0).UnixMilli() } - obj.SetAnnotations(annotations) + meta.SetManagerProperties(m) + meta.SetSourceProperties(s) out, err := dr.createOrUpdateDash(ctx, obj, cmd.OrgID) if err != nil { @@ -1590,16 +1595,16 @@ func (dr *DashboardServiceImpl) saveDashboardThroughK8s(ctx context.Context, cmd return out, nil } -func (dr *DashboardServiceImpl) createOrUpdateDash(ctx context.Context, obj unstructured.Unstructured, orgID int64) (*dashboards.Dashboard, error) { +func (dr *DashboardServiceImpl) createOrUpdateDash(ctx context.Context, obj *unstructured.Unstructured, orgID int64) (*dashboards.Dashboard, error) { var out *unstructured.Unstructured current, err := dr.k8sclient.Get(ctx, obj.GetName(), orgID, v1.GetOptions{}) if current == nil || err != nil { - out, err = dr.k8sclient.Create(ctx, &obj, orgID) + out, err = dr.k8sclient.Create(ctx, obj, orgID) if err != nil { return nil, err } } else { - out, err = dr.k8sclient.Update(ctx, &obj, orgID) + out, err = dr.k8sclient.Update(ctx, obj, orgID) if err != nil { return nil, err } @@ -1719,30 +1724,35 @@ func (dr *DashboardServiceImpl) searchDashboardsThroughK8sRaw(ctx context.Contex }) } - if query.ProvisionedRepo != "" { - req := []*resource.Requirement{{ - Key: resource.SEARCH_FIELD_REPOSITORY_NAME, - Operator: string(selection.In), - Values: []string{query.ProvisionedRepo}, - }} - request.Options.Fields = append(request.Options.Fields, req...) + if query.ManagedBy != "" { + request.Options.Fields = append(request.Options.Fields, &resource.Requirement{ + Key: resource.SEARCH_FIELD_MANAGER_KIND, + Operator: string(selection.Equals), + Values: []string{string(query.ManagedBy)}, + }) } - if len(query.ProvisionedReposNotIn) > 0 { - req := []*resource.Requirement{{ - Key: resource.SEARCH_FIELD_REPOSITORY_NAME, - Operator: string(selection.NotIn), - Values: query.ProvisionedReposNotIn, - }} - request.Options.Fields = append(request.Options.Fields, req...) - } - if query.ProvisionedPath != "" { - req := []*resource.Requirement{{ - Key: resource.SEARCH_FIELD_REPOSITORY_PATH, + if query.ManagerIdentity != "" { + request.Options.Fields = append(request.Options.Fields, &resource.Requirement{ + Key: resource.SEARCH_FIELD_MANAGER_ID, Operator: string(selection.In), - Values: []string{query.ProvisionedPath}, - }} - request.Options.Fields = append(request.Options.Fields, req...) + Values: []string{query.ManagerIdentity}, + }) + } + + if len(query.ManagerIdentityNotIn) > 0 { + request.Options.Fields = append(request.Options.Fields, &resource.Requirement{ + Key: resource.SEARCH_FIELD_MANAGER_ID, + Operator: string(selection.NotIn), + Values: query.ManagerIdentityNotIn, + }) + } + if query.SourcePath != "" { + request.Options.Fields = append(request.Options.Fields, &resource.Requirement{ + Key: resource.SEARCH_FIELD_SOURCE_PATH, + Operator: string(selection.In), + Values: []string{query.SourcePath}, + }) } if query.Title != "" { @@ -1836,18 +1846,6 @@ func (dr *DashboardServiceImpl) searchProvisionedDashboardsThroughK8s(ctx contex ctx, _ = identity.WithServiceIdentity(ctx, query.OrgId) - if query.ProvisionedRepo != "" { - query.ProvisionedRepo = dashboard.ProvisionedFileNameWithPrefix(query.ProvisionedRepo) - } - - if len(query.ProvisionedReposNotIn) > 0 { - repos := make([]string, len(query.ProvisionedReposNotIn)) - for i, v := range query.ProvisionedReposNotIn { - repos[i] = dashboard.ProvisionedFileNameWithPrefix(v) - } - query.ProvisionedReposNotIn = repos - } - query.Type = searchstore.TypeDashboard searchResults, err := dr.searchDashboardsThroughK8sRaw(ctx, query) @@ -1874,26 +1872,27 @@ func (dr *DashboardServiceImpl) searchProvisionedDashboardsThroughK8s(ctx contex return err } - // ensure the repo is set due to file provisioning, otherwise skip it - fileRepo, found := dashboard.GetProvisionedFileNameFromMeta(meta.GetRepositoryName()) - if !found { + m, ok := meta.GetManagerProperties() + if !ok || m.Kind != utils.ManagerKindClassicFP { // nolint:staticcheck + return nil + } + + source, ok := meta.GetSourceProperties() + if !ok { return nil } provisioning := &dashboardProvisioningWithUID{ + DashboardProvisioning: dashboards.DashboardProvisioning{ + Name: m.Identity, + ExternalID: source.Path, + CheckSum: source.Checksum, + DashboardID: meta.GetDeprecatedInternalID(), // nolint:staticcheck + }, DashboardUID: hit.Name, } - provisioning.Name = fileRepo - provisioning.ExternalID = meta.GetRepositoryPath() - provisioning.CheckSum = meta.GetRepositoryHash() - provisioning.DashboardID = meta.GetDeprecatedInternalID() // nolint:staticcheck - - updated, err := meta.GetRepositoryTimestamp() - if err != nil { - return err - } - if updated != nil { - provisioning.Updated = updated.Unix() + if source.TimestampMillis > 0 { + provisioning.Updated = time.UnixMilli(source.TimestampMillis).Unix() } mu.Lock() @@ -2023,13 +2022,13 @@ func (dr *DashboardServiceImpl) UnstructuredToLegacyDashboard(ctx context.Contex return &out, nil } -func LegacySaveCommandToUnstructured(cmd *dashboards.SaveDashboardCommand, namespace string) (unstructured.Unstructured, error) { +func LegacySaveCommandToUnstructured(cmd *dashboards.SaveDashboardCommand, namespace string) (*unstructured.Unstructured, error) { uid := cmd.GetDashboardModel().UID if uid == "" { uid = uuid.NewString() } - finalObj := unstructured.Unstructured{ + finalObj := &unstructured.Unstructured{ Object: map[string]interface{}{}, } @@ -2057,7 +2056,7 @@ func LegacySaveCommandToUnstructured(cmd *dashboards.SaveDashboardCommand, names finalObj.SetNamespace(namespace) finalObj.SetGroupVersionKind(dashboardv0alpha1.DashboardResourceInfo.GroupVersionKind()) - meta, err := utils.MetaAccessor(&finalObj) + meta, err := utils.MetaAccessor(finalObj) if err != nil { return finalObj, err } diff --git a/pkg/services/dashboards/service/dashboard_service_test.go b/pkg/services/dashboards/service/dashboard_service_test.go index 32656bf0d19..85edc00433c 100644 --- a/pkg/services/dashboards/service/dashboard_service_test.go +++ b/pkg/services/dashboards/service/dashboard_service_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "fmt" "reflect" "testing" "time" @@ -9,13 +10,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" - "github.com/grafana/grafana/pkg/apis/dashboard" + dashboardv0alpha1 "github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/apiserver/client" @@ -540,31 +540,38 @@ func TestGetProvisionedDashboardData(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled and get from relevant org", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) + provisioningTimestamp := int64(1234567) k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ - "metadata": map[string]any{ - "name": "uid", - "labels": map[string]any{ - utils.LabelKeyDeprecatedInternalID: "1", // nolint:staticcheck + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": dashboardv0alpha1.DashboardResourceInfo.GroupVersion().String(), + "kind": dashboardv0alpha1.DashboardResourceInfo.GroupVersionKind().Kind, + "metadata": map[string]interface{}{ + "name": "uid", + "labels": map[string]interface{}{ + utils.LabelKeyDeprecatedInternalID: "1", // nolint:staticcheck + }, + "annotations": map[string]interface{}{ + utils.AnnoKeyManagerKind: string(utils.ManagerKindClassicFP), // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "test", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: fmt.Sprintf("%d", time.Unix(provisioningTimestamp, 0).UnixMilli()), + }, }, - "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.ProvisionedFileNameWithPrefix("test"), - utils.AnnoKeyRepoHash: "hash", - utils.AnnoKeyRepoPath: "path/to/file", - utils.AnnoKeyRepoTimestamp: "2025-01-01T00:00:00Z", + "spec": map[string]interface{}{ + "test": "test", + "version": int64(1), + "title": "testing slugify", }, }, - "spec": map[string]any{ - "test": "test", - "version": int64(1), - "title": "testing slugify", - }, - }}, nil).Once() + }, nil).Once() repo := "test" k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { - // ensure the prefix is added to the query - return req.Options.Fields[0].Values[0] == dashboard.ProvisionedFileNameWithPrefix(repo) + // make sure the kind is added to the query + return req.Options.Fields[0].Values[0] == string(utils.ManagerKindClassicFP) && // nolint:staticcheck + req.Options.Fields[1].Values[0] == repo })).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{}, @@ -573,8 +580,9 @@ func TestGetProvisionedDashboardData(t *testing.T) { TotalHits: 0, }, nil).Once() k8sCliMock.On("Search", mock.Anything, int64(2), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { - // ensure the prefix is added to the query - return req.Options.Fields[0].Values[0] == dashboard.ProvisionedFileNameWithPrefix(repo) + // make sure the kind is added to the query + return req.Options.Fields[0].Values[0] == string(utils.ManagerKindClassicFP) && // nolint:staticcheck + req.Options.Fields[1].Values[0] == repo })).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -611,7 +619,7 @@ func TestGetProvisionedDashboardData(t *testing.T) { Name: "test", ExternalID: "path/to/file", CheckSum: "hash", - Updated: 1735689600, + Updated: provisioningTimestamp, }) k8sCliMock.AssertExpectations(t) }) @@ -639,21 +647,25 @@ func TestGetProvisionedDashboardDataByDashboardID(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled and get from whatever org it is in", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) + provisioningTimestamp := int64(1234567) k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ - "metadata": map[string]any{ + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": dashboardv0alpha1.DashboardResourceInfo.GroupVersion().String(), + "kind": dashboardv0alpha1.DashboardResourceInfo.GroupVersionKind().Kind, + "metadata": map[string]interface{}{ "name": "uid", - "labels": map[string]any{ + "labels": map[string]interface{}{ utils.LabelKeyDeprecatedInternalID: "1", // nolint:staticcheck }, - "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.ProvisionedFileNameWithPrefix("test"), - utils.AnnoKeyRepoHash: "hash", - utils.AnnoKeyRepoPath: "path/to/file", - utils.AnnoKeyRepoTimestamp: "2025-01-01T00:00:00Z", + "annotations": map[string]interface{}{ + utils.AnnoKeyManagerKind: string(utils.ManagerKindClassicFP), // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "test", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: fmt.Sprintf("%d", time.Unix(provisioningTimestamp, 0).UnixMilli()), }, }, - "spec": map[string]any{ + "spec": map[string]interface{}{ "test": "test", "version": int64(1), "title": "testing slugify", @@ -701,7 +713,7 @@ func TestGetProvisionedDashboardDataByDashboardID(t *testing.T) { Name: "test", ExternalID: "path/to/file", CheckSum: "hash", - Updated: 1735689600, + Updated: provisioningTimestamp, }) k8sCliMock.AssertExpectations(t) }) @@ -729,21 +741,25 @@ func TestGetProvisionedDashboardDataByDashboardUID(t *testing.T) { t.Run("Should use Kubernetes client if feature flags are enabled", func(t *testing.T) { ctx, k8sCliMock := setupK8sDashboardTests(service) + provisioningTimestamp := int64(1234567) k8sCliMock.On("GetNamespace", mock.Anything, mock.Anything).Return("default") - k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]any{ - "metadata": map[string]any{ + k8sCliMock.On("Get", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": dashboardv0alpha1.DashboardResourceInfo.GroupVersion().String(), + "kind": dashboardv0alpha1.DashboardResourceInfo.GroupVersionKind().Kind, + "metadata": map[string]interface{}{ "name": "uid", - "labels": map[string]any{ + "labels": map[string]interface{}{ utils.LabelKeyDeprecatedInternalID: "1", // nolint:staticcheck }, - "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.ProvisionedFileNameWithPrefix("test"), - utils.AnnoKeyRepoHash: "hash", - utils.AnnoKeyRepoPath: "path/to/file", - utils.AnnoKeyRepoTimestamp: "2025-01-01T00:00:00Z", + "annotations": map[string]interface{}{ + utils.AnnoKeyManagerKind: string(utils.ManagerKindClassicFP), // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "test", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: fmt.Sprintf("%d", time.Unix(provisioningTimestamp, 0).UnixMilli()), }, }, - "spec": map[string]any{ + "spec": map[string]interface{}{ "test": "test", "version": int64(1), "title": "testing slugify", @@ -784,7 +800,7 @@ func TestGetProvisionedDashboardDataByDashboardUID(t *testing.T) { Name: "test", ExternalID: "path/to/file", CheckSum: "hash", - Updated: 1735689600, + Updated: provisioningTimestamp, }) k8sCliMock.AssertExpectations(t) }) @@ -826,10 +842,11 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { "metadata": map[string]any{ "name": "uid", "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.ProvisionedFileNameWithPrefix("orphaned"), - utils.AnnoKeyRepoHash: "hash", - utils.AnnoKeyRepoPath: "path/to/file", - utils.AnnoKeyRepoTimestamp: "2025-01-01T00:00:00Z", + utils.AnnoKeyManagerKind: string(utils.ManagerKindClassicFP), // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "orphaned", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: "2025-01-01T00:00:00Z", }, }, "spec": map[string]any{}, @@ -839,8 +856,8 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { "metadata": map[string]any{ "name": "uid2", "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.PluginIDRepoName, - utils.AnnoKeyRepoHash: "app", + utils.AnnoKeyManagerKind: string(utils.ManagerKindPlugin), + utils.AnnoKeyManagerIdentity: "app", }, }, "spec": map[string]any{}, @@ -850,16 +867,17 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { "metadata": map[string]any{ "name": "uid3", "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.ProvisionedFileNameWithPrefix("orphaned"), - utils.AnnoKeyRepoHash: "hash", - utils.AnnoKeyRepoPath: "path/to/file", - utils.AnnoKeyRepoTimestamp: "2025-01-01T00:00:00Z", + utils.AnnoKeyManagerKind: string(utils.ManagerKindClassicFP), // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "orphaned", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: "2025-01-01T00:00:00Z", }, }, "spec": map[string]any{}, }}, nil).Once() k8sCliMock.On("Search", mock.Anything, int64(1), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { - return req.Options.Fields[0].Key == "repo.name" && req.Options.Fields[0].Values[0] == dashboard.ProvisionedFileNameWithPrefix("test") && req.Options.Fields[0].Operator == "notin" + return req.Options.Fields[0].Key == "manager.id" && req.Options.Fields[0].Values[0] == "test" && req.Options.Fields[0].Operator == "notin" })).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -889,7 +907,7 @@ func TestDeleteOrphanedProvisionedDashboards(t *testing.T) { }, nil).Once() k8sCliMock.On("Search", mock.Anything, int64(2), mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { - return req.Options.Fields[0].Key == "repo.name" && req.Options.Fields[0].Values[0] == dashboard.ProvisionedFileNameWithPrefix("test") && req.Options.Fields[0].Operator == "notin" + return req.Options.Fields[0].Key == "manager.id" && req.Options.Fields[0].Values[0] == "test" && req.Options.Fields[0].Operator == "notin" })).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -960,10 +978,11 @@ func TestUnprovisionDashboard(t *testing.T) { "metadata": map[string]any{ "name": "uid", "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.ProvisionedFileNameWithPrefix("test"), - utils.AnnoKeyRepoHash: "hash", - utils.AnnoKeyRepoPath: "path/to/file", - utils.AnnoKeyRepoTimestamp: "2025-01-01T00:00:00Z", + utils.AnnoKeyManagerKind: utils.ManagerKindClassicFP, // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "test", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: "2025-01-01T00:00:00Z", }, }, "spec": map[string]any{}, @@ -983,7 +1002,7 @@ func TestUnprovisionDashboard(t *testing.T) { }, }} // should update it to be without annotations - k8sCliMock.On("Update", mock.Anything, dashWithoutAnnotations, mock.Anything, mock.Anything).Return(dashWithoutAnnotations, nil) + k8sCliMock.On("Update", mock.Anything, dashWithoutAnnotations, mock.Anything).Return(dashWithoutAnnotations, nil) k8sCliMock.On("GetNamespace", mock.Anything).Return("default") k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.Anything).Return(&resource.ResourceSearchResponse{ @@ -1054,8 +1073,9 @@ func TestGetDashboardsByPluginID(t *testing.T) { k8sCliMock.On("Get", mock.Anything, "uid", mock.Anything, mock.Anything, mock.Anything).Return(uidUnstructured, nil) k8sCliMock.On("GetUserFromMeta", mock.Anything, mock.Anything).Return(&user.User{}, nil) k8sCliMock.On("Search", mock.Anything, mock.Anything, mock.MatchedBy(func(req *resource.ResourceSearchRequest) bool { - return req.Options.Fields[0].Key == "repo.name" && req.Options.Fields[0].Values[0] == dashboard.PluginIDRepoName && - req.Options.Fields[1].Key == "repo.path" && req.Options.Fields[1].Values[0] == "testing" + return ( // gofmt comment helper + req.Options.Fields[0].Key == "manager.kind" && req.Options.Fields[0].Values[0] == string(utils.ManagerKindPlugin) && + req.Options.Fields[1].Key == "manager.id" && req.Options.Fields[1].Values[0] == "testing") })).Return(&resource.ResourceSearchResponse{ Results: &resource.ResourceTable{ Columns: []*resource.ResourceTableColumnDefinition{ @@ -1989,14 +2009,16 @@ func TestSearchProvisionedDashboardsThroughK8sRaw(t *testing.T) { query := &dashboards.FindPersistedDashboardsQuery{ OrgId: 1, } + provisioningTimestamp := int64(1234567) dashboardUnstructuredProvisioned := unstructured.Unstructured{Object: map[string]any{ "metadata": map[string]any{ "name": "uid", "annotations": map[string]any{ - utils.AnnoKeyRepoName: dashboard.ProvisionedFileNameWithPrefix("test"), - utils.AnnoKeyRepoHash: "hash", - utils.AnnoKeyRepoPath: "path/to/file", - utils.AnnoKeyRepoTimestamp: "2025-01-01T00:00:00Z", + utils.AnnoKeyManagerKind: string(utils.ManagerKindClassicFP), // nolint:staticcheck + utils.AnnoKeyManagerIdentity: "test", + utils.AnnoKeySourceChecksum: "hash", + utils.AnnoKeySourcePath: "path/to/file", + utils.AnnoKeySourceTimestamp: fmt.Sprintf("%d", time.Unix(provisioningTimestamp, 0).UnixMilli()), }, }, "spec": map[string]any{}, @@ -2056,7 +2078,7 @@ func TestSearchProvisionedDashboardsThroughK8sRaw(t *testing.T) { Name: "test", ExternalID: "path/to/file", CheckSum: "hash", - Updated: 1735689600, + Updated: provisioningTimestamp, }, }, }, res) // only should return the one provisioned dashboard diff --git a/pkg/services/dashboards/service/search/search.go b/pkg/services/dashboards/service/search/search.go index c0f3a5598f5..1352ec52776 100644 --- a/pkg/services/dashboards/service/search/search.go +++ b/pkg/services/dashboards/service/search/search.go @@ -29,10 +29,11 @@ var ( resource.SEARCH_FIELD_CREATED_BY, resource.SEARCH_FIELD_UPDATED, resource.SEARCH_FIELD_UPDATED_BY, - resource.SEARCH_FIELD_REPOSITORY_NAME, - resource.SEARCH_FIELD_REPOSITORY_PATH, - resource.SEARCH_FIELD_REPOSITORY_HASH, - resource.SEARCH_FIELD_REPOSITORY_TIME, + resource.SEARCH_FIELD_MANAGER_KIND, + resource.SEARCH_FIELD_MANAGER_ID, + resource.SEARCH_FIELD_SOURCE_PATH, + resource.SEARCH_FIELD_SOURCE_CHECKSUM, + resource.SEARCH_FIELD_SOURCE_TIME, } ) diff --git a/pkg/services/featuremgmt/codeowners.go b/pkg/services/featuremgmt/codeowners.go index 13520e2a02b..a201b5e4b23 100644 --- a/pkg/services/featuremgmt/codeowners.go +++ b/pkg/services/featuremgmt/codeowners.go @@ -13,6 +13,7 @@ const ( grafanaBackendServicesSquad codeowner = "@grafana/grafana-backend-services-squad" grafanaSearchAndStorageSquad codeowner = "@grafana/search-and-storage" grafanaPluginsPlatformSquad codeowner = "@grafana/plugins-platform-backend" + grafanaFrontendOpsWG codeowner = "@grafana/frontend-ops" grafanaAsCodeSquad codeowner = "@grafana/grafana-as-code" identityAccessTeam codeowner = "@grafana/identity-access-team" grafanaObservabilityLogsSquad codeowner = "@grafana/observability-logs" diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index f3b1da31b5c..27da9533003 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1775,6 +1775,13 @@ var ( HideFromDocs: true, FrontendOnly: true, }, + { + Name: "assetSriChecks", + Description: "Enables SRI checks for Grafana JavaScript assets", + Stage: FeatureStageExperimental, + Owner: grafanaFrontendOpsWG, + FrontendOnly: true, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index fc0b0c8dfb7..ace09d69f56 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -235,3 +235,4 @@ alertingJiraIntegration,experimental,@grafana/alerting-squad,false,false,true alertingRuleVersionHistoryRestore,GA,@grafana/alerting-squad,false,false,true newShareReportDrawer,experimental,@grafana/sharing-squad,false,false,false rendererDisableAppPluginsPreload,experimental,@grafana/sharing-squad,false,false,true +assetSriChecks,experimental,@grafana/frontend-ops,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 56c3a7d53da..d651c7290f6 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -950,4 +950,8 @@ const ( // FlagRendererDisableAppPluginsPreload // Disable pre-loading app plugins when the request is coming from the renderer FlagRendererDisableAppPluginsPreload = "rendererDisableAppPluginsPreload" + + // FlagAssetSriChecks + // Enables SRI checks for Grafana JavaScript assets + FlagAssetSriChecks = "assetSriChecks" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 3f5045123d9..b093cb5dd21 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -598,6 +598,22 @@ "codeowner": "@grafana/grafana-frontend-platform" } }, + { + "metadata": { + "name": "assetSriChecks", + "resourceVersion": "1739984409734", + "creationTimestamp": "2025-02-19T15:56:59Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-02-19 17:00:09.734088 +0000 UTC" + } + }, + "spec": { + "description": "Enables SRI checks for Grafana JavaScript assets", + "stage": "experimental", + "codeowner": "@grafana/frontend-ops", + "frontend": true + } + }, { "metadata": { "name": "authAPIAccessTokenAuth", diff --git a/pkg/services/folder/folderimpl/conversions.go b/pkg/services/folder/folderimpl/conversions.go index 29ef06d14fc..86c749556be 100644 --- a/pkg/services/folder/folderimpl/conversions.go +++ b/pkg/services/folder/folderimpl/conversions.go @@ -6,13 +6,14 @@ import ( "strconv" "strings" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/user" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) func (ss *FolderUnifiedStoreImpl) UnstructuredToLegacyFolder(ctx context.Context, item *unstructured.Unstructured) (*folder.Folder, error) { @@ -57,7 +58,7 @@ func (ss *FolderUnifiedStoreImpl) UnstructuredToLegacyFolder(ctx context.Context if updater.UID == "" { updater = creator } - + manager, _ := meta.GetManagerProperties() return &folder.Folder{ UID: uid, Title: title, @@ -65,7 +66,7 @@ func (ss *FolderUnifiedStoreImpl) UnstructuredToLegacyFolder(ctx context.Context ID: meta.GetDeprecatedInternalID(), // nolint:staticcheck ParentUID: meta.GetFolder(), Version: int(meta.GetGeneration()), - Repository: meta.GetRepositoryName(), + ManagedBy: manager.Kind, URL: url, Created: created, diff --git a/pkg/services/folder/folderimpl/conversions_test.go b/pkg/services/folder/folderimpl/conversions_test.go index a6120c7c23d..537193d5a15 100644 --- a/pkg/services/folder/folderimpl/conversions_test.go +++ b/pkg/services/folder/folderimpl/conversions_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" @@ -63,7 +64,7 @@ func TestFolderConversions(t *testing.T) { Title: "test folder", Description: "Something set in the file", URL: "/dashboards/f/be79sztagf20wd/test-folder", - Repository: "example-repo", + ManagedBy: utils.ManagerKindRepo, Created: created, Updated: created.Add(time.Hour * 5), CreatedBy: 10, diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index e257c7d8761..13a9943b7b2 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -39,6 +39,10 @@ func (ss *FolderStoreImpl) Create(ctx context.Context, cmd folder.CreateFolderCo return nil, folder.ErrBadRequest.Errorf("missing UID") } + if cmd.UID == cmd.ParentUID { + return nil, folder.ErrFolderCannotBeParentOfItself + } + var foldr *folder.Folder /* version := 1 diff --git a/pkg/services/folder/folderimpl/sqlstore_test.go b/pkg/services/folder/folderimpl/sqlstore_test.go index e76607cb401..aff18e4e3d7 100644 --- a/pkg/services/folder/folderimpl/sqlstore_test.go +++ b/pkg/services/folder/folderimpl/sqlstore_test.go @@ -60,6 +60,18 @@ func TestIntegrationCreate(t *testing.T) { require.Error(t, err) }) + t.Run("creating a folder with itself as a parent should fail", func(t *testing.T) { + uid := util.GenerateShortUID() + _, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ + Title: folderTitle, + OrgID: orgID, + ParentUID: uid, + Description: folderDsc, + UID: uid, + }) + require.ErrorIs(t, err, folder.ErrFolderCannotBeParentOfItself) + }) + t.Run("creating a folder without providing a parent should default to the empty parent folder", func(t *testing.T) { uid := util.GenerateShortUID() f, err := folderStore.Create(context.Background(), folder.CreateFolderCommand{ diff --git a/pkg/services/folder/folderimpl/unifiedstore.go b/pkg/services/folder/folderimpl/unifiedstore.go index b1615eb4b1e..0d64cdcf2f3 100644 --- a/pkg/services/folder/folderimpl/unifiedstore.go +++ b/pkg/services/folder/folderimpl/unifiedstore.go @@ -77,6 +77,10 @@ func (ss *FolderUnifiedStoreImpl) Delete(ctx context.Context, UIDs []string, org func (ss *FolderUnifiedStoreImpl) Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) { obj, err := ss.k8sclient.Get(ctx, cmd.UID, cmd.OrgID, v1.GetOptions{}) if err != nil { + if apierrors.IsNotFound(err) { + return nil, dashboards.ErrFolderNotFound + } + return nil, err } updated := obj.DeepCopy() @@ -144,7 +148,7 @@ func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetPa parentUid := q.UID for parentUid != "" { - out, err := ss.k8sclient.Get(ctx, parentUid, q.OrgID, v1.GetOptions{}) + folder, err := ss.Get(ctx, folder.GetFolderQuery{UID: &parentUid, OrgID: q.OrgID}) if err != nil { var statusError *apierrors.StatusError if errors.As(err, &statusError) && statusError.ErrStatus.Code == http.StatusForbidden { @@ -155,11 +159,6 @@ func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetPa return nil, err } - folder, err := ss.UnstructuredToLegacyFolder(ctx, out) - if err != nil { - return nil, err - } - parentUid = folder.ParentUID hits = append(hits, folder) } @@ -183,6 +182,15 @@ func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetC q.Page = 1 } + if q.UID != "" { + // the original get children query fails if the parent folder does not exist + // check that the parent exists first + _, err := ss.Get(ctx, folder.GetFolderQuery{UID: &q.UID, OrgID: q.OrgID}) + if err != nil { + return nil, err + } + } + req := &resource.ResourceSearchRequest{ Options: &resource.ListOptions{ Fields: []*resource.Requirement{ @@ -229,12 +237,7 @@ func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetC } // search only returns a subset of info, get all info of the folder - item, err := ss.k8sclient.Get(ctx, item.Name, q.OrgID, v1.GetOptions{}) - if err != nil { - return nil, err - } - - f, err := ss.UnstructuredToLegacyFolder(ctx, item) + f, err := ss.Get(ctx, folder.GetFolderQuery{UID: &item.Name, OrgID: q.OrgID}) if err != nil { return nil, err } @@ -411,6 +414,10 @@ func getDescendants(nodes map[string]*folder.Folder, tree map[string]map[string] func (ss *FolderUnifiedStoreImpl) CountFolderContent(ctx context.Context, orgID int64, ancestor_uid string) (folder.DescendantCounts, error) { counts, err := ss.k8sclient.Get(ctx, ancestor_uid, orgID, v1.GetOptions{}, "counts") if err != nil { + if apierrors.IsNotFound(err) { + return nil, dashboards.ErrFolderNotFound + } + return nil, err } diff --git a/pkg/services/folder/folderimpl/unifiedstore_test.go b/pkg/services/folder/folderimpl/unifiedstore_test.go index f056ae96ba5..6c51829f671 100644 --- a/pkg/services/folder/folderimpl/unifiedstore_test.go +++ b/pkg/services/folder/folderimpl/unifiedstore_test.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/apiserver/client" + "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/stretchr/testify/mock" @@ -16,6 +17,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/selection" ) @@ -170,6 +172,16 @@ func TestGetParents(t *testing.T) { require.Len(t, result, 1) require.Equal(t, "parenttwo", result[0].UID) }) + t.Run("should stop if parent folder is not found", func(t *testing.T) { + mockCli.On("Get", mock.Anything, "parentone", orgID, mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "parentone")).Once() + + _, err := store.GetParents(ctx, folder.GetParentsQuery{ + UID: "parentone", + OrgID: orgID, + }) + + require.ErrorIs(t, err, dashboards.ErrFolderNotFound) + }) } func TestGetChildren(t *testing.T) { @@ -213,6 +225,11 @@ func TestGetChildren(t *testing.T) { }, TotalHits: 1, }, nil).Once() + mockCli.On("Get", mock.Anything, "folder1", orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{"name": "folder1"}, + }, + }, nil).Once() mockCli.On("Get", mock.Anything, "folder2", orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ Object: map[string]interface{}{ "metadata": map[string]interface{}{"name": "folder2"}, @@ -235,6 +252,47 @@ func TestGetChildren(t *testing.T) { require.Equal(t, "folder3", result[1].UID) }) + t.Run("should return an error if the folder is not found", func(t *testing.T) { + mockCli.On("Search", mock.Anything, orgID, &resource.ResourceSearchRequest{ + Options: &resource.ListOptions{ + Fields: []*resource.Requirement{ + { + Key: resource.SEARCH_FIELD_FOLDER, + Operator: string(selection.In), + Values: []string{"folder1"}, + }, + }, + }, + Limit: folderSearchLimit, // should default to folderSearchLimit + Offset: 0, // should be set as limit * (page - 1) + Page: 1, // should be set to 1 by default + }).Return(&resource.ResourceSearchResponse{ + Results: &resource.ResourceTable{ + Columns: []*resource.ResourceTableColumnDefinition{ + {Name: "folder", Type: resource.ResourceTableColumnDefinition_STRING}, + }, + Rows: []*resource.ResourceTableRow{ + { + Key: &resource.ResourceKey{Name: "folder2", Resource: "folder"}, + Cells: [][]byte{[]byte("folder1")}, + }, + { + Key: &resource.ResourceKey{Name: "folder3", Resource: "folder"}, + Cells: [][]byte{[]byte("folder1")}, + }, + }, + }, + TotalHits: 1, + }, nil).Once() + mockCli.On("Get", mock.Anything, "folder1", orgID, mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "folder1")).Once() + + _, err := store.GetChildren(ctx, folder.GetChildrenQuery{ + UID: "folder1", + OrgID: orgID, + }) + require.ErrorIs(t, err, dashboards.ErrFolderNotFound) + }) + t.Run("pages should be able to be set, general folder should be turned to empty string, and folder uids should be passed in", func(t *testing.T) { mockCli.On("Search", mock.Anything, orgID, &resource.ResourceSearchRequest{ Options: &resource.ListOptions{ @@ -301,6 +359,11 @@ func TestGetChildren(t *testing.T) { }, TotalHits: 1, }, nil) + mockCli.On("Get", mock.Anything, "folder", orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{"name": "folder"}, + }, + }, nil) mockCli.On("Get", mock.Anything, accesscontrol.K6FolderUID, orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{ Object: map[string]interface{}{ "metadata": map[string]interface{}{"name": accesscontrol.K6FolderUID}, diff --git a/pkg/services/folder/model.go b/pkg/services/folder/model.go index 769ee259f3f..ffd947c6737 100644 --- a/pkg/services/folder/model.go +++ b/pkg/services/folder/model.go @@ -1,11 +1,13 @@ package folder import ( + "errors" "fmt" "time" "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" @@ -20,6 +22,7 @@ var ErrInternal = errutil.Internal("folder.internal") var ErrCircularReference = errutil.BadRequest("folder.circular-reference", errutil.WithPublicMessage("Circular reference detected")) var ErrTargetRegistrySrvConflict = errutil.Internal("folder.target-registry-srv-conflict") var ErrFolderNotEmpty = errutil.BadRequest("folder.not-empty", errutil.WithPublicMessage("Folder cannot be deleted: folder is not empty")) +var ErrFolderCannotBeParentOfItself = errors.New("folder cannot be parent of itself") const ( GeneralFolderUID = "general" @@ -54,10 +57,10 @@ type Folder struct { Fullpath string `xorm:"fullpath"` FullpathUIDs string `xorm:"fullpath_uids"` - // When the folder belongs to a repository + // The folder is managed by an external process // NOTE: this is only populated when folders are managed by unified storage // This is not ever used by xorm, but the translation functions flow through this type - Repository string `json:"repository,omitempty"` + ManagedBy utils.ManagerKind `json:"managedBy,omitempty"` } var GeneralFolder = Folder{ID: 0, Title: "General"} diff --git a/pkg/services/ngalert/api/api_convert_prometheus_test.go b/pkg/services/ngalert/api/api_convert_prometheus_test.go index 945a0558365..5fd920438d0 100644 --- a/pkg/services/ngalert/api/api_convert_prometheus_test.go +++ b/pkg/services/ngalert/api/api_convert_prometheus_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" @@ -22,6 +23,7 @@ import ( apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/provisioning" + "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/services/ngalert/tests/fakes" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" @@ -151,6 +153,32 @@ func TestRouteConvertPrometheusPostRuleGroup(t *testing.T) { require.NotNil(t, remaining) }) + t.Run("with no access to the datasource should return 403", func(t *testing.T) { + acFake := &acfakes.FakeRuleService{} + srv, _, _, _ := createConvertPrometheusSrv(t, withFakeAccessControlRuleService(acFake)) + + acFake.AuthorizeRuleChangesFunc = func(context.Context, identity.Requester, *store.GroupDelta) error { + return datasources.ErrDataSourceAccessDenied + } + + rc := createRequestCtx() + response := srv.RouteConvertPrometheusPostRuleGroup(rc, "folder", simpleGroup) + require.Equal(t, http.StatusForbidden, response.Status()) + require.Contains(t, string(response.Body()), "data source access denied") + }) + + t.Run("when alert rule quota limit exceeded", func(t *testing.T) { + quotas := &provisioning.MockQuotaChecker{} + quotas.EXPECT().LimitExceeded() + + srv, _, _, _ := createConvertPrometheusSrv(t, withQuotaChecker(quotas)) + + rc := createRequestCtx() + response := srv.RouteConvertPrometheusPostRuleGroup(rc, "folder", simpleGroup) + require.Equal(t, http.StatusForbidden, response.Status()) + require.Contains(t, string(response.Body()), "quota has been exceeded") + }) + t.Run("with valid pause header values should return 202", func(t *testing.T) { testCases := []struct { name string @@ -749,7 +777,9 @@ func TestRouteConvertPrometheusDeleteRuleGroup(t *testing.T) { } type convertPrometheusSrvOptions struct { - provenanceStore provisioning.ProvisioningStore + provenanceStore provisioning.ProvisioningStore + fakeAccessControlRuleService *acfakes.FakeRuleService + quotaChecker *provisioning.MockQuotaChecker } type convertPrometheusSrvOptionsFunc func(*convertPrometheusSrvOptions) @@ -760,11 +790,29 @@ func withProvenanceStore(store provisioning.ProvisioningStore) convertPrometheus } } +func withFakeAccessControlRuleService(service *acfakes.FakeRuleService) convertPrometheusSrvOptionsFunc { + return func(opts *convertPrometheusSrvOptions) { + opts.fakeAccessControlRuleService = service + } +} + +func withQuotaChecker(checker *provisioning.MockQuotaChecker) convertPrometheusSrvOptionsFunc { + return func(opts *convertPrometheusSrvOptions) { + opts.quotaChecker = checker + } +} + func createConvertPrometheusSrv(t *testing.T, opts ...convertPrometheusSrvOptionsFunc) (*ConvertPrometheusSrv, datasources.CacheService, *fakes.RuleStore, *foldertest.FakeService) { t.Helper() + // By default the quota checker will allow the operation + quotas := &provisioning.MockQuotaChecker{} + quotas.EXPECT().LimitOK() + options := convertPrometheusSrvOptions{ - provenanceStore: fakes.NewFakeProvisioningStore(), + provenanceStore: fakes.NewFakeProvisioningStore(), + fakeAccessControlRuleService: &acfakes.FakeRuleService{}, + quotaChecker: quotas, } for _, opt := range opts { @@ -782,23 +830,20 @@ func createConvertPrometheusSrv(t *testing.T, opts ...convertPrometheusSrvOption } dsCache.DataSources = append(dsCache.DataSources, ds) - quotas := &provisioning.MockQuotaChecker{} - quotas.EXPECT().LimitOK() - folderService := foldertest.NewFakeService() alertRuleService := provisioning.NewAlertRuleService( ruleStore, options.provenanceStore, folderService, - quotas, + options.quotaChecker, &provisioning.NopTransactionManager{}, 60, 10, 100, log.New("test"), &provisioning.NotificationSettingsValidatorProviderFake{}, - &acfakes.FakeRuleService{}, + options.fakeAccessControlRuleService, ) cfg := &setting.UnifiedAlertingSettings{ diff --git a/pkg/services/ngalert/api/errors.go b/pkg/services/ngalert/api/errors.go index a66db1e4fb2..138eb6a3cd1 100644 --- a/pkg/services/ngalert/api/errors.go +++ b/pkg/services/ngalert/api/errors.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/services/datasources" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" + "github.com/grafana/grafana/pkg/services/ngalert/models" ) var ( @@ -42,5 +43,11 @@ func errorToResponse(err error) response.Response { if errors.Is(err, errFolderAccess) { return toNamespaceErrorResponse(err) } + if errors.Is(err, datasources.ErrDataSourceAccessDenied) { + return ErrResp(http.StatusForbidden, err, "") + } + if errors.Is(err, models.ErrQuotaReached) { + return ErrResp(http.StatusForbidden, err, "") + } return ErrResp(http.StatusInternalServerError, err, "") } diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index 54694664cca..9c79b624179 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -832,9 +832,9 @@ func (a *State) transition(alertRule *models.AlertRule, result eval.Result, extr } func resultStateReason(result eval.Result, rule *models.AlertRule) string { - if rule.ExecErrState == models.KeepLastErrState || rule.NoDataState == models.KeepLast { + if result.State == eval.Error && rule.ExecErrState == models.KeepLastErrState || + result.State == eval.NoData && rule.NoDataState == models.KeepLast { return models.ConcatReasons(result.State.String(), models.StateReasonKeepLast) } - return result.State.String() } diff --git a/pkg/services/ngalert/state/state_test.go b/pkg/services/ngalert/state/state_test.go index 7216784a65c..8200696d385 100644 --- a/pkg/services/ngalert/state/state_test.go +++ b/pkg/services/ngalert/state/state_test.go @@ -1132,3 +1132,61 @@ func TestPatch(t *testing.T) { assert.EqualValues(t, orig.Annotations, state.Annotations) }) } + +func TestResultStateReason(t *testing.T) { + gen := ngmodels.RuleGen + tests := []struct { + name string + result eval.Result + rule *ngmodels.AlertRule + expected string + }{ + { + name: "Error state with KeepLast", + result: eval.Result{ + State: eval.Error, + }, + rule: gen.With(ngmodels.RuleMuts.WithErrorExecAs(ngmodels.KeepLastErrState)).GenerateRef(), + expected: "Error, KeepLast", + }, + { + name: "Error state without KeepLast", + result: eval.Result{ + State: eval.Error, + }, + rule: gen.With(ngmodels.RuleMuts.WithErrorExecAs(ngmodels.ErrorErrState)).GenerateRef(), + expected: "Error", + }, + { + name: "NoData state with KeepLast state", + result: eval.Result{ + State: eval.NoData, + }, + rule: gen.With(ngmodels.RuleMuts.WithNoDataExecAs(ngmodels.KeepLast)).GenerateRef(), + expected: "NoData, KeepLast", + }, + { + name: "NoData state without KeepLast", + result: eval.Result{ + State: eval.NoData, + }, + rule: gen.With(ngmodels.RuleMuts.WithNoDataExecAs(ngmodels.NoData)).GenerateRef(), + expected: "NoData", + }, + { + name: "Normal state", + result: eval.Result{ + State: eval.NoData, + }, + rule: gen.With(ngmodels.RuleMuts.WithErrorExecAs(ngmodels.ErrorErrState), ngmodels.RuleMuts.WithNoDataExecAs(ngmodels.NoData)).GenerateRef(), + expected: "NoData", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := resultStateReason(tc.result, tc.rule) + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/pkg/services/store/kind/dashboard/dashboard.go b/pkg/services/store/kind/dashboard/dashboard.go index 76a62c506df..3229acbe80d 100644 --- a/pkg/services/store/kind/dashboard/dashboard.go +++ b/pkg/services/store/kind/dashboard/dashboard.go @@ -211,15 +211,27 @@ func readDashboardIter(iter *jsoniter.Iterator, lookup DatasourceLookup) (*Dashb } case "annotations": - for sub := iter.ReadObject(); sub != ""; sub = iter.ReadObject() { - if sub == "list" { - for iter.ReadArray() { - v := iter.Read() - logf("[dash.anno] %v\n", v) - } - } else { - iter.Skip() + switch iter.WhatIsNext() { + case jsoniter.ArrayValue: + // dashboards v2 is an array + for iter.ReadArray() { + v := iter.Read() + logf("[dash.anno] %v\n", v) } + case jsoniter.ObjectValue: + // dashboards v0/v1 are an object + for sub := iter.ReadObject(); sub != ""; sub = iter.ReadObject() { + if sub == "list" { + for iter.ReadArray() { + v := iter.Read() + logf("[dash.anno] %v\n", v) + } + } else { + iter.Skip() + } + } + default: + iter.Skip() } case "templating": diff --git a/pkg/storage/unified/apistore/prepare.go b/pkg/storage/unified/apistore/prepare.go index e4bb5ed7d18..c9677fd0763 100644 --- a/pkg/storage/unified/apistore/prepare.go +++ b/pkg/storage/unified/apistore/prepare.go @@ -9,12 +9,13 @@ import ( "time" "github.com/google/uuid" - authtypes "github.com/grafana/authlib/types" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/storage" "k8s.io/klog/v2" + authtypes "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/storage/unified/resource" ) @@ -73,12 +74,6 @@ func (s *Storage) prepareObjectForStorage(ctx context.Context, newObject runtime obj.SetResourceVersion("") obj.SetSelfLink("") - // Read+write will verify that repository format is accurate - repo, err := obj.GetRepositoryInfo() - if err != nil { - return nil, err - } - obj.SetRepositoryInfo(repo) obj.SetUpdatedBy("") obj.SetUpdatedTimestamp(nil) obj.SetCreatedBy(info.GetUID()) @@ -136,12 +131,6 @@ func (s *Storage) prepareObjectForUpdate(ctx context.Context, updateObject runti obj.SetDeprecatedInternalID(previousInternalID) // nolint:staticcheck } - // Read+write will verify that origin format is accurate - repo, err := obj.GetRepositoryInfo() - if err != nil { - return nil, err - } - obj.SetRepositoryInfo(repo) obj.SetUpdatedBy(info.GetUID()) obj.SetUpdatedTimestampMillis(time.Now().UnixMilli()) diff --git a/pkg/storage/unified/apistore/prepare_test.go b/pkg/storage/unified/apistore/prepare_test.go index 18d87952615..c71f18572f8 100644 --- a/pkg/storage/unified/apistore/prepare_test.go +++ b/pkg/storage/unified/apistore/prepare_test.go @@ -6,16 +6,17 @@ import ( "time" "github.com/bwmarrin/snowflake" - authtypes "github.com/grafana/authlib/types" - "github.com/grafana/grafana/pkg/apimachinery/identity" - "github.com/grafana/grafana/pkg/apimachinery/utils" - "github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1" "github.com/stretchr/testify/require" "golang.org/x/exp/rand" "k8s.io/apimachinery/pkg/api/apitesting" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apiserver/pkg/storage" + + authtypes "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/apimachinery/utils" + "github.com/grafana/grafana/pkg/apis/dashboard/v0alpha1" ) var scheme = runtime.NewScheme() @@ -87,11 +88,14 @@ func TestPrepareObjectForStorage(t *testing.T) { meta, err := utils.MetaAccessor(obj) require.NoError(t, err) now := time.Now() - meta.SetRepositoryInfo(&utils.ResourceRepositoryInfo{ - Name: "test-repo", - Path: "test/path", - Hash: "hash", - Timestamp: &now, + meta.SetManagerProperties(utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "test-repo", + }) + meta.SetSourceProperties(utils.SourceProperties{ + Path: "test/path", + Checksum: "hash", + TimestampMillis: now.UnixMilli(), }) encodedData, err := s.prepareObjectForStorage(ctx, obj) @@ -101,14 +105,16 @@ func TestPrepareObjectForStorage(t *testing.T) { require.NoError(t, err) meta, err = utils.MetaAccessor(newObject) require.NoError(t, err) - require.Equal(t, meta.GetRepositoryHash(), "hash") - require.Equal(t, meta.GetRepositoryName(), "test-repo") - require.Equal(t, meta.GetRepositoryPath(), "test/path") - ts, err := meta.GetRepositoryTimestamp() - require.NoError(t, err) - parsed, err := time.Parse(time.RFC3339, now.UTC().Format(time.RFC3339)) - require.NoError(t, err) - require.Equal(t, ts, &parsed) + + m, ok := meta.GetManagerProperties() + require.True(t, ok) + s, ok := meta.GetSourceProperties() + require.True(t, ok) + + require.Equal(t, m.Identity, "test-repo") + require.Equal(t, s.Checksum, "hash") + require.Equal(t, s.Path, "test/path") + require.Equal(t, s.TimestampMillis, now.UnixMilli()) }) s.opts.RequireDeprecatedInternalID = true diff --git a/pkg/storage/unified/resource/document.go b/pkg/storage/unified/resource/document.go index 4d0682c09d7..2229527bb56 100644 --- a/pkg/storage/unified/resource/document.go +++ b/pkg/storage/unified/resource/document.go @@ -102,7 +102,10 @@ type IndexableDocument struct { References ResourceReferences `json:"reference,omitempty"` // When the resource is managed by an upstream repository - RepoInfo *utils.ResourceRepositoryInfo `json:"repo,omitempty"` + Manager *utils.ManagerProperties `json:"manager,omitempty"` + + // When the manager knows about file paths + Source *utils.SourceProperties `json:"source,omitempty"` } func (m *IndexableDocument) Type() string { @@ -173,7 +176,14 @@ func NewIndexableDocument(key *ResourceKey, rv int64, obj utils.GrafanaMetaAcces CreatedBy: obj.GetCreatedBy(), UpdatedBy: obj.GetUpdatedBy(), } - doc.RepoInfo, _ = obj.GetRepositoryInfo() + m, ok := obj.GetManagerProperties() + if ok { + doc.Manager = &m + } + s, ok := obj.GetSourceProperties() + if ok { + doc.Source = &s + } ts := obj.GetCreationTimestamp() if !ts.Time.IsZero() { doc.Created = ts.Time.UnixMilli() @@ -265,10 +275,11 @@ const SEARCH_FIELD_CREATED_BY = "createdBy" const SEARCH_FIELD_UPDATED = "updated" const SEARCH_FIELD_UPDATED_BY = "updatedBy" -const SEARCH_FIELD_REPOSITORY_NAME = "repo.name" -const SEARCH_FIELD_REPOSITORY_PATH = "repo.path" -const SEARCH_FIELD_REPOSITORY_HASH = "repo.hash" -const SEARCH_FIELD_REPOSITORY_TIME = "repo.time" +const SEARCH_FIELD_MANAGER_KIND = "manager.kind" +const SEARCH_FIELD_MANAGER_ID = "manager.id" +const SEARCH_FIELD_SOURCE_PATH = "source.path" +const SEARCH_FIELD_SOURCE_CHECKSUM = "source.checksum" +const SEARCH_FIELD_SOURCE_TIME = "source.timestampMillis" const SEARCH_FIELD_SCORE = "_score" // the match score const SEARCH_FIELD_EXPLAIN = "_explain" // score explanation as JSON object diff --git a/pkg/storage/unified/resource/document_test.go b/pkg/storage/unified/resource/document_test.go index 593d1fbffd9..2135748533d 100644 --- a/pkg/storage/unified/resource/document_test.go +++ b/pkg/storage/unified/resource/document_test.go @@ -33,17 +33,20 @@ func TestStandardDocumentBuilder(t *testing.T) { "resource": "playlists", "name": "test1" }, + "name": "test1", "rv": 10, "title": "test playlist unified storage", "title_phrase": "test playlist unified storage", "created": 1717236672000, "createdBy": "user:ABC", "updatedBy": "user:XYZ", - "name": "test1", - "repo": { - "name": "something", + "manager": { + "kind": "repo", + "id": "something" + }, + "source": { "path": "path/in/system.json", - "hash": "xyz" + "checksum": "xyz" } - }`, string(jj)) + }`, string(jj)) } diff --git a/pkg/storage/unified/resource/server.go b/pkg/storage/unified/resource/server.go index 3314cdd6ffc..9cc505d40f6 100644 --- a/pkg/storage/unified/resource/server.go +++ b/pkg/storage/unified/resource/server.go @@ -456,12 +456,9 @@ func (s *server) newEvent(ctx context.Context, user claims.AuthInfo, key *Resour } } - repo, err := obj.GetRepositoryInfo() - if err != nil { - return nil, NewBadRequestError("invalid repository info") - } - if repo != nil { - err = s.writeHooks.CanWriteValueFromRepository(ctx, user, repo.Name) + m, ok := obj.GetManagerProperties() + if ok && m.Kind == utils.ManagerKindRepo { + err = s.writeHooks.CanWriteValueFromRepository(ctx, user, m.Identity) if err != nil { return nil, AsErrorResult(err) } diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go index d67b145da4d..73eee078f69 100644 --- a/pkg/storage/unified/search/bleve.go +++ b/pkg/storage/unified/search/bleve.go @@ -18,11 +18,12 @@ import ( "github.com/blevesearch/bleve/v2/search/query" bleveSearch "github.com/blevesearch/bleve/v2/search/searcher" index "github.com/blevesearch/bleve_index_api" - "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" - "github.com/grafana/grafana/pkg/services/featuremgmt" "go.opentelemetry.io/otel/trace" "k8s.io/apimachinery/pkg/selection" + "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" + "github.com/grafana/grafana/pkg/services/featuremgmt" + authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/infra/log" @@ -304,19 +305,20 @@ func (b *bleveIndex) ListRepositoryObjects(ctx context.Context, req *resource.Li found, err := b.index.SearchInContext(ctx, &bleve.SearchRequest{ Query: &query.TermQuery{ Term: req.Name, - FieldVal: resource.SEARCH_FIELD_REPOSITORY_NAME, + FieldVal: resource.SEARCH_FIELD_MANAGER_ID, }, Fields: []string{ resource.SEARCH_FIELD_TITLE, resource.SEARCH_FIELD_FOLDER, - resource.SEARCH_FIELD_REPOSITORY_NAME, - resource.SEARCH_FIELD_REPOSITORY_PATH, - resource.SEARCH_FIELD_REPOSITORY_HASH, - resource.SEARCH_FIELD_REPOSITORY_TIME, + resource.SEARCH_FIELD_MANAGER_KIND, + resource.SEARCH_FIELD_MANAGER_ID, + resource.SEARCH_FIELD_SOURCE_PATH, + resource.SEARCH_FIELD_SOURCE_CHECKSUM, + resource.SEARCH_FIELD_SOURCE_TIME, }, Sort: search.SortOrder{ &search.SortField{ - Field: resource.SEARCH_FIELD_REPOSITORY_PATH, + Field: resource.SEARCH_FIELD_SOURCE_PATH, Type: search.SortFieldAsString, Desc: false, }, @@ -347,6 +349,10 @@ func (b *bleveIndex) ListRepositoryObjects(ctx context.Context, req *resource.Li if ok { return intV } + floatV, ok := v.(float64) + if ok { + return int64(floatV) + } str, ok := v.(string) if ok { t, _ := time.Parse(time.RFC3339, str) @@ -359,9 +365,9 @@ func (b *bleveIndex) ListRepositoryObjects(ctx context.Context, req *resource.Li for _, hit := range found.Hits { item := &resource.ListRepositoryObjectsResponse_Item{ Object: &resource.ResourceKey{}, - Hash: asString(hit.Fields[resource.SEARCH_FIELD_REPOSITORY_HASH]), - Path: asString(hit.Fields[resource.SEARCH_FIELD_REPOSITORY_PATH]), - Time: asTime(hit.Fields[resource.SEARCH_FIELD_REPOSITORY_TIME]), + Hash: asString(hit.Fields[resource.SEARCH_FIELD_SOURCE_CHECKSUM]), + Path: asString(hit.Fields[resource.SEARCH_FIELD_SOURCE_PATH]), + Time: asTime(hit.Fields[resource.SEARCH_FIELD_SOURCE_TIME]), Title: asString(hit.Fields[resource.SEARCH_FIELD_TITLE]), Folder: asString(hit.Fields[resource.SEARCH_FIELD_FOLDER]), } @@ -379,7 +385,7 @@ func (b *bleveIndex) CountRepositoryObjects(ctx context.Context) ([]*resource.Co Query: bleve.NewMatchAllQuery(), Size: 0, Facets: bleve.FacetsRequest{ - "count": bleve.NewFacetRequest(resource.SEARCH_FIELD_REPOSITORY_NAME, 1000), // typically less then 5 + "count": bleve.NewFacetRequest(resource.SEARCH_FIELD_MANAGER_ID, 1000), // typically less then 5 }, }) if err != nil { diff --git a/pkg/storage/unified/search/bleve_mappings.go b/pkg/storage/unified/search/bleve_mappings.go index 027fd87b947..7c59bbae559 100644 --- a/pkg/storage/unified/search/bleve_mappings.go +++ b/pkg/storage/unified/search/bleve_mappings.go @@ -69,9 +69,9 @@ func getBleveDocMappings(_ resource.SearchableDocumentFields) *mapping.DocumentM mapper.AddFieldMappingsAt(resource.SEARCH_FIELD_FOLDER, folderMapping) // Repositories - repo := bleve.NewDocumentStaticMapping() - repo.AddFieldMappingsAt("name", &mapping.FieldMapping{ - Name: "name", + manager := bleve.NewDocumentStaticMapping() + manager.AddFieldMappingsAt("kind", &mapping.FieldMapping{ + Name: "kind", Type: "text", Analyzer: keyword.Name, Store: true, @@ -79,7 +79,18 @@ func getBleveDocMappings(_ resource.SearchableDocumentFields) *mapping.DocumentM IncludeTermVectors: false, IncludeInAll: true, }) - repo.AddFieldMappingsAt("path", &mapping.FieldMapping{ + manager.AddFieldMappingsAt("id", &mapping.FieldMapping{ + Name: "id", + Type: "text", + Analyzer: keyword.Name, + Store: true, + Index: true, + IncludeTermVectors: false, + IncludeInAll: true, + }) + + source := bleve.NewDocumentStaticMapping() + source.AddFieldMappingsAt("path", &mapping.FieldMapping{ Name: "path", Type: "text", Analyzer: keyword.Name, @@ -88,8 +99,8 @@ func getBleveDocMappings(_ resource.SearchableDocumentFields) *mapping.DocumentM IncludeTermVectors: false, IncludeInAll: true, }) - repo.AddFieldMappingsAt("hash", &mapping.FieldMapping{ - Name: "hash", + source.AddFieldMappingsAt("checksum", &mapping.FieldMapping{ + Name: "checksum", Type: "text", Analyzer: keyword.Name, Store: true, @@ -97,9 +108,10 @@ func getBleveDocMappings(_ resource.SearchableDocumentFields) *mapping.DocumentM IncludeTermVectors: false, IncludeInAll: true, }) - repo.AddFieldMappingsAt("time", mapping.NewDateTimeFieldMapping()) + source.AddFieldMappingsAt("timestampMillis", mapping.NewNumericFieldMapping()) - mapper.AddSubDocumentMapping("repo", repo) + mapper.AddSubDocumentMapping("manager", manager) + mapper.AddSubDocumentMapping("source", source) labelMapper := bleve.NewDocumentMapping() mapper.AddSubDocumentMapping(resource.SEARCH_FIELD_LABELS, labelMapper) diff --git a/pkg/storage/unified/search/bleve_mappings_test.go b/pkg/storage/unified/search/bleve_mappings_test.go index d1a07fdf116..7a563fff827 100644 --- a/pkg/storage/unified/search/bleve_mappings_test.go +++ b/pkg/storage/unified/search/bleve_mappings_test.go @@ -25,11 +25,14 @@ func TestDocumentMapping(t *testing.T) { "x": "y", }, RV: 1234, - RepoInfo: &utils.ResourceRepositoryInfo{ - Name: "nnn", - Path: "ppp", - Hash: "hhh", - Timestamp: asTimePointer(1234), + Manager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "rrr", + }, + Source: &utils.SourceProperties{ + Path: "ppp", + Checksum: "ooo", + TimestampMillis: 1234, }, } @@ -43,5 +46,5 @@ func TestDocumentMapping(t *testing.T) { fmt.Printf("DOC: fields %d\n", len(doc.Fields)) fmt.Printf("DOC: size %d\n", doc.Size()) - require.Equal(t, 13, len(doc.Fields)) + require.Equal(t, 14, len(doc.Fields)) } diff --git a/pkg/storage/unified/search/bleve_test.go b/pkg/storage/unified/search/bleve_test.go index b2d90f25c0f..fa572536d05 100644 --- a/pkg/storage/unified/search/bleve_test.go +++ b/pkg/storage/unified/search/bleve_test.go @@ -7,20 +7,18 @@ import ( "os" "path/filepath" "testing" - "time" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/apimachinery/identity" - authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" - "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/infra/tracing" + authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/store/kind/dashboard" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/storage/unified/resource" ) @@ -90,11 +88,14 @@ func TestBleveBackend(t *testing.T) { utils.LabelKeyDeprecatedInternalID: "10", // nolint:staticcheck }, Tags: []string{"aa", "bb"}, - RepoInfo: &utils.ResourceRepositoryInfo{ - Name: "repo-1", - Path: "path/to/aaa.json", - Hash: "xyz", - Timestamp: asTimePointer(1609462800000), // 2021 + Manager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + Source: &utils.SourceProperties{ + Path: "path/to/aaa.json", + Checksum: "xyz", + TimestampMillis: 1609462800000, // 2021 }, }) _ = index.Write(&resource.IndexableDocument{ @@ -119,11 +120,14 @@ func TestBleveBackend(t *testing.T) { "region": "east", utils.LabelKeyDeprecatedInternalID: "11", // nolint:staticcheck }, - RepoInfo: &utils.ResourceRepositoryInfo{ - Name: "repo-1", - Path: "path/to/bbb.json", - Hash: "hijk", - Timestamp: asTimePointer(1640998800000), // 2022 + Manager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + Source: &utils.SourceProperties{ + Path: "path/to/bbb.json", + Checksum: "hijk", + TimestampMillis: 1640998800000, // 2022 }, }) _ = index.Write(&resource.IndexableDocument{ @@ -138,8 +142,11 @@ func TestBleveBackend(t *testing.T) { Title: "ccc (dash)", TitlePhrase: "ccc (dash)", Folder: "zzz", - RepoInfo: &utils.ResourceRepositoryInfo{ - Name: "repo2", + Manager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo2", + }, + Source: &utils.SourceProperties{ Path: "path/in/repo2.yaml", }, Fields: map[string]any{}, @@ -263,6 +270,7 @@ func TestBleveBackend(t *testing.T) { jj, err := json.MarshalIndent(found, "", " ") require.NoError(t, err) fmt.Printf("%s\n", string(jj)) + // NOTE "hash" -> "checksum" requires changing the protobuf require.JSONEq(t, `{ "items": [ { @@ -334,11 +342,14 @@ func TestBleveBackend(t *testing.T) { }, Title: "zzz (folder)", TitlePhrase: "zzz (folder)", - RepoInfo: &utils.ResourceRepositoryInfo{ - Name: "repo-1", - Path: "path/to/folder.json", - Hash: "xxxx", - Timestamp: asTimePointer(300), + Manager: &utils.ManagerProperties{ + Kind: utils.ManagerKindRepo, + Identity: "repo-1", + }, + Source: &utils.SourceProperties{ + Path: "path/to/folder.json", + Checksum: "xxxx", + TimestampMillis: 300, }, }) _ = index.Write(&resource.IndexableDocument{ @@ -559,14 +570,6 @@ func TestGetSortFields(t *testing.T) { }) } -func asTimePointer(milli int64) *time.Time { - if milli > 0 { - t := time.UnixMilli(milli) - return &t - } - return nil -} - var _ authlib.AccessClient = (*StubAccessClient)(nil) func NewStubAccessClient(permissions map[string]bool) *StubAccessClient { diff --git a/pkg/storage/unified/search/document_test.go b/pkg/storage/unified/search/document_test.go index 04f9637cb77..c11ada997a6 100644 --- a/pkg/storage/unified/search/document_test.go +++ b/pkg/storage/unified/search/document_test.go @@ -81,14 +81,26 @@ func TestDashboardDocumentBuilder(t *testing.T) { // Standard builder = resource.StandardDocumentBuilder() - doSnapshotTests(t, builder, "folder", key, []string{ + doSnapshotTests(t, builder, "folder", &resource.ResourceKey{ + Namespace: "default", + Group: "folder.grafana.app", + Resource: "folders", + }, []string{ "aaa", "bbb", }) - doSnapshotTests(t, builder, "playlist", key, []string{ + doSnapshotTests(t, builder, "playlist", &resource.ResourceKey{ + Namespace: "default", + Group: "playlist.grafana.app", + Resource: "playlists", + }, []string{ "aaa", }) - doSnapshotTests(t, builder, "report", key, []string{ + doSnapshotTests(t, builder, "report", &resource.ResourceKey{ + Namespace: "default", + Group: "reporting.grafana.app", + Resource: "reports", + }, []string{ "aaa", }) } diff --git a/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json b/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json index 86cb4f5df16..c2807e7f48a 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/folder-aaa-out.json @@ -1,8 +1,8 @@ { "key": { "namespace": "default", - "group": "dashboard.grafana.app", - "resource": "dashboards", + "group": "folder.grafana.app", + "resource": "folders", "name": "aaa" }, "name": "aaa", @@ -11,7 +11,8 @@ "title_phrase": "test-aaa", "created": 1730490142000, "createdBy": "user:1", - "repo": { - "name": "SQL" + "manager": { + "kind": "repo", + "id": "MyGIT" } } \ No newline at end of file diff --git a/pkg/storage/unified/search/testdata/doc/folder-aaa.json b/pkg/storage/unified/search/testdata/doc/folder-aaa.json index 97262d5926e..27ef4d28ffb 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-aaa.json +++ b/pkg/storage/unified/search/testdata/doc/folder-aaa.json @@ -8,7 +8,7 @@ "creationTimestamp": "2024-11-01T19:42:22Z", "annotations": { "grafana.app/createdBy": "user:1", - "grafana.app/originName": "SQL" + "grafana.app/repoName": "MyGIT" } }, "spec": { diff --git a/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json b/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json index 5d268564f5b..cabacb8b404 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json +++ b/pkg/storage/unified/search/testdata/doc/folder-bbb-out.json @@ -1,8 +1,8 @@ { "key": { "namespace": "default", - "group": "dashboard.grafana.app", - "resource": "dashboards", + "group": "folder.grafana.app", + "resource": "folders", "name": "bbb" }, "name": "bbb", @@ -11,7 +11,8 @@ "title_phrase": "test-bbb", "created": 1730490142000, "createdBy": "user:1", - "repo": { - "name": "SQL" + "manager": { + "kind": "repo", + "id": "MyGIT" } } \ No newline at end of file diff --git a/pkg/storage/unified/search/testdata/doc/folder-bbb.json b/pkg/storage/unified/search/testdata/doc/folder-bbb.json index 6d5f66d648d..81f97cc332a 100644 --- a/pkg/storage/unified/search/testdata/doc/folder-bbb.json +++ b/pkg/storage/unified/search/testdata/doc/folder-bbb.json @@ -8,7 +8,7 @@ "creationTimestamp": "2024-11-01T19:42:22Z", "annotations": { "grafana.app/createdBy": "user:1", - "grafana.app/originName": "SQL" + "grafana.app/repoName": "MyGIT" } }, "spec": { diff --git a/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json b/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json index c0f12814338..0bc5b824883 100644 --- a/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/playlist-aaa-out.json @@ -1,8 +1,8 @@ { "key": { "namespace": "default", - "group": "dashboard.grafana.app", - "resource": "dashboards", + "group": "playlist.grafana.app", + "resource": "playlists", "name": "aaa" }, "name": "aaa", @@ -10,10 +10,5 @@ "title": "Test AAA", "title_phrase": "test aaa", "created": 1731336353000, - "createdBy": "user:t000000001", - "repo": { - "name": "UI", - "path": "/playlists/new", - "hash": "Grafana v11.4.0-pre (c0de407fee)" - } + "createdBy": "user:t000000001" } \ No newline at end of file diff --git a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json index 6b193267072..656f57c46f1 100644 --- a/pkg/storage/unified/search/testdata/doc/report-aaa-out.json +++ b/pkg/storage/unified/search/testdata/doc/report-aaa-out.json @@ -1,8 +1,8 @@ { "key": { "namespace": "default", - "group": "dashboard.grafana.app", - "resource": "dashboards", + "group": "reporting.grafana.app", + "resource": "reports", "name": "aaa" }, "name": "aaa", diff --git a/pkg/tests/apis/dashboard/dashboards_test.go b/pkg/tests/apis/dashboard/dashboards_test.go index e13e26a1b3d..317945da7f3 100644 --- a/pkg/tests/apis/dashboard/dashboards_test.go +++ b/pkg/tests/apis/dashboard/dashboards_test.go @@ -68,7 +68,9 @@ func runDashboardTest(t *testing.T, helper *apis.K8sTestHelper, gvr schema.Group wrap, err := utils.MetaAccessor(obj) require.NoError(t, err) - require.Empty(t, wrap.GetRepositoryName()) // no SQL repo stub + + m, _ := wrap.GetManagerProperties() + require.Empty(t, m.Identity) // no SQL repo stub require.Equal(t, helper.Org1.Admin.Identity.GetUID(), wrap.GetCreatedBy()) // Commented out because the dynamic client does not like lists as sub-resource diff --git a/public/api-merged.json b/public/api-merged.json index c14b5edc8c6..7d1e7214293 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -15514,6 +15514,9 @@ "type": "integer", "format": "int64" }, + "managedBy": { + "$ref": "#/definitions/ManagerKind" + }, "orgId": { "type": "integer", "format": "int64" @@ -15529,10 +15532,6 @@ "$ref": "#/definitions/Folder" } }, - "repository": { - "description": "When the folder belongs to a repository\nNOTE: this is only populated when folders are managed by unified storage", - "type": "string" - }, "title": { "type": "string" }, @@ -15562,11 +15561,10 @@ "type": "integer", "format": "int64" }, - "parentUid": { - "type": "string" + "managedBy": { + "$ref": "#/definitions/ManagerKind" }, - "repository": { - "description": "When the folder belongs to a repository\nNOTE: this is only populated when folders are managed by unified storage", + "parentUid": { "type": "string" }, "title": { @@ -17047,6 +17045,11 @@ } } }, + "ManagerKind": { + "description": "It can be a user or a tool or a generic API client.\n+enum", + "type": "string", + "title": "ManagerKind is the type of manager, which is responsible for managing the resource." + }, "MassDeleteAnnotationsCmd": { "type": "object", "properties": { diff --git a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts index a0d40e6c874..b6d8c812163 100644 --- a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts +++ b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts @@ -40,7 +40,7 @@ export const featureDiscoveryApi = alertingApi.injectEndpoints({ queryFn: async (rulesSourceIdentifier) => { const dataSourceUID = getDataSourceUID(rulesSourceIdentifier); if (!dataSourceUID) { - return { error: new Error(`Unable to find data source for ${rulesSourceIdentifier}`) }; + return { error: new Error(`Unable to find data source for ${JSON.stringify(rulesSourceIdentifier)}`) }; } if (dataSourceUID === GrafanaRulesSourceSymbol) { diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx index f8e85ba76d6..36d2d6f06d1 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx @@ -10,6 +10,7 @@ import { AccessControlAction, FolderDTO } from 'app/types'; import { setupMswServer } from '../mockApi'; import { mockDataSource, mockFolder, mockRulerAlertingRule, mockRulerGrafanaRule } from '../mocks'; import { setupDataSources } from '../testSetup/datasources'; +import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; import { useFolder } from './useFolder'; import { useIsRuleEditable } from './useIsRuleEditable'; @@ -56,7 +57,9 @@ describe('useIsRuleEditable', () => { const wrapper = getProviderWrapper(); - const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + const { result } = renderHook(() => useIsRuleEditable(GRAFANA_RULES_SOURCE_NAME, mockRulerGrafanaRule()), { + wrapper, + }); await waitFor(() => expect(result.current.loading).toBe(false)); expect(result.current.isRemovable).toBe(true); diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts index 50ca91b8188..017e008c02c 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts @@ -3,7 +3,6 @@ import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { featureDiscoveryApi } from '../api/featureDiscoveryApi'; import { getRulesPermissions } from '../utils/access-control'; -import { getDatasourceAPIUid } from '../utils/datasource'; import { isGrafanaRulerRule } from '../utils/rules'; import { useFolder } from './useFolder'; @@ -13,11 +12,16 @@ interface ResultBag { isEditable?: boolean; isRemovable?: boolean; loading: boolean; + error?: unknown; } export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): ResultBag { - const { currentData: dsFeatures, isLoading } = featureDiscoveryApi.endpoints.discoverDsFeatures.useQuery({ - uid: getDatasourceAPIUid(rulesSourceName), + const { + currentData: dsFeatures, + isLoading, + error, + } = featureDiscoveryApi.endpoints.discoverDsFeatures.useQuery({ + rulesSourceName, }); const folderUID = rule && isGrafanaRulerRule(rule) ? rule.grafana_alert.namespace_uid : undefined; @@ -25,6 +29,17 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): const rulePermission = getRulesPermissions(rulesSourceName); const { folder, loading } = useFolder(folderUID); + // handle discovery and data source errors + if (error) { + return { + isEditable: false, + isRemovable: false, + loading: false, + isRulerAvailable: false, + error, + }; + } + if (!rule) { return { isEditable: false, isRemovable: false, loading: false }; } diff --git a/public/app/features/alerting/unified/mocks.ts b/public/app/features/alerting/unified/mocks.ts index 353a9531b9d..8e2701e0e04 100644 --- a/public/app/features/alerting/unified/mocks.ts +++ b/public/app/features/alerting/unified/mocks.ts @@ -57,6 +57,7 @@ import { import { DashboardSearchItem, DashboardSearchItemType } from '../../search/types'; import { SimpleConditionIdentifier } from './components/rule-editor/query-and-alert-condition/SimpleCondition'; +import { GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; import { parsePromQLStyleMatcherLooseSafe } from './utils/matchers'; let nextDataSourceId = 1; @@ -686,7 +687,7 @@ export function getGrafanaRule(override?: Partial, rulerOverride?: namespace: { groups: [], name: 'Grafana', - rulesSource: 'grafana', + rulesSource: GRAFANA_RULES_SOURCE_NAME, }, rulerRule: mockGrafanaRulerRule(rulerOverride), ...override, diff --git a/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx b/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx index 00641db1eda..3ca54fdd9aa 100644 --- a/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx +++ b/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx @@ -17,17 +17,30 @@ interface ExistingRuleEditorProps { } export function ExistingRuleEditor({ identifier, prefill }: ExistingRuleEditorProps) { - const { - loading: loadingAlertRule, - result: ruleWithLocation, - error, - } = useRuleWithLocation({ ruleIdentifier: identifier }); const [queryParams] = useQueryParams(); const isManualRestore = Boolean(queryParams.isManualRestore); - const ruleSourceName = ruleId.ruleIdentifierToRuleSourceName(identifier); + const { + loading: loadingAlertRule, + result: ruleWithLocation, + error: fetchRuleError, + } = useRuleWithLocation({ ruleIdentifier: identifier }); - const { isEditable, loading: loadingEditable } = useIsRuleEditable(ruleSourceName, ruleWithLocation?.rule); + const ruleSourceName = ruleId.ruleIdentifierToRuleSourceName(identifier); + const { + isEditable, + loading: loadingEditable, + error: errorEditable, + } = useIsRuleEditable(ruleSourceName, ruleWithLocation?.rule); + + // error handling for fetching rule and rule RBAC + if (fetchRuleError || errorEditable) { + return ( + + {stringifyErrorLike(errorEditable ?? fetchRuleError)} + + ); + } const loading = loadingAlertRule || loadingEditable; @@ -35,14 +48,6 @@ export function ExistingRuleEditor({ identifier, prefill }: ExistingRuleEditorPr return ; } - if (error) { - return ( - - {stringifyErrorLike(error)} - - ); - } - if (!ruleWithLocation && !loading) { return Sorry! This rule does not exist.; } diff --git a/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx index 1781ab75d2b..8f6538f60b7 100644 --- a/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx @@ -4,7 +4,6 @@ import { render, screen } from 'test/test-utils'; import { contextSrv } from 'app/core/services/context_srv'; import { setFolderResponse } from 'app/features/alerting/unified/mocks/server/configure'; -import { MIMIR_DATASOURCE_UID } from 'app/features/alerting/unified/mocks/server/constants'; import { captureRequests } from 'app/features/alerting/unified/mocks/server/events'; import { DashboardSearchItemType } from 'app/features/search/types'; import { AccessControlAction } from 'app/types'; @@ -12,6 +11,7 @@ import { AccessControlAction } from 'app/types'; import { setupMswServer } from '../mockApi'; import { grantUserPermissions, mockDataSource, mockFolder } from '../mocks'; import { grafanaRulerRule } from '../mocks/grafanaRulerApi'; +import { MIMIR_DATASOURCE_UID } from '../mocks/server/constants'; import { setupDataSources } from '../testSetup/datasources'; import { Annotation } from '../utils/constants'; @@ -151,3 +151,15 @@ describe('RuleEditor grafana managed rules', () => { expect(postBody.interval).toBe('12m'); }); }); + +describe('Data source managed rules', () => { + beforeEach(() => { + jest.clearAllMocks(); + grantUserPermissions([AccessControlAction.AlertingRuleExternalRead, AccessControlAction.AlertingRuleExternalWrite]); + }); + + it('should show an error if the data source does not exist', async () => { + renderRuleEditor('cri%24grafana-cloudd%24delete me%24delete me 3%24recording_rule_delete_2%24-476183141'); + expect(await screen.findByText(/unable to find data source/i)).toBeInTheDocument(); + }); +}); diff --git a/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx b/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx index 558d2294899..3a4a0972a05 100644 --- a/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx +++ b/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx @@ -1,16 +1,15 @@ -import { ReactNode } from 'react'; - import { SceneObjectState, SceneObjectBase, sceneGraph, VariableDependencyConfig, SceneObject } from '@grafana/scenes'; import { t } from 'app/core/internationalization'; import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor'; +import { getDefaultVizPanel } from '../../utils/utils'; import { ResponsiveGridLayoutManager } from '../layout-responsive-grid/ResponsiveGridLayoutManager'; import { BulkActionElement } from '../types/BulkActionElement'; import { DashboardLayoutManager } from '../types/DashboardLayoutManager'; import { EditableDashboardElement, EditableDashboardElementInfo } from '../types/EditableDashboardElement'; import { LayoutParent } from '../types/LayoutParent'; -import { getEditOptions, renderActions } from './TabItemEditor'; +import { getEditOptions } from './TabItemEditor'; import { TabItemRenderer } from './TabItemRenderer'; import { TabItems } from './TabItems'; import { TabsLayoutManager } from './TabsLayoutManager'; @@ -56,14 +55,6 @@ export class TabItem return getEditOptions(this); } - public renderActions(): ReactNode { - return renderActions(this); - } - - public getParentLayout(): TabsLayoutManager { - return sceneGraph.getAncestor(this, TabsLayoutManager); - } - public onDelete() { const layout = sceneGraph.getAncestor(this, TabsLayoutManager); layout.removeTab(this); @@ -73,7 +64,43 @@ export class TabItem return new TabItems(items.filter((item) => item instanceof TabItem)); } + public onAddPanel(panel = getDefaultVizPanel()) { + this.getLayout().addPanel(panel); + } + + public onAddTabBefore() { + this._getParentLayout().addTabBefore(this); + } + + public onAddTabAfter() { + this._getParentLayout().addTabAfter(this); + } + + public onMoveLeft() { + this._getParentLayout().moveTabLeft(this); + } + + public onMoveRight() { + this._getParentLayout().moveTabRight(this); + } + + public isFirstTab(): boolean { + return this._getParentLayout().isFirstTab(this); + } + + public isLastTab(): boolean { + return this._getParentLayout().isLastTab(this); + } + public onChangeTitle(title: string) { this.setState({ title }); } + + public getParentLayout(): TabsLayoutManager { + return this._getParentLayout(); + } + + private _getParentLayout(): TabsLayoutManager { + return sceneGraph.getAncestor(this, TabsLayoutManager); + } } diff --git a/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx b/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx index 7d0e8c70b8c..2769620060e 100644 --- a/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx +++ b/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx @@ -1,10 +1,11 @@ -import { ReactNode, useMemo } from 'react'; +import { useMemo } from 'react'; -import { Button, Input } from '@grafana/ui'; +import { Input } from '@grafana/ui'; import { t } from 'app/core/internationalization'; import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor'; import { OptionsPaneItemDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneItemDescriptor'; +import { EditPaneHeader } from '../../edit-pane/EditPaneHeader'; import { useLayoutCategory } from '../layouts-shared/DashboardLayoutSelector'; import { TabItem } from './TabItem'; @@ -12,9 +13,12 @@ import { TabItem } from './TabItem'; export function getEditOptions(model: TabItem): OptionsPaneCategoryDescriptor[] { const tabOptions = useMemo(() => { return new OptionsPaneCategoryDescriptor({ - title: t('dashboard.tabs-layout.tab-options.title', 'Tab options'), + title: '', id: 'tab-options', - isOpenDefault: true, + isOpenable: false, + renderTitle: () => ( + model.onDelete()} /> + ), }).addItem( new OptionsPaneItemDescriptor({ title: t('dashboard.tabs-layout.tab-options.title-option', 'Title'), @@ -29,15 +33,6 @@ export function getEditOptions(model: TabItem): OptionsPaneCategoryDescriptor[] return [tabOptions, layoutOptions]; } -export function renderActions(tab: TabItem): ReactNode { - return ( - <> -