Rename ctrld v2.0.0 to ctrld-client

Updates #565

cmd/cli/cli.go

@@ -77,6 +77,16 @@ func isNoConfigStart(cmd *cobra.Command) bool {
 	return false
 }
 
+// cliName is this client's identity: the command name Cobra prints in help and
+// in "--version" output, the file the release pipeline publishes
+// (scripts/build.sh executable_name), and therefore the file the self-upgrader
+// downloads.
+//
+// Referenced rather than repeated because "--version" output is parsed as well as
+// printed: binaryVersion builds its expected prefix from this, and a literal on
+// either side would let a rename break rollback silently.
+const cliName = "ctrld-client"
+
 const rootShortDesc = `
         __         .__       .___
   _____/  |________|  |    __| _/
@@ -96,7 +106,7 @@ func curVersion() string {
 	// Return version directly if it's not empty and not a dev build
 	// This avoids unnecessary commit hash concatenation for release versions
 	if version != "" && version != "dev" {
-		return version
+		return displayVersion(version)
 	}
 	// Truncate commit hash to 7 characters for readability
 	// Git commit hashes are typically 40 characters, but 7 is sufficient for identification
@@ -106,6 +116,28 @@ func curVersion() string {
 	return fmt.Sprintf("%s-%s", version, commit)
 }
 
+// displayVersion converts the build tag into the version the client reports.
+//
+// master carries v2.x.x tags so its releases can be tracked alongside the v1.x.x
+// line still cut from the v1.0 branch, but the client presents itself as v1.x.x:
+// a v2.0.0 tag reads as v1.0.0, v2.3.1 as v1.3.1. Only master ever carries a
+// major >= 2, so the split tag spaces scope this to master by themselves and the
+// binary needs no build-time signal for which branch produced it - the v1.0
+// branch's tags have major 1 and pass through untouched, as does anything that is
+// not a semantic version ("dev", commit-suffixed builds).
+//
+// Minor, patch, prerelease and build metadata are preserved, so a v2.1.0-rc1 tag
+// stays a release candidate at v1.1.0-rc1 and isStableVersion still classifies it
+// the same way.
+func displayVersion(v string) string {
+	sv, err := semver.NewVersion(v)
+	if err != nil || sv.Major() < 2 {
+		return v
+	}
+	shifted := semver.New(sv.Major()-1, sv.Minor(), sv.Patch(), sv.Prerelease(), sv.Metadata())
+	return "v" + shifted.String()
+}
+
 func initCLI() *cobra.Command {
 	// Enable opening via explorer.exe on Windows.
 	// See: https://github.com/spf13/cobra/issues/844.
@@ -113,7 +145,7 @@ func initCLI() *cobra.Command {
 	cobra.EnableCommandSorting = false
 
 	rootCmd := &cobra.Command{
-		Use:     "ctrld",
+		Use:     cliName,
 		Short:   strings.TrimLeft(rootShortDesc, "\n"),
 		Version: appVersion,
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
@@ -2233,16 +2265,20 @@ func goArm() string {
 	return "5"
 }
 
-// upgradeUrl returns the url for downloading new ctrld binary.
+// upgradeUrl builds the download URL for this platform's binary.
+//
+// The path carries no version segment: the v1 line publishes as "ctrld" and this
+// one as "ctrld-client", so the file name distinguishes them and they can share
+// one path.
 func upgradeUrl(baseUrl string) string {
-	dlPath := fmt.Sprintf("v2/%s-%s/ctrld", runtime.GOOS, runtime.GOARCH)
+	dlPath := fmt.Sprintf("%s-%s/%s", runtime.GOOS, runtime.GOARCH, cliName)
 	// Use arm version set during build time, v5 binary can be run on higher arm version system.
 	if armVersion := goArm(); armVersion != "" {
-		dlPath = fmt.Sprintf("%s-%sv%s/ctrld", runtime.GOOS, runtime.GOARCH, armVersion)
+		dlPath = fmt.Sprintf("%s-%sv%s/%s", runtime.GOOS, runtime.GOARCH, armVersion, cliName)
 	}
 	// linux/amd64 has nocgo version, to support systems that missing some libc (like openwrt).
 	if !cgoEnabled && runtime.GOOS == "linux" && runtime.GOARCH == "amd64" {
-		dlPath = fmt.Sprintf("%s-%s-nocgo/ctrld", runtime.GOOS, runtime.GOARCH)
+		dlPath = fmt.Sprintf("%s-%s-nocgo/%s", runtime.GOOS, runtime.GOARCH, cliName)
 	}
 	dlUrl := fmt.Sprintf("%s/%s", baseUrl, dlPath)
 	if runtime.GOOS == "windows" {

cmd/cli/cli_test.go

@@ -44,3 +44,55 @@ func Test_isStableVersion(t *testing.T) {
 		})
 	}
 }
+
+// Test_displayVersion pins the tag-to-display transform. master carries v2.x.x
+// tags so its releases can be tracked next to the v1.x.x line still cut from the
+// v1.0 branch, and the client reports the tag with its major decremented.
+//
+// The v1.0-branch cases are the ones that make the split safe without a
+// build-time branch signal: a major of 1 has to pass through untouched, or the
+// v1.0 line would start reporting v0.x.x.
+func Test_displayVersion(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{"master tag", "v2.0.0", "v1.0.0"},
+		{"master tag with minor and patch", "v2.3.1", "v1.3.1"},
+		{"master prerelease keeps its suffix", "v2.1.0-rc1", "v1.1.0-rc1"},
+		{"master tag with build metadata", "v2.1.0+build.5", "v1.1.0+build.5"},
+		{"a later major still decrements by one", "v3.2.1", "v2.2.1"},
+		// v1.0 branch: untouched, which is what scopes the transform to master.
+		{"v1.0 branch tag", "v1.3.5", "v1.3.5"},
+		{"v1.0 branch prerelease", "v1.3.5-next", "v1.3.5-next"},
+		// Not semantic versions: dev and commit-suffixed builds pass through.
+		{"dev", "dev", "dev"},
+		{"dev with commit", "dev-abc1234", "dev-abc1234"},
+		{"empty", "", ""},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			if got := displayVersion(tc.in); got != tc.want {
+				t.Errorf("displayVersion(%q) = %q, want %q", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+// Test_displayVersionKeepsStabilityClassification guards the coupling between the
+// transform and isStableVersion, which selects the self-upgrade channel: a tag
+// must not change from prerelease to stable (or back) by being renumbered.
+func Test_displayVersionKeepsStabilityClassification(t *testing.T) {
+	for _, ver := range []string{"v2.0.0", "v2.1.0-rc1", "v1.3.5", "v1.3.5-next", "dev"} {
+		ver := ver
+		t.Run(ver, func(t *testing.T) {
+			t.Parallel()
+			if got, want := isStableVersion(displayVersion(ver)), isStableVersion(ver); got != want {
+				t.Errorf("isStableVersion(displayVersion(%q)) = %v, want %v", ver, got, want)
+			}
+		})
+	}
+}

cmd/cli/commands_service.go

@@ -70,7 +70,7 @@ func NewServiceCommand() *ServiceCommand {
 func (sc *ServiceCommand) createServiceConfig() *service.Config {
 	return &service.Config{
 		Name:        ctrldServiceName,
-		DisplayName: "Control-D Helper Service",
+		DisplayName: ctrldServiceDisplayName,
 		Description: "A highly configurable, multi-protocol DNS forwarding proxy",
 		Option:      service.KeyValue{},
 	}

cmd/cli/commands_service_manager.go

@@ -23,7 +23,7 @@ func NewServiceManager() (*ServiceManager, error) {
 	// Create a proper service configuration
 	svcConfig := &service.Config{
 		Name:        ctrldServiceName,
-		DisplayName: "Control-D Helper Service",
+		DisplayName: ctrldServiceDisplayName,
 		Description: "A highly configurable, multi-protocol DNS forwarding proxy",
 		Option:      service.KeyValue{},
 	}

cmd/cli/commands_test.go

@@ -15,7 +15,7 @@ func TestBasicCommandStructure(t *testing.T) {
 	rootCmd := initCLI()
 
 	// Test that root command has basic properties
-	assert.Equal(t, "ctrld", rootCmd.Use)
+	assert.Equal(t, "ctrld-client", rootCmd.Use)
 	assert.NotEmpty(t, rootCmd.Short, "Root command should have a short description")
 
 	// Test that root command has subcommands
@@ -46,14 +46,19 @@ func TestServiceCommandCreation(t *testing.T) {
 	config := sc.createServiceConfig()
 	require.NotNil(t, config, "Service config should be created")
 	assert.Equal(t, ctrldServiceName, config.Name)
-	assert.Equal(t, "Control-D Helper Service", config.DisplayName)
+	assert.Equal(t, ctrldServiceDisplayName, config.DisplayName)
+	// Windows requires service display names to be unique and rejects a second
+	// registration with ERROR_DUPLICATE_SERVICE_NAME. Reusing the v1 service's
+	// display name would make "ctrld-client start" unable to install on hosts
+	// that still have v1 installed, so pin that they stay distinct (#565).
+	assert.NotEqual(t, "Control-D Helper Service", config.DisplayName)
 	assert.Equal(t, "A highly configurable, multi-protocol DNS forwarding proxy", config.Description)
 }
 
 // TestServiceCommandSubCommands tests service command sub commands
 func TestServiceCommandSubCommands(t *testing.T) {
 	rootCmd := &cobra.Command{
-		Use:   "ctrld",
+		Use:   "ctrld-client",
 		Short: "DNS forwarding proxy",
 	}
 

cmd/cli/commands_upgrade.go

@@ -329,11 +329,11 @@ var binaryVersionFn = binaryVersion
 // answers "can this binary actually run on this host", which is what rollback needs
 // to know before making a file the installed ctrld.
 //
-// On Windows path is ctrld.exe_previous, whose extension is not in PATHEXT. That
+// On Windows path is ctrld-client.exe_previous, whose extension is not in PATHEXT. That
 // resolves because os/exec only falls back to appending PATHEXT entries when the path
 // has no extension at all (lp_windows.go findExecutable): with one present and the
 // file on disk, it is used as-is. A suffix that left no extension - renaming
-// oldBinSuffix such that the result is "ctrld_previous" - would break this probe with
+// oldBinSuffix such that the result is "ctrld-client_previous" - would break this probe with
 // "executable file not found in %PATH%", and rollback would then refuse to restore a
 // perfectly good binary.
 func binaryVersion(path string) (string, error) {
@@ -343,13 +343,32 @@ func binaryVersion(path string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("running %s --version: %w", path, err)
 	}
-	ver, found := strings.CutPrefix(strings.TrimSpace(string(out)), "ctrld version ")
-	if !found {
+	ver, ok := parseVersionOutput(string(out))
+	if !ok {
 		return "", fmt.Errorf("unexpected --version output from %s: %q", path, strings.TrimSpace(string(out)))
 	}
 	return ver, nil
 }
 
+// parseVersionOutput extracts the version from a binary's "--version" output.
+//
+// The expected prefix is built from cliName, which is also what the root command
+// is named, because Cobra renders "--version" as "<name> version <version>" from
+// that same name. Spelling the prefix out here instead would make a rename of the
+// client silently break this parser - and with it rollback, which refuses to
+// restore a previous binary whose version it cannot read. That failure mode is
+// the worst one this code has: the service is already stopped, so a wrongly
+// rejected previous binary leaves the host with no ctrld enforcement at all.
+func parseVersionOutput(out string) (string, bool) {
+	// Not CutPrefix's own return: on a miss it hands back the whole input, which a
+	// caller that forgot to check the bool would store as if it were a version.
+	ver, ok := strings.CutPrefix(strings.TrimSpace(out), cliName+" version ")
+	if !ok {
+		return "", false
+	}
+	return ver, true
+}
+
 // InitUpgradeCmd creates the upgrade command with proper logic
 func InitUpgradeCmd(rootCmd *cobra.Command) *cobra.Command {
 	upgradeCmd := &cobra.Command{

cmd/cli/commands_upgrade_test.go

@@ -1,9 +1,11 @@
 package cli
 
 import (
+	"bytes"
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -137,7 +139,7 @@ func TestRemoveBinaryWithRetry(t *testing.T) {
 
 func TestBinaryVersion(t *testing.T) {
 	t.Run("reports the version", func(t *testing.T) {
-		t.Setenv(envFakeVersionOutput, "ctrld version dev-94fbd3f")
+		t.Setenv(envFakeVersionOutput, cliName+" version dev-94fbd3f")
 		got, err := binaryVersion(os.Args[0])
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
@@ -287,3 +289,77 @@ func TestRollbackToPreviousBinaryAbortsWhenStopFails(t *testing.T) {
 		t.Errorf("binary was modified even though the stop failed: %v", err)
 	}
 }
+
+// TestVersionOutputParsesThroughRollbackProbe ties the "--version" output the root
+// command actually produces to the parser rollback reads it with.
+//
+// These are two halves of one contract that live in different files: Cobra renders
+// "<Use> version <Version>", and binaryVersion cuts a prefix off it. Renaming the
+// client moved the first half; if the second half had kept its literal, every
+// upgrade would have logged "unknown version" and - the part that matters -
+// rollbackToPreviousBinary would have judged a perfectly good previous binary
+// "not usable" and left the host stopped with the broken one installed.
+//
+// The version output is taken from the real root command rather than assembled
+// here, so a future change to the name, the template, or the parser has to keep
+// them agreeing.
+func TestVersionOutputParsesThroughRollbackProbe(t *testing.T) {
+	rootCmd := initCLI()
+	rootCmd.SetVersionTemplate(rootCmd.VersionTemplate())
+
+	var out bytes.Buffer
+	rootCmd.SetOut(&out)
+	rootCmd.SetErr(&out)
+	rootCmd.SetArgs([]string{"--version"})
+	if err := rootCmd.Execute(); err != nil {
+		t.Fatalf("running --version: %v", err)
+	}
+
+	got := out.String()
+	if strings.TrimSpace(got) == "" {
+		t.Fatal("--version produced no output")
+	}
+	ver, ok := parseVersionOutput(got)
+	if !ok {
+		t.Fatalf("the version probe cannot parse the root command's own --version output %q; "+
+			"rollback would reject a working previous binary as unusable", strings.TrimSpace(got))
+	}
+	if ver != appVersion {
+		t.Errorf("parsed version = %q, want %q", ver, appVersion)
+	}
+}
+
+// TestParseVersionOutput covers the shapes the probe must accept and reject. The
+// rejected ones are what a genuinely broken previous binary produces - the
+// incident's ctrld.exe_previous printed nothing at all - and rollback depends on
+// telling those apart from a healthy binary under a new name.
+func TestParseVersionOutput(t *testing.T) {
+	tests := []struct {
+		name string
+		out  string
+		want string
+		ok   bool
+	}{
+		{"current identity", cliName + " version v1.0.0", "v1.0.0", true},
+		{"trailing newline", cliName + " version v1.0.0\n", "v1.0.0", true},
+		{"dev build", cliName + " version dev-94fbd3f", "dev-94fbd3f", true},
+		// The pre-rename identity: a v1-line binary is not a valid rollback target
+		// for this client, and must not be read as one.
+		{"previous identity", "ctrld version v1.3.5", "", false},
+		{"no output", "", "", false},
+		{"unrelated output", "some other program", "", false},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			got, ok := parseVersionOutput(tc.out)
+			if ok != tc.ok {
+				t.Fatalf("parseVersionOutput(%q) ok = %v, want %v", tc.out, ok, tc.ok)
+			}
+			if got != tc.want {
+				t.Errorf("parseVersionOutput(%q) = %q, want %q", tc.out, got, tc.want)
+			}
+		})
+	}
+}

cmd/cli/library_retry_test.go

@@ -37,7 +37,7 @@ func (rt *denyingRoundTripper) RoundTrip(req *http.Request) (*http.Response, err
 
 func TestDoWithRetryPreservesHostnameError(t *testing.T) {
 	const hostname = "dl.controld.dev"
-	req, err := http.NewRequest(http.MethodGet, "https://"+hostname+"/v2/windows-amd64/ctrld.exe", nil)
+	req, err := http.NewRequest(http.MethodGet, "https://"+hostname+"/windows-amd64/ctrld-client.exe", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -219,7 +219,7 @@ func TestDoWithFallbackClassificationEndToEnd(t *testing.T) {
 // and not only in the hand-built shape.
 func TestDoWithRetryComposesHostnameAttemptFirst(t *testing.T) {
 	const hostname = "dl.controld.dev"
-	req, err := http.NewRequest(http.MethodGet, "https://"+hostname+"/v2/windows-amd64/ctrld.exe", nil)
+	req, err := http.NewRequest(http.MethodGet, "https://"+hostname+"/windows-amd64/ctrld-client.exe", nil)
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/cli/prog.go

@@ -48,7 +48,23 @@ const (
 	upstreamOS                 = upstreamPrefix + "os"
 	upstreamOSLocal            = upstreamOS + ".local"
 	dnsWatchdogDefaultInterval = 20 * time.Second
-	ctrldServiceName           = "ctrld"
+	ctrldServiceName           = "ctrld-client"
+	// ctrldServiceDisplayName must differ from the v1 service's display name
+	// ("Control-D Helper Service"). Windows requires display names to be unique
+	// across all installed services and fails registration with
+	// ERROR_DUPLICATE_SERVICE_NAME otherwise, so reusing v1's name would make
+	// "ctrld-client start" unable to install on any host that still has the v1
+	// service. It moves with ctrldServiceName: both identify this service.
+	ctrldServiceDisplayName = "Control-D Client Service"
+)
+
+// Service-manager paths derived from ctrldServiceName. Every init system names
+// its unit after the service identifier, so these must move with it: a rename
+// that missed one would leave ctrld managing a unit it no longer installs.
+const (
+	systemdUnitFile  = "/etc/systemd/system/" + ctrldServiceName + ".service"
+	sysVInitScript   = "/etc/init.d/" + ctrldServiceName
+	launchdPlistFile = "/Library/LaunchDaemons/" + ctrldServiceName + ".plist"
 )
 
 // RecoveryReason provides context for why we are waiting for recovery.

cmd/cli/service.go

@@ -41,7 +41,7 @@ type sysV struct {
 }
 
 func (s *sysV) installed() bool {
-	fi, err := os.Stat("/etc/init.d/ctrld")
+	fi, err := os.Stat(sysVInitScript)
 	if err != nil {
 		return false
 	}
@@ -53,7 +53,7 @@ func (s *sysV) Start() error {
 	if !s.installed() {
 		return service.ErrNotInstalled
 	}
-	_, err := exec.Command("/etc/init.d/ctrld", "start").CombinedOutput()
+	_, err := exec.Command(sysVInitScript, "start").CombinedOutput()
 	return err
 }
 
@@ -61,7 +61,7 @@ func (s *sysV) Stop() error {
 	if !s.installed() {
 		return service.ErrNotInstalled
 	}
-	_, err := exec.Command("/etc/init.d/ctrld", "stop").CombinedOutput()
+	_, err := exec.Command(sysVInitScript, "stop").CombinedOutput()
 	return err
 }
 
@@ -89,7 +89,7 @@ type systemd struct {
 }
 
 func (s *systemd) Status() (service.Status, error) {
-	out, _ := exec.Command("systemctl", "status", "ctrld").CombinedOutput()
+	out, _ := exec.Command("systemctl", "status", ctrldServiceName).CombinedOutput()
 	if bytes.Contains(out, []byte("/FAILURE)")) {
 		return service.StatusStopped, nil
 	}
@@ -97,7 +97,6 @@ func (s *systemd) Status() (service.Status, error) {
 }
 
 func (s *systemd) Start() error {
-	const systemdUnitFile = "/etc/systemd/system/ctrld.service"
 	f, err := os.Open(systemdUnitFile)
 	if err != nil {
 		return err
@@ -223,7 +222,7 @@ func checkHasElevatedPrivilege() {
 
 // unixSystemVServiceStatus checks the status of a Unix System V service
 func unixSystemVServiceStatus() (service.Status, error) {
-	out, err := exec.Command("/etc/init.d/ctrld", "status").CombinedOutput()
+	out, err := exec.Command(sysVInitScript, "status").CombinedOutput()
 	if err != nil {
 		return service.StatusUnknown, nil
 	}

cmd/cli/service_args_darwin.go

@@ -9,7 +9,7 @@ import (
 	"strings"
 )
 
-const launchdPlistPath = "/Library/LaunchDaemons/ctrld.plist"
+const launchdPlistPath = launchdPlistFile
 
 // serviceConfigFileExists returns true if the launchd plist for ctrld exists on disk.
 // This is more reliable than checking launchctl status, which may report "not found"
@@ -23,7 +23,7 @@ func serviceConfigFileExists() bool {
 // service's launch arguments. This is used when upgrading an existing installation
 // to intercept mode without losing the existing --cd flag and other arguments.
 //
-// On macOS, this modifies the launchd plist at /Library/LaunchDaemons/ctrld.plist
+// On macOS, this modifies the launchd plist named after ctrldServiceName
 // using PlistBuddy for exact array reads and writes.
 //
 // The function is idempotent: if the flag already exists, it's a no-op.

cmd/cli/service_args_others.go

@@ -14,11 +14,11 @@ var errServiceFlagsUnsupported = errors.New("modifying service flags is not supp
 // serviceConfigFileExists checks common service config file locations on Linux.
 func serviceConfigFileExists() bool {
 	// systemd unit file
-	if _, err := os.Stat("/etc/systemd/system/ctrld.service"); err == nil {
+	if _, err := os.Stat(systemdUnitFile); err == nil {
 		return true
 	}
 	// SysV init script
-	if _, err := os.Stat("/etc/init.d/ctrld"); err == nil {
+	if _, err := os.Stat(sysVInitScript); err == nil {
 		return true
 	}
 	return false

cmd/cli/service_image_path.go

@@ -6,7 +6,7 @@ import "strings"
 // ImagePath value, which carries the command line rather than a bare path: it may be
 // quoted and is usually followed by arguments, e.g.
 //
-//	"C:\Program Files\Control D\ctrld.exe" run --config C:\...\ctrld.toml
+//	"C:\Program Files\Control D\ctrld-client.exe" run --config C:\...\ctrld.toml
 //
 // It returns "" when no path can be read, which callers must treat as "cannot tell"
 // rather than "does not match".

cmd/cli/service_image_path_windows.go

@@ -11,7 +11,7 @@ import (
 // installedServiceDirMatches reports whether this executable is the installed service
 // binary, by comparing its directory with the one in the service's registered ImagePath.
 //
-// socketDir() on Windows is relative to the running executable, so a ctrld.exe run from
+// socketDir() on Windows is relative to the running executable, so a ctrld-client.exe run from
 // somewhere else - a download directory, a build tree - looks for the control socket in
 // its own directory and never finds the installed daemon's. A failed probe from there
 // says nothing about the service's health, and reporting "not ready" for it would tell