Repository: kopia/kopia
Pull request #3980 (2024-07-11 20:06:55)

fix(server): ensure repo disconnection on `server start` exit
Ensure repository disconnection at the end of the `server start` CLI command.
This was caught as a result of fixing the test below.

Fix `TestServerStartInsecure`:
Remove `--password=xxx` parameter, which causes a server start failure due to incorrect repo password, and not for the case being checked, which is the lack of the `--insecure` parameter.

Update test comments accordingly.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/kopia/kopia/pull/3980?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
Attention: Patch coverage is `60.00000%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 77.19%. Comparing base [(`cb455c6`)](https://app.codecov.io/gh/kopia/kopia/commit/cb455c699a04e54cb47e25a676d7d65063ce883e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) to head [(`d1b7a92`)](https://app.codecov.io/gh/kopia/kopia/commit/d1b7a92635fcd29aff92be538e1ccbc5df59ceb7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).
> Report is 188 commits behind head on master.

| [Files](https://app.codecov.io/gh/kopia/kopia/pull/3980?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) | Patch % | Lines |
|---|---|---|
| [cli/command\_server\_start.go](https://app.codecov.io/gh/kopia/kopia/pull/3980?src=pr&el=tree&filepath=cli%2Fcommand_server_start.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-Y2xpL2NvbW1hbmRfc2VydmVyX3N0YXJ0Lmdv) | 60.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/kopia/kopia/pull/3980?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #3980      +/-   ##
==========================================
+ Coverage   75.86%   77.19%   +1.33%     
==========================================
  Files         470      481      +11     
  Lines       37301    28806    -8495     
==========================================
- Hits        28299    22238    -6061     
+ Misses       7071     4667    -2404     
+ Partials     1931     1901      -30     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/kopia/kopia/pull/3980?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).


Patch:
diff --git a/cli/command_server_start.go b/cli/command_server_start.go
index d92c4d43..5e7211a5 100644
--- a/cli/command_server_start.go
+++ b/cli/command_server_start.go
@@ -4,6 +4,7 @@
 	"context"
 	"crypto/rand"
 	"encoding/hex"
+	stderrors "errors"
 	"fmt"
 	"io"
 	"net"
@@ -178,7 +179,7 @@ func (c *commandServerStart) initRepositoryPossiblyAsync(ctx context.Context, sr
 	return nil
 }
 
-func (c *commandServerStart) run(ctx context.Context) error {
+func (c *commandServerStart) run(ctx context.Context) (reterr error) {
 	opts, err := c.serverStartOptions(ctx)
 	if err != nil {
 		return err
@@ -193,6 +194,13 @@ func (c *commandServerStart) run(ctx context.Context) error {
 		return errors.Wrap(err, "unable to initialize repository")
 	}
 
+	defer func() {
+		// cleanup: disconnect repository
+		if err := srv.SetRepository(ctx, nil); err != nil {
+			reterr = stderrors.Join(reterr, errors.Wrap(err, "error disconnecting repository"))
+		}
+	}()
+
 	httpServer := &http.Server{
 		ReadHeaderTimeout: 15 * time.Second, //nolint:mnd
 		Addr:              stripProtocol(c.sf.serverAddress),
@@ -262,7 +270,7 @@ func (c *commandServerStart) run(ctx context.Context) error {
 		return err
 	}
 
-	return errors.Wrap(srv.SetRepository(ctx, nil), "error setting active repository")
+	return nil
 }
 
 func shutdownHTTPServer(ctx context.Context, httpServer *http.Server) {
diff --git a/internal/feature/feature.go b/internal/feature/feature.go
index 10f32763..e1788520 100644
--- a/internal/feature/feature.go
+++ b/internal/feature/feature.go
@@ -6,9 +6,6 @@
 	"fmt"
 )
 
-// Behavior specifies how kopia should behave if it encounters a Feature it does not understand.
-type Behavior string
-
 // IfNotUnderstood describes the behavior of Kopia when a required feature is not understood.
 type IfNotUnderstood struct {
 	Warn             bool   `json:"warn"`                       // warn instead of failing


Test patch:
diff --git a/cli/terminate_signal_test.go b/cli/terminate_signal_test.go
index 4813a0ea..5669f508 100644
--- a/cli/terminate_signal_test.go
+++ b/cli/terminate_signal_test.go
@@ -1,26 +1,23 @@
 package cli_test
 
 import (
-	"strings"
 	"syscall"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/kopia/kopia/internal/testutil"
 	"github.com/kopia/kopia/tests/testenv"
 )
 
-// Waits until the server advertises its address on the line.
-func serverStarted(line string) bool {
-	return !strings.HasPrefix(line, "SERVER ADDRESS: ")
-}
-
 func TestTerminate(t *testing.T) {
 	env := testenv.NewCLITest(t, testenv.RepoFormatNotImportant, testenv.NewExeRunner(t))
 
 	env.RunAndExpectSuccess(t, "repo", "create", "filesystem", "--path", env.RepoDir)
 
-	wait, interrupt := env.RunAndProcessStderrInt(t, serverStarted, "server", "start",
+	var sp testutil.ServerParameters
+
+	wait, interrupt := env.RunAndProcessStderrInt(t, sp.ProcessOutput, "server", "start",
 		"--address=localhost:0",
 		"--insecure")
 
diff --git a/tests/end_to_end_test/server_start_test.go b/tests/end_to_end_test/server_start_test.go
index f858bb6f..c22786da 100644
--- a/tests/end_to_end_test/server_start_test.go
+++ b/tests/end_to_end_test/server_start_test.go
@@ -529,12 +529,12 @@ func TestServerStartInsecure(t *testing.T) {
 
 	waitUntilServerStarted(ctx, t, cli)
 
-	// server fails to start without a password but with TLS.
-	e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--tls-generate-cert", "--without-password")
+	// server fails to start with --without-password when `--insecure` is not specified
+	e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--without-password")                        // without TLS
+	e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--without-password", "--tls-generate-cert") // with TLS
 
-	// server fails to start with TLS but without password.
-	e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--password=foo")
-	e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0", "--without-password")
+	// server fails to start when TLS is not configured and `--insecure` is not specified
+	e.RunAndExpectFailure(t, "server", "start", "--ui", "--address=localhost:0")
 }
 
 func verifyServerConnected(t *testing.T, cli *apiclient.KopiaAPIClient, want bool) *serverapi.StatusResponse {