Bypass download if CLI version matches

This is a bit faster than waiting for a 304 but more importantly it
works if the user customizes the download source to be a place that does
not have the same etag support that Coder does.

The test bypasses Windows since we would need to create an executable
there for the test.  We could split out the exec and the parsing
although I am not sure it is worth the noise.  The part most likely to
break between platforms is probably the exec, not the parsing?

Author: code-asher

src/main/kotlin/com/coder/gateway/sdk/CoderCLIManager.kt

@@ -2,6 +2,7 @@ package com.coder.gateway.sdk
 
 import com.coder.gateway.models.WorkspaceAgentModel
 import com.coder.gateway.views.steps.CoderWorkspacesStepView
+import com.google.gson.Gson
 import com.intellij.openapi.diagnostic.Logger
 import org.zeroturnaround.exec.ProcessExecutor
 import java.io.BufferedInputStream
@@ -277,10 +278,24 @@ class CoderCLIManager @JvmOverloads constructor(
     }
 
     /**
-     * Return the binary version.
+     * Version output from the CLI's version command.
      */
-    fun version(): String {
-        return exec("version")
+    private data class Version(
+        val version: String,
+    )
+
+    /**
+     * Return the binary version or null if it could not be determined.
+     */
+    fun version(): String? {
+        return try {
+            val raw = exec("version", "--output", "json")
+            val json = Gson().fromJson(raw, Version::class.java)
+            json.version
+        } catch (e: Exception) {
+            logger.warn("Unable to determine CLI version", e)
+            null
+        }
     }
 
     private fun exec(vararg args: String): String {

src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt

@@ -467,8 +467,17 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
                 appPropertiesService.setValue(CODER_URL_KEY, deploymentURL.toString())
                 appPropertiesService.setValue(SESSION_TOKEN, token.first)
 
-                this.indicator.text = "Downloading Coder CLI..."
-                cliManager.downloadCLI()
+                // Short-circuit if we already have the expected version.  This
+                // lets us bypass the 304 which is slower and may not be
+                // supported if the binary is downloaded from alternate sources.
+                // For CLIs without the JSON output flag we will fall back to
+                // the 304 method.
+                if (cliManager.version() != clientService.buildVersion) {
+                    this.indicator.text = "Downloading Coder CLI..."
+                    cliManager.downloadCLI()
+                } else {
+                    logger.info("Found existing binary at ${cliManager.localBinaryPath} with the expected version ${clientService.buildVersion}")
+                }
 
                 this.indicator.text = "Authenticating Coder CLI..."
                 cliManager.login(token.first)

src/test/groovy/CoderCLIManagerTest.groovy

@@ -1,14 +1,12 @@
 package com.coder.gateway.sdk
 
+
 import com.sun.net.httpserver.HttpExchange
 import com.sun.net.httpserver.HttpHandler
 import com.sun.net.httpserver.HttpServer
 import org.zeroturnaround.exec.InvalidExitValueException
 import org.zeroturnaround.exec.ProcessInitException
-import spock.lang.Requires
-import spock.lang.Shared
-import spock.lang.Specification
-import spock.lang.Unroll
+import spock.lang.*
 
 import java.nio.file.Files
 import java.nio.file.Path
@@ -137,7 +135,7 @@ class CoderCLIManagerTest extends Specification {
 
         then:
         downloaded
-        ccm.version().contains("Coder")
+        CoderSemVer.isValidVersion(ccm.version())
 
         // Make sure login failures propagate correctly.
         when:
@@ -161,7 +159,7 @@ class CoderCLIManagerTest extends Specification {
         // 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().text == "#!/bin/sh\necho '$url'"
+        ccm.localBinaryPath.toFile().text.contains(url)
 
         cleanup:
         srv.stop(0)
@@ -172,7 +170,7 @@ class CoderCLIManagerTest extends Specification {
         def ccm = new CoderCLIManager(new URL("https://foo"), tmpdir.resolve("does-not-exist"))
 
         when:
-        ccm.version()
+        ccm.login("token")
 
         then:
         thrown(ProcessInitException)
@@ -193,7 +191,7 @@ class CoderCLIManagerTest extends Specification {
         downloaded
         ccm.localBinaryPath.toFile().readBytes() != "cli".getBytes()
         ccm.localBinaryPath.toFile().lastModified() > 0
-        ccm.localBinaryPath.toFile().text == "#!/bin/sh\necho '$url'"
+        ccm.localBinaryPath.toFile().text.contains(url)
 
         cleanup:
         srv.stop(0)
@@ -207,6 +205,7 @@ class CoderCLIManagerTest extends Specification {
         when:
         def downloaded1 = ccm.downloadCLI()
         ccm.localBinaryPath.toFile().setLastModified(0)
+        // Download will be skipped due to a 304.
         def downloaded2 = ccm.downloadCLI()
 
         then:
@@ -231,8 +230,8 @@ class CoderCLIManagerTest extends Specification {
 
         then:
         ccm1.localBinaryPath != ccm2.localBinaryPath
-        ccm1.localBinaryPath.toFile().text == "#!/bin/sh\necho '$url1'"
-        ccm2.localBinaryPath.toFile().text == "#!/bin/sh\necho '$url2'"
+        ccm1.localBinaryPath.toFile().text.contains(url1)
+        ccm2.localBinaryPath.toFile().text.contains(url2)
 
         cleanup:
         srv1.stop(0)
@@ -249,7 +248,7 @@ class CoderCLIManagerTest extends Specification {
 
         then:
         downloaded
-        ccm.localBinaryPath.toFile().text == "#!/bin/sh\necho '${expected.replace("{{url}}", url)}'"
+        ccm.localBinaryPath.toFile().text.contains(expected.replace("{{url}}", url))
 
         cleanup:
         srv.stop(0)
@@ -429,4 +428,27 @@ class CoderCLIManagerTest extends Specification {
                 "malformed-start-after-end",
         ]
     }
+
+    @IgnoreIf({ os.windows })
+    def "parses version"() {
+        given:
+        def ccm = new CoderCLIManager(new URL("https://test.coder.invalid"), tmpdir)
+        Files.createDirectories(ccm.localBinaryPath.parent)
+
+        when:
+        ccm.localBinaryPath.toFile().text = "#!/bin/sh\n$contents"
+        ccm.localBinaryPath.toFile().setExecutable(true)
+
+        then:
+        ccm.version() == expected
+
+        where:
+        contents                                                 | expected
+        """echo '{"version": "1.0.0"}'"""                        | "1.0.0"
+        """echo '{"version": "1.0.0", "foo": true, "baz": 1}'""" | "1.0.0"
+        """echo '{"foo": true, "baz": 1}'"""                     | null
+        """echo '{"version: "1.0.0", "foo": true, "baz": 1}'"""  | null
+        "exit 0"                                                 | null
+        "exit 1"                                                 | null
+    }
 }