From e2a1d59a960218817e114dfaa1270a2a7a7bd530 Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Mon, 19 Aug 2024 14:53:12 +0100 Subject: [PATCH] Alerting: Remove disable logic for name field on time intervals when using k8s API (#91885) Co-authored-by: Gilles De Mey --- .../alerting/unified/MuteTimings.test.tsx | 17 ++++++------- .../mute-timings/MuteTimingActionsButtons.tsx | 2 +- .../mute-timings/MuteTimingForm.tsx | 9 ------- .../mute-timings/useMuteTimings.tsx | 15 +++++++----- .../server/handlers/k8s/timeIntervals.k8s.ts | 24 +++++++++++++++---- .../mocks/server/handlers/k8s/utils.ts | 20 ++++++++++++++++ .../unified/openapi/timeIntervalsApi.gen.ts | 17 ------------- scripts/generate-alerting-rtk-apis.ts | 1 - 8 files changed, 57 insertions(+), 48 deletions(-) create mode 100644 public/app/features/alerting/unified/mocks/server/handlers/k8s/utils.ts diff --git a/public/app/features/alerting/unified/MuteTimings.test.tsx b/public/app/features/alerting/unified/MuteTimings.test.tsx index 788d64b3dcb..3a9bb12b262 100644 --- a/public/app/features/alerting/unified/MuteTimings.test.tsx +++ b/public/app/features/alerting/unified/MuteTimings.test.tsx @@ -13,8 +13,8 @@ import { import { captureRequests } from 'app/features/alerting/unified/mocks/server/events'; import { MOCK_DATASOURCE_EXTERNAL_VANILLA_ALERTMANAGER_UID } from 'app/features/alerting/unified/mocks/server/handlers/datasources'; import { - TIME_INTERVAL_UID_HAPPY_PATH, - TIME_INTERVAL_UID_FILE_PROVISIONED, + TIME_INTERVAL_NAME_HAPPY_PATH, + TIME_INTERVAL_NAME_FILE_PROVISIONED, } from 'app/features/alerting/unified/mocks/server/handlers/k8s/timeIntervals.k8s'; import { setupDataSources } from 'app/features/alerting/unified/testSetup/datasources'; import { AlertManagerCortexConfig, MuteTimeInterval } from 'app/plugins/datasource/alertmanager/types'; @@ -185,7 +185,7 @@ const fillOutForm = async ({ const saveMuteTiming = async () => { const user = userEvent.setup(); - await user.click(screen.getByText(/save mute timing/i)); + await user.click(await screen.findByText(/save mute timing/i)); }; setupMswServer(); @@ -365,7 +365,7 @@ describe('Mute timings', () => { }); it('allows creation of new mute timings', async () => { - await renderMuteTimings({ + renderMuteTimings({ pathname: '/alerting/routes/mute-timing/new', }); @@ -378,7 +378,7 @@ describe('Mute timings', () => { it('shows error when mute timing does not exist', async () => { renderMuteTimings({ pathname: '/alerting/routes/mute-timing/edit', - search: `?alertmanager=${GRAFANA_RULES_SOURCE_NAME}&muteName=${TIME_INTERVAL_UID_HAPPY_PATH + '_force_breakage'}`, + search: `?alertmanager=${GRAFANA_RULES_SOURCE_NAME}&muteName=${TIME_INTERVAL_NAME_HAPPY_PATH + '_force_breakage'}`, }); expect(await screen.findByText(/No matching mute timing found/i)).toBeInTheDocument(); @@ -387,12 +387,9 @@ describe('Mute timings', () => { it('loads edit form correctly and allows saving', async () => { renderMuteTimings({ pathname: '/alerting/routes/mute-timing/edit', - search: `?alertmanager=${GRAFANA_RULES_SOURCE_NAME}&muteName=${TIME_INTERVAL_UID_HAPPY_PATH}`, + search: `?alertmanager=${GRAFANA_RULES_SOURCE_NAME}&muteName=${TIME_INTERVAL_NAME_HAPPY_PATH}`, }); - // For now, we expect the name field to be disabled editing via the k8s API - expect(await ui.nameField.find()).toBeDisabled(); - await saveMuteTiming(); await expectedToHaveRedirectedToRoutesRoute(); }); @@ -400,7 +397,7 @@ describe('Mute timings', () => { it('loads view form for provisioned interval', async () => { renderMuteTimings({ pathname: '/alerting/routes/mute-timing/edit', - search: `?muteName=${TIME_INTERVAL_UID_FILE_PROVISIONED}`, + search: `?muteName=${TIME_INTERVAL_NAME_FILE_PROVISIONED}`, }); expect(await screen.findByText(/This mute timing cannot be edited through the UI/i)).toBeInTheDocument(); diff --git a/public/app/features/alerting/unified/components/mute-timings/MuteTimingActionsButtons.tsx b/public/app/features/alerting/unified/components/mute-timings/MuteTimingActionsButtons.tsx index 05ce0335f86..6b69ffdd90d 100644 --- a/public/app/features/alerting/unified/components/mute-timings/MuteTimingActionsButtons.tsx +++ b/public/app/features/alerting/unified/components/mute-timings/MuteTimingActionsButtons.tsx @@ -30,7 +30,7 @@ export const MuteTimingActionsButtons = ({ muteTiming, alertManagerSourceName }: const isGrafanaDataSource = alertManagerSourceName === GRAFANA_RULES_SOURCE_NAME; const viewOrEditHref = makeAMLink(`/alerting/routes/mute-timing/edit`, alertManagerSourceName, { - muteName: muteTiming?.metadata?.name || muteTiming.name, + muteName: muteTiming.id, }); const viewOrEditButton = ( diff --git a/public/app/features/alerting/unified/components/mute-timings/MuteTimingForm.tsx b/public/app/features/alerting/unified/components/mute-timings/MuteTimingForm.tsx index df15dd85679..9768cf4b95e 100644 --- a/public/app/features/alerting/unified/components/mute-timings/MuteTimingForm.tsx +++ b/public/app/features/alerting/unified/components/mute-timings/MuteTimingForm.tsx @@ -11,7 +11,6 @@ import { useUpdateMuteTiming, useValidateMuteTiming, } from 'app/features/alerting/unified/components/mute-timings/useMuteTimings'; -import { shouldUseK8sApi } from 'app/features/alerting/unified/utils/k8s/utils'; import { useAlertmanager } from '../../state/AlertmanagerContext'; import { MuteTimingFields } from '../../types/mute-timing-form'; @@ -65,13 +64,6 @@ const MuteTimingForm = ({ muteTiming, showError, loading, provisioned, editMode const [updateTimeInterval] = useUpdateMuteTiming(hookArgs); const validateMuteTiming = useValidateMuteTiming(hookArgs); - /** - * The k8s API approach does not support renaming an entity at this time, - * as it requires renaming all other references of this entity. - * - * For now, the cleanest solution is to disabled renaming the field in this scenario - */ - const disableNameField = editMode && shouldUseK8sApi(selectedAlertmanager!); const styles = useStyles2(getStyles); const defaultValues = useDefaultValues(muteTiming); @@ -115,7 +107,6 @@ const MuteTimingForm = ({ muteTiming, showError, loading, provisioned, editMode description="A unique name for the mute timing" invalid={!!formApi.formState.errors?.name} error={formApi.formState.errors.name?.message} - disabled={disableNameField} > { export const useGetMuteTiming = ({ alertmanager, name: nameToFind }: BaseAlertmanagerArgs & { name: string }) => { const useK8sApi = shouldUseK8sApi(alertmanager); - const [getGrafanaTimeInterval, k8sResponse] = useLazyReadNamespacedTimeIntervalQuery({ + const [getGrafanaTimeInterval, k8sResponse] = useLazyListNamespacedTimeIntervalQuery({ selectFromResult: ({ data, ...rest }) => { if (!data) { return { data, ...rest }; } + if (data.items.length === 0) { + return { ...rest, data: undefined, isError: true }; + } + return { - data: parseK8sTimeInterval(data), + data: parseK8sTimeInterval(data.items[0]), ...rest, }; }, @@ -192,7 +195,7 @@ export const useGetMuteTiming = ({ alertmanager, name: nameToFind }: BaseAlertma useEffect(() => { if (useK8sApi) { const namespace = getK8sNamespace(); - getGrafanaTimeInterval({ namespace, name: nameToFind }, true); + getGrafanaTimeInterval({ namespace, fieldSelector: `spec.name=${nameToFind}` }, true); } else { getAlertmanagerTimeInterval(alertmanager, true); } diff --git a/public/app/features/alerting/unified/mocks/server/handlers/k8s/timeIntervals.k8s.ts b/public/app/features/alerting/unified/mocks/server/handlers/k8s/timeIntervals.k8s.ts index c87d9b89d93..126423077d7 100644 --- a/public/app/features/alerting/unified/mocks/server/handlers/k8s/timeIntervals.k8s.ts +++ b/public/app/features/alerting/unified/mocks/server/handlers/k8s/timeIntervals.k8s.ts @@ -1,13 +1,18 @@ import { HttpResponse, http } from 'msw'; +import { filterBySelector } from 'app/features/alerting/unified/mocks/server/handlers/k8s/utils'; import { ALERTING_API_SERVER_BASE_URL, getK8sResponse } from 'app/features/alerting/unified/mocks/server/utils'; import { ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TimeInterval } from 'app/features/alerting/unified/openapi/timeIntervalsApi.gen'; import { PROVENANCE_ANNOTATION, PROVENANCE_NONE } from 'app/features/alerting/unified/utils/k8s/constants'; /** UID of a time interval that we expect to follow all happy paths within tests/mocks */ export const TIME_INTERVAL_UID_HAPPY_PATH = 'f4eae7a4895fa786'; +/** Display name of a time interval that we expect to follow all happy paths within tests/mocks */ +export const TIME_INTERVAL_NAME_HAPPY_PATH = 'Some interval'; + /** UID of a (file) provisioned time interval */ export const TIME_INTERVAL_UID_FILE_PROVISIONED = 'd7b8515fc39e90f7'; +export const TIME_INTERVAL_NAME_FILE_PROVISIONED = 'A provisioned interval'; const allTimeIntervals = getK8sResponse( 'TimeIntervalList', @@ -22,7 +27,7 @@ const allTimeIntervals = getK8sResponse { }; export const listNamespacedTimeIntervalHandler = () => - http.get<{ namespace: string }>( + http.get<{ namespace: string }, { fieldSelector: string }>( `${ALERTING_API_SERVER_BASE_URL}/namespaces/:namespace/timeintervals`, - ({ params }) => { + ({ params, request }) => { const { namespace } = params; // k8s APIs expect `default` rather than `org-1` - this is one particular example @@ -59,6 +64,17 @@ export const listNamespacedTimeIntervalHandler = () => { status: 403 } ); } + + // Rudimentary filter support for `spec.name` + const url = new URL(request.url); + const fieldSelector = url.searchParams.get('fieldSelector'); + + if (fieldSelector && fieldSelector.includes('spec.name')) { + const filteredItems = filterBySelector(allTimeIntervals.items, fieldSelector); + + return HttpResponse.json({ items: filteredItems }); + } + return HttpResponse.json(allTimeIntervals); } ); diff --git a/public/app/features/alerting/unified/mocks/server/handlers/k8s/utils.ts b/public/app/features/alerting/unified/mocks/server/handlers/k8s/utils.ts new file mode 100644 index 00000000000..610f1559824 --- /dev/null +++ b/public/app/features/alerting/unified/mocks/server/handlers/k8s/utils.ts @@ -0,0 +1,20 @@ +import { chain, filter, matchesProperty, trim } from 'lodash'; + +/** + * Filters a list of k8s items by a selector string + */ +export function filterBySelector(items: T[], selector: string) { + // e.g. [['path.to.key', 'value'], ['other.path', 'value']] + const filters: string[][] = chain(selector) + .split(',') + .map(trim) + .map((s) => s.split('=')) + .value(); + + return filter(items, (item) => + filters.every(([key, value]) => { + const matcher = matchesProperty(key, value); + return matcher(item); + }) + ); +} diff --git a/public/app/features/alerting/unified/openapi/timeIntervalsApi.gen.ts b/public/app/features/alerting/unified/openapi/timeIntervalsApi.gen.ts index 68dc3434f20..42eb90bf6ff 100644 --- a/public/app/features/alerting/unified/openapi/timeIntervalsApi.gen.ts +++ b/public/app/features/alerting/unified/openapi/timeIntervalsApi.gen.ts @@ -42,13 +42,6 @@ const injectedRtkApi = api }), invalidatesTags: ['TimeInterval'], }), - readNamespacedTimeInterval: build.query({ - query: (queryArg) => ({ - url: `/apis/notifications.alerting.grafana.app/v0alpha1/namespaces/${queryArg['namespace']}/timeintervals/${queryArg.name}`, - params: { pretty: queryArg.pretty }, - }), - providesTags: ['TimeInterval'], - }), replaceNamespacedTimeInterval: build.mutation< ReplaceNamespacedTimeIntervalApiResponse, ReplaceNamespacedTimeIntervalApiArg @@ -171,16 +164,6 @@ export type CreateNamespacedTimeIntervalApiArg = { fieldValidation?: string; comGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TimeInterval: ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TimeInterval; }; -export type ReadNamespacedTimeIntervalApiResponse = - /** status 200 OK */ ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TimeInterval; -export type ReadNamespacedTimeIntervalApiArg = { - /** name of the TimeInterval */ - name: string; - /** object name and auth scope, such as for teams and projects */ - namespace: string; - /** If 'true', then the output is pretty printed. Defaults to 'false' unless the user-agent indicates a browser or command-line HTTP tool (curl and wget). */ - pretty?: string; -}; export type ReplaceNamespacedTimeIntervalApiResponse = /** status 200 OK */ | ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TimeInterval | /** status 201 Created */ ComGithubGrafanaGrafanaPkgApisAlertingNotificationsV0Alpha1TimeInterval; diff --git a/scripts/generate-alerting-rtk-apis.ts b/scripts/generate-alerting-rtk-apis.ts index 7727619b0f9..561bde193b8 100644 --- a/scripts/generate-alerting-rtk-apis.ts +++ b/scripts/generate-alerting-rtk-apis.ts @@ -31,7 +31,6 @@ const config: ConfigFile = { filterEndpoints: [ 'listNamespacedTimeInterval', 'createNamespacedTimeInterval', - 'readNamespacedTimeInterval', 'deleteNamespacedTimeInterval', 'patchNamespacedTimeInterval', 'replaceNamespacedTimeInterval',