From a89bb6d6a59dbc20fb39ee093f09800cc5737bab Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Fri, 17 Jul 2026 11:16:49 +0200 Subject: [PATCH 1/3] feature: run against IDEA using runIdeIdea (crw-11741) Signed-off-by: Andre Dietisheim Co-authored-by: Cursor --- ...ests.run.xml => Run Plugin (IDEA).run.xml} | 9 +++-- build.gradle.kts | 35 +++++++++++-------- gradle.properties | 2 -- 3 files changed, 25 insertions(+), 21 deletions(-) rename .run/{Run IDE for UI Tests.run.xml => Run Plugin (IDEA).run.xml} (77%) diff --git a/.run/Run IDE for UI Tests.run.xml b/.run/Run Plugin (IDEA).run.xml similarity index 77% rename from .run/Run IDE for UI Tests.run.xml rename to .run/Run Plugin (IDEA).run.xml index ee99b7ed..9b0cae16 100644 --- a/.run/Run IDE for UI Tests.run.xml +++ b/.run/Run Plugin (IDEA).run.xml @@ -1,5 +1,5 @@ - + - true true false - false - \ No newline at end of file + diff --git a/build.gradle.kts b/build.gradle.kts index b1f66122..a17eee0c 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,5 +1,6 @@ import org.jetbrains.changelog.Changelog import org.jetbrains.changelog.markdownToHTML +import org.jetbrains.intellij.platform.gradle.IntelliJPlatformType import org.jetbrains.intellij.platform.gradle.TestFrameworkType import org.gradle.api.tasks.testing.TestDescriptor import org.gradle.api.tasks.testing.TestResult @@ -18,6 +19,19 @@ plugins { group = providers.gradleProperty("pluginGroup").get() version = providers.gradleProperty("pluginVersion").get() +// -Pidea switches from Gateway (GW) to IntelliJ IDEA (IU) +val isIdea = providers.gradleProperty("idea").isPresent +val resolvedPlatformType = if (isIdea) { + "IU" +} else { + providers.gradleProperty("platformType").get() +} +val resolvedBundledPlugins = if (isIdea) { + "com.jetbrains.gateway" +} else { + providers.gradleProperty("platformBundledPlugins").get() +} + // Set the JVM language level used to build the project. kotlin { jvmToolchain(21) @@ -56,10 +70,10 @@ dependencies { // IntelliJ Platform Gradle Plugin Dependencies Extension - read more: https://plugins.jetbrains.com/docs/intellij/tools-intellij-platform-gradle-plugin-dependencies-extension.html intellijPlatform { - create(providers.gradleProperty("platformType"), providers.gradleProperty("platformVersion")) + create(resolvedPlatformType, providers.gradleProperty("platformVersion")) // Plugin Dependencies. Uses `platformBundledPlugins` property from the gradle.properties file for bundled IntelliJ Platform plugins. - bundledPlugins(providers.gradleProperty("platformBundledPlugins").map { it.split(',') }) + bundledPlugins(resolvedBundledPlugins.split(',').filter { it.isNotBlank() }) // Plugin Dependencies. Uses `platformPlugins` property from the gradle.properties file for plugin from JetBrains Marketplace. plugins(providers.gradleProperty("platformPlugins").map { it.split(',') }) @@ -227,20 +241,13 @@ tasks { intellijPlatformTesting { runIde { - register("runIdeForUiTests") { - task { - jvmArgumentProviders += CommandLineArgumentProvider { - listOf( - "-Drobot-server.port=8082", - "-Dide.mac.message.dialogs.as.sheets=false", - "-Djb.privacy.policy.text=", - "-Djb.consents.confirmation.enabled=false", - ) - } - } + // Visible next to runIde under "intellij platform" in the Gradle tool window + register("runIdeIdea") { + type = IntelliJPlatformType.IntellijIdeaUltimate + version = providers.gradleProperty("platformVersion") plugins { - robotServerPlugin() + bundledPlugin("com.jetbrains.gateway") } } } diff --git a/gradle.properties b/gradle.properties index 1ded4a0e..536d762a 100644 --- a/gradle.properties +++ b/gradle.properties @@ -8,7 +8,6 @@ pluginVersion = 0.0.18 # Supported build number ranges and IntelliJ Platform versions -> https://plugins.jetbrains.com/docs/intellij/build-number-ranges.html pluginSinceBuild = 251 -#pluginUntilBuild = 252.* # IntelliJ Platform Properties -> https://plugins.jetbrains.com/docs/intellij/tools-gradle-intellij-plugin.html#configuration-intellij-extension platformType = GW @@ -20,7 +19,6 @@ platformVersion = 2025.1.1 platformPlugins = # Example: platformBundledPlugins = com.intellij.java platformBundledPlugins = - # Gradle Releases -> https://github.com/gradle/gradle/releases gradleVersion = 8.10.2 From 53627f0a644a086c107f4976b8f7d0ad39f24577 Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Sat, 18 Jul 2026 21:56:39 +0200 Subject: [PATCH 2/3] fix: async kubeconfig refreshes overwrite cluster list with an empty/partial (crw-11741) Signed-off-by: Andre Dietisheim Co-authored-by: Cursor --- .../gateway/kubeconfig/FileWatcher.kt | 130 ++++++++--- .../gateway/kubeconfig/KubeConfigMonitor.kt | 72 ++++-- .../gateway/kubeconfig/FileWatcherTest.kt | 206 ++++++++++++------ .../kubeconfig/KubeConfigMonitorTest.kt | 131 ++++++----- 4 files changed, 372 insertions(+), 167 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt index 2910cda7..8de6131c 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcher.kt @@ -14,6 +14,8 @@ package com.redhat.devtools.gateway.kubeconfig import kotlinx.coroutines.* import java.nio.file.* import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.TimeUnit +import java.util.concurrent.locks.ReentrantLock import kotlin.io.path.exists import kotlin.io.path.isRegularFile @@ -23,80 +25,144 @@ class FileWatcher( private val watchService: WatchService = FileSystems.getDefault().newWatchService() ) { private var onFileChanged: ((Path) -> Unit)? = null - private val registeredKeys = ConcurrentHashMap() + private val registeredDirectories = ConcurrentHashMap() private val monitoredFiles = ConcurrentHashMap.newKeySet() + private val notifyLock = ReentrantLock() private var watchJob: Job? = null fun start() { this.watchJob = scope.launch(dispatcher) { - try { - while (isActive) { - val key = watchService.poll(100, java.util.concurrent.TimeUnit.MILLISECONDS) - if (key == null) { - @Suppress("ConvertLongToDuration") - delay(100) - continue - } - val dir = registeredKeys[key] ?: continue - pollEvents(key, dir) - key.reset() + pollDirectoryEvents() + } + } + + private suspend fun CoroutineScope.pollDirectoryEvents() { + try { + while (isActive) { + val key = watchService.poll(100, TimeUnit.MILLISECONDS) + if (key == null) { + @Suppress("ConvertLongToDuration") + delay(100) + continue } - } catch (e: ClosedWatchServiceException) { - // Watch service was closed, exit gracefully + val dir = registeredDirectories[key] ?: continue + handleDirectoryEvent(key, dir) + key.reset() } + } catch (_: ClosedWatchServiceException) { + // Watch service was closed, exit gracefully } } fun stop() { watchJob?.cancel() watchJob = null + registeredDirectories.keys.forEach { it.cancel() } + registeredDirectories.clear() + monitoredFiles.clear() try { watchService.close() - } catch (e: Exception) { + } catch (_: Exception) { // Ignore exceptions when closing } } + /** + * Registers [path] for watching and invokes the [onFileChanged] callback if this is + * a new file that was not previously monitored. For non-existent files, the callback + * is invoked so listeners can track them for a later CREATE event. If [path] is + * already in the monitored set, no callback is issued — this avoids redundant refreshes. + * + * @param path the file to monitor; may be non-existent (e.g. a kubeconfig file expected + * to appear later). If [path.parent] is null, the path will not be registered. + * @return this instance for chaining. + */ fun addFile(path: Path): FileWatcher { - if (!path.exists() - || !path.isRegularFile()) { - return this - } val parentDir = path.parent if (parentDir != null && !monitoredFiles.contains(path)) { - val watchKey = parentDir.register(watchService, - StandardWatchEventKinds.ENTRY_CREATE, - StandardWatchEventKinds.ENTRY_MODIFY, - StandardWatchEventKinds.ENTRY_DELETE - ) - registeredKeys[watchKey] = parentDir + registerDirectory(parentDir) monitoredFiles.add(path) - onFileChanged?.invoke(path) + invokeOnFileChanged(path) } return this } fun removeFile(path: Path): FileWatcher { - monitoredFiles.remove(path) + if (!monitoredFiles.remove(path)) { + return this + } + val parentDir = path.parent ?: return this + if (monitoredFiles.none { it.parent == parentDir }) { + unregisterDirectory(parentDir) + } return this } + fun getMonitoredFiles(): Set = monitoredFiles + + /** Parent directories that currently have an active [WatchKey]. For tests. */ + internal fun getWatchedDirectories(): Set = registeredDirectories.values.toSet() + fun onFileChanged(action: ((Path) -> Unit)?) { - this.onFileChanged = action + notifyLock.lock() + try { + this.onFileChanged = action + } finally { + notifyLock.unlock() + } } - private fun pollEvents(key: WatchKey, dir: Path) { + private fun registerDirectory(directory: Path) { + if (registeredDirectories.values.any { it == directory }) { + return + } + val watchKey = directory.register( + watchService, + StandardWatchEventKinds.ENTRY_CREATE, + StandardWatchEventKinds.ENTRY_MODIFY, + StandardWatchEventKinds.ENTRY_DELETE + ) + registeredDirectories[watchKey] = directory + } + + private fun unregisterDirectory(directory: Path) { + val keys = registeredDirectories.filterValues { it == directory }.keys + keys.forEach { key -> + registeredDirectories.remove(key) + key.cancel() + } + } + + private fun handleDirectoryEvent(key: WatchKey, dir: Path) { for (event in key.pollEvents()) { val relativePath = event.context() as? Path ?: continue val changedFile = dir.resolve(relativePath) + val kind = event.kind() - if (monitoredFiles.contains(changedFile) - && event.kind() != StandardWatchEventKinds.OVERFLOW + if (!monitoredFiles.contains(changedFile) + || kind == StandardWatchEventKinds.OVERFLOW ) { - onFileChanged?.invoke(changedFile) + continue + } + + // DELETE: file is already gone — still notify so callers can refresh. + // CREATE/MODIFY: only notify for an existing regular file. + val shouldNotify = kind == StandardWatchEventKinds.ENTRY_DELETE + || (changedFile.exists() && changedFile.isRegularFile()) + + if (shouldNotify) { + invokeOnFileChanged(changedFile) } } } + private fun invokeOnFileChanged(path: Path) { + notifyLock.lock() + try { + onFileChanged?.invoke(path) + } finally { + notifyLock.unlock() + } + } } diff --git a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt index 434ca530..8d6e9794 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitor.kt @@ -14,10 +14,14 @@ package com.redhat.devtools.gateway.kubeconfig import com.intellij.openapi.diagnostic.thisLogger import com.redhat.devtools.gateway.openshift.Cluster import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import java.nio.file.Path +import java.util.concurrent.atomic.AtomicLong class KubeConfigMonitor( private val scope: CoroutineScope, @@ -26,17 +30,19 @@ class KubeConfigMonitor( ) { private val logger = thisLogger() - private val _clusters = MutableSharedFlow>(replay = 1) - private val clusters = _clusters.asSharedFlow() + /** null until the first successful refresh — avoids pushing an empty list to late collectors. */ + private val _clusters = MutableStateFlow?>(null) + private val clusters = _clusters.asStateFlow() - private val monitoredPaths = mutableSetOf() + private val refreshMutex = Mutex() + private val refreshGeneration = AtomicLong(0) /** - * Runs the given action for each collected cluster. + * Runs the given action for each collected cluster list. */ suspend fun onClustersCollected(action: suspend (clusters: List) -> Unit) { - logger.info("Setting up SharedFlow collection for cluster updates") - clusters.collect { collected -> + logger.info("Setting up StateFlow collection for cluster updates") + clusters.filterNotNull().collect { collected -> logger.info("Found ${collected.size} clusters") action(collected) } @@ -47,7 +53,7 @@ class KubeConfigMonitor( * * @see [onClustersCollected] */ - internal fun getCurrentClusters(): List = _clusters.replayCache.firstOrNull() ?: emptyList() + internal fun getCurrentClusters(): List = _clusters.value.orEmpty() fun start() { fileWatcher.onFileChanged(::onFileChanged) @@ -55,6 +61,7 @@ class KubeConfigMonitor( fileWatcher.start() } updateMonitoredPaths() + // Barrier refresh: ensures a generation after all sync addFile() notifies. refreshClusters() } @@ -64,38 +71,59 @@ class KubeConfigMonitor( } internal fun updateMonitoredPaths() { - val newPaths = mutableSetOf() - newPaths.addAll(kubeConfigUtils.getAllConfigFiles()) + val newPaths = kubeConfigUtils.getAllConfigFiles().toSet() stopWatchingRemoved(newPaths) startWatchingNew(newPaths) - - monitoredPaths.clear() - monitoredPaths.addAll(newPaths) - logger.info("Monitored paths: $monitoredPaths") + logger.info("Monitored paths: ${fileWatcher.getMonitoredFiles()}") } private fun stopWatchingRemoved(newPaths: Set) { - (monitoredPaths - newPaths).forEach { path -> + (fileWatcher.getMonitoredFiles() - newPaths).forEach { path -> fileWatcher.removeFile(path) logger.info("Stopped monitoring kubeconfig file: $path") } } private fun startWatchingNew(newPaths: Set) { - (newPaths - monitoredPaths).forEach { path -> + (newPaths - fileWatcher.getMonitoredFiles()).forEach { path -> fileWatcher.addFile(path) logger.info("Started monitoring kubeconfig file: $path") } } + /** + * Schedules a cluster refresh. Multiple rapid calls (e.g. per-file [FileWatcher.addFile] + * notifies during bootstrap) coalesce: only the latest generation parses and emits. + * Paths are snapshotted when the work runs, not when it is scheduled, so a burst of + * sync registrations yields one full-list emit. + * + * [MutableStateFlow] skips collector notifications when the new list equals the previous. + */ internal fun refreshClusters() { - logger.info("Reparsing kubeconfig files. Monitored paths: $monitoredPaths") - val allClusters = kubeConfigUtils.getClusters(monitoredPaths.toList()) + val gen = refreshGeneration.incrementAndGet() scope.launch { - logger.info("Emitting ${allClusters.size} clusters to SharedFlow") - _clusters.emit(allClusters) + refreshMutex.withLock { + if (gen != refreshGeneration.get()) { + return@withLock + } + val monitored = fileWatcher.getMonitoredFiles().toList() + logger.info("Reparsing kubeconfig files. Monitored paths: $monitored") + val allClusters = kubeConfigUtils.getClusters(monitored) + if (gen != refreshGeneration.get()) { + return@withLock + } + val previous = _clusters.value + _clusters.value = allClusters + if (previous == allClusters) { + logger.info("Skipping notify; clusters unchanged (${allClusters.size})") + } else { + logger.info( + "Updated clusters (${allClusters.size}): " + + allClusters.map { "${it.name}@${it.url}" } + ) + } + } } - logger.info("Reparsed kubeconfig files. Found ${allClusters.size} clusters: ${allClusters.map { "${it.name}@${it.url}" }}") } fun onFileChanged(filePath: Path) { diff --git a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt index c77c9f15..f539322f 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/FileWatcherTest.kt @@ -12,23 +12,27 @@ package com.redhat.devtools.gateway.kubeconfig import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import kotlinx.coroutines.withTimeout - - import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test - import org.junit.jupiter.api.io.TempDir import java.nio.file.Path import kotlin.io.path.createFile +import kotlin.io.path.deleteExisting import kotlin.io.path.writeText @ExperimentalCoroutinesApi @@ -57,136 +61,217 @@ class FileWatcherTest { @Test fun `#addFile() invokes callback when a file is added`() = runTest { - // given - var onFileChangedCount = 0 - watcher.onFileChanged { onFileChangedCount++ } + var onFileChangedPath: Path? = null + watcher.onFileChanged { path -> onFileChangedPath = path } - // when watcher.addFile(testFile) advanceUntilIdle() - // then - Initial notification when file is added - assertThat(onFileChangedCount).isEqualTo(1) + assertThat(onFileChangedPath).isEqualTo(testFile) + assertThat(watcher.getMonitoredFiles()).containsExactly(testFile) } @Test - fun `#addFile() does not invoke callback for non-existent files`() = runTest { - // given + fun `#addFile() invokes callback for non-existent files that are tracked`() = runTest { var onFileChangedCount = 0 watcher.onFileChanged { onFileChangedCount++ } val nonExistentFile = tempDir.resolve("non-existent.yaml") - // when watcher.addFile(nonExistentFile) advanceUntilIdle() - // then - No callback should be invoked for non-existent files - assertThat(onFileChangedCount).isEqualTo(0) + assertThat(onFileChangedCount).isEqualTo(1) + assertThat(watcher.getMonitoredFiles()).containsExactly(nonExistentFile) } @Test - fun `#addFile() invokes callback when multiple files are added`() = runTest { - // given + fun `#addFile() invokes callback for each file when multiple are added`() = runTest { var onFileChangedCount = 0 watcher.onFileChanged { onFileChangedCount++ } - val secondFile = createFile("second-file.yaml","second content") + val secondFile = createFile("second-file.yaml", "second content") - // when watcher .addFile(testFile) .addFile(secondFile) advanceUntilIdle() - // then - Callback should be invoked for both files assertThat(onFileChangedCount).isEqualTo(2) + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, secondFile) } @Test - fun `#removeFile() removes file from monitoring`() = runTest { - // given + fun `#addFile() notifies again when re-adding after remove`() = runTest { var onFileChangedCount = 0 watcher.onFileChanged { onFileChangedCount++ } - // when - add file and verify it triggers callback watcher.addFile(testFile) advanceUntilIdle() assertThat(onFileChangedCount).isEqualTo(1) - // when - remove file from monitoring watcher.removeFile(testFile) advanceUntilIdle() + assertThat(watcher.getMonitoredFiles()).isEmpty() + + watcher.addFile(testFile) + advanceUntilIdle() + assertThat(onFileChangedCount).isEqualTo(2) + assertThat(watcher.getMonitoredFiles()).containsExactly(testFile) + } + + @Test + fun `#addFile() does not invoke callback when file is already monitored`() = runTest { + var onFileChangedPath: Path? = null + watcher.onFileChanged { path -> onFileChangedPath = path } + + watcher.addFile(testFile) + advanceUntilIdle() - val initialOnFileChangedCount = onFileChangedCount - - // Re-adding the same file should trigger callback again since it was removed + onFileChangedPath = null watcher.addFile(testFile) advanceUntilIdle() - - assertThat(onFileChangedCount).isEqualTo(initialOnFileChangedCount + 1) + + assertThat(onFileChangedPath).isNull() + assertThat(watcher.getMonitoredFiles()).containsExactly(testFile) } @Test fun `#removeFile() handles non-existent files gracefully`() = runTest { - // given val nonExistentFile = tempDir.resolve("never-added.yaml") - // when - removing a file that was never added val result = watcher.removeFile(nonExistentFile) - // then - should return the watcher instance and not throw any exceptions assertThat(result).isSameAs(watcher) } + @Test + fun `#removeFile() unregisters directory when last file is removed`() = runTest { + watcher.addFile(testFile) + advanceUntilIdle() + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + + watcher.removeFile(testFile) + advanceUntilIdle() + + assertThat(watcher.getMonitoredFiles()).isEmpty() + assertThat(watcher.getWatchedDirectories()).isEmpty() + } + + @Test + fun `#removeFile() keeps directory watch while sibling files remain`() = runTest { + val secondFile = createFile("second-file.yaml", "second content") + + watcher.addFile(testFile).addFile(secondFile) + advanceUntilIdle() + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + + watcher.removeFile(testFile) + advanceUntilIdle() + + assertThat(watcher.getMonitoredFiles()).containsExactly(secondFile) + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + } + + @Test + fun `#addFile() registers a directory watch only once for siblings`() = runTest { + val secondFile = createFile("second-file.yaml", "second content") + + watcher.addFile(testFile).addFile(secondFile) + advanceUntilIdle() + + assertThat(watcher.getWatchedDirectories()).containsExactly(testFile.parent) + } + @Test fun `#removeFile() keeps other files monitored`() = runTest { - // given val secondFile = createFile("second-file.yaml", "second content") val thirdFile = createFile("third-file.yaml", "third content") - var onFileChangedCount = 0 - watcher.onFileChanged { onFileChangedCount++ } - // when - add multiple files watcher .addFile(testFile) .addFile(secondFile) .addFile(thirdFile) advanceUntilIdle() - - // then - all files should trigger callbacks - assertThat(onFileChangedCount).isEqualTo(3) - // when - remove one file + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, secondFile, thirdFile) + watcher.removeFile(secondFile) advanceUntilIdle() - // then - other files should still be monitored - val initialOnFileChangedCount = onFileChangedCount - watcher.addFile(testFile) // Re-adding should not trigger (already added) - watcher.addFile(thirdFile) // Re-adding should not trigger (already added) - watcher.addFile(secondFile) // Adding should trigger (was removed) - advanceUntilIdle() + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, thirdFile) - assertThat(onFileChangedCount).isEqualTo(initialOnFileChangedCount + 1) + watcher.addFile(secondFile) + advanceUntilIdle() + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(testFile, secondFile, thirdFile) } @Test - fun `#onFileChanged() is invoked when a watched file is modified`() = runTest { - // given - val callbackReceived = CompletableDeferred() - watcher.onFileChanged { - callbackReceived.complete(Unit) - } + fun `#getMonitoredFiles() includes non-existent files`() = runTest { + val existingFile = createFile("existing.yaml", "content") + val nonExistentFile = tempDir.resolve("never-exists.yaml") - watcher.start() - watcher.addFile(testFile) + watcher.addFile(existingFile) + watcher.addFile(nonExistentFile) advanceUntilIdle() - // when - testFile.writeText("updated content") + assertThat(watcher.getMonitoredFiles()).containsExactlyInAnyOrder(existingFile, nonExistentFile) + } - // then - withTimeout(1000) { - callbackReceived.await() + @Test + fun `#onFileChanged() is invoked when a watched file is modified`() = runBlocking { + val ioScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + val ioWatcher = FileWatcher(ioScope, Dispatchers.IO) + try { + val callbackReceived = CompletableDeferred() + var notifyCount = 0 + ioWatcher.onFileChanged { + notifyCount++ + // First notify is from addFile; complete on a subsequent FS event. + if (notifyCount > 1) { + callbackReceived.complete(Unit) + } + } + ioWatcher.start() + ioWatcher.addFile(testFile) + delay(200) + + testFile.writeText("updated content") + + @Suppress("ConvertLongToDuration") + withTimeout(5_000) { + callbackReceived.await() + } + } finally { + ioWatcher.stop() + ioScope.cancel() + } + } + + @Test + fun `#onFileChanged() is invoked when a watched file is deleted`() = runBlocking { + val ioScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + val ioWatcher = FileWatcher(ioScope, Dispatchers.IO) + try { + val callbackReceived = CompletableDeferred() + var notifyCount = 0 + ioWatcher.onFileChanged { path -> + notifyCount++ + if (notifyCount > 1) { + callbackReceived.complete(path) + } + } + ioWatcher.start() + ioWatcher.addFile(testFile) + delay(200) + + testFile.deleteExisting() + + @Suppress("ConvertLongToDuration") + withTimeout(5_000) { + assertThat(callbackReceived.await()).isEqualTo(testFile) + } + } finally { + ioWatcher.stop() + ioScope.cancel() } } @@ -196,5 +281,4 @@ class FileWatcherTest { file.writeText(content) return file } - -} \ No newline at end of file +} diff --git a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt index aea169d6..1038cf0e 100644 --- a/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt +++ b/src/test/kotlin/com/redhat/devtools/gateway/kubeconfig/KubeConfigMonitorTest.kt @@ -11,16 +11,16 @@ */ package com.redhat.devtools.gateway.kubeconfig -import com.redhat.devtools.gateway.kubeconfig.FileWatcher -import com.redhat.devtools.gateway.kubeconfig.KubeConfigUtils -import com.redhat.devtools.gateway.kubeconfig.KubeConfigMonitor import com.redhat.devtools.gateway.openshift.Cluster import io.mockk.every +import io.mockk.justRun import io.mockk.mockk +import io.mockk.spyk import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -35,14 +35,13 @@ import kotlin.io.path.createFile @ExperimentalCoroutinesApi class KubeConfigMonitorTest { - // notsecret — cluster token literals in this class are invented test fixtures only. - @TempDir lateinit var tempDir: Path + private lateinit var testDispatcher: TestDispatcher private lateinit var testScope: TestScope - private lateinit var mockFileWatcher: FileWatcher - private lateinit var mockKubeConfigBuilder: KubeConfigUtils + private lateinit var fileWatcher: FileWatcher + private lateinit var mockKubeConfigUtils: KubeConfigUtils private lateinit var kubeconfigMonitor: KubeConfigMonitor private lateinit var kubeconfigPath1: Path @@ -50,15 +49,18 @@ class KubeConfigMonitorTest { @BeforeEach fun beforeEach() { - // given - testScope = TestScope(StandardTestDispatcher()) - mockFileWatcher = mockk(relaxed = true) - mockKubeConfigBuilder = mockk(relaxed = true) + testDispatcher = StandardTestDispatcher() + testScope = TestScope(testDispatcher) + // Real FileWatcher for path tracking; stub start() so its infinite poll/delay loop + // does not run on the test dispatcher (advanceUntilIdle would never complete). + fileWatcher = spyk(FileWatcher(testScope, testDispatcher)) + justRun { fileWatcher.start() } + mockKubeConfigUtils = mockk(relaxed = true) kubeconfigPath1 = tempDir.resolve("kubeconfig1.yaml").createFile() kubeconfigPath2 = tempDir.resolve("kubeconfig2.yaml").createFile() - kubeconfigMonitor = KubeConfigMonitor(testScope, mockFileWatcher, mockKubeConfigBuilder) + kubeconfigMonitor = KubeConfigMonitor(testScope, fileWatcher, mockKubeConfigUtils) } @AfterEach @@ -67,92 +69,117 @@ class KubeConfigMonitorTest { } @Test - fun `#start should initially parse and publish clusters`() = testScope.runTest { - // given - val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + fun `#start should initially parse and publish clusters`() = runTest(testDispatcher) { + val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) - // when kubeconfigMonitor.start() advanceUntilIdle() - // then assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) - verify(exactly = 1) { mockFileWatcher.addFile(kubeconfigPath1) } - verify(exactly = 1) { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } + assertThat(fileWatcher.getMonitoredFiles()).containsExactly(kubeconfigPath1) + // addFile notify + barrier refreshClusters coalesce to one parse of the full set + verify(exactly = 1) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } + } + + @Test + fun `#start coalesces multi-file addFile notifies into one full-list parse`() = runTest(testDispatcher) { + val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) + val cluster2 = Cluster(name = "obi-wan", url = "url2", token = null) + + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1, kubeconfigPath2) + every { + mockKubeConfigUtils.getClusters(match { it.toSet() == setOf(kubeconfigPath1, kubeconfigPath2) }) + } returns listOf(cluster1, cluster2) + + kubeconfigMonitor.start() + advanceUntilIdle() + + verify(exactly = 0) { mockKubeConfigUtils.getClusters(emptyList()) } + verify(exactly = 0) { + mockKubeConfigUtils.getClusters(match { it.size == 1 }) + } + verify(exactly = 1) { + mockKubeConfigUtils.getClusters(match { it.toSet() == setOf(kubeconfigPath1, kubeconfigPath2) }) + } + assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactlyInAnyOrder(cluster1, cluster2) + assertThat(fileWatcher.getMonitoredFiles()).containsExactlyInAnyOrder(kubeconfigPath1, kubeconfigPath2) } @Test - fun `#onFileChanged should reparse and publish updated clusters`() = testScope.runTest { - // given + fun `#onFileChanged should reparse and publish updated clusters`() = runTest(testDispatcher) { val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) val cluster1Updated = Cluster(name = "skywalker", url = "url1", token = "token1") - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) kubeconfigMonitor.start() advanceUntilIdle() assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) - // when - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1Updated) - kubeconfigMonitor.onFileChanged(kubeconfigPath1) // Simulate watcher callback + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1Updated) + kubeconfigMonitor.onFileChanged(kubeconfigPath1) advanceUntilIdle() - // then assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1Updated) - verify(exactly = 2) { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } // Initial + update + verify(exactly = 2) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } } @Test - fun `#updateMonitoredPaths should add and remove files based on KUBECONFIG env var`() = testScope.runTest { - // given + fun `#refreshClusters skips emit when cluster list is unchanged`() = runTest(testDispatcher) { + val cluster1 = Cluster(name = "skywalker", url = "url1", token = null) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + + kubeconfigMonitor.start() + advanceUntilIdle() + assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) + + kubeconfigMonitor.refreshClusters() + advanceUntilIdle() + + assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) + verify(exactly = 2) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } + } + + @Test + fun `#updateMonitoredPaths should add and remove files based on KUBECONFIG env var`() = runTest(testDispatcher) { val cluster1 = Cluster(name = "skywalker", url = "url1") val cluster2 = Cluster(name = "obi-wan", url = "url2") - // Initial KUBECONFIG - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath1) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath1)) } returns listOf(cluster1) kubeconfigMonitor.start() advanceUntilIdle() assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster1) - verify(exactly = 1) { mockFileWatcher.addFile(kubeconfigPath1) } + assertThat(fileWatcher.getMonitoredFiles()).containsExactly(kubeconfigPath1) - // when: Change KUBECONFIG to include kubeconfigPath2 and remove kubeconfigPath1 - every { mockKubeConfigBuilder.getAllConfigFiles(any()) } returns listOf(kubeconfigPath2) - every { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath2)) } returns listOf(cluster2) + every { mockKubeConfigUtils.getAllConfigFiles(any()) } returns listOf(kubeconfigPath2) + every { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath2)) } returns listOf(cluster2) - // Manually trigger updateMonitoredPaths and reparse kubeconfigMonitor.updateMonitoredPaths() kubeconfigMonitor.refreshClusters() advanceUntilIdle() - // then assertThat(kubeconfigMonitor.getCurrentClusters()).containsExactly(cluster2) - verify(exactly = 1) { - mockFileWatcher.removeFile(kubeconfigPath1) - mockFileWatcher.addFile(kubeconfigPath2) - } - verify(exactly = 1) { mockKubeConfigBuilder.getClusters(listOf(kubeconfigPath2)) } + assertThat(fileWatcher.getMonitoredFiles()).containsExactly(kubeconfigPath2) + verify(atLeast = 1) { mockKubeConfigUtils.getClusters(listOf(kubeconfigPath2)) } } @Test - fun `#stop should not cancel the provided scope`() = testScope.runTest { - // given + fun `#stop should not cancel the provided scope`() = runTest(testDispatcher) { val mockScope = mockk(relaxed = true) - val monitor = KubeConfigMonitor(mockScope, mockFileWatcher, mockKubeConfigBuilder) + val monitor = KubeConfigMonitor(mockScope, fileWatcher, mockKubeConfigUtils) every { mockScope.cancel() } returns Unit - // when monitor.stop() advanceUntilIdle() - // then verify(exactly = 0) { mockScope.cancel() } } -} \ No newline at end of file +} From 8f54463b34ddb7de4226c254aff9334347acf08c Mon Sep 17 00:00:00 2001 From: mokhtar Date: Tue, 21 Jul 2026 17:18:31 -0400 Subject: [PATCH 3/3] fix: bypass JVM proxy for Kubernetes API client connections OkHttp inherits the JVM default ProxySelector, which in corporate environments routes cluster API calls through an HTTP forward proxy. This may cause silent failures (403, proxy auth errors, PKIX) Set Proxy.NO_PROXY on all ApiClient OkHttp instances. Co-authored-by: Cursor --- .../openshift/apiclient/BaseClientBuilder.kt | 3 +++ .../openshift/apiclient/LinkClientBuilder.kt | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt index 33437d08..d9fe8bb5 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/BaseClientBuilder.kt @@ -14,6 +14,7 @@ package com.redhat.devtools.gateway.openshift.apiclient import io.kubernetes.client.openapi.ApiClient import okhttp3.OkHttpClient import okhttp3.Protocol +import java.net.Proxy import java.util.concurrent.TimeUnit import javax.net.ssl.SSLContext import javax.net.ssl.X509TrustManager @@ -55,9 +56,11 @@ abstract class BaseClientBuilder : OpenShiftClientBuilder { /** * Creates an [OkHttpClient] with the given [sslContext] and [trustManager]. * Uses HTTP/1.1 (not HTTP/2): some OpenShift clusters hang on HTTP/2. + * Bypasses JVM proxy settings — the Kubernetes API server is always accessed directly. */ protected fun createHttpClient(sslContext: SSLContext, trustManager: X509TrustManager): OkHttpClient = OkHttpClient.Builder() + .proxy(Proxy.NO_PROXY) .sslSocketFactory(sslContext.socketFactory, trustManager) .connectTimeout(DEFAULT_HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) .readTimeout(DEFAULT_HTTP_TIMEOUT_SECONDS, TimeUnit.SECONDS) diff --git a/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt index 65f971f2..4b4000c3 100644 --- a/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt +++ b/src/main/kotlin/com/redhat/devtools/gateway/openshift/apiclient/LinkClientBuilder.kt @@ -15,6 +15,7 @@ import com.intellij.openapi.diagnostic.thisLogger import com.redhat.devtools.gateway.kubeconfig.KubeConfigUtils import io.kubernetes.client.util.ClientBuilder import io.kubernetes.client.openapi.ApiClient +import java.net.Proxy /** * Builder for API clients used in the deep-link flow (no wizard). @@ -27,25 +28,32 @@ class LinkClientBuilder( val paths = configUtils.getAllConfigFiles() if (paths.isEmpty()) { thisLogger().debug("No effective kubeconfig found. Falling back to default ApiClient.") - return ClientBuilder.defaultClient() + return bypassProxy(ClientBuilder.defaultClient()) } return try { val allConfigs = configUtils.getAllConfigs(paths) if (allConfigs.isEmpty()) { thisLogger().debug("No valid kubeconfig content found. Falling back to default ApiClient.") - return ClientBuilder.defaultClient() + return bypassProxy(ClientBuilder.defaultClient()) } val kubeConfig = configUtils.mergeConfigs(allConfigs) - val client = ClientBuilder.kubeconfig(kubeConfig).build() + val client = bypassProxy(ClientBuilder.kubeconfig(kubeConfig).build()) applyReadTimeout(client) } catch (e: Exception) { thisLogger().debug( "Failed to build effective Kube config from discovered files due to error: ${e.message}. " + "Falling back to the default ApiClient." ) - ClientBuilder.defaultClient() + bypassProxy(ClientBuilder.defaultClient()) } } + + private fun bypassProxy(client: ApiClient): ApiClient { + client.httpClient = client.httpClient.newBuilder() + .proxy(Proxy.NO_PROXY) + .build() + return client + } }