From 1f2693340c51b94f487681bb9e3fa19c842dfd28 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 9 May 2023 13:16:14 -0800 Subject: [PATCH 1/2] Allow configuring CLI directory separately from data This is so admins can download the CLI to some restricted location (like ProgramFiles). --- .../gateway/CoderSettingsConfigurable.kt | 45 +++++-- .../com/coder/gateway/sdk/CoderCLIManager.kt | 121 ++++++++++++++--- .../gateway/services/CoderSettingsState.kt | 5 +- .../views/steps/CoderWorkspacesStepView.kt | 80 +++++------ .../messages/CoderGatewayBundle.properties | 24 +++- src/test/groovy/CoderCLIManagerTest.groovy | 125 ++++++++++++++++-- .../groovy/CoderWorkspacesStepViewTest.groovy | 4 + 7 files changed, 324 insertions(+), 80 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt index 2c6957a8..84e6d676 100644 --- a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt +++ b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt @@ -9,6 +9,8 @@ import com.intellij.openapi.ui.DialogPanel import com.intellij.openapi.ui.ValidationInfo import com.intellij.ui.components.JBTextField import com.intellij.ui.dsl.builder.AlignX +import com.intellij.ui.dsl.builder.RowLayout +import com.intellij.ui.dsl.builder.bindSelected import com.intellij.ui.dsl.builder.bindText import com.intellij.ui.dsl.builder.panel import com.intellij.ui.layout.ValidationInfoBuilder @@ -19,6 +21,18 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") { override fun createPanel(): DialogPanel { val state: CoderSettingsState = service() return panel { + row(CoderGatewayBundle.message("gateway.connector.settings.data-directory.title")) { + textField().resizableColumn().align(AlignX.FILL) + .bindText(state::dataDirectory) + .validationOnApply(validateDataDirectory()) + .validationOnInput(validateDataDirectory()) + .comment( + CoderGatewayBundle.message( + "gateway.connector.settings.data-directory.comment", + CoderCLIManager.getDataDir(), + ) + ) + }.layout(RowLayout.PARENT_GRID) row(CoderGatewayBundle.message("gateway.connector.settings.binary-source.title")) { textField().resizableColumn().align(AlignX.FILL) .bindText(state::binarySource) @@ -28,23 +42,34 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") { CoderCLIManager(URL("http://localhost"), CoderCLIManager.getDataDir()).remoteBinaryURL.path, ) ) - } + }.layout(RowLayout.PARENT_GRID) + row { + cell() // For alignment. + checkBox(CoderGatewayBundle.message("gateway.connector.settings.enable-downloads.title")) + .bindSelected(state::enableDownloads) + .comment( + CoderGatewayBundle.message("gateway.connector.settings.enable-downloads.comment") + ) + }.layout(RowLayout.PARENT_GRID) + // The binary directory is not validated because it could be a + // read-only path that is pre-downloaded by admins. row(CoderGatewayBundle.message("gateway.connector.settings.binary-destination.title")) { textField().resizableColumn().align(AlignX.FILL) - .bindText(state::binaryDestination) - .validationOnApply(validateBinaryDestination()) - .validationOnInput(validateBinaryDestination()) + .bindText(state::binaryDirectory) + .comment(CoderGatewayBundle.message("gateway.connector.settings.binary-destination.comment")) + }.layout(RowLayout.PARENT_GRID) + row { + cell() // For alignment. + checkBox(CoderGatewayBundle.message("gateway.connector.settings.enable-binary-directory-fallback.title")) + .bindSelected(state::enableBinaryDirectoryFallback) .comment( - CoderGatewayBundle.message( - "gateway.connector.settings.binary-destination.comment", - CoderCLIManager.getDataDir(), - ) + CoderGatewayBundle.message("gateway.connector.settings.enable-binary-directory-fallback.comment") ) - } + }.layout(RowLayout.PARENT_GRID) } } - private fun validateBinaryDestination(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = { + private fun validateDataDirectory(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = { if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) { error("Cannot create this directory") } else { diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 13b064f9..d89fe75c 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -1,13 +1,17 @@ package com.coder.gateway.sdk import com.coder.gateway.models.WorkspaceAgentModel +import com.coder.gateway.services.CoderSettingsState import com.coder.gateway.views.steps.CoderWorkspacesStepView import com.google.gson.Gson +import com.google.gson.JsonSyntaxException import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.progress.ProgressIndicator import org.zeroturnaround.exec.ProcessExecutor import java.io.BufferedInputStream import java.io.FileInputStream import java.io.FileNotFoundException +import java.net.ConnectException import java.net.HttpURLConnection import java.net.IDN import java.net.URL @@ -26,7 +30,8 @@ import javax.xml.bind.annotation.adapters.HexBinaryAdapter */ class CoderCLIManager @JvmOverloads constructor( private val deploymentURL: URL, - destinationDir: Path, + dataDir: Path, + cliDir: Path? = null, remoteBinaryURLOverride: String? = null, private val sshConfigPath: Path = Path.of(System.getProperty("user.home")).resolve(".ssh/config"), ) { @@ -52,8 +57,8 @@ class CoderCLIManager @JvmOverloads constructor( } val host = getSafeHost(deploymentURL) val subdir = if (deploymentURL.port > 0) "${host}-${deploymentURL.port}" else host - localBinaryPath = destinationDir.resolve(subdir).resolve(binaryName).toAbsolutePath() - coderConfigPath = destinationDir.resolve(subdir).resolve("config").toAbsolutePath() + localBinaryPath = (cliDir ?: dataDir).resolve(subdir).resolve(binaryName).toAbsolutePath() + coderConfigPath = dataDir.resolve(subdir).resolve("config").toAbsolutePath() } /** @@ -125,6 +130,9 @@ class CoderCLIManager @JvmOverloads constructor( return false } } + } catch (e: ConnectException) { + // Add the URL so this is more easily debugged. + throw ConnectException("${e.message} to $remoteBinaryURL") } finally { conn.disconnect() } @@ -293,26 +301,47 @@ class CoderCLIManager @JvmOverloads constructor( val raw = exec("version", "--output", "json") val json = Gson().fromJson(raw, Version::class.java) if (json?.version == null) { - throw InvalidVersionException("No version found in output") + throw MissingVersionException("No version found in output") } return CoderSemVer.parse(json.version) } /** * Returns true if the CLI has the same major/minor/patch version as the - * provided version and false if it does not match or the CLI version could - * not be determined or the provided version is invalid. + * provided version, false if it does not match or either version is + * invalid, or null if the CLI version could not be determined because the + * binary could not be executed. */ - fun matchesVersion(buildVersion: String): Boolean { - return try { - val cliVersion = version() - val matches = cliVersion == CoderSemVer.parse(buildVersion) - logger.info("$localBinaryPath version $cliVersion matches $buildVersion: $matches") - matches + fun matchesVersion(rawBuildVersion: String): Boolean? { + val cliVersion = try { + version() } catch (e: Exception) { - logger.info("Unable to determine $localBinaryPath version: ${e.message}") - false + when (e) { + is JsonSyntaxException, + is IllegalArgumentException -> { + logger.info("Got invalid version from $localBinaryPath: ${e.message}") + return false + } + else -> { + // An error here most likely means the CLI does not exist or + // it executed successfully but output no version which + // suggests it is not the right binary. + logger.info("Unable to determine $localBinaryPath version: ${e.message}") + return null + } + } + } + + val buildVersion = try { + CoderSemVer.parse(rawBuildVersion) + } catch (e: IllegalArgumentException) { + logger.info("Got invalid build version: $rawBuildVersion") + return false } + + val matches = cliVersion == buildVersion + logger.info("$localBinaryPath version $cliVersion matches $buildVersion: $matches") + return matches } private fun exec(vararg args: String): String { @@ -404,6 +433,68 @@ class CoderCLIManager @JvmOverloads constructor( fun getHostName(url: URL, ws: WorkspaceAgentModel): String { return "coder-jetbrains--${ws.name}--${getSafeHost(url)}" } + + /** + * Do as much as possible to get a valid, up-to-date CLI. + */ + @JvmStatic + @JvmOverloads + fun ensureCLI( + deploymentURL: URL, + buildVersion: String, + settings: CoderSettingsState, + indicator: ProgressIndicator? = null, + ): CoderCLIManager { + val dataDir = + if (settings.dataDirectory.isBlank()) getDataDir() + else Path.of(settings.dataDirectory).toAbsolutePath() + val binDir = + if (settings.binaryDirectory.isBlank()) null + else Path.of(settings.binaryDirectory).toAbsolutePath() + + val cli = CoderCLIManager(deploymentURL, dataDir, binDir, settings.binarySource) + + // Short-circuit if we already have the expected version. This + // lets us bypass the 304 which is slower and may not be + // supported if the binary is downloaded from alternate sources. + // For CLIs without the JSON output flag we will fall back to + // the 304 method. + val cliMatches = cli.matchesVersion(buildVersion) + if (cliMatches == true) { + return cli + } + + // If downloads are enabled download the new version. + if (settings.enableDownloads) { + indicator?.text = "Downloading Coder CLI..." + try { + cli.downloadCLI() + return cli + } catch (e: java.nio.file.AccessDeniedException) { + // Might be able to fall back. + if (binDir == null || binDir == dataDir || !settings.enableBinaryDirectoryFallback) { + throw e + } + } + } + + // Try falling back to the data directory. + val dataCLI = CoderCLIManager(deploymentURL, dataDir, null, settings.binarySource) + val dataCLIMatches = dataCLI.matchesVersion(buildVersion) + if (dataCLIMatches == true) { + return dataCLI + } + + if (settings.enableDownloads) { + indicator?.text = "Downloading Coder CLI..." + dataCLI.downloadCLI() + return dataCLI + } + + // Prefer the binary directory unless the data directory has a + // working binary and the binary directory does not. + return if (cliMatches == null && dataCLIMatches != null) dataCLI else cli + } } } @@ -418,5 +509,5 @@ class Environment(private val env: Map = emptyMap()) { } class ResponseException(message: String, val code: Int) : Exception(message) - class SSHConfigFormatException(message: String) : Exception(message) +class MissingVersionException(message: String) : Exception(message) diff --git a/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt b/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt index c7cb4d37..99c6d8af 100644 --- a/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt +++ b/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt @@ -14,7 +14,10 @@ import com.intellij.util.xmlb.XmlSerializerUtil ) class CoderSettingsState : PersistentStateComponent { var binarySource: String = "" - var binaryDestination: String = "" + var binaryDirectory: String = "" + var dataDirectory: String = "" + var enableDownloads: Boolean = true + var enableBinaryDirectoryFallback: Boolean = false override fun getState(): CoderSettingsState { return this } diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt index 1a4c17f8..0d9803c9 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -69,6 +69,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import org.zeroturnaround.exec.InvalidExitValueException import java.awt.Component import java.awt.Dimension import java.awt.event.MouseEvent @@ -79,7 +80,7 @@ import java.awt.font.TextAttribute.UNDERLINE_ON import java.net.ConnectException import java.net.SocketTimeoutException import java.net.URL -import java.nio.file.Path +import java.net.UnknownHostException import javax.swing.Icon import javax.swing.JCheckBox import javax.swing.JTable @@ -99,6 +100,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod private val cs = CoroutineScope(Dispatchers.Main) private var localWizardModel = CoderWorkspacesWizardModel() private val clientService: CoderRestClientService = service() + private var cliManager: CoderCLIManager? = null private val iconDownloader: TemplateIconDownloader = service() private val settings: CoderSettingsState = service() @@ -339,6 +341,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } override fun onInit(wizardModel: CoderWorkspacesWizardModel) { + cliManager = null tableOfWorkspaces.listTableModel.items = emptyList() if (localWizardModel.coderURL.isNotBlank() && localWizardModel.token != null) { triggerWorkspacePolling(true) @@ -443,6 +446,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod onAuthFailure: (() -> Unit)? = null, ): Job { // Clear out old deployment details. + cliManager = null poller?.cancel() tableOfWorkspaces.setEmptyState("Connecting to $deploymentURL...") tableOfWorkspaces.listTableModel.items = emptyList() @@ -454,12 +458,6 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod canBeCancelled = false, isIndeterminate = true ) { - val cliManager = CoderCLIManager( - deploymentURL, - if (settings.binaryDestination.isNotBlank()) Path.of(settings.binaryDestination) - else CoderCLIManager.getDataDir(), - settings.binarySource, - ) try { this.indicator.text = "Authenticating client..." authenticate(deploymentURL, token.first) @@ -467,18 +465,15 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod appPropertiesService.setValue(CODER_URL_KEY, deploymentURL.toString()) appPropertiesService.setValue(SESSION_TOKEN, token.first) - // Short-circuit if we already have the expected version. This - // lets us bypass the 304 which is slower and may not be - // supported if the binary is downloaded from alternate sources. - // For CLIs without the JSON output flag we will fall back to - // the 304 method. - if (!cliManager.matchesVersion(clientService.buildVersion)) { - this.indicator.text = "Downloading Coder CLI..." - cliManager.downloadCLI() - } + val cli = CoderCLIManager.ensureCLI( + deploymentURL, + clientService.buildVersion, + settings, + this.indicator, + ) this.indicator.text = "Authenticating Coder CLI..." - cliManager.login(token.first) + cli.login(token.first) this.indicator.text = "Retrieving workspaces..." loadWorkspaces() @@ -486,9 +481,15 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod updateWorkspaceActions() triggerWorkspacePolling(false) + cliManager = cli tableOfWorkspaces.setEmptyState("Connected to $deploymentURL") } catch (e: Exception) { - val errorSummary = e.message ?: "No reason was provided" + val errorSummary = when (e) { + is java.nio.file.AccessDeniedException -> "Access denied to ${e.file}" + is UnknownHostException -> "Unknown host ${e.message}" + is InvalidExitValueException -> "CLI exited unexpectedly with ${e.exitValue}" + else -> e.message ?: "No reason was provided" + } var msg = CoderGatewayBundle.message( "gateway.connector.view.workspaces.connect.failed", deploymentURL, @@ -513,7 +514,6 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod is ResponseException, is ConnectException -> { msg = CoderGatewayBundle.message( "gateway.connector.view.workspaces.connect.download-failed", - cliManager.remoteBinaryURL, errorSummary, ) } @@ -700,29 +700,30 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod token = localWizardModel.token } + // These being null would be a developer error. val workspace = tableOfWorkspaces.selectedObject - if (workspace != null) { - wizardModel.selectedWorkspace = workspace - poller?.cancel() - - logger.info("Configuring Coder CLI...") - val cliManager = CoderCLIManager( - wizardModel.coderURL.toURL(), - if (settings.binaryDestination.isNotBlank()) Path.of(settings.binaryDestination) - else CoderCLIManager.getDataDir(), - settings.binarySource, - ) - cliManager.configSsh(tableOfWorkspaces.items) + val cli = cliManager + if (workspace == null) { + logger.error("No selected workspace") + return false + } else if (cli == null) { + logger.error("No configured CLI") + return false + } - // The config directory can be used to pull the URL and token in - // order to query this workspace's status in other flows, for - // example from the recent connections screen. - wizardModel.configDirectory = cliManager.coderConfigPath.toString() + wizardModel.selectedWorkspace = workspace + poller?.cancel() - logger.info("Opening IDE and Project Location window for ${workspace.name}") - return true - } - return false + logger.info("Configuring Coder CLI...") + cli.configSsh(tableOfWorkspaces.items) + + // The config directory can be used to pull the URL and token in + // order to query this workspace's status in other flows, for + // example from the recent connections screen. + wizardModel.configDirectory = cli.coderConfigPath.toString() + + logger.info("Opening IDE and Project Location window for ${workspace.name}") + return true } override fun dispose() { @@ -910,5 +911,4 @@ class WorkspacesTable : TableView(WorkspacesTableModel()) { } return listTableModel.items.indexOfFirst { it.workspaceName == oldSelection.workspaceName } } - } diff --git a/src/main/resources/messages/CoderGatewayBundle.properties b/src/main/resources/messages/CoderGatewayBundle.properties index fc2b3750..0286b64a 100644 --- a/src/main/resources/messages/CoderGatewayBundle.properties +++ b/src/main/resources/messages/CoderGatewayBundle.properties @@ -22,8 +22,8 @@ gateway.connector.view.coder.workspaces.invalid.coder.version=Could not parse Co gateway.connector.view.coder.workspaces.unsupported.coder.version=Coder version {0} might not be compatible with this plugin version. Connect to a Coder workspace manually gateway.connector.view.workspaces.connect.unauthorized=Token was rejected by {0}; has your token expired? gateway.connector.view.workspaces.connect.timeout=Unable to connect to {0}; is it up? -gateway.connector.view.workspaces.connect.download-failed=Failed to download Coder CLI from {0}: {1} -gateway.connector.view.workspaces.connect.failed=Failed to configure connection to {0}: {1} +gateway.connector.view.workspaces.connect.download-failed=Failed to download Coder CLI: {0} +gateway.connector.view.workspaces.connect.failed=Failed to connect to {0}: {1} gateway.connector.view.workspaces.token.comment=The last used token is shown above. gateway.connector.view.workspaces.token.rejected=This token was rejected. gateway.connector.view.workspaces.token.injected=This token was pulled from your CLI config. @@ -48,6 +48,11 @@ gateway.connector.coder.connecting=Connecting... gateway.connector.coder.connecting.retry=Connecting (attempt {0})... gateway.connector.coder.connection.failed=Failed to connect gateway.connector.coder.connecting.failed.retry=Failed to connect...retrying {0} +gateway.connector.settings.data-directory.title=Data directory: +gateway.connector.settings.data-directory.comment=Directories are created \ + here that store the credentials for each domain to which the plugin \ + connects. \ + Defaults to {0}. gateway.connector.settings.binary-source.title=CLI source: gateway.connector.settings.binary-source.comment=Used to download the Coder \ CLI which is necessary to make SSH connections. The If-None-Matched header \ @@ -55,9 +60,16 @@ gateway.connector.settings.binary-source.comment=Used to download the Coder \ URLs will be used as-is; otherwise this value will be resolved against the \ deployment domain. \ Defaults to {0}. -gateway.connector.settings.binary-destination.title=Data directory: +gateway.connector.settings.enable-downloads.title=Enable CLI downloads +gateway.connector.settings.enable-downloads.comment=Checking this box will \ + allow the plugin to download the CLI if the current one is out of date or \ + does not exist. +gateway.connector.settings.binary-destination.title=CLI directory: gateway.connector.settings.binary-destination.comment=Directories are created \ - here that store the CLI and credentials for each domain to which the plugin \ - connects. \ - Defaults to {0}. + here that store the CLI for each domain to which the plugin connects. \ + Defaults to the data directory. +gateway.connector.settings.enable-binary-directory-fallback.title=Fall back to data directory +gateway.connector.settings.enable-binary-directory-fallback.comment=Checking this \ + box will allow the plugin to fall back to the data directory when the CLI \ + directory is not writable. gateway.connector.no-details="The error did not provide any further details" diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index 166a04cf..181f088a 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -1,5 +1,6 @@ package com.coder.gateway.sdk +import com.coder.gateway.services.CoderSettingsState import com.google.gson.JsonSyntaxException import com.sun.net.httpserver.HttpExchange import com.sun.net.httpserver.HttpHandler @@ -8,6 +9,7 @@ import org.zeroturnaround.exec.InvalidExitValueException import org.zeroturnaround.exec.ProcessInitException import spock.lang.* +import java.nio.file.AccessDeniedException import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption @@ -118,6 +120,25 @@ class CoderCLIManagerTest extends Specification { srv.stop(0) } + @IgnoreIf({ os.windows }) + def "fails to write"() { + given: + def (srv, url) = mockServer() + def dir = tmpdir.resolve("cli-dir-fallver") + def ccm = new CoderCLIManager(new URL(url), tmpdir, dir) + Files.createDirectories(ccm.localBinaryPath.getParent()) + ccm.localBinaryPath.parent.toFile().setWritable(false) + + when: + ccm.downloadCLI() + + then: + thrown(AccessDeniedException) + + cleanup: + srv.stop(0) + } + // This test uses a real deployment if possible to make sure we really // download a working CLI and that it runs on each platform. @Requires({ env["CODER_GATEWAY_TEST_DEPLOYMENT"] != "mock" }) @@ -242,7 +263,7 @@ class CoderCLIManagerTest extends Specification { def "overrides binary URL"() { given: def (srv, url) = mockServer() - def ccm = new CoderCLIManager(new URL(url), tmpdir, override.replace("{{url}}", url)) + def ccm = new CoderCLIManager(new URL(url), tmpdir, null, override.replace("{{url}}", url)) when: def downloaded = ccm.downloadCLI() @@ -362,7 +383,7 @@ class CoderCLIManagerTest extends Specification { def "configures an SSH file"() { given: def sshConfigPath = tmpdir.resolve(input + "_to_" + output + ".conf") - def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), tmpdir, null, sshConfigPath) + def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), tmpdir, null, null, sshConfigPath) if (input != null) { Files.createDirectories(sshConfigPath.getParent()) def originalConf = Path.of("src/test/fixtures/inputs").resolve(input + ".conf").toFile().text @@ -407,7 +428,7 @@ class CoderCLIManagerTest extends Specification { def "fails if config is malformed"() { given: def sshConfigPath = tmpdir.resolve("configured" + input + ".conf") - def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), tmpdir, null, sshConfigPath) + def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), tmpdir, null, null, sshConfigPath) Files.createDirectories(sshConfigPath.getParent()) Files.copy( Path.of("src/test/fixtures/inputs").resolve(input + ".conf"), @@ -468,9 +489,10 @@ class CoderCLIManagerTest extends Specification { where: contents | expected null | ProcessInitException - """echo '{"foo": true, "baz": 1}'""" | InvalidVersionException + """echo '{"foo": true, "baz": 1}'""" | MissingVersionException """echo '{"version: '""" | JsonSyntaxException - "exit 0" | InvalidVersionException + """echo '{"version": "invalid"}'""" | IllegalArgumentException + "exit 0" | MissingVersionException "exit 1" | InvalidExitValueException } @@ -491,7 +513,7 @@ class CoderCLIManagerTest extends Specification { where: contents | build | matches - null | "v1.0.0" | false + null | "v1.0.0" | null """echo '{"version": "v1.0.0"}'""" | "v1.0.0" | true """echo '{"version": "v1.0.0"}'""" | "v1.0.0-devel+b5b5b5b5" | true """echo '{"version": "v1.0.0-devel+b5b5b5b5"}'""" | "v1.0.0-devel+b5b5b5b5" | true @@ -505,7 +527,94 @@ class CoderCLIManagerTest extends Specification { """echo '{"version": ""}'""" | "v1.0.0" | false """echo '{"version": "v1.0.0"}'""" | "" | false """echo '{"version'""" | "v1.0.0" | false - """exit 0""" | "v1.0.0" | false - """exit 1""" | "v1.0.0" | false + """exit 0""" | "v1.0.0" | null + """exit 1""" | "v1.0.0" | null + } + + def "separately configures cli path from data dir"() { + given: + def dir = tmpdir.resolve("cli-dir") + def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), tmpdir, dir) + + expect: + ccm.localBinaryPath.getParent() == dir.resolve("test.coder.invalid") + } + + enum Result { + ERROR, + USE_BIN, + USE_DATA, + } + + @IgnoreIf({ os.windows }) + def "use a separate cli dir"() { + given: + def (srv, url) = mockServer() + def dataDir = tmpdir.resolve("data-dir") + def binDir = tmpdir.resolve("bin-dir") + def mainCCM = new CoderCLIManager(new URL(url), dataDir, binDir) + def fallbackCCM = new CoderCLIManager(new URL(url), dataDir) + + when: + def settings = new CoderSettingsState() + settings.binaryDirectory = binDir.toAbsolutePath() + settings.dataDirectory = dataDir.toAbsolutePath() + settings.enableDownloads = download + settings.enableBinaryDirectoryFallback = fallback + Files.createDirectories(mainCCM.localBinaryPath.parent) + if (version != null) { + mainCCM.localBinaryPath.toFile().text = """#!/bin/sh\necho '{"version": "$version"}'""" + mainCCM.localBinaryPath.toFile().setExecutable(true) + } + mainCCM.localBinaryPath.parent.toFile().setWritable(writable) + if (fallver != null) { + Files.createDirectories(fallbackCCM.localBinaryPath.parent) + fallbackCCM.localBinaryPath.toFile().text = """#!/bin/sh\necho '{"version": "$fallver"}'""" + fallbackCCM.localBinaryPath.toFile().setExecutable(true) + } + def ccm + try { + ccm = CoderCLIManager.ensureCLI(new URL(url), build, settings) + } catch (Exception e) { + ccm = e + } + + then: + expect == Result.ERROR + ? ccm instanceof AccessDeniedException + : ccm.localBinaryPath.parent.parent == (expect == Result.USE_DATA ? dataDir : binDir) + mainCCM.localBinaryPath.toFile().exists() == (version != null || (download && writable)) + fallbackCCM.localBinaryPath.toFile().exists() == (fallver != null || (download && !writable && fallback)) + + + cleanup: + srv.stop(0) + mainCCM.localBinaryPath.parent.toFile().setWritable(true) // So it can get cleaned up. + + where: + version | fallver | build | writable | download | fallback | expect + + // CLI is writable. + null | null | "1.0.0" | true | true | true | Result.USE_BIN // Download. + null | null | "1.0.0" | true | false | true | Result.USE_BIN // No download, error when used. + "1.0.1" | null | "1.0.0" | true | true | true | Result.USE_BIN // Update. + "1.0.1" | null | "1.0.0" | true | false | true | Result.USE_BIN // No update, use outdated. + "1.0.0" | null | "1.0.0" | true | false | true | Result.USE_BIN // Use existing. + + // CLI is *not* writable and fallback is disabled. + null | null | "1.0.0" | false | true | false | Result.ERROR // Fail to download. + null | null | "1.0.0" | false | false | false | Result.USE_BIN // No download, error when used. + "1.0.1" | null | "1.0.0" | false | true | false | Result.ERROR // Fail to update. + "1.0.1" | null | "1.0.0" | false | false | false | Result.USE_BIN // No update, use outdated. + "1.0.0" | null | "1.0.0" | false | false | false | Result.USE_BIN // Use existing. + + // CLI is *not* writable and fallback is enabled. + null | null | "1.0.0" | false | true | true | Result.USE_DATA // Download to fallback. + null | null | "1.0.0" | false | false | true | Result.USE_BIN // No download, error when used. + "1.0.1" | "1.0.1" | "1.0.0" | false | true | true | Result.USE_DATA // Update fallback. + "1.0.1" | "1.0.2" | "1.0.0" | false | false | true | Result.USE_BIN // No update, use outdated. + null | "1.0.2" | "1.0.0" | false | false | true | Result.USE_DATA // No update, use outdated fallback. + "1.0.0" | null | "1.0.0" | false | false | true | Result.USE_BIN // Use existing. + "1.0.1" | "1.0.0" | "1.0.0" | false | false | true | Result.USE_DATA // Use existing fallback. } } diff --git a/src/test/groovy/CoderWorkspacesStepViewTest.groovy b/src/test/groovy/CoderWorkspacesStepViewTest.groovy index c684e963..3d5769e7 100644 --- a/src/test/groovy/CoderWorkspacesStepViewTest.groovy +++ b/src/test/groovy/CoderWorkspacesStepViewTest.groovy @@ -51,4 +51,8 @@ class CoderWorkspacesStepViewTest extends Specification { DataGen.workspace("gone", "ws6") | 7 // Agent gone, workspace comes first. DataGen.workspace("agent3", "ws6") | 8 // Agent exact match. } + + def "gets cli manager"() { + + } } From 46a02ec0b9743f1ce762a52b8d081e52fe9456e2 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 18 May 2023 10:44:16 -0800 Subject: [PATCH 2/2] Remove empty test --- src/test/groovy/CoderWorkspacesStepViewTest.groovy | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/groovy/CoderWorkspacesStepViewTest.groovy b/src/test/groovy/CoderWorkspacesStepViewTest.groovy index 3d5769e7..c684e963 100644 --- a/src/test/groovy/CoderWorkspacesStepViewTest.groovy +++ b/src/test/groovy/CoderWorkspacesStepViewTest.groovy @@ -51,8 +51,4 @@ class CoderWorkspacesStepViewTest extends Specification { DataGen.workspace("gone", "ws6") | 7 // Agent gone, workspace comes first. DataGen.workspace("agent3", "ws6") | 8 // Agent exact match. } - - def "gets cli manager"() { - - } }