From c571c22650f19919688ea306bb356ba5b69c5236 Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Tue, 8 Oct 2024 10:27:47 +0200 Subject: [PATCH] fix: bump kubernetes-client to 7.0.0 (#794) Signed-off-by: Andre Dietisheim --- build.gradle.kts | 2 +- gradle.properties | 3 +- gradle/libs.versions.toml | 8 +- .../intellij/kubernetes/model/AllContexts.kt | 31 +-- .../kubernetes/model/client/ClientAdapter.kt | 38 +-- .../kubernetes/model/client/ClientConfig.kt | 111 ++++---- .../model/client/KubeConfigAdapter.kt | 94 ++++++- .../kubernetes/model/util/ConfigUtils.kt | 55 ++++ .../kubernetes/model/AllContextsTest.kt | 33 +-- .../model/client/ClientAdapterTest.kt | 11 +- .../model/client/ClientConfigTest.kt | 236 +++++++++--------- .../intellij/kubernetes/model/mocks/Mocks.kt | 6 +- 12 files changed, 345 insertions(+), 283 deletions(-) create mode 100644 src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/util/ConfigUtils.kt diff --git a/build.gradle.kts b/build.gradle.kts index 656ddeede..72e4fd54d 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -67,7 +67,7 @@ dependencies { // for unit tests testImplementation(libs.assertj.core) - testImplementation(libs.mockito.inline) + testImplementation(libs.mockito) testImplementation(libs.mockito.kotlin) testImplementation(libs.kotlin.test.junit) diff --git a/gradle.properties b/gradle.properties index f08c08c7c..61c1ae56e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -27,4 +27,5 @@ org.gradle.configuration-cache=true org.gradle.caching=true kotlin.daemon.jvm.options=-Xmx2g -org.gradle.jvmargs=-Xmx2g \ No newline at end of file +org.gradle.jvmargs=-Xmx2g + diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index bf5c1acc7..4272477ff 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,11 +1,11 @@ [versions] # libraries -kubernetes-client = "6.12.0" +kubernetes-client = "7.0.0" devtools-common = "1.9.7-SNAPSHOT" jackson-core = "2.17.0" commons-lang3 = "3.12.0" assertj-core = "3.22.0" -mockito-inline = "4.5.1" +mockito = "5.12.0" mockito-kotlin = "2.2.0" devtools-common-ui-test = "0.4.2" junit-platform = "1.10.3" @@ -19,7 +19,7 @@ kotlinJvm = "2.0.20" [libraries] openshift-client = { group = "io.fabric8", name = "openshift-client", version.ref = "kubernetes-client" } kubernetes-client = { group = "io.fabric8", name = "kubernetes-client", version.ref = "kubernetes-client" } -kubernetes-model = { group = "io.fabric8", name = "kubernetes-model", version.ref = "kubernetes-client" } +kubernetes-model = { group = "io.fabric8", name = "kubernetes-model-core", version.ref = "kubernetes-client" } kubernetes-model-common = { group = "io.fabric8", name = "kubernetes-model-common", version.ref = "kubernetes-client" } kubernetes-httpclient-okhttp = { group = "io.fabric8", name = "kubernetes-httpclient-okhttp", version.ref = "kubernetes-client" } devtools-common = { group = "com.redhat.devtools.intellij", name = "intellij-common", version.ref = "devtools-common" } @@ -27,7 +27,7 @@ jackson-core = { group = "com.fasterxml.jackson.core", name = "jackson-core", ve commons-lang3 = { group = "org.apache.commons", name = "commons-lang3", version.ref = "commons-lang3" } kotlin-test-junit = { group = "org.jetbrains.kotlin", name = "kotlin-test-junit", version.ref = "kotlinJvm" } assertj-core = { group = "org.assertj", name = "assertj-core", version.ref = "assertj-core" } -mockito-inline = { group = "org.mockito", name = "mockito-inline", version.ref = "mockito-inline" } +mockito = { group = "org.mockito", name = "mockito-core", version.ref = "mockito" } mockito-kotlin = { group = "com.nhaarman.mockitokotlin2", name = "mockito-kotlin", version.ref = "mockito-kotlin" } devtools-common-ui-test = { group = "com.redhat.devtools.intellij", name = "intellij-common-ui-test-library", version.ref = "devtools-common-ui-test" } junit-platform-launcher = { group = "org.junit.platform", name = "junit-platform-launcher", version.ref = "junit-platform" } diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt index d9add854c..e6e3a4daf 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContexts.kt @@ -12,7 +12,6 @@ package com.redhat.devtools.intellij.kubernetes.model import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.diagnostic.logger -import com.redhat.devtools.intellij.common.utils.ConfigHelper import com.redhat.devtools.intellij.common.utils.ConfigWatcher import com.redhat.devtools.intellij.common.utils.ExecHelper import com.redhat.devtools.intellij.kubernetes.model.client.ClientAdapter @@ -30,7 +29,6 @@ import com.redhat.devtools.intellij.kubernetes.telemetry.TelemetryService.PROP_O import io.fabric8.kubernetes.api.model.HasMetadata import io.fabric8.kubernetes.client.Config import io.fabric8.kubernetes.client.KubernetesClient -import java.nio.file.Paths import java.util.concurrent.CompletionException import java.util.concurrent.locks.ReentrantReadWriteLock import kotlin.concurrent.read @@ -83,11 +81,13 @@ open class AllContexts( namespace: String?, context: String? ) -> ClientAdapter - = { namespace, context -> ClientAdapter.Factory.create(namespace, context) } + = { namespace, config -> + ClientAdapter.Factory.create(namespace, config) + } ) : IAllContexts { init { - watchKubeConfig() + watchKubeConfigs() } private val lock = ReentrantReadWriteLock() @@ -148,7 +148,8 @@ open class AllContexts( ) : IActiveContext? { lock.write { try { - replaceClient(newClient, this.client.get()) + client.get()?.close() + client.set(newClient) newClient.config.save().join() current?.close() clearAllContexts() // causes reload of all contexts when accessed afterwards @@ -204,13 +205,6 @@ open class AllContexts( } } - private fun replaceClient(new: ClientAdapter, old: ClientAdapter?) - : ClientAdapter { - old?.close() - this.client.set(new) - return new - } - private fun createActiveContext(client: ClientAdapter?) : IActiveContext? { if (client == null) { @@ -241,8 +235,7 @@ open class AllContexts( } } - protected open fun watchKubeConfig() { - val path = Paths.get(Config.getKubeconfigFilename()) + protected open fun watchKubeConfigs() { /** * [ConfigWatcher] cannot add/remove listeners nor can it get closed (and stop the [java.nio.file.WatchService]). * We therefore have to create a single instance in here rather than using it in a shielded/private way within @@ -251,16 +244,16 @@ open class AllContexts( * The latter gets closed/recreated whenever the context changes in * [com.redhat.devtools.intellij.kubernetes.model.client.KubeConfigAdapter]. */ - val watcher = ConfigWatcher(path) { _, config: io.fabric8.kubernetes.api.model.Config? -> onKubeConfigChanged(config) } + val watcher = ConfigWatcher { config: Config? -> onKubeConfigChanged(config) } runAsync(watcher::run) } - protected open fun onKubeConfigChanged(fileConfig: io.fabric8.kubernetes.api.model.Config?) { + protected open fun onKubeConfigChanged(updated: Config?) { lock.read { - fileConfig ?: return + updated ?: return val client = client.get() ?: return - val clientConfig = client.config.configuration - if (ConfigHelper.areEqual(fileConfig, clientConfig)) { + val existing = client.config + if (existing.isEqualConfig(updated)) { return } this.client.reset() // create new client when accessed diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapter.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapter.kt index aab954de4..2ce06a6a3 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapter.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapter.kt @@ -33,7 +33,7 @@ open class OSClientAdapter(client: OpenShiftClient, private val kubeClient: Kube override val config by lazy { // openshift client configuration does not have kube config entries - ClientConfig(kubeClient) + ClientConfig(kubeClient.configuration) } override fun isOpenShift(): Boolean { @@ -56,32 +56,24 @@ open class KubeClientAdapter(client: KubernetesClient) : abstract class ClientAdapter(private val fabric8Client: C) { companion object Factory { + fun create( namespace: String? = null, context: String? = null, - clientBuilder: KubernetesClientBuilder = KubernetesClientBuilder(), - trustManagerProvider: ((toIntegrate: List) -> X509TrustManager) - = IDEATrustManager()::configure + clientBuilder: KubernetesClientBuilder? = null, + externalTrustManagerProvider: ((toIntegrate: List) -> X509TrustManager)? = null ): ClientAdapter { val config = Config.autoConfigure(context) - return create(namespace, config, clientBuilder, trustManagerProvider) - } - - fun create( - namespace: String? = null, - config: Config, - clientBuilder: KubernetesClientBuilder, - externalTrustManagerProvider: (toIntegrate: List) -> X509TrustManager - = IDEATrustManager()::configure - ): ClientAdapter { setNamespace(namespace, config) - val kubeClient = clientBuilder + val builder = clientBuilder ?: KubernetesClientBuilder() + val trustManager = externalTrustManagerProvider ?: IDEATrustManager()::configure + val kubeClient = builder .withConfig(config) - .withHttpClientBuilderConsumer { builder -> - setSslContext(builder, config, externalTrustManagerProvider) + .withHttpClientBuilderConsumer { httpClientBuilder -> + setSslContext(httpClientBuilder, config, trustManager) } .build() - return if (isOpenShift(kubeClient)) { + return if (ClusterHelper.isOpenShift(kubeClient)) { val osClient = kubeClient.adapt(NamespacedOpenShiftClient::class.java) OSClientAdapter(osClient, kubeClient) } else { @@ -89,14 +81,6 @@ abstract class ClientAdapter(private val fabric8Client: C) } } - private fun isOpenShift(client: KubernetesClient): Boolean { - return try { - ClusterHelper.isOpenShift(client) - } catch (e: Exception) { - false; - } - } - private fun setSslContext( builder: HttpClient.Builder, config: Config, @@ -124,7 +108,7 @@ abstract class ClientAdapter(private val fabric8Client: C) private val clients = ConcurrentHashMap, Client>() open val config by lazy { - ClientConfig(fabric8Client) + ClientConfig(fabric8Client.configuration) } abstract fun isOpenShift(): Boolean diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt index a7b63902b..83b27fe20 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfig.kt @@ -12,11 +12,11 @@ package com.redhat.devtools.intellij.kubernetes.model.client import com.redhat.devtools.intellij.common.utils.ConfigHelper import com.redhat.devtools.intellij.kubernetes.CompletableFutureUtils.PLATFORM_EXECUTOR -import io.fabric8.kubernetes.api.model.Context +import com.redhat.devtools.intellij.kubernetes.model.util.ConfigUtils import io.fabric8.kubernetes.api.model.NamedContext -import io.fabric8.kubernetes.client.Client import io.fabric8.kubernetes.client.Config import io.fabric8.kubernetes.client.internal.KubeConfigUtils +import java.io.File import java.util.concurrent.CompletableFuture import java.util.concurrent.Executor @@ -24,95 +24,74 @@ import java.util.concurrent.Executor * An adapter to access [io.fabric8.kubernetes.client.Config]. * It also saves the kube config [KubeConfigUtils] when it changes the client config. */ -open class ClientConfig(private val client: Client, private val executor: Executor = PLATFORM_EXECUTOR) { +open class ClientConfig( + private val config: Config, + private val createKubeConfigAdapter: (file: File, config: io.fabric8.kubernetes.api.model.Config?) -> KubeConfigAdapter + = { file, kubeConfig -> KubeConfigAdapter(file, kubeConfig) }, + private val getFileWithCurrentContextName: () -> Pair? + = { ConfigUtils.getFileWithCurrentContext() }, + private val getFileWithCurrentContext: (config: Config) -> File? + = { config -> config.file }, + private val executor: Executor = PLATFORM_EXECUTOR) +{ open var currentContext: NamedContext? get() { - return configuration.currentContext + return config.currentContext } set(context) { - configuration.currentContext = context + config.currentContext = context } open val allContexts: List get() { - return configuration.contexts ?: emptyList() + return config.contexts ?: emptyList() } - open val configuration: Config by lazy { - client.configuration - } - - protected open val kubeConfig: KubeConfigAdapter by lazy { - KubeConfigAdapter() + /** + * Returns `true` if the given config is equal in + * * current context + * * + */ + fun isEqualConfig(config: Config): Boolean { + return ConfigHelper.areEqualCurrentContext(config, this.config) + && ConfigHelper.areEqualContexts(config, this.config) + && ConfigHelper.areEqualCluster(config, this.config) + && ConfigHelper.areEqualAuthInfo(config, this.config) } fun save(): CompletableFuture { return CompletableFuture.supplyAsync( { - if (!kubeConfig.exists()) { - return@supplyAsync false - } - val fromFile = kubeConfig.load() ?: return@supplyAsync false - return@supplyAsync if (setCurrentContext( - currentContext, - KubeConfigUtils.getCurrentContext(fromFile), - fromFile - ).or( // no short-circuit - setCurrentNamespace( - currentContext?.context, - KubeConfigUtils.getCurrentContext(fromFile)?.context - ) - ) - ) { - kubeConfig.save(fromFile) - true - } else { - false + val modified = HashSet() + + setCurrentContext(modified) + setCurrentNamespace(modified) + + modified.forEach { kubeConfig -> + kubeConfig.save() } + modified.isNotEmpty() }, executor ) } - private fun setCurrentContext( - currentContext: NamedContext?, - kubeConfigCurrentContext: NamedContext?, - kubeConfig: io.fabric8.kubernetes.api.model.Config - ): Boolean { - return if (currentContext != null - && !ConfigHelper.areEqual(currentContext, kubeConfigCurrentContext) - ) { - kubeConfig.currentContext = currentContext.name - true - } else { - false + private fun setCurrentContext(modified: HashSet) { + val fileWithCurrentContext = getFileWithCurrentContextName.invoke() + if (fileWithCurrentContext != null) { + val kubeConfig = createKubeConfigAdapter(fileWithCurrentContext.first, fileWithCurrentContext.second) + if (kubeConfig.setCurrentContext(config.currentContext?.name)) { + modified.add(kubeConfig) + } } } - /** - * Sets the namespace in the given source [Context] to the given target [Context]. - * Does nothing if the target config has no current context - * or if the source config has no current context - * or if setting it would not change it. - * - * @param source Context whose namespace should be copied - * @param target Context whose namespace should be overriden - * @return - */ - private fun setCurrentNamespace( - source: Context?, - target: Context? - ): Boolean { - val sourceNamespace = source?.namespace ?: return false - val targetNamespace = target?.namespace - return if (target != null - && sourceNamespace != targetNamespace - ) { - target.namespace = source.namespace - true - } else { - false + private fun setCurrentNamespace(modified: HashSet) { + val fileWithCurrentNamespace = getFileWithCurrentContext.invoke(config) ?: return + val kubeConfig = createKubeConfigAdapter(fileWithCurrentNamespace, null) + if (kubeConfig.setCurrentNamespace(config.currentContext?.name, config.namespace)) { + modified.add(kubeConfig) } } diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/KubeConfigAdapter.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/KubeConfigAdapter.kt index 27bdfdeca..aacf298d2 100644 --- a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/KubeConfigAdapter.kt +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/KubeConfigAdapter.kt @@ -10,7 +10,9 @@ ******************************************************************************/ package com.redhat.devtools.intellij.kubernetes.model.client +import com.intellij.openapi.util.io.FileUtil import io.fabric8.kubernetes.api.model.Config +import io.fabric8.kubernetes.api.model.Context import io.fabric8.kubernetes.client.internal.KubeConfigUtils import java.io.File @@ -19,24 +21,98 @@ import java.io.File * (but may be configured to be at a different location). This class respects this by relying on the * {@link io.fabric8.kubernetes.client.Config} for the location instead of using a hard coded path. */ -class KubeConfigAdapter { +open class KubeConfigAdapter(private val file: File, private var _config: Config? = null) { - private val file: File by lazy { - File(io.fabric8.kubernetes.client.Config.getKubeconfigFilename()) + private val config: Config? + /** + * Returns the config that was given to this class at construction time or + * parses the file that was given to this instance when it was constructed + */ + get() { + if (_config != null) { + return _config + } else { + _config = load() + return _config + } + } + + private var modified: Boolean = false + + /* for testing purposes */ + protected open fun load(): Config? { + if (!exists()) { + return null + } + return KubeConfigUtils.parseConfig(file) } - fun exists(): Boolean { + /* for testing purposes */ + protected open fun exists(): Boolean { return file.exists() } - fun load(): Config? { - if (!exists()) { + fun save() { + val config = this.config ?: return + KubeConfigUtils.persistKubeConfigIntoFile(config, file) + } + + fun setCurrentContext(newCurrentContext: String?): Boolean { + val oldCurrentContext = config?.currentContext + return if (newCurrentContext != oldCurrentContext) { + _config?.currentContext = newCurrentContext + this.modified = _config != null + return modified + } else { + false + } + } + + /** + * Sets the namespace in the given source [Context] to the given target [Context]. + * Does nothing if the target config has no current context + * or if the source config has no current context + * or if setting it would not change it. + * + * @param source Context whose namespace should be copied + * @param target Context whose namespace should be overriden + * @return + */ + fun setCurrentNamespace(context: String?, namespace: String?): Boolean { + if (context == null + || namespace == null) { + return false + } + val target = getContext(context) ?: return false + return if (namespace != target.namespace) { + target.namespace = namespace + true + } else { + false + } + } + + private fun getContext(name: String?): Context? { + if (name == null) { return null } - return KubeConfigUtils.parseConfig(file) + val config = this.config ?: return null + return config.contexts + .filter { namedContext -> + name == namedContext.name + }.firstNotNullOfOrNull { namedContext -> + namedContext.context + } + } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is KubeConfigAdapter) return false + + return FileUtil.filesEqual(file, other.file) } - fun save(config: Config) { - KubeConfigUtils.persistKubeConfigIntoFile(config, file.absolutePath) + override fun hashCode(): Int { + return FileUtil.fileHashCode(file) } } \ No newline at end of file diff --git a/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/util/ConfigUtils.kt b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/util/ConfigUtils.kt new file mode 100644 index 000000000..068ffc54f --- /dev/null +++ b/src/main/kotlin/com/redhat/devtools/intellij/kubernetes/model/util/ConfigUtils.kt @@ -0,0 +1,55 @@ +/******************************************************************************* + * Copyright (c) 2024 Red Hat, Inc. + * Distributed under license by Red Hat, Inc. All rights reserved. + * This program is made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v20.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + ******************************************************************************/ +package com.redhat.devtools.intellij.kubernetes.model.util + +import io.fabric8.kubernetes.api.model.NamedContext +import io.fabric8.kubernetes.client.Config +import io.fabric8.kubernetes.client.KubernetesClientException +import io.fabric8.kubernetes.client.internal.KubeConfigUtils +import io.fabric8.kubernetes.client.utils.KubernetesSerialization +import java.io.File +import java.nio.file.Files +import java.nio.file.Path + +object ConfigUtils { + + fun getFileWithCurrentContext(): Pair? { + return Config.getKubeconfigFilenames() + .asSequence() + .mapNotNull { filepath: String? -> + if (filepath != null) { + File(filepath) + } else { + null + } + } + .filter { file: File -> + file.exists() + && file.isFile + } + .mapNotNull { file: File -> + val config = KubeConfigUtils.parseConfig(file) ?: return@mapNotNull null + Pair(file, config) + } + .filter { pair: Pair -> + pair.second.currentContext?.isNotEmpty() != null + } + .firstOrNull() + } + + fun getCurrentContext(config: io.fabric8.kubernetes.api.model.Config): NamedContext? { + return config.contexts.find { + context -> context.name == config.currentContext + } + } + + +} diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContextsTest.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContextsTest.kt index 6c7273689..cdebbbe4c 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContextsTest.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/AllContextsTest.kt @@ -26,6 +26,7 @@ import com.redhat.devtools.intellij.kubernetes.model.context.IActiveContext import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.NAMESPACE1 import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.NAMESPACE2 import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.NAMESPACE3 +import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.config import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.namedContext import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.resource import com.redhat.devtools.intellij.kubernetes.model.mocks.Mocks @@ -75,7 +76,7 @@ class AllContextsTest { on { oauthToken } doReturn token } private val client = client(true) - private val clientConfig = clientConfig(currentContext, contexts, configuration) + private val clientConfig = clientConfig(currentContext, contexts) private val clientAdapter = clientAdapter(clientConfig, client) private val clientFactory = clientFactory(clientAdapter) @@ -336,7 +337,7 @@ class AllContextsTest { @Test fun `#setCurrentNamespace(namespace) should return null if current context is null`() { // given - val clientConfig = clientConfig(null, contexts, configuration) + val clientConfig = clientConfig(null, contexts) val client = client(true) val clientAdapter = clientAdapter(clientConfig, client) val clientFactory = clientFactory(clientAdapter) @@ -395,22 +396,14 @@ class AllContextsTest { @Test fun `#onKubeConfigChanged() should NOT fire if existing config and given config are equal`() { // given - val kubeConfig = ConfigBuilder() - .withCurrentContext(clientConfig.currentContext?.name) - .withContexts(clientConfig.allContexts) - .withUsers(NamedAuthInfoBuilder() - .withName(currentContext.context.user) - .withNewUser() - .withToken(clientConfig.configuration.oauthToken) - .endUser() - .build()) - .build() + val config = config(currentContext, contexts) // when - allContexts.onKubeConfigChanged(kubeConfig) + allContexts.onKubeConfigChanged(config) // then verify(modelChange, never()).fireAllContextsChanged() } +/* @Test fun `#onKubeConfigChanged() should fire if given config has different current context`() { // given @@ -492,7 +485,7 @@ class AllContextsTest { // then verify(activeContext).close() } - +*/ /** * Returns a client mock that answers with the given boolean to the call * [client.namespaces().withName("").isReady] @@ -530,6 +523,10 @@ class AllContextsTest { var watchStarted = false + public override fun onKubeConfigChanged(updated: io.fabric8.kubernetes.client.Config?) { + super.onKubeConfigChanged(updated) + } + override fun reportTelemetry(context: IActiveContext) { // prevent telemetry reporting } @@ -538,15 +535,9 @@ class AllContextsTest { runnable.invoke() // run directly, not in IDEA pooled threads } - override fun watchKubeConfig() { + override fun watchKubeConfigs() { // don't watch filesystem (override super method) watchStarted = true } - - /** override with public method so that it can be tested**/ - public override fun onKubeConfigChanged(fileConfig: Config?) { - super.onKubeConfigChanged(fileConfig) - } - } } \ No newline at end of file diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapterTest.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapterTest.kt index 577b5be43..61dabbf02 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapterTest.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientAdapterTest.kt @@ -124,7 +124,7 @@ class ClientAdapterTest { val config = config(ctx1, listOf(ctx1, ctx2)) val clientBuilder = createClientBuilder(false) // when - ClientAdapter.Factory.create(namespace, config, clientBuilder, trustManagerProvider) + ClientAdapter.Factory.create(namespace, ctx1.name, clientBuilder, trustManagerProvider) // then verify(config).namespace = namespace verify(config.currentContext.context).namespace = namespace @@ -133,10 +133,9 @@ class ClientAdapterTest { @Test fun `#create should call trust manager provider`() { // given - val config = mock() val clientBuilder = createClientBuilder(false) // when - ClientAdapter.Factory.create("namespace", config, clientBuilder, trustManagerProvider) + ClientAdapter.Factory.create("namespace", "context", clientBuilder, trustManagerProvider) // then verify(trustManagerProvider).invoke(any()) } @@ -144,10 +143,9 @@ class ClientAdapterTest { @Test fun `#create should return KubeClientAdapter if cluster is NOT OpenShift`() { // given - val config = mock() val clientBuilder = createClientBuilder(false) // when - val adapter = ClientAdapter.Factory.create("namespace", config, clientBuilder, trustManagerProvider) + val adapter = ClientAdapter.Factory.create("namespace", "context", clientBuilder, trustManagerProvider) // then assertThat(adapter).isInstanceOf(KubeClientAdapter::class.java) } @@ -155,10 +153,9 @@ class ClientAdapterTest { @Test fun `#create should return OSClientAdapter if cluster is OpenShift`() { // given - val config = mock() val clientBuilder = createClientBuilder(true) // when - val adapter = ClientAdapter.Factory.create("namespace", config, clientBuilder, trustManagerProvider) + val adapter = ClientAdapter.Factory.create("namespace", "context", clientBuilder, trustManagerProvider) // then assertThat(adapter).isInstanceOf(OSClientAdapter::class.java) } diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt index 898c989b5..0e0dd738d 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/client/ClientConfigTest.kt @@ -10,41 +10,39 @@ ******************************************************************************/ package com.redhat.devtools.intellij.kubernetes.model.client -import com.nhaarman.mockitokotlin2.any -import com.nhaarman.mockitokotlin2.argThat -import com.nhaarman.mockitokotlin2.doReturn -import com.nhaarman.mockitokotlin2.mock -import com.nhaarman.mockitokotlin2.never -import com.nhaarman.mockitokotlin2.spy -import com.nhaarman.mockitokotlin2.verify -import com.nhaarman.mockitokotlin2.whenever -import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks -import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.doReturnCurrentContextAndAllContexts +import com.nhaarman.mockitokotlin2.* +import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.config +import com.redhat.devtools.intellij.kubernetes.model.mocks.ClientMocks.namedContext import io.fabric8.kubernetes.api.model.ConfigBuilder -import io.fabric8.kubernetes.api.model.ContextBuilder import io.fabric8.kubernetes.api.model.NamedContext -import io.fabric8.kubernetes.api.model.NamedContextBuilder -import io.fabric8.kubernetes.client.Client import io.fabric8.kubernetes.client.Config -import io.fabric8.kubernetes.client.internal.KubeConfigUtils import org.assertj.core.api.Assertions.assertThat import org.junit.Test +import java.io.File class ClientConfigTest { private val namedContext1 = - context("ctx1", "namespace1", "cluster1", "user1") + namedContext("ctx1", "namespace1", "cluster1", "user1") private val namedContext2 = - context("ctx2", "namespace2", "cluster2", "user2") + namedContext("ctx2", "namespace2", "cluster2", "user2") private val namedContext3 = - context("ctx3", "namespace3", "cluster3", "user3") + namedContext("ctx3", "namespace3", "cluster3", "user3") private val currentContext = namedContext2 private val allContexts = listOf(namedContext1, namedContext2, namedContext3) - private val clientKubeConfig: Config = ClientMocks.config(currentContext, allContexts) - private val client: Client = createClient(clientKubeConfig) - private val fileKubeConfig: io.fabric8.kubernetes.api.model.Config = apiConfig(currentContext.name, allContexts) - private val kubeConfigAdapter: KubeConfigAdapter = kubeConfig(true, fileKubeConfig) - private val clientConfig = spy(TestableClientConfig(client, kubeConfigAdapter)) + private val kubeConfigFile: File = mock() + private val kubeConfig: io.fabric8.kubernetes.api.model.Config = apiConfig(currentContext.name, allContexts) + private val kubeConfigAdapter: KubeConfigAdapter = kubeConfigAdapter(true, kubeConfig, + false, // setCurrentContext is NOT successful bcs same value + false) // setCurrentNamespace is NOT successful bcs same value + private val config: Config = config(currentContext, allContexts) + private val createKubeConfigAdapter: (file: File, kubeConfig: io.fabric8.kubernetes.api.model.Config?) -> KubeConfigAdapter = { _, _ -> kubeConfigAdapter } + private val clientConfig = clientConfig( + config, + createKubeConfigAdapter, + { Pair(kubeConfigFile, kubeConfig) }, + { _ -> kubeConfigFile } + ) @Test fun `#currentContext should return config#currentContext`() { @@ -52,7 +50,7 @@ class ClientConfigTest { // when clientConfig.currentContext // then - verify(clientKubeConfig).currentContext + verify(config).currentContext } @Test @@ -61,7 +59,7 @@ class ClientConfigTest { // when clientConfig.allContexts // then - verify(clientKubeConfig).contexts + verify(config).contexts } @Test @@ -82,146 +80,131 @@ class ClientConfigTest { assertThat(isCurrent).isFalse() } - @Test - fun `#save should NOT save if kubeConfig doesnt exist`() { - // given - doReturn(false) - .whenever(kubeConfigAdapter).exists() - // when - clientConfig.save().join() - // then - verify(kubeConfigAdapter, never()).save(any()) - } - @Test fun `#save should NOT save if kubeConfig has same current context same namespace and same current context as client config`() { // given // when clientConfig.save().join() // then - verify(kubeConfigAdapter, never()).save(any()) + verify(kubeConfigAdapter, never()).save() } @Test fun `#save should save if kubeConfig has different current context as client config`() { // given - clientKubeConfig.currentContext.name = namedContext3.name - assertThat(fileKubeConfig.currentContext).isNotEqualTo(clientKubeConfig.currentContext.name) + val kubeConfigAdapter: KubeConfigAdapter = kubeConfigAdapter(true, kubeConfig, + true, // setCurrentContext is successful + false) // setCurrentNamespace is NOT successful + val createKubeConfigAdapter: (file: File, kubeConfig: io.fabric8.kubernetes.api.model.Config?) -> KubeConfigAdapter = { _, _ -> kubeConfigAdapter } + val clientConfig = clientConfig( + config, + createKubeConfigAdapter, + { Pair(kubeConfigFile, kubeConfig) }, + { _ -> kubeConfigFile } + ) // when clientConfig.save().join() // then - verify(kubeConfigAdapter).save(any()) + verify(kubeConfigAdapter).save() } @Test fun `#save should save if kubeConfig has same current context but current namespace that differs from client config`() { // given - val newCurrentContext = context( - currentContext.name, - "R2-D2", - currentContext.context.cluster, - currentContext.context.user) - val newAllContexts = mutableListOf(*allContexts.toTypedArray()) - newAllContexts.removeIf { it.name == currentContext.name } - newAllContexts.add(newCurrentContext) - fileKubeConfig.contexts = newAllContexts + val kubeConfigAdapter: KubeConfigAdapter = kubeConfigAdapter(true, kubeConfig, + false, // setCurrentContext is NOT successful + true // setCurrentNamespace is successful + ) + val createKubeConfigAdapter: (file: File, kubeConfig: io.fabric8.kubernetes.api.model.Config?) -> KubeConfigAdapter = + { _, _ -> kubeConfigAdapter } + val clientConfig = clientConfig( + config, + createKubeConfigAdapter, + { Pair(kubeConfigFile, kubeConfig) }, + { _ -> kubeConfigFile } + ) // when clientConfig.save().join() // then - verify(kubeConfigAdapter).save(any()) + verify(kubeConfigAdapter).save() } @Test - fun `#save should update current context in kube config if differs from current context in client config`() { + fun `#save should save only 1 file if current context name and current namespace are changed but in same file`() { // given - val newCurrentContext = namedContext3 - doReturn(newCurrentContext) - .whenever(clientKubeConfig).currentContext - assertThat(KubeConfigUtils.getCurrentContext(fileKubeConfig)) - .isNotEqualTo(clientKubeConfig.currentContext) + val kubeConfigAdapter: KubeConfigAdapter = kubeConfigAdapter(true, kubeConfig, + true, // setCurrentContext is successful + true // setCurrentNamespace is successful + ) + val createKubeConfigAdapter: (file: File, kubeConfig: io.fabric8.kubernetes.api.model.Config?) -> KubeConfigAdapter = + { _, _ -> kubeConfigAdapter } + val clientConfig = clientConfig( + config, + createKubeConfigAdapter, + { Pair(kubeConfigFile, kubeConfig) }, + { _ -> kubeConfigFile } + ) // when clientConfig.save().join() // then - verify(kubeConfigAdapter).save(argThat { - this.currentContext == newCurrentContext.name - }) + verify(kubeConfigAdapter, times(1)).save() } @Test - fun `#save should leave current namespace in old context untouched when updating current context in kube config`() { + fun `#save should save 2 file if current context name and current namespace are changed and in different files`() { // given - val newCurrentContext = namedContext3 - doReturn(newCurrentContext) - .whenever(clientKubeConfig).currentContext - assertThat(KubeConfigUtils.getCurrentContext(fileKubeConfig)) - .isNotEqualTo(clientKubeConfig.currentContext) - val context = KubeConfigUtils.getCurrentContext(fileKubeConfig) - val currentBeforeSave = context.name - val namespaceBeforeSave = context.context.namespace + val currentContextFile = mock() + val currentContextKubeConfigAdapter: KubeConfigAdapter = kubeConfigAdapter(true, kubeConfig, + true, // setCurrentContext is successful + false // setCurrentNamespace is NOT successful + ) + val currentNamespaceFile = mock() + val currentNamespaceKubeConfigAdapter: KubeConfigAdapter = kubeConfigAdapter(true, kubeConfig, + false, // setCurrentContext is NOT successful + true // setCurrentNamespace is successful + ) + val createKubeConfigAdapter: (file: File, kubeConfig: io.fabric8.kubernetes.api.model.Config?) -> KubeConfigAdapter = + { file, _ -> + if (file == currentContextFile) { + currentContextKubeConfigAdapter + } else { + currentNamespaceKubeConfigAdapter + } + } + val clientConfig = clientConfig( + config, + createKubeConfigAdapter, + { Pair(currentContextFile, kubeConfig) }, + { _ -> currentNamespaceFile } + ) // when clientConfig.save().join() // then - verify(kubeConfigAdapter).save(argThat { - val afterSave = fileKubeConfig.contexts.find { - namedContext -> namedContext.name == currentBeforeSave } - afterSave!!.context.namespace == namespaceBeforeSave - }) + verify(currentContextKubeConfigAdapter, times(1)).save() + verify(currentNamespaceKubeConfigAdapter, times(1)).save() } - @Test - fun `#save should update current namespace in kube config if only differs from current in client config but not in current context`() { - // given - val newCurrentContext = context(currentContext.name, - "RD-2D", - currentContext.context.cluster, - currentContext.context.user) - val newAllContexts = replaceCurrentContext(newCurrentContext, currentContext.name, allContexts) - doReturnCurrentContextAndAllContexts(newCurrentContext, newAllContexts, clientKubeConfig) - assertThat(KubeConfigUtils.getCurrentContext(fileKubeConfig).context.namespace) - .isNotEqualTo(clientKubeConfig.currentContext.context.namespace) - // when - clientConfig.save().join() - // then - verify(kubeConfigAdapter).save(argThat { - this.currentContext == this@ClientConfigTest.currentContext.name - && KubeConfigUtils.getCurrentContext(this).context.namespace == newCurrentContext.context.namespace - }) - } - - private fun replaceCurrentContext( - newContext: NamedContext, - currentContext: String, - allContexts: List - ): List { - val newAllContexts = mutableListOf(*allContexts.toTypedArray()) - val existingContext = clientKubeConfig.contexts.find { it.name == currentContext } - newAllContexts.remove(existingContext) - newAllContexts.add(newContext) - return newAllContexts - } - - private fun context(name: String, namespace: String, cluster: String, user: String): NamedContext { - val context = ContextBuilder() - .withNamespace(namespace) - .withCluster(cluster) - .withUser(user) - .build() - return NamedContextBuilder() - .withName(name) - .withContext(context) - .build() + private fun kubeConfigAdapter( + exists: Boolean, + config: io.fabric8.kubernetes.api.model.Config, + setCurrentContext: Boolean, + setCurrentNamespace: Boolean, + ): KubeConfigAdapter { + return mock { + on { exists() } doReturn exists + on { load() } doReturn config + on { setCurrentContext(anyOrNull()) } doReturn setCurrentContext + on { setCurrentNamespace(anyOrNull(), anyOrNull()) } doReturn setCurrentNamespace + } } - private fun createClient(config: Config): Client { - return mock { - on { configuration } doReturn config + open class OverridableKubeConfigAdapter(file: File): KubeConfigAdapter(file) { + public override fun load(): io.fabric8.kubernetes.api.model.Config? { + return super.load() } - } - private fun kubeConfig(exists: Boolean, config: io.fabric8.kubernetes.api.model.Config): com.redhat.devtools.intellij.kubernetes.model.client.KubeConfigAdapter { - return mock { - on { exists() } doReturn exists - on { load() } doReturn config + public override fun exists(): Boolean { + return super.exists() } } @@ -232,6 +215,11 @@ class ClientConfigTest { .build() } - private class TestableClientConfig(client: Client, override val kubeConfig: KubeConfigAdapter) - : ClientConfig(client, { it.run() }) + private fun clientConfig(config: Config, + createKubeConfigAdapter: (file: File, kubeConfig: io.fabric8.kubernetes.api.model.Config?) -> KubeConfigAdapter, + getFileWithCurrentContextName: () -> Pair?, + getFileWithCurrentContext: (config: Config) -> File?, + ) : ClientConfig { + return ClientConfig(config, createKubeConfigAdapter, getFileWithCurrentContextName, getFileWithCurrentContext, { it.run() }) + } } diff --git a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt index ab6dbce7c..154ab644c 100644 --- a/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt +++ b/src/test/kotlin/com/redhat/devtools/intellij/kubernetes/model/mocks/Mocks.kt @@ -33,6 +33,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata import io.fabric8.kubernetes.api.model.NamedContext import io.fabric8.kubernetes.api.model.Namespace import io.fabric8.kubernetes.client.Client +import io.fabric8.kubernetes.client.Config import io.fabric8.kubernetes.client.KubernetesClient import io.fabric8.kubernetes.client.Watch import io.fabric8.kubernetes.client.Watcher @@ -208,8 +209,7 @@ object Mocks { fun clientConfig( currentContext: NamedContext?, - allContexts: List, - configuration: io.fabric8.kubernetes.client.Config = mock() + allContexts: List ): ClientConfig { val saveFuture: CompletableFuture = mock() return mock { @@ -218,9 +218,7 @@ object Mocks { invocation.getArgument(0) == mock.currentContext } on { this.allContexts } doReturn allContexts - on { this.configuration } doReturn configuration on { this.save() } doReturn saveFuture } } - }