diff --git a/.github/workflows/labeler-build.yml b/.github/workflows/labeler-build.yml index d7af15a4..88deee57 100644 --- a/.github/workflows/labeler-build.yml +++ b/.github/workflows/labeler-build.yml @@ -10,11 +10,13 @@ on: push: branches: [main] paths: + - '.github/labels.yaml' - 'utilities/labeler/Dockerfile*' - 'utilities/labeler/**' pull_request: branches: [main] paths: + - '.github/labels.yaml' - 'utilities/labeler/Dockerfile*' - 'utilities/labeler/**' workflow_dispatch: @@ -45,6 +47,12 @@ jobs: with: persist-credentials: false + - name: Test labeler + working-directory: ./utilities/labeler + run: | + go test -v + go vet ./... + # Install the cosign tool except on PR # https://github.com/sigstore/cosign-installer - name: Install cosign diff --git a/utilities/labeler/Dockerfile b/utilities/labeler/Dockerfile index dd5a9021..53b96055 100644 --- a/utilities/labeler/Dockerfile +++ b/utilities/labeler/Dockerfile @@ -1,5 +1,5 @@ # Use the official Golang image as a base image -FROM golang:1.26.6 AS builder +FROM golang:1.26.1 AS builder # Set the working directory WORKDIR /app diff --git a/utilities/labeler/README.md b/utilities/labeler/README.md index 92ee52d3..390a0482 100644 --- a/utilities/labeler/README.md +++ b/utilities/labeler/README.md @@ -11,7 +11,8 @@ Process slash commands in comments: kind: match spec: command: "/triage" - matchList: ["valid", "duplicate", "needs-information", "not-planned"] + rules: + - matchList: ["triage/valid", "triage/duplicate", "triage/needs-information", "triage/not-planned"] actions: - kind: remove-label spec: @@ -21,6 +22,9 @@ Process slash commands in comments: label: "triage/{{ argv.0 }}" ``` +Commands must occupy the first whitespace-delimited token of a line. A `rules.matchList` +entry may be either the command argument or the label rendered by an `apply-label` action. + ### 2. Label Rules (`kind: label`) Apply labels based on existing label presence: ```yaml diff --git a/utilities/labeler/labeler.go b/utilities/labeler/labeler.go index c50ed3c0..2de9372d 100644 --- a/utilities/labeler/labeler.go +++ b/utilities/labeler/labeler.go @@ -4,11 +4,13 @@ import ( "context" "errors" "fmt" + "io" "log" "net/http" "path" "slices" "strings" + "time" "github.com/bmatcuk/doublestar/v4" "github.com/google/go-github/v55/github" @@ -18,7 +20,6 @@ import ( // GitHubClient interface to allow mocking type GitHubClient interface { - GetIssue(ctx context.Context, owner, repo string, number int) (*github.Issue, *github.Response, error) ListLabelsByIssue(ctx context.Context, owner, repo string, number int, opts *github.ListOptions) ([]*github.Label, *github.Response, error) AddLabelsToIssue(ctx context.Context, owner, repo string, number int, labels []string) ([]*github.Label, *github.Response, error) RemoveLabelForIssue(ctx context.Context, owner, repo string, number int, label string) (*github.Response, error) @@ -34,10 +35,6 @@ type GitHubClientWrapper struct { client *github.Client } -func (g *GitHubClientWrapper) GetIssue(ctx context.Context, owner, repo string, number int) (*github.Issue, *github.Response, error) { - return g.client.Issues.Get(ctx, owner, repo, number) -} - func (g *GitHubClientWrapper) ListLabelsByIssue(ctx context.Context, owner, repo string, number int, opts *github.ListOptions) ([]*github.Label, *github.Response, error) { return g.client.Issues.ListLabelsByIssue(ctx, owner, repo, number, opts) } @@ -86,28 +83,27 @@ func NewLabeler(client GitHubClient, config *LabelsYAML) *Labeler { // ProcessRequest processes a labeling request func (l *Labeler) ProcessRequest(ctx context.Context, req *LabelRequest) error { + var errs []error if l.config.AutoDelete { if err := l.deleteUndefinedLabels(ctx, req.Owner, req.Repo); err != nil { - log.Printf("failed to delete undefined labels: %v", err) + errs = append(errs, fmt.Errorf("delete undefined labels: %w", err)) } } if l.config.AutoCreate { if err := l.ensureDefinedLabelsExist(ctx, req.Owner, req.Repo); err != nil { - log.Printf("failed to ensure defined labels exist: %v", err) + errs = append(errs, fmt.Errorf("ensure defined labels exist: %w", err)) } } - issue, _, err := l.client.GetIssue(ctx, req.Owner, req.Repo, req.IssueNumber) - if err != nil { - return fmt.Errorf("failed to fetch issue: %v", err) - } - if l.config.Debug { - log.Printf("Processing issue #%d: %s", *issue.Number, *issue.Title) + log.Printf("Processing issue #%d", req.IssueNumber) } - return l.processRules(ctx, req, issue) + if err := l.processRules(ctx, req); err != nil { + errs = append(errs, err) + } + return errors.Join(errs...) } // LabelRequest represents a labeling request @@ -119,13 +115,14 @@ type LabelRequest struct { ChangedFiles []string } -func (l *Labeler) processRules(ctx context.Context, req *LabelRequest, issue *github.Issue) error { +func (l *Labeler) processRules(ctx context.Context, req *LabelRequest) error { + var errs []error for _, rule := range l.config.Ruleset { if err := l.processRule(ctx, req, rule); err != nil { - log.Printf("error processing rule %s: %v", rule.Name, err) + errs = append(errs, fmt.Errorf("rule %q: %w", rule.Name, err)) } } - return nil + return errors.Join(errs...) } func (l *Labeler) processRule(ctx context.Context, req *LabelRequest, rule Rule) error { @@ -149,24 +146,25 @@ func (l *Labeler) processFilePathRule(ctx context.Context, req *LabelRequest, ru return nil } + matchedAny := false for _, file := range req.ChangedFiles { matched, err := doublestar.Match(rule.Spec.MatchPath, file) if err != nil { return fmt.Errorf("error matching file path: %v", err) } - shouldApply := matched - if rule.Spec.MatchCondition == "NOT" { - shouldApply = !matched + if matched { + matchedAny = true + break } + } - if shouldApply { - for _, action := range rule.Actions { - if err := l.executeAction(ctx, req, action, nil); err != nil { - log.Printf("error executing action: %v", err) - } - } - } + shouldApply := matchedAny + if strings.EqualFold(rule.Spec.MatchCondition, "NOT") { + shouldApply = !matchedAny + } + if shouldApply { + return l.executeActions(ctx, req, rule.Actions, nil) } return nil } @@ -185,31 +183,93 @@ func (l *Labeler) processMatchRule(ctx context.Context, req *LabelRequest, rule lines := strings.Split(req.CommentBody, "\n") for _, line := range lines { - line = strings.TrimSpace(line) - if strings.HasPrefix(line, rule.Spec.Command) { - parts := strings.Fields(line) - argv := []string{} - if len(parts) > 1 { - argv = parts[1:] + parts := strings.Fields(line) + if len(parts) == 0 || parts[0] != rule.Spec.Command { + continue + } + argv := parts[1:] + if !l.commandAllowed(rule, argv) { + if l.config.Debug { + log.Printf("Invalid arguments %q for command %s", argv, rule.Spec.Command) + } + continue + } + if err := l.executeActions(ctx, req, rule.Actions, argv); err != nil { + return err + } + } + return nil +} + +func (l *Labeler) commandAllowed(rule Rule, argv []string) bool { + if len(rule.Spec.Rules) == 0 && len(rule.Spec.MatchList) == 0 { + return true + } + + var rawAllowlist []string + var renderedAllowlist []string + + for _, m := range rule.Spec.MatchList { + if m != "" { + rawAllowlist = append(rawAllowlist, m) + renderedAllowlist = append(renderedAllowlist, m) + } + } + for _, predicate := range rule.Spec.Rules { + if predicate.Match != "" { + rawAllowlist = append(rawAllowlist, predicate.Match) + renderedAllowlist = append(renderedAllowlist, predicate.Match) + } + for _, m := range predicate.MatchList { + if m != "" { + renderedAllowlist = append(renderedAllowlist, m) } + } + } - if len(rule.Spec.MatchList) > 0 && len(argv) > 0 { - if !slices.Contains(rule.Spec.MatchList, argv[0]) { - if l.config.Debug { - log.Printf("Invalid argument `%s` for command %s", argv[0], rule.Spec.Command) - } - continue - } + if len(rawAllowlist) == 0 && len(renderedAllowlist) == 0 { + return true + } + + requiresArg := false + for _, action := range rule.Actions { + if strings.Contains(action.Spec.Label, "{{") || strings.Contains(action.Spec.Match, "{{") { + requiresArg = true + break + } + } + if requiresArg && len(argv) == 0 { + return false + } + + for _, action := range rule.Actions { + if action.Kind == "apply-label" { + rendered := l.renderLabel(action.Spec.Label, argv) + if !strings.Contains(rendered, "{{") && slices.Contains(renderedAllowlist, rendered) { + return true } + } + } + if len(argv) > 0 { + arg := argv[0] + if slices.Contains(rawAllowlist, arg) { for _, action := range rule.Actions { - if err := l.executeAction(ctx, req, action, argv); err != nil { - log.Printf("error executing action: %v", err) + if action.Kind == "apply-label" && strings.Contains(action.Spec.Label, "{{") { + rendered := l.renderLabel(action.Spec.Label, argv) + if strings.Contains(rendered, "{{") { + return false + } + if l.config.DefinitionRequired && !l.isValidLabel(rendered) { + return false + } } } + return true } } - return nil + + return false } func (l *Labeler) processLabelRule(ctx context.Context, req *LabelRequest, rule Rule) error { @@ -258,13 +318,32 @@ func (l *Labeler) processLabelRule(ctx context.Context, req *LabelRequest, rule } if shouldApply { - for _, action := range rule.Actions { - if err := l.executeAction(ctx, req, action, nil); err != nil { - log.Printf("error executing action: %v", err) + return l.executeActions(ctx, req, rule.Actions, nil) + } + return nil +} + +func (l *Labeler) executeActions(ctx context.Context, req *LabelRequest, actions []Action, argv []string) error { + // Preflight all apply-label actions before executing any actions (to prevent partial removals). + for _, action := range actions { + if action.Kind == "apply-label" { + label := l.renderLabel(action.Spec.Label, argv) + if strings.Contains(label, "{{") { + return fmt.Errorf("action apply-label has unresolved template: %q", label) + } + _, _, resolvedLabel := l.getLabelDefinition(label) + if resolvedLabel == "" { + return fmt.Errorf("label %s is not defined in labels.yaml and auto-create is disabled", label) } } } - return nil + var errs []error + for _, action := range actions { + if err := l.executeAction(ctx, req, action, argv); err != nil { + errs = append(errs, err) + } + } + return errors.Join(errs...) } func (l *Labeler) executeAction(ctx context.Context, req *LabelRequest, action Action, argv []string) error { @@ -550,21 +629,17 @@ func isLabelNotFoundError(err error) bool { } func (l *Labeler) ensureDefinedLabelsExist(ctx context.Context, owner, repo string) error { + var errs []error for _, label := range l.config.Labels { color, description, labelName := l.getLabelDefinition(label.Name) if err := l.ensureLabelExists(ctx, owner, repo, labelName, color, description); err != nil { - log.Printf("skipping label %s due to error: %v", labelName, err) + errs = append(errs, fmt.Errorf("label %s: %w", labelName, err)) } } - return nil + return errors.Join(errs...) } func (l *Labeler) deleteUndefinedLabels(ctx context.Context, owner, repo string) error { - existingLabels, _, err := l.client.ListLabels(ctx, owner, repo, nil) - if err != nil { - return fmt.Errorf("failed to fetch existing labels: %v", err) - } - definedLabels := map[string]bool{} for _, label := range l.config.Labels { definedLabels[label.Name] = true @@ -573,14 +648,25 @@ func (l *Labeler) deleteUndefinedLabels(ctx context.Context, owner, repo string) } } + var existingLabels []*github.Label + for page := 1; page > 0; { + labels, response, err := l.client.ListLabels(ctx, owner, repo, &github.ListOptions{Page: page, PerPage: 100}) + if err != nil { + return fmt.Errorf("failed to fetch existing labels: %w", err) + } + existingLabels = append(existingLabels, labels...) + if response == nil { + break + } + page = response.NextPage + } for _, lbl := range existingLabels { if !definedLabels[lbl.GetName()] { if l.config.Debug { log.Printf("deleting undefined label: %s", lbl.GetName()) } - _, err := l.client.DeleteLabel(ctx, owner, repo, lbl.GetName()) - if err != nil { - log.Printf("failed to delete label %s: %v", lbl.GetName(), err) + if _, err := l.client.DeleteLabel(ctx, owner, repo, lbl.GetName()); err != nil { + return fmt.Errorf("delete label %s: %w", lbl.GetName(), err) } } } @@ -589,9 +675,13 @@ func (l *Labeler) deleteUndefinedLabels(ctx context.Context, owner, repo string) // LoadConfigFromURL loads configuration from a URL func LoadConfigFromURL(url string) (*LabelsYAML, error) { - resp, err := http.Get(url) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("create labels.yaml request: %w", err) + } + resp, err := (&http.Client{Timeout: 30 * time.Second}).Do(req) if err != nil { - return nil, fmt.Errorf("failed to fetch labels.yaml from URL: %v", err) + return nil, fmt.Errorf("fetch labels.yaml from URL: %w", err) } defer resp.Body.Close() @@ -599,14 +689,58 @@ func LoadConfigFromURL(url string) (*LabelsYAML, error) { return nil, fmt.Errorf("failed to fetch labels.yaml: HTTP %d", resp.StatusCode) } + return loadConfig(resp.Body) +} + +func loadConfig(r io.Reader) (*LabelsYAML, error) { var cfg LabelsYAML - dec := yaml.NewDecoder(resp.Body) + dec := yaml.NewDecoder(r) + dec.KnownFields(true) if err := dec.Decode(&cfg); err != nil { - return nil, fmt.Errorf("failed to decode labels.yaml: %v", err) + return nil, fmt.Errorf("decode labels.yaml: %w", err) + } + if err := validateConfig(&cfg); err != nil { + return nil, err } return &cfg, nil } +func validateConfig(cfg *LabelsYAML) error { + for _, rule := range cfg.Ruleset { + if rule.Spec.MatchCondition != "" { + upper := strings.ToUpper(rule.Spec.MatchCondition) + if upper != "AND" && upper != "NOT" { + return fmt.Errorf("rule %q: invalid matchCondition %q (must be AND or NOT)", rule.Name, rule.Spec.MatchCondition) + } + } + switch rule.Kind { + case "match": + if !strings.HasPrefix(rule.Spec.Command, "/") { + return fmt.Errorf("rule %q: match rules require a slash command", rule.Name) + } + case "filePath": + if rule.Spec.MatchPath == "" { + return fmt.Errorf("rule %q: filePath rules require matchPath", rule.Name) + } + case "label": + if rule.Spec.Match == "" { + return fmt.Errorf("rule %q: label rules require match", rule.Name) + } + default: + return fmt.Errorf("rule %q: unknown kind %q", rule.Name, rule.Kind) + } + for _, action := range rule.Actions { + if action.Kind != "apply-label" && action.Kind != "remove-label" { + return fmt.Errorf("rule %q: unknown action kind %q", rule.Name, action.Kind) + } + if action.Spec.Label == "" && action.Spec.Match == "" { + return fmt.Errorf("rule %q: action %q requires label or match", rule.Name, action.Kind) + } + } + } + return nil +} + // CreateGitHubClient creates a new GitHub client func CreateGitHubClient(token string) (*GitHubClientWrapper, error) { if token == "" { diff --git a/utilities/labeler/labeler_test.go b/utilities/labeler/labeler_test.go index 9585a662..35de26e1 100644 --- a/utilities/labeler/labeler_test.go +++ b/utilities/labeler/labeler_test.go @@ -2,7 +2,9 @@ package main import ( "context" + "fmt" "net/http" + "os" "strings" "testing" @@ -11,19 +13,18 @@ import ( // MockGitHubClient implements GitHubClient for testing type MockGitHubClient struct { - Issues map[int]*github.Issue - Labels []*github.Label - IssueLabels map[int][]*github.Label - CreatedLabels map[string]*github.Label - DeletedLabels []string - AppliedLabels map[int][]string - RemovedLabels map[int][]string - GetLabelError error + Labels []*github.Label + IssueLabels map[int][]*github.Label + CreatedLabels map[string]*github.Label + DeletedLabels []string + AppliedLabels map[int][]string + RemovedLabels map[int][]string + GetLabelError error + ListLabelsFunc func(opts *github.ListOptions) ([]*github.Label, *github.Response, error) } func NewMockGitHubClient() *MockGitHubClient { return &MockGitHubClient{ - Issues: make(map[int]*github.Issue), IssueLabels: make(map[int][]*github.Label), CreatedLabels: make(map[string]*github.Label), DeletedLabels: []string{}, @@ -32,14 +33,6 @@ func NewMockGitHubClient() *MockGitHubClient { } } -func (m *MockGitHubClient) GetIssue(ctx context.Context, owner, repo string, number int) (*github.Issue, *github.Response, error) { - if issue, exists := m.Issues[number]; exists { - return issue, nil, nil - } - title := "Test Issue" - return &github.Issue{Number: &number, Title: &title}, nil, nil -} - func (m *MockGitHubClient) ListLabelsByIssue(ctx context.Context, owner, repo string, number int, opts *github.ListOptions) ([]*github.Label, *github.Response, error) { return m.IssueLabels[number], nil, nil } @@ -75,6 +68,9 @@ func (m *MockGitHubClient) RemoveLabelForIssue(ctx context.Context, owner, repo } func (m *MockGitHubClient) ListLabels(ctx context.Context, owner, repo string, opts *github.ListOptions) ([]*github.Label, *github.Response, error) { + if m.ListLabelsFunc != nil { + return m.ListLabelsFunc(opts) + } return m.Labels, nil, nil } @@ -894,6 +890,185 @@ func TestEnsureLabelExists_GetLabelNon404Error(t *testing.T) { } } +func TestLoadProductionConfig(t *testing.T) { + configFile, err := os.Open("../../.github/labels.yaml") + if err != nil { + t.Fatalf("open production labels config: %v", err) + } + defer configFile.Close() + + if _, err := loadConfig(configFile); err != nil { + t.Fatalf("load production labels config: %v", err) + } +} + +func TestLoadConfigRejectsUnknownFields(t *testing.T) { + _, err := loadConfig(strings.NewReader("labels: []\nunknown: true\n")) + if err == nil || !strings.Contains(err.Error(), "field unknown") { + t.Fatalf("expected unknown field error, got: %v", err) + } +} + +func TestLabeler_ProcessMatchRule_RequiresExactCommandAndAllowedArgument(t *testing.T) { + config := &LabelsYAML{ + AutoCreate: true, + DefinitionRequired: true, + Labels: []Label{ + {Name: "needs-priority", Color: "ededed", Description: "Needs priority"}, + {Name: "priority/high", Color: "ff0000", Description: "High priority"}, + }, + Ruleset: []Rule{{ + Name: "priority", + Kind: "match", + Spec: RuleSpec{ + Command: "/priority", + Rules: []CommandRule{{MatchList: []string{"priority/high"}}}, + }, + Actions: []Action{ + {Kind: "remove-label", Spec: ActionSpec{Match: "needs-priority"}}, + {Kind: "apply-label", Spec: ActionSpec{Label: "priority/{{ argv.0 }}"}}, + }, + }}, + } + + for _, tc := range []struct { + name string + comment string + wantApply bool + wantRemove bool + }{ + {name: "accepted rendered label", comment: "/priority high", wantApply: true, wantRemove: true}, + {name: "rejected argument", comment: "/priority invalid"}, + {name: "bare command rejected", comment: "/priority"}, + {name: "rendered label as argument rejected", comment: "/priority priority/high"}, + {name: "prefix is not a command", comment: "/priority-high high"}, + } { + t.Run(tc.name, func(t *testing.T) { + client := NewMockGitHubClient() + client.IssueLabels[1] = []*github.Label{{Name: stringPtr("needs-priority")}} + labeler := NewLabeler(client, config) + if err := labeler.ProcessRequest(context.Background(), &LabelRequest{Owner: "o", Repo: "r", IssueNumber: 1, CommentBody: tc.comment}); err != nil { + t.Fatalf("ProcessRequest failed: %v", err) + } + if got := sliceContains(client.AppliedLabels[1], "priority/high"); got != tc.wantApply { + t.Errorf("priority/high applied=%v, want %v", got, tc.wantApply) + } + if got := sliceContains(client.RemovedLabels[1], "needs-priority"); got != tc.wantRemove { + t.Errorf("needs-priority removed=%v, want %v", got, tc.wantRemove) + } + }) + } +} + +func TestLabeler_ProcessFilePathRule_NotRequiresNoMatches(t *testing.T) { + config := &LabelsYAML{ + AutoCreate: true, + DefinitionRequired: true, + Labels: []Label{{Name: "needs-docs", Color: "ededed", Description: "Needs docs"}}, + Ruleset: []Rule{{ + Name: "non-docs", + Kind: "filePath", + Spec: RuleSpec{MatchPath: "docs/**", MatchCondition: "NOT"}, + Actions: []Action{{Kind: "apply-label", Spec: ActionSpec{Label: "needs-docs"}}}, + }}, + } + for _, tc := range []struct { + name string + files []string + want bool + }{ + {name: "only nonmatching files", files: []string{"main.go"}, want: true}, + {name: "mixed files", files: []string{"docs/readme.md", "main.go"}}, + } { + t.Run(tc.name, func(t *testing.T) { + client := NewMockGitHubClient() + labeler := NewLabeler(client, config) + if err := labeler.ProcessRequest(context.Background(), &LabelRequest{Owner: "o", Repo: "r", IssueNumber: 1, ChangedFiles: tc.files}); err != nil { + t.Fatalf("ProcessRequest failed: %v", err) + } + if got := sliceContains(client.AppliedLabels[1], "needs-docs"); got != tc.want { + t.Errorf("needs-docs applied=%v, want %v", got, tc.want) + } + }) + } +} + +func TestLabeler_ProcessRequest_ReturnsRuleErrors(t *testing.T) { + labeler := NewLabeler(NewMockGitHubClient(), &LabelsYAML{ + Ruleset: []Rule{{Name: "unknown", Kind: "unknown"}}, + }) + if err := labeler.ProcessRequest(context.Background(), &LabelRequest{Owner: "o", Repo: "r", IssueNumber: 1}); err == nil { + t.Fatal("expected ProcessRequest to return an unknown-rule error") + } +} + +func TestLabeler_DeleteUndefinedLabels_PaginatesBeforeDeleting(t *testing.T) { + client := NewMockGitHubClient() + client.ListLabelsFunc = func(opts *github.ListOptions) ([]*github.Label, *github.Response, error) { + switch opts.Page { + case 1: + return []*github.Label{{Name: stringPtr("defined")}, {Name: stringPtr("undefined-one")}}, &github.Response{NextPage: 2}, nil + case 2: + return []*github.Label{{Name: stringPtr("undefined-two")}}, &github.Response{}, nil + default: + t.Fatalf("unexpected page %d", opts.Page) + return nil, nil, nil + } + } + labeler := NewLabeler(client, &LabelsYAML{Labels: []Label{{Name: "defined"}}}) + if err := labeler.deleteUndefinedLabels(context.Background(), "o", "r"); err != nil { + t.Fatalf("deleteUndefinedLabels failed: %v", err) + } + if !slicesEqual(client.DeletedLabels, []string{"undefined-one", "undefined-two"}) { + t.Errorf("deleted labels = %v, want both undefined labels", client.DeletedLabels) + } +} + +func TestLoadConfigRejectsInvalidMatchCondition(t *testing.T) { + for _, invalid := range []string{"ADN", "NTO", "or", "invalid"} { + t.Run(invalid, func(t *testing.T) { + yamlStr := fmt.Sprintf("ruleset:\n- name: test\n kind: label\n spec:\n match: foo/*\n matchCondition: %s\n actions:\n - kind: apply-label\n spec:\n label: bar\n", invalid) + _, err := loadConfig(strings.NewReader(yamlStr)) + if err == nil || !strings.Contains(err.Error(), "invalid matchCondition") { + t.Fatalf("expected invalid matchCondition error for %q, got: %v", invalid, err) + } + }) + } +} + +func TestLabeler_PreflightPreventsRemovalsOnInvalidLabel(t *testing.T) { + config := &LabelsYAML{ + AutoCreate: false, + DefinitionRequired: true, + Labels: []Label{ + {Name: "needs-priority", Color: "ededed", Description: "Needs priority"}, + }, + Ruleset: []Rule{{ + Name: "priority", + Kind: "match", + Spec: RuleSpec{ + Command: "/priority", + }, + Actions: []Action{ + {Kind: "remove-label", Spec: ActionSpec{Match: "needs-priority"}}, + {Kind: "apply-label", Spec: ActionSpec{Label: "priority/{{ argv.0 }}"}}, + }, + }}, + } + client := NewMockGitHubClient() + client.IssueLabels[1] = []*github.Label{{Name: stringPtr("needs-priority")}} + labeler := NewLabeler(client, config) + err := labeler.ProcessRequest(context.Background(), &LabelRequest{ + Owner: "o", Repo: "r", IssueNumber: 1, CommentBody: "/priority nonexistent", + }) + if err == nil { + t.Fatal("expected error on undefined label apply") + } + if sliceContains(client.RemovedLabels[1], "needs-priority") { + t.Error("needs-priority was removed despite preflight failure") + } +} + // Helper functions func stringPtr(s string) *string { return &s diff --git a/utilities/labeler/main.go b/utilities/labeler/main.go index 617dc125..2edec067 100644 --- a/utilities/labeler/main.go +++ b/utilities/labeler/main.go @@ -12,7 +12,7 @@ import ( func main() { flag.Parse() - if len(flag.Args()) < 5 { + if len(flag.Args()) < 6 { fmt.Println("Usage: labeler [flags] ") os.Exit(1) } diff --git a/utilities/labeler/types.go b/utilities/labeler/types.go index 415df474..213cd48a 100644 --- a/utilities/labeler/types.go +++ b/utilities/labeler/types.go @@ -25,13 +25,19 @@ type Action struct { // RuleSpec represents rule specifications type RuleSpec struct { Command string `yaml:"command,omitempty"` - Rules []interface{} `yaml:"rules,omitempty"` + Rules []CommandRule `yaml:"rules,omitempty"` Match string `yaml:"match,omitempty"` MatchCondition string `yaml:"matchCondition,omitempty"` MatchPath string `yaml:"matchPath,omitempty"` MatchList []string `yaml:"matchList,omitempty"` } +// CommandRule limits the arguments accepted by a match rule. +type CommandRule struct { + Match string `yaml:"match,omitempty"` + MatchList []string `yaml:"matchList,omitempty"` +} + // Rule represents a labeling rule type Rule struct { Name string `yaml:"name"`