From 1e995530c6f3420dcccaf965557cb75a4072b36e Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Thu, 3 Nov 2022 19:34:27 +0000 Subject: [PATCH 1/6] deployed-labeler: bump commit limit to 5 pages, 100 results --- plugins/deployed-labeler/README.md | 2 +- plugins/deployed-labeler/main.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/plugins/deployed-labeler/README.md b/plugins/deployed-labeler/README.md index def6195..083110b 100644 --- a/plugins/deployed-labeler/README.md +++ b/plugins/deployed-labeler/README.md @@ -7,7 +7,7 @@ Accepts `POST` requests, while requiring to parameters: - `commit`: Commit that has just been deployed to production. - `team`: Which team just deployed to production. -`deployed-labeler` will look for the last 100 commits of the repository's default branch, alongside their associated Pull Requests and labels. +`deployed-labeler` will look for the last 500 commits of the repository's default branch, alongside their associated Pull Requests and labels. ![image](https://user-images.githubusercontent.com/24193764/139254510-9f8ed8e1-e9ac-4177-b447-49932b804edd.png) diff --git a/plugins/deployed-labeler/main.go b/plugins/deployed-labeler/main.go index 3fadd1d..ab4ff0d 100644 --- a/plugins/deployed-labeler/main.go +++ b/plugins/deployed-labeler/main.go @@ -212,8 +212,12 @@ func (s *server) getMergedPRs(ctx context.Context, commitSHA string) ([]pullRequ var commits []commitNodes // we get 100 commits per page - // 3x100 = 300 in total - for i := 0; i < 3; i++ { + // 5x100 = 500 in total + // + // Note that this value is sensitive to the commit rate of the given repo and the interval at which teams deploy; + // if more than 500 commits are merged within a week and a given team only deploys on a weekly basis then some commits + // might not be labeled. + for i := 0; i < 5; i++ { err := s.gh.Query(ctx, &q, variables) if err != nil { s.log.WithError(err).Error("Error running query.") From 9a515253b31bd28662c053be612a773e6e3d9eea Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Thu, 3 Nov 2022 21:05:58 +0000 Subject: [PATCH 2/6] deployed-labeler: emit error messages from /deployed endpoint This commit generates user readable error messages when the /deployed endpoint is called without required parameters. --- plugins/deployed-labeler/main.go | 53 +++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/plugins/deployed-labeler/main.go b/plugins/deployed-labeler/main.go index ab4ff0d..eb652ba 100644 --- a/plugins/deployed-labeler/main.go +++ b/plugins/deployed-labeler/main.go @@ -90,27 +90,44 @@ func main() { } func (s *server) markDeployedPR(w http.ResponseWriter, req *http.Request) { - switch req.Method { - case "POST": - var commitSHA string - var team string - for k, v := range req.URL.Query() { - switch k { - case "commit": - commitSHA = v[0] - case "team": - team = v[0] - default: - s.log.Warnf("Unrecognized parameter received: %s", k) - } + var commitSHA string + var team string + for k, v := range req.URL.Query() { + switch k { + case "commit": + commitSHA = v[0] + case "team": + team = v[0] + default: + s.log.Warnf("Unrecognized parameter received: %s", k) } - if team == "" || commitSHA == "" { - w.WriteHeader(http.StatusPreconditionFailed) - w.Write([]byte(http.StatusText(http.StatusPreconditionFailed))) - s.log.Errorf("team and commit are required parameters. Team: %v, commit: %v", team, commitSHA) - break + } + if team == "" || commitSHA == "" { + + var preconditionFailed struct { + Errors []string } + preconditionFailed.Errors = make([]string, 0, 1) + preconditionFailed.Errors = append(preconditionFailed.Errors, fmt.Sprintf("team and commit are required parameters. Team: '%v', commit: '%v'", team, commitSHA)) + + s.log.WithFields( + logrus.Fields{ + "team": team, + "commit": commitSHA, + }, + ).Error("team and commit are required parameters") + + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.Header().Set("X-Content-Type-Options", "nosniff") + w.WriteHeader(http.StatusPreconditionFailed) + json.NewEncoder(w).Encode(preconditionFailed) + return + } + + switch req.Method { + case "POST": + var msg struct { DeployedPRs struct { Team []string `json:"team"` From fa3a71a88404beb34362ba6816338c5423d1a6ef Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Thu, 3 Nov 2022 23:38:24 +0000 Subject: [PATCH 3/6] deployed-labeler: add GET handler for deployed to indicate label status --- plugins/deployed-labeler/main.go | 46 ++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/plugins/deployed-labeler/main.go b/plugins/deployed-labeler/main.go index eb652ba..79d72e1 100644 --- a/plugins/deployed-labeler/main.go +++ b/plugins/deployed-labeler/main.go @@ -126,8 +126,21 @@ func (s *server) markDeployedPR(w http.ResponseWriter, req *http.Request) { } switch req.Method { - case "POST": + case "GET": + var msg struct { + PRs struct { + Labeled []string `json:"labeled"` + Unlabeled []string `json:"unlabeled"` + } + Errors []error `json:"errors"` + } + msg.PRs.Labeled, msg.PRs.Unlabeled, msg.Errors = s.handleGetUnmarkedPRs(req.Context(), commitSHA, team) + + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + enc.Encode(msg) + case "POST": var msg struct { DeployedPRs struct { Team []string `json:"team"` @@ -157,6 +170,15 @@ func (s *server) handleMarkDeployedPRs(ctx context.Context, commitSHA, team stri return s.updatePullRequests(prs, team) } +func (s *server) handleGetUnmarkedPRs(ctx context.Context, commitSHA, team string) (labeledPRs, unlabeledPRs []string, errors []error) { + prs, err := s.getMergedPRs(ctx, commitSHA) + if err != nil { + return nil, nil, []error{err} + } + + return s.findTeamPullRequests(prs, team) +} + const ( org = "gitpod-io" repo = "gitpod" @@ -204,7 +226,27 @@ func (s *server) updatePullRequests(prs []pullRequest, team string) (teamDeploye errs = append(errs, err) } } else { - s.log.Infof("PR %v already has label %v", pr.Number, labelDeployed) + s.log.Debugf("PR %v already has label %v", pr.Number, labelDeployed) + } + } + return +} + +// Find labeled and unlabeled pull requests for the given team. +func (s *server) findTeamPullRequests(prs []pullRequest, team string) (labeled, unlabeled []string, errs []error) { + for _, pr := range prs { + + lblTeam := teamLabel(team) + if _, belongs := pr.Labels[lblTeam]; !belongs { + s.log.Debugf("PR %v does not belong to %v, skipping it", pr.Number, team) + continue + } + + teamDeployedLabel := deployedLabel(team) + if _, hasLabel := pr.Labels[teamDeployedLabel]; !hasLabel { + unlabeled = append(unlabeled, pr.URL) + } else { + labeled = append(labeled, pr.URL) } } return From 56102d135dc1936454ea9ecbc7c18c0eeccde43c Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Thu, 3 Nov 2022 23:29:40 +0000 Subject: [PATCH 4/6] deployed-labeler: serialise errors as strings per https://github.com/golang/go/issues/5161 serializing an `error` object to json emits an empty object; this caused deployed-labeler to emit an empty object when erroring. This commit converts all errors to strings before returning to the user to provide more context. Signed-off-by: Tarun Pothulapati --- plugins/deployed-labeler/main.go | 54 ++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/plugins/deployed-labeler/main.go b/plugins/deployed-labeler/main.go index 79d72e1..1f773c1 100644 --- a/plugins/deployed-labeler/main.go +++ b/plugins/deployed-labeler/main.go @@ -92,25 +92,20 @@ func main() { func (s *server) markDeployedPR(w http.ResponseWriter, req *http.Request) { var commitSHA string var team string + for k, v := range req.URL.Query() { switch k { case "commit": commitSHA = v[0] case "team": team = v[0] + default: s.log.Warnf("Unrecognized parameter received: %s", k) } } - if team == "" || commitSHA == "" { - - var preconditionFailed struct { - Errors []string - } - - preconditionFailed.Errors = make([]string, 0, 1) - preconditionFailed.Errors = append(preconditionFailed.Errors, fmt.Sprintf("team and commit are required parameters. Team: '%v', commit: '%v'", team, commitSHA)) + if team == "" || commitSHA == "" { s.log.WithFields( logrus.Fields{ "team": team, @@ -118,13 +113,15 @@ func (s *server) markDeployedPR(w http.ResponseWriter, req *http.Request) { }, ).Error("team and commit are required parameters") - w.Header().Set("Content-Type", "application/json; charset=utf-8") - w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(http.StatusPreconditionFailed) - json.NewEncoder(w).Encode(preconditionFailed) + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + enc.Encode(map[string]string{ + "error": "team and commit are required parameters", + }) return } - + var err error switch req.Method { case "GET": var msg struct { @@ -132,23 +129,38 @@ func (s *server) markDeployedPR(w http.ResponseWriter, req *http.Request) { Labeled []string `json:"labeled"` Unlabeled []string `json:"unlabeled"` } - Errors []error `json:"errors"` + Error string `json:"error"` + } + + msg.PRs.Labeled, msg.PRs.Unlabeled, err = s.handleGetUnmarkedPRs(req.Context(), commitSHA, team) + if err != nil { + msg.Error = err.Error() + w.WriteHeader(http.StatusUnprocessableEntity) } - msg.PRs.Labeled, msg.PRs.Unlabeled, msg.Errors = s.handleGetUnmarkedPRs(req.Context(), commitSHA, team) enc := json.NewEncoder(w) enc.SetIndent("", " ") enc.Encode(msg) case "POST": + var errs []error var msg struct { DeployedPRs struct { Team []string `json:"team"` All []string `json:"all"` } `json:"deployedPRs"` - Errors []error `json:"errors"` + Errors []string `json:"errors"` + } + msg.DeployedPRs.Team, msg.DeployedPRs.All, errs = s.handleMarkDeployedPRs(req.Context(), commitSHA, team) + + msg.Errors = make([]string, 0, len(errs)) + for _, err := range errs { + msg.Errors = append(msg.Errors, err.Error()) + } + + if len(msg.Errors) > 0 { + w.WriteHeader(http.StatusUnprocessableEntity) } - msg.DeployedPRs.Team, msg.DeployedPRs.All, msg.Errors = s.handleMarkDeployedPRs(req.Context(), commitSHA, team) enc := json.NewEncoder(w) enc.SetIndent("", " ") @@ -170,13 +182,15 @@ func (s *server) handleMarkDeployedPRs(ctx context.Context, commitSHA, team stri return s.updatePullRequests(prs, team) } -func (s *server) handleGetUnmarkedPRs(ctx context.Context, commitSHA, team string) (labeledPRs, unlabeledPRs []string, errors []error) { +func (s *server) handleGetUnmarkedPRs(ctx context.Context, commitSHA, team string) (labeledPRs, unlabeledPRs []string, err error) { prs, err := s.getMergedPRs(ctx, commitSHA) if err != nil { - return nil, nil, []error{err} + return nil, nil, err } - return s.findTeamPullRequests(prs, team) + labeledPRs, unlabeledPRs = s.findTeamPullRequests(prs, team) + + return labeledPRs, unlabeledPRs, nil } const ( @@ -233,7 +247,7 @@ func (s *server) updatePullRequests(prs []pullRequest, team string) (teamDeploye } // Find labeled and unlabeled pull requests for the given team. -func (s *server) findTeamPullRequests(prs []pullRequest, team string) (labeled, unlabeled []string, errs []error) { +func (s *server) findTeamPullRequests(prs []pullRequest, team string) (labeled, unlabeled []string) { for _, pr := range prs { lblTeam := teamLabel(team) From 54fdfe87f4bf8acde987e6f9058d28e428a5abd5 Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Thu, 3 Nov 2022 23:48:42 +0000 Subject: [PATCH 5/6] update readme --- plugins/deployed-labeler/README.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/plugins/deployed-labeler/README.md b/plugins/deployed-labeler/README.md index 083110b..3bb578e 100644 --- a/plugins/deployed-labeler/README.md +++ b/plugins/deployed-labeler/README.md @@ -51,9 +51,15 @@ docker run \ And `curl` the endpoint -```sh -curl -XPOST "http://localhost:8080/deployed?commit=01f4897c5323433e7831ca948f7d340c3c762885&team=webapp" -``` +- Check for unlabeled pull requests: + ```sh + curl "http://localhost:8080/deployed?commit=01f4897c5323433e7831ca948f7d340c3c762885&team=webapp" + ``` + +- Upload pull request labels: + ```sh + curl -XPOST "http://localhost:8080/deployed?commit=01f4897c5323433e7831ca948f7d340c3c762885&team=webapp" + ``` ## Deploying From 11b2df5f21b80c83b5d3cf1084b4e1474bbc1c6b Mon Sep 17 00:00:00 2001 From: Adrien Thebo Date: Thu, 3 Nov 2022 23:37:22 +0000 Subject: [PATCH 6/6] deployed-labeler: expose log level CLI option, adjust log levels Signed-off-by: Tarun Pothulapati --- plugins/deployed-labeler/main.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/plugins/deployed-labeler/main.go b/plugins/deployed-labeler/main.go index 1f773c1..08607c6 100644 --- a/plugins/deployed-labeler/main.go +++ b/plugins/deployed-labeler/main.go @@ -28,6 +28,7 @@ type options struct { dryRun bool github prowflagutil.GitHubOptions hmacSecret string + logLevel string } func newOptions() *options { @@ -36,6 +37,7 @@ func newOptions() *options { fs.IntVar(&o.port, "port", 8080, "Port to listen to.") fs.BoolVar(&o.dryRun, "dry-run", false, "Dry run for testing (uses API tokens but does not mutate).") fs.StringVar(&o.hmacSecret, "hmac", "/etc/webhook/hmac", "Path to the file containing the GitHub HMAC secret.") + fs.StringVar(&o.logLevel, "log-level", "debug", "Application log level") for _, group := range []flagutil.OptionGroup{&o.github} { group.AddFlags(fs) @@ -56,7 +58,7 @@ func main() { o := newOptions() logrus.SetFormatter(&logrus.JSONFormatter{}) - logrus.SetLevel(logrus.DebugLevel) + logrus.ParseLevel(o.logLevel) log := logrus.StandardLogger().WithField("plugin", pluginName) secretAgent := &secret.Agent{} @@ -92,14 +94,12 @@ func main() { func (s *server) markDeployedPR(w http.ResponseWriter, req *http.Request) { var commitSHA string var team string - for k, v := range req.URL.Query() { switch k { case "commit": commitSHA = v[0] case "team": team = v[0] - default: s.log.Warnf("Unrecognized parameter received: %s", k) } @@ -240,7 +240,7 @@ func (s *server) updatePullRequests(prs []pullRequest, team string) (teamDeploye errs = append(errs, err) } } else { - s.log.Debugf("PR %v already has label %v", pr.Number, labelDeployed) + s.log.Infof("PR %v already has label %v", pr.Number, labelDeployed) } } return @@ -252,7 +252,7 @@ func (s *server) findTeamPullRequests(prs []pullRequest, team string) (labeled, lblTeam := teamLabel(team) if _, belongs := pr.Labels[lblTeam]; !belongs { - s.log.Debugf("PR %v does not belong to %v, skipping it", pr.Number, team) + s.log.Infof("PR %v does not belong to %v, skipping it", pr.Number, team) continue } @@ -316,6 +316,14 @@ func (s *server) getMergedPRs(ctx context.Context, commitSHA string) ([]pullRequ pr.Labels[string(lbl.Name)] = struct{}{} } res = append(res, pr) + + s.log.WithFields( + logrus.Fields{ + "Number": pr.Number, + "URL": pr.URL, + "Labels": pr.Labels, + }, + ).Info("Added pull request for commit %s", c) } }