fix(discovery): BUILTIN_DISCOVERY_DISABLED can be specified even when…

… PLATFORM is set (#1865) (#1870) (cherry picked from commit d627360) Co-authored-by: Andrew Azores <[email protected]>
cryostatio · Feb 6, 2024 · 756d5ba · 756d5ba
1 parent ff1f555
commit 756d5ba
Show file tree

Hide file tree

Showing 19 changed files with 120 additions and 46 deletions.
diff --git a/src/main/java/io/cryostat/discovery/BuiltInDiscovery.java b/src/main/java/io/cryostat/discovery/BuiltInDiscovery.java
@@ -74,6 +74,12 @@ public void start(Promise<Void> start) {
                 .distinct()
                 .forEach(
                         platform -> {
+                            if (!platform.isEnabled()) {
+                                logger.info(
+                                        "Skipping built-in discovery with {} : not enabled",
+                                        platform.getClass().getSimpleName());
+                                return;
+                            }
                             logger.info(
                                     "Starting built-in discovery with {}",
                                     platform.getClass().getSimpleName());

diff --git a/src/main/java/io/cryostat/discovery/DiscoveryStorage.java b/src/main/java/io/cryostat/discovery/DiscoveryStorage.java
@@ -120,6 +120,11 @@ public class DiscoveryStorage extends AbstractPlatformClientVerticle {
         this.logger = logger;
     }
 
+    @Override
+    public final boolean isEnabled() {
+        return true;
+    }
+
     @Override
     public void start(Promise<Void> future) throws Exception {
         pingPrune()

diff --git a/src/main/java/io/cryostat/platform/AbstractPlatformClient.java b/src/main/java/io/cryostat/platform/AbstractPlatformClient.java
@@ -19,16 +19,26 @@
 import java.util.Set;
 import java.util.function.Consumer;
 
+import io.cryostat.configuration.Variables;
 import io.cryostat.core.net.discovery.JvmDiscoveryClient.EventKind;
+import io.cryostat.core.sys.Environment;
 
 public abstract class AbstractPlatformClient implements PlatformClient {
 
+    protected final Environment environment;
     protected final Set<Consumer<TargetDiscoveryEvent>> discoveryListeners;
 
-    protected AbstractPlatformClient() {
+    protected AbstractPlatformClient(Environment environment) {
+        this.environment = environment;
         this.discoveryListeners = new HashSet<>();
     }
 
+    @Override
+    public boolean isEnabled() {
+        return !Boolean.parseBoolean(
+                environment.getEnv(Variables.DISABLE_BUILTIN_DISCOVERY, "false"));
+    }
+
     @Override
     public void addTargetDiscoveryListener(Consumer<TargetDiscoveryEvent> listener) {
         this.discoveryListeners.add(listener);

diff --git a/src/main/java/io/cryostat/platform/PlatformClient.java b/src/main/java/io/cryostat/platform/PlatformClient.java
@@ -23,6 +23,9 @@
 import io.vertx.core.Promise;
 
 public interface PlatformClient {
+
+    boolean isEnabled();
+
     default void start() throws Exception {}
 
     default void load(Promise<EnvironmentNode> promise) {

diff --git a/src/main/java/io/cryostat/platform/PlatformModule.java b/src/main/java/io/cryostat/platform/PlatformModule.java
@@ -90,8 +90,6 @@ static Set<PlatformDetectionStrategy<?>> provideSelectedPlatformStrategies(
             List<String> platforms =
                     Arrays.asList(env.getEnv(Variables.PLATFORM_STRATEGY_ENV_VAR).split(","));
             fn = s -> platforms.contains(s.getClass().getCanonicalName());
-        } else if (env.hasEnv(Variables.DISABLE_BUILTIN_DISCOVERY)) {
-            fn = s -> false;
         } else {
             fn = PlatformDetectionStrategy::isAvailable;
         }

diff --git a/src/main/java/io/cryostat/platform/internal/CustomTargetPlatformClient.java b/src/main/java/io/cryostat/platform/internal/CustomTargetPlatformClient.java
@@ -25,6 +25,7 @@
 import java.util.TreeSet;
 
 import io.cryostat.core.net.discovery.JvmDiscoveryClient.EventKind;
+import io.cryostat.core.sys.Environment;
 import io.cryostat.discovery.DiscoveryStorage;
 import io.cryostat.platform.AbstractPlatformClient;
 import io.cryostat.platform.ServiceRef;
@@ -46,11 +47,17 @@ public class CustomTargetPlatformClient extends AbstractPlatformClient {
     private final SortedSet<ServiceRef> targets;
 
     @SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = "Field is never mutated")
-    public CustomTargetPlatformClient(Lazy<DiscoveryStorage> storage) {
+    public CustomTargetPlatformClient(Environment environment, Lazy<DiscoveryStorage> storage) {
+        super(environment);
         this.storage = storage;
         this.targets = new TreeSet<>((u1, u2) -> u1.getServiceUri().compareTo(u2.getServiceUri()));
     }
 
+    @Override
+    public boolean isEnabled() {
+        return true;
+    }
+
     @Override
     public void load(Promise<EnvironmentNode> promise) {
         storage.get()

diff --git a/src/main/java/io/cryostat/platform/internal/DefaultPlatformClient.java b/src/main/java/io/cryostat/platform/internal/DefaultPlatformClient.java
@@ -31,6 +31,7 @@
 import io.cryostat.core.net.discovery.DiscoveredJvmDescriptor;
 import io.cryostat.core.net.discovery.JvmDiscoveryClient;
 import io.cryostat.core.net.discovery.JvmDiscoveryClient.JvmDiscoveryEvent;
+import io.cryostat.core.sys.Environment;
 import io.cryostat.platform.AbstractPlatformClient;
 import io.cryostat.platform.ServiceRef;
 import io.cryostat.platform.ServiceRef.AnnotationKey;
@@ -47,12 +48,14 @@ public class DefaultPlatformClient extends AbstractPlatformClient
 
     public static final NodeType NODE_TYPE = BaseNodeType.JVM;
 
-    private final Logger logger;
     private final JvmDiscoveryClient discoveryClient;
+    private final Logger logger;
 
-    DefaultPlatformClient(Logger logger, JvmDiscoveryClient discoveryClient) {
-        this.logger = logger;
+    DefaultPlatformClient(
+            Environment environment, JvmDiscoveryClient discoveryClient, Logger logger) {
+        super(environment);
         this.discoveryClient = discoveryClient;
+        this.logger = logger;
     }
 
     @Override

diff --git a/src/main/java/io/cryostat/platform/internal/DefaultPlatformStrategy.java b/src/main/java/io/cryostat/platform/internal/DefaultPlatformStrategy.java
@@ -17,23 +17,27 @@
 
 import io.cryostat.core.log.Logger;
 import io.cryostat.core.net.discovery.JvmDiscoveryClient;
+import io.cryostat.core.sys.Environment;
 import io.cryostat.net.AuthManager;
 
 import dagger.Lazy;
 
 class DefaultPlatformStrategy implements PlatformDetectionStrategy<DefaultPlatformClient> {
 
-    private final Logger logger;
+    private final Environment environment;
     private final Lazy<? extends AuthManager> authMgr;
     private final Lazy<JvmDiscoveryClient> discoveryClient;
+    private final Logger logger;
 
     DefaultPlatformStrategy(
-            Logger logger,
+            Environment environment,
             Lazy<? extends AuthManager> authMgr,
-            Lazy<JvmDiscoveryClient> discoveryClient) {
-        this.logger = logger;
+            Lazy<JvmDiscoveryClient> discoveryClient,
+            Logger logger) {
+        this.environment = environment;
         this.authMgr = authMgr;
         this.discoveryClient = discoveryClient;
+        this.logger = logger;
     }
 
     @Override
@@ -44,7 +48,7 @@ public boolean isAvailable() {
     @Override
     public DefaultPlatformClient getPlatformClient() {
         logger.info("Selected Default Platform Strategy");
-        return new DefaultPlatformClient(logger, discoveryClient.get());
+        return new DefaultPlatformClient(environment, discoveryClient.get(), logger);
     }
 
     @Override

diff --git a/src/main/java/io/cryostat/platform/internal/DockerPlatformClient.java b/src/main/java/io/cryostat/platform/internal/DockerPlatformClient.java
@@ -36,6 +36,7 @@
 import io.cryostat.core.log.Logger;
 import io.cryostat.core.net.JFRConnectionToolkit;
 import io.cryostat.core.net.discovery.JvmDiscoveryClient.EventKind;
+import io.cryostat.core.sys.Environment;
 import io.cryostat.platform.AbstractPlatformClient;
 import io.cryostat.platform.ServiceRef;
 import io.cryostat.platform.ServiceRef.AnnotationKey;
@@ -73,12 +74,14 @@ public class DockerPlatformClient extends AbstractPlatformClient {
     private final CopyOnWriteArrayList<ContainerSpec> containers = new CopyOnWriteArrayList<>();
 
     DockerPlatformClient(
+            Environment environment,
             Lazy<WebClient> webClient,
             Lazy<Vertx> vertx,
             SocketAddress dockerSocket,
             Lazy<JFRConnectionToolkit> connectionToolkit,
             Gson gson,
             Logger logger) {
+        super(environment);
         this.webClient = webClient;
         this.vertx = vertx;
         this.dockerSocket = dockerSocket;

diff --git a/src/main/java/io/cryostat/platform/internal/DockerPlatformStrategy.java b/src/main/java/io/cryostat/platform/internal/DockerPlatformStrategy.java
@@ -23,6 +23,7 @@
 
 import io.cryostat.core.log.Logger;
 import io.cryostat.core.net.JFRConnectionToolkit;
+import io.cryostat.core.sys.Environment;
 import io.cryostat.core.sys.FileSystem;
 import io.cryostat.net.AuthManager;
 
@@ -38,29 +39,32 @@
 class DockerPlatformStrategy implements PlatformDetectionStrategy<DockerPlatformClient> {
 
     private static final String DOCKER_SOCKET_PATH = "/var/run/docker.sock";
-    private final Logger logger;
     private final Lazy<? extends AuthManager> authMgr;
     private final Lazy<WebClient> webClient;
     private final Lazy<Vertx> vertx;
     private final Lazy<JFRConnectionToolkit> connectionToolkit;
     private final Gson gson;
+    private final Environment environment;
     private final FileSystem fs;
+    private final Logger logger;
 
     DockerPlatformStrategy(
-            Logger logger,
             Lazy<? extends AuthManager> authMgr,
             Lazy<WebClient> webClient,
             Lazy<Vertx> vertx,
             Lazy<JFRConnectionToolkit> connectionToolkit,
             Gson gson,
-            FileSystem fs) {
-        this.logger = logger;
+            Environment environment,
+            FileSystem fs,
+            Logger logger) {
         this.authMgr = authMgr;
         this.webClient = webClient;
         this.vertx = vertx;
         this.connectionToolkit = connectionToolkit;
         this.gson = gson;
+        this.environment = environment;
         this.fs = fs;
+        this.logger = logger;
     }
 
     @Override
@@ -129,7 +133,7 @@ private boolean testDockerApi() {
     public DockerPlatformClient getPlatformClient() {
         logger.info("Selected {} Strategy", getClass().getSimpleName());
         return new DockerPlatformClient(
-                webClient, vertx, getSocket(), connectionToolkit, gson, logger);
+                environment, webClient, vertx, getSocket(), connectionToolkit, gson, logger);
     }
 
     @Override

diff --git a/src/main/java/io/cryostat/platform/internal/KubeApiPlatformClient.java b/src/main/java/io/cryostat/platform/internal/KubeApiPlatformClient.java
@@ -35,6 +35,7 @@
 import io.cryostat.core.log.Logger;
 import io.cryostat.core.net.JFRConnectionToolkit;
 import io.cryostat.core.net.discovery.JvmDiscoveryClient.EventKind;
+import io.cryostat.core.sys.Environment;
 import io.cryostat.platform.AbstractPlatformClient;
 import io.cryostat.platform.ServiceRef;
 import io.cryostat.platform.ServiceRef.AnnotationKey;
@@ -105,10 +106,12 @@ protected HashMap<String, SharedIndexInformer<Endpoints>> initialize()
             new ConcurrentHashMap<>();
 
     KubeApiPlatformClient(
+            Environment environment,
             Collection<String> namespaces,
             KubernetesClient k8sClient,
             Lazy<JFRConnectionToolkit> connectionToolkit,
             Logger logger) {
+        super(environment);
         this.namespaces = new HashSet<>(namespaces);
         this.k8sClient = k8sClient;
         this.connectionToolkit = connectionToolkit;

diff --git a/src/main/java/io/cryostat/platform/internal/KubeApiPlatformStrategy.java b/src/main/java/io/cryostat/platform/internal/KubeApiPlatformStrategy.java
@@ -39,18 +39,18 @@
 
 class KubeApiPlatformStrategy implements PlatformDetectionStrategy<KubeApiPlatformClient> {
 
-    protected final Logger logger;
     protected final Lazy<? extends AuthManager> authMgr;
     protected final Environment env;
     protected final FileSystem fs;
     protected final Lazy<JFRConnectionToolkit> connectionToolkit;
+    protected final Logger logger;
 
     KubeApiPlatformStrategy(
-            Logger logger,
             Lazy<? extends AuthManager> authMgr,
             Lazy<JFRConnectionToolkit> connectionToolkit,
             Environment env,
-            FileSystem fs) {
+            FileSystem fs,
+            Logger logger) {
         this.logger = logger;
         this.authMgr = authMgr;
         this.connectionToolkit = connectionToolkit;
@@ -73,7 +73,7 @@ public boolean isAvailable() {
     public KubeApiPlatformClient getPlatformClient() {
         logger.info("Selected {} Strategy", getClass().getSimpleName());
         return new KubeApiPlatformClient(
-                getNamespaces(), createClient(), connectionToolkit, logger);
+                env, getNamespaces(), createClient(), connectionToolkit, logger);
     }
 
     @Override

diff --git a/src/main/java/io/cryostat/platform/internal/OpenShiftPlatformStrategy.java b/src/main/java/io/cryostat/platform/internal/OpenShiftPlatformStrategy.java
@@ -28,12 +28,12 @@
 class OpenShiftPlatformStrategy extends KubeApiPlatformStrategy {
 
     OpenShiftPlatformStrategy(
-            Logger logger,
             Lazy<? extends AuthManager> authMgr,
             Lazy<JFRConnectionToolkit> connectionToolkit,
             Environment env,
-            FileSystem fs) {
-        super(logger, authMgr, connectionToolkit, env, fs);
+            FileSystem fs,
+            Logger logger) {
+        super(authMgr, connectionToolkit, env, fs, logger);
     }
 
     @Override