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

Allow configuring the network name for project-based containers #561

Merged
merged 12 commits into from
Mar 29, 2023
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-561.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: feature
feature:
description: Projects using DockerProxyExtension for project-based containers can
now override the docker network name used by the proxy
links:
- https://github.com/palantir/docker-proxy-rule/pull/561
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,24 @@ public static DockerProxyExtension fromProjectName(ProjectName projectName, Clas
public static DockerProxyExtension fromProjectName(
ProjectName projectName, Class<?> classToLogFor, String imageNameOverride) {
return new DockerProxyExtension(
docker -> new ProjectBasedDockerContainerInfo(docker, projectName, Optional.of(imageNameOverride)),
docker -> new ProjectBasedDockerContainerInfo(
docker, projectName, Optional.empty(), Optional.of(imageNameOverride)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

These parameters are the wrong way around I think

classToLogFor);
}

/**
* Creates a {@link DockerProxyExtension} using a {@link ProjectBasedDockerContainerInfo}.
*
* @param projectName The docker-compose-rule ProjectName to use to find the containers
* @param networkNameOverride The docker network name to use instead of the default
* @param imageNameOverride The docker image name to use instead of the default
* @param classToLogFor The class using {@link DockerProxyExtension}
*/
public static DockerProxyExtension fromProjectName(
ProjectName projectName, Class<?> classToLogFor, String imageNameOverride, String networkNameOverride) {
return new DockerProxyExtension(
docker -> new ProjectBasedDockerContainerInfo(
docker, projectName, Optional.of(networkNameOverride), Optional.of(imageNameOverride)),
classToLogFor);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,22 @@
public final class ProjectBasedDockerContainerInfo implements DockerContainerInfo {
private final DockerExecutable docker;
private final ProjectName projectName;
private final Optional<String> networkNameOverride;
private final Optional<String> imageNameOverride;

public ProjectBasedDockerContainerInfo(
DockerExecutable docker, ProjectName projectName, Optional<String> imageNameOverride) {
DockerExecutable docker,
ProjectName projectName,
Optional<String> networkNameOverride,
Optional<String> imageNameOverride) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also do the same here to keep the constructors consistent

this.docker = docker;
this.projectName = projectName;
this.networkNameOverride = networkNameOverride;
this.imageNameOverride = imageNameOverride;
}

public ProjectBasedDockerContainerInfo(DockerExecutable docker, ProjectName projectName) {
this(docker, projectName, Optional.empty());
this(docker, projectName, Optional.empty(), Optional.empty());
}

@Override
Expand Down Expand Up @@ -62,7 +67,7 @@ public Optional<String> getHostForIp(String ip) {

@Override
public String getNetworkName() {
return projectName.asString() + "_default";
return networkNameOverride.orElseGet(() -> projectName.asString() + "_default");
}

@Override
Expand Down