Skip to content

Commit 4da78f0

Browse files
code-ashercode-asher
committed
Refactor CLI manager to support multiple deployments
Currently multiple deployments will delete each other's binaries so this uses a directory for each deployment host to store the binaries. Add a catch for download errors; previously they would go unnoticed. Also refactor the download a little; instead of checking for the file's existence catch the appropriate error and set the permissions from Kotlin code rather than spawning chmod. Remove a chunk of code where we configure the CLI again before moving to the next step; we already do that when we first connect (plus it is missing some checks like making sure the CLI has actually been downloaded and is the right version, etc). This also makes it unnecessary to globally store the version and CLI path on the model since we now only do it in one spot. This is just a first step; in a future PR the config code will all be extracted out since we will want to configure every time we try to connect (rather than just in the initial setup step) since otherwise the recent connections can fail if you have configured a different deployment in the meantime (and probably reconnections would also fail if you already have an IDE open) or if you have restarted (since the binaries are currently in tmp). We will also need to configure ourselves since `config-ssh` does not support multiple deployments and we want to add an environment variable via SetEnv.
1 parent 2c26120 commit 4da78f0

File tree

4 files changed

+146
-66
lines changed

4 files changed

+146
-66
lines changed

‎src/main/kotlin/com/coder/gateway/models/CoderWorkspacesWizardModel.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package com.coder.gateway.models
33
data class CoderWorkspacesWizardModel(
44
var coderURL: String = "https://coder.example.com",
55
var token: String = "",
6-
var buildVersion: String = "",
7-
var localCliPath: String = "",
86
var selectedWorkspace: WorkspaceAgentModel? = null,
97
var useExistingToken: Boolean = false
108
)

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

Lines changed: 61 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,43 @@ package com.coder.gateway.sdk
33
import com.intellij.openapi.diagnostic.Logger
44
import java.io.InputStream
55
import java.net.URL
6-
import java.nio.file.FileVisitOption
76
import java.nio.file.Files
87
import java.nio.file.Path
9-
import java.nio.file.Paths
10-
import java.nio.file.StandardCopyOption
8+
import java.nio.file.attribute.PosixFilePermissions
119

12-
class CoderCLIManager(url: URL, buildVersion: String) {
13-
var remoteCli: URL
14-
var localCli: Path
15-
private var cliNamePrefix: String
16-
private var tmpDir: String
17-
private var cliFileName: String
10+
/**
11+
* Manage the CLI for a single deployment.
12+
*/
13+
class CoderCLIManager(deployment: URL, buildVersion: String) {
14+
private var remoteBinaryUrl: URL
15+
var localBinaryPath: Path
16+
private var binaryNamePrefix: String
17+
private var destinationDir: Path
18+
private var localBinaryName: String
1819

1920
init {
21+
// TODO: Should use a persistent path to avoid needing to download on
22+
// each restart.
23+
destinationDir = Path.of(System.getProperty("java.io.tmpdir"))
24+
.resolve("coder-gateway").resolve(deployment.host)
2025
val os = getOS()
21-
cliNamePrefix = getCoderCLIForOS(os, getArch())
22-
val cliNameWithExt = if (os == OS.WINDOWS) "$cliNamePrefix.exe" else cliNamePrefix
23-
cliFileName = if (os == OS.WINDOWS) "${cliNamePrefix}-${buildVersion}.exe" else "${cliNamePrefix}-${buildVersion}"
24-
25-
remoteCli = URL(url.protocol, url.host, url.port, "/bin/$cliNameWithExt")
26-
tmpDir = System.getProperty("java.io.tmpdir")
27-
localCli = Paths.get(tmpDir, cliFileName)
26+
binaryNamePrefix = getCoderCLIForOS(os, getArch())
27+
val binaryName = if (os == OS.WINDOWS) "$binaryNamePrefix.exe" else binaryNamePrefix
28+
localBinaryName =
29+
if (os == OS.WINDOWS) "${binaryNamePrefix}-${buildVersion}.exe" else "${binaryNamePrefix}-${buildVersion}"
30+
remoteBinaryUrl = URL(
31+
deployment.protocol,
32+
deployment.host,
33+
deployment.port,
34+
"/bin/$binaryName"
35+
)
36+
localBinaryPath = destinationDir.resolve(localBinaryName)
2837
}
2938

39+
/**
40+
* Return the name of the binary (sans extension) for the provided OS and
41+
* architecture.
42+
*/
3043
private fun getCoderCLIForOS(os: OS?, arch: Arch?): String {
3144
logger.info("Resolving coder cli for $os $arch")
3245
if (os == null) {
@@ -55,31 +68,50 @@ class CoderCLIManager(url: URL, buildVersion: String) {
5568
}
5669
}
5770

71+
/**
72+
* Download the CLI from the deployment if necessary.
73+
*/
5874
fun downloadCLI(): Boolean {
59-
if (Files.exists(localCli)) {
60-
logger.info("${localCli.toAbsolutePath()} already exists, skipping download")
75+
Files.createDirectories(destinationDir)
76+
try {
77+
logger.info("Downloading Coder CLI to ${localBinaryPath.toAbsolutePath()}")
78+
remoteBinaryUrl.openStream().use {
79+
Files.copy(it as InputStream, localBinaryPath)
80+
}
81+
} catch (e: java.nio.file.FileAlreadyExistsException) {
82+
// This relies on the provided build version being the latest. It
83+
// must be freshly fetched immediately before downloading.
84+
// TODO: Use etags instead?
85+
logger.info("${localBinaryPath.toAbsolutePath()} already exists, skipping download")
6186
return false
6287
}
63-
logger.info("Starting Coder CLI download to ${localCli.toAbsolutePath()}")
64-
remoteCli.openStream().use {
65-
Files.copy(it as InputStream, localCli, StandardCopyOption.REPLACE_EXISTING)
88+
if (getOS() != OS.WINDOWS) {
89+
Files.setPosixFilePermissions(
90+
localBinaryPath,
91+
PosixFilePermissions.fromString("rwxr-x---")
92+
)
6693
}
6794
return true
6895
}
6996

97+
/**
98+
* Remove all versions of the CLI for this deployment that do not match the
99+
* current build version.
100+
*/
70101
fun removeOldCli() {
71-
val tmpPath = Path.of(tmpDir)
72-
if (Files.isReadable(tmpPath)) {
73-
Files.walk(tmpPath, 1).use {
74-
it.sorted().map { pt -> pt.toFile() }.filter { fl -> fl.name.contains(cliNamePrefix) && !fl.name.contains(cliFileName) }.forEach { fl ->
75-
logger.info("Removing $fl because it is an old coder cli")
76-
fl.delete()
77-
}
102+
if (Files.isReadable(destinationDir)) {
103+
Files.walk(destinationDir, 1).use {
104+
it.sorted().map { pt -> pt.toFile() }
105+
.filter { fl -> fl.name.contains(binaryNamePrefix) && fl.name != localBinaryName }
106+
.forEach { fl ->
107+
logger.info("Removing $fl because it is an old version")
108+
fl.delete()
109+
}
78110
}
79111
}
80112
}
81113

82114
companion object {
83115
val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName)
84116
}
85-
}
117+
}

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

Lines changed: 11 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ import com.intellij.openapi.application.ApplicationManager
3434
import com.intellij.openapi.application.ModalityState
3535
import com.intellij.openapi.components.service
3636
import com.intellij.openapi.diagnostic.Logger
37-
import com.intellij.openapi.progress.ProgressIndicator
38-
import com.intellij.openapi.progress.ProgressManager
39-
import com.intellij.openapi.progress.Task
4037
import com.intellij.openapi.rd.util.launchUnderBackgroundProgress
4138
import com.intellij.openapi.ui.panel.ComponentPanelBuilder
4239
import com.intellij.openapi.wm.impl.welcomeScreen.WelcomeScreenUIManager
@@ -472,12 +469,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
472469
}
473470

474471
val cliManager = CoderCLIManager(localWizardModel.coderURL.toURL(), coderClient.buildVersion)
475-
476-
localWizardModel.apply {
477-
this.token = token
478-
buildVersion = coderClient.buildVersion
479-
localCliPath = cliManager.localCli.toAbsolutePath().toString()
480-
}
472+
localWizardModel.token = token
481473

482474
this.indicator.apply {
483475
isIndeterminate = false
@@ -492,12 +484,11 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
492484
text = "Downloading Coder CLI..."
493485
fraction = 0.3
494486
}
495-
496-
cliManager.downloadCLI()
497-
if (getOS() != OS.WINDOWS) {
498-
this.indicator.fraction = 0.4
499-
val chmodOutput = ProcessExecutor().command("chmod", "+x", localWizardModel.localCliPath).readOutput(true).execute().outputUTF8()
500-
logger.info("chmod +x ${cliManager.localCli.toAbsolutePath()} $chmodOutput")
487+
try {
488+
cliManager.downloadCLI()
489+
} catch (e: Exception) {
490+
logger.error("Failed to download Coder CLI", e)
491+
return@launchUnderBackgroundProgress
501492
}
502493
this.indicator.apply {
503494
text = "Configuring Coder CLI..."
@@ -575,7 +566,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
575566
* versions do not match.
576567
*/
577568
private fun authenticate(token: String) {
578-
logger.info("Authenticating...")
569+
logger.info("Authenticating to ${localWizardModel.coderURL}...")
579570
coderClient.initClientSession(localWizardModel.coderURL.toURL(), token)
580571

581572
logger.info("Checking Coder version...")
@@ -593,10 +584,14 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
593584
component.isVisible = true
594585
showWarning(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.unsupported.coder.version", coderClient.buildVersion))
595586
}
587+
} else {
588+
logger.info("Got version ${coderClient.buildVersion}...")
596589
}
597590
}
598591

599592
logger.info("Authenticated successfully")
593+
594+
// Remember these in order to default to them for future attempts.
600595
appPropertiesService.setValue(CODER_URL_KEY, localWizardModel.coderURL)
601596
appPropertiesService.setValue(SESSION_TOKEN, token)
602597
}
@@ -714,28 +709,9 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
714709
}
715710

716711
override fun onNext(wizardModel: CoderWorkspacesWizardModel): Boolean {
717-
if (localWizardModel.localCliPath.isNotBlank()) {
718-
val configSSHTask = object : Task.Modal(null, CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.cli.configssh.dialog.title"), false) {
719-
override fun run(pi: ProgressIndicator) {
720-
pi.apply {
721-
isIndeterminate = false
722-
text = "Configuring coder cli..."
723-
fraction = 0.1
724-
}
725-
val sshConfigOutput = ProcessExecutor().command(localWizardModel.localCliPath, "config-ssh", "--yes", "--use-previous-options").readOutput(true).execute().outputUTF8()
726-
pi.fraction = 0.8
727-
logger.info("Result of `${localWizardModel.localCliPath} config-ssh --yes --use-previous-options`: $sshConfigOutput")
728-
pi.fraction = 1.0
729-
}
730-
}
731-
ProgressManager.getInstance().run(configSSHTask)
732-
}
733-
734712
wizardModel.apply {
735713
coderURL = localWizardModel.coderURL
736714
token = localWizardModel.token
737-
buildVersion = localWizardModel.buildVersion
738-
localCliPath = localWizardModel.localCliPath
739715
}
740716

741717
val workspace = tableOfWorkspaces.selectedObject

‎src/test/groovy/CoderCLIManagerTest.groovy

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package com.coder.gateway.sdk
2+
3+
import org.zeroturnaround.exec.ProcessExecutor
4+
import spock.lang.Unroll
5+
6+
import java.nio.file.Files
7+
import java.nio.file.Path
8+
9+
@Unroll
10+
class CoderCLIManagerTest extends spock.lang.Specification {
11+
def "deletes old versions"() {
12+
given:
13+
// Simulate downloading an old version.
14+
def oldVersion = new CoderCLIManager(new URL("https://test.coder.invalid"), "0.0.1").localBinaryPath.toFile()
15+
def dir = oldVersion.toPath().getParent()
16+
dir.toFile().deleteDir()
17+
Files.createDirectories(dir)
18+
oldVersion.write("old-version")
19+
20+
// Simulate downloading a new version.
21+
def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), "1.0.2")
22+
def newVersion = ccm.localBinaryPath.toFile()
23+
newVersion.write("new-version")
24+
25+
// Anything that does not start with the expected prefix is ignored.
26+
def otherOsVersion = dir.resolve("coder-alpine-amd64-1.0.2").toFile()
27+
otherOsVersion.write("alpine")
28+
29+
// Anything else matching the prefix is deleted.
30+
def invalidVersion = dir.resolve(newVersion.getName() + "-extra-random-text").toFile()
31+
invalidVersion.write("invalid")
32+
33+
when:
34+
ccm.removeOldCli()
35+
36+
then:
37+
!oldVersion.exists()
38+
newVersion.exists()
39+
otherOsVersion.exists()
40+
!invalidVersion.exists()
41+
}
42+
43+
// TODO: Probably not good to depend on dev.coder.com being up...should use
44+
// a mock? Or spin up a Coder deployment in CI?
45+
def "downloads a working cli"() {
46+
given:
47+
def ccm = new CoderCLIManager(new URL("https://dev.coder.com"), "1.0.1")
48+
def dir = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway/dev.coder.com")
49+
ccm.localBinaryPath.getParent().toFile().deleteDir()
50+
51+
when:
52+
def downloaded = ccm.downloadCLI()
53+
54+
then:
55+
downloaded
56+
ccm.localBinaryPath.getParent() == dir
57+
ccm.localBinaryPath.toFile().exists()
58+
new ProcessExecutor().command(ccm.localBinaryPath.toString(), "version").readOutput(true).execute().outputUTF8().contains("Coder")
59+
}
60+
61+
def "skips cli download if it already exists"() {
62+
given:
63+
def ccm = new CoderCLIManager(new URL("https://dev.coder.com"), "1.0.1")
64+
Files.createDirectories(ccm.localBinaryPath.getParent())
65+
ccm.localBinaryPath.toFile().write("cli")
66+
67+
when:
68+
def downloaded = ccm.downloadCLI()
69+
70+
then:
71+
!downloaded
72+
ccm.localBinaryPath.toFile().readLines() == ["cli"]
73+
}
74+
}

0 commit comments

Comments
 (0)