Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react';
import { PluginInfoEntry } from '@openshift/dynamic-plugin-sdk';
import { Alert, Button } from '@patternfly/react-core';
import { PencilAltIcon } from '@patternfly/react-icons/dist/esm/icons/pencil-alt-icon';
import {
Expand Down Expand Up @@ -30,7 +31,7 @@ import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watc
import { KebabAction } from '@console/internal/components/utils/kebab';
import { asAccessReview, RequireCreatePermission } from '@console/internal/components/utils/rbac';
import { ResourceLink } from '@console/internal/components/utils/resource-link';
import { EmptyBox, LoadingBox } from '@console/internal/components/utils/status-box';
import { EmptyBox } from '@console/internal/components/utils/status-box';
import { ConsoleOperatorConfigModel, ConsolePluginModel } from '@console/internal/models';
import {
ConsolePluginKind,
Expand All @@ -39,11 +40,6 @@ import {
referenceForModel,
} from '@console/internal/module/k8s';
import { RootState } from '@console/internal/redux';
import {
isLoadedDynamicPluginInfo,
DynamicPluginInfo,
isNotLoadedDynamicPluginInfo,
} from '@console/plugin-sdk/src';
import { usePluginInfo } from '@console/plugin-sdk/src/api/usePluginInfo';
import PaneBody from '@console/shared/src/components/layout/PaneBody';
import { consolePluginModal } from '@console/shared/src/components/modals/ConsolePluginModal';
Expand Down Expand Up @@ -94,7 +90,7 @@ export const useConsoleOperatorConfigData = () => {
export const ConsolePluginStatus: React.FC<ConsolePluginStatusProps> = ({
status,
errorMessage,
}) => <Status status={status} title={status === 'Failed' ? errorMessage : undefined} />;
}) => <Status status={status} title={status === 'failed' ? errorMessage : undefined} />;

export const ConsolePluginEnabledStatus: React.FC<ConsolePluginEnabledStatusProps> = ({
pluginName,
Expand Down Expand Up @@ -159,7 +155,7 @@ export const ConsolePluginCSPStatus: React.FC<ConsolePluginCSPStatusProps> = ({
);
};

const ConsolePluginsTable: React.FC<ConsolePluginsTableProps> = ({ obj, rows, loaded }) => {
const ConsolePluginsTable: React.FC<ConsolePluginsTableProps> = ({ obj, rows }) => {
const { t } = useTranslation();

const [sortBy, setSortBy] = React.useState<ISortBy>(() => ({
Expand Down Expand Up @@ -223,9 +219,7 @@ const ConsolePluginsTable: React.FC<ConsolePluginsTableProps> = ({ obj, rows, lo

const sortedRows = React.useMemo(() => rows.sort(compare), [rows, compare]);

return !loaded ? (
<LoadingBox />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loaded prop value comes from usePluginInfo hook's second tuple element

const [pluginInfoEntries, allPluginsProcessed] = usePluginInfo();

which is true once all Console plugins have finished loading, i.e. there are no plugins in "pending" state.

So currently <ConsolePluginsTable> renders a loading state until all plugins are done loading.

I think it makes more sense for <ConsolePluginsTable> to render plugin information regardless of the plugin state so 👍 on this change.

) : (
return (
<PaneBody>
{obj.spec?.managementState === 'Unmanaged' && (
<Alert
Expand Down Expand Up @@ -300,30 +294,30 @@ const ConsolePluginsTable: React.FC<ConsolePluginsTableProps> = ({ obj, rows, lo
};

const DevPluginsPage: React.FCC<ConsoleOperatorConfigPageProps> = (props) => {
const [pluginInfo, pluginInfoLoaded] = usePluginInfo();
const pluginInfo = usePluginInfo();
const cspViolations = useSelector<RootState, PluginCSPViolations>(({ UI }) =>
UI.get('pluginCSPViolations'),
);

const rows = React.useMemo<ConsolePluginTableRow[]>(
() =>
!pluginInfoLoaded
? []
: pluginInfo.filter(isLoadedDynamicPluginInfo).map((plugin) => ({
name: plugin.metadata.name,
version: plugin.metadata.version,
description: plugin.metadata?.customProperties?.console?.description,
enabled: plugin.enabled,
status: plugin.status,
hasCSPViolations: cspViolations[plugin.metadata.name] ?? false,
})),
[pluginInfo, pluginInfoLoaded, cspViolations],
pluginInfo
.filter((plugin) => plugin.status === 'loaded')
.map((plugin) => ({
name: plugin.metadata.name,
version: plugin.metadata.version,
description: plugin.metadata?.customProperties?.console?.description,
enabled: plugin.enabled,
status: plugin.status,
hasCSPViolations: cspViolations[plugin.metadata.name] ?? false,
})),
[pluginInfo, cspViolations],
);
return <ConsolePluginsTable {...props} rows={rows} loaded={pluginInfoLoaded} />;
return <ConsolePluginsTable {...props} rows={rows} />;
};

const PluginsPage: React.FC<ConsoleOperatorConfigPageProps> = (props) => {
const [pluginInfo] = usePluginInfo();
const pluginInfo = usePluginInfo();
const [consolePlugins, consolePluginsLoaded] = useK8sWatchResource<ConsolePluginKind[]>({
isList: true,
kind: referenceForModel(ConsolePluginModel),
Expand All @@ -342,10 +336,10 @@ const PluginsPage: React.FC<ConsoleOperatorConfigPageProps> = (props) => {
const pluginName = plugin?.metadata?.name;
const enabled = enabledPlugins.includes(pluginName);
const loadedPluginInfo = pluginInfo
.filter(isLoadedDynamicPluginInfo)
.filter((p) => p.status === 'loaded')
.find((i) => i?.metadata?.name === pluginName);
const notLoadedPluginInfo = pluginInfo
.filter(isNotLoadedDynamicPluginInfo)
.filter((p) => p.status !== 'loaded')
.find((i) => i?.pluginName === pluginName);
if (loadedPluginInfo) {
return {
Expand All @@ -362,15 +356,15 @@ const PluginsPage: React.FC<ConsoleOperatorConfigPageProps> = (props) => {
enabled,
status: notLoadedPluginInfo?.status,
errorMessage:
notLoadedPluginInfo?.status === 'Failed' ? notLoadedPluginInfo?.errorMessage : undefined,
notLoadedPluginInfo?.status === 'failed' ? notLoadedPluginInfo?.errorMessage : undefined,
errorCause:
notLoadedPluginInfo?.status === 'Failed'
notLoadedPluginInfo?.status === 'failed'
? notLoadedPluginInfo?.errorCause?.toString()
: undefined,
};
});
}, [consolePluginsLoaded, consolePlugins, pluginInfo, enabledPlugins, cspViolations]);
return <ConsolePluginsTable {...props} rows={rows} loaded={consolePluginsLoaded} />;
return <ConsolePluginsTable {...props} rows={rows} />;
};

const ConsoleOperatorConfigPluginsPage: React.FC<ConsoleOperatorConfigPageProps> = developmentMode
Expand Down Expand Up @@ -425,7 +419,7 @@ export type ConsolePluginTableRow = {
name: string;
version?: string;
description?: string;
status: DynamicPluginInfo['status'];
status: PluginInfoEntry['status'];
enabled: boolean;
errorMessage?: string;
hasCSPViolations?: boolean;
Expand All @@ -439,11 +433,10 @@ type TableColumn = {

type ConsolePluginsTableProps = ConsoleOperatorConfigPageProps & {
rows: ConsolePluginTableRow[];
loaded: boolean;
};

type ConsolePluginStatusProps = {
status: DynamicPluginInfo['status'];
status: PluginInfoEntry['status'];
errorMessage?: string;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
import * as React from 'react';
import { DetailsItemComponentProps } from '@console/dynamic-plugin-sdk/src/extensions/details-item';
import { isLoadedDynamicPluginInfo } from '@console/plugin-sdk/src';
import { usePluginInfo } from '@console/plugin-sdk/src/api/usePluginInfo';
import { DASH } from '@console/shared/src/constants';

const ConsolePluginDescriptionDetail: React.FC<DetailsItemComponentProps> = ({ obj }) => {
const pluginName = React.useMemo(() => obj?.metadata?.name, [obj?.metadata?.name]);

const [pluginInfoEntries] = usePluginInfo();
const pluginInfoEntries = usePluginInfo();
const pluginInfo = React.useMemo(
() =>
pluginInfoEntries.find((entry) =>
isLoadedDynamicPluginInfo(entry)
entry.status === 'loaded'
? entry.metadata.name === pluginName
: entry.pluginName === pluginName,
),
[pluginInfoEntries, pluginName],
);

return isLoadedDynamicPluginInfo(pluginInfo) ? (
return pluginInfo?.status === 'loaded' ? (
<>{pluginInfo.metadata.customProperties?.console?.description || DASH}</>
) : (
<>{DASH}</>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import { DASH } from '@console/dynamic-plugin-sdk/src/app/constants';
import { DetailsItemComponentProps } from '@console/dynamic-plugin-sdk/src/extensions/details-item';
import { isLoadedDynamicPluginInfo } from '@console/plugin-sdk/src';
import { usePluginInfo } from '@console/plugin-sdk/src/api/usePluginInfo';
import {
ConsolePluginEnabledStatus,
Expand All @@ -10,15 +9,15 @@ import {
} from './ConsoleOperatorConfig';

const ConsolePluginEnabledStatusDetail: React.FC<DetailsItemComponentProps> = ({ obj }) => {
const [pluginInfoEntries] = usePluginInfo();
const pluginInfoEntries = usePluginInfo();
const { consoleOperatorConfig, consoleOperatorConfigLoaded } = useConsoleOperatorConfigData();

const pluginName = React.useMemo(() => obj?.metadata?.name, [obj?.metadata?.name]);

const pluginInfo = React.useMemo(
() =>
pluginInfoEntries.find((entry) =>
isLoadedDynamicPluginInfo(entry)
entry.status === 'loaded'
? entry.metadata.name === pluginName
: entry.pluginName === pluginName,
),
Expand All @@ -28,12 +27,12 @@ const ConsolePluginEnabledStatusDetail: React.FC<DetailsItemComponentProps> = ({
consoleOperatorConfig?.spec?.plugins,
]);

return consoleOperatorConfigLoaded ? (
return consoleOperatorConfigLoaded && pluginName ? (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All k8s objects should have a name, do we really need to check up on pluginName explicitly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have null checks for pluginName, so in theory this could be null. This just prevents some linting errors

<ConsolePluginEnabledStatus
pluginName={pluginName}
enabled={
developmentMode
? (isLoadedDynamicPluginInfo(pluginInfo) && pluginInfo.enabled) ?? false
? (pluginInfo?.status === 'loaded' && pluginInfo.enabled) ?? false
: enabledPlugins.includes(pluginName) ?? false
}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import * as React from 'react';
import { DetailsItemComponentProps } from '@console/dynamic-plugin-sdk/src/extensions/details-item';
import { isLoadedDynamicPluginInfo } from '@console/plugin-sdk/src';
import { usePluginInfo } from '@console/plugin-sdk/src/api/usePluginInfo';
import { DASH } from '@console/shared/src/constants';
import { ConsolePluginStatus } from './ConsoleOperatorConfig';

const ConsolePluginStatusDetail: React.FC<DetailsItemComponentProps> = ({ obj }) => {
const [pluginInfoEntries] = usePluginInfo();
const pluginInfoEntries = usePluginInfo();
const pluginName = React.useMemo(() => obj?.metadata?.name, [obj?.metadata?.name]);

const pluginInfo = React.useMemo(
() =>
pluginInfoEntries.find((entry) =>
isLoadedDynamicPluginInfo(entry)
entry.status === 'loaded'
? entry.metadata.name === pluginName
: entry.pluginName === pluginName,
),
Expand All @@ -22,7 +21,7 @@ const ConsolePluginStatusDetail: React.FC<DetailsItemComponentProps> = ({ obj })
return pluginInfo ? (
<ConsolePluginStatus
status={pluginInfo.status}
errorMessage={pluginInfo.status === 'Failed' ? pluginInfo.errorMessage : undefined}
errorMessage={pluginInfo.status === 'failed' ? pluginInfo.errorMessage : undefined}
/>
) : (
<>{DASH}</>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
import * as React from 'react';
import { DetailsItemComponentProps } from '@console/dynamic-plugin-sdk/src/extensions/details-item';
import { isLoadedDynamicPluginInfo } from '@console/plugin-sdk/src';
import { usePluginInfo } from '@console/plugin-sdk/src/api/usePluginInfo';
import { DASH } from '@console/shared/src/constants';

const ConsolePluginVersionDetail: React.FC<DetailsItemComponentProps> = ({ obj }) => {
const [pluginInfoEntries] = usePluginInfo();
const pluginInfoEntries = usePluginInfo();

const pluginName = React.useMemo(() => obj?.metadata?.name, [obj?.metadata?.name]);

const pluginInfo = React.useMemo(
() =>
pluginInfoEntries.find((entry) =>
isLoadedDynamicPluginInfo(entry)
entry.status === 'loaded'
? entry.metadata.name === pluginName
: entry.pluginName === pluginName,
),
[pluginInfoEntries, pluginName],
);

return isLoadedDynamicPluginInfo(pluginInfo) ? <>{pluginInfo.metadata.version}</> : <>{DASH}</>;
return pluginInfo?.status === 'loaded' ? <>{pluginInfo.metadata.version}</> : <>{DASH}</>;
};

export default ConsolePluginVersionDetail;
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,22 @@ import { PluginCSPViolations } from '@console/internal/actions/ui';
import { ConsoleOperatorConfigModel, ConsolePluginModel } from '@console/internal/models';
import { ConsolePluginKind, referenceForModel } from '@console/internal/module/k8s';
import { RootState } from '@console/internal/redux';
import { isLoadedDynamicPluginInfo, isNotLoadedDynamicPluginInfo } from '@console/plugin-sdk/src';
import { usePluginInfo } from '@console/plugin-sdk/src/api/usePluginInfo';
import { StatusPopupSection } from '@console/shared/src/components/dashboard/status-card/StatusPopup';
import NotLoadedDynamicPlugins from './NotLoadedDynamicPlugins';

const DynamicPluginsPopover: React.FC<DynamicPluginsPopoverProps> = ({ consolePlugins }) => {
const { t } = useTranslation();
const [pluginInfoEntries] = usePluginInfo();
const pluginInfoEntries = usePluginInfo();
const cspViolations = useSelector<RootState, PluginCSPViolations>(({ UI }) =>
UI.get('pluginCSPViolations'),
);
const notLoadedDynamicPluginInfo = pluginInfoEntries.filter(isNotLoadedDynamicPluginInfo);
const failedPlugins = notLoadedDynamicPluginInfo.filter((plugin) => plugin.status === 'Failed');
const pendingPlugins = notLoadedDynamicPluginInfo.filter((plugin) => plugin.status === 'Pending');
const loadedPlugins = pluginInfoEntries.filter(isLoadedDynamicPluginInfo);
const notLoadedDynamicPluginInfo = pluginInfoEntries.filter(
(plugin) => plugin.status !== 'loaded',
);
const failedPlugins = notLoadedDynamicPluginInfo.filter((plugin) => plugin.status === 'failed');
const pendingPlugins = notLoadedDynamicPluginInfo.filter((plugin) => plugin.status === 'pending');
const loadedPlugins = pluginInfoEntries.filter((plugin) => plugin.status === 'loaded');
const loadedPluginsWithCSPViolations = loadedPlugins.filter(
(plugin) => cspViolations[plugin.metadata.name] ?? false,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import { pluginStore } from '@console/internal/plugins';

export const getDynamicPluginHealthState = (): SubsystemHealth => {
const dynamicPluginInfo = pluginStore.getPluginInfo();
if (dynamicPluginInfo.some((plugin) => plugin.status === 'Failed')) {
if (dynamicPluginInfo.some((plugin) => plugin.status === 'failed')) {
return { state: HealthState.ERROR };
}
if (dynamicPluginInfo.some((plugin) => plugin.status === 'Pending')) {
if (dynamicPluginInfo.some((plugin) => plugin.status === 'pending')) {
return { state: HealthState.PROGRESS };
}
if (dynamicPluginInfo.every((plugin) => plugin.status === 'Loaded')) {
if (dynamicPluginInfo.every((plugin) => plugin.status === 'loaded')) {
return { state: HealthState.OK };
}
return { state: HealthState.UNKNOWN };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { useTranslation } from 'react-i18next';
import { useDispatch, useSelector } from 'react-redux';
import { setPluginCSPViolations, PluginCSPViolations } from '@console/internal/actions/ui';
import { RootState } from '@console/internal/redux';
import { isLoadedDynamicPluginInfo } from '@console/plugin-sdk/src';
import { usePluginStore } from '@console/plugin-sdk/src/api/usePluginStore';
import { useToast } from '@console/shared/src/components/toast';
import { IS_PRODUCTION } from '@console/shared/src/constants/common';
Expand Down Expand Up @@ -104,13 +103,13 @@ export const useCSPViolationDetector = () => {
const pluginInfo = pluginStore
.getPluginInfo()
.find((entry) =>
isLoadedDynamicPluginInfo(entry)
entry.status === 'loaded'
? entry.metadata.name === pluginName
: entry.pluginName === pluginName,
);

const validPlugin = !!pluginInfo;
const pluginIsLoaded = validPlugin && pluginInfo.status === 'Loaded';
const pluginIsLoaded = validPlugin && pluginInfo.status === 'loaded';

// eslint-disable-next-line no-console
console.warn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('resolvePluginDependencies', () => {
const getLoadedDynamicPluginInfo = (
manifest: StandardConsolePluginManifest,
): LoadedDynamicPluginInfo => ({
status: 'Loaded',
status: 'loaded',
pluginID: getPluginID(manifest),
metadata: _.omit(manifest, ['extensions', 'loadScripts', 'registrationMethod']),
enabled: true,
Expand All @@ -37,14 +37,14 @@ describe('resolvePluginDependencies', () => {
const getPendingDynamicPluginInfo = (
manifest: StandardConsolePluginManifest,
): NotLoadedDynamicPluginInfo => ({
status: 'Pending',
status: 'pending',
pluginName: manifest.name,
});

const getFailedDynamicPluginInfo = (
manifest: StandardConsolePluginManifest,
): NotLoadedDynamicPluginInfo => ({
status: 'Failed',
status: 'failed',
pluginName: manifest.name,
errorMessage: `Test error message for plugin ${manifest.name}`,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,14 @@ export const resolvePluginDependencies = (

const unsubListener = subscribeToDynamicPlugins((entries) => {
const loadedPlugins = entries.reduce<Record<string, string>>((acc, e) => {
if (e.status === 'Loaded' && preloadPluginNames.includes(e.metadata.name)) {
if (e.status === 'loaded' && preloadPluginNames.includes(e.metadata.name)) {
acc[e.metadata.name] = e.metadata.version;
}
return acc;
}, {});

const failedPluginNames = entries.reduce<string[]>((acc, e) => {
if (e.status === 'Failed' && preloadPluginNames.includes(e.pluginName)) {
if (e.status === 'failed' && preloadPluginNames.includes(e.pluginName)) {
acc.push(e.pluginName);
}
return acc;
Expand Down
Loading