Skip to content

Commit

Permalink
Log group config errors to the pod and refine logic
Browse files Browse the repository at this point in the history
  • Loading branch information
DaoDaoNoCode committed Oct 6, 2023
1 parent 00a6200 commit b80669d
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 53 deletions.
79 changes: 41 additions & 38 deletions backend/src/routes/api/groups-config/groupsConfigUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,12 @@ const SYSTEM_AUTHENTICATED = 'system:authenticated';
export const getGroupsConfig = async (fastify: KubeFastifyInstance): Promise<GroupsConfig> => {
const customObjectsApi = fastify.kube.customObjectsApi;

try {
const groupsCluster = await getAllGroups(customObjectsApi);
const groupsData = getGroupsCR();
const groupsProcessed = processGroupData(groupsData);
const groupsConfigProcessed = processGroupConfig(groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, groupsConfigProcessed.groupsCRData);

return groupsConfigProcessed.groupsConfig;
} catch (e) {
fastify.log.error(e, 'Error retrieving group configuration.');
const error = createError(500, 'Error retrieving group configuration');
throw error;
}
const groupsCluster = await getAllGroups(customObjectsApi);
const groupsData = getGroupsCR();
const groupsProcessed = processGroupData(groupsData);
const groupsConfigProcessed = processGroupConfig(fastify, groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, groupsConfigProcessed.groupsCRData);
return groupsConfigProcessed.groupsConfig;
};

const transformGroupsConfig = (groupStatus: GroupStatus[]): string[] => {
Expand All @@ -38,7 +31,7 @@ const transformGroupsConfig = (groupStatus: GroupStatus[]): string[] => {
export const updateGroupsConfig = async (
fastify: KubeFastifyInstance,
request: FastifyRequest<{ Body: GroupsConfig }>,
): Promise<{ success: GroupsConfig | null; error: string | null }> => {
): Promise<GroupsConfig> => {
const customObjectsApi = fastify.kube.customObjectsApi;
const { namespace } = fastify.kube;

Expand All @@ -58,26 +51,17 @@ export const updateGroupsConfig = async (
const error = createError(403, 'Error, groups cannot be empty');
throw error;
}
try {
const dataUpdated: GroupsConfigBody = {
adminGroups: adminConfig.join(','),
allowedGroups: allowedConfig.join(','),
};

const groupsData = await updateGroupsCR(fastify, dataUpdated);
const groupsProcessed = processGroupData(groupsData);
const groupsCluster = await getAllGroups(customObjectsApi);
const updatedConfig = processGroupConfig(groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, updatedConfig.groupsCRData);
return {
success: updatedConfig.groupsConfig,
error: null,
};
} catch (e) {
fastify.log.error(e, 'Error updating group configuration.');
const error = createError(500, 'Error updating group configuration');
throw error;
}
const dataUpdated: GroupsConfigBody = {
adminGroups: adminConfig.join(','),
allowedGroups: allowedConfig.join(','),
};

const groupsData = await updateGroupsCR(fastify, dataUpdated);
const groupsProcessed = processGroupData(groupsData);
const groupsCluster = await getAllGroups(customObjectsApi);
const updatedConfig = processGroupConfig(fastify, groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, updatedConfig.groupsCRData);
return updatedConfig.groupsConfig;
};

const processGroupData = (groupsData: GroupsConfigBody): GroupsConfigBodyList => {
Expand Down Expand Up @@ -105,6 +89,7 @@ const mapListToGroupStatus =
* @returns Processed object with the groups, removing missing groups that might be selected
*/
const processGroupConfig = (
fastify: KubeFastifyInstance,
groupsDataList: GroupsConfigBodyList,
groups: string[],
): { groupsConfig: GroupsConfig; groupsCRData: GroupsConfigBody } => {
Expand All @@ -120,9 +105,14 @@ const processGroupConfig = (
const groupsConfig: GroupsConfig = {
adminGroups: adminGroupsConfig,
allowedGroups: allowedGroupsConfig,
errorAdmin: getError(groupsDataList.adminGroups, (group) => !groups.includes(group)),
errorAdmin: getError(
fastify,
groupsDataList.adminGroups.filter((group) => group),
(group) => !groups.includes(group),
),
errorUser: getError(
groupsDataList.allowedGroups,
fastify,
groupsDataList.allowedGroups.filter((group) => group),
(group) => !groups.includes(group) && group !== SYSTEM_AUTHENTICATED,
),
};
Expand All @@ -137,13 +127,26 @@ const processGroupConfig = (
return { groupsConfig, groupsCRData: updatedBody };
};

const getError = (array: string[], predicate: (group: string) => boolean): string | undefined => {
const getError = (
fastify: KubeFastifyInstance,
array: string[],
predicate: (group: string) => boolean,
): string | undefined => {
let error;
if (array.length === 0) {
error = 'No group is set in the group config, please set one or more group.';
fastify.log.error(error);
return error;
}

const missingItems = array.filter(predicate);
if (missingItems.length === 0) return undefined;

return `The group${missingItems.length === 1 ? '' : 's'} ${missingItems.join(
error = `The group${missingItems.length === 1 ? '' : 's'} ${missingItems.join(
', ',
)} no longer exists in OpenShift and has been removed from the selected group list.`;
fastify.log.error(error);
return error;
};

/**
Expand Down
9 changes: 7 additions & 2 deletions backend/src/routes/api/groups-config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@ import { secureAdminRoute } from '../../../utils/route-security';
export default async (fastify: FastifyInstance): Promise<void> => {
fastify.get(
'/',
secureAdminRoute(fastify)(async () => {
return getGroupsConfig(fastify);
secureAdminRoute(fastify)(async (request, reply) => {
return getGroupsConfig(fastify).catch((e) => {
fastify.log.error(
`Error retrieving group configuration, ${e.response?.body?.message || e.message}`,
);
reply.status(500).send({ message: e.response?.body?.message || e.message });
});
}),
);

Expand Down
2 changes: 1 addition & 1 deletion backend/src/utils/groupsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class MissingGroupError extends Error {
}

export const getGroupsCR = (): GroupsConfigBody => {
if (typeof getDashboardConfig().spec.groupsConfig !== 'undefined') {
if (getDashboardConfig().spec.groupsConfig) {
return getDashboardConfig().spec.groupsConfig;
}
throw new Error(`Failed to retrieve Dashboard CR groups configuration`);
Expand Down
4 changes: 1 addition & 3 deletions frontend/src/services/groupSettingsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ export const fetchGroupsSettings = (): Promise<GroupsConfig> => {
});
};

export const updateGroupsSettings = (
settings: GroupsConfig,
): Promise<{ success: GroupsConfig | null; error: string | null }> => {
export const updateGroupsSettings = (settings: GroupsConfig): Promise<GroupsConfig> => {
const url = '/api/groups-config';
return axios
.put(url, settings)
Expand Down
34 changes: 34 additions & 0 deletions frontend/src/utilities/__tests__/useWatchGroups.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { testHook } from '~/__tests__/unit/testUtils/hooks';
import { fetchGroupsSettings } from '~/services/groupSettingsService';
import { useWatchGroups } from '~/utilities/useWatchGroups';

jest.mock('~/services/groupSettingsService', () => ({
fetchGroupsSettings: jest.fn(),
}));

jest.mock('react-redux', () => ({
useDispatch: jest.fn(),
}));

const fetchGroupSettingsMock = fetchGroupsSettings as jest.Mock;

describe('useWatchGroups', () => {
it('should fetch groups successfully', async () => {
const mockEmptyGroupSettings = {
adminGroups: [],
allowedGroups: [],
};
const mockGroupSettings = {
adminGroups: ['odh-admins'],
allowedGroups: [],
};
fetchGroupSettingsMock.mockReturnValue(Promise.resolve(mockGroupSettings));

const renderResult = testHook(useWatchGroups)();
expect(fetchGroupSettingsMock).toHaveBeenCalledTimes(1);
expect(renderResult.result.current.groupSettings).toStrictEqual(mockEmptyGroupSettings);

await renderResult.waitForNextUpdate();
expect(renderResult.result.current.groupSettings).toStrictEqual(mockGroupSettings);
});
});
16 changes: 7 additions & 9 deletions frontend/src/utilities/useWatchGroups.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,24 +48,22 @@ export const useWatchGroups = (): {

React.useEffect(() => {
if (errorAdmin) {
notification.error(`Group no longer exists`, errorAdmin);
notification.error(`Group error`, errorAdmin);
}
if (errorUser) {
notification.error(`Group no longer exists`, errorUser);
notification.error(`Group error`, errorUser);
}
}, [errorAdmin, errorUser, notification]);

const updateGroups = (group: GroupsConfig) => {
setIsLoading(true);
updateGroupsSettings(group)
.then((response) => {
if (response.success) {
setGroupSettings(response.success);
notification.success(
'Group settings changes saved',
'It may take up to 2 minutes for configuration changes to be applied.',
);
}
setGroupSettings(response);
notification.success(
'Group settings changes saved',
'It may take up to 2 minutes for configuration changes to be applied.',
);
})
.catch((error) => {
setLoadError(error);
Expand Down

0 comments on commit b80669d

Please sign in to comment.