Repository: mageddo/dns-proxy-server
Pull request #443 (2024-05-08 12:22:56)

Feature toggle to turn off Host Machine IP fallback
Fixe #442 


Patch:
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index a0400cc3..512c6d93 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -1,3 +1,6 @@
+## 3.17.0 
+* Feature toggle to turn off Docker Solver Host Machine IP fallback, see #442
+
 ## 3.16.3
 * Fixed NPE when no IPV6 is found to answer to the client #428
 
diff --git a/docs/content/3-configuration/_index.en.md b/docs/content/3-configuration/_index.en.md
index f734a155..45a53089 100755
--- a/docs/content/3-configuration/_index.en.md
+++ b/docs/content/3-configuration/_index.en.md
@@ -119,6 +119,16 @@ Response code to use when no entries are returned by the configured solvers. Def
 ### Local Entries Solving (LocalDB)
 See [Local Entries Solving][2] docs.
 
+### Docker Solver Host Machine IP Fallback
+
+Whether should answer host machine IP when a matching container is found but it hasn't
+an IP to be answered, see Github Issue [#442](https://github.com/mageddo/dns-proxy-server/issues/442).
+Default: true
+
+| Env                                           | JSON                                    | Terminal  |
+|-----------------------------------------------|-----------------------------------------|-----------|
+| MG_DOCKER_SOLVER_HOST_MACHINE_FALLBACK_ACTIVE | `dockerSolverHostMachineFallbackActive` | See -help |
+
 ## Example JSON configuration
 
 __Version 2__
@@ -156,7 +166,8 @@ __Version 2__
   "dockerHost": null,
   "resolvConfOverrideNameServers": true,
   "noRemoteServers": false,
-  "noEntriesResponseCode": 3
+  "noEntriesResponseCode": 3,
+  "dockerSolverHostMachineFallbackActive": true
 }
 ```
 
diff --git a/gradle.properties b/gradle.properties
index 4f738183..286eaee6 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -1,1 +1,1 @@
-version=3.16.3-snapshot
+version=3.17.0-snapshot
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/Config.java b/src/main/java/com/mageddo/dnsproxyserver/config/Config.java
index 3ebf46c4..2bd3bc60 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/Config.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/Config.java
@@ -89,6 +89,8 @@ public class Config {
 
   private int noEntriesResponseCode;
 
+  private boolean dockerSolverHostMachineFallbackActive;
+
   public void resetConfigFile() {
     try {
       Files.deleteIfExists(this.getConfigPath());
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/Configs.java b/src/main/java/com/mageddo/dnsproxyserver/config/Configs.java
index 681d0ea0..0f4a738a 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/Configs.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/Configs.java
@@ -74,6 +74,11 @@ public class Configs {
       .noEntriesResponseCode(firstNonNullRequiring(
         env.getNoEntriesResponseCode(), json.getNoEntriesResponseCode(), flag.getNoEntriesResponseCode()
       ))
+      .dockerSolverHostMachineFallbackActive(firstNonNullRequiring(
+        env.getDockerSolverHostMachineFallbackActive(),
+        json.getDockerSolverHostMachineFallbackActive(),
+        flag.getDockerSolverHostMachineFallbackActive()
+      ))
       .build();
   }
 
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigEnv.java b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigEnv.java
index 11c5e307..e2cba77a 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigEnv.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigEnv.java
@@ -45,6 +45,8 @@ public class ConfigEnv {
 
   private static final String MG_NO_ENTRIES_RESPONSE_CODE = "MG_NO_ENTRIES_RESPONSE_CODE";
 
+  private static final String MG_DOCKER_SOLVER_HOST_MACHINE_FALLBACK_ACTIVE = "MG_DOCKER_SOLVER_HOST_MACHINE_FALLBACK_ACTIVE";
+
   private Path currentPath;
   private String resolvConfPath;
   private String logFile;
@@ -58,6 +60,7 @@ public class ConfigEnv {
   private Boolean resolvConfOverrideNameServers;
   private Boolean noRemoteServers;
   private Integer noEntriesResponseCode;
+  private Boolean dockerSolverHostMachineFallbackActive;
 
   public static ConfigEnv fromEnv() {
     return ConfigEnv
@@ -75,6 +78,7 @@ public class ConfigEnv {
       .resolvConfOverrideNameServers(Envs.getBooleanOrNull(MG_RESOLVCONF_OVERRIDE_NAMESERVERS))
       .noRemoteServers(Envs.getBooleanOrNull(MG_NO_REMOTE_SERVERS))
       .noEntriesResponseCode(Envs.getIntegerOrNull(MG_NO_ENTRIES_RESPONSE_CODE))
+      .dockerSolverHostMachineFallbackActive(Envs.getBooleanOrNull(MG_DOCKER_SOLVER_HOST_MACHINE_FALLBACK_ACTIVE))
       .build();
   }
 
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigFlag.java b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigFlag.java
index e052ce5a..6971fabd 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigFlag.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigFlag.java
@@ -173,6 +173,16 @@ public class ConfigFlag implements Callable<Boolean> {
   )
   private Boolean help;
 
+  @Option(
+    names = {"-docker-solver-host-machine-fallback", "--docker-solver-host-machine-fallback"},
+    description = """
+      Whether should answer host machine IP when a matching container is found but it hasn't
+      an IP to be answered. See Github Issue #442
+      """,
+    defaultValue = "true"
+  )
+  private Boolean dockerSolverHostMachineFallbackActive;
+
   @JsonIgnore
   private String[] args;
 
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJson.java b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJson.java
index d37f8b14..3f0ac205 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJson.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJson.java
@@ -44,4 +44,6 @@ public interface ConfigJson {
   Boolean getNoRemoteServers();
 
   Integer getNoEntriesResponseCode();
+
+  Boolean getDockerSolverHostMachineFallbackActive();
 }
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV1.java b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV1.java
index 35b2595d..73215580 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV1.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV1.java
@@ -13,6 +13,11 @@ import lombok.experimental.Accessors;
 import java.net.URI;
 import java.util.List;
 
+/**
+ * @deprecated it won't be extended, so it makes no sense to maintain it,
+ * a roadmap to remove that feature is necessary.
+ */
+@Deprecated
 @Data
 public class ConfigJsonV1 implements ConfigJson {
 
@@ -97,6 +102,11 @@ public class ConfigJsonV1 implements ConfigJson {
     return null;
   }
 
+  @Override
+  public Boolean getDockerSolverHostMachineFallbackActive() {
+    return null;
+  }
+
   public ConfigJsonV2 toConfigV2() {
     return new ConfigJsonV2()
       .setDomain(this.getDomain())
diff --git a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV2.java b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV2.java
index 7a57f443..3c33e0a3 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV2.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/config/entrypoint/ConfigJsonV2.java
@@ -59,6 +59,8 @@ public class ConfigJsonV2 implements ConfigJson {
 
   private Integer noEntriesResponseCode;
 
+  private Boolean dockerSolverHostMachineFallbackActive;
+
   @JsonIgnore
   public List<IpAddr> getRemoteDnsServers() {
     return this.remoteDnsServers
@@ -73,6 +75,11 @@ public class ConfigJsonV2 implements ConfigJson {
     return ConfigJsonV2EnvsConverter.toDomainEnvs(this._envs);
   }
 
+  @Override
+  public Boolean getDockerSolverHostMachineFallbackActive() {
+    return this.dockerSolverHostMachineFallbackActive;
+  }
+
   @Data
   @Accessors(chain = true)
   public static class Env {
diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/ContainerSolvingService.java b/src/main/java/com/mageddo/dnsproxyserver/docker/ContainerSolvingService.java
index 0fc04f88..afd32740 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/docker/ContainerSolvingService.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/docker/ContainerSolvingService.java
@@ -1,6 +1,7 @@
 package com.mageddo.dnsproxyserver.docker;
 
 import com.github.dockerjava.api.command.InspectContainerResponse;
+import com.mageddo.dnsproxyserver.config.Configs;
 import com.mageddo.dnsproxyserver.server.dns.solver.HostnameQuery;
 import com.mageddo.net.IP;
 import com.mageddo.net.Networks;
@@ -27,6 +28,12 @@ import static com.mageddo.dnsproxyserver.docker.domain.Network.BRIDGE;
 import static com.mageddo.dnsproxyserver.docker.domain.Network.DPS;
 import static com.mageddo.dnsproxyserver.docker.domain.Network.HOST;
 
+/**
+ * Todo that's an application service with is high coupled to infrastructure docker adapter,
+ *      the docker api classes  must be isolated to a port then that port be used on that service.
+ *      See hexagonal architecture.
+ */
+
 @Slf4j
 @Default
 @Singleton
@@ -70,12 +77,12 @@ public class ContainerSolvingService {
     return this.findBestIpMatch(
       inspect,
       buildNetworks(inspect),
-      () -> mapOrNull(this.dockerDAO.findHostMachineIp(version), IP::toText),
+      () -> mapOrNull(this.dockerDAO.findHostMachineIp(version), IP::toText), // todo there is no need to pass it as an argument, just build this supplier when it is needled
       version
     );
   }
 
-  public String findBestIpMatch(
+  String findBestIpMatch(
     InspectContainerResponse c,
     Collection<String> networksNames,
     Supplier<String> hostMachineSup,
@@ -128,16 +135,26 @@ public class ContainerSolvingService {
       .orElseGet(() -> {
         return Optional
           .ofNullable(buildDefaultIp(c, version))
-          .orElseGet(() -> {
-            final var hostIp = hostMachineSup.get();
-            log.debug("status=noNetworkAvailable, usingHostMachineIp={}", hostIp);
-            return hostIp;
-          });
+          .orElseGet(() -> buildHostMachineIpWhenActive(hostMachineSup));
       })
       ;
 
   }
 
+  String buildHostMachineIpWhenActive(Supplier<String> hostMachineSup) {
+    if(isDockerSolverHostMachineFallbackActive()){
+      final var hostIp = hostMachineSup.get();
+      log.debug("status=noNetworkAvailable, usingHostMachineIp={}", hostIp);
+      return hostIp;
+    }
+    log.debug("dockerSolverHostMachineFallback=inactive");
+    return null;
+  }
+
+  boolean isDockerSolverHostMachineFallbackActive() {
+    return Configs.getInstance().isDockerSolverHostMachineFallbackActive();
+  }
+
   static String buildDefaultIp(InspectContainerResponse c, IP.Version version) {
     final var settings = c.getNetworkSettings();
     if (settings == null) {


Test patch:
diff --git a/src/test/java/com/mageddo/dnsproxyserver/docker/ContainerSolvingServiceTest.java b/src/test/java/com/mageddo/dnsproxyserver/docker/ContainerSolvingServiceTest.java
new file mode 100644
index 00000000..22811a98
--- /dev/null
+++ b/src/test/java/com/mageddo/dnsproxyserver/docker/ContainerSolvingServiceTest.java
@@ -0,0 +1,86 @@
+package com.mageddo.dnsproxyserver.docker;
+
+import com.mageddo.net.IP;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Spy;
+import org.mockito.junit.jupiter.MockitoExtension;
+import testing.templates.IpTemplates;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static testing.templates.docker.InspectContainerResponseTemplates.ngixWithDefaultBridgeNetworkOnly;
+
+
+@ExtendWith(MockitoExtension.class)
+class ContainerSolvingServiceTest {
+
+  @Mock
+  DockerDAO dockerDAO;
+
+  @Mock
+  DockerNetworkDAO networkDAO;
+
+  @Mock
+  MatchingContainerService matchingContainerService;
+
+  @Spy
+  @InjectMocks
+  ContainerSolvingService containerSolvingService;
+
+  @Test
+  void mustReturnHostMachineIPWhenThereIsNoBetterMatch() {
+
+    // arrange
+    final var inspect = ngixWithDefaultBridgeNetworkOnly();
+    final var version = IP.Version.IPV6;
+    final var expectedIp = IP.of(IpTemplates.LOCAL_IPV6);
+
+    doReturn(expectedIp)
+      .when(this.dockerDAO)
+      .findHostMachineIp(eq(version))
+    ;
+
+    doReturn(true)
+      .when(this.containerSolvingService)
+      .isDockerSolverHostMachineFallbackActive()
+    ;
+
+    // act
+    final var ip = this.containerSolvingService.findBestIpMatch(inspect, version);
+
+    // assert
+    assertNotNull(ip);
+    assertEquals(expectedIp.toText(), ip);
+
+  }
+
+  @Test
+  void mustReturnNoIPWhenHostMachineFallbackIsDisabled() {
+
+    // arrange
+    final var inspect = ngixWithDefaultBridgeNetworkOnly();
+    final var version = IP.Version.IPV6;
+
+    doReturn(false)
+      .when(this.containerSolvingService)
+      .isDockerSolverHostMachineFallbackActive()
+    ;
+
+    // act
+    final var ip = this.containerSolvingService.findBestIpMatch(inspect, version);
+
+    // assert
+    assertNull(ip);
+    verify(this.dockerDAO, never()).findHostMachineIp(eq(version));
+
+  }
+
+}
diff --git a/src/test/resources/config-json-v1-test/002.json b/src/test/resources/config-json-v1-test/002.json
index 24c3876c..acbb2051 100644
--- a/src/test/resources/config-json-v1-test/002.json
+++ b/src/test/resources/config-json-v1-test/002.json
@@ -3,6 +3,7 @@
   "defaultDns" : null,
   "dnsServerPort" : 8980,
   "dockerHost" : null,
+  "dockerSolverHostMachineFallbackActive" : null,
   "domain" : null,
   "dpsNetwork" : null,
   "dpsNetworkAutoConnect" : null,
diff --git a/src/test/resources/configs-test/001.json b/src/test/resources/configs-test/001.json
index 422bbfc7..3721382d 100644
--- a/src/test/resources/configs-test/001.json
+++ b/src/test/resources/configs-test/001.json
@@ -1,6 +1,7 @@
 {
   "defaultDns" : true,
   "dnsServerPort" : 53,
+  "dockerSolverHostMachineFallbackActive" : true,
   "domain" : "docker",
   "dpsNetwork" : false,
   "dpsNetworkAutoConnect" : false,
diff --git a/src/test/resources/configs-test/002.json b/src/test/resources/configs-test/002.json
index 9b6160ca..76802aef 100644
--- a/src/test/resources/configs-test/002.json
+++ b/src/test/resources/configs-test/002.json
@@ -3,6 +3,7 @@
   "defaultDns" : null,
   "dnsServerPort" : null,
   "dockerHost" : null,
+  "dockerSolverHostMachineFallbackActive" : null,
   "domain" : null,
   "dpsNetwork" : null,
   "dpsNetworkAutoConnect" : null,
diff --git a/src/test/resources/configs-test/004.json b/src/test/resources/configs-test/004.json
index 0074ada2..0cda85d8 100644
--- a/src/test/resources/configs-test/004.json
+++ b/src/test/resources/configs-test/004.json
@@ -1,6 +1,7 @@
 {
   "defaultDns" : false,
   "dnsServerPort" : 5391,
+  "dockerSolverHostMachineFallbackActive" : true,
   "domain" : "acme",
   "dpsNetwork" : true,
   "dpsNetworkAutoConnect" : true,
diff --git a/src/test/resources/flags-test/001.json b/src/test/resources/flags-test/001.json
index 8b60ef3a..0cbb5490 100644
--- a/src/test/resources/flags-test/001.json
+++ b/src/test/resources/flags-test/001.json
@@ -3,6 +3,7 @@
   "webServerPort" : 5380,
   "dnsServerPort" : 53,
   "dockerHost" : null,
+  "dockerSolverHostMachineFallbackActive" : true,
   "defaultDns" : true,
   "configPath" : "conf/config.json",
   "service" : null,
diff --git a/src/test/resources/flags-test/002.txt b/src/test/resources/flags-test/002.txt
index db7d5dc7..e771a8bd 100644
--- a/src/test/resources/flags-test/002.txt
+++ b/src/test/resources/flags-test/002.txt
@@ -1,9 +1,9 @@
-Usage: <main class> [-default-dns] [-dps-network] [-dps-network-auto-connect] [-help] [-no-remote-servers]
-                    [-register-container-names] [-resolvconf-override-name-servers] [-service-publish-web-port]
-                    [-version] [-conf-path=<configPath>] [-docker-host=<dockerHost>] [-domain=<domain>]
-                    [-host-machine-hostname=<hostMachineHostname>] [-log-file=<logToFile>] [-log-level=<logLevel>]
-                    [-no-entries-response-code=<noEntriesResponseCode>] [-server-port=<dnsServerPort>]
-                    [-service=<service>] [-web-server-port=<webServerPort>]
+Usage: <main class> [-default-dns] [-docker-solver-host-machine-fallback] [-dps-network] [-dps-network-auto-connect]
+                    [-help] [-no-remote-servers] [-register-container-names] [-resolvconf-override-name-servers]
+                    [-service-publish-web-port] [-version] [-conf-path=<configPath>] [-docker-host=<dockerHost>]
+                    [-domain=<domain>] [-host-machine-hostname=<hostMachineHostname>] [-log-file=<logToFile>]
+                    [-log-level=<logLevel>] [-no-entries-response-code=<noEntriesResponseCode>]
+                    [-server-port=<dnsServerPort>] [-service=<service>] [-web-server-port=<webServerPort>]
       -conf-path, --conf-path=<configPath>
                             The config file path (default conf/config.json)
       -default-dns, --default-dns
@@ -12,6 +12,10 @@ Usage: <main class> [-default-dns] [-dps-network] [-dps-network-auto-connect] [-
                             The docker host address.
                             (default the default docker host value based on the OS)
 
+      -docker-solver-host-machine-fallback, --docker-solver-host-machine-fallback
+                            Whether should answer host machine IP when a matching container is found but it hasn't
+                            an IP to be answered. See Github Issue #442
+
       -domain, --domain=<domain>
                             Domain utilized to solver containers and services hostnames (default docker)
       -dps-network, --dps-network
diff --git a/src/test/resources/json-configs-test/001.json b/src/test/resources/json-configs-test/001.json
index 960a23eb..b682a129 100644
--- a/src/test/resources/json-configs-test/001.json
+++ b/src/test/resources/json-configs-test/001.json
@@ -3,6 +3,7 @@
   "defaultDns": null,
   "dnsServerPort": 8980,
   "dockerHost" : null,
+  "dockerSolverHostMachineFallbackActive" : null,
   "domain": null,
   "dpsNetwork": null,
   "dpsNetworkAutoConnect": null,