Allow configuring CLI directory separately from data

code-asher · code-asher · commit ff6495137f06 · 2023-05-10T15:25:01.000-08:00
This is so admins can download the CLI to some restricted location (like
ProgramFiles).

This will break if they upload the wrong version and the plugin tries to
download a new one since it will not have permissions to do so.
diff --git a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt
@@ -29,22 +29,29 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
                         )
                     )
             }
-            row(CoderGatewayBundle.message("gateway.connector.settings.binary-destination.title")) {
+            row(CoderGatewayBundle.message("gateway.connector.settings.data-directory.title")) {
                 textField().resizableColumn().align(AlignX.FILL)
-                    .bindText(state::binaryDestination)
-                    .validationOnApply(validateBinaryDestination())
-                    .validationOnInput(validateBinaryDestination())
+                    .bindText(state::dataDirectory)
+                    .validationOnApply(validateDataDirectory())
+                    .validationOnInput(validateDataDirectory())
                     .comment(
                         CoderGatewayBundle.message(
-                            "gateway.connector.settings.binary-destination.comment",
+                            "gateway.connector.settings.data-directory.comment",
                             CoderCLIManager.getDataDir(),
                         )
                     )
             }
+            // 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"))
+            }
         }
     }
 
-    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
@@ -26,7 +26,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 +53,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()
     }
 
     /**
diff --git a/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt b/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt
@@ -14,7 +14,8 @@ import com.intellij.util.xmlb.XmlSerializerUtil
 )
 class CoderSettingsState : PersistentStateComponent<CoderSettingsState> {
     var binarySource: String = ""
-    var binaryDestination: String = ""
+    var binaryDirectory: String = ""
+    var dataDirectory: String = ""
     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
@@ -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
@@ -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,13 @@ 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,
-            )
+            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)
             try {
                 this.indicator.text = "Authenticating client..."
                 authenticate(deploymentURL, token.first)
@@ -472,23 +477,42 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
                 // 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)) {
+                if (!cli.matchesVersion(clientService.buildVersion)) {
                     this.indicator.text = "Downloading Coder CLI..."
-                    cliManager.downloadCLI()
+                    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..."
-                cliManager.login(token.first)
+                cli.login(token.first)
 
                 this.indicator.text = "Retrieving workspaces..."
                 loadWorkspaces()
 
                 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 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 +537,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
                     is ResponseException, is ConnectException -> {
                         msg = CoderGatewayBundle.message(
                             "gateway.connector.view.workspaces.connect.download-failed",
-                            cliManager.remoteBinaryURL,
+                            cli.remoteBinaryURL,
                             errorSummary,
                         )
                     }
@@ -700,29 +724,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() {
diff --git a/src/main/resources/messages/CoderGatewayBundle.properties b/src/main/resources/messages/CoderGatewayBundle.properties
@@ -23,7 +23,7 @@ gateway.connector.view.coder.workspaces.unsupported.coder.version=Coder version
 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.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.
@@ -55,9 +55,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.binary-destination.comment=Directories are created \
-  here that store the CLI and credentials for each domain to which the plugin \
+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-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. \
+  Defaults to the data directory.
 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
@@ -8,6 +8,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 +119,24 @@ class CoderCLIManagerTest extends Specification {
         srv.stop(0)
     }
 
+    def "fails to write"() {
+        given:
+        def (srv, url) = mockServer()
+        def dir = tmpdir.resolve("cli-dir-fallback")
+        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 +261,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 +381,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 +426,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"),
@@ -508,4 +527,13 @@ class CoderCLIManagerTest extends Specification {
         """exit 0"""                                      | "v1.0.0"                | false
         """exit 1"""                                      | "v1.0.0"                | false
     }
+
+    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")
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -29,22 +29,29 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {`
`29`	`29`	`)`
`30`	`30`	`)`
`31`	`31`	`}`
`32`		`- row(CoderGatewayBundle.message("gateway.connector.settings.binary-destination.title")) {`
	`32`	`+ row(CoderGatewayBundle.message("gateway.connector.settings.data-directory.title")) {`
`33`	`33`	`textField().resizableColumn().align(AlignX.FILL)`
`34`		`- .bindText(state::binaryDestination)`
`35`		`- .validationOnApply(validateBinaryDestination())`
`36`		`- .validationOnInput(validateBinaryDestination())`
	`34`	`+ .bindText(state::dataDirectory)`
	`35`	`+ .validationOnApply(validateDataDirectory())`
	`36`	`+ .validationOnInput(validateDataDirectory())`
`37`	`37`	`.comment(`
`38`	`38`	`CoderGatewayBundle.message(`
`39`		`- "gateway.connector.settings.binary-destination.comment",`
	`39`	`+ "gateway.connector.settings.data-directory.comment",`
`40`	`40`	`CoderCLIManager.getDataDir(),`
`41`	`41`	`)`
`42`	`42`	`)`
`43`	`43`	`}`
	`44`	`+ // The binary directory is not validated because it could be a`
	`45`	`+ // read-only path that is pre-downloaded by admins.`
	`46`	`+ row(CoderGatewayBundle.message("gateway.connector.settings.binary-destination.title")) {`
	`47`	`+ textField().resizableColumn().align(AlignX.FILL)`
	`48`	`+ .bindText(state::binaryDirectory)`
	`49`	`+ .comment(CoderGatewayBundle.message("gateway.connector.settings.binary-destination.comment"))`
	`50`	`+ }`
`44`	`51`	`}`
`45`	`52`	`}`
`46`	`53`
`47`		`- private fun validateBinaryDestination(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = {`
	`54`	`+ private fun validateDataDirectory(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = {`
`48`	`55`	`if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) {`
`49`	`56`	`error("Cannot create this directory")`
`50`	`57`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,8 @@ import com.intellij.util.xmlb.XmlSerializerUtil`
`14`	`14`	`)`
`15`	`15`	`class CoderSettingsState : PersistentStateComponent<CoderSettingsState> {`
`16`	`16`	`var binarySource: String = ""`
`17`		`- var binaryDestination: String = ""`
	`17`	`+ var binaryDirectory: String = ""`
	`18`	`+ var dataDirectory: String = ""`
`18`	`19`	`override fun getState(): CoderSettingsState {`
`19`	`20`	`return this`
`20`	`21`	`}`