Allow configuring CLI directory separately from data

This is so admins can download the CLI to some restricted location (like
ProgramFiles).

Author: code-asher

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,16 +21,6 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
     override fun createPanel(): DialogPanel {
         val state: CoderSettingsState = service()
         return panel {
-            row(CoderGatewayBundle.message("gateway.connector.settings.binary-source.title")) {
-                textField().resizableColumn().align(AlignX.FILL)
-                    .bindText(state::binarySource)
-                    .comment(
-                        CoderGatewayBundle.message(
-                            "gateway.connector.settings.binary-source.comment",
-                            CoderCLIManager(URL("http://localhost"), CoderCLIManager.getDataDir()).remoteBinaryURL.path,
-                        )
-                    )
-            }
             row(CoderGatewayBundle.message("gateway.connector.settings.data-directory.title")) {
                 textField().resizableColumn().align(AlignX.FILL)
                     .bindText(state::dataDirectory)
@@ -40,14 +32,40 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
                             CoderCLIManager.getDataDir(),
                         )
                     )
-            }
+            }.layout(RowLayout.PARENT_GRID)
+            row(CoderGatewayBundle.message("gateway.connector.settings.binary-source.title")) {
+                textField().resizableColumn().align(AlignX.FILL)
+                    .bindText(state::binarySource)
+                    .comment(
+                        CoderGatewayBundle.message(
+                            "gateway.connector.settings.binary-source.comment",
+                            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::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-data-fallback.title"))
+                    .bindSelected(state::enableCLIDataFallback)
+                    .comment(
+                        CoderGatewayBundle.message("gateway.connector.settings.enable-binary-data-fallback.comment")
+                    )
+            }.layout(RowLayout.PARENT_GRID)
         }
     }
 

src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt

@@ -3,6 +3,7 @@ package com.coder.gateway.sdk
 import com.coder.gateway.models.WorkspaceAgentModel
 import com.coder.gateway.views.steps.CoderWorkspacesStepView
 import com.google.gson.Gson
+import com.google.gson.JsonSyntaxException
 import com.intellij.openapi.diagnostic.Logger
 import org.zeroturnaround.exec.ProcessExecutor
 import java.io.BufferedInputStream
@@ -294,26 +295,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 {
@@ -419,5 +441,5 @@ class Environment(private val env: Map<String, String> = emptyMap()) {
 }
 
 class ResponseException(message: String, val code: Int) : Exception(message)
-
 class SSHConfigFormatException(message: String) : Exception(message)
+class MissingVersionException(message: String) : Exception(message)

src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt

@@ -16,6 +16,8 @@ class CoderSettingsState : PersistentStateComponent<CoderSettingsState> {
     var binarySource: String = ""
     var binaryDirectory: String = ""
     var dataDirectory: String = ""
+    var enableDownloads: Boolean = true
+    var enableCLIDataFallback: Boolean = false
     override fun getState(): CoderSettingsState {
         return this
     }

src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt

@@ -33,6 +33,7 @@ import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.application.ModalityState
 import com.intellij.openapi.components.service
 import com.intellij.openapi.diagnostic.Logger
+import com.intellij.openapi.progress.ProgressIndicator
 import com.intellij.openapi.rd.util.launchUnderBackgroundProgress
 import com.intellij.openapi.ui.panel.ComponentPanelBuilder
 import com.intellij.openapi.ui.setEmptyState
@@ -458,44 +459,14 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
             canBeCancelled = false,
             isIndeterminate = true
         ) {
-            val dataDir =
-                if (settings.dataDirectory.isBlank()) CoderCLIManager.getDataDir()
-                else Path.of(settings.dataDirectory).toAbsolutePath()
-            val cliDir =
-                if (settings.binaryDirectory.isBlank()) null
-                else Path.of(settings.binaryDirectory).toAbsolutePath()
-            var cli = CoderCLIManager(deploymentURL, dataDir, cliDir, settings.binarySource)
+            val cli = getCLIManager(deploymentURL, this.indicator)
             try {
                 this.indicator.text = "Authenticating client..."
                 authenticate(deploymentURL, token.first)
                 // Remember these in order to default to them for future attempts.
                 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 (!cli.matchesVersion(clientService.buildVersion)) {
-                    this.indicator.text = "Downloading Coder CLI..."
-                    try {
-                        cli.downloadCLI()
-                    } catch (e: java.nio.file.AccessDeniedException) {
-                        // Try the data directory instead.
-                        if (cliDir != null && !cliDir.startsWith(dataDir)) {
-                            val oldPath = cli.localBinaryPath
-                            cli = CoderCLIManager(deploymentURL, dataDir, null, settings.binarySource )
-                            logger.info("Cannot write to $oldPath, falling back to ${cli.localBinaryPath}")
-                            if (!cli.matchesVersion(clientService.buildVersion)) {
-                                cli.downloadCLI()
-                            }
-                        } else {
-                            throw e
-                        }
-                    }
-                }
-
                 this.indicator.text = "Authenticating Coder CLI..."
                 cli.login(token.first)
 
@@ -548,6 +519,58 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
         }
     }
 
+    private fun getCLIManager(deploymentURL: URL, indicator: ProgressIndicator): CoderCLIManager {
+        val dataDir =
+            if (settings.dataDirectory.isBlank()) CoderCLIManager.getDataDir()
+            else Path.of(settings.dataDirectory).toAbsolutePath()
+        val cliDir =
+            if (settings.binaryDirectory.isBlank()) null
+            else Path.of(settings.binaryDirectory).toAbsolutePath()
+
+        val cli = CoderCLIManager(deploymentURL, dataDir, cliDir, 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(clientService.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 (cliDir != null && !cliDir.startsWith(dataDir) && settings.enableCLIDataFallback) {
+                    throw e
+                }
+            }
+        }
+
+        // Try falling back to the data directory.
+        val dataCLI = CoderCLIManager(deploymentURL, dataDir, null, settings.binarySource)
+        if (dataCLI.matchesVersion(clientService.buildVersion) == true) {
+            return dataCLI
+        }
+
+        if (settings.enableDownloads) {
+            indicator.text = "Downloading Coder CLI..."
+            dataCLI.downloadCLI()
+            return dataCLI
+        }
+
+        // In the case where both directories do not have a matching binary
+        // prefer whichever does have a binary, if any, and prefer the CLI
+        // directory if they both have out-of-date versions.
+        return if (cliMatches != null) cli else dataCLI
+    }
+
     /**
      * Open a dialog for providing the token.  Show any existing token so the
      * user can validate it if a previous connection failed.  If we are not
@@ -935,5 +958,4 @@ class WorkspacesTable : TableView<WorkspaceAgentModel>(WorkspacesTableModel()) {
         }
         return listTableModel.items.indexOfFirst { it.workspaceName == oldSelection.workspaceName }
     }
-
 }

src/main/resources/messages/CoderGatewayBundle.properties

@@ -48,23 +48,28 @@ 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 \
   will be set to the SHA1 of the CLI and can be used for caching. Absolute \
   URLs will be used as-is; otherwise this value will be resolved against the \
   deployment domain. \
   Defaults to {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.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 for each domain to which the plugin connects. If the \
-  plugin does not have permissions to write here any existing CLI will still be \
-  used but if the CLI needs to be downloaded or updated the data directory will \
-  be used instead. \
+  here that store the CLI for each domain to which the plugin connects. \
   Defaults to the data directory.
+gateway.connector.settings.enable-binary-data-fallback.title=Fall back to data directory
+gateway.connector.settings.enable-binary-data-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"

src/test/groovy/CoderCLIManagerTest.groovy

@@ -487,9 +487,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
     }
 
@@ -510,7 +511,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
@@ -524,8 +525,8 @@ 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"() {

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"() {
+
+    }
 }