fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var (#1506)

* fix(discovery): fix broken DISABLE_BUILT_DISCOVERY env var

* remove no longer necessary 'priority' for platforms

src/main/java/io/cryostat/discovery/BuiltInDiscovery.java

@@ -41,20 +41,16 @@
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
-import java.util.SortedSet;
 import java.util.UUID;
 import java.util.function.Consumer;
-import java.util.stream.Stream;
 
 import io.cryostat.core.log.Logger;
 import io.cryostat.messaging.notifications.NotificationFactory;
 import io.cryostat.platform.PlatformClient;
 import io.cryostat.platform.TargetDiscoveryEvent;
 import io.cryostat.platform.discovery.EnvironmentNode;
-import io.cryostat.platform.internal.CustomTargetPlatformClient;
 import io.cryostat.platform.internal.PlatformDetectionStrategy;
 
-import dagger.Lazy;
 import io.vertx.core.AbstractVerticle;
 import io.vertx.core.Promise;
 
@@ -65,22 +61,19 @@ public class BuiltInDiscovery extends AbstractVerticle implements Consumer<Targe
     private final DiscoveryStorage storage;
     private final Set<PlatformDetectionStrategy<?>> selectedStrategies;
     private final Set<PlatformDetectionStrategy<?>> unselectedStrategies;
-    private final Lazy<CustomTargetPlatformClient> customTargets;
     private final Set<PlatformClient> enabledClients = new HashSet<>();
     private final NotificationFactory notificationFactory;
     private final Logger logger;
 
     BuiltInDiscovery(
             DiscoveryStorage storage,
-            SortedSet<PlatformDetectionStrategy<?>> selectedStrategies,
-            SortedSet<PlatformDetectionStrategy<?>> unselectedStrategies,
-            Lazy<CustomTargetPlatformClient> customTargets,
+            Set<PlatformDetectionStrategy<?>> selectedStrategies,
+            Set<PlatformDetectionStrategy<?>> unselectedStrategies,
             NotificationFactory notificationFactory,
             Logger logger) {
         this.storage = storage;
         this.selectedStrategies = selectedStrategies;
         this.unselectedStrategies = unselectedStrategies;
-        this.customTargets = customTargets;
         this.notificationFactory = notificationFactory;
         this.logger = logger;
     }
@@ -98,12 +91,8 @@ public void start(Promise<Void> start) {
                                         .map(PluginInfo::getId)
                                         .ifPresent(storage::deregister));
 
-        Stream.concat(
-                        // ensure custom targets is always available regardless of other
-                        // configurations
-                        Stream.of(customTargets.get()),
-                        selectedStrategies.stream()
-                                .map(PlatformDetectionStrategy::getPlatformClient))
+        selectedStrategies.stream()
+                .map(PlatformDetectionStrategy::getPlatformClient)
                 .distinct()
                 .forEach(
                         platform -> {

src/main/java/io/cryostat/discovery/DiscoveryModule.java

@@ -39,7 +39,6 @@
 
 import java.time.Duration;
 import java.util.Set;
-import java.util.SortedSet;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
@@ -53,7 +52,6 @@
 import io.cryostat.messaging.notifications.NotificationFactory;
 import io.cryostat.platform.PlatformModule;
 import io.cryostat.platform.discovery.AbstractNode;
-import io.cryostat.platform.internal.CustomTargetPlatformClient;
 import io.cryostat.platform.internal.PlatformDetectionStrategy;
 import io.cryostat.recordings.JvmIdHelper;
 import io.cryostat.rules.MatchExpressionEvaluator;
@@ -119,19 +117,13 @@ static DiscoveryStorage provideDiscoveryStorage(
     static BuiltInDiscovery provideBuiltInDiscovery(
             DiscoveryStorage storage,
             @Named(PlatformModule.SELECTED_PLATFORMS)
-                    SortedSet<PlatformDetectionStrategy<?>> selectedStrategies,
+                    Set<PlatformDetectionStrategy<?>> selectedStrategies,
             @Named(PlatformModule.UNSELECTED_PLATFORMS)
-                    SortedSet<PlatformDetectionStrategy<?>> unselectedStrategies,
-            Lazy<CustomTargetPlatformClient> customTargets,
+                    Set<PlatformDetectionStrategy<?>> unselectedStrategies,
             NotificationFactory notificationFactory,
             Logger logger) {
         return new BuiltInDiscovery(
-                storage,
-                selectedStrategies,
-                unselectedStrategies,
-                customTargets,
-                notificationFactory,
-                logger);
+                storage, selectedStrategies, unselectedStrategies, notificationFactory, logger);
     }
 
     @Provides

src/main/java/io/cryostat/platform/PlatformModule.java

@@ -38,28 +38,27 @@
 package io.cryostat.platform;
 
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
 import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Set;
-import java.util.SortedSet;
-import java.util.TreeSet;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import javax.inject.Named;
 import javax.inject.Singleton;
 
 import io.cryostat.configuration.Variables;
 import io.cryostat.core.log.Logger;
 import io.cryostat.core.sys.Environment;
-import io.cryostat.discovery.DiscoveryStorage;
 import io.cryostat.net.AuthManager;
 import io.cryostat.platform.discovery.PlatformDiscoveryModule;
-import io.cryostat.platform.internal.CustomTargetPlatformClient;
+import io.cryostat.platform.internal.CustomTargetPlatformStrategy;
 import io.cryostat.platform.internal.PlatformDetectionStrategy;
 import io.cryostat.platform.internal.PlatformStrategyModule;
 
-import dagger.Lazy;
 import dagger.Module;
 import dagger.Provides;
 
@@ -69,13 +68,6 @@ public abstract class PlatformModule {
     public static final String SELECTED_PLATFORMS = "SELECTED_PLATFORMS";
     public static final String UNSELECTED_PLATFORMS = "UNSELECTED_PLATFORMS";
 
-    @Provides
-    @Singleton
-    static CustomTargetPlatformClient provideCustomTargetPlatformClient(
-            Lazy<DiscoveryStorage> storage) {
-        return new CustomTargetPlatformClient(storage);
-    }
-
     @Provides
     @Singleton
     static AuthManager provideAuthManager(
@@ -111,11 +103,10 @@ static AuthManager provideAuthManager(
     @Provides
     @Singleton
     @Named(SELECTED_PLATFORMS)
-    static SortedSet<PlatformDetectionStrategy<?>> provideSelectedPlatformStrategies(
-            Set<PlatformDetectionStrategy<?>> platformStrategies, Environment env) {
-        // reverse sort, higher priorities should be earlier in the stream
-        SortedSet<PlatformDetectionStrategy<?>> selectedStrategies =
-                new TreeSet<>((a, b) -> Integer.compare(b.getPriority(), a.getPriority()));
+    static Set<PlatformDetectionStrategy<?>> provideSelectedPlatformStrategies(
+            CustomTargetPlatformStrategy customTargets,
+            Set<PlatformDetectionStrategy<?>> platformStrategies,
+            Environment env) {
         Predicate<PlatformDetectionStrategy<?>> fn;
         if (env.hasEnv(Variables.PLATFORM_STRATEGY_ENV_VAR)) {
             List<String> platforms =
@@ -126,22 +117,17 @@ static SortedSet<PlatformDetectionStrategy<?>> provideSelectedPlatformStrategies
         } else {
             fn = PlatformDetectionStrategy::isAvailable;
         }
-        for (PlatformDetectionStrategy<?> s : platformStrategies) {
-            if (fn.test(s)) {
-                selectedStrategies.add(s);
-            }
-        }
-        return selectedStrategies;
+        return Stream.concat(Stream.of(customTargets), platformStrategies.stream().filter(fn))
+                .collect(Collectors.toSet());
     }
 
     @Provides
     @Singleton
     @Named(UNSELECTED_PLATFORMS)
-    static SortedSet<PlatformDetectionStrategy<?>> provideUnselectedPlatformStrategies(
-            @Named(SELECTED_PLATFORMS) SortedSet<PlatformDetectionStrategy<?>> selectedStrategies,
+    static Set<PlatformDetectionStrategy<?>> provideUnselectedPlatformStrategies(
+            @Named(SELECTED_PLATFORMS) Set<PlatformDetectionStrategy<?>> selectedStrategies,
             Set<PlatformDetectionStrategy<?>> platformStrategies) {
-        SortedSet<PlatformDetectionStrategy<?>> unselected =
-                new TreeSet<>((a, b) -> Integer.compare(b.getPriority(), a.getPriority()));
+        Set<PlatformDetectionStrategy<?>> unselected = new HashSet<>();
         unselected.addAll(platformStrategies);
         unselected.removeAll(selectedStrategies);
         return unselected;
@@ -150,7 +136,7 @@ static SortedSet<PlatformDetectionStrategy<?>> provideUnselectedPlatformStrategi
     @Provides
     @Singleton
     static PlatformDetectionStrategy<?> providePlatformStrategy(
-            @Named(SELECTED_PLATFORMS) SortedSet<PlatformDetectionStrategy<?>> selectedStrategies,
+            @Named(SELECTED_PLATFORMS) Set<PlatformDetectionStrategy<?>> selectedStrategies,
             Set<PlatformDetectionStrategy<?>> strategies) {
         return selectedStrategies.stream()
                 .findFirst()
@@ -161,11 +147,6 @@ static PlatformDetectionStrategy<?> providePlatformStrategy(
                                                 "No selected platforms found. Available platforms:"
                                                         + " %s",
                                                 strategies.stream()
-                                                        .sorted(
-                                                                (a, b) ->
-                                                                        Integer.compare(
-                                                                                b.getPriority(),
-                                                                                a.getPriority()))
                                                         .map(s -> s.getClass().getCanonicalName())
                                                         .toList())));
     }

src/main/java/io/cryostat/platform/internal/CustomTargetPlatformStrategy.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright The Cryostat Authors
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or data
+ * (collectively the "Software"), free of charge and under any and all copyright
+ * rights in the Software, and any and all patent rights owned or freely
+ * licensable by each licensor hereunder covering either (i) the unmodified
+ * Software as contributed to or provided by such licensor, or (ii) the Larger
+ * Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software (each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ * The above copyright notice and either this complete permission notice or at
+ * a minimum a reference to the UPL must be included in all copies or
+ * substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package io.cryostat.platform.internal;
+
+import io.cryostat.core.log.Logger;
+import io.cryostat.net.AuthManager;
+
+import dagger.Lazy;
+
+public class CustomTargetPlatformStrategy
+        implements PlatformDetectionStrategy<CustomTargetPlatformClient> {
+
+    private final Logger logger;
+    private final Lazy<? extends AuthManager> authMgr;
+    private final Lazy<CustomTargetPlatformClient> client;
+
+    CustomTargetPlatformStrategy(
+            Logger logger,
+            Lazy<? extends AuthManager> authMgr,
+            Lazy<CustomTargetPlatformClient> client) {
+        this.logger = logger;
+        this.authMgr = authMgr;
+        this.client = client;
+    }
+
+    @Override
+    public boolean isAvailable() {
+        return true;
+    }
+
+    @Override
+    public CustomTargetPlatformClient getPlatformClient() {
+        logger.info("Selected Default Platform Strategy");
+        return client.get();
+    }
+
+    @Override
+    public AuthManager getAuthManager() {
+        return authMgr.get();
+    }
+}

src/main/java/io/cryostat/platform/internal/DefaultPlatformStrategy.java

@@ -58,11 +58,6 @@ class DefaultPlatformStrategy implements PlatformDetectionStrategy<DefaultPlatfo
         this.discoveryClient = discoveryClient;
     }
 
-    @Override
-    public int getPriority() {
-        return PRIORITY_DEFAULT;
-    }
-
     @Override
     public boolean isAvailable() {
         return true;

src/main/java/io/cryostat/platform/internal/KubeApiPlatformStrategy.java

@@ -80,11 +80,6 @@ class KubeApiPlatformStrategy implements PlatformDetectionStrategy<KubeApiPlatfo
         this.fs = fs;
     }
 
-    @Override
-    public int getPriority() {
-        return PRIORITY_PLATFORM + 10;
-    }
-
     @Override
     public boolean isAvailable() {
         logger.trace("Testing {} Availability", getClass().getSimpleName());

src/main/java/io/cryostat/platform/internal/OpenShiftPlatformStrategy.java

@@ -58,11 +58,6 @@ class OpenShiftPlatformStrategy extends KubeApiPlatformStrategy {
         super(logger, authMgr, connectionToolkit, env, fs);
     }
 
-    @Override
-    public int getPriority() {
-        return PRIORITY_PLATFORM + 15;
-    }
-
     @Override
     protected boolean testAvailability(KubernetesClient client) {
         return super.testAvailability(client) && (((OpenShiftClient) client).isSupported());

src/main/java/io/cryostat/platform/internal/PlatformDetectionStrategy.java

@@ -41,11 +41,6 @@
 import io.cryostat.platform.PlatformClient;
 
 public interface PlatformDetectionStrategy<T extends PlatformClient> {
-    int PRIORITY_DEFAULT = 0;
-    int PRIORITY_PLATFORM = 50;
-
-    int getPriority();
-
     boolean isAvailable();
 
     T getPlatformClient();