From 559cd35842123b922f5b3588e49d656c23b7ebd9 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 29 Mar 2023 12:17:57 -0800 Subject: [PATCH 01/21] Remove unused wizard variable --- .../gateway/views/steps/CoderLocateRemoteProjectStepView.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderLocateRemoteProjectStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderLocateRemoteProjectStepView.kt index 2b8d44de..a3b6a833 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderLocateRemoteProjectStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderLocateRemoteProjectStepView.kt @@ -78,7 +78,6 @@ class CoderLocateRemoteProjectStepView(private val setNextButtonEnabled: (Boolea private var ideComboBoxModel = DefaultComboBoxModel() private lateinit var titleLabel: JLabel - private lateinit var wizard: CoderWorkspacesWizardModel private lateinit var cbIDE: IDEComboBox private lateinit var cbIDEComment: JLabel private var tfProject = JBTextField() @@ -152,7 +151,6 @@ class CoderLocateRemoteProjectStepView(private val setNextButtonEnabled: (Boolea override fun onInit(wizardModel: CoderWorkspacesWizardModel) { cbIDE.renderer = IDECellRenderer() ideComboBoxModel.removeAllElements() - wizard = wizardModel val selectedWorkspace = wizardModel.selectedWorkspace if (selectedWorkspace == null) { logger.warn("No workspace was selected. Please go back to the previous step and select a Coder Workspace") From 920018653616ae8957582e412f13dbeb4eeff177 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 29 Mar 2023 12:39:00 -0800 Subject: [PATCH 02/21] Add more logs around authenticating Also rename the function since it loads all workspaces, not just one. --- .../gateway/views/steps/CoderWorkspacesStepView.kt | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 69b6e1cf..6c4730f3 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -353,7 +353,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod localWizardModel.token = token } if (!url.isNullOrBlank() && !token.isNullOrBlank()) { - loginAndLoadWorkspace(token, true) + loginAndLoadWorkspaces(token, true) } } updateWorkspaceActions() @@ -451,10 +451,10 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } // False so that subsequent authentication failures do not keep opening // the browser as it was already opened earlier. - loginAndLoadWorkspace(pastedToken, false) + loginAndLoadWorkspaces(pastedToken, false) } - private fun loginAndLoadWorkspace(token: String, openBrowser: Boolean) { + private fun loginAndLoadWorkspaces(token: String, openBrowser: Boolean) { LifetimeDefinition().launchUnderBackgroundProgress(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.cli.downloader.dialog.title"), canBeCancelled = false, isIndeterminate = true) { this.indicator.apply { text = "Authenticating..." @@ -575,9 +575,12 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod * versions do not match. */ private fun authenticate(token: String) { + logger.info("Authenticating...") coderClient.initClientSession(localWizardModel.coderURL.toURL(), token) + logger.info("Checking Coder version...") if (!CoderSemVer.isValidVersion(coderClient.buildVersion)) { + logger.warn("Got invalid version ${coderClient.buildVersion}") notificationBanner.apply { component.isVisible = true showWarning(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.invalid.coder.version", coderClient.buildVersion)) @@ -585,6 +588,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } else { val coderVersion = CoderSemVer.parse(coderClient.buildVersion) if (!coderVersion.isInClosedRange(CoderSupportedVersions.minCompatibleCoderVersion, CoderSupportedVersions.maxCompatibleCoderVersion)) { + logger.warn("Running with incompatible version ${coderClient.buildVersion}") notificationBanner.apply { component.isVisible = true showWarning(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.unsupported.coder.version", coderClient.buildVersion)) @@ -592,6 +596,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } } + logger.info("Authenticated successfully") appPropertiesService.setValue(CODER_URL_KEY, localWizardModel.coderURL) appPropertiesService.setValue(SESSION_TOKEN, token) } From 2c26120136fc88641e7c46ccc1058000cf98ed32 Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 31 Mar 2023 09:34:59 -0800 Subject: [PATCH 03/21] Test on macOS and Windows Since the comments are now out of date I removed them; I figure it is more or less as easy to just read the workflow to see what it does. --- .github/workflows/build.yml | 74 +++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d5fe638a..0eaf8799 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,46 +1,61 @@ -# GitHub Actions Workflow created for testing and preparing the plugin release in following steps: -# - validate Gradle Wrapper, -# - run 'test' and 'verifyPlugin' tasks, -# - run Qodana inspections, -# - run 'buildPlugin' task and prepare artifact for the further tests, -# - run 'runPluginVerifier' task, -# - create a draft release. -# -# Workflow is triggered on push and pull_request events. -# +# GitHub Actions workflow for testing and preparing the plugin release. # GitHub Actions reference: https://help.github.com/en/actions -# - name: Coder Gateway Plugin Build + on: - # Trigger the workflow on pushes to only the 'main' branch (this avoids duplicate checks being run e.g. for dependabot pull requests) push: - branches: [ main ] - # Trigger the workflow on any pull request + branches: + - main pull_request: jobs: - # Run Gradle Wrapper Validation Action to verify the wrapper's checksum - # Run verifyPlugin, IntelliJ Plugin Verifier, and test Gradle tasks - # Build plugin and provide the artifact for the next workflow jobs + # Run plugin tests on every supported platform. + test: + strategy: + matrix: + platform: + - ubuntu-latest + - macos-latest + - windows-latest + runs-on: ${{ matrix.platform }} + steps: + - uses: actions/checkout@v3.5.0 + + - uses: actions/setup-java@v3 + with: + distribution: zulu + java-version: 17 + cache: gradle + + - uses: gradle/wrapper-validation-action@v1.0.6 + + # Run tests + - run: ./gradlew test + + # Collect Tests Result of failed tests + - if: ${{ failure() }} + uses: actions/upload-artifact@v3 + with: + name: tests-result + path: ${{ github.workspace }}/build/reports/tests + + # Run Gradle Wrapper Validation Action to verify the wrapper's checksum. Run + # verifyPlugin and IntelliJ Plugin Verifier. Build plugin and provide the + # artifact for the next workflow jobs. build: name: Build + needs: test runs-on: ubuntu-latest outputs: version: ${{ steps.properties.outputs.version }} changelog: ${{ steps.properties.outputs.changelog }} steps: - # Check out current repository - name: Fetch Sources uses: actions/checkout@v3.5.0 - # Validate wrapper - - name: Gradle Wrapper Validation - uses: gradle/wrapper-validation-action@v1.0.6 - # Setup Java 11 environment for the next steps - name: Setup Java uses: actions/setup-java@v3 @@ -66,17 +81,6 @@ jobs: echo "::set-output name=changelog::$CHANGELOG" echo "::set-output name=pluginVerifierHomeDir::~/.pluginVerifier" ./gradlew listProductsReleases # prepare list of IDEs for Plugin Verifier - # Run tests - - name: Run Tests - run: ./gradlew test - - # Collect Tests Result of failed tests - - name: Collect Tests Result - if: ${{ failure() }} - uses: actions/upload-artifact@v3 - with: - name: tests-result - path: ${{ github.workspace }}/build/reports/tests # Run plugin build - name: Run Build @@ -156,4 +160,4 @@ jobs: --notes "$(cat << 'EOM' ${{ needs.build.outputs.changelog }} EOM - )" \ No newline at end of file + )" From 4da78f0f16a5e80796d3bcda64057b69f4912490 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 3 Apr 2023 13:01:54 -0800 Subject: [PATCH 04/21] 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. --- .../models/CoderWorkspacesWizardModel.kt | 2 - .../com/coder/gateway/sdk/CoderCLIManager.kt | 90 +++++++++++++------ .../views/steps/CoderWorkspacesStepView.kt | 46 +++------- src/test/groovy/CoderCLIManagerTest.groovy | 74 +++++++++++++++ 4 files changed, 146 insertions(+), 66 deletions(-) create mode 100644 src/test/groovy/CoderCLIManagerTest.groovy diff --git a/src/main/kotlin/com/coder/gateway/models/CoderWorkspacesWizardModel.kt b/src/main/kotlin/com/coder/gateway/models/CoderWorkspacesWizardModel.kt index cdf9db22..78c63120 100644 --- a/src/main/kotlin/com/coder/gateway/models/CoderWorkspacesWizardModel.kt +++ b/src/main/kotlin/com/coder/gateway/models/CoderWorkspacesWizardModel.kt @@ -3,8 +3,6 @@ package com.coder.gateway.models data class CoderWorkspacesWizardModel( var coderURL: String = "https://coder.example.com", var token: String = "", - var buildVersion: String = "", - var localCliPath: String = "", var selectedWorkspace: WorkspaceAgentModel? = null, var useExistingToken: Boolean = false ) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 335d7f6e..5fe6b6db 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -3,30 +3,43 @@ package com.coder.gateway.sdk import com.intellij.openapi.diagnostic.Logger import java.io.InputStream import java.net.URL -import java.nio.file.FileVisitOption import java.nio.file.Files import java.nio.file.Path -import java.nio.file.Paths -import java.nio.file.StandardCopyOption +import java.nio.file.attribute.PosixFilePermissions -class CoderCLIManager(url: URL, buildVersion: String) { - var remoteCli: URL - var localCli: Path - private var cliNamePrefix: String - private var tmpDir: String - private var cliFileName: String +/** + * Manage the CLI for a single deployment. + */ +class CoderCLIManager(deployment: URL, buildVersion: String) { + private var remoteBinaryUrl: URL + var localBinaryPath: Path + private var binaryNamePrefix: String + private var destinationDir: Path + private var localBinaryName: String init { + // TODO: Should use a persistent path to avoid needing to download on + // each restart. + destinationDir = Path.of(System.getProperty("java.io.tmpdir")) + .resolve("coder-gateway").resolve(deployment.host) val os = getOS() - cliNamePrefix = getCoderCLIForOS(os, getArch()) - val cliNameWithExt = if (os == OS.WINDOWS) "$cliNamePrefix.exe" else cliNamePrefix - cliFileName = if (os == OS.WINDOWS) "${cliNamePrefix}-${buildVersion}.exe" else "${cliNamePrefix}-${buildVersion}" - - remoteCli = URL(url.protocol, url.host, url.port, "/bin/$cliNameWithExt") - tmpDir = System.getProperty("java.io.tmpdir") - localCli = Paths.get(tmpDir, cliFileName) + binaryNamePrefix = getCoderCLIForOS(os, getArch()) + val binaryName = if (os == OS.WINDOWS) "$binaryNamePrefix.exe" else binaryNamePrefix + localBinaryName = + if (os == OS.WINDOWS) "${binaryNamePrefix}-${buildVersion}.exe" else "${binaryNamePrefix}-${buildVersion}" + remoteBinaryUrl = URL( + deployment.protocol, + deployment.host, + deployment.port, + "/bin/$binaryName" + ) + localBinaryPath = destinationDir.resolve(localBinaryName) } + /** + * Return the name of the binary (sans extension) for the provided OS and + * architecture. + */ private fun getCoderCLIForOS(os: OS?, arch: Arch?): String { logger.info("Resolving coder cli for $os $arch") if (os == null) { @@ -55,26 +68,45 @@ class CoderCLIManager(url: URL, buildVersion: String) { } } + /** + * Download the CLI from the deployment if necessary. + */ fun downloadCLI(): Boolean { - if (Files.exists(localCli)) { - logger.info("${localCli.toAbsolutePath()} already exists, skipping download") + Files.createDirectories(destinationDir) + try { + logger.info("Downloading Coder CLI to ${localBinaryPath.toAbsolutePath()}") + remoteBinaryUrl.openStream().use { + Files.copy(it as InputStream, localBinaryPath) + } + } catch (e: java.nio.file.FileAlreadyExistsException) { + // This relies on the provided build version being the latest. It + // must be freshly fetched immediately before downloading. + // TODO: Use etags instead? + logger.info("${localBinaryPath.toAbsolutePath()} already exists, skipping download") return false } - logger.info("Starting Coder CLI download to ${localCli.toAbsolutePath()}") - remoteCli.openStream().use { - Files.copy(it as InputStream, localCli, StandardCopyOption.REPLACE_EXISTING) + if (getOS() != OS.WINDOWS) { + Files.setPosixFilePermissions( + localBinaryPath, + PosixFilePermissions.fromString("rwxr-x---") + ) } return true } + /** + * Remove all versions of the CLI for this deployment that do not match the + * current build version. + */ fun removeOldCli() { - val tmpPath = Path.of(tmpDir) - if (Files.isReadable(tmpPath)) { - Files.walk(tmpPath, 1).use { - it.sorted().map { pt -> pt.toFile() }.filter { fl -> fl.name.contains(cliNamePrefix) && !fl.name.contains(cliFileName) }.forEach { fl -> - logger.info("Removing $fl because it is an old coder cli") - fl.delete() - } + if (Files.isReadable(destinationDir)) { + Files.walk(destinationDir, 1).use { + it.sorted().map { pt -> pt.toFile() } + .filter { fl -> fl.name.contains(binaryNamePrefix) && fl.name != localBinaryName } + .forEach { fl -> + logger.info("Removing $fl because it is an old version") + fl.delete() + } } } } @@ -82,4 +114,4 @@ class CoderCLIManager(url: URL, buildVersion: String) { companion object { val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName) } -} \ No newline at end of file +} 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 6c4730f3..4d43d6ad 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -34,9 +34,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.progress.ProgressManager -import com.intellij.openapi.progress.Task import com.intellij.openapi.rd.util.launchUnderBackgroundProgress import com.intellij.openapi.ui.panel.ComponentPanelBuilder import com.intellij.openapi.wm.impl.welcomeScreen.WelcomeScreenUIManager @@ -472,12 +469,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } val cliManager = CoderCLIManager(localWizardModel.coderURL.toURL(), coderClient.buildVersion) - - localWizardModel.apply { - this.token = token - buildVersion = coderClient.buildVersion - localCliPath = cliManager.localCli.toAbsolutePath().toString() - } + localWizardModel.token = token this.indicator.apply { isIndeterminate = false @@ -492,12 +484,11 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod text = "Downloading Coder CLI..." fraction = 0.3 } - - cliManager.downloadCLI() - if (getOS() != OS.WINDOWS) { - this.indicator.fraction = 0.4 - val chmodOutput = ProcessExecutor().command("chmod", "+x", localWizardModel.localCliPath).readOutput(true).execute().outputUTF8() - logger.info("chmod +x ${cliManager.localCli.toAbsolutePath()} $chmodOutput") + try { + cliManager.downloadCLI() + } catch (e: Exception) { + logger.error("Failed to download Coder CLI", e) + return@launchUnderBackgroundProgress } this.indicator.apply { text = "Configuring Coder CLI..." @@ -575,7 +566,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod * versions do not match. */ private fun authenticate(token: String) { - logger.info("Authenticating...") + logger.info("Authenticating to ${localWizardModel.coderURL}...") coderClient.initClientSession(localWizardModel.coderURL.toURL(), token) logger.info("Checking Coder version...") @@ -593,10 +584,14 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod component.isVisible = true showWarning(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.unsupported.coder.version", coderClient.buildVersion)) } + } else { + logger.info("Got version ${coderClient.buildVersion}...") } } logger.info("Authenticated successfully") + + // Remember these in order to default to them for future attempts. appPropertiesService.setValue(CODER_URL_KEY, localWizardModel.coderURL) appPropertiesService.setValue(SESSION_TOKEN, token) } @@ -714,28 +709,9 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } override fun onNext(wizardModel: CoderWorkspacesWizardModel): Boolean { - if (localWizardModel.localCliPath.isNotBlank()) { - val configSSHTask = object : Task.Modal(null, CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.cli.configssh.dialog.title"), false) { - override fun run(pi: ProgressIndicator) { - pi.apply { - isIndeterminate = false - text = "Configuring coder cli..." - fraction = 0.1 - } - val sshConfigOutput = ProcessExecutor().command(localWizardModel.localCliPath, "config-ssh", "--yes", "--use-previous-options").readOutput(true).execute().outputUTF8() - pi.fraction = 0.8 - logger.info("Result of `${localWizardModel.localCliPath} config-ssh --yes --use-previous-options`: $sshConfigOutput") - pi.fraction = 1.0 - } - } - ProgressManager.getInstance().run(configSSHTask) - } - wizardModel.apply { coderURL = localWizardModel.coderURL token = localWizardModel.token - buildVersion = localWizardModel.buildVersion - localCliPath = localWizardModel.localCliPath } val workspace = tableOfWorkspaces.selectedObject diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy new file mode 100644 index 00000000..b0f07afb --- /dev/null +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -0,0 +1,74 @@ +package com.coder.gateway.sdk + +import org.zeroturnaround.exec.ProcessExecutor +import spock.lang.Unroll + +import java.nio.file.Files +import java.nio.file.Path + +@Unroll +class CoderCLIManagerTest extends spock.lang.Specification { + def "deletes old versions"() { + given: + // Simulate downloading an old version. + def oldVersion = new CoderCLIManager(new URL("https://test.coder.invalid"), "0.0.1").localBinaryPath.toFile() + def dir = oldVersion.toPath().getParent() + dir.toFile().deleteDir() + Files.createDirectories(dir) + oldVersion.write("old-version") + + // Simulate downloading a new version. + def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), "1.0.2") + def newVersion = ccm.localBinaryPath.toFile() + newVersion.write("new-version") + + // Anything that does not start with the expected prefix is ignored. + def otherOsVersion = dir.resolve("coder-alpine-amd64-1.0.2").toFile() + otherOsVersion.write("alpine") + + // Anything else matching the prefix is deleted. + def invalidVersion = dir.resolve(newVersion.getName() + "-extra-random-text").toFile() + invalidVersion.write("invalid") + + when: + ccm.removeOldCli() + + then: + !oldVersion.exists() + newVersion.exists() + otherOsVersion.exists() + !invalidVersion.exists() + } + + // TODO: Probably not good to depend on dev.coder.com being up...should use + // a mock? Or spin up a Coder deployment in CI? + def "downloads a working cli"() { + given: + def ccm = new CoderCLIManager(new URL("https://dev.coder.com"), "1.0.1") + def dir = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway/dev.coder.com") + ccm.localBinaryPath.getParent().toFile().deleteDir() + + when: + def downloaded = ccm.downloadCLI() + + then: + downloaded + ccm.localBinaryPath.getParent() == dir + ccm.localBinaryPath.toFile().exists() + new ProcessExecutor().command(ccm.localBinaryPath.toString(), "version").readOutput(true).execute().outputUTF8().contains("Coder") + } + + def "skips cli download if it already exists"() { + given: + def ccm = new CoderCLIManager(new URL("https://dev.coder.com"), "1.0.1") + Files.createDirectories(ccm.localBinaryPath.getParent()) + ccm.localBinaryPath.toFile().write("cli") + + when: + def downloaded = ccm.downloadCLI() + + then: + !downloaded + ccm.localBinaryPath.toFile().readLines() == ["cli"] + } +} From 848b1d07ff97a642ffbae1f5046b7ca1aa74e092 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 3 Apr 2023 13:02:21 -0800 Subject: [PATCH 05/21] Encapsulate process execution in CLI manager We will need to create a new function for config-ssh anyway since we will be doing our own thing rather than spawning the CLI in order to support multiple deployments so this preemptively encapsulates that logic. --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 40 +++++++++++++++++++ .../views/steps/CoderWorkspacesStepView.kt | 16 ++++---- src/test/groovy/CoderCLIManagerTest.groovy | 3 +- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 5fe6b6db..bf5ff914 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -1,6 +1,7 @@ package com.coder.gateway.sdk import com.intellij.openapi.diagnostic.Logger +import org.zeroturnaround.exec.ProcessExecutor import java.io.InputStream import java.net.URL import java.nio.file.Files @@ -111,6 +112,45 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { } } + /** + * Use the provided credentials to authenticate the CLI. + */ + fun login(url: String, token: String): String { + return exec("login", url, "--token", token) + } + + /** + * Configure SSH to use this binary. + * + * TODO: Support multiple deployments; currently they will clobber each + * other. + */ + fun configSsh(): String { + return exec("config-ssh", "--yes", "--use-previous-options") + } + + /** + * Return the binary version. + */ + fun version(): String { + return exec("version") + } + + /** + * Execute the binary with the provided arguments. + * + * @return The command's output. + */ + private fun exec(vararg args: String): String { + val stdout = ProcessExecutor() + .command(localBinaryPath.toString(), *args) + .readOutput(true) + .execute() + .outputUTF8() + logger.info("`${localBinaryPath} ${listOf(*args).joinToString(" ")}`: $stdout") + return stdout + } + companion object { val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName) } 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 4d43d6ad..8c93982b 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -59,7 +59,6 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import org.zeroturnaround.exec.ProcessExecutor import java.awt.Component import java.awt.Dimension import java.awt.event.MouseEvent @@ -491,18 +490,19 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod return@launchUnderBackgroundProgress } this.indicator.apply { - text = "Configuring Coder CLI..." + text = "Logging in..." fraction = 0.5 } + cliManager.login(localWizardModel.coderURL, localWizardModel.token) - val loginOutput = ProcessExecutor().command(localWizardModel.localCliPath, "login", localWizardModel.coderURL, "--token", localWizardModel.token).readOutput(true).execute().outputUTF8() - logger.info("coder-cli login output: $loginOutput") - this.indicator.fraction = 0.8 - val sshConfigOutput = ProcessExecutor().command(localWizardModel.localCliPath, "config-ssh", "--yes", "--use-previous-options").readOutput(true).execute().outputUTF8() - logger.info("Result of `${localWizardModel.localCliPath} config-ssh --yes --use-previous-options`: $sshConfigOutput") + this.indicator.apply { + text = "Configuring SSH..." + fraction = 0.7 + } + cliManager.configSsh() this.indicator.apply { - text = "Remove old Coder CLI versions..." + text = "Removing old Coder CLI versions..." fraction = 0.9 } cliManager.removeOldCli() diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index b0f07afb..a000cb2a 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -1,6 +1,5 @@ package com.coder.gateway.sdk -import org.zeroturnaround.exec.ProcessExecutor import spock.lang.Unroll import java.nio.file.Files @@ -55,7 +54,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { downloaded ccm.localBinaryPath.getParent() == dir ccm.localBinaryPath.toFile().exists() - new ProcessExecutor().command(ccm.localBinaryPath.toString(), "version").readOutput(true).execute().outputUTF8().contains("Coder") + ccm.version().contains("Coder") } def "skips cli download if it already exists"() { From e065929dc259c740750efed5a770d277496af347 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 3 Apr 2023 13:05:49 -0800 Subject: [PATCH 06/21] Move CLI config read into CLI manager --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 42 +++++++++++++++++ .../views/steps/CoderWorkspacesStepView.kt | 46 +------------------ 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index bf5ff914..a97265b5 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -1,11 +1,13 @@ package com.coder.gateway.sdk +import com.coder.gateway.views.steps.CoderWorkspacesStepView import com.intellij.openapi.diagnostic.Logger import org.zeroturnaround.exec.ProcessExecutor import java.io.InputStream import java.net.URL import java.nio.file.Files import java.nio.file.Path +import java.nio.file.Paths import java.nio.file.attribute.PosixFilePermissions /** @@ -153,5 +155,45 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { companion object { val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName) + + /** + * Return the URL and token from the CLI config. + */ + @JvmStatic + fun readConfig(): Pair { + val configDir = getConfigDir() + CoderWorkspacesStepView.logger.info("Reading config from $configDir") + return try { + val url = Files.readString(configDir.resolve("url")) + val token = Files.readString(configDir.resolve("session")) + url to token + } catch (e: Exception) { + null to null // Probably has not configured the CLI yet. + } + } + + /** + * Return the config directory used by the CLI. + */ + @JvmStatic + fun getConfigDir(): Path { + var dir = System.getenv("CODER_CONFIG_DIR") + if (!dir.isNullOrBlank()) { + return Path.of(dir) + } + // The Coder CLI uses https://github.com/kirsle/configdir so this should + // match how it behaves. + return when (getOS()) { + OS.WINDOWS -> Paths.get(System.getenv("APPDATA"), "coderv2") + OS.MAC -> Paths.get(System.getenv("HOME"), "Library/Application Support/coderv2") + else -> { + dir = System.getenv("XDG_CONFIG_HOME") + if (!dir.isNullOrBlank()) { + return Paths.get(dir, "coderv2") + } + return Paths.get(System.getenv("HOME"), ".config/coderv2") + } + } + } } } 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 8c93982b..8e5832f2 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -19,7 +19,6 @@ import com.coder.gateway.sdk.TemplateIconDownloader import com.coder.gateway.sdk.ex.AuthenticationResponseException import com.coder.gateway.sdk.ex.TemplateResponseException import com.coder.gateway.sdk.ex.WorkspaceResponseException -import com.coder.gateway.sdk.getOS import com.coder.gateway.sdk.toURL import com.coder.gateway.sdk.v2.models.Workspace import com.coder.gateway.sdk.withPath @@ -66,9 +65,6 @@ import java.awt.event.MouseListener import java.awt.event.MouseMotionListener import java.awt.font.TextAttribute import java.awt.font.TextAttribute.UNDERLINE_ON -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.Paths import java.net.SocketTimeoutException import javax.swing.Icon import javax.swing.JCheckBox @@ -364,45 +360,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod if (!url.isNullOrBlank() && !token.isNullOrBlank()) { return url to token } - return readConfig() - } - - /** - * Return the URL and token from the CLI config. - */ - private fun readConfig(): Pair { - val configDir = getConfigDir() - logger.info("Reading config from $configDir") - try { - val url = Files.readString(configDir.resolve("url")) - val token = Files.readString(configDir.resolve("session")) - return url to token - } catch (e: Exception) { - return null to null // Probably has not configured the CLI yet. - } - } - - /** - * Return the config directory used by the CLI. - */ - private fun getConfigDir(): Path { - var dir = System.getenv("CODER_CONFIG_DIR") - if (!dir.isNullOrBlank()) { - return Path.of(dir) - } - // The Coder CLI uses https://github.com/kirsle/configdir so this should - // match how it behaves. - return when(getOS()) { - OS.WINDOWS -> Paths.get(System.getenv("APPDATA"), "coderv2") - OS.MAC -> Paths.get(System.getenv("HOME"), "Library/Application Support/coderv2") - else -> { - dir = System.getenv("XDG_CONFIG_HOME") - if (!dir.isNullOrBlank()) { - return Paths.get(dir, "coderv2") - } - return Paths.get(System.getenv("HOME"), ".config/coderv2") - } - } + return CoderCLIManager.readConfig() } private fun updateWorkspaceActions() { @@ -518,7 +476,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod if (openBrowser && !localWizardModel.useExistingToken) { BrowserUtil.browse(getTokenUrl) } else if (localWizardModel.useExistingToken) { - val (url, token) = readConfig() + val (url, token) = CoderCLIManager.readConfig() if (url == localWizardModel.coderURL && !token.isNullOrBlank()) { logger.info("Injecting valid token from CLI config") localWizardModel.token = token From cda750e1d9917cfac154722cde8324ddcdaa2d48 Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 31 Mar 2023 14:25:24 -0800 Subject: [PATCH 07/21] Add tests for config dir --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 26 ++++++--- src/test/groovy/CoderCLIManagerTest.groovy | 57 +++++++++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index a97265b5..5af3bc51 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -160,8 +160,8 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { * Return the URL and token from the CLI config. */ @JvmStatic - fun readConfig(): Pair { - val configDir = getConfigDir() + fun readConfig(env: Environment = Environment()): Pair { + val configDir = getConfigDir(env) CoderWorkspacesStepView.logger.info("Reading config from $configDir") return try { val url = Files.readString(configDir.resolve("url")) @@ -176,24 +176,34 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { * Return the config directory used by the CLI. */ @JvmStatic - fun getConfigDir(): Path { - var dir = System.getenv("CODER_CONFIG_DIR") + fun getConfigDir(env: Environment = Environment()): Path { + var dir = env.get("CODER_CONFIG_DIR") if (!dir.isNullOrBlank()) { return Path.of(dir) } // The Coder CLI uses https://github.com/kirsle/configdir so this should // match how it behaves. return when (getOS()) { - OS.WINDOWS -> Paths.get(System.getenv("APPDATA"), "coderv2") - OS.MAC -> Paths.get(System.getenv("HOME"), "Library/Application Support/coderv2") + OS.WINDOWS -> Paths.get(env.get("APPDATA"), "coderv2") + OS.MAC -> Paths.get(env.get("HOME"), "Library/Application Support/coderv2") else -> { - dir = System.getenv("XDG_CONFIG_HOME") + dir = env.get("XDG_CONFIG_HOME") if (!dir.isNullOrBlank()) { return Paths.get(dir, "coderv2") } - return Paths.get(System.getenv("HOME"), ".config/coderv2") + return Paths.get(env.get("HOME"), ".config/coderv2") } } } } } + +class Environment(private val env: Map = emptyMap()) { + fun get(name: String): String? { + val e = env[name] + if (e != null) { + return e + } + return System.getenv(name) + } +} diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index a000cb2a..341b6bd7 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -1,5 +1,6 @@ package com.coder.gateway.sdk +import spock.lang.Requires import spock.lang.Unroll import java.nio.file.Files @@ -70,4 +71,60 @@ class CoderCLIManagerTest extends spock.lang.Specification { !downloaded ccm.localBinaryPath.toFile().readLines() == ["cli"] } + + /** + * Get a config dir using default environment variable values. + */ + def configDir(Map env = [:]) { + return CoderCLIManager.getConfigDir(new Environment([ + "APPDATA" : "/tmp/coder-gateway-test/appdata", + "HOME" : "/tmp/coder-gateway-test/home", + "XDG_CONFIG_HOME" : "/tmp/coder-gateway-test/xdg", + "CODER_CONFIG_DIR": "", + ] + env)) + } + + // Mostly just a sanity check to make sure the default System.getenv runs + // without throwing any errors. + def "gets config dir"() { + when: + def dir = CoderCLIManager.getConfigDir(new Environment([ + "CODER_CONFIG_DIR": "", + ])) + + then: + dir.toString().contains("coderv2") + } + + def "gets config dir from CODER_CONFIG_DIR"() { + expect: + Path.of(path) == configDir(env) + + where: + env || path + ["CODER_CONFIG_DIR": "/tmp/coder-gateway-test/conf"] || "/tmp/coder-gateway-test/conf" + } + + @Requires({ os.linux }) + def "gets config dir from XDG or HOME"() { + expect: + Path.of(path) == configDir(env) + + where: + env || path + [:] || "/tmp/coder-gateway-test/xdg/coderv2" + ["XDG_CONFIG_HOME": ""] || "/tmp/coder-gateway-test/home/.config/coderv2" + } + + @Requires({ os.macOs }) + def "gets config dir from HOME"() { + expect: + Path.of("/tmp/coder-gateway-test/home/Library/Application Support/coderv2") == configDir() + } + + @Requires({ os.windows }) + def "gets config dir from APPDATA"() { + expect: + Path.of("/tmp/coder-gateway-test/appdata/coderv2") == configDir() + } } From 125b1f22357c1bd2b55da701adb3f22b2bec6cb1 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 3 Apr 2023 08:33:01 -0800 Subject: [PATCH 08/21] Redact token from logger output --- src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 5af3bc51..d2940d97 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -149,13 +149,16 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { .readOutput(true) .execute() .outputUTF8() - logger.info("`${localBinaryPath} ${listOf(*args).joinToString(" ")}`: $stdout") + val redactedArgs = listOf(*args).joinToString(" ").replace(tokenRegex, "--token ") + logger.info("`$localBinaryPath $redactedArgs`: $stdout") return stdout } companion object { val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName) + private val tokenRegex = "--token [^ ]+".toRegex() + /** * Return the URL and token from the CLI config. */ From ae7b8dfee821966b4346995bba2f81aa73b93254 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 5 Apr 2023 11:06:49 -0800 Subject: [PATCH 09/21] Use ETag for downloading binary This way we will not need to name the binaries with their versions or otherwise extract the version from each binary since we can just grab their hash and set that in the request. This also means we will not need to remove old versions as there will only be the one file. --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 94 ++++++++++++------- .../views/steps/CoderWorkspacesStepView.kt | 8 +- src/test/groovy/CoderCLIManagerTest.groovy | 65 ++++++------- 3 files changed, 87 insertions(+), 80 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index d2940d97..2f715958 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -3,22 +3,29 @@ package com.coder.gateway.sdk import com.coder.gateway.views.steps.CoderWorkspacesStepView import com.intellij.openapi.diagnostic.Logger import org.zeroturnaround.exec.ProcessExecutor -import java.io.InputStream +import java.io.BufferedInputStream +import java.io.FileInputStream +import java.io.FileNotFoundException +import java.net.HttpURLConnection import java.net.URL import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths +import java.nio.file.StandardCopyOption import java.nio.file.attribute.PosixFilePermissions +import java.security.DigestInputStream +import java.security.MessageDigest +import javax.xml.bind.annotation.adapters.HexBinaryAdapter + /** * Manage the CLI for a single deployment. */ -class CoderCLIManager(deployment: URL, buildVersion: String) { +class CoderCLIManager(deployment: URL) { private var remoteBinaryUrl: URL var localBinaryPath: Path private var binaryNamePrefix: String private var destinationDir: Path - private var localBinaryName: String init { // TODO: Should use a persistent path to avoid needing to download on @@ -28,15 +35,13 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { val os = getOS() binaryNamePrefix = getCoderCLIForOS(os, getArch()) val binaryName = if (os == OS.WINDOWS) "$binaryNamePrefix.exe" else binaryNamePrefix - localBinaryName = - if (os == OS.WINDOWS) "${binaryNamePrefix}-${buildVersion}.exe" else "${binaryNamePrefix}-${buildVersion}" remoteBinaryUrl = URL( deployment.protocol, deployment.host, deployment.port, "/bin/$binaryName" ) - localBinaryPath = destinationDir.resolve(localBinaryName) + localBinaryPath = destinationDir.resolve(binaryName) } /** @@ -44,7 +49,7 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { * architecture. */ private fun getCoderCLIForOS(os: OS?, arch: Arch?): String { - logger.info("Resolving coder cli for $os $arch") + logger.info("Resolving binary for $os $arch") if (os == null) { logger.error("Could not resolve client OS and architecture, defaulting to WINDOWS AMD64") return "coder-windows-amd64" @@ -75,42 +80,59 @@ class CoderCLIManager(deployment: URL, buildVersion: String) { * Download the CLI from the deployment if necessary. */ fun downloadCLI(): Boolean { - Files.createDirectories(destinationDir) - try { - logger.info("Downloading Coder CLI to ${localBinaryPath.toAbsolutePath()}") - remoteBinaryUrl.openStream().use { - Files.copy(it as InputStream, localBinaryPath) - } - } catch (e: java.nio.file.FileAlreadyExistsException) { - // This relies on the provided build version being the latest. It - // must be freshly fetched immediately before downloading. - // TODO: Use etags instead? - logger.info("${localBinaryPath.toAbsolutePath()} already exists, skipping download") - return false + val etag = getBinaryETag() + val conn = remoteBinaryUrl.openConnection() as HttpURLConnection + if (etag != null) { + logger.info("Found existing binary at ${localBinaryPath.toAbsolutePath()}; calculated hash as $etag") + conn.setRequestProperty("If-None-Match", "\"$etag\"") } - if (getOS() != OS.WINDOWS) { - Files.setPosixFilePermissions( - localBinaryPath, - PosixFilePermissions.fromString("rwxr-x---") - ) + conn.connect() + logger.info("GET ${conn.responseCode} $remoteBinaryUrl") + when (conn.responseCode) { + 200 -> { + logger.info("Downloading binary to ${localBinaryPath.toAbsolutePath()}") + Files.createDirectories(destinationDir) + conn.inputStream.use { + Files.copy(it, localBinaryPath, StandardCopyOption.REPLACE_EXISTING) + } + if (getOS() != OS.WINDOWS) { + Files.setPosixFilePermissions( + localBinaryPath, + PosixFilePermissions.fromString("rwxr-x---") + ) + } + conn.disconnect() + return true + } + + 304 -> { + logger.info("Using cached binary at ${localBinaryPath.toAbsolutePath()}") + conn.disconnect() + return false + } } - return true + conn.disconnect() + throw Exception("Unable to download $remoteBinaryUrl (got response code `${conn.responseCode}`)") } /** - * Remove all versions of the CLI for this deployment that do not match the - * current build version. + * Return the entity tag for the binary on disk, if any. */ - fun removeOldCli() { - if (Files.isReadable(destinationDir)) { - Files.walk(destinationDir, 1).use { - it.sorted().map { pt -> pt.toFile() } - .filter { fl -> fl.name.contains(binaryNamePrefix) && fl.name != localBinaryName } - .forEach { fl -> - logger.info("Removing $fl because it is an old version") - fl.delete() - } + private fun getBinaryETag(): String? { + try { + val md = MessageDigest.getInstance("SHA-1") + val fis = FileInputStream(localBinaryPath.toFile()) + val dis = DigestInputStream(BufferedInputStream(fis), md) + fis.use { + while (dis.read() != -1) { + } } + return HexBinaryAdapter().marshal(md.digest()).lowercase() + } catch (e: FileNotFoundException) { + return null + } catch (e: Exception) { + logger.warn("Unable to calculate hash for ${localBinaryPath.toAbsolutePath()}", e) + return null } } 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 8e5832f2..f2265b67 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -425,7 +425,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod return@launchUnderBackgroundProgress } - val cliManager = CoderCLIManager(localWizardModel.coderURL.toURL(), coderClient.buildVersion) + val cliManager = CoderCLIManager(localWizardModel.coderURL.toURL()) localWizardModel.token = token this.indicator.apply { @@ -459,12 +459,6 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } cliManager.configSsh() - this.indicator.apply { - text = "Removing old Coder CLI versions..." - fraction = 0.9 - } - cliManager.removeOldCli() - this.indicator.fraction = 1.0 updateWorkspaceActions() triggerWorkspacePolling(false) diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index 341b6bd7..f5003cb9 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -8,68 +8,59 @@ import java.nio.file.Path @Unroll class CoderCLIManagerTest extends spock.lang.Specification { - def "deletes old versions"() { + // TODO: Probably not good to depend on dev.coder.com being up...should use + // a mock? Or spin up a Coder deployment in CI? + def "downloads a working cli"() { given: - // Simulate downloading an old version. - def oldVersion = new CoderCLIManager(new URL("https://test.coder.invalid"), "0.0.1").localBinaryPath.toFile() - def dir = oldVersion.toPath().getParent() - dir.toFile().deleteDir() - Files.createDirectories(dir) - oldVersion.write("old-version") - - // Simulate downloading a new version. - def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), "1.0.2") - def newVersion = ccm.localBinaryPath.toFile() - newVersion.write("new-version") - - // Anything that does not start with the expected prefix is ignored. - def otherOsVersion = dir.resolve("coder-alpine-amd64-1.0.2").toFile() - otherOsVersion.write("alpine") - - // Anything else matching the prefix is deleted. - def invalidVersion = dir.resolve(newVersion.getName() + "-extra-random-text").toFile() - invalidVersion.write("invalid") + def ccm = new CoderCLIManager(new URL("https://dev.coder.com")) + ccm.localBinaryPath.getParent().toFile().deleteDir() when: - ccm.removeOldCli() + def downloaded = ccm.downloadCLI() then: - !oldVersion.exists() - newVersion.exists() - otherOsVersion.exists() - !invalidVersion.exists() + downloaded + ccm.version().contains("Coder") } - // TODO: Probably not good to depend on dev.coder.com being up...should use - // a mock? Or spin up a Coder deployment in CI? - def "downloads a working cli"() { + def "overwrites cli if incorrect version"() { given: - def ccm = new CoderCLIManager(new URL("https://dev.coder.com"), "1.0.1") - def dir = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway/dev.coder.com") - ccm.localBinaryPath.getParent().toFile().deleteDir() + def ccm = new CoderCLIManager(new URL("https://dev.coder.com")) + Files.createDirectories(ccm.localBinaryPath.getParent()) + ccm.localBinaryPath.toFile().write("cli") when: def downloaded = ccm.downloadCLI() then: downloaded - ccm.localBinaryPath.getParent() == dir - ccm.localBinaryPath.toFile().exists() ccm.version().contains("Coder") } def "skips cli download if it already exists"() { given: - def ccm = new CoderCLIManager(new URL("https://dev.coder.com"), "1.0.1") - Files.createDirectories(ccm.localBinaryPath.getParent()) - ccm.localBinaryPath.toFile().write("cli") + def ccm = new CoderCLIManager(new URL("https://dev.coder.com")) when: + ccm.downloadCLI() def downloaded = ccm.downloadCLI() then: !downloaded - ccm.localBinaryPath.toFile().readLines() == ["cli"] + ccm.version().contains("Coder") + } + + def "does not clobber other deployments"() { + given: + def ccm1 = new CoderCLIManager(new URL("https://oss.demo.coder.com")) + def ccm2 = new CoderCLIManager(new URL("https://dev.coder.com")) + + when: + ccm1.downloadCLI() + ccm2.downloadCLI() + + then: + ccm1.localBinaryPath != ccm2.localBinaryPath } /** From 1ac681ca6c4795d0683e1d4dab648c52322b923c Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 5 Apr 2023 11:09:51 -0800 Subject: [PATCH 10/21] Accept gzip when downloading binary --- src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 2f715958..c89eb95a 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -15,6 +15,7 @@ import java.nio.file.StandardCopyOption import java.nio.file.attribute.PosixFilePermissions import java.security.DigestInputStream import java.security.MessageDigest +import java.util.zip.GZIPInputStream import javax.xml.bind.annotation.adapters.HexBinaryAdapter @@ -86,6 +87,7 @@ class CoderCLIManager(deployment: URL) { logger.info("Found existing binary at ${localBinaryPath.toAbsolutePath()}; calculated hash as $etag") conn.setRequestProperty("If-None-Match", "\"$etag\"") } + conn.setRequestProperty("Accept-Encoding", "gzip") conn.connect() logger.info("GET ${conn.responseCode} $remoteBinaryUrl") when (conn.responseCode) { @@ -93,7 +95,11 @@ class CoderCLIManager(deployment: URL) { logger.info("Downloading binary to ${localBinaryPath.toAbsolutePath()}") Files.createDirectories(destinationDir) conn.inputStream.use { - Files.copy(it, localBinaryPath, StandardCopyOption.REPLACE_EXISTING) + Files.copy( + if (conn.contentEncoding == "gzip") GZIPInputStream(it) else it, + localBinaryPath, + StandardCopyOption.REPLACE_EXISTING, + ) } if (getOS() != OS.WINDOWS) { Files.setPosixFilePermissions( From 6636f31cfb0a0d853548f63f73f338d288bd0ef7 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 5 Apr 2023 12:16:31 -0800 Subject: [PATCH 11/21] Store binary in data directory The @JvmOverloads annotations make Kotlin generate overloads otherwise the test code thinks the optional arguments are not optional. --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 47 +++++--- src/test/groovy/CoderCLIManagerTest.groovy | 104 +++++++++++++++--- 2 files changed, 115 insertions(+), 36 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index c89eb95a..1371e8d7 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -22,44 +22,35 @@ import javax.xml.bind.annotation.adapters.HexBinaryAdapter /** * Manage the CLI for a single deployment. */ -class CoderCLIManager(deployment: URL) { +class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: Path = getDataDir()) { private var remoteBinaryUrl: URL var localBinaryPath: Path - private var binaryNamePrefix: String - private var destinationDir: Path init { - // TODO: Should use a persistent path to avoid needing to download on - // each restart. - destinationDir = Path.of(System.getProperty("java.io.tmpdir")) - .resolve("coder-gateway").resolve(deployment.host) - val os = getOS() - binaryNamePrefix = getCoderCLIForOS(os, getArch()) - val binaryName = if (os == OS.WINDOWS) "$binaryNamePrefix.exe" else binaryNamePrefix + val binaryName = getCoderCLIForOS(getOS(), getArch()) remoteBinaryUrl = URL( deployment.protocol, deployment.host, deployment.port, "/bin/$binaryName" ) - localBinaryPath = destinationDir.resolve(binaryName) + localBinaryPath = destinationDir.resolve(deployment.host).resolve(binaryName) } - /** - * Return the name of the binary (sans extension) for the provided OS and + * Return the name of the binary (with extension) for the provided OS and * architecture. */ private fun getCoderCLIForOS(os: OS?, arch: Arch?): String { logger.info("Resolving binary for $os $arch") if (os == null) { logger.error("Could not resolve client OS and architecture, defaulting to WINDOWS AMD64") - return "coder-windows-amd64" + return "coder-windows-amd64.exe" } return when (os) { OS.WINDOWS -> when (arch) { - Arch.AMD64 -> "coder-windows-amd64" - Arch.ARM64 -> "coder-windows-arm64" - else -> "coder-windows-amd64" + Arch.AMD64 -> "coder-windows-amd64.exe" + Arch.ARM64 -> "coder-windows-arm64.exe" + else -> "coder-windows-amd64.exe" } OS.LINUX -> when (arch) { @@ -93,7 +84,7 @@ class CoderCLIManager(deployment: URL) { when (conn.responseCode) { 200 -> { logger.info("Downloading binary to ${localBinaryPath.toAbsolutePath()}") - Files.createDirectories(destinationDir) + Files.createDirectories(localBinaryPath.parent) conn.inputStream.use { Files.copy( if (conn.contentEncoding == "gzip") GZIPInputStream(it) else it, @@ -207,6 +198,7 @@ class CoderCLIManager(deployment: URL) { * Return the config directory used by the CLI. */ @JvmStatic + @JvmOverloads fun getConfigDir(env: Environment = Environment()): Path { var dir = env.get("CODER_CONFIG_DIR") if (!dir.isNullOrBlank()) { @@ -226,6 +218,25 @@ class CoderCLIManager(deployment: URL) { } } } + + /** + * Return the data directory. + */ + @JvmStatic + @JvmOverloads + fun getDataDir(env: Environment = Environment()): Path { + return when (getOS()) { + OS.WINDOWS -> Paths.get(env.get("LOCALAPPDATA"), "coder-gateway") + OS.MAC -> Paths.get(env.get("HOME"), "Library/Application Support/coder-gateway") + else -> { + val dir = env.get("XDG_DATA_HOME") + if (!dir.isNullOrBlank()) { + return Paths.get(dir, "coder-gateway") + } + return Paths.get(env.get("HOME"), ".local/share/coder-gateway") + } + } + } } } diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index f5003cb9..65297aed 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -8,11 +8,38 @@ import java.nio.file.Path @Unroll class CoderCLIManagerTest extends spock.lang.Specification { - // TODO: Probably not good to depend on dev.coder.com being up...should use - // a mock? Or spin up a Coder deployment in CI? + /** + * Create a CLI manager pointing to the URL in the environment or to the + default URLs. + * + * @TODO: Implement a mock. + */ + def createCLIManager(Boolean alternate = false) { + var url = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT") + if (url == null) { + url = "https://dev.coder.com" + } + if (alternate) { + url = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT_ALT") + if (url == null) { + url = "https://oss.demo.coder.com" + } + } + var tmpdir = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway-test") + return new CoderCLIManager(new URL(url), tmpdir) + } + + def "defaults to a sub-directory in the data directory"() { + given: + def ccm = new CoderCLIManager(new URL("https://test.coder.invalid")) + + expect: + ccm.localBinaryPath.getParent() == CoderCLIManager.getDataDir().resolve("test.coder.invalid") + } + def "downloads a working cli"() { given: - def ccm = new CoderCLIManager(new URL("https://dev.coder.com")) + def ccm = createCLIManager() ccm.localBinaryPath.getParent().toFile().deleteDir() when: @@ -25,7 +52,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { def "overwrites cli if incorrect version"() { given: - def ccm = new CoderCLIManager(new URL("https://dev.coder.com")) + def ccm = createCLIManager() Files.createDirectories(ccm.localBinaryPath.getParent()) ccm.localBinaryPath.toFile().write("cli") @@ -39,7 +66,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { def "skips cli download if it already exists"() { given: - def ccm = new CoderCLIManager(new URL("https://dev.coder.com")) + def ccm = createCLIManager() when: ccm.downloadCLI() @@ -52,8 +79,8 @@ class CoderCLIManagerTest extends spock.lang.Specification { def "does not clobber other deployments"() { given: - def ccm1 = new CoderCLIManager(new URL("https://oss.demo.coder.com")) - def ccm2 = new CoderCLIManager(new URL("https://dev.coder.com")) + def ccm1 = createCLIManager(true) + def ccm2 = createCLIManager() when: ccm1.downloadCLI() @@ -63,25 +90,27 @@ class CoderCLIManagerTest extends spock.lang.Specification { ccm1.localBinaryPath != ccm2.localBinaryPath } + def testEnv = [ + "APPDATA" : "/tmp/coder-gateway-test/appdata", + "LOCALAPPDATA" : "/tmp/coder-gateway-test/localappdata", + "HOME" : "/tmp/coder-gateway-test/home", + "XDG_CONFIG_HOME" : "/tmp/coder-gateway-test/xdg-config", + "XDG_DATA_HOME" : "/tmp/coder-gateway-test/xdg-data", + "CODER_CONFIG_DIR": "", + ] + /** * Get a config dir using default environment variable values. */ def configDir(Map env = [:]) { - return CoderCLIManager.getConfigDir(new Environment([ - "APPDATA" : "/tmp/coder-gateway-test/appdata", - "HOME" : "/tmp/coder-gateway-test/home", - "XDG_CONFIG_HOME" : "/tmp/coder-gateway-test/xdg", - "CODER_CONFIG_DIR": "", - ] + env)) + return CoderCLIManager.getConfigDir(new Environment(testEnv + env)) } // Mostly just a sanity check to make sure the default System.getenv runs // without throwing any errors. def "gets config dir"() { when: - def dir = CoderCLIManager.getConfigDir(new Environment([ - "CODER_CONFIG_DIR": "", - ])) + def dir = CoderCLIManager.getConfigDir() then: dir.toString().contains("coderv2") @@ -97,13 +126,13 @@ class CoderCLIManagerTest extends spock.lang.Specification { } @Requires({ os.linux }) - def "gets config dir from XDG or HOME"() { + def "gets config dir from XDG_CONFIG_HOME or HOME"() { expect: Path.of(path) == configDir(env) where: env || path - [:] || "/tmp/coder-gateway-test/xdg/coderv2" + [:] || "/tmp/coder-gateway-test/xdg-config/coderv2" ["XDG_CONFIG_HOME": ""] || "/tmp/coder-gateway-test/home/.config/coderv2" } @@ -118,4 +147,43 @@ class CoderCLIManagerTest extends spock.lang.Specification { expect: Path.of("/tmp/coder-gateway-test/appdata/coderv2") == configDir() } + + /** + * Get a data dir using default environment variable values. + */ + def dataDir(Map env = [:]) { + return CoderCLIManager.getDataDir(new Environment(testEnv + env)) + } + // Mostly just a sanity check to make sure the default System.getenv runs + // without throwing any errors. + def "gets data dir"() { + when: + def dir = CoderCLIManager.getDataDir() + + then: + dir.toString().contains("coder-gateway") + } + + @Requires({ os.linux }) + def "gets data dir from XDG_DATA_HOME or HOME"() { + expect: + Path.of(path) == dataDir(env) + + where: + env || path + [:] || "/tmp/coder-gateway-test/xdg-data/coder-gateway" + ["XDG_DATA_HOME": ""] || "/tmp/coder-gateway-test/home/.local/share/coder-gateway" + } + + @Requires({ os.macOs }) + def "gets data dir from HOME"() { + expect: + Path.of("/tmp/coder-gateway-test/home/Library/Application Support/coder-gateway") == dataDir() + } + + @Requires({ os.windows }) + def "gets data dir from LOCALAPPDATA"() { + expect: + Path.of("/tmp/coder-gateway-test/localappdata/coder-gateway") == dataDir() + } } From 5bb8182cd6b07ea731901317d3546358b20435a9 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 5 Apr 2023 13:12:00 -0800 Subject: [PATCH 12/21] Include port in sub-directory if it is set --- src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt | 3 ++- src/test/groovy/CoderCLIManagerTest.groovy | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 1371e8d7..c745e61f 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -34,7 +34,8 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: deployment.port, "/bin/$binaryName" ) - localBinaryPath = destinationDir.resolve(deployment.host).resolve(binaryName) + val subdir = if (deployment.port > 0) "${deployment.host}-${deployment.port}" else deployment.host + localBinaryPath = destinationDir.resolve(subdir).resolve(binaryName) } /** * Return the name of the binary (with extension) for the provided OS and diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index 65297aed..52a21178 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -37,6 +37,14 @@ class CoderCLIManagerTest extends spock.lang.Specification { ccm.localBinaryPath.getParent() == CoderCLIManager.getDataDir().resolve("test.coder.invalid") } + def "includes port in sub-directory if included"() { + given: + def ccm = new CoderCLIManager(new URL("https://test.coder.invalid:3000")) + + expect: + ccm.localBinaryPath.getParent() == CoderCLIManager.getDataDir().resolve("test.coder.invalid-3000") + } + def "downloads a working cli"() { given: def ccm = createCLIManager() From be064e6822760fbfad51ccc7257c50149f2db13e Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 5 Apr 2023 13:36:01 -0800 Subject: [PATCH 13/21] Test against a mocked server Environment variables can be used to test against a real deployment. I now just check if the file is executable rather than trying to actually execute it since we would need to actually compile something that can run as a .exe on Windows. --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 11 +-- src/test/groovy/CoderCLIManagerTest.groovy | 95 ++++++++++++++----- 2 files changed, 74 insertions(+), 32 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index c745e61f..bb24b269 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -83,7 +83,7 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: conn.connect() logger.info("GET ${conn.responseCode} $remoteBinaryUrl") when (conn.responseCode) { - 200 -> { + HttpURLConnection.HTTP_OK -> { logger.info("Downloading binary to ${localBinaryPath.toAbsolutePath()}") Files.createDirectories(localBinaryPath.parent) conn.inputStream.use { @@ -103,7 +103,7 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: return true } - 304 -> { + HttpURLConnection.HTTP_NOT_MODIFIED -> { logger.info("Using cached binary at ${localBinaryPath.toAbsolutePath()}") conn.disconnect() return false @@ -151,13 +151,6 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: return exec("config-ssh", "--yes", "--use-previous-options") } - /** - * Return the binary version. - */ - fun version(): String { - return exec("version") - } - /** * Execute the binary with the provided arguments. * diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index 52a21178..675163e8 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -1,6 +1,10 @@ package com.coder.gateway.sdk +import com.sun.net.httpserver.HttpExchange +import com.sun.net.httpserver.HttpHandler +import com.sun.net.httpserver.HttpServer import spock.lang.Requires +import spock.lang.Shared import spock.lang.Unroll import java.nio.file.Files @@ -8,25 +12,70 @@ import java.nio.file.Path @Unroll class CoderCLIManagerTest extends spock.lang.Specification { - /** - * Create a CLI manager pointing to the URL in the environment or to the - default URLs. - * - * @TODO: Implement a mock. - */ - def createCLIManager(Boolean alternate = false) { - var url = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT") - if (url == null) { - url = "https://dev.coder.com" - } - if (alternate) { - url = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT_ALT") - if (url == null) { - url = "https://oss.demo.coder.com" + @Shared + private Path tmpdir = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway-test") + @Shared + private String deploymentURL = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT") + @Shared + private String altDeploymentURL = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT_ALT") + @Shared + private def servers = [] + + String startMockServer() { + HttpServer srv = HttpServer.create(new InetSocketAddress(0), 0) + srv.createContext("/", new HttpHandler() { + void handle(HttpExchange exchange) { + int code = HttpURLConnection.HTTP_OK + String response = "not a real binary" + + String[] etags = exchange.requestHeaders.get("If-None-Match") + if (etags != null && etags.contains("\"cb25cf6f41bb3127d7e05b0c3c6403be9ab052bc\"")) { + code = HttpURLConnection.HTTP_NOT_MODIFIED + response = "not modified" + } + + if (!exchange.requestURI.path.startsWith("/bin/coder-")) { + code = HttpURLConnection.HTTP_NOT_FOUND + response = "not found" + } + + byte[] body = response.getBytes() + exchange.sendResponseHeaders(code, code == HttpURLConnection.HTTP_OK ? body.length : -1) + exchange.responseBody.write(body) + exchange.close() } + }) + servers << srv + srv.start() + return "http://localhost:" + srv.address.port + } + + void setupSpec() { + // Clean up from previous runs otherwise they get cluttered since the + // port is random. + tmpdir.toFile().deleteDir() + + // Spin up mock server(s). + if (deploymentURL == null) { + deploymentURL = startMockServer() + } + if (altDeploymentURL == null) { + altDeploymentURL = startMockServer() } - var tmpdir = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway-test") - return new CoderCLIManager(new URL(url), tmpdir) + } + + void cleanupSpec() { + servers.each { + it.stop(0) + } + } + + /** + * Create a CLI manager pointing to the URLs in the environment or to mocked + * servers. + */ + CoderCLIManager createCLIManager(Boolean alternate = false) { + return new CoderCLIManager(new URL(alternate ? altDeploymentURL : deploymentURL), tmpdir) } def "defaults to a sub-directory in the data directory"() { @@ -55,7 +104,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { then: downloaded - ccm.version().contains("Coder") + ccm.localBinaryPath.toFile().canExecute() } def "overwrites cli if incorrect version"() { @@ -69,7 +118,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { then: downloaded - ccm.version().contains("Coder") + ccm.localBinaryPath.toFile().canExecute() } def "skips cli download if it already exists"() { @@ -82,7 +131,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { then: !downloaded - ccm.version().contains("Coder") + ccm.localBinaryPath.toFile().canExecute() } def "does not clobber other deployments"() { @@ -98,7 +147,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { ccm1.localBinaryPath != ccm2.localBinaryPath } - def testEnv = [ + Map testEnv = [ "APPDATA" : "/tmp/coder-gateway-test/appdata", "LOCALAPPDATA" : "/tmp/coder-gateway-test/localappdata", "HOME" : "/tmp/coder-gateway-test/home", @@ -110,7 +159,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { /** * Get a config dir using default environment variable values. */ - def configDir(Map env = [:]) { + Path configDir(Map env = [:]) { return CoderCLIManager.getConfigDir(new Environment(testEnv + env)) } @@ -159,7 +208,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { /** * Get a data dir using default environment variable values. */ - def dataDir(Map env = [:]) { + Path dataDir(Map env = [:]) { return CoderCLIManager.getDataDir(new Environment(testEnv + env)) } // Mostly just a sanity check to make sure the default System.getenv runs From 9b0d21eceb6efcc94bce74fccdafc9b221bfb26d Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 5 Apr 2023 14:05:16 -0800 Subject: [PATCH 14/21] Split out version check for testing --- .../com/coder/gateway/sdk/CoderSemVer.kt | 25 ++++++++++- .../views/steps/CoderWorkspacesStepView.kt | 27 ++++++----- src/test/groovy/CoderSemVerTest.groovy | 45 +++++++++++++++++++ 3 files changed, 82 insertions(+), 15 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderSemVer.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderSemVer.kt index 1df22547..2d970f50 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderSemVer.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderSemVer.kt @@ -1,5 +1,6 @@ package com.coder.gateway.sdk +import com.coder.gateway.CoderSupportedVersions class CoderSemVer(private val major: Long = 0, private val minor: Long = 0, private val patch: Long = 0) : Comparable { @@ -62,5 +63,27 @@ class CoderSemVer(private val major: Long = 0, private val minor: Long = 0, priv if (matchResult.groupValues[3].isNotEmpty()) matchResult.groupValues[3].toLong() else 0, ) } + + /** + * Check to see if the plugin is compatible with the provided version. + * Throws if not valid. + */ + @JvmStatic + fun checkVersionCompatibility(buildVersion: String) { + if (!isValidVersion(buildVersion)) { + throw InvalidVersionException("Invalid version $buildVersion") + } + + if (!parse(buildVersion).isInClosedRange( + CoderSupportedVersions.minCompatibleCoderVersion, + CoderSupportedVersions.maxCompatibleCoderVersion + ) + ) { + throw IncompatibleVersionException("Incompatible version $buildVersion") + } + } } -} \ No newline at end of file +} + +class InvalidVersionException(message: String) : Exception(message) +class IncompatibleVersionException(message: String) : Exception(message) 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 f2265b67..2663cfcc 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -1,7 +1,6 @@ package com.coder.gateway.views.steps import com.coder.gateway.CoderGatewayBundle -import com.coder.gateway.CoderSupportedVersions import com.coder.gateway.icons.CoderIcons import com.coder.gateway.models.CoderWorkspacesWizardModel import com.coder.gateway.models.WorkspaceAgentModel @@ -14,6 +13,8 @@ import com.coder.gateway.sdk.Arch import com.coder.gateway.sdk.CoderCLIManager import com.coder.gateway.sdk.CoderRestClientService import com.coder.gateway.sdk.CoderSemVer +import com.coder.gateway.sdk.IncompatibleVersionException +import com.coder.gateway.sdk.InvalidVersionException import com.coder.gateway.sdk.OS import com.coder.gateway.sdk.TemplateIconDownloader import com.coder.gateway.sdk.ex.AuthenticationResponseException @@ -521,23 +522,21 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod logger.info("Authenticating to ${localWizardModel.coderURL}...") coderClient.initClientSession(localWizardModel.coderURL.toURL(), token) - logger.info("Checking Coder version...") - if (!CoderSemVer.isValidVersion(coderClient.buildVersion)) { - logger.warn("Got invalid version ${coderClient.buildVersion}") + try { + logger.info("Checking compatibility with Coder version ${coderClient.buildVersion}...") + CoderSemVer.checkVersionCompatibility(coderClient.buildVersion) + logger.info("${coderClient.buildVersion} is compatible") + } catch (e: InvalidVersionException) { + logger.warn(e) notificationBanner.apply { component.isVisible = true showWarning(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.invalid.coder.version", coderClient.buildVersion)) } - } else { - val coderVersion = CoderSemVer.parse(coderClient.buildVersion) - if (!coderVersion.isInClosedRange(CoderSupportedVersions.minCompatibleCoderVersion, CoderSupportedVersions.maxCompatibleCoderVersion)) { - logger.warn("Running with incompatible version ${coderClient.buildVersion}") - notificationBanner.apply { - component.isVisible = true - showWarning(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.unsupported.coder.version", coderClient.buildVersion)) - } - } else { - logger.info("Got version ${coderClient.buildVersion}...") + } catch (e: IncompatibleVersionException) { + logger.warn(e) + notificationBanner.apply { + component.isVisible = true + showWarning(CoderGatewayBundle.message("gateway.connector.view.coder.workspaces.unsupported.coder.version", coderClient.buildVersion)) } } diff --git a/src/test/groovy/CoderSemVerTest.groovy b/src/test/groovy/CoderSemVerTest.groovy index e20dbec1..705c5705 100644 --- a/src/test/groovy/CoderSemVerTest.groovy +++ b/src/test/groovy/CoderSemVerTest.groovy @@ -278,4 +278,49 @@ class CoderSemVerTest extends spock.lang.Specification { ] } + + def "should be invalid"() { + when: + CoderSemVer.checkVersionCompatibility(version) + + then: + thrown(InvalidVersionException) + + where: + version << [ + "", + "foo", + "1.foo.2", + ] + } + + def "should be incompatible"() { + when: + CoderSemVer.checkVersionCompatibility(version) + + then: + thrown(IncompatibleVersionException) + + where: + version << [ + "0.0.0", + "0.12.8", + "9999999999.99999.99", + ] + } + + def "should be compatible"() { + when: + CoderSemVer.checkVersionCompatibility(version) + + then: + noExceptionThrown() + + where: + version << [ + "0.12.9", + "0.99.99", + "1.0.0", + ] + } } From 8aa11246260b2c54a1822bc8007b34aff18faf8b Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 6 Apr 2023 09:00:24 -0800 Subject: [PATCH 15/21] Use try/finally to close conn --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index bb24b269..e18b5521 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -37,6 +37,7 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: val subdir = if (deployment.port > 0) "${deployment.host}-${deployment.port}" else deployment.host localBinaryPath = destinationDir.resolve(subdir).resolve(binaryName) } + /** * Return the name of the binary (with extension) for the provided OS and * architecture. @@ -80,36 +81,38 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: conn.setRequestProperty("If-None-Match", "\"$etag\"") } conn.setRequestProperty("Accept-Encoding", "gzip") - conn.connect() - logger.info("GET ${conn.responseCode} $remoteBinaryUrl") - when (conn.responseCode) { - HttpURLConnection.HTTP_OK -> { - logger.info("Downloading binary to ${localBinaryPath.toAbsolutePath()}") - Files.createDirectories(localBinaryPath.parent) - conn.inputStream.use { - Files.copy( - if (conn.contentEncoding == "gzip") GZIPInputStream(it) else it, - localBinaryPath, - StandardCopyOption.REPLACE_EXISTING, - ) - } - if (getOS() != OS.WINDOWS) { - Files.setPosixFilePermissions( - localBinaryPath, - PosixFilePermissions.fromString("rwxr-x---") - ) + + try { + conn.connect() + logger.info("GET ${conn.responseCode} $remoteBinaryUrl") + when (conn.responseCode) { + HttpURLConnection.HTTP_OK -> { + logger.info("Downloading binary to ${localBinaryPath.toAbsolutePath()}") + Files.createDirectories(localBinaryPath.parent) + conn.inputStream.use { + Files.copy( + if (conn.contentEncoding == "gzip") GZIPInputStream(it) else it, + localBinaryPath, + StandardCopyOption.REPLACE_EXISTING, + ) + } + if (getOS() != OS.WINDOWS) { + Files.setPosixFilePermissions( + localBinaryPath, + PosixFilePermissions.fromString("rwxr-x---") + ) + } + return true } - conn.disconnect() - return true - } - HttpURLConnection.HTTP_NOT_MODIFIED -> { - logger.info("Using cached binary at ${localBinaryPath.toAbsolutePath()}") - conn.disconnect() - return false + HttpURLConnection.HTTP_NOT_MODIFIED -> { + logger.info("Using cached binary at ${localBinaryPath.toAbsolutePath()}") + return false + } } + } finally { + conn.disconnect() } - conn.disconnect() throw Exception("Unable to download $remoteBinaryUrl (got response code `${conn.responseCode}`)") } From ab202ca284aeae3c02f4d066761104a1868a0606 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 6 Apr 2023 09:12:20 -0800 Subject: [PATCH 16/21] Handle IDN --- src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt | 6 +++++- src/test/groovy/CoderCLIManagerTest.groovy | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index e18b5521..48575374 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -7,6 +7,7 @@ import java.io.BufferedInputStream import java.io.FileInputStream import java.io.FileNotFoundException import java.net.HttpURLConnection +import java.net.IDN import java.net.URL import java.nio.file.Files import java.nio.file.Path @@ -34,7 +35,10 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: deployment.port, "/bin/$binaryName" ) - val subdir = if (deployment.port > 0) "${deployment.host}-${deployment.port}" else deployment.host + // Convert IDN to ASCII in case the file system cannot support the + // necessary character set. + val host = IDN.toASCII(deployment.host, IDN.ALLOW_UNASSIGNED) + val subdir = if (deployment.port > 0) "${host}-${deployment.port}" else host localBinaryPath = destinationDir.resolve(subdir).resolve(binaryName) } diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index 675163e8..f3d99dad 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -94,6 +94,14 @@ class CoderCLIManagerTest extends spock.lang.Specification { ccm.localBinaryPath.getParent() == CoderCLIManager.getDataDir().resolve("test.coder.invalid-3000") } + def "encodes IDN with punycode"() { + given: + def ccm = new CoderCLIManager(new URL("https://test.😉.invalid")) + + expect: + ccm.localBinaryPath.getParent() == CoderCLIManager.getDataDir().resolve("test.xn--n28h.invalid") + } + def "downloads a working cli"() { given: def ccm = createCLIManager() From 4fd74821044719c9ca320a6fb63e5f06e798620f Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 6 Apr 2023 09:30:10 -0800 Subject: [PATCH 17/21] Lift return out of try --- src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 48575374..84d1f5d1 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -124,7 +124,7 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: * Return the entity tag for the binary on disk, if any. */ private fun getBinaryETag(): String? { - try { + return try { val md = MessageDigest.getInstance("SHA-1") val fis = FileInputStream(localBinaryPath.toFile()) val dis = DigestInputStream(BufferedInputStream(fis), md) @@ -132,12 +132,12 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: while (dis.read() != -1) { } } - return HexBinaryAdapter().marshal(md.digest()).lowercase() + HexBinaryAdapter().marshal(md.digest()).lowercase() } catch (e: FileNotFoundException) { - return null + null } catch (e: Exception) { logger.warn("Unable to calculate hash for ${localBinaryPath.toAbsolutePath()}", e) - return null + null } } From 0c472586d13377042ca14bd962bbdac326c8a0e6 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 6 Apr 2023 09:30:59 -0800 Subject: [PATCH 18/21] Suppress empty body control flow warning --- src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 84d1f5d1..aac1da2f 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -123,6 +123,7 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: /** * Return the entity tag for the binary on disk, if any. */ + @Suppress("ControlFlowWithEmptyBody") private fun getBinaryETag(): String? { return try { val md = MessageDigest.getInstance("SHA-1") From 7f654e41b90aa8ac4789698de679f802d336c55e Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 6 Apr 2023 10:32:22 -0800 Subject: [PATCH 19/21] Test download failure --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 4 +++- .../views/steps/CoderWorkspacesStepView.kt | 4 ++++ src/test/groovy/CoderCLIManagerTest.groovy | 24 ++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index aac1da2f..6cbab66d 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -117,7 +117,7 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: } finally { conn.disconnect() } - throw Exception("Unable to download $remoteBinaryUrl (got response code `${conn.responseCode}`)") + throw ResponseException("Unexpected response from $remoteBinaryUrl", conn.responseCode) } /** @@ -251,3 +251,5 @@ class Environment(private val env: Map = emptyMap()) { return System.getenv(name) } } + +class ResponseException(message: String, val code: Int) : Exception(message) 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 2663cfcc..b33de337 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -16,6 +16,7 @@ import com.coder.gateway.sdk.CoderSemVer import com.coder.gateway.sdk.IncompatibleVersionException import com.coder.gateway.sdk.InvalidVersionException import com.coder.gateway.sdk.OS +import com.coder.gateway.sdk.ResponseException import com.coder.gateway.sdk.TemplateIconDownloader import com.coder.gateway.sdk.ex.AuthenticationResponseException import com.coder.gateway.sdk.ex.TemplateResponseException @@ -444,6 +445,9 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } try { cliManager.downloadCLI() + } catch (e: ResponseException) { + logger.error("Download failed with response code ${e.code}", e) + return@launchUnderBackgroundProgress } catch (e: Exception) { logger.error("Failed to download Coder CLI", e) return@launchUnderBackgroundProgress diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index f3d99dad..564372f8 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -21,7 +21,10 @@ class CoderCLIManagerTest extends spock.lang.Specification { @Shared private def servers = [] - String startMockServer() { + /** + * Create, start, and return a server that mocks Coder. + */ + HttpServer startMockServer(errorCode = 0) { HttpServer srv = HttpServer.create(new InetSocketAddress(0), 0) srv.createContext("/", new HttpHandler() { void handle(HttpExchange exchange) { @@ -39,6 +42,11 @@ class CoderCLIManagerTest extends spock.lang.Specification { response = "not found" } + if (errorCode != 0) { + code = errorCode + response = "error code ${code}" + } + byte[] body = response.getBytes() exchange.sendResponseHeaders(code, code == HttpURLConnection.HTTP_OK ? body.length : -1) exchange.responseBody.write(body) @@ -102,6 +110,20 @@ class CoderCLIManagerTest extends spock.lang.Specification { ccm.localBinaryPath.getParent() == CoderCLIManager.getDataDir().resolve("test.xn--n28h.invalid") } + def "fails to download"() { + given: + HttpServer server = startMockServer(HttpURLConnection.HTTP_INTERNAL_ERROR) + String url = "http://localhost:" + server.address.port + def ccm = new CoderCLIManager(new URL(url), tmpdir) + + when: + ccm.downloadCLI() + + then: + def e = thrown(ResponseException) + e.code == HttpURLConnection.HTTP_INTERNAL_ERROR + } + def "downloads a working cli"() { given: def ccm = createCLIManager() From 9f2d4995f49f2857bcaee2346ab7b24a9bd201ff Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 6 Apr 2023 11:11:54 -0800 Subject: [PATCH 20/21] Use real deployment for just one test That is enough to prove the downloading works and the rest can just test their own extra assumptions on top of that. --- .../com/coder/gateway/sdk/CoderCLIManager.kt | 8 +- src/test/groovy/CoderCLIManagerTest.groovy | 115 +++++++++++------- 2 files changed, 74 insertions(+), 49 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt index 6cbab66d..ddf99dc7 100644 --- a/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt @@ -160,10 +160,12 @@ class CoderCLIManager @JvmOverloads constructor(deployment: URL, destinationDir: } /** - * Execute the binary with the provided arguments. - * - * @return The command's output. + * Return the binary version. */ + fun version(): String { + return exec("version") + } + private fun exec(vararg args: String): String { val stdout = ProcessExecutor() .command(localBinaryPath.toString(), *args) diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index 564372f8..66c4d6d7 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -14,25 +14,23 @@ import java.nio.file.Path class CoderCLIManagerTest extends spock.lang.Specification { @Shared private Path tmpdir = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway-test") - @Shared - private String deploymentURL = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT") - @Shared - private String altDeploymentURL = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT_ALT") - @Shared - private def servers = [] + private String mockBinaryContent = "#!/bin/sh\necho Coder" /** * Create, start, and return a server that mocks Coder. */ - HttpServer startMockServer(errorCode = 0) { + def mockServer(errorCode = 0) { HttpServer srv = HttpServer.create(new InetSocketAddress(0), 0) srv.createContext("/", new HttpHandler() { void handle(HttpExchange exchange) { int code = HttpURLConnection.HTTP_OK - String response = "not a real binary" + // TODO: Is there some simple way to create an executable file + // on Windows without having to execute something to generate + // said executable or having to commit one to the repo? + String response = mockBinaryContent String[] etags = exchange.requestHeaders.get("If-None-Match") - if (etags != null && etags.contains("\"cb25cf6f41bb3127d7e05b0c3c6403be9ab052bc\"")) { + if (etags != null && etags.contains("\"2f1960264fc0f332a2a7fef2fe678f258dcdff9c\"")) { code = HttpURLConnection.HTTP_NOT_MODIFIED response = "not modified" } @@ -53,37 +51,14 @@ class CoderCLIManagerTest extends spock.lang.Specification { exchange.close() } }) - servers << srv srv.start() - return "http://localhost:" + srv.address.port + return [srv, "http://localhost:" + srv.address.port] } void setupSpec() { // Clean up from previous runs otherwise they get cluttered since the - // port is random. + // mock server port is random. tmpdir.toFile().deleteDir() - - // Spin up mock server(s). - if (deploymentURL == null) { - deploymentURL = startMockServer() - } - if (altDeploymentURL == null) { - altDeploymentURL = startMockServer() - } - } - - void cleanupSpec() { - servers.each { - it.stop(0) - } - } - - /** - * Create a CLI manager pointing to the URLs in the environment or to mocked - * servers. - */ - CoderCLIManager createCLIManager(Boolean alternate = false) { - return new CoderCLIManager(new URL(alternate ? altDeploymentURL : deploymentURL), tmpdir) } def "defaults to a sub-directory in the data directory"() { @@ -112,8 +87,7 @@ class CoderCLIManagerTest extends spock.lang.Specification { def "fails to download"() { given: - HttpServer server = startMockServer(HttpURLConnection.HTTP_INTERNAL_ERROR) - String url = "http://localhost:" + server.address.port + def (srv, url) = mockServer(HttpURLConnection.HTTP_INTERNAL_ERROR) def ccm = new CoderCLIManager(new URL(url), tmpdir) when: @@ -122,11 +96,35 @@ class CoderCLIManagerTest extends spock.lang.Specification { then: def e = thrown(ResponseException) e.code == HttpURLConnection.HTTP_INTERNAL_ERROR + + 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" }) + def "downloads a real working cli"() { + given: + def url = System.getenv("CODER_GATEWAY_TEST_DEPLOYMENT") + if (url == null) { + url = "https://dev.coder.com" + } + def ccm = new CoderCLIManager(new URL(url), tmpdir) + ccm.localBinaryPath.getParent().toFile().deleteDir() + + when: + def downloaded = ccm.downloadCLI() + + then: + downloaded + ccm.version().contains("Coder") } - def "downloads a working cli"() { + def "downloads a mocked cli"() { given: - def ccm = createCLIManager() + def (srv, url) = mockServer() + def ccm = new CoderCLIManager(new URL(url), tmpdir) ccm.localBinaryPath.getParent().toFile().deleteDir() when: @@ -134,40 +132,59 @@ class CoderCLIManagerTest extends spock.lang.Specification { then: downloaded - ccm.localBinaryPath.toFile().canExecute() + // The mock does not serve a binary that works on Windows so do not + // actually execute. Checking the contents works just as well as proof + // that the binary was correctly downloaded anyway. + ccm.localBinaryPath.toFile().readBytes() == mockBinaryContent.getBytes() + + cleanup: + srv.stop(0) } def "overwrites cli if incorrect version"() { given: - def ccm = createCLIManager() + def (srv, url) = mockServer() + def ccm = new CoderCLIManager(new URL(url), tmpdir) Files.createDirectories(ccm.localBinaryPath.getParent()) ccm.localBinaryPath.toFile().write("cli") + ccm.localBinaryPath.toFile().setLastModified(0) when: def downloaded = ccm.downloadCLI() then: downloaded - ccm.localBinaryPath.toFile().canExecute() + ccm.localBinaryPath.toFile().readBytes() != "cli".getBytes() + ccm.localBinaryPath.toFile().lastModified() > 0 + + cleanup: + srv.stop(0) } def "skips cli download if it already exists"() { given: - def ccm = createCLIManager() + def (srv, url) = mockServer() + def ccm = new CoderCLIManager(new URL(url), tmpdir) when: ccm.downloadCLI() + ccm.localBinaryPath.toFile().setLastModified(0) def downloaded = ccm.downloadCLI() then: !downloaded - ccm.localBinaryPath.toFile().canExecute() + ccm.localBinaryPath.toFile().lastModified() == 0 + + cleanup: + srv.stop(0) } def "does not clobber other deployments"() { - given: - def ccm1 = createCLIManager(true) - def ccm2 = createCLIManager() + setup: + def (srv1, url1) = mockServer() + def (srv2, url2) = mockServer() + def ccm1 = new CoderCLIManager(new URL(url1), tmpdir) + def ccm2 = new CoderCLIManager(new URL(url2), tmpdir) when: ccm1.downloadCLI() @@ -175,6 +192,12 @@ class CoderCLIManagerTest extends spock.lang.Specification { then: ccm1.localBinaryPath != ccm2.localBinaryPath + ccm1.localBinaryPath.toFile().exists() + ccm2.localBinaryPath.toFile().exists() + + cleanup: + srv1.stop(0) + srv2.stop(0) } Map testEnv = [ From 1e87ae393cc55e236e60dd9d6d024a934700dcb4 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 6 Apr 2023 11:12:34 -0800 Subject: [PATCH 21/21] Mention tests in the readme --- README.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index fa9fe22f..e7bc6ea9 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Coder Gateway Plugin +# Coder Gateway Plugin [!["Join us on Discord"](https://img.shields.io/badge/join-us%20on%20Discord-gray.svg?longCache=true&logo=discord&colorB=purple)](https://discord.gg/coder) @@ -87,7 +87,9 @@ The properties listed define the plugin itself or configure the [gradle-intellij ### Testing -No functional or UI tests are available yet. +Run tests with `./gradlew test`. By default this will test against +`https://dev.coder.com` but you can set `CODER_GATEWAY_TEST_DEPLOYMENT` to a URL +of your choice or to `mock` to use mocks only. ### Code Monitoring @@ -127,7 +129,8 @@ In the `.github/workflows` directory, you can find definitions for the following - Triggered on `Publish release` event. - Updates `CHANGELOG.md` file with the content provided with the release note. - Publishes the plugin to JetBrains Marketplace using the provided `PUBLISH_TOKEN`. - - Sets publish channel depending on the plugin version, i.e. `1.0.0-beta` -> `beta` channel. For now, both `main` and `eap` branches are published on default release channel. + - Sets publish channel depending on the plugin version, i.e. `1.0.0-beta` -> `beta` channel. For now, both `main` + and `eap` branches are published on default release channel. - Patches the Changelog and commits. ### Release flow