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

@@ -60,10 +60,10 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
             }.layout(RowLayout.PARENT_GRID)
             row {
                 cell() // For alignment.
-                checkBox(CoderGatewayBundle.message("gateway.connector.settings.enable-binary-data-fallback.title"))
-                    .bindSelected(state::enableCLIDataFallback)
+                checkBox(CoderGatewayBundle.message("gateway.connector.settings.enable-binary-directory-fallback.title"))
+                    .bindSelected(state::enableBinaryDirectoryFallback)
                     .comment(
-                        CoderGatewayBundle.message("gateway.connector.settings.enable-binary-data-fallback.comment")
+                        CoderGatewayBundle.message("gateway.connector.settings.enable-binary-directory-fallback.comment")
                     )
             }.layout(RowLayout.PARENT_GRID)
         }

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

@@ -1,10 +1,12 @@
 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
@@ -427,6 +429,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
+        }
     }
 }
 

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

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

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

@@ -33,7 +33,6 @@ 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
@@ -81,7 +80,6 @@ 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 javax.swing.Icon
 import javax.swing.JCheckBox
 import javax.swing.JTable
@@ -459,7 +457,12 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
             canBeCancelled = false,
             isIndeterminate = true
         ) {
-            val cli = getCLIManager(deploymentURL, this.indicator)
+            val cli = CoderCLIManager.ensureCLI(
+                deploymentURL,
+                clientService.buildVersion,
+                settings,
+                this.indicator,
+            )
             try {
                 this.indicator.text = "Authenticating client..."
                 authenticate(deploymentURL, token.first)
@@ -519,58 +522,6 @@ 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

src/main/resources/messages/CoderGatewayBundle.properties

@@ -68,8 +68,8 @@ 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. \
   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 \
+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"

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
@@ -537,4 +538,80 @@ class CoderCLIManagerTest extends Specification {
         expect:
         ccm.localBinaryPath.getParent() == dir.resolve("test.coder.invalid")
     }
+
+    @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 = enableFallback
+        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 (fallback != null) {
+            Files.createDirectories(fallbackCCM.localBinaryPath.parent)
+            fallbackCCM.localBinaryPath.toFile().text = """#!/bin/sh\necho '{"version": "$fallback"}'"""
+            fallbackCCM.localBinaryPath.toFile().setExecutable(true)
+        }
+        def ccm
+        try {
+            ccm = CoderCLIManager.ensureCLI(new URL(url), build, settings)
+        } catch (Exception e) {
+            ccm = e
+        }
+
+        then:
+        // The binaries will exist if either they already existed or were downloaded.
+        mainCCM.localBinaryPath.toFile().exists() == (version != null || (download && writable))
+        fallbackCCM.localBinaryPath.toFile().exists() == (fallback != null || (download && !writable && enableFallback))
+        // We will get an error if we tried to download to the binary directory and it was not writable and we could
+        // not fall back to the data directory, and we should be using whichever already had the most recent
+        // binary, managed to download the most recent binary, or the binary directory if neither were able.
+        download && !writable && !enableFallback && version != build
+                ? ccm instanceof AccessDeniedException
+                : ccm.localBinaryPath.parent.parent == (enableFallback && !writable && version != build &&
+                (download || fallback == build || (fallback != null && version == null)) ? dataDir : binDir)
+
+        cleanup:
+        srv.stop(0)
+        mainCCM.localBinaryPath.parent.toFile().setWritable(true) // So it can get cleaned up.
+
+        where:
+        version | fallback | build   | writable | download | enableFallback
+
+        // CLI is writable.
+        null    | null     | "1.0.0" | true     | true     | true
+        null    | null     | "1.0.0" | true     | false    | true
+        "1.0.1" | null     | "1.0.0" | true     | true     | true
+        "1.0.1" | null     | "1.0.0" | true     | false    | true
+        "1.0.0" | null     | "1.0.0" | true     | false    | true
+
+        // CLI is *not* writable and fallback is disabled.
+        null    | null     | "1.0.0" | false    | true     | false
+        null    | null     | "1.0.0" | false    | false    | false
+        "1.0.1" | null     | "1.0.0" | false    | true     | false
+        "1.0.1" | null     | "1.0.0" | false    | false    | false
+        "1.0.0" | null     | "1.0.0" | false    | false    | false
+
+        // CLI is *not* writable and fallback is enabled.
+        null    | null     | "1.0.0" | false    | true     | true
+        null    | null     | "1.0.0" | false    | false    | true
+        "1.0.1" | "1.0.1"  | "1.0.0" | false    | true     | true
+        "1.0.1" | "1.0.2"  | "1.0.0" | false    | false    | true
+        null    | "1.0.2"  | "1.0.0" | false    | false    | true
+        "1.0.0" | null     | "1.0.0" | false    | false    | true
+        "1.0.1" | "1.0.0"  | "1.0.0" | false    | false    | true
+    }
 }