Skip to content

prevent unintended early return from skipping writing wildcard configs #539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt
Original file line number Diff line number Diff line change
@@ -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
}
}
}
97 changes: 47 additions & 50 deletions src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt
Original file line number Diff line number Diff line change
@@ -257,7 +257,6 @@
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 @@
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 @@
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 @@
*
* 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,

Check notice on line 487 in src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt

GitHub Actions / Qodana Community for JVM

String concatenation that can be converted to string template

'String' concatenation can be converted to a template
)

private fun exec(vararg args: String): String {
val stdout =
@@ -518,8 +518,7 @@
/*
* 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()}"

Check notice on line 521 in src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt

GitHub Actions / Qodana Community for JVM

Class member can have 'private' visibility

Function 'getHostPrefix' could be private

/**
* This function returns the ssh host name generated for connecting to the workspace.
@@ -528,29 +527,27 @@
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)
@@ -577,7 +574,7 @@
}
// non-wildcard case
if (parts[0] == "coder-jetbrains") {
return hostname + "--bg"

Check notice on line 577 in src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt

GitHub Actions / Qodana Community for JVM

String concatenation that can be converted to string template

'String' concatenation can be converted to a template
}
// wildcard case
parts[0] += "-bg"
83 changes: 41 additions & 42 deletions src/main/kotlin/com/coder/gateway/icons/CoderIcons.kt
Original file line number Diff line number Diff line change
@@ -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) {
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 17 additions & 0 deletions src/test/fixtures/inputs/wildcard.conf
Original file line number Diff line number Diff line change
@@ -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
18 changes: 15 additions & 3 deletions src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt
Original file line number Diff line number Diff line change
@@ -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
)
}
}

Unchanged files with check annotations Beta

init {
init()
title = CoderGatewayBundle.message("gateway.connector.view.coder.remoteproject.choose.text", CoderCLIManager.getWorkspaceParts(state.workspace, state.agent))

Check warning on line 41 in src/main/kotlin/com/coder/gateway/util/Dialogs.kt

GitHub Actions / Qodana Community for JVM

Incorrect string capitalization

String 'Choose IDE and project for workspace {0}' is not properly capitalized. It should have title capitalization
}
override fun show() {
)
// Check the provided setting to see if there's a default IDE to set.
val defaultIde = ides.find { it ->

Check notice on line 276 in src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspaceProjectIDEStepView.kt

GitHub Actions / Qodana Community for JVM

Redundant lambda arrow

Redundant lambda arrow
// Using contains on the displayable version of the ide means they can be as specific or as vague as they want
// CL 2023.3.6 233.15619.8 -> a specific Clion build
// CL 2023.3.6 -> a specific Clion version
}
private class WorkspaceVersionColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentListModel, String>(columnName) {
override fun valueOf(workspace: WorkspaceAgentListModel?): String? = if (workspace == null) {

Check warning on line 913 in src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt

GitHub Actions / Qodana Community for JVM

Redundant nullable return type

'valueOf' always returns non-null type
"Unknown"
} else if (workspace.workspace.outdated) {
"Outdated"