prevent unintended early return from skipping writing wildcard configs (#539)

bcpeinhardt · web-flow · commit 7d8ad4b9402a · 2025-02-20T09:15:44.000-06:00
prevent unintended early return from skipping writing wildcard configs
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@
 
 ## Unreleased
 
+### Fixed
+
+- Fix bug where wildcard configs would not be written under certain conditions.
+
 ## 2.18.1 - 2025-02-14
 
 ### Changed
diff --git a/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt b/src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt
@@ -167,12 +167,11 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
         }
     }
 
-    private fun validateDataDirectory(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? =
-        {
-            if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) {
-                error("Cannot create this directory")
-            } else {
-                null
-            }
+    private fun validateDataDirectory(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = {
+        if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) {
+            error("Cannot create this directory")
+        } else {
+            null
         }
+    }
 }
diff --git a/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt b/src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt
@@ -257,7 +257,6 @@ class CoderCLIManager(
         val host = deploymentURL.safeHost()
         val startBlock = "# --- START CODER JETBRAINS $host"
         val endBlock = "# --- END CODER JETBRAINS $host"
-        val isRemoving = workspaceNames.isEmpty()
         val baseArgs =
             listOfNotNull(
                 escape(localBinaryPath.toString()),
@@ -308,35 +307,36 @@ class CoderCLIManager(
                             Host ${getHostPrefix()}-bg--*
                               ProxyCommand ${backgroundProxyArgs.joinToString(" ")} --ssh-host-prefix ${getHostPrefix()}-bg-- %h
                             """.trimIndent()
-                            .plus("\n" + sshOpts.prependIndent("  "))
-                            .plus(extraConfig),
+                                .plus("\n" + sshOpts.prependIndent("  "))
+                                .plus(extraConfig),
                         ).replace("\n", System.lineSeparator()) +
                     System.lineSeparator() + endBlock
-
-        } else {
-            workspaceNames.joinToString(
-                System.lineSeparator(),
-                startBlock + System.lineSeparator(),
-                System.lineSeparator() + endBlock,
-                transform = {
-                    """
+            } else if (workspaceNames.isEmpty()) {
+                ""
+            } else {
+                workspaceNames.joinToString(
+                    System.lineSeparator(),
+                    startBlock + System.lineSeparator(),
+                    System.lineSeparator() + endBlock,
+                    transform = {
+                        """
                     Host ${getHostName(it.first, currentUser, it.second)}
                       ProxyCommand ${proxyArgs.joinToString(" ")} ${getWorkspaceParts(it.first, it.second)}
-                    """.trimIndent()
-                        .plus("\n" + sshOpts.prependIndent("  "))
-                        .plus(extraConfig)
-                        .plus("\n")
-                        .plus(
-                            """
+                        """.trimIndent()
+                            .plus("\n" + sshOpts.prependIndent("  "))
+                            .plus(extraConfig)
+                            .plus("\n")
+                            .plus(
+                                """
                             Host ${getBackgroundHostName(it.first, currentUser, it.second)}
                               ProxyCommand ${backgroundProxyArgs.joinToString(" ")} ${getWorkspaceParts(it.first, it.second)}
-                            """.trimIndent()
-                                .plus("\n" + sshOpts.prependIndent("  "))
-                                .plus(extraConfig),
-                        ).replace("\n", System.lineSeparator())
-                },
-            )
-        }
+                                """.trimIndent()
+                                    .plus("\n" + sshOpts.prependIndent("  "))
+                                    .plus(extraConfig),
+                            ).replace("\n", System.lineSeparator())
+                    },
+                )
+            }
 
         if (contents == null) {
             logger.info("No existing SSH config to modify")
@@ -346,6 +346,8 @@ class CoderCLIManager(
         val start = "(\\s*)$startBlock".toRegex().find(contents)
         val end = "$endBlock(\\s*)".toRegex().find(contents)
 
+        val isRemoving = blockContent.isEmpty()
+
         if (start == null && end == null && isRemoving) {
             logger.info("No workspaces and no existing config blocks to remove")
             return null
@@ -477,15 +479,13 @@ class CoderCLIManager(
      *
      * Throws if the command execution fails.
      */
-    fun startWorkspace(workspaceOwner: String, workspaceName: String): String {
-        return exec(
-            "--global-config",
-            coderConfigPath.toString(),
-            "start",
-            "--yes",
-            workspaceOwner+"/"+workspaceName,
-        )
-    }
+    fun startWorkspace(workspaceOwner: String, workspaceName: String): String = exec(
+        "--global-config",
+        coderConfigPath.toString(),
+        "start",
+        "--yes",
+        workspaceOwner + "/" + workspaceName,
+    )
 
     private fun exec(vararg args: String): String {
         val stdout =
@@ -518,8 +518,7 @@ class CoderCLIManager(
     /*
      * This function returns the ssh-host-prefix used for Host entries.
      */
-    fun getHostPrefix(): String =
-        "coder-jetbrains-${deploymentURL.safeHost()}"
+    fun getHostPrefix(): String = "coder-jetbrains-${deploymentURL.safeHost()}"
 
     /**
      * This function returns the ssh host name generated for connecting to the workspace.
@@ -528,29 +527,27 @@ class CoderCLIManager(
         workspace: Workspace,
         currentUser: User,
         agent: WorkspaceAgent,
-    ): String =
-        if (features.wildcardSSH) {
-           "${getHostPrefix()}--${workspace.ownerName}--${workspace.name}.${agent.name}"
+    ): String = if (features.wildcardSSH) {
+        "${getHostPrefix()}--${workspace.ownerName}--${workspace.name}.${agent.name}"
+    } else {
+        // For a user's own workspace, we use the old syntax without a username for backwards compatibility,
+        // since the user might have recent connections that still use the old syntax.
+        if (currentUser.username == workspace.ownerName) {
+            "coder-jetbrains--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
         } else {
-           // For a user's own workspace, we use the old syntax without a username for backwards compatibility,
-           // since the user might have recent connections that still use the old syntax.
-           if (currentUser.username == workspace.ownerName) {
-               "coder-jetbrains--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
-           } else {
-               "coder-jetbrains--${workspace.ownerName}--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
+            "coder-jetbrains--${workspace.ownerName}--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
         }
     }
 
     fun getBackgroundHostName(
         workspace: Workspace,
         currentUser: User,
         agent: WorkspaceAgent,
-    ): String =
-        if (features.wildcardSSH) {
-            "${getHostPrefix()}-bg--${workspace.ownerName}--${workspace.name}.${agent.name}"
-        } else {
-            getHostName(workspace, currentUser, agent) + "--bg"
-        }
+    ): String = if (features.wildcardSSH) {
+        "${getHostPrefix()}-bg--${workspace.ownerName}--${workspace.name}.${agent.name}"
+    } else {
+        getHostName(workspace, currentUser, agent) + "--bg"
+    }
 
     companion object {
         val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName)
diff --git a/src/main/kotlin/com/coder/gateway/icons/CoderIcons.kt b/src/main/kotlin/com/coder/gateway/icons/CoderIcons.kt
@@ -63,48 +63,47 @@ object CoderIcons {
     private val Y = IconLoader.getIcon("symbols/y.svg", javaClass)
     private val Z = IconLoader.getIcon("symbols/z.svg", javaClass)
 
-    fun fromChar(c: Char) =
-        when (c) {
-            '0' -> ZERO
-            '1' -> ONE
-            '2' -> TWO
-            '3' -> THREE
-            '4' -> FOUR
-            '5' -> FIVE
-            '6' -> SIX
-            '7' -> SEVEN
-            '8' -> EIGHT
-            '9' -> NINE
-
-            'a' -> A
-            'b' -> B
-            'c' -> C
-            'd' -> D
-            'e' -> E
-            'f' -> F
-            'g' -> G
-            'h' -> H
-            'i' -> I
-            'j' -> J
-            'k' -> K
-            'l' -> L
-            'm' -> M
-            'n' -> N
-            'o' -> O
-            'p' -> P
-            'q' -> Q
-            'r' -> R
-            's' -> S
-            't' -> T
-            'u' -> U
-            'v' -> V
-            'w' -> W
-            'x' -> X
-            'y' -> Y
-            'z' -> Z
-
-            else -> UNKNOWN
-        }
+    fun fromChar(c: Char) = when (c) {
+        '0' -> ZERO
+        '1' -> ONE
+        '2' -> TWO
+        '3' -> THREE
+        '4' -> FOUR
+        '5' -> FIVE
+        '6' -> SIX
+        '7' -> SEVEN
+        '8' -> EIGHT
+        '9' -> NINE
+
+        'a' -> A
+        'b' -> B
+        'c' -> C
+        'd' -> D
+        'e' -> E
+        'f' -> F
+        'g' -> G
+        'h' -> H
+        'i' -> I
+        'j' -> J
+        'k' -> K
+        'l' -> L
+        'm' -> M
+        'n' -> N
+        'o' -> O
+        'p' -> P
+        'q' -> Q
+        'r' -> R
+        's' -> S
+        't' -> T
+        'u' -> U
+        'v' -> V
+        'w' -> W
+        'x' -> X
+        'y' -> Y
+        'z' -> Z
+
+        else -> UNKNOWN
+    }
 }
 
 fun alignToInt(g: Graphics) {
diff --git a/src/main/kotlin/com/coder/gateway/models/WorkspaceAndAgentStatus.kt b/src/main/kotlin/com/coder/gateway/models/WorkspaceAndAgentStatus.kt
@@ -47,13 +47,12 @@ enum class WorkspaceAndAgentStatus(val label: String, val description: String) {
     READY("Ready", "The agent is ready to accept connections."),
     ;
 
-    fun statusColor(): JBColor =
-        when (this) {
-            READY, AGENT_STARTING_READY, START_TIMEOUT_READY -> JBColor.GREEN
-            CREATED, START_ERROR, START_TIMEOUT, SHUTDOWN_TIMEOUT -> JBColor.YELLOW
-            FAILED, DISCONNECTED, TIMEOUT, SHUTDOWN_ERROR -> JBColor.RED
-            else -> if (JBColor.isBright()) JBColor.LIGHT_GRAY else JBColor.DARK_GRAY
-        }
+    fun statusColor(): JBColor = when (this) {
+        READY, AGENT_STARTING_READY, START_TIMEOUT_READY -> JBColor.GREEN
+        CREATED, START_ERROR, START_TIMEOUT, SHUTDOWN_TIMEOUT -> JBColor.YELLOW
+        FAILED, DISCONNECTED, TIMEOUT, SHUTDOWN_ERROR -> JBColor.RED
+        else -> if (JBColor.isBright()) JBColor.LIGHT_GRAY else JBColor.DARK_GRAY
+    }
 
     /**
      * Return true if the agent is in a connectable state.
diff --git a/src/main/kotlin/com/coder/gateway/sdk/convertors/InstantConverter.kt b/src/main/kotlin/com/coder/gateway/sdk/convertors/InstantConverter.kt
@@ -12,10 +12,9 @@ import java.time.temporal.TemporalAccessor
 class InstantConverter {
     @ToJson fun toJson(src: Instant?): String = FORMATTER.format(src)
 
-    @FromJson fun fromJson(src: String): Instant? =
-        FORMATTER.parse(src) { temporal: TemporalAccessor? ->
-            Instant.from(temporal)
-        }
+    @FromJson fun fromJson(src: String): Instant? = FORMATTER.parse(src) { temporal: TemporalAccessor? ->
+        Instant.from(temporal)
+    }
 
     companion object {
         private val FORMATTER = DateTimeFormatter.ISO_INSTANT
diff --git a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt
@@ -188,7 +188,7 @@ open class CoderSettings(
      * Whether to check for IDE updates.
      */
     val checkIDEUpdate: Boolean
-    get() = state.checkIDEUpdates
+        get() = state.checkIDEUpdates
 
     /**
      * Whether to ignore a failed setup command.
diff --git a/src/test/fixtures/inputs/wildcard.conf b/src/test/fixtures/inputs/wildcard.conf
@@ -0,0 +1,17 @@
+# --- START CODER JETBRAINS test.coder.invalid
+Host coder-jetbrains-test.coder.invalid--*
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config --url https://test.coder.invalid ssh --stdio --ssh-host-prefix coder-jetbrains-test.coder.invalid-- %h
+  ConnectTimeout 0
+  StrictHostKeyChecking no
+  UserKnownHostsFile /dev/null
+  LogLevel ERROR
+  SetEnv CODER_SSH_SESSION_TYPE=JetBrains
+
+Host coder-jetbrains-test.coder.invalid-bg--*
+  ProxyCommand /tmp/coder-gateway/test.coder.invalid/coder-linux-amd64 --global-config /tmp/coder-gateway/test.coder.invalid/config --url https://test.coder.invalid ssh --stdio --ssh-host-prefix coder-jetbrains-test.coder.invalid-bg-- %h
+  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
@@ -423,7 +423,7 @@ internal class CoderCLIManagerTest {
                     listOf(workspace),
                     input = null,
                     output = "wildcard",
-                    remove = "blank",
+                    remove = "wildcard",
                     features = Features(
                         wildcardSSH = true,
                     ),
@@ -472,6 +472,19 @@ internal class CoderCLIManagerTest {
                         }
                     }
 
+            val inputConf =
+                Path.of("src/test/fixtures/inputs/").resolve(it.remove + ".conf").toFile().readText()
+                    .replace(newlineRe, System.lineSeparator())
+                    .replace("/tmp/coder-gateway/test.coder.invalid/config", escape(coderConfigPath.toString()))
+                    .replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", escape(ccm.localBinaryPath.toString()))
+                    .let { conf ->
+                        if (it.sshLogDirectory != null) {
+                            conf.replace("/tmp/coder-gateway/test.coder.invalid/logs", it.sshLogDirectory.toString())
+                        } else {
+                            conf
+                        }
+                    }
+
             // Add workspaces.
             ccm.configSsh(
                 it.workspaces.flatMap { ws ->
@@ -496,8 +509,7 @@ internal class CoderCLIManagerTest {
             // Remove is the configuration we expect after removing.
             assertEquals(
                 settings.sshConfigPath.toFile().readText(),
-                Path.of("src/test/fixtures/inputs").resolve(it.remove + ".conf").toFile()
-                    .readText().replace(newlineRe, System.lineSeparator()),
+                inputConf
             )
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -167,12 +167,11 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {`
`167`	`167`	`}`
`168`	`168`	`}`
`169`	`169`
`170`		`- private fun validateDataDirectory(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? =`
`171`		`- {`
`172`		`- if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) {`
`173`		`- error("Cannot create this directory")`
`174`		`- } else {`
`175`		`- null`
`176`		`- }`
	`170`	`+ private fun validateDataDirectory(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = {`
	`171`	`+ if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) {`
	`172`	`+ error("Cannot create this directory")`
	`173`	`+ } else {`
	`174`	`+ null`
`177`	`175`	`}`
	`176`	`+ }`
`178`	`177`	`}`