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

Improve port mapping warnings/errors #2020

Merged
merged 6 commits into from
Sep 2, 2024
Merged
Changes from 3 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
Expand Up @@ -16,8 +16,8 @@ import {
PortProtocol
} from "@dappnode/types";

const maxPortNumber = 32768 - 1;
const maxEphemeralPortNumber = 65535;
const maxRegisteredPortNumber = 32768 - 1;
const maxPortNumber = 65535;

export function PortsByService({
dnpName,
Expand Down Expand Up @@ -120,59 +120,101 @@ export function PortsByService({
return conflictingPorts;
}

function getPortsOverTheMax(): PortMapping[] {
// Function to get any container ports in the ephemeral range, excluding port 51820
function getContainerPortsInEphemeralRange(): PortMapping[] {
return ports.filter(
({ host, container, deletable }) =>
(deletable &&
(container > maxPortNumber || (host && host > maxPortNumber))) ||
(host && host > maxEphemeralPortNumber)
({ container, deletable }) =>
deletable && container > maxRegisteredPortNumber && container <= maxPortNumber && container !== 51820
dappnodedev marked this conversation as resolved.
Show resolved Hide resolved
);
}

// Function to get any host ports in the ephemeral range, excluding port 51820
function getHostPortsInEphemeralRange(): PortMapping[] {
return ports.filter(
({ host, deletable }) =>
deletable && host !== undefined && host > maxRegisteredPortNumber && host <= maxPortNumber && host !== 51820
dappnodedev marked this conversation as resolved.
Show resolved Hide resolved
);
}


// Function to get any container ports over the max allowed port number
function getContainerPortsOverTheMax(): PortMapping[] {
return ports.filter(
({ container, deletable }) =>
deletable && container > maxPortNumber
);
}

// Function to get any host ports over the max allowed port number
function getHostPortsOverTheMax(): PortMapping[] {
return ports.filter(
({ host, deletable }) =>
deletable && host !== undefined && host > maxPortNumber
);
}


const areNewMappingsInvalid = ports.some(
({ container, protocol, deletable }) =>
deletable && (!container || !protocol)
);
const duplicatedContainerPorts = getDuplicatedContainerPorts();
const duplicatedHostPorts = getDuplicatedHostPorts();
const conflictingPorts = getConflictingPorts();
const portsOverTheMax = getPortsOverTheMax();
const containerPortsEphimeralRange = getContainerPortsInEphemeralRange();
const hostPortsEphimeralRange = getHostPortsInEphemeralRange();
const containerPortsOverTheMax = getContainerPortsOverTheMax();
const hostPortsOverTheMax = getHostPortsOverTheMax();
const arePortsTheSame = portsToId(portsFromDnp) === portsToId(ports);

// Aggregate error messages as an array of strings
// Aggregate error & warning messages as an array of strings
const errors: string[] = [];
const warnings: string[] = [];
for (const duplicatedHostPort of duplicatedHostPorts)
errors.push(
`Duplicated mapping for host port ${duplicatedHostPort.host}/${duplicatedHostPort.protocol}. Each host port can only be mapped once.`
`Duplicated mapping for host port ${duplicatedHostPort.host}/${duplicatedHostPort.protocol}. Each host port can only be mapped once.`
);

for (const duplicatedContainerPort of duplicatedContainerPorts)
errors.push(
`Duplicated mapping for package port ${duplicatedContainerPort.container}/${duplicatedContainerPort.protocol}. Each package port can only be mapped once.`
`Duplicated mapping for package port ${duplicatedContainerPort.container}/${duplicatedContainerPort.protocol}. Each package port can only be mapped once.`
);

for (const conflictingPort of conflictingPorts) {
const portName = `${conflictingPort.host}/${conflictingPort.protocol}`;
const ownerName = prettyDnpName(conflictingPort.owner);
errors.push(
`Port ${portName} is already mapped by the DAppNode Package ${ownerName}`
`Port ${portName} is already mapped by the DAppNode Package ${ownerName}`
);
}

for (const portOverTheMax of portsOverTheMax)
errors.push(
`Host port mapping ${portOverTheMax.host}/${portOverTheMax.protocol} is in the ephemeral port range (32768-65535). It must be avoided.`
);
// Adjusting the loops for error and warning outputs accordingly
containerPortsOverTheMax.forEach(port => {
errors.push(`❌ Container port mapping ${port.container}/${port.protocol} exceeds the maximum allowed port number of ${maxPortNumber} and can't be used.`);
});

hostPortsOverTheMax.forEach(port => {
errors.push(`❌ Host port mapping ${port.host}/${port.protocol} exceeds the maximum allowed port number of ${maxPortNumber} and can't be used.`);
});

containerPortsEphimeralRange.forEach(port => {
warnings.push(`⚠️ Package Port mapping ${port.container}/${port.protocol} is in the ephemeral port range (${maxRegisteredPortNumber + 1}-${maxPortNumber}).`);
});

hostPortsEphimeralRange.forEach(port => {
warnings.push(`⚠️ Host Port mapping ${port.host}/${port.protocol} is in the ephemeral port range (${maxRegisteredPortNumber + 1}-${maxPortNumber}).`);
});

// Aggregate conditions to disable the update
const disableUpdate = Boolean(
areNewMappingsInvalid ||
duplicatedContainerPorts.length > 0 ||
duplicatedHostPorts.length > 0 ||
conflictingPorts.length > 0 ||
portsOverTheMax.length > 0 ||
arePortsTheSame ||
updating
duplicatedContainerPorts.length > 0 ||
duplicatedHostPorts.length > 0 ||
conflictingPorts.length > 0 ||
arePortsTheSame ||
updating ||
containerPortsOverTheMax.length > 0 ||
hostPortsOverTheMax.length > 0
);

return (
Expand Down Expand Up @@ -210,7 +252,7 @@ export function PortsByService({
<Input
lock={true}
value={container}
onValueChange={() => {}}
onValueChange={() => { }}
/>
)}
</td>
Expand All @@ -230,7 +272,7 @@ export function PortsByService({
<Input
lock={true}
value={protocol}
onValueChange={() => {}}
onValueChange={() => { }}
/>
)}
</td>
Expand All @@ -256,6 +298,12 @@ export function PortsByService({
</div>
))}

{warnings.map(warning => (
<div key={warning}>
{warning}
</div>
))}

<div className="button-row">
<Button
variant={"dappnode"}
Expand Down
Loading