From 157164c3aa001e6173a4440c5f3746513bcbe0e7 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 22 Aug 2024 10:12:21 +0100 Subject: [PATCH 1/4] fix(cli): ensure that the support bundle command does not panic on zero values --- cli/support.go | 45 ++++++++++++++++++++++++++++++++++++--------- cli/support_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/cli/support.go b/cli/support.go index 5dfe7a45a151b..75e1a1719519c 100644 --- a/cli/support.go +++ b/cli/support.go @@ -184,16 +184,8 @@ func (r *RootCmd) supportBundle() *serpent.Command { _ = os.Remove(outputPath) // best effort return xerrors.Errorf("create support bundle: %w", err) } - docsURL := bun.Deployment.Config.Values.DocsURL.String() - deployHealthSummary := bun.Deployment.HealthReport.Summarize(docsURL) - if len(deployHealthSummary) > 0 { - cliui.Warn(inv.Stdout, "Deployment health issues detected:", deployHealthSummary...) - } - clientNetcheckSummary := bun.Network.Netcheck.Summarize("Client netcheck:", docsURL) - if len(clientNetcheckSummary) > 0 { - cliui.Warn(inv.Stdout, "Networking issues detected:", deployHealthSummary...) - } + summarizeBundle(inv, bun) bun.CLILogs = cliLogBuf.Bytes() if err := writeBundle(bun, zwr); err != nil { @@ -225,6 +217,41 @@ func (r *RootCmd) supportBundle() *serpent.Command { return cmd } +// summarizeBundle makes a best-effort attempt to write a short summary +// of the support bundle to the user's terminal. +func summarizeBundle(inv *serpent.Invocation, bun *support.Bundle) { + if bun == nil { + cliui.Error(inv.Stdout, "No support bundle generated!") + return + } + + if bun.Deployment.Config == nil { + cliui.Error(inv.Stdout, "No deployment configuration available!") + return + } + + docsURL := bun.Deployment.Config.Values.DocsURL.String() + if bun.Deployment.HealthReport == nil { + cliui.Error(inv.Stdout, "No deployment health report available!") + return + } + + deployHealthSummary := bun.Deployment.HealthReport.Summarize(docsURL) + if len(deployHealthSummary) > 0 { + cliui.Warn(inv.Stdout, "Deployment health issues detected:", deployHealthSummary...) + } + + if bun.Network.Netcheck == nil { + cliui.Error(inv.Stdout, "No network troubleshooting information available!") + return + } + + clientNetcheckSummary := bun.Network.Netcheck.Summarize("Client netcheck:", docsURL) + if len(clientNetcheckSummary) > 0 { + cliui.Warn(inv.Stdout, "Networking issues detected:", deployHealthSummary...) + } +} + func findAgent(agentName string, haystack []codersdk.WorkspaceResource) (*codersdk.WorkspaceAgent, bool) { for _, res := range haystack { for _, agt := range res.Agents { diff --git a/cli/support_test.go b/cli/support_test.go index d53aac66c820c..bc49ddb4f406b 100644 --- a/cli/support_test.go +++ b/cli/support_test.go @@ -5,6 +5,9 @@ import ( "bytes" "encoding/json" "io" + "net/http" + "net/http/httptest" + "net/url" "os" "path/filepath" "runtime" @@ -14,6 +17,7 @@ import ( "tailscale.com/ipn/ipnstate" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/agent" @@ -156,6 +160,42 @@ func TestSupportBundle(t *testing.T) { err := inv.Run() require.ErrorContains(t, err, "failed authorization check") }) + + // This ensures that the CLI does not panic when trying to generate a support bundle + // against a fake server that returns a 200 OK for all requests. This essentially + // ensures that (almost) all of the support bundle generating code paths get a zero value. + t.Run("DontPanic", func(t *testing.T) { + t.Parallel() + + // Start up a fake server that will return a blank 200 OK response for everything. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Logf("received request: %s %s", r.Method, r.URL) + switch r.URL.Path { + case "/api/v2/authcheck": + // Fake auth check + resp := codersdk.AuthorizationResponse{ + "Read DeploymentValues": true, + } + w.WriteHeader(http.StatusOK) + assert.NoError(t, json.NewEncoder(w).Encode(resp)) + default: + // Simply return a 200 OK for everything else. + w.WriteHeader(http.StatusOK) + } + })) + u, err := url.Parse(srv.URL) + require.NoError(t, err) + client := codersdk.New(u) + defer srv.Close() + + d := t.TempDir() + path := filepath.Join(d, "bundle.zip") + + inv, root := clitest.New(t, "support", "bundle", "--url-override", srv.URL, "--output-file", path, "--yes") + clitest.SetupConfig(t, client, root) + err = inv.Run() + require.NoError(t, err) + }) } // nolint:revive // It's a control flag, but this is just a test. From b10a361e97c8473a4370824ea8643162f8b249da Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 22 Aug 2024 10:51:36 +0100 Subject: [PATCH 2/4] fixup! fix(cli): ensure that the support bundle command does not panic on zero values --- cli/support_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/support_test.go b/cli/support_test.go index bc49ddb4f406b..3b02834a6da00 100644 --- a/cli/support_test.go +++ b/cli/support_test.go @@ -183,10 +183,10 @@ func TestSupportBundle(t *testing.T) { w.WriteHeader(http.StatusOK) } })) + defer srv.Close() u, err := url.Parse(srv.URL) require.NoError(t, err) client := codersdk.New(u) - defer srv.Close() d := t.TempDir() path := filepath.Join(d, "bundle.zip") From 99e396aabfa7a60e637b6b07da59147459eb0203 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 22 Aug 2024 11:04:57 +0100 Subject: [PATCH 3/4] Update cli/support.go Co-authored-by: Danny Kopping --- cli/support.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/support.go b/cli/support.go index 75e1a1719519c..fa7c58261bd41 100644 --- a/cli/support.go +++ b/cli/support.go @@ -235,7 +235,6 @@ func summarizeBundle(inv *serpent.Invocation, bun *support.Bundle) { cliui.Error(inv.Stdout, "No deployment health report available!") return } - deployHealthSummary := bun.Deployment.HealthReport.Summarize(docsURL) if len(deployHealthSummary) > 0 { cliui.Warn(inv.Stdout, "Deployment health issues detected:", deployHealthSummary...) From 7fac003b27e4f5426518ce8aaa5cda377b05dc2e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 22 Aug 2024 11:04:49 +0100 Subject: [PATCH 4/4] test against multiple status codes --- cli/support_test.go | 65 ++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/cli/support_test.go b/cli/support_test.go index 3b02834a6da00..6fe8f015c3f2b 100644 --- a/cli/support_test.go +++ b/cli/support_test.go @@ -162,39 +162,50 @@ func TestSupportBundle(t *testing.T) { }) // This ensures that the CLI does not panic when trying to generate a support bundle - // against a fake server that returns a 200 OK for all requests. This essentially + // against a fake server that returns an empty response for all requests. This essentially // ensures that (almost) all of the support bundle generating code paths get a zero value. t.Run("DontPanic", func(t *testing.T) { t.Parallel() - // Start up a fake server that will return a blank 200 OK response for everything. - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - t.Logf("received request: %s %s", r.Method, r.URL) - switch r.URL.Path { - case "/api/v2/authcheck": - // Fake auth check - resp := codersdk.AuthorizationResponse{ - "Read DeploymentValues": true, - } - w.WriteHeader(http.StatusOK) - assert.NoError(t, json.NewEncoder(w).Encode(resp)) - default: - // Simply return a 200 OK for everything else. - w.WriteHeader(http.StatusOK) - } - })) - defer srv.Close() - u, err := url.Parse(srv.URL) - require.NoError(t, err) - client := codersdk.New(u) + for _, code := range []int{ + http.StatusOK, + http.StatusUnauthorized, + http.StatusForbidden, + http.StatusNotFound, + http.StatusInternalServerError, + } { + t.Run(http.StatusText(code), func(t *testing.T) { + t.Parallel() + // Start up a fake server + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Logf("received request: %s %s", r.Method, r.URL) + switch r.URL.Path { + case "/api/v2/authcheck": + // Fake auth check + resp := codersdk.AuthorizationResponse{ + "Read DeploymentValues": true, + } + w.WriteHeader(http.StatusOK) + assert.NoError(t, json.NewEncoder(w).Encode(resp)) + default: + // Simply return a blank response for everything else. + w.WriteHeader(code) + } + })) + defer srv.Close() + u, err := url.Parse(srv.URL) + require.NoError(t, err) + client := codersdk.New(u) - d := t.TempDir() - path := filepath.Join(d, "bundle.zip") + d := t.TempDir() + path := filepath.Join(d, "bundle.zip") - inv, root := clitest.New(t, "support", "bundle", "--url-override", srv.URL, "--output-file", path, "--yes") - clitest.SetupConfig(t, client, root) - err = inv.Run() - require.NoError(t, err) + inv, root := clitest.New(t, "support", "bundle", "--url-override", srv.URL, "--output-file", path, "--yes") + clitest.SetupConfig(t, client, root) + err = inv.Run() + require.NoError(t, err) + }) + } }) }