Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [M3-6021] - Invalidate Firewall devices cache when a Linode is deleted #8848

Merged
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
10 changes: 8 additions & 2 deletions packages/api-v4/src/linodes/linodes.ts
Original file line number Diff line number Diff line change
@@ -145,8 +145,14 @@ export const changeLinodePassword = (linodeId: number, root_pass: string) =>
* View Firewall information for Firewalls associated with this Linode
*/

export const getLinodeFirewalls = (linodeId: number) =>
export const getLinodeFirewalls = (
linodeId: number,
params?: any,
filter?: any
) =>
Request<Page<Firewall>>(
setURL(`${API_ROOT}/linode/instances/${linodeId}/firewalls`),
setMethod('GET')
setMethod('GET'),
setXFilter(filter),
setParams(params)
);
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { FirewallDevice } from '@linode/api-v4/lib/firewalls/types';
import { APIError } from '@linode/api-v4/lib/types';
import * as React from 'react';
import { compose } from 'recompose';
import TableBody from 'src/components/core/TableBody';
import TableHead from 'src/components/core/TableHead';
import OrderBy from 'src/components/OrderBy';
@@ -19,27 +18,16 @@ interface Props {
error?: APIError[];
loading: boolean;
disabled: boolean;
lastUpdated: number;
triggerRemoveDevice: (deviceID: number, deviceLabel: string) => void;
triggerRemoveDevice: (deviceID: number) => void;
}

type CombinedProps = Props;
const FirewallTable = (props: Props) => {
const { devices, error, loading, disabled, triggerRemoveDevice } = props;

const FirewallTable: React.FC<CombinedProps> = (props) => {
const {
devices,
error,
lastUpdated,
loading,
disabled,
triggerRemoveDevice,
} = props;

const _error =
error && lastUpdated === 0
? // @todo change to Devices or make dynamic when NBs are possible as Devices
getAPIErrorOrDefault(error, 'Unable to retrieve Linodes')
: undefined;
const _error = error
? // @todo change to Devices or make dynamic when NBs are possible as Devices
getAPIErrorOrDefault(error, 'Unable to retrieve Linodes')
: undefined;

return (
<OrderBy data={devices} orderBy={'entity:label'} order={'asc'}>
@@ -72,9 +60,8 @@ const FirewallTable: React.FC<CombinedProps> = (props) => {
<TableBody>
<TableContentWrapper
length={paginatedAndOrderedData.length}
loading={loading && lastUpdated === 0}
loading={loading}
error={_error}
lastUpdated={lastUpdated}
>
{paginatedAndOrderedData.map((thisDevice) => (
<FirewallDeviceRow
@@ -105,4 +92,4 @@ const FirewallTable: React.FC<CombinedProps> = (props) => {
);
};

export default compose<CombinedProps, Props>(React.memo)(FirewallTable);
export default React.memo(FirewallTable);
Original file line number Diff line number Diff line change
@@ -11,12 +11,13 @@ import Notice from 'src/components/Notice';
import withLinodes, {
Props as LinodesProps,
} from 'src/containers/withLinodes.container';
import { useDialog } from 'src/hooks/useDialog';
import useFirewallDevices from 'src/hooks/useFirewallDevices';
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
import AddDeviceDrawer from './AddDeviceDrawer';
import FirewallDevicesTable from './FirewallDevicesTable';
import RemoveDeviceDialog from './RemoveDeviceDialog';
import {
useAddFirewallDeviceMutation,
useAllFirewallDevicesQuery,
} from 'src/queries/firewalls';

const useStyles = makeStyles((theme: Theme) => ({
copy: {
@@ -51,37 +52,23 @@ type CombinedProps = RouteComponentProps & Props & LinodesProps;
const FirewallLinodesLanding: React.FC<CombinedProps> = (props) => {
const { firewallID, firewallLabel, disabled } = props;
const classes = useStyles();
const {
devices,
requestDevices,
removeDevice,
addDevice,
} = useFirewallDevices(firewallID);

const deviceList = Object.values(devices.itemsById ?? {}); // Gives the devices as an array or [] if nothing is found
React.useEffect(() => {
if (devices.lastUpdated === 0 && !(devices.loading || devices.error.read)) {
requestDevices();
}
}, [
devices.error,
devices.lastUpdated,
devices.loading,
firewallID,
requestDevices,
]);
const { data: devices, isLoading, error } = useAllFirewallDevicesQuery(
firewallID
);

const { mutateAsync: addDevice } = useAddFirewallDeviceMutation(firewallID);

const [
isRemoveDeviceDialogOpen,
setIsRemoveDeviceDialogOpen,
] = React.useState<boolean>(false);

const {
dialog,
openDialog,
closeDialog,
handleError,
submitDialog,
} = useDialog<number>(removeDevice);
const [selectedDeviceId, setSelectedDeviceId] = React.useState<number>(-1);

const _openDialog = React.useCallback(openDialog, [dialog, openDialog]);
const _closeDialog = React.useCallback(closeDialog, [dialog, closeDialog]);
const _submitDialog = React.useCallback(submitDialog, [dialog, submitDialog]);
const selectedDevice = devices?.find(
(device) => device.id === selectedDeviceId
);

const [addDeviceDrawerOpen, setDeviceDrawerOpen] = React.useState<boolean>(
false
@@ -134,12 +121,6 @@ const FirewallLinodesLanding: React.FC<CombinedProps> = (props) => {
setDeviceSubmitting(false);
};

const handleRemoveDevice = () => {
_submitDialog(dialog.entityID).catch((e) =>
handleError(getAPIErrorOrDefault(e, 'Error removing Device')[0].reason)
);
};

return (
<>
{disabled ? (
@@ -170,30 +151,30 @@ const FirewallLinodesLanding: React.FC<CombinedProps> = (props) => {
</Grid>
</Grid>
<FirewallDevicesTable
devices={deviceList}
error={devices.error.read}
lastUpdated={devices.lastUpdated}
loading={devices.loading}
devices={devices ?? []}
error={error ?? undefined}
loading={isLoading}
disabled={disabled}
triggerRemoveDevice={_openDialog}
triggerRemoveDevice={(id) => {
setSelectedDeviceId(id);
setIsRemoveDeviceDialogOpen(true);
}}
/>
<AddDeviceDrawer
open={addDeviceDrawerOpen}
error={addDeviceError}
isSubmitting={deviceSubmitting}
currentDevices={deviceList.map((thisDevice) => thisDevice.entity.id)}
currentDevices={devices?.map((device) => device.entity.id) ?? []}
firewallLabel={firewallLabel}
onClose={handleClose}
addDevice={handleAddDevice}
/>
<RemoveDeviceDialog
open={dialog.isOpen}
loading={dialog.isLoading}
error={dialog.error}
onClose={_closeDialog}
onRemove={handleRemoveDevice}
deviceLabel={dialog.entityLabel ?? 'this device'}
open={isRemoveDeviceDialogOpen}
onClose={() => setIsRemoveDeviceDialogOpen(false)}
device={selectedDevice}
firewallLabel={firewallLabel}
firewallId={firewallID}
/>
</>
);
Original file line number Diff line number Diff line change
@@ -1,71 +1,66 @@
import { FirewallDevice } from '@linode/api-v4';
import * as React from 'react';
import ActionsPanel from 'src/components/ActionsPanel';
import Button from 'src/components/Button';
import ConfirmationDialog from 'src/components/ConfirmationDialog';
import Typography from 'src/components/core/Typography';
import Notice from 'src/components/Notice';
import { useRemoveFirewallDeviceMutation } from 'src/queries/firewalls';

export interface Props {
open: boolean;
error?: string;
loading: boolean;
deviceLabel: string;
firewallLabel: string;
onClose: () => void;
onRemove: () => void;
device: FirewallDevice | undefined;
firewallLabel: string;
firewallId: number;
}

export const RemoveDeviceDialog: React.FC<Props> = (props) => {
const {
deviceLabel,
error,
firewallLabel,
loading,
onClose,
onRemove,
open,
} = props;
export const RemoveDeviceDialog = (props: Props) => {
const { device, firewallLabel, firewallId, onClose, open } = props;

const { mutateAsync, isLoading, error } = useRemoveFirewallDeviceMutation(
firewallId,
device?.id ?? -1
);

const onDelete = async () => {
await mutateAsync();
onClose();
};

return (
<ConfirmationDialog
title={`Remove ${deviceLabel} from ${firewallLabel}?`}
title={`Remove ${device?.entity.label} from ${firewallLabel}?`}
open={open}
onClose={onClose}
actions={renderActions(loading, onClose, onRemove)}
error={error?.[0]?.reason}
actions={
<ActionsPanel style={{ padding: 0 }}>
<Button
buttonType="secondary"
onClick={onClose}
data-qa-cancel
data-testid={'dialog-cancel'}
>
Cancel
</Button>
<Button
buttonType="primary"
onClick={onDelete}
loading={isLoading}
data-qa-confirm
data-testid={'dialog-confirm'}
>
Remove
</Button>
</ActionsPanel>
}
>
{error && <Notice error text={error} />}
<Typography>
Are you sure you want to remove {deviceLabel} from {firewallLabel}?
Are you sure you want to remove {device?.entity.label} from{' '}
{firewallLabel}?
</Typography>
</ConfirmationDialog>
);
};

const renderActions = (
loading: boolean,
onClose: () => void,
onDelete: () => void
) => {
return (
<ActionsPanel style={{ padding: 0 }}>
<Button
buttonType="secondary"
onClick={onClose}
data-qa-cancel
data-testid={'dialog-cancel'}
>
Cancel
</Button>
<Button
buttonType="primary"
onClick={onDelete}
loading={loading}
data-qa-confirm
data-testid={'dialog-confirm'}
>
Remove
</Button>
</ActionsPanel>
);
};

export default React.memo(RemoveDeviceDialog);
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ import { makeStyles, Theme } from 'src/components/core/styles';
import StatusIcon from 'src/components/StatusIcon';
import TableCell from 'src/components/TableCell';
import TableRow from 'src/components/TableRow';
import useFirewallDevices from 'src/hooks/useFirewallDevices';
import { useAllFirewallDevicesQuery } from 'src/queries/firewalls';
import capitalize from 'src/utilities/capitalize';
import ActionMenu, { ActionHandlers } from './FirewallActionMenu';

@@ -31,17 +31,7 @@ export const FirewallRow: React.FC<CombinedProps> = (props) => {

const { id, label, status, rules, ...actionHandlers } = props;

const {
devices: { itemsById, error, loading, lastUpdated },
requestDevices,
} = useFirewallDevices(id);
const devices = Object.values(itemsById);

React.useEffect(() => {
if (lastUpdated === 0 && !(loading || error.read)) {
requestDevices();
}
}, [error, lastUpdated, loading, requestDevices]);
const { data: devices, isLoading, error } = useAllFirewallDevicesQuery(id);

const count = getCountOfRules(rules);

@@ -63,7 +53,7 @@ export const FirewallRow: React.FC<CombinedProps> = (props) => {
<Hidden smDown>
<TableCell>{getRuleString(count)}</TableCell>
<TableCell>
{getLinodesCellString(devices, loading, error.read)}
{getLinodesCellString(devices ?? [], isLoading, error ?? undefined)}
</TableCell>
</Hidden>
<TableCell actionCell>
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ import Typography from 'src/components/core/Typography';
import Link from 'src/components/Link';
import Notice from 'src/components/Notice';
import { lishLaunch } from 'src/features/Lish/lishUtils';
import { useLinodeFirewalls } from 'src/queries/linodeFirewalls';
import { useLinodeFirewalls } from 'src/queries/linodes';

const rescueDescription = {
text: `If you suspect that your primary filesystem is corrupt, use the Linode Manager to boot your Linode into Rescue Mode. This is a safe environment for performing many system recovery and disk management tasks.`,
Loading