Open
Conversation
- Added components for handling and displaying Docker networks, including creation, editing, and listing of networks. - Introduced a new API router for network operations, integrating with the database schema for network management. - Updated the sidebar layout to include a link to the networks dashboard, ensuring user access to network features. - Created necessary database migrations for the network table and its associated types. - Enhanced the dashboard layout to support the new network management interface.
- Eliminated the unused DOKPLOY_SERVER_VALUE constant from the network handling component. - Updated the default serverId to undefined in the network form values and adjusted related logic to ensure compatibility with cloud environments. - Enhanced server validation in the createNetwork function to require a serverId when in cloud mode, improving error handling for network creation.
- Introduced a new column "networkIds" of type text[] with a default value of an empty array to the application, compose, mariadb, mongo, mysql, postgres, and redis tables. - Updated the application and compose schemas to include the new networkIds field in their respective TypeScript definitions. - Added corresponding entries in the migration journal and created a snapshot for version 7 to reflect these changes.
- Deleted the SQL files for the "network" table and its associated constraints, as well as the snapshots for versions 0146 and 0147, to clean up the database schema and remove unused components. - Updated the migration journal to reflect these deletions, ensuring the database remains consistent and manageable.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+105
to
+119
| export const removeNetwork = async (networkId: string) => { | ||
| const [deleted] = await db | ||
| .delete(network) | ||
| .where(eq(network.networkId, networkId)) | ||
| .returning(); | ||
|
|
||
| if (!deleted) { | ||
| throw new TRPCError({ | ||
| code: "NOT_FOUND", | ||
| message: "Network not found", | ||
| }); | ||
| } | ||
|
|
||
| return deleted; | ||
| }; |
Contributor
There was a problem hiding this comment.
Deletes from database but doesn't remove the actual Docker network via docker.getNetwork(networkName).remove(). This will leave orphaned networks in Docker.
Suggested change
| export const removeNetwork = async (networkId: string) => { | |
| const [deleted] = await db | |
| .delete(network) | |
| .where(eq(network.networkId, networkId)) | |
| .returning(); | |
| if (!deleted) { | |
| throw new TRPCError({ | |
| code: "NOT_FOUND", | |
| message: "Network not found", | |
| }); | |
| } | |
| return deleted; | |
| }; | |
| export const removeNetwork = async (networkId: string) => { | |
| const networkData = await findNetworkById(networkId); | |
| const docker = await getRemoteDocker(networkData.serverId ?? null); | |
| await docker.getNetwork(networkData.name).remove(); | |
| const [deleted] = await db | |
| .delete(network) | |
| .where(eq(network.networkId, networkId)) | |
| .returning(); | |
| if (!deleted) { | |
| throw new TRPCError({ | |
| code: "NOT_FOUND", | |
| message: "Network not found", | |
| }); | |
| } | |
| return deleted; | |
| }; |
Comment on lines
+87
to
+103
| export const updateNetwork = async (input: typeof apiUpdateNetwork._type) => { | ||
| const { networkId, ...rest } = input; | ||
| const [updated] = await db | ||
| .update(network) | ||
| .set(rest) | ||
| .where(eq(network.networkId, networkId)) | ||
| .returning(); | ||
|
|
||
| if (!updated) { | ||
| throw new TRPCError({ | ||
| code: "NOT_FOUND", | ||
| message: "Network not found", | ||
| }); | ||
| } | ||
|
|
||
| return updated; | ||
| }; |
Contributor
There was a problem hiding this comment.
Updates database record but doesn't update the actual Docker network. Docker networks are immutable after creation - most properties cannot be changed. Consider either:
- Preventing updates to Docker-related properties (driver, IPAM, etc.), or
- Recreating the network if critical properties change
Note: Only the database name can be updated safely without affecting Docker.
Comment on lines
33
to
39
| if (IS_CLOUD) | ||
| if (!input.serverId) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: "Server is required", | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Missing braces - the if (!input.serverId) check executes regardless of IS_CLOUD value.
Suggested change
| if (IS_CLOUD) | |
| if (!input.serverId) { | |
| throw new TRPCError({ | |
| code: "BAD_REQUEST", | |
| message: "Server is required", | |
| }); | |
| } | |
| if (IS_CLOUD) { | |
| if (!input.serverId) { | |
| throw new TRPCError({ | |
| code: "BAD_REQUEST", | |
| message: "Server is required", | |
| }); | |
| } | |
| } |
Comment on lines
+276
to
+280
| {!isCloud && ( | ||
| <SelectItem value={undefined}> | ||
| Dokploy server | ||
| </SelectItem> | ||
| )} |
Contributor
There was a problem hiding this comment.
SelectItem with value={undefined} may not work as expected in React. Consider using an empty string or a sentinel value like "__local__".
- Added a new "network" table with various attributes including networkId, name, driver, and constraints for organizationId and serverId. - Introduced a custom ENUM type "networkDriver" for network drivers. - Updated existing tables (application, compose, mariadb, mongo, mysql, postgres, redis) to include a new "networkIds" column of type text[]. - Created a snapshot for version 7 to reflect these schema changes in the database.
- Updated the createNetwork function to ensure serverId is required when in cloud mode, improving error handling for network creation. - Added braces for clarity in the conditional statement, enhancing code readability.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR about?
Please describe in a short paragraph what this PR is about.
Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
Screenshots (if applicable)
Greptile Summary
This PR adds Docker network management functionality to Dokploy, enabling users to create, view, and manage Docker networks through the UI. The implementation includes:
networkdatabase table with support for Docker network properties (driver types, IPAM config, IPv4/IPv6 settings)networkIdscolumns added to application and database service tables for future network assignmentCritical Issues Found:
removeNetwork()only deletes the database record but doesn't remove the actual Docker network, which will leave orphaned networksupdateNetwork()only updates the database without modifying the Docker network (most Docker network properties are immutable after creation)IS_CLOUDconditional causes server validation to execute incorrectlyConfidence Score: 2/5
removeNetworkandupdateNetworkfunctions that don't synchronize with Docker, and a syntax error in the cloud validation logic. These issues will lead to orphaned Docker networks and database/runtime inconsistencies.packages/server/src/services/network.tsrequires immediate attention for Docker API integration in remove/update operationsLast reviewed commit: a8f941b