diff --git a/CHANGELOG.md b/CHANGELOG.md index 82912ce8..9d700b16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,13 @@ ### Added -- support for using a locally provided CLI binary when downloads are disabled +- `Binary destination` can now point directly to an executable, used as-is; otherwise it is treated as a base directory + as before + +### Removed + +- support for enabling or disabling the fallback to data dir. CLI resolution will always fall back on data dir if binary + destination is not configured. ## 0.8.6 - 2026-03-05 @@ -17,7 +23,8 @@ ### Added - support for configuring the SSH connection timeout, defaults to 10 seconds -- enhanced IDE resolution by supporting latest EAP, latest release, latest installed labels with clear fallback behavior in URI handlers +- enhanced IDE resolution by supporting latest EAP, latest release, latest installed labels with clear fallback behavior + in URI handlers ### Fixed diff --git a/README.md b/README.md index 53c3359a..cd598a5b 100644 --- a/README.md +++ b/README.md @@ -447,19 +447,16 @@ storage paths. The options can be configured from the plugin's main Workspaces p - `Binary source` specifies the source URL or relative path from which the Coder CLI should be downloaded. If a relative path is provided, it is resolved against the deployment domain. -- `Enable downloads` allows automatic downloading of the CLI if the current version is missing or outdated. - Defaults to enabled. +- `Enable downloads` allows automatic downloading of the CLI if the current version is missing or outdated. Enabled by + default. - `Binary destination` specifies where the CLI binary is placed. This can be a path to an existing executable (used as-is) or a base directory (the CLI is placed under a host-specific subdirectory). If blank, the data directory is used. Supports `~` and `$HOME` expansion. -- `Enable binary directory fallback` when enabled, if the binary destination is not writable the - plugin falls back to the data directory instead of failing. Only takes effect when downloads are - enabled and the binary destination differs from the data directory. Defaults to disabled. - - `Data directory` directory where deployment-specific data such as session tokens and CLI binaries - are stored. Each deployment gets a host-specific subdirectory (e.g. `coder.example.com`). + are stored. Each deployment gets a host-specific subdirectory (e.g. `coder.example.com`). Supports `~` and `$HOME` + expansion. - `Header command` command that outputs additional HTTP headers. Each line of output must be in the format key=value. The environment variable CODER_URL will be available to the command process. @@ -477,20 +474,21 @@ storage paths. The options can be configured from the plugin's main Workspaces p #### How CLI resolution works When connecting to a deployment the plugin ensures a compatible CLI binary is available. -The settings above interact as follows: - -1. If a CLI already exists at the binary destination and its version matches the deployment, it is - used immediately. -2. If **downloads are enabled**, the plugin downloads the matching version to the binary destination. - - If the download fails with a permission error and **binary directory fallback** is enabled (and - the binary destination is not already in the data directory), the plugin checks whether the data - directory already has a matching CLI. If so it is used; otherwise the plugin downloads to the - data directory instead. - - Any other download error is reported to the user. -3. If **downloads are disabled**, the plugin checks the data directory for a CLI whose version - matches the deployment. If no exact match is found anywhere, whichever CLI is available is - returned — preferring the binary destination unless it is missing, in which case the data - directory CLI is used regardless of its version. If no CLI exists at all, an error is raised. +The binary location is resolved as follows: + +- If **binary destination** points to an existing executable file, it is used as-is. +- If **binary destination** is set but is not an executable file, it is treated as a base + directory and the CLI is placed under a host-specific subdirectory (e.g. + `/coder.example.com/`). +- If **binary destination** is not set, the data directory is used instead. + +Once the binary location is resolved: + +1. If a CLI already exists there and its version matches the deployment, it is used immediately. +2. Otherwise, if **downloads are enabled**, the plugin downloads the matching version to that location. + Any download error is reported to the user. +3. If **downloads are disabled** and the CLI exists but its version does not match, the stale + CLI is used with a warning. If no CLI exists at all, an error is raised. ### TLS settings diff --git a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt index 4bbe68af..6a8617fe 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/CoderCLIManager.kt @@ -48,24 +48,17 @@ internal data class Version( /** - * Do as much as possible to get a valid, up-to-date CLI. + * Best effort to get an up-to-date CLI. * * 1. Create a CLI manager for the deployment URL. * 2. If the CLI version matches the build version, return it immediately. - * 3. If downloads are enabled, attempt to download the CLI. + * 3. Otherwise, if downloads are enabled, attempt to download the CLI. * a. On success, return the CLI. - * b. On [java.nio.file.AccessDeniedException]: rethrow if the binary - * path parent equals the data directory or if binary directory - * fallback is disabled. Otherwise, if the fallback data directory - * CLI already matches the build version return it; if not, download - * to the data directory and return the fallback CLI. - * c. Any other exception propagates to the caller. + * b. Any exception propagates to the user. * 4. If downloads are disabled: - * a. If the data directory CLI version matches, return it. - * b. If neither the configured binary nor the data directory CLI can - * report a version, throw [IllegalStateException]. - * c. Prefer the configured binary; fall back to the data directory CLI - * only when the configured binary is missing or unexecutable. + * a. [IllegalStateException] is raised if the CLI does not exist (look into binary destination if it was configured, + * fallback to data dir otherwise) + * b. Otherwise, warn the user and return the mismatched version. */ suspend fun ensureCLI( context: CoderToolboxContext, @@ -95,44 +88,15 @@ suspend fun ensureCLI( // If downloads are enabled download the new version. if (settings.enableDownloads) { reportProgress("Downloading Coder CLI...") - try { - cli.download(buildVersion, showTextProgress) - return cli - } catch (e: java.nio.file.AccessDeniedException) { - // Might be able to fall back to the data directory. - val binPath = settings.binPath(deploymentURL) - val dataDir = settings.dataDir(deploymentURL) - if (binPath.parent == dataDir || !settings.enableBinaryDirectoryFallback) { - throw e - } - // fall back to the data directory. - val fallbackCLI = CoderCLIManager(context, deploymentURL, true) - val fallbackMatches = fallbackCLI.matchesVersion(buildVersion) - if (fallbackMatches == true) { - reportProgress("Local CLI version from data directory matches server version: $buildVersion") - return fallbackCLI - } - - reportProgress("Downloading Coder CLI to the data directory...") - fallbackCLI.download(buildVersion, showTextProgress) - return fallbackCLI - } - } - - // Try falling back to the data directory. - val dataCLI = CoderCLIManager(context, deploymentURL, true) - val dataCLIMatches = dataCLI.matchesVersion(buildVersion) - if (dataCLIMatches == true) { - reportProgress("Local CLI version from data directory matches server version: $buildVersion") - return dataCLI + cli.download(buildVersion, showTextProgress) + return cli } - // Prefer the binary directory unless the data directory has a - // working binary and the binary directory does not. - if (cliMatches == null && dataCLIMatches == null && !settings.enableDownloads) { + if (cliMatches == null) { throw IllegalStateException("Can't resolve Coder CLI and downloads are disabled") } - return if (cliMatches == null && dataCLIMatches != null) dataCLI else cli + reportProgress("Downloads are disabled, and a cached CLI is used which does not match the server version $buildVersion and could cause compatibility issues") + return cli } /** @@ -151,16 +115,13 @@ data class Features( class CoderCLIManager( private val context: CoderToolboxContext, // The URL of the deployment this CLI is for. - private val deploymentURL: URL, - // If the binary directory is not writable, this can be used to force the - // manager to download to the data directory instead. - private val forceDownloadToData: Boolean = false, + private val deploymentURL: URL ) { private val downloader = createDownloadService() private val gpgVerifier = GPGVerifier(context) val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentURL) - val localBinaryPath: Path = context.settingsStore.binPath(deploymentURL, forceDownloadToData) + val localBinaryPath: Path = context.settingsStore.binPath(deploymentURL) val coderConfigPath: Path = context.settingsStore.dataDir(deploymentURL).resolve("config") private fun createDownloadService(): CoderDownloadService { @@ -180,7 +141,7 @@ class CoderCLIManager( .build() val service = retrofit.create(CoderDownloadApi::class.java) - return CoderDownloadService(context, service, deploymentURL, forceDownloadToData) + return CoderDownloadService(context, service, deploymentURL) } /** diff --git a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt index 2c2e87c6..da5e2f21 100644 --- a/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt +++ b/src/main/kotlin/com/coder/toolbox/cli/downloader/CoderDownloadService.kt @@ -38,17 +38,17 @@ private val SUPPORTED_BIN_MIME_TYPES = listOf( "application/x-ms-dos-executable", "application/vnd.microsoft.portable-executable" ) + /** * Handles the download steps of Coder CLI */ class CoderDownloadService( private val context: CoderToolboxContext, private val downloadApi: CoderDownloadApi, - private val deploymentUrl: URL, - forceDownloadToData: Boolean, + private val deploymentUrl: URL ) { private val remoteBinaryURL: URL = context.settingsStore.binSource(deploymentUrl) - private val cliFinalDst: Path = context.settingsStore.binPath(deploymentUrl, forceDownloadToData) + private val cliFinalDst: Path = context.settingsStore.binPath(deploymentUrl) private val cliTempDst: Path = cliFinalDst.resolveSibling("${cliFinalDst.name}.tmp") suspend fun downloadCli(buildVersion: String, showTextProgress: (String) -> Unit): DownloadResult { diff --git a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt index a925ba1e..74013751 100644 --- a/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt +++ b/src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt @@ -90,12 +90,6 @@ interface ReadOnlyCoderSettings { */ val enableDownloads: Boolean - /** - * Whether to allow the plugin to fall back to the data directory when the - * CLI directory is not writable. - */ - val enableBinaryDirectoryFallback: Boolean - /** * An external command that outputs additional HTTP headers added to all * requests. The command must output each header as `key=value` on its own @@ -179,9 +173,9 @@ interface ReadOnlyCoderSettings { fun binSource(url: URL): URL /** - * To where the specified deployment should download the binary. + * Returns a path to where the specified deployment should place the CLI binary. */ - fun binPath(url: URL, forceDownloadToData: Boolean = false): Path + fun binPath(url: URL): Path /** * Return the URL and token from the config, if they exist. diff --git a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt index 1dcc8959..08341326 100644 --- a/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt +++ b/src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt @@ -54,8 +54,6 @@ class CoderSettingsStore( override val globalDataDirectory: String get() = getDefaultGlobalDataDir().normalize().toString() override val globalConfigDir: String get() = getDefaultGlobalConfigDir().normalize().toString() override val enableDownloads: Boolean get() = store[ENABLE_DOWNLOADS]?.toBooleanStrictOrNull() ?: true - override val enableBinaryDirectoryFallback: Boolean - get() = store[ENABLE_BINARY_DIR_FALLBACK]?.toBooleanStrictOrNull() ?: false override val headerCommand: String? get() = store[HEADER_COMMAND] override val tls: ReadOnlyTLSSettings get() = TLSSettings( @@ -127,30 +125,19 @@ class CoderSettingsStore( * * Resolution logic: * 1. If [binaryDestination] is null/blank, return the deployment's data - * directory with the default CLI binary name. [forceDownloadToData] - * is ignored because both paths resolve to the same location. - * 2. If [forceDownloadToData] is true, return a host-specific subdirectory - * under the deployment's data directory with the default CLI binary name. - * 3. If the expanded (~ and $HOME) [binaryDestination] is an existing executable file, + * directory with the default CLI binary name. + * 2. If the expanded (~ and $HOME) [binaryDestination] is an existing executable file, * return it as-is. - * 4. Otherwise, treat [binaryDestination] as a base directory and return a + * 3. Otherwise, treat [binaryDestination] as a base directory and return a * host-specific subdirectory with the default CLI binary name. */ - override fun binPath( - url: URL, - forceDownloadToData: Boolean, - ): Path { - if (binaryDestination.isNullOrBlank()) { - return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() - } - - val dest = Path.of(expand(binaryDestination!!)) - val isExecutable = Files.isRegularFile(dest) && Files.isExecutable(dest) + override fun binPath(url: URL): Path { + val raw = binaryDestination?.takeIf { it.isNotBlank() } ?: return dataDir(url).resolve( + defaultCliBinaryNameByOsAndArch + ).toAbsolutePath() - if (forceDownloadToData) { - return dataDir(url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() - } - if (isExecutable) { + val dest = Path.of(expand(raw)) + if (Files.isRegularFile(dest) && Files.isExecutable(dest)) { return dest.toAbsolutePath() } return withHost(dest, url).resolve(defaultCliBinaryNameByOsAndArch).toAbsolutePath() @@ -222,10 +209,6 @@ class CoderSettingsStore( store[HTTP_CLIENT_LOG_LEVEL] = level.toString() } - fun updateBinaryDirectoryFallback(shouldEnableBinDirFallback: Boolean) { - store[ENABLE_BINARY_DIR_FALLBACK] = shouldEnableBinDirFallback.toString() - } - fun updateHeaderCommand(cmd: String) { store[HEADER_COMMAND] = cmd } diff --git a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt index 3325faef..f73ab9eb 100644 --- a/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt +++ b/src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt @@ -25,8 +25,6 @@ internal const val DATA_DIRECTORY = "dataDirectory" internal const val ENABLE_DOWNLOADS = "enableDownloads" -internal const val ENABLE_BINARY_DIR_FALLBACK = "enableBinaryDirectoryFallback" - internal const val HEADER_COMMAND = "headerCommand" internal const val TLS_CERT_PATH = "tlsCertPath" diff --git a/src/main/kotlin/com/coder/toolbox/util/Error.kt b/src/main/kotlin/com/coder/toolbox/util/Error.kt index 54c4e97c..752aa074 100644 --- a/src/main/kotlin/com/coder/toolbox/util/Error.kt +++ b/src/main/kotlin/com/coder/toolbox/util/Error.kt @@ -4,31 +4,29 @@ import com.coder.toolbox.cli.ex.ResponseException import com.coder.toolbox.sdk.ex.APIResponseException import org.zeroturnaround.exec.InvalidExitValueException import java.net.ConnectException -import java.net.SocketTimeoutException -import java.net.URL import java.net.UnknownHostException -import javax.net.ssl.SSLHandshakeException -fun humanizeConnectionError(deploymentURL: URL, requireTokenAuth: Boolean, e: Exception): String { - val reason = e.message ?: "No reason was provided." - return when (e) { - is java.nio.file.AccessDeniedException -> "Access denied to ${e.file}." - is UnknownHostException -> "Unknown host ${e.message ?: deploymentURL.host}." - is InvalidExitValueException -> "CLI exited unexpectedly with ${e.exitValue}." +private fun accessDenied(file: String) = "Access denied to $file" +private fun fileSystemFailed(file: String) = "A file system operation failed when trying to access $file" + +fun Throwable.prettify(): String { + val reason = this.message ?: "No reason was provided" + return when (this) { + is AccessDeniedException -> accessDenied(this.file.toString()) + is java.nio.file.AccessDeniedException -> accessDenied(this.file) + is FileSystemException -> fileSystemFailed(this.file.toString()) + is java.nio.file.FileSystemException -> fileSystemFailed(this.file) + is UnknownHostException -> "Unknown host $reason" + is InvalidExitValueException -> "CLI exited unexpectedly with ${this.exitValue}." is APIResponseException -> { - if (e.isUnauthorized) { - if (requireTokenAuth) { - "Token was rejected by $deploymentURL; has your token expired?" - } else { - "Authorization failed to $deploymentURL." - } + if (this.isUnauthorized) { + "Authorization failed" } else { reason } } - is SocketTimeoutException -> "Unable to connect to $deploymentURL; is it up?" + is ResponseException, is ConnectException -> "Failed to download Coder CLI: $reason" - is SSLHandshakeException -> "Connection to $deploymentURL failed: $reason. See the documentation for TLS certificates for information on how to make your system trust certificates coming from your deployment." else -> reason } } diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt index 28a40f39..0a7cd393 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt @@ -68,10 +68,6 @@ class CoderSettingsPage( ) ) - private val enableBinaryDirectoryFallbackField = CheckboxField( - settings.enableBinaryDirectoryFallback, - context.i18n.ptrl("Enable binary directory fallback") - ) private val headerCommandField = TextField( context.i18n.ptrl("Header command"), settings.headerCommand ?: "", @@ -132,7 +128,6 @@ class CoderSettingsPage( enableDownloadsField, useAppNameField, binaryDestinationField, - enableBinaryDirectoryFallbackField, disableSignatureVerificationField, signatureFallbackStrategyField, httpLoggingField, @@ -163,7 +158,6 @@ class CoderSettingsPage( updateDisableSignatureVerification(disableSignatureVerificationField.checkedState.value) updateSignatureFallbackStrategy(signatureFallbackStrategyField.checkedState.value) updateHttpClientLogLevel(httpLoggingField.selectedValueState.value) - updateBinaryDirectoryFallback(enableBinaryDirectoryFallbackField.checkedState.value) updateHeaderCommand(headerCommandField.contentState.value) updateCertPath(tlsCertPathField.contentState.value) updateKeyPath(tlsKeyPathField.contentState.value) @@ -216,10 +210,6 @@ class CoderSettingsPage( settings.fallbackOnCoderForSignatures.isAllowed() } - enableBinaryDirectoryFallbackField.checkedState.update { - settings.enableBinaryDirectoryFallback - } - headerCommandField.contentState.update { settings.headerCommand ?: "" } diff --git a/src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt b/src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt index 88ace652..e6d6ac53 100644 --- a/src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt +++ b/src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt @@ -1,7 +1,7 @@ package com.coder.toolbox.views import com.coder.toolbox.CoderToolboxContext -import com.coder.toolbox.sdk.ex.APIResponseException +import com.coder.toolbox.util.prettify import com.jetbrains.toolbox.api.remoteDev.ProviderVisibilityState import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch @@ -46,22 +46,18 @@ private class ErrorReporterImpl( } private fun showError(ex: Throwable) { - val textError = if (ex is APIResponseException) { - if (!ex.reason.isNullOrBlank()) { - ex.reason - } else ex.message - } else ex.message ?: ex.toString() + val textError = ex.prettify() + context.cs.launch { context.ui.showSnackbar( UUID.randomUUID().toString(), context.i18n.ptrl("Error encountered while setting up Coder"), - context.i18n.pnotr(textError ?: ""), + context.i18n.pnotr(textError), context.i18n.ptrl("Dismiss") ) } } - override fun flush() { if (errorBuffer.isNotEmpty() && visibilityState.value.applicationVisible) { errorBuffer.forEach { diff --git a/src/main/resources/localization/defaultMessages.po b/src/main/resources/localization/defaultMessages.po index ad6a692f..6be1db2f 100644 --- a/src/main/resources/localization/defaultMessages.po +++ b/src/main/resources/localization/defaultMessages.po @@ -82,9 +82,6 @@ msgstr "" msgid "Verify binary signature using releases.coder.com when CLI signatures are not available from the deployment" msgstr "" -msgid "Enable binary directory fallback" -msgstr "" - msgid "Header command" msgstr "" diff --git a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt index d0caf714..bcd321f8 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/CoderCLIManagerTest.kt @@ -190,16 +190,10 @@ internal class CoderCLIManagerTest { ) val url = URL("http://localhost") - val ccm1 = CoderCLIManager(context.copy(settingsStore = settings), url) - assertEquals(settings.binSource(url), ccm1.remoteBinaryURL) - assertEquals(settings.dataDir(url), ccm1.coderConfigPath.parent) - assertEquals(settings.binPath(url), ccm1.localBinaryPath) - - // Can force using data directory. - val ccm2 = CoderCLIManager(context.copy(settingsStore = settings), url, true) - assertEquals(settings.binSource(url), ccm2.remoteBinaryURL) - assertEquals(settings.dataDir(url), ccm2.coderConfigPath.parent) - assertEquals(settings.binPath(url, true), ccm2.localBinaryPath) + val ccm = CoderCLIManager(context.copy(settingsStore = settings), url) + assertEquals(settings.binSource(url), ccm.remoteBinaryURL) + assertEquals(settings.dataDir(url), ccm.coderConfigPath.parent) + assertEquals(settings.binPath(url), ccm.localBinaryPath) } @Test diff --git a/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt b/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt index a35ab4df..2876fbe0 100644 --- a/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt +++ b/src/test/kotlin/com/coder/toolbox/cli/EnsureCLITest.kt @@ -8,7 +8,6 @@ import com.coder.toolbox.store.CoderSecretsStore import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.store.DATA_DIRECTORY import com.coder.toolbox.store.DISABLE_SIGNATURE_VALIDATION -import com.coder.toolbox.store.ENABLE_BINARY_DIR_FALLBACK import com.coder.toolbox.store.ENABLE_DOWNLOADS import com.coder.toolbox.util.ConnectionMonitoringService import com.coder.toolbox.util.IgnoreOnWindows @@ -38,7 +37,6 @@ import java.net.InetSocketAddress import java.net.Proxy import java.net.ProxySelector import java.net.URL -import java.nio.file.AccessDeniedException import java.nio.file.Files import java.nio.file.Path import kotlin.test.BeforeTest @@ -89,7 +87,6 @@ internal class EnsureCLITest { coEvery { ui.showYesNoPopup(any(), any(), any(), any()) } returns true } - @Test @IgnoreOnWindows fun `given binaryDestination is a directory and downloads are enabled, returns the CLI when version already matches`() { @@ -99,8 +96,6 @@ internal class EnsureCLITest { DATA_DIRECTORY to testDir("dir-dest-dl-on-match/data").toString(), DISABLE_SIGNATURE_VALIDATION to "true", ) - // Compute expected path before creating files so isExecutable on the - // directory does not interfere with the resolution. val expected = s.binPath(url) createBinary(expected, "1.0.0") @@ -132,7 +127,7 @@ internal class EnsureCLITest { } @Test - fun `given binaryDestination is a directory and downloads are enabled, propagates non-AccessDenied errors during download to the caller`() { + fun `given binaryDestination is a directory and downloads are enabled, propagates errors during download to the caller`() { val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) try { val s = settings( @@ -148,129 +143,6 @@ internal class EnsureCLITest { } } - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are enabled, falls back to the data directory CLI when access is denied and data directory version matches`() { - val (srv, url) = mockServer(version = "1.9.0") - val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-on-fallback-match/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-on-fallback-match/data").toString(), - ENABLE_BINARY_DIR_FALLBACK to "true", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val binParent = s.binPath(url).parent - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "2.0.0") - try { - binParent.toFile().mkdirs() - binParent.toFile().setWritable(false) - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(2, 0, 0), ccm.version()) - } finally { - binParent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are enabled, downloads CLI to the data directory when access is denied and data directory CLI is missing`() { - val (srv, url) = mockServer(version = "1.0.0") - val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-on-fallback-missing/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-on-fallback-missing/data").toString(), - ENABLE_BINARY_DIR_FALLBACK to "true", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val binParent = s.binPath(url).parent - val fallbackPath = s.binPath(url, true) - try { - binParent.toFile().mkdirs() - binParent.toFile().setWritable(false) - val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertTrue(ccm.localBinaryPath.toFile().exists()) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } finally { - binParent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are enabled, rethrows AccessDeniedException when fallback is enabled but binary path is already inside the data directory`() { - val (srv, url) = mockServer() - // Set BINARY_DESTINATION to the same base as DATA_DIRECTORY so that - // binPath(url).parent == dataDir(url). - val sharedBase = testDir("dir-dest-dl-on-fallback-same-dir/shared") - val s = settings( - BINARY_DESTINATION to sharedBase.toString(), - DATA_DIRECTORY to sharedBase.toString(), - ENABLE_BINARY_DIR_FALLBACK to "true", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val binParent = s.binPath(url).parent - try { - binParent.toFile().mkdirs() - binParent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - } - } finally { - binParent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are enabled, rethrows AccessDeniedException when binary directory fallback is disabled`() { - val (srv, url) = mockServer() - val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-on-no-fallback/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-on-no-fallback/data").toString(), - ENABLE_BINARY_DIR_FALLBACK to "false", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val binParent = s.binPath(url).parent - try { - binParent.toFile().mkdirs() - binParent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - } - } finally { - binParent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are enabled, rethrows AccessDeniedException when fallback is disabled and binary path is inside the data directory`() { - val (srv, url) = mockServer() - val sharedBase = testDir("dir-dest-dl-on-no-fallback-same-dir/shared") - val s = settings( - BINARY_DESTINATION to sharedBase.toString(), - DATA_DIRECTORY to sharedBase.toString(), - ENABLE_BINARY_DIR_FALLBACK to "false", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val binParent = s.binPath(url).parent - try { - binParent.toFile().mkdirs() - binParent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - } - } finally { - binParent.toFile().setWritable(true) - srv.stop(0) - } - } - @Test @IgnoreOnWindows fun `given binaryDestination is an existing executable and downloads are enabled, returns the CLI when version already matches`() { @@ -314,7 +186,7 @@ internal class EnsureCLITest { @Test @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are enabled, propagates non-AccessDenied errors during download to the caller`() { + fun `given binaryDestination is an existing executable and downloads are enabled, propagates errors during download to the caller`() { val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) try { val binaryFile = testDir("exec-dest-dl-on-error/bin/my-coder") @@ -334,134 +206,7 @@ internal class EnsureCLITest { @Test @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are enabled, falls back to the data directory CLI when access is denied and data directory version matches`() { - val (srv, url) = mockServer(version = "1.9.9") - val binaryFile = testDir("exec-dest-dl-on-fallback-match/bin/my-coder") - createBinary(binaryFile, "1.0.0") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-on-fallback-match/data").toString(), - ENABLE_BINARY_DIR_FALLBACK to "true", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "2.0.0") - try { - binaryFile.parent.toFile().setWritable(false) - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(2, 0, 0), ccm.version()) - } finally { - binaryFile.parent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are enabled, downloads CLI to the data directory when access is denied and data directory CLI is missing`() { - val (srv, url) = mockServer(version = "2.0.0") - val binaryFile = testDir("exec-dest-dl-on-fallback-missing/bin/my-coder") - createBinary(binaryFile, "1.0.0") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-on-fallback-missing/data").toString(), - ENABLE_BINARY_DIR_FALLBACK to "true", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val fallbackPath = s.binPath(url, true) - try { - binaryFile.parent.toFile().setWritable(false) - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertTrue(ccm.localBinaryPath.toFile().exists()) - assertEquals(SemVer(2, 0, 0), ccm.version()) - } finally { - binaryFile.parent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are enabled, rethrows AccessDeniedException when fallback is enabled but binary path is already inside the data directory`() { - val (srv, url) = mockServer() - // Place the executable inside the data dir so binPath.parent == dataDir. - val dataBase = testDir("exec-dest-dl-on-fallback-same-dir/data") - val dataDirForUrl = dataBase.resolve(hostDir(url)) - val binaryFile = dataDirForUrl.resolve("my-coder") - createBinary(binaryFile, "1.0.0") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to dataBase.toString(), - ENABLE_BINARY_DIR_FALLBACK to "true", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - // Verify the precondition: binPath.parent == dataDir - assertEquals(s.dataDir(url), s.binPath(url).parent) - try { - binaryFile.parent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - } - } finally { - binaryFile.parent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are enabled, rethrows AccessDeniedException when binary directory fallback is disabled`() { - val (srv, url) = mockServer() - val binaryFile = testDir("exec-dest-dl-on-no-fallback/bin/my-coder") - createBinary(binaryFile, "1.0.0") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-on-no-fallback/data").toString(), - ENABLE_BINARY_DIR_FALLBACK to "false", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - try { - binaryFile.parent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - } - } finally { - binaryFile.parent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are enabled, rethrows AccessDeniedException when fallback is disabled and binary path is inside the data directory`() { - val (srv, url) = mockServer() - val dataBase = testDir("exec-dest-dl-on-no-fallback-same-dir/data") - val dataDirForUrl = dataBase.resolve(hostDir(url)) - val binaryFile = dataDirForUrl.resolve("my-coder") - createBinary(binaryFile, "1.0.0") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to dataBase.toString(), - ENABLE_BINARY_DIR_FALLBACK to "false", - DISABLE_SIGNATURE_VALIDATION to "true", - ) - assertEquals(s.dataDir(url), s.binPath(url).parent) - try { - binaryFile.parent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - } - } finally { - binaryFile.parent.toFile().setWritable(true) - srv.stop(0) - } - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are disabled, returns the CLI at destination when version matches`() { + fun `given binaryDestination is an existing executable and downloads are disabled, returns the CLI when version matches`() { val url = URL("http://test.coder.invalid") val binaryFile = testDir("exec-dest-dl-off-match/bin/my-coder") createBinary(binaryFile, "1.0.0") @@ -477,96 +222,22 @@ internal class EnsureCLITest { @Test @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are disabled, returns the data directory CLI when destination version mismatches but data directory version matches`() { + fun `given binaryDestination is an existing executable and downloads are disabled, returns the stale CLI when version mismatches`() { val url = URL("http://test.coder.invalid") - val binaryFile = testDir("exec-dest-dl-off-mismatch-datacli-match/bin/my-coder") + val binaryFile = testDir("exec-dest-dl-off-mismatch/bin/my-coder") createBinary(binaryFile, "1.0.0") val s = settings( BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-off-mismatch-datacli-match/data").toString(), + DATA_DIRECTORY to testDir("exec-dest-dl-off-mismatch/data").toString(), ENABLE_DOWNLOADS to "false", ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "2.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(2, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are disabled, returns the stale destination CLI when both destination and data directory versions mismatch`() { - val url = URL("http://test.coder.invalid") - val binaryFile = testDir("exec-dest-dl-off-mismatch-datacli-wrong/bin/my-coder") - createBinary(binaryFile, "1.0.0") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-off-mismatch-datacli-wrong/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.5.0") - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } assertEquals(binaryFile.toAbsolutePath(), ccm.localBinaryPath) assertEquals(SemVer(1, 0, 0), ccm.version()) } @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are disabled, returns the stale destination CLI when version mismatches and data directory CLI is missing`() { - val url = URL("http://test.coder.invalid") - val binaryFile = testDir("exec-dest-dl-off-mismatch-datacli-missing/bin/my-coder") - createBinary(binaryFile, "1.0.0") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-off-mismatch-datacli-missing/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(binaryFile.toAbsolutePath(), ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are disabled, returns the data directory CLI when destination is missing and data directory version matches`() { - val url = URL("http://test.coder.invalid") - val binaryFile = testDir("exec-dest-dl-off-no-cli-datacli-match/bin/my-coder") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-off-no-cli-datacli-match/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is an existing executable and downloads are disabled, returns the stale data directory CLI when destination is missing and data directory version mismatches`() { - val url = URL("http://test.coder.invalid") - val binaryFile = testDir("exec-dest-dl-off-no-cli-datacli-wrong/bin/my-coder") - val s = settings( - BINARY_DESTINATION to binaryFile.toString(), - DATA_DIRECTORY to testDir("exec-dest-dl-off-no-cli-datacli-wrong/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - fun `given binaryDestination is an existing executable and downloads are disabled, throws when both destination and data directory CLIs are missing`() { + fun `given binaryDestination points to a non-existent file and downloads are disabled, throws when no CLI binary exists`() { val url = URL("http://test.coder.invalid") val binaryFile = testDir("exec-dest-dl-off-both-missing/bin/my-coder") val s = settings( @@ -606,7 +277,7 @@ internal class EnsureCLITest { @Test @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are disabled, returns the CLI at destination when version matches`() { + fun `given binaryDestination is a directory and downloads are disabled, returns the CLI when version matches`() { val url = URL("http://test.coder.invalid") val s = settings( BINARY_DESTINATION to testDir("dir-dest-dl-off-match/bin").toString(), @@ -623,36 +294,15 @@ internal class EnsureCLITest { @Test @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are disabled, returns the data directory CLI when destination version mismatches but data directory version matches`() { + fun `given binaryDestination is a directory and downloads are disabled, returns the stale CLI when version mismatches`() { val url = URL("http://test.coder.invalid") val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-off-mismatch-datacli-match/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-off-mismatch-datacli-match/data").toString(), + BINARY_DESTINATION to testDir("dir-dest-dl-off-mismatch/bin").toString(), + DATA_DIRECTORY to testDir("dir-dest-dl-off-mismatch/data").toString(), ENABLE_DOWNLOADS to "false", ) val expected = s.binPath(url) createBinary(expected, "1.0.0") - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "2.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(2, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are disabled, returns the stale destination CLI when both destination and data directory versions mismatch`() { - val url = URL("http://test.coder.invalid") - val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-off-mismatch-datacli-wrong/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-off-mismatch-datacli-wrong/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val expected = s.binPath(url) - createBinary(expected, "1.0.0") - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.5.0") val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } assertEquals(expected, ccm.localBinaryPath) @@ -661,58 +311,7 @@ internal class EnsureCLITest { @Test @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are disabled, returns the stale destination CLI when version mismatches and data directory CLI is missing`() { - val url = URL("http://test.coder.invalid") - val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-off-mismatch-datacli-missing/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-off-mismatch-datacli-missing/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val expected = s.binPath(url) - createBinary(expected, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(expected, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are disabled, returns the data directory CLI when destination CLI is missing and data directory version matches`() { - val url = URL("http://test.coder.invalid") - val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-off-no-cli-datacli-match/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-off-no-cli-datacli-match/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are disabled, returns the stale data directory CLI when destination CLI is missing and data directory version mismatches`() { - val url = URL("http://test.coder.invalid") - val s = settings( - BINARY_DESTINATION to testDir("dir-dest-dl-off-no-cli-datacli-wrong/bin").toString(), - DATA_DIRECTORY to testDir("dir-dest-dl-off-no-cli-datacli-wrong/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given binaryDestination is a directory and downloads are disabled, throws when both destination and data directory CLIs are missing`() { + fun `given binaryDestination is a directory and downloads are disabled, throws when no CLI binary exists`() { val url = URL("http://test.coder.invalid") val s = settings( BINARY_DESTINATION to testDir("dir-dest-dl-off-both-missing/bin").toString(), @@ -742,52 +341,13 @@ internal class EnsureCLITest { @Test @IgnoreOnWindows - fun `given no binaryDestination configured and downloads are disabled, returns the CLI when the shared path is overwritten with a matching version`() { - val url = URL("http://test.coder.invalid") - val s = settings( - DATA_DIRECTORY to testDir("no-dest-dl-off-mismatch-datacli-match/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val expected = s.binPath(url) - createBinary(expected, "1.0.0") - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "2.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(2, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given no binaryDestination configured and downloads are disabled, returns the stale CLI when version mismatches confirming CLI and data directory paths are identical`() { + fun `given no binaryDestination configured and downloads are disabled, returns the stale CLI when version mismatches`() { val url = URL("http://test.coder.invalid") val s = settings( DATA_DIRECTORY to testDir("no-dest-dl-off-mismatch/data").toString(), ENABLE_DOWNLOADS to "false", ) val expected = s.binPath(url) - // When binaryDestination is not configured, binPath(url) and - // binPath(url, true) resolve to the same path because the - // isNullOrBlank() early return in binPath fires before the - // forceDownloadToData check. So cli and dataCLI share a binary. - assertEquals(expected, s.binPath(url, true)) - createBinary(expected, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(expected, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given no binaryDestination configured and downloads are disabled, returns the stale CLI when version mismatches and no separate data directory CLI path exists`() { - val url = URL("http://test.coder.invalid") - val s = settings( - DATA_DIRECTORY to testDir("no-dest-dl-off-mismatch-datacli-missing/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val expected = s.binPath(url) createBinary(expected, "1.0.0") val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } @@ -795,38 +355,6 @@ internal class EnsureCLITest { assertEquals(SemVer(1, 0, 0), ccm.version()) } - @Test - @IgnoreOnWindows - fun `given no binaryDestination configured and downloads are disabled, returns the CLI when version matches at the shared data directory path`() { - val url = URL("http://test.coder.invalid") - val s = settings( - DATA_DIRECTORY to testDir("no-dest-dl-off-no-cli-datacli-match/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - - @Test - @IgnoreOnWindows - fun `given no binaryDestination configured and downloads are disabled, returns the stale CLI when version mismatches at the shared data directory path`() { - val url = URL("http://test.coder.invalid") - val s = settings( - DATA_DIRECTORY to testDir("no-dest-dl-off-no-cli-datacli-wrong/data").toString(), - ENABLE_DOWNLOADS to "false", - ) - val fallbackPath = s.binPath(url, true) - createBinary(fallbackPath, "1.0.0") - - val ccm = runBlocking { ensureCLI(ctx(s), url, "2.0.0", noOpTextProgress) } - assertEquals(fallbackPath, ccm.localBinaryPath) - assertEquals(SemVer(1, 0, 0), ccm.version()) - } - @Test fun `given no binaryDestination configured and downloads are disabled, throws when no CLI binary exists`() { val url = URL("http://test.coder.invalid") @@ -877,7 +405,7 @@ internal class EnsureCLITest { } @Test - fun `given no binaryDestination configured and downloads are enabled, propagates non-AccessDenied errors during download to the caller`() { + fun `given no binaryDestination configured and downloads are enabled, propagates errors during download to the caller`() { val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) try { val s = settings( @@ -892,27 +420,6 @@ internal class EnsureCLITest { } } - @Test - @IgnoreOnWindows - fun `given no binaryDestination configured and downloads are enabled, rethrows AccessDeniedException since the binary path is always inside the data directory`() { - val (srv, url) = mockServer() - val s = settings( - DATA_DIRECTORY to testDir("no-dest-dl-on-access-denied/data").toString(), - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val binParent = s.binPath(url).parent - try { - binParent.toFile().mkdirs() - binParent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - } - } finally { - binParent.toFile().setWritable(true) - srv.stop(0) - } - } - @Test fun `given no binaryDestination configured and downloads are enabled, downloads and returns a fresh CLI when none exists yet`() { val (srv, url) = mockServer(version = "1.0.0") @@ -935,7 +442,7 @@ internal class EnsureCLITest { } @Test - fun `given no binaryDestination configured and downloads are enabled, propagates non-AccessDenied errors during download when CLI is missing`() { + fun `given no binaryDestination configured and downloads are enabled, propagates errors during download when CLI is missing`() { val (srv, url) = mockServer(errorCode = HttpURLConnection.HTTP_INTERNAL_ERROR) try { val s = settings( @@ -950,27 +457,6 @@ internal class EnsureCLITest { } } - @Test - @IgnoreOnWindows - fun `given no binaryDestination configured and downloads are enabled, rethrows AccessDeniedException when CLI is missing and access to the data directory is denied`() { - val (srv, url) = mockServer() - val s = settings( - DATA_DIRECTORY to testDir("no-dest-dl-on-no-cli-access-denied/data").toString(), - DISABLE_SIGNATURE_VALIDATION to "true", - ) - val binParent = s.binPath(url).parent - try { - binParent.toFile().mkdirs() - binParent.toFile().setWritable(false) - assertFailsWith(AccessDeniedException::class) { - runBlocking { ensureCLI(ctx(s), url, "1.0.0", noOpTextProgress) } - } - } finally { - binParent.toFile().setWritable(true) - srv.stop(0) - } - } - // Utilities private fun testDir(id: String): Path = tmpdir.resolve(id) @@ -1032,10 +518,6 @@ internal class EnsureCLITest { return Pair(srv, URL("http://localhost:" + srv.address.port)) } - /** Build the host directory component used by [CoderSettingsStore.withHost]. */ - private fun hostDir(url: URL): String = - if (url.port > 0) "${url.host}-${url.port}" else url.host - companion object { private val tmpdir: Path = Path.of(System.getProperty("java.io.tmpdir")) .resolve("coder-toolbox-test").resolve("ensure-cli") diff --git a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt index b57354c0..fa4da2df 100644 --- a/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/toolbox/settings/CoderSettingsTest.kt @@ -3,7 +3,6 @@ package com.coder.toolbox.settings import com.coder.toolbox.store.CODER_SSH_CONFIG_OPTIONS import com.coder.toolbox.store.CoderSettingsStore import com.coder.toolbox.store.DISABLE_AUTOSTART -import com.coder.toolbox.store.ENABLE_BINARY_DIR_FALLBACK import com.coder.toolbox.store.ENABLE_DOWNLOADS import com.coder.toolbox.store.HEADER_COMMAND import com.coder.toolbox.store.SSH_CONFIG_OPTIONS @@ -123,11 +122,6 @@ internal class CoderSettingsTest { var expected = "/tmp/coder-toolbox-test/bin-dir/test.coder.com" assertEquals(Path.of(expected).toAbsolutePath(), settings.readOnly().binPath(url).parent) - // Second argument bypasses override. - settings.updateDataDirectory("/tmp/coder-toolbox-test/data-dir") - expected = "/tmp/coder-toolbox-test/data-dir/test.coder.com" - assertEquals(Path.of(expected).toAbsolutePath(), settings.readOnly().binPath(url, true).parent) - // Binary name is always determined by OS and architecture. assertEquals( settings.readOnly().defaultCliBinaryNameByOsAndArch, @@ -280,7 +274,6 @@ internal class CoderSettingsTest { // Test defaults for the remaining settings. val settings = CoderSettingsStore(pluginTestSettingsStore(), Environment(), logger) assertEquals(true, settings.readOnly().enableDownloads) - assertEquals(false, settings.readOnly().enableBinaryDirectoryFallback) assertEquals(null, settings.readOnly().headerCommand) assertEquals(null, settings.readOnly().tls.certPath) assertEquals(null, settings.readOnly().tls.keyPath) @@ -296,7 +289,6 @@ internal class CoderSettingsTest { CoderSettingsStore( pluginTestSettingsStore( ENABLE_DOWNLOADS to false.toString(), - ENABLE_BINARY_DIR_FALLBACK to true.toString(), HEADER_COMMAND to "test header", TLS_CERT_PATH to "tls cert path", TLS_KEY_PATH to "tls key path", @@ -310,7 +302,6 @@ internal class CoderSettingsTest { ) assertEquals(false, settings.readOnly().enableDownloads) - assertEquals(true, settings.readOnly().enableBinaryDirectoryFallback) assertEquals("test header", settings.readOnly().headerCommand) assertEquals("tls cert path", settings.readOnly().tls.certPath) assertEquals("tls key path", settings.readOnly().tls.keyPath) diff --git a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt index 0158c83e..3a71f169 100644 --- a/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt +++ b/src/test/kotlin/com/coder/toolbox/store/CoderSettingsStoreTest.kt @@ -5,7 +5,6 @@ import com.coder.toolbox.util.pluginTestSettingsStore import com.jetbrains.toolbox.api.core.diagnostics.Logger import io.mockk.mockk import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import java.net.URL import java.nio.file.Files @@ -97,19 +96,6 @@ class CoderSettingsStoreTest { assertTrue(result.endsWith(Path.of("test.coder.com", "coder-linux-amd64"))) } - @Test - fun `binPath uses dataDir when forceDownloadToData is true even with binaryDestination set`() { - setOsAndArch("Linux", "x86_64") - val settings = storeWith(BINARY_DESTINATION to "/custom/path") - val url = URL("https://test.coder.com") - val result = settings.binPath(url, forceDownloadToData = true) - - assertTrue(result.isAbsolute) - assertTrue(result.endsWith(Path.of("test.coder.com", "coder-linux-amd64"))) - // The custom path should NOT be part of the result when forceDownloadToData is true. - assertFalse(result.startsWith(Path.of("/custom/path").toAbsolutePath())) - } - @Test fun `binPath returns absolute binaryDestination path when it points to an existing executable`() { val tmpBin = Files.createTempFile("coder-test", null)