diff --git a/.github/workflows/docker_build.yml b/.github/workflows/docker_build.yml index 2a11e0b9be..b9a09e4f1e 100644 --- a/.github/workflows/docker_build.yml +++ b/.github/workflows/docker_build.yml @@ -1,4 +1,5 @@ name: docker +# Trigger rebuild 2 on: push: @@ -40,6 +41,14 @@ jobs: with: platforms: linux/amd64,linux/arm64 + - name: Compute image tag + id: meta + run: | + # For PRs use the source branch name; for pushes use the ref name + REF="${{ github.head_ref || github.ref_name }}" + # Replace / with - for valid Docker tags + echo "tag=${REF//\//-}" >> "$GITHUB_OUTPUT" + - name: Build and push uses: docker/build-push-action@v5 with: @@ -48,6 +57,6 @@ jobs: build-args: | BINGO=false COMMIT_SHA=${{ github.sha }} - BUILD_BRANCH=main + BUILD_BRANCH=${{ github.head_ref || github.ref_name }} push: true - tags: intergral/grafana:${{ github.ref_name }} + tags: intergral/grafana:${{ steps.meta.outputs.tag }} \ No newline at end of file diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 3c1deeb193..d9ad70f631 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -383,6 +383,11 @@ func (s *Service) getCachedTeamsPermissions(ctx context.Context, user identity.R func (s *Service) ClearUserPermissionCache(user identity.Requester) { s.cache.Delete(accesscontrol.GetUserDirectPermissionCacheKey(user)) + // Also clear basic role caches so managed role permissions for newly created + // resources are picked up on the next request. + for _, role := range accesscontrol.GetOrgRoles(user) { + s.cache.Delete(accesscontrol.GetBasicRolePermissionCacheKey(role, user.GetOrgID())) + } } func (s *Service) DeleteUserPermissions(ctx context.Context, orgID int64, userID int64) error { diff --git a/pkg/services/accesscontrol/resolvers.go b/pkg/services/accesscontrol/resolvers.go index 56f2903735..5ae879468e 100644 --- a/pkg/services/accesscontrol/resolvers.go +++ b/pkg/services/accesscontrol/resolvers.go @@ -88,6 +88,11 @@ func (s *Resolvers) GetScopeAttributeMutator(orgID int64) ScopeAttributeMutator } } +// ClearCacheFor removes the cached scope resolution for a specific scope in an org. +func (s *Resolvers) ClearCacheFor(orgID int64, scope string) { + s.cache.Delete(getScopeCacheKey(orgID, scope)) +} + // getScopeCacheKey creates an identifier to fetch and store resolution of scopes in the cache func getScopeCacheKey(orgID int64, scope string) string { return fmt.Sprintf("%s-%v", scope, orgID) diff --git a/public/app/features/apiserver/client.ts b/public/app/features/apiserver/client.ts index 9aeda6c53e..1dbc2fff57 100644 --- a/public/app/features/apiserver/client.ts +++ b/public/app/features/apiserver/client.ts @@ -1,7 +1,7 @@ import { Observable, from, retry, catchError, filter, map, mergeMap } from 'rxjs'; import { isLiveChannelMessageEvent, LiveChannelScope } from '@grafana/data'; -import { config, getBackendSrv, getGrafanaLiveSrv } from '@grafana/runtime'; +import { BackendSrvRequest, config, getBackendSrv, getGrafanaLiveSrv } from '@grafana/runtime'; import { contextSrv } from 'app/core/core'; import { getAPINamespace } from '../../api/utils'; @@ -98,8 +98,13 @@ export class ScopedResourceClient implements ); } - public async subresource(name: string, path: string, params?: Record): Promise { - return getBackendSrv().get(`${this.url}/${name}/${path}`, params); + public async subresource( + name: string, + path: string, + params?: Record, + options?: Partial + ): Promise { + return getBackendSrv().get(`${this.url}/${name}/${path}`, params, undefined, options); } public async list(opts?: ListOptions | undefined): Promise> { diff --git a/public/app/features/apiserver/types.ts b/public/app/features/apiserver/types.ts index 3e77035f13..17d75e5ca2 100644 --- a/public/app/features/apiserver/types.ts +++ b/public/app/features/apiserver/types.ts @@ -10,6 +10,8 @@ import { Observable } from 'rxjs'; +import { BackendSrvRequest } from '@grafana/runtime'; + /** The object type and version */ export interface TypeMeta { apiVersion: string; @@ -261,7 +263,7 @@ export interface ResourceClient { update(obj: ResourceForCreate, params?: ResourceClientWriteParams): Promise>; delete(name: string, showSuccessAlert?: boolean): Promise; list(opts?: ListOptions): Promise>; - subresource(name: string, path: string, params?: Record): Promise; + subresource(name: string, path: string, params?: Record, options?: Partial): Promise; watch(opts?: WatchOptions): Observable>; } diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts index e17f7e36cf..4738eb5cc8 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts +++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts @@ -573,19 +573,50 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag } return result; } - default: - // If reloadDashboardsOnParamsChange is on, we need to process query params for dashboard load - // Since the scene is not yet there, we need to process whatever came through URL - if (config.featureToggles.reloadDashboardsOnParamsChange) { - const queryParamsObject = processQueryParamsForDashboardLoad(); - rsp = await dashboardLoaderSrv.loadDashboard(type || 'db', slug || '', uid, queryParamsObject); - } else { - rsp = await dashboardLoaderSrv.loadDashboard(type || 'db', slug || '', uid); + default: { + // In multi-instance deployments, a post-save redirect may land on an instance + // that hasn't propagated the new dashboard yet. Retry on 404/403 when afterSave is set. + const searchParams = new URLSearchParams(locationService.getLocation().search); + const isAfterSave = searchParams.has('afterSave'); + const maxAttempts = isAfterSave ? 6 : 1; + + for (let attempt = 0; attempt < maxAttempts; attempt++) { + try { + // If reloadDashboardsOnParamsChange is on, we need to process query params for dashboard load + // Since the scene is not yet there, we need to process whatever came through URL + if (config.featureToggles.reloadDashboardsOnParamsChange) { + const queryParamsObject = processQueryParamsForDashboardLoad(); + rsp = await dashboardLoaderSrv.loadDashboard(type || 'db', slug || '', uid, queryParamsObject); + } else { + rsp = await dashboardLoaderSrv.loadDashboard(type || 'db', slug || '', uid); + } + break; + } catch (e) { + const isRetryable = + isAfterSave && + attempt < maxAttempts - 1 && + isFetchError(e) && + (e.status === 404 || e.status === 403 || e.status === 500); + if (!isRetryable) { + throw e; + } + await new Promise((resolve) => setTimeout(resolve, 500)); + } + } + + // Strip afterSave param now that the dashboard loaded successfully + if (isAfterSave) { + searchParams.delete('afterSave'); + locationService.replace({ + ...locationService.getLocation(), + search: searchParams.toString(), + }); } if (route === DashboardRoutes.Embedded) { rsp.meta.isEmbedded = true; } + } } // Fix outdated URLs (e.g., old slugs from title changes) but skip during playlist navigation @@ -770,13 +801,44 @@ export class DashboardScenePageStateManagerV2 extends DashboardScenePageStateMan case DashboardRoutes.Public: { return await this.dashboardLoader.loadDashboard('public', '', uid); } - default: - rsp = await this.dashboardLoader.loadDashboard(type || 'db', slug || '', uid); + default: { + // In multi-instance deployments, a post-save redirect may land on an instance + // that hasn't propagated the new dashboard yet. Retry on 404/403/500 when afterSave is set. + const searchParams = new URLSearchParams(locationService.getLocation().search); + const isAfterSave = searchParams.has('afterSave'); + const maxAttempts = isAfterSave ? 6 : 1; + + for (let attempt = 0; attempt < maxAttempts; attempt++) { + try { + rsp = await this.dashboardLoader.loadDashboard(type || 'db', slug || '', uid); + break; + } catch (e) { + const isRetryable = + isAfterSave && + attempt < maxAttempts - 1 && + isFetchError(e) && + (e.status === 404 || e.status === 403 || e.status === 500); + if (!isRetryable) { + throw e; + } + await new Promise((resolve) => setTimeout(resolve, 500)); + } + } + + // Strip afterSave param now that the dashboard loaded successfully + if (isAfterSave) { + searchParams.delete('afterSave'); + locationService.replace({ + ...locationService.getLocation(), + search: searchParams.toString(), + }); + } if (route === DashboardRoutes.Embedded) { rsp.metadata.annotations = rsp.metadata.annotations || {}; rsp.metadata.annotations[AnnoKeyEmbedded] = 'embedded'; } + } } // Fix outdated URLs (e.g., old slugs from title changes) but skip during playlist navigation // Playlists manage their own URL generation and redirects would break the navigation flow diff --git a/public/app/features/dashboard-scene/saving/useSaveDashboard.ts b/public/app/features/dashboard-scene/saving/useSaveDashboard.ts index e9bb81b026..355e3f89c7 100644 --- a/public/app/features/dashboard-scene/saving/useSaveDashboard.ts +++ b/public/app/features/dashboard-scene/saving/useSaveDashboard.ts @@ -91,8 +91,13 @@ export function useSaveDashboard(isCopy = false) { const newUrl = locationUtil.stripBaseFromUrl(resultData.url); if (newUrl !== currentLocation.pathname) { + // Signal to the dashboard loader that this is a post-save redirect. + // In multi-instance deployments, the serving instance may not have the + // dashboard propagated yet — the loader will retry on 404/403. + const search = new URLSearchParams(currentLocation.search); + search.set('afterSave', '1'); setTimeout(() => { - locationService.push({ pathname: newUrl, search: currentLocation.search }); + locationService.push({ pathname: newUrl, search: search.toString() }); }); } diff --git a/public/app/features/dashboard/api/v1.ts b/public/app/features/dashboard/api/v1.ts index 6f67451178..8dddfffb39 100644 --- a/public/app/features/dashboard/api/v1.ts +++ b/public/app/features/dashboard/api/v1.ts @@ -121,7 +121,9 @@ export class K8sDashboardAPI implements DashboardAPI { async getDashboardDTO(uid: string, params?: UrlQueryMap) { try { - const dash = await this.client.subresource>(uid, 'dto', params); + const dash = await this.client.subresource>( + uid, 'dto', params, { showErrorAlert: false } + ); // This could come as conversion error from v0 or v2 to V1. if (dash.status?.conversion?.failed && isV2StoredVersion(dash.status.conversion.storedVersion)) { @@ -172,7 +174,7 @@ export class K8sDashboardAPI implements DashboardAPI { result.meta.folderId = folder.id; } catch (e) { // If user has access to dashboard but not to folder, continue without folder info - if (getStatusFromError(e) !== 403) { + if (getStatusFromError(e) !== 403 && getStatusFromError(e) !== 500) { throw new Error('Failed to load folder'); } // we still want to save the folder uid so that we can properly handle disabling the folder picker in Settings -> General diff --git a/public/app/features/dashboard/api/v2.ts b/public/app/features/dashboard/api/v2.ts index dfcbcc638f..0da3d87009 100644 --- a/public/app/features/dashboard/api/v2.ts +++ b/public/app/features/dashboard/api/v2.ts @@ -42,7 +42,9 @@ export class K8sDashboardV2API async getDashboardDTO(uid: string) { try { - const dashboard = await this.client.subresource>(uid, 'dto'); + const dashboard = await this.client.subresource>( + uid, 'dto', undefined, { showErrorAlert: false } + ); // FOR /dto calls returning v2 spec we are ignoring the conversion status to avoid runtime errors caused by the status // being saved for v2 resources that's been client-side converted to v2 and then PUT to the API server. @@ -64,7 +66,7 @@ export class K8sDashboardV2API dashboard.metadata.annotations[AnnoKeyFolderUrl] = folder.url; } catch (e) { // If user has access to dashboard but not to folder, continue without folder info - if (getStatusFromError(e) !== 403) { + if (getStatusFromError(e) !== 403 && getStatusFromError(e) !== 500) { throw new Error('Failed to load folder'); } } diff --git a/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx b/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx index 360510b613..dea1a33850 100644 --- a/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx +++ b/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx @@ -8,6 +8,7 @@ import { Dashboard } from '@grafana/schema'; import appEvents from 'app/core/app_events'; import { useAppNotification } from 'app/core/copy/appNotification'; import { updateDashboardName } from 'app/core/reducers/navBarTree'; +import { contextSrv } from 'app/core/services/context_srv'; import { useSaveDashboardMutation } from 'app/features/browse-dashboards/api/browseDashboardsAPI'; import { DashboardModel } from 'app/features/dashboard/state/DashboardModel'; import { DashboardInteractions } from 'app/features/dashboard-scene/utils/interactions'; @@ -72,6 +73,9 @@ export const useDashboardSave = (isCopy = false) => { const newUrl = locationUtil.stripBaseFromUrl(result.url); if (newUrl !== currentPath && result.url) { + // Refresh permissions before redirect to avoid "not allowed to view" on new dashboards. + // The save creates new permission entries that the cached user permissions don't include yet. + await contextSrv.fetchUserPermissions(); setTimeout(() => locationService.replace(newUrl)); } if (dashboard.meta.isStarred) { diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx index e26b1099d2..ac766634c8 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx @@ -169,6 +169,7 @@ describe('DashboardPageProxy', () => { }); it('should not render DashboardScenePage if route is Normal and has uid', async () => { + jest.spyOn(console, 'error').mockImplementation(); getDashboardScenePageStateManager().setDashboardCache('abc-def', dashMockEditable); act(() => { setup({