Skip to content

allow CreateContainer to specify NetworkConfig #2556

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.

Sign up for GitHub

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

Open
nbrugger-tgm wants to merge 1 commit into
base: main
Choose a base branch
from
Open

allow CreateContainer to specify NetworkConfig #2556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions docker-java-api/src/main/java/com/github/dockerjava/api/command/CreateContainerCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.github.dockerjava.api.model.Link;
import com.github.dockerjava.api.model.LogConfig;
import com.github.dockerjava.api.model.LxcConf;
import com.github.dockerjava.api.model.NetworkingConfig;
import com.github.dockerjava.api.model.PortBinding;
import com.github.dockerjava.api.model.Ports;
import com.github.dockerjava.api.model.RestartPolicy;
Expand Down Expand Up @@ -209,6 +210,13 @@ default CreateContainerCmd withLinks(List<Link> links) {

CreateContainerCmd withIpv6Address(String ipv6Address);

NetworkingConfig getNetworkingConfig();

/**
* Set the networking configuration for the container.
*/
CreateContainerCmd withNetworkingConfig(NetworkingConfig networkingConfig);

@CheckForNull
Map<String, String> getLabels();

Expand Down
32 changes: 32 additions & 0 deletions docker-java-api/src/main/java/com/github/dockerjava/api/model/NetworkingConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.github.dockerjava.api.model;

import com.fasterxml.jackson.annotation.JsonProperty;

import javax.annotation.CheckForNull;
import java.util.Map;

/**
* Networking configuration for a container.
*/
public class NetworkingConfig {

public NetworkingConfig() {
this.endpointsConfig = new HashMap<>();
}
public NetworkingConfig(Map<String, ContainerNetwork> endpointsConfig) {
this.endpointsConfig = endpointsConfig;
}
Comment on lines +11 to +18
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

NetworkingConfig as added won’t compile and will fail existing architecture/serialization tests: it uses HashMap without importing it, and it does not extend DockerObject nor implement Serializable/serialVersionUID (see docker-java-api ArchUnit test and ModelsSerializableTest which enforce these conventions for com.github.dockerjava.api.model classes). Update the class to follow the existing model conventions (extend DockerObject, implement Serializable, add serialVersionUID, and fix missing imports).

Copilot uses AI. Check for mistakes.

@JsonProperty("EndpointsConfig")
private Map<String, ContainerNetwork> endpointsConfig;

@CheckForNull
public Map<String, ContainerNetwork> getEndpointsConfig() {
return endpointsConfig;
}

public NetworkingConfig withEndpointsConfig(Map<String, ContainerNetwork> endpointsConfig) {
this.endpointsConfig = endpointsConfig;
return this;
}
}
38 changes: 21 additions & 17 deletions ...er-java-core/src/main/java/com/github/dockerjava/core/command/CreateContainerCmdImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.github.dockerjava.api.model.ExposedPorts;
import com.github.dockerjava.api.model.HealthCheck;
import com.github.dockerjava.api.model.HostConfig;
import com.github.dockerjava.api.model.NetworkingConfig;
import com.github.dockerjava.api.model.Volume;
import com.github.dockerjava.api.model.Volumes;
import org.apache.commons.lang3.builder.EqualsBuilder;
Expand Down Expand Up @@ -127,7 +128,7 @@ public class CreateContainerCmdImpl extends AbstrDockerCmd<CreateContainerCmd, C
private List<String> shell;

@JsonProperty("NetworkingConfig")
private NetworkingConfig networkingConfig;
private NetworkingConfig networkingConfig = new NetworkingConfig();

private String ipv4Address = null;

Expand Down Expand Up @@ -544,6 +545,18 @@ public CreateContainerCmd withIpv6Address(String ipv6Address) {
return this;
}

@Override
public NetworkingConfig getNetworkingConfig() {
return networkingConfig;
}

@Override
public CreateContainerCmd withNetworkingConfig(NetworkingConfig networkingConfig) {
Objects.requireNonNull(networkingConfig, "no networkingConfig was specified");
this.networkingConfig = networkingConfig;
return this;
}

@CheckForNull
public List<String> getOnBuild() {
return onBuild;
Expand Down Expand Up @@ -601,8 +614,13 @@ public CreateContainerResponse exec() throws NotFoundException, ConflictExceptio
}

if (containerNetwork != null && hostConfig.getNetworkMode() != null) {
networkingConfig = new NetworkingConfig()
.withEndpointsConfig(singletonMap(hostConfig.getNetworkMode(), containerNetwork));
// If the user explicitly sets networkingConfig or endpointConfig to null, its reasonable to not overwrite it
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

Typo in comment: "its reasonable" should be "it's reasonable".

Suggested change
// If the user explicitly sets networkingConfig or endpointConfig to null, its reasonable to not overwrite it
// If the user explicitly sets networkingConfig or endpointConfig to null, it's reasonable to not overwrite it

Copilot uses AI. Check for mistakes.
if (networkingConfig != null
&& networkingConfig.getEndpointsConfig() != null
&& !networkingConfig.getEndpointsConfig().containsKey(hostConfig.getNetworkMode())
) {
Comment on lines +617 to +621
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

This change adds a new public API surface (withNetworkingConfig/getNetworkingConfig) and changes create-container networking behavior (merging endpoint configs). There are existing integration tests for container networking in CreateContainerCmdIT, but none that assert attaching to multiple networks at creation via NetworkingConfig. Adding a test that creates two networks and verifies the container is attached to both would help prevent regressions.

Suggested change
// If the user explicitly sets networkingConfig or endpointConfig to null, its reasonable to not overwrite it
if (networkingConfig != null
&& networkingConfig.getEndpointsConfig() != null
&& !networkingConfig.getEndpointsConfig().containsKey(hostConfig.getNetworkMode())
) {
// If the user already provided endpointsConfig, merge without overwriting existing entries.
if (networkingConfig == null || networkingConfig.getEndpointsConfig() == null) {
networkingConfig = new NetworkingConfig()
.withEndpointsConfig(singletonMap(hostConfig.getNetworkMode(), containerNetwork));
} else if (!networkingConfig.getEndpointsConfig().containsKey(hostConfig.getNetworkMode())) {

Copilot uses AI. Check for mistakes.
networkingConfig.getEndpointsConfig().put(hostConfig.getNetworkMode(), containerNetwork);
}
Comment on lines 616 to +623
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

singletonMap is no longer used in this class after the networkingConfig logic change. Because the build runs Checkstyle with failOnViolation=true, this unused static import will likely fail CI; remove the now-unused import static java.util.Collections.singletonMap;.

Copilot uses AI. Check for mistakes.
Comment on lines 616 to +623
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

networkingConfig.getEndpointsConfig().put(...) mutates the caller-provided map. If the user supplied an immutable map (e.g., Map.of(...)/Collections.unmodifiableMap(...)), this will throw UnsupportedOperationException at runtime. Consider copying to a mutable map (and setting it back via withEndpointsConfig) before adding/merging the endpoint, and also handle the null endpointsConfig case by creating a new map when you need to add the default network endpoint.

Copilot uses AI. Check for mistakes.
}

return super.exec();
Expand All @@ -623,18 +641,4 @@ public boolean equals(Object o) {
public int hashCode() {
return HashCodeBuilder.reflectionHashCode(this);
}

public static class NetworkingConfig {
@JsonProperty("EndpointsConfig")
public Map<String, ContainerNetwork> endpointsConfig;

public Map<String, ContainerNetwork> getEndpointsConfig() {
return endpointsConfig;
}

public NetworkingConfig withEndpointsConfig(Map<String, ContainerNetwork> endpointsConfig) {
this.endpointsConfig = endpointsConfig;
return this;
}
}
}
Loading