From 1d008a0aad90f09e1d6134e70d0a3b36ecedd518 Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Fri, 1 Apr 2022 14:17:48 +0100 Subject: [PATCH 1/6] Run `go fmt` over the codebase --- logserver.go | 18 ++++++++---------- main.go | 2 +- submit.go | 14 ++++++-------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/logserver.go b/logserver.go index 5f10d32..235fcdd 100644 --- a/logserver.go +++ b/logserver.go @@ -69,7 +69,6 @@ func (f *logServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { serveFile(w, r, upath) } - func serveFile(w http.ResponseWriter, r *http.Request, path string) { d, err := os.Stat(path) if err != nil { @@ -144,9 +143,9 @@ func serveDirectory(w http.ResponseWriter, r *http.Request, path string) { } // Streams a dynamically created tar.gz file with the contents of the given directory -// Will serve a partial, corrupted response if there is a error partway through the +// Will serve a partial, corrupted response if there is a error partway through the // operation as we stream the response. -// +// // The resultant tarball will contain a single directory containing all the files // so it can unpack cleanly without overwriting other files. // @@ -163,14 +162,14 @@ func serveTarball(w http.ResponseWriter, r *http.Request, dir string) error { // and removes leading and trailing `/` and replaces internal `/` with `_` // to form a suitable filename for use in the content-disposition header // dfilename would turn into `2022-01-10_184843-BZZXEGYH` - dfilename := strings.Trim(r.URL.Path,"/") - dfilename = strings.Replace(dfilename, "/","_",-1) + dfilename := strings.Trim(r.URL.Path, "/") + dfilename = strings.Replace(dfilename, "/", "_", -1) - // There is no application/tgz or similar; return a gzip file as best option. - // This tends to trigger archive type tools, which will then use the filename to + // There is no application/tgz or similar; return a gzip file as best option. + // This tends to trigger archive type tools, which will then use the filename to // identify the contents correctly. w.Header().Set("Content-Type", "application/gzip") - w.Header().Set("Content-Disposition", "attachment; filename=" + dfilename + ".tar.gz") + w.Header().Set("Content-Disposition", "attachment; filename="+dfilename+".tar.gz") files, err := directory.Readdir(-1) if err != nil { @@ -182,7 +181,6 @@ func serveTarball(w http.ResponseWriter, r *http.Request, dir string) error { targz := tar.NewWriter(gzip) defer targz.Close() - for _, file := range files { if file.IsDir() { // We avoid including nested directories @@ -206,7 +204,7 @@ func serveTarball(w http.ResponseWriter, r *http.Request, dir string) error { return nil } -// Add a single file into the archive. +// Add a single file into the archive. func addToArchive(targz *tar.Writer, dfilename string, filename string) error { file, err := os.Open(filename) if err != nil { diff --git a/main.go b/main.go index 52ff5db..65a300d 100644 --- a/main.go +++ b/main.go @@ -180,7 +180,7 @@ func main() { log.Fatal(http.ListenAndServe(*bindAddr, nil)) } -func configureGenericWebhookClient(cfg *config) (*http.Client) { +func configureGenericWebhookClient(cfg *config) *http.Client { if len(cfg.GenericWebhookURLs) == 0 { fmt.Println("No generic_webhook_urls configured.") return nil diff --git a/submit.go b/submit.go index c6bb669..7ebda8c 100644 --- a/submit.go +++ b/submit.go @@ -58,7 +58,7 @@ type submitServer struct { slack *slackClient genericWebhookClient *http.Client - cfg *config + cfg *config } // the type of payload which can be uploaded as JSON to the submit endpoint @@ -77,11 +77,10 @@ type jsonLogEntry struct { Lines string `json:"lines"` } - type genericWebhookPayload struct { parsedPayload - ReportURL string `json:"report_url"` - ListingURL string `json:"listing_url"` + ReportURL string `json:"report_url"` + ListingURL string `json:"listing_url"` } // the payload after parsing @@ -509,7 +508,7 @@ func (s *submitServer) saveReport(ctx context.Context, p parsedPayload, reportDi // submitGenericWebhook submits a basic JSON body to an endpoint configured in the config // -// The request does not include the log body, only the metadata in the parsedPayload, +// The request does not include the log body, only the metadata in the parsedPayload, // with the required listingURL to obtain the logs over http if required. // // If a github or gitlab issue was previously made, the reportURL will also be passed. @@ -523,8 +522,8 @@ func (s *submitServer) submitGenericWebhook(p parsedPayload, listingURL string, } genericHookPayload := genericWebhookPayload{ parsedPayload: p, - ReportURL: reportURL, - ListingURL: listingURL, + ReportURL: reportURL, + ListingURL: listingURL, } for _, url := range s.cfg.GenericWebhookURLs { // Enrich the parsedPayload with a reportURL and listingURL, to convert a single struct @@ -554,7 +553,6 @@ func (s *submitServer) sendGenericWebhook(req *http.Request) { } } - func (s *submitServer) submitGithubIssue(ctx context.Context, p parsedPayload, listingURL string, resp *submitResponse) error { if s.ghClient == nil { return nil From 7cb7309097245d4c038c449c695a770ca5cd3bd1 Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Fri, 1 Apr 2022 14:17:59 +0100 Subject: [PATCH 2/6] Add ID field containing the prefix used to create the listing. This listing needs to be unique for the rageshake to be stored to disk, so can be relied upon in future to be unique(enough) for other services working with the rageshake. --- submit.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/submit.go b/submit.go index 7ebda8c..df6d84e 100644 --- a/submit.go +++ b/submit.go @@ -85,6 +85,7 @@ type genericWebhookPayload struct { // the payload after parsing type parsedPayload struct { + ID string `json:"id"` UserText string `json:"user_text"` AppName string `json:"app"` Data map[string]string `json:"data"` @@ -178,6 +179,11 @@ func (s *submitServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } + // We use this prefix (eg, 2022-05-01/125223-abcde) as a unique identifier for this rageshake. + // This is going to be used to uniquely identify rageshakes, even if they are not submitted to + // an issue tracker for instance with automatic rageshakes that can be plentiful + p.ID = prefix + resp, err := s.saveReport(req.Context(), *p, reportDir, listingURL) if err != nil { log.Println("Error handling report submission:", err) From 8cdcc4bba8e37925466fa2f059f9821b7c19505e Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 6 Apr 2022 15:59:09 +0100 Subject: [PATCH 3/6] Add changelog entry for unique ID --- changelog.d/54.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/54.feature diff --git a/changelog.d/54.feature b/changelog.d/54.feature new file mode 100644 index 0000000..c4ce489 --- /dev/null +++ b/changelog.d/54.feature @@ -0,0 +1 @@ +Pass the prefix as a unique ID for the rageshake to the generic webhook mechanism. From 62e52e880d7635b2cbc22a6db4954c9ba6a8d1a6 Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 13 Apr 2022 13:21:46 +0100 Subject: [PATCH 4/6] Rename parsedPayload into just "payload", it's not just about parsing any more. --- submit.go | 48 ++++++++++++++++++++++++------------------------ submit_test.go | 8 ++++---- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/submit.go b/submit.go index df6d84e..0d27a33 100644 --- a/submit.go +++ b/submit.go @@ -78,13 +78,13 @@ type jsonLogEntry struct { } type genericWebhookPayload struct { - parsedPayload + payload ReportURL string `json:"report_url"` ListingURL string `json:"listing_url"` } // the payload after parsing -type parsedPayload struct { +type payload struct { ID string `json:"id"` UserText string `json:"user_text"` AppName string `json:"app"` @@ -96,7 +96,7 @@ type parsedPayload struct { FileErrors []string `json:"fileErrors"` } -func (p parsedPayload) WriteTo(out io.Writer) { +func (p payload) WriteTo(out io.Writer) { fmt.Fprintf( out, "%s\n\nNumber of logs: %d\nApplication: %s\n", @@ -198,7 +198,7 @@ func (s *submitServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { // parseRequest attempts to parse a received request as a bug report. If // the request cannot be parsed, it responds with an error and returns nil. -func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *parsedPayload { +func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *payload { length, err := strconv.Atoi(req.Header.Get("Content-Length")) if err != nil { log.Println("Couldn't parse content-length", err) @@ -234,13 +234,13 @@ func parseRequest(w http.ResponseWriter, req *http.Request, reportDir string) *p return p } -func parseJSONRequest(w http.ResponseWriter, req *http.Request, reportDir string) (*parsedPayload, error) { +func parseJSONRequest(w http.ResponseWriter, req *http.Request, reportDir string) (*payload, error) { var p jsonPayload if err := json.NewDecoder(req.Body).Decode(&p); err != nil { return nil, err } - parsed := parsedPayload{ + parsed := payload{ UserText: strings.TrimSpace(p.Text), Data: make(map[string]string), Labels: p.Labels, @@ -295,13 +295,13 @@ func parseJSONRequest(w http.ResponseWriter, req *http.Request, reportDir string return &parsed, nil } -func parseMultipartRequest(w http.ResponseWriter, req *http.Request, reportDir string) (*parsedPayload, error) { +func parseMultipartRequest(w http.ResponseWriter, req *http.Request, reportDir string) (*payload, error) { rdr, err := req.MultipartReader() if err != nil { return nil, err } - p := parsedPayload{ + p := payload{ Data: make(map[string]string), } @@ -320,7 +320,7 @@ func parseMultipartRequest(w http.ResponseWriter, req *http.Request, reportDir s return &p, nil } -func parseFormPart(part *multipart.Part, p *parsedPayload, reportDir string) error { +func parseFormPart(part *multipart.Part, p *payload, reportDir string) error { defer part.Close() field := part.FormName() partName := part.FileName() @@ -381,7 +381,7 @@ func parseFormPart(part *multipart.Part, p *parsedPayload, reportDir string) err // formPartToPayload updates the relevant part of *p from a name/value pair // read from the form data. -func formPartToPayload(field, data string, p *parsedPayload) { +func formPartToPayload(field, data string, p *payload) { if field == "text" { p.UserText = data } else if field == "app" { @@ -481,7 +481,7 @@ func saveLogPart(logNum int, filename string, reader io.Reader, reportDir string return leafName, nil } -func (s *submitServer) saveReport(ctx context.Context, p parsedPayload, reportDir, listingURL string) (*submitResponse, error) { +func (s *submitServer) saveReport(ctx context.Context, p payload, reportDir, listingURL string) (*submitResponse, error) { var summaryBuf bytes.Buffer resp := submitResponse{} p.WriteTo(&summaryBuf) @@ -514,7 +514,7 @@ func (s *submitServer) saveReport(ctx context.Context, p parsedPayload, reportDi // submitGenericWebhook submits a basic JSON body to an endpoint configured in the config // -// The request does not include the log body, only the metadata in the parsedPayload, +// The request does not include the log body, only the metadata in the payload, // with the required listingURL to obtain the logs over http if required. // // If a github or gitlab issue was previously made, the reportURL will also be passed. @@ -522,17 +522,17 @@ func (s *submitServer) saveReport(ctx context.Context, p parsedPayload, reportDi // Uses a goroutine to handle the http request asynchronously as by this point all critical // information has been stored. -func (s *submitServer) submitGenericWebhook(p parsedPayload, listingURL string, reportURL string) error { +func (s *submitServer) submitGenericWebhook(p payload, listingURL string, reportURL string) error { if s.genericWebhookClient == nil { return nil } genericHookPayload := genericWebhookPayload{ - parsedPayload: p, + payload: p, ReportURL: reportURL, ListingURL: listingURL, } for _, url := range s.cfg.GenericWebhookURLs { - // Enrich the parsedPayload with a reportURL and listingURL, to convert a single struct + // Enrich the payload with a reportURL and listingURL, to convert a single struct // to JSON easily payloadBuffer := new(bytes.Buffer) @@ -559,7 +559,7 @@ func (s *submitServer) sendGenericWebhook(req *http.Request) { } } -func (s *submitServer) submitGithubIssue(ctx context.Context, p parsedPayload, listingURL string, resp *submitResponse) error { +func (s *submitServer) submitGithubIssue(ctx context.Context, p payload, listingURL string, resp *submitResponse) error { if s.ghClient == nil { return nil } @@ -590,7 +590,7 @@ func (s *submitServer) submitGithubIssue(ctx context.Context, p parsedPayload, l return nil } -func (s *submitServer) submitGitlabIssue(p parsedPayload, listingURL string, resp *submitResponse) error { +func (s *submitServer) submitGitlabIssue(p payload, listingURL string, resp *submitResponse) error { if s.glClient == nil { return nil } @@ -613,7 +613,7 @@ func (s *submitServer) submitGitlabIssue(p parsedPayload, listingURL string, res return nil } -func (s *submitServer) submitSlackNotification(p parsedPayload, listingURL string) error { +func (s *submitServer) submitSlackNotification(p payload, listingURL string) error { if s.slack == nil { return nil } @@ -631,7 +631,7 @@ func (s *submitServer) submitSlackNotification(p parsedPayload, listingURL strin return nil } -func buildReportTitle(p parsedPayload) string { +func buildReportTitle(p payload) string { // set the title to the first (non-empty) line of the user's report, if any trimmedUserText := strings.TrimSpace(p.UserText) if trimmedUserText == "" { @@ -645,7 +645,7 @@ func buildReportTitle(p parsedPayload) string { return trimmedUserText } -func buildReportBody(p parsedPayload, newline, quoteChar string) *bytes.Buffer { +func buildReportBody(p payload, newline, quoteChar string) *bytes.Buffer { var bodyBuf bytes.Buffer fmt.Fprintf(&bodyBuf, "User message:\n\n%s\n\n", p.UserText) var dataKeys []string @@ -661,7 +661,7 @@ func buildReportBody(p parsedPayload, newline, quoteChar string) *bytes.Buffer { return &bodyBuf } -func buildGenericIssueRequest(p parsedPayload, listingURL string) (title, body string) { +func buildGenericIssueRequest(p payload, listingURL string) (title, body string) { bodyBuf := buildReportBody(p, " \n", "`") // Add log links to the body @@ -683,7 +683,7 @@ func buildGenericIssueRequest(p parsedPayload, listingURL string) (title, body s return } -func buildGithubIssueRequest(p parsedPayload, listingURL string) github.IssueRequest { +func buildGithubIssueRequest(p payload, listingURL string) github.IssueRequest { title, body := buildGenericIssueRequest(p, listingURL) labels := p.Labels @@ -698,7 +698,7 @@ func buildGithubIssueRequest(p parsedPayload, listingURL string) github.IssueReq } } -func buildGitlabIssueRequest(p parsedPayload, listingURL string, labels []string, confidential bool) *gitlab.CreateIssueOptions { +func buildGitlabIssueRequest(p payload, listingURL string, labels []string, confidential bool) *gitlab.CreateIssueOptions { title, body := buildGenericIssueRequest(p, listingURL) if p.Labels != nil { @@ -713,7 +713,7 @@ func buildGitlabIssueRequest(p parsedPayload, listingURL string, labels []string } } -func (s *submitServer) sendEmail(p parsedPayload, reportDir string) error { +func (s *submitServer) sendEmail(p payload, reportDir string) error { if len(s.cfg.EmailAddresses) == 0 { return nil } diff --git a/submit_test.go b/submit_test.go index e19be5f..e0f15b8 100644 --- a/submit_test.go +++ b/submit_test.go @@ -35,7 +35,7 @@ import ( // // if tempDir is empty, a new temp dir is created, and deleted when the test // completes. -func testParsePayload(t *testing.T, body, contentType string, tempDir string) (*parsedPayload, *http.Response) { +func testParsePayload(t *testing.T, body, contentType string, tempDir string) (*payload, *http.Response) { req, err := http.NewRequest("POST", "/api/submit", strings.NewReader(body)) if err != nil { t.Fatal(err) @@ -232,7 +232,7 @@ Content-Type: application/octet-stream return } -func checkParsedMultipartUpload(t *testing.T, p *parsedPayload) { +func checkParsedMultipartUpload(t *testing.T, p *payload) { wanted := "test words." if p.UserText != wanted { t.Errorf("User text: got %s, want %s", p.UserText, wanted) @@ -478,7 +478,7 @@ user_id: id } var buf bytes.Buffer for _, v := range sample { - p := parsedPayload{Data: v.data} + p := payload{Data: v.data} buf.Reset() p.WriteTo(&buf) got := strings.TrimSpace(buf.String()) @@ -488,7 +488,7 @@ user_id: id } for k, v := range sample { - p := parsedPayload{Data: v.data} + p := payload{Data: v.data} res := buildGithubIssueRequest(p, "") got := *res.Body if k == 0 { From 79454de54ff961e135494182f8278023b33f7496 Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 13 Apr 2022 13:28:17 +0100 Subject: [PATCH 5/6] Add a comment about each part of the payload. --- submit.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/submit.go b/submit.go index 0d27a33..f01162b 100644 --- a/submit.go +++ b/submit.go @@ -77,23 +77,24 @@ type jsonLogEntry struct { Lines string `json:"lines"` } +// Stores additional information created during processing of a payload type genericWebhookPayload struct { payload - ReportURL string `json:"report_url"` - ListingURL string `json:"listing_url"` + ReportURL string `json:"report_url"` // If a github/gitlab report is generated, this is set. + ListingURL string `json:"listing_url"` // Complete link to the listing URL that contains all uploaded logs } -// the payload after parsing +// Stores information about a request made to this server type payload struct { - ID string `json:"id"` - UserText string `json:"user_text"` - AppName string `json:"app"` - Data map[string]string `json:"data"` - Labels []string `json:"labels"` - Logs []string `json:"logs"` - LogErrors []string `json:"logErrors"` - Files []string `json:"files"` - FileErrors []string `json:"fileErrors"` + ID string `json:"id"` // A unique ID for this payload, generated within this server + UserText string `json:"user_text"` // A multi-line string containing the user description of the fault. + AppName string `json:"app"` // A short slug to identify the app making the report + Data map[string]string `json:"data"` // Arbitrary data to annotate the report + Labels []string `json:"labels"` // Short labels to group reports + Logs []string `json:"logs"` // A list of names of logs recognised by the server + LogErrors []string `json:"logErrors"` // Set if there are log parsing errors + Files []string `json:"files"` // A list of other files (not logs) uploaded as part of the rageshake + FileErrors []string `json:"fileErrors"` // Set if there are file parsing errors } func (p payload) WriteTo(out io.Writer) { From 2ac7af0c18ad4d50a0fae5134d71851d7a792bba Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Wed, 13 Apr 2022 13:31:54 +0100 Subject: [PATCH 6/6] Move comments onto their own line --- submit.go | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/submit.go b/submit.go index f01162b..9b875da 100644 --- a/submit.go +++ b/submit.go @@ -80,21 +80,32 @@ type jsonLogEntry struct { // Stores additional information created during processing of a payload type genericWebhookPayload struct { payload - ReportURL string `json:"report_url"` // If a github/gitlab report is generated, this is set. - ListingURL string `json:"listing_url"` // Complete link to the listing URL that contains all uploaded logs + // If a github/gitlab report is generated, this is set. + ReportURL string `json:"report_url"` + // Complete link to the listing URL that contains all uploaded logs + ListingURL string `json:"listing_url"` } // Stores information about a request made to this server type payload struct { - ID string `json:"id"` // A unique ID for this payload, generated within this server - UserText string `json:"user_text"` // A multi-line string containing the user description of the fault. - AppName string `json:"app"` // A short slug to identify the app making the report - Data map[string]string `json:"data"` // Arbitrary data to annotate the report - Labels []string `json:"labels"` // Short labels to group reports - Logs []string `json:"logs"` // A list of names of logs recognised by the server - LogErrors []string `json:"logErrors"` // Set if there are log parsing errors - Files []string `json:"files"` // A list of other files (not logs) uploaded as part of the rageshake - FileErrors []string `json:"fileErrors"` // Set if there are file parsing errors + // A unique ID for this payload, generated within this server + ID string `json:"id"` + // A multi-line string containing the user description of the fault. + UserText string `json:"user_text"` + // A short slug to identify the app making the report + AppName string `json:"app"` + // Arbitrary data to annotate the report + Data map[string]string `json:"data"` + // Short labels to group reports + Labels []string `json:"labels"` + // A list of names of logs recognised by the server + Logs []string `json:"logs"` + // Set if there are log parsing errors + LogErrors []string `json:"logErrors"` + // A list of other files (not logs) uploaded as part of the rageshake + Files []string `json:"files"` + // Set if there are file parsing errors + FileErrors []string `json:"fileErrors"` } func (p payload) WriteTo(out io.Writer) {