Skip to content

Commit 7d8ad4b

Browse files
bcpeinhardtbcpeinhardt
authored
prevent unintended early return from skipping writing wildcard configs (#539)
prevent unintended early return from skipping writing wildcard configs
1 parent cdc6fda commit 7d8ad4b

File tree

9 files changed

+140
-114
lines changed

9 files changed

+140
-114
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
## Unreleased
66

7+
### Fixed
8+
9+
- Fix bug where wildcard configs would not be written under certain conditions.
10+
711
## 2.18.1 - 2025-02-14
812

913
### Changed

‎src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,11 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
167167
}
168168
}
169169

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
177175
}
176+
}
178177
}

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

Lines changed: 47 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,6 @@ class CoderCLIManager(
257257
val host = deploymentURL.safeHost()
258258
val startBlock = "# --- START CODER JETBRAINS $host"
259259
val endBlock = "# --- END CODER JETBRAINS $host"
260-
val isRemoving = workspaceNames.isEmpty()
261260
val baseArgs =
262261
listOfNotNull(
263262
escape(localBinaryPath.toString()),
@@ -308,35 +307,36 @@ class CoderCLIManager(
308307
Host ${getHostPrefix()}-bg--*
309308
ProxyCommand ${backgroundProxyArgs.joinToString(" ")} --ssh-host-prefix ${getHostPrefix()}-bg-- %h
310309
""".trimIndent()
311-
.plus("\n" + sshOpts.prependIndent(" "))
312-
.plus(extraConfig),
310+
.plus("\n" + sshOpts.prependIndent(" "))
311+
.plus(extraConfig),
313312
).replace("\n", System.lineSeparator()) +
314313
System.lineSeparator() + endBlock
315-
316-
} else {
317-
workspaceNames.joinToString(
318-
System.lineSeparator(),
319-
startBlock + System.lineSeparator(),
320-
System.lineSeparator() + endBlock,
321-
transform = {
322-
"""
314+
} else if (workspaceNames.isEmpty()) {
315+
""
316+
} else {
317+
workspaceNames.joinToString(
318+
System.lineSeparator(),
319+
startBlock + System.lineSeparator(),
320+
System.lineSeparator() + endBlock,
321+
transform = {
322+
"""
323323
Host ${getHostName(it.first, currentUser, it.second)}
324324
ProxyCommand ${proxyArgs.joinToString(" ")} ${getWorkspaceParts(it.first, it.second)}
325-
""".trimIndent()
326-
.plus("\n" + sshOpts.prependIndent(" "))
327-
.plus(extraConfig)
328-
.plus("\n")
329-
.plus(
330-
"""
325+
""".trimIndent()
326+
.plus("\n" + sshOpts.prependIndent(" "))
327+
.plus(extraConfig)
328+
.plus("\n")
329+
.plus(
330+
"""
331331
Host ${getBackgroundHostName(it.first, currentUser, it.second)}
332332
ProxyCommand ${backgroundProxyArgs.joinToString(" ")} ${getWorkspaceParts(it.first, it.second)}
333-
""".trimIndent()
334-
.plus("\n" + sshOpts.prependIndent(" "))
335-
.plus(extraConfig),
336-
).replace("\n", System.lineSeparator())
337-
},
338-
)
339-
}
333+
""".trimIndent()
334+
.plus("\n" + sshOpts.prependIndent(" "))
335+
.plus(extraConfig),
336+
).replace("\n", System.lineSeparator())
337+
},
338+
)
339+
}
340340

341341
if (contents == null) {
342342
logger.info("No existing SSH config to modify")
@@ -346,6 +346,8 @@ class CoderCLIManager(
346346
val start = "(\\s*)$startBlock".toRegex().find(contents)
347347
val end = "$endBlock(\\s*)".toRegex().find(contents)
348348

349+
val isRemoving = blockContent.isEmpty()
350+
349351
if (start == null && end == null && isRemoving) {
350352
logger.info("No workspaces and no existing config blocks to remove")
351353
return null
@@ -477,15 +479,13 @@ class CoderCLIManager(
477479
*
478480
* Throws if the command execution fails.
479481
*/
480-
fun startWorkspace(workspaceOwner: String, workspaceName: String): String {
481-
return exec(
482-
"--global-config",
483-
coderConfigPath.toString(),
484-
"start",
485-
"--yes",
486-
workspaceOwner+"/"+workspaceName,
487-
)
488-
}
482+
fun startWorkspace(workspaceOwner: String, workspaceName: String): String = exec(
483+
"--global-config",
484+
coderConfigPath.toString(),
485+
"start",
486+
"--yes",
487+
workspaceOwner + "/" + workspaceName,
488+
)
489489

490490
private fun exec(vararg args: String): String {
491491
val stdout =
@@ -518,8 +518,7 @@ class CoderCLIManager(
518518
/*
519519
* This function returns the ssh-host-prefix used for Host entries.
520520
*/
521-
fun getHostPrefix(): String =
522-
"coder-jetbrains-${deploymentURL.safeHost()}"
521+
fun getHostPrefix(): String = "coder-jetbrains-${deploymentURL.safeHost()}"
523522

524523
/**
525524
* This function returns the ssh host name generated for connecting to the workspace.
@@ -528,29 +527,27 @@ class CoderCLIManager(
528527
workspace: Workspace,
529528
currentUser: User,
530529
agent: WorkspaceAgent,
531-
): String =
532-
if (features.wildcardSSH) {
533-
"${getHostPrefix()}--${workspace.ownerName}--${workspace.name}.${agent.name}"
530+
): String = if (features.wildcardSSH) {
531+
"${getHostPrefix()}--${workspace.ownerName}--${workspace.name}.${agent.name}"
532+
} else {
533+
// For a user's own workspace, we use the old syntax without a username for backwards compatibility,
534+
// since the user might have recent connections that still use the old syntax.
535+
if (currentUser.username == workspace.ownerName) {
536+
"coder-jetbrains--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
534537
} else {
535-
// For a user's own workspace, we use the old syntax without a username for backwards compatibility,
536-
// since the user might have recent connections that still use the old syntax.
537-
if (currentUser.username == workspace.ownerName) {
538-
"coder-jetbrains--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
539-
} else {
540-
"coder-jetbrains--${workspace.ownerName}--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
538+
"coder-jetbrains--${workspace.ownerName}--${workspace.name}.${agent.name}--${deploymentURL.safeHost()}"
541539
}
542540
}
543541

544542
fun getBackgroundHostName(
545543
workspace: Workspace,
546544
currentUser: User,
547545
agent: WorkspaceAgent,
548-
): String =
549-
if (features.wildcardSSH) {
550-
"${getHostPrefix()}-bg--${workspace.ownerName}--${workspace.name}.${agent.name}"
551-
} else {
552-
getHostName(workspace, currentUser, agent) + "--bg"
553-
}
546+
): String = if (features.wildcardSSH) {
547+
"${getHostPrefix()}-bg--${workspace.ownerName}--${workspace.name}.${agent.name}"
548+
} else {
549+
getHostName(workspace, currentUser, agent) + "--bg"
550+
}
554551

555552
companion object {
556553
val logger = Logger.getInstance(CoderCLIManager::class.java.simpleName)

‎src/main/kotlin/com/coder/gateway/icons/CoderIcons.kt

Lines changed: 41 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -63,48 +63,47 @@ object CoderIcons {
6363
private val Y = IconLoader.getIcon("symbols/y.svg", javaClass)
6464
private val Z = IconLoader.getIcon("symbols/z.svg", javaClass)
6565

66-
fun fromChar(c: Char) =
67-
when (c) {
68-
'0' -> ZERO
69-
'1' -> ONE
70-
'2' -> TWO
71-
'3' -> THREE
72-
'4' -> FOUR
73-
'5' -> FIVE
74-
'6' -> SIX
75-
'7' -> SEVEN
76-
'8' -> EIGHT
77-
'9' -> NINE
78-
79-
'a' -> A
80-
'b' -> B
81-
'c' -> C
82-
'd' -> D
83-
'e' -> E
84-
'f' -> F
85-
'g' -> G
86-
'h' -> H
87-
'i' -> I
88-
'j' -> J
89-
'k' -> K
90-
'l' -> L
91-
'm' -> M
92-
'n' -> N
93-
'o' -> O
94-
'p' -> P
95-
'q' -> Q
96-
'r' -> R
97-
's' -> S
98-
't' -> T
99-
'u' -> U
100-
'v' -> V
101-
'w' -> W
102-
'x' -> X
103-
'y' -> Y
104-
'z' -> Z
105-
106-
else -> UNKNOWN
107-
}
66+
fun fromChar(c: Char) = when (c) {
67+
'0' -> ZERO
68+
'1' -> ONE
69+
'2' -> TWO
70+
'3' -> THREE
71+
'4' -> FOUR
72+
'5' -> FIVE
73+
'6' -> SIX
74+
'7' -> SEVEN
75+
'8' -> EIGHT
76+
'9' -> NINE
77+
78+
'a' -> A
79+
'b' -> B
80+
'c' -> C
81+
'd' -> D
82+
'e' -> E
83+
'f' -> F
84+
'g' -> G
85+
'h' -> H
86+
'i' -> I
87+
'j' -> J
88+
'k' -> K
89+
'l' -> L
90+
'm' -> M
91+
'n' -> N
92+
'o' -> O
93+
'p' -> P
94+
'q' -> Q
95+
'r' -> R
96+
's' -> S
97+
't' -> T
98+
'u' -> U
99+
'v' -> V
100+
'w' -> W
101+
'x' -> X
102+
'y' -> Y
103+
'z' -> Z
104+
105+
else -> UNKNOWN
106+
}
108107
}
109108

110109
fun alignToInt(g: Graphics) {

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,12 @@ enum class WorkspaceAndAgentStatus(val label: String, val description: String) {
4747
READY("Ready", "The agent is ready to accept connections."),
4848
;
4949

50-
fun statusColor(): JBColor =
51-
when (this) {
52-
READY, AGENT_STARTING_READY, START_TIMEOUT_READY -> JBColor.GREEN
53-
CREATED, START_ERROR, START_TIMEOUT, SHUTDOWN_TIMEOUT -> JBColor.YELLOW
54-
FAILED, DISCONNECTED, TIMEOUT, SHUTDOWN_ERROR -> JBColor.RED
55-
else -> if (JBColor.isBright()) JBColor.LIGHT_GRAY else JBColor.DARK_GRAY
56-
}
50+
fun statusColor(): JBColor = when (this) {
51+
READY, AGENT_STARTING_READY, START_TIMEOUT_READY -> JBColor.GREEN
52+
CREATED, START_ERROR, START_TIMEOUT, SHUTDOWN_TIMEOUT -> JBColor.YELLOW
53+
FAILED, DISCONNECTED, TIMEOUT, SHUTDOWN_ERROR -> JBColor.RED
54+
else -> if (JBColor.isBright()) JBColor.LIGHT_GRAY else JBColor.DARK_GRAY
55+
}
5756

5857
/**
5958
* Return true if the agent is in a connectable state.

‎src/main/kotlin/com/coder/gateway/sdk/convertors/InstantConverter.kt

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@ import java.time.temporal.TemporalAccessor
1212
class InstantConverter {
1313
@ToJson fun toJson(src: Instant?): String = FORMATTER.format(src)
1414

15-
@FromJson fun fromJson(src: String): Instant? =
16-
FORMATTER.parse(src) { temporal: TemporalAccessor? ->
17-
Instant.from(temporal)
18-
}
15+
@FromJson fun fromJson(src: String): Instant? = FORMATTER.parse(src) { temporal: TemporalAccessor? ->
16+
Instant.from(temporal)
17+
}
1918

2019
companion object {
2120
private val FORMATTER = DateTimeFormatter.ISO_INSTANT

‎src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ open class CoderSettings(
188188
* Whether to check for IDE updates.
189189
*/
190190
val checkIDEUpdate: Boolean
191-
get() = state.checkIDEUpdates
191+
get() = state.checkIDEUpdates
192192

193193
/**
194194
* Whether to ignore a failed setup command.

‎src/test/fixtures/inputs/wildcard.conf

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# --- START CODER JETBRAINS test.coder.invalid
2+
Host coder-jetbrains-test.coder.invalid--*
3+
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
4+
ConnectTimeout 0
5+
StrictHostKeyChecking no
6+
UserKnownHostsFile /dev/null
7+
LogLevel ERROR
8+
SetEnv CODER_SSH_SESSION_TYPE=JetBrains
9+
10+
Host coder-jetbrains-test.coder.invalid-bg--*
11+
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
12+
ConnectTimeout 0
13+
StrictHostKeyChecking no
14+
UserKnownHostsFile /dev/null
15+
LogLevel ERROR
16+
SetEnv CODER_SSH_SESSION_TYPE=JetBrains
17+
# --- END CODER JETBRAINS test.coder.invalid

‎src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ internal class CoderCLIManagerTest {
423423
listOf(workspace),
424424
input = null,
425425
output = "wildcard",
426-
remove = "blank",
426+
remove = "wildcard",
427427
features = Features(
428428
wildcardSSH = true,
429429
),
@@ -472,6 +472,19 @@ internal class CoderCLIManagerTest {
472472
}
473473
}
474474

475+
val inputConf =
476+
Path.of("src/test/fixtures/inputs/").resolve(it.remove + ".conf").toFile().readText()
477+
.replace(newlineRe, System.lineSeparator())
478+
.replace("/tmp/coder-gateway/test.coder.invalid/config", escape(coderConfigPath.toString()))
479+
.replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", escape(ccm.localBinaryPath.toString()))
480+
.let { conf ->
481+
if (it.sshLogDirectory != null) {
482+
conf.replace("/tmp/coder-gateway/test.coder.invalid/logs", it.sshLogDirectory.toString())
483+
} else {
484+
conf
485+
}
486+
}
487+
475488
// Add workspaces.
476489
ccm.configSsh(
477490
it.workspaces.flatMap { ws ->
@@ -496,8 +509,7 @@ internal class CoderCLIManagerTest {
496509
// Remove is the configuration we expect after removing.
497510
assertEquals(
498511
settings.sshConfigPath.toFile().readText(),
499-
Path.of("src/test/fixtures/inputs").resolve(it.remove + ".conf").toFile()
500-
.readText().replace(newlineRe, System.lineSeparator()),
512+
inputConf
501513
)
502514
}
503515
}

0 commit comments

Comments
 (0)