diff --git a/CHANGELOG.md b/CHANGELOG.md index b3428209..dff09f10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ ## Unreleased +### Changed + +- Disable autostarting workspaces by default on macOS to prevent an issue where + it wakes periodically and keeps the workspace on. This can be toggled via the + "Disable autostart" setting. + ## 2.9.3 - 2024-02-10 ### Fixed diff --git a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt index 73aaff1f..70ffb85c 100644 --- a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt +++ b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt @@ -1,8 +1,8 @@ package com.coder.gateway import com.coder.gateway.services.CoderSettingsService -import com.coder.gateway.util.canCreateDirectory import com.coder.gateway.services.CoderSettingsState +import com.coder.gateway.util.canCreateDirectory import com.intellij.openapi.components.service import com.intellij.openapi.options.BoundConfigurable import com.intellij.openapi.ui.DialogPanel @@ -102,6 +102,13 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") { CoderGatewayBundle.message("gateway.connector.settings.tls-alt-name.comment") ) }.layout(RowLayout.PARENT_GRID) + row(CoderGatewayBundle.message("gateway.connector.settings.disable-autostart.heading")) { + checkBox(CoderGatewayBundle.message("gateway.connector.settings.disable-autostart.title")) + .bindSelected(state::disableAutostart) + .comment( + CoderGatewayBundle.message("gateway.connector.settings.disable-autostart.comment") + ) + }.layout(RowLayout.PARENT_GRID) } } diff --git a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt index 689fcd11..89b409aa 100644 --- a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt +++ b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt @@ -96,6 +96,13 @@ fun ensureCLI( return if (cliMatches == null && dataCLIMatches != null) dataCLI else cli } +/** + * The supported features of the CLI. + */ +data class Features ( + val disableAutostart: Boolean = false, +) + /** * Manage the CLI for a single deployment. */ @@ -199,9 +206,11 @@ class CoderCLIManager( /** * Configure SSH to use this binary. + * + * This can take supported features for testing purposes only. */ - fun configSsh(workspaceNames: List) { - writeSSHConfig(modifySSHConfig(readSSHConfig(), workspaceNames)) + fun configSsh(workspaceNames: List, feats: Features = features) { + writeSSHConfig(modifySSHConfig(readSSHConfig(), workspaceNames, feats)) } /** @@ -219,8 +228,11 @@ class CoderCLIManager( * Given an existing SSH config modify it to add or remove the config for * this deployment and return the modified config or null if it does not * need to be modified. + * + * If features are not provided, calculate them based on the binary + * version. */ - private fun modifySSHConfig(contents: String?, workspaceNames: List): String? { + private fun modifySSHConfig(contents: String?, workspaceNames: List, feats: Features): String? { val host = deploymentURL.safeHost() val startBlock = "# --- START CODER JETBRAINS $host" val endBlock = "# --- END CODER JETBRAINS $host" @@ -230,7 +242,8 @@ class CoderCLIManager( "--global-config", escape(coderConfigPath.toString()), if (settings.headerCommand.isNotBlank()) "--header-command" else null, if (settings.headerCommand.isNotBlank()) escapeSubcommand(settings.headerCommand) else null, - "ssh", "--stdio") + "ssh", "--stdio", + if (settings.disableAutostart && feats.disableAutostart) "--disable-autostart" else null) val blockContent = workspaceNames.joinToString( System.lineSeparator(), startBlock + System.lineSeparator(), @@ -238,7 +251,7 @@ class CoderCLIManager( transform = { """ Host ${getHostName(deploymentURL, it)} - ProxyCommand ${proxyArgs.joinToString(" ")} ${it} + ProxyCommand ${proxyArgs.joinToString(" ")} $it ConnectTimeout 0 StrictHostKeyChecking no UserKnownHostsFile /dev/null @@ -333,31 +346,36 @@ class CoderCLIManager( } /** - * Returns true if the CLI has the same major/minor/patch version as the - * provided version, false if it does not match, or null if the CLI version - * could not be determined because the binary could not be executed or the - * version could not be parsed. + * Like version(), but logs errors instead of throwing them. */ - fun matchesVersion(rawBuildVersion: String): Boolean? { - val cliVersion = try { + private fun tryVersion(): SemVer? { + return try { version() } catch (e: Exception) { when (e) { is JsonSyntaxException, is InvalidVersionException -> { logger.info("Got invalid version from $localBinaryPath: ${e.message}") - return null } else -> { // An error here most likely means the CLI does not exist or // it executed successfully but output no version which // suggests it is not the right binary. logger.info("Unable to determine $localBinaryPath version: ${e.message}") - return null } } + null } + } + /** + * Returns true if the CLI has the same major/minor/patch version as the + * provided version, false if it does not match, or null if the CLI version + * could not be determined because the binary could not be executed or the + * version could not be parsed. + */ + fun matchesVersion(rawBuildVersion: String): Boolean? { + val cliVersion = tryVersion() ?: return null val buildVersion = try { SemVer.parse(rawBuildVersion) } catch (e: InvalidVersionException) { @@ -383,6 +401,18 @@ class CoderCLIManager( return stdout } + val features: Features + get() { + val version = tryVersion() + return if (version == null) { + Features() + } else { + Features( + // Autostart with SSH was added in 2.5.0. + disableAutostart = version >= SemVer(2, 5, 0)) + } + } + companion object { val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName) diff --git a/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt b/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt index da7fcb3e..28081297 100644 --- a/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt +++ b/src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt @@ -1,5 +1,7 @@ package com.coder.gateway.services +import com.coder.gateway.util.OS +import com.coder.gateway.util.getOS import com.intellij.openapi.components.PersistentStateComponent import com.intellij.openapi.components.RoamingType import com.intellij.openapi.components.Service @@ -56,6 +58,10 @@ class CoderSettingsState( // connections. This is useful when the hostname used to connect to the // Coder service does not match the hostname in the TLS certificate. var tlsAlternateHostname: String = "", + // Whether to add --disable-autostart to the proxy command. This works + // around issues on macOS where it periodically wakes and Gateway + // reconnects, keeping the workspace constantly up. + var disableAutostart: Boolean = getOS() == OS.MAC, ) : PersistentStateComponent { override fun getState(): CoderSettingsState { return this diff --git a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt index d2904c2c..f0050191 100644 --- a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt +++ b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt @@ -35,6 +35,9 @@ open class CoderSettings( val headerCommand: String get() = state.headerCommand + val disableAutostart: Boolean + get() = state.disableAutostart + /** * Where the specified deployment should put its data. */ diff --git a/src/main/resources/messages/CoderGatewayBundle.properties b/src/main/resources/messages/CoderGatewayBundle.properties index 1dac9df4..4b1a334f 100644 --- a/src/main/resources/messages/CoderGatewayBundle.properties +++ b/src/main/resources/messages/CoderGatewayBundle.properties @@ -111,3 +111,9 @@ gateway.connector.settings.tls-alt-name.comment=Optionally set this to \ an alternate hostname used for verifying TLS connections. This is useful \ when the hostname used to connect to the Coder service does not match the \ hostname in the TLS certificate. +gateway.connector.settings.disable-autostart.heading=Autostart: +gateway.connector.settings.disable-autostart.title=Disable autostart +gateway.connector.settings.disable-autostart.comment=Checking this box will \ + cause the plugin to configure the CLI with --disable-autostart. You must go \ + through the IDE selection again for the plugin to reconfigure the CLI with \ + this setting. diff --git a/src/test/fixtures/outputs/disable-autostart.conf b/src/test/fixtures/outputs/disable-autostart.conf new file mode 100644 index 00000000..a22e34d1 --- /dev/null +++ b/src/test/fixtures/outputs/disable-autostart.conf @@ -0,0 +1,9 @@ +# --- START CODER JETBRAINS test.coder.invalid +Host coder-jetbrains--foo--test.coder.invalid + ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio --disable-autostart foo + ConnectTimeout 0 + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + LogLevel ERROR + SetEnv CODER_SSH_SESSION_TYPE=JetBrains +# --- END CODER JETBRAINS test.coder.invalid diff --git a/src/test/fixtures/outputs/no-disable-autostart.conf b/src/test/fixtures/outputs/no-disable-autostart.conf new file mode 100644 index 00000000..217d332d --- /dev/null +++ b/src/test/fixtures/outputs/no-disable-autostart.conf @@ -0,0 +1,9 @@ +# --- START CODER JETBRAINS test.coder.invalid +Host coder-jetbrains--foo--test.coder.invalid + ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config ssh --stdio foo + ConnectTimeout 0 + StrictHostKeyChecking no + UserKnownHostsFile /dev/null + LogLevel ERROR + SetEnv CODER_SSH_SESSION_TYPE=JetBrains +# --- END CODER JETBRAINS test.coder.invalid diff --git a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt index 3c7383b4..1b1a29f5 100644 --- a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt @@ -33,18 +33,18 @@ import kotlin.test.assertTrue internal class CoderCLIManagerTest { private fun mkbin(version: String): String { - return listOf("#!/bin/sh", """echo '{"version": "${version}"}'""") + return listOf("#!/bin/sh", """echo '{"version": "$version"}'""") .joinToString("\n") } - private fun mockServer(errorCode: Int = 0): Pair { + private fun mockServer(errorCode: Int = 0, version: String? = null): Pair { val srv = HttpServer.create(InetSocketAddress(0), 0) srv.createContext("/") {exchange -> var code = HttpURLConnection.HTTP_OK // 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? - var response = mkbin("${srv.address.port}.0.0") + var response = mkbin(version ?: "${srv.address.port}.0.0") val eTags = exchange.requestHeaders["If-None-Match"] if (exchange.requestURI.path == "/bin/override") { code = HttpURLConnection.HTTP_OK @@ -234,7 +234,15 @@ internal class CoderCLIManagerTest { srv2.stop(0) } - data class SSHTest(val workspaces: List, val input: String?, val output: String, val remove: String, val headerCommand: String?) + data class SSHTest( + val workspaces: List, + val input: String?, + val output: String, + val remove: String, + val headerCommand: String?, + val disableAutostart: Boolean = false, + val features: Features? = null, + ) @Test fun testConfigureSSH() { @@ -256,13 +264,16 @@ internal class CoderCLIManagerTest { SSHTest(listOf("header"), null, "header-command-windows", "blank", """"C:\Program Files\My Header Command\HeaderCommand.exe" --url="%CODER_URL%" --test="foo bar"""") } else { SSHTest(listOf("header"), null, "header-command", "blank", "my-header-command --url=\"\$CODER_URL\" --test=\"foo bar\" --literal='\$CODER_URL'") - } + }, + SSHTest(listOf("foo"), null, "disable-autostart", "blank", null, true, Features(true)), + SSHTest(listOf("foo"), null, "no-disable-autostart", "blank", null, true, Features(false)), ) val newlineRe = "\r?\n".toRegex() tests.forEach { val settings = CoderSettings(CoderSettingsState( + disableAutostart = it.disableAutostart, dataDirectory = tmpdir.resolve("configure-ssh").toString(), headerCommand = it.headerCommand ?: ""), sshConfigPath = tmpdir.resolve(it.input + "_to_" + it.output + ".conf")) @@ -285,12 +296,12 @@ internal class CoderCLIManagerTest { .replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", escape(ccm.localBinaryPath.toString())) // Add workspaces. - ccm.configSsh(it.workspaces) + ccm.configSsh(it.workspaces, it.features ?: Features()) assertEquals(expectedConf, settings.sshConfigPath.toFile().readText()) // Remove configuration. - ccm.configSsh(emptyList()) + ccm.configSsh(emptyList(), it.features ?: Features()) // Remove is the configuration we expect after removing. assertEquals( @@ -540,6 +551,31 @@ internal class CoderCLIManagerTest { srv.stop(0) } + @Test + fun testFeatures() { + if (getOS() == OS.WINDOWS) { + return // Cannot execute mock binaries on Windows. + } + + val tests = listOf( + Pair("2.5.0", Features(true)), + Pair("4.9.0", Features(true)), + Pair("2.4.9", Features(false)), + Pair("1.0.1", Features(false)), + ) + + tests.forEach { + val (srv, url) = mockServer(version = it.first) + val ccm = CoderCLIManager(url, CoderSettings(CoderSettingsState( + dataDirectory = tmpdir.resolve("features").toString())) + ) + assertEquals(true, ccm.download()) + assertEquals(it.second, ccm.features, "version: ${it.first}") + + srv.stop(0) + } + } + companion object { private val tmpdir: Path = Path.of(System.getProperty("java.io.tmpdir")).resolve("coder-gateway-test/cli-manager") diff --git a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt index 26a121f2..ab1943c6 100644 --- a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt @@ -1,15 +1,14 @@ package com.coder.gateway.settings import com.coder.gateway.services.CoderSettingsState -import kotlin.test.Test -import kotlin.test.assertContains -import kotlin.test.assertEquals - import com.coder.gateway.util.OS import com.coder.gateway.util.getOS import com.coder.gateway.util.withPath import java.net.URL import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertEquals internal class CoderSettingsTest { @Test @@ -179,14 +178,15 @@ internal class CoderSettingsTest { // Make sure the remaining settings are being conveyed. val settings = CoderSettings( CoderSettingsState( - enableDownloads = false, - enableBinaryDirectoryFallback = true, - headerCommand = "test header", - tlsCertPath = "tls cert path", - tlsKeyPath = "tls key path", - tlsCAPath = "tls ca path", - tlsAlternateHostname = "tls alt hostname", - ) + enableDownloads = false, + enableBinaryDirectoryFallback = true, + headerCommand = "test header", + tlsCertPath = "tls cert path", + tlsKeyPath = "tls key path", + tlsCAPath = "tls ca path", + tlsAlternateHostname = "tls alt hostname", + disableAutostart = true, + ) ) assertEquals(false, settings.enableDownloads) @@ -196,5 +196,6 @@ internal class CoderSettingsTest { assertEquals("tls key path", settings.tls.keyPath) assertEquals("tls ca path", settings.tls.caPath) assertEquals("tls alt hostname", settings.tls.altHostname) + assertEquals(true, settings.disableAutostart) } }