diff --git a/.github/actions/check-github-config/check.py b/.github/actions/check-github-config/check.py index 072bdab..b01bc51 100644 --- a/.github/actions/check-github-config/check.py +++ b/.github/actions/check-github-config/check.py @@ -11,7 +11,9 @@ FIELD_PATTERN = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") COLOR_PATTERN = re.compile(r"^[0-9A-Fa-f]{6}$") +TYPE_COLORS = frozenset({"GRAY", "BLUE", "GREEN", "YELLOW", "ORANGE", "RED", "PINK", "PURPLE"}) LABELS_CHECK = "labels" +TYPES_CHECK = "types" @dataclass(frozen=True) @@ -50,40 +52,72 @@ def load_config(config_path): if not isinstance(field, str) or not FIELD_PATTERN.fullmatch(field): raise ValueError("github repo check names must be GraphQL field names") - labels = config.get("labels") + issues = config.get("issues") + if not isinstance(issues, dict): + raise ValueError("github repo config must contain an issues mapping") + + labels = issues.get("labels") if not isinstance(labels, dict): - raise ValueError("github repo config must contain a labels mapping") + raise ValueError("github repo config must contain an issues.labels mapping") required = validate_label_group(labels.get("required"), "required") optional = validate_label_group(labels.get("optional", {}), "optional", allow_empty=True) if duplicate := set(required) & set(optional): names = ", ".join(sorted(duplicate)) raise ValueError(f"GitHub labels cannot be both required and optional: {names}") + types = issues.get("types") + if not isinstance(types, dict): + raise ValueError("github repo config must contain an issues.types mapping") + types_required = validate_type_group(types.get("required"), "required") + types_optional = validate_type_group(types.get("optional", {}), "optional", allow_empty=True) + if duplicate := set(types_required) & set(types_optional): + names = ", ".join(sorted(duplicate)) + raise ValueError(f"GitHub issue types cannot be both required and optional: {names}") + checks = dict(repository_config) checks[LABELS_CHECK] = {"required": required, "optional": optional} + checks[TYPES_CHECK] = {"required": types_required, "optional": types_optional} return checks -def validate_label_group(group, name, allow_empty=False): +def validate_named_group(group, kind, name, allow_empty, validate_color, color_hint, normalize_color): if not isinstance(group, dict) or (not group and not allow_empty): - raise ValueError(f"GitHub {name} labels must be a{' non-empty' if not allow_empty else ''} mapping") + raise ValueError(f"GitHub {name} {kind} must be a{' non-empty' if not allow_empty else ''} mapping") validated = {} - for label, settings in group.items(): - if not isinstance(label, str) or not label: - raise ValueError(f"GitHub {name} label names must be non-empty strings") + for item, settings in group.items(): + if not isinstance(item, str) or not item: + raise ValueError(f"GitHub {name} {kind[:-1]} names must be non-empty strings") if not isinstance(settings, dict) or set(settings) != {"color", "description"}: - raise ValueError(f"GitHub label {label} must contain only color and description") + raise ValueError(f"GitHub {kind[:-1]} {item} must contain only color and description") color = settings["color"] - if not isinstance(color, str) or not COLOR_PATTERN.fullmatch(color): - raise ValueError(f"GitHub label {label} color must be a six-digit hex value") + if not isinstance(color, str) or not validate_color(color): + raise ValueError(f"GitHub {kind[:-1]} {item} color must be {color_hint}") description = settings["description"] if not isinstance(description, str) or not description: - raise ValueError(f"GitHub label {label} description must be a non-empty string") - validated[label] = {"color": color.lower(), "description": description} + raise ValueError(f"GitHub {kind[:-1]} {item} description must be a non-empty string") + validated[item] = {"color": normalize_color(color), "description": description} return validated +def validate_label_group(group, name, allow_empty=False): + return validate_named_group( + group, "labels", name, allow_empty, COLOR_PATTERN.fullmatch, "a six-digit hex value", str.lower + ) + + +def validate_type_group(group, name, allow_empty=False): + return validate_named_group( + group, + "types", + name, + allow_empty, + lambda color: color in TYPE_COLORS, + f"one of {sorted(TYPE_COLORS)}", + lambda color: color, + ) + + def github_request(args): result = subprocess.run(["gh", *args], capture_output=True, text=True) if result.returncode: @@ -131,50 +165,89 @@ def github_labels(repository): raise RuntimeError("GitHub returned an invalid labels response") from error +def github_issue_types(repository): + owner, name = repository_name(repository) + query = ( + "query($owner: String!, $name: String!) { " + "repository(owner: $owner, name: $name) { " + "issueTypes(first: 100) { nodes { name color description isEnabled } } } }" + ) + response = github_request( + ["api", "graphql", "-f", f"query={query}", "-F", f"owner={owner}", "-F", f"name={name}"] + ) + try: + return response["data"]["repository"]["issueTypes"]["nodes"] + except (KeyError, TypeError) as error: + raise RuntimeError("GitHub returned an invalid issue types response") from error + + def format_value(value): return json.dumps(value, separators=(",", ":"), sort_keys=True) -def evaluate_label_check(policy, repository, request=github_labels): - try: - labels = request(repository) - actual = { - label["name"]: { - "color": label["color"].lower(), - "description": label.get("description") or "", - } - for label in labels - } - except (KeyError, TypeError, RuntimeError) as error: - return CheckResult(LABELS_CHECK, "failed", f"GitHub request failed: {error}") - +def evaluate_style_check(name, noun, policy, actual, format_color=lambda color: f"#{color}", check_unexpected=True): required = policy["required"] optional = policy["optional"] allowed = {**required, **optional} problems = [] if missing := set(required) - set(actual): problems.append(f"missing: {', '.join(sorted(missing))}") - if unexpected := set(actual) - set(allowed): + if check_unexpected and (unexpected := set(actual) - set(allowed)): problems.append(f"unexpected: {', '.join(sorted(unexpected))}") incorrect_colors = [ - f"{name} expected #{allowed[name]['color']}, got #{actual[name]['color']}" - for name in sorted(set(actual) & set(allowed)) - if actual[name]["color"] != allowed[name]["color"] + f"{key} expected {format_color(allowed[key]['color'])}, got {format_color(actual[key]['color'])}" + for key in sorted(set(actual) & set(allowed)) + if actual[key]["color"] != allowed[key]["color"] ] if incorrect_colors: problems.append(f"incorrect colors: {'; '.join(incorrect_colors)}") incorrect_descriptions = [ - f"{name} expected {format_value(allowed[name]['description'])}, " - f"got {format_value(actual[name]['description'])}" - for name in sorted(set(actual) & set(allowed)) - if actual[name]["description"] != allowed[name]["description"] + f"{key} expected {format_value(allowed[key]['description'])}, " + f"got {format_value(actual[key]['description'])}" + for key in sorted(set(actual) & set(allowed)) + if actual[key]["description"] != allowed[key]["description"] ] if incorrect_descriptions: problems.append(f"incorrect descriptions: {'; '.join(incorrect_descriptions)}") if problems: - return CheckResult(LABELS_CHECK, "failed", "; ".join(problems)) - return CheckResult(LABELS_CHECK, "passed", f"{len(actual)} labels match policy") + return CheckResult(name, "failed", "; ".join(problems)) + return CheckResult(name, "passed", f"{len(actual)} {noun} match policy") + + +def evaluate_label_check(policy, repository, request=github_labels): + try: + labels = request(repository) + actual = { + label["name"]: { + "color": label["color"].lower(), + "description": label.get("description") or "", + } + for label in labels + } + except (KeyError, TypeError, RuntimeError) as error: + return CheckResult(LABELS_CHECK, "failed", f"GitHub request failed: {error}") + + return evaluate_style_check(LABELS_CHECK, "labels", policy, actual) + + +def evaluate_type_check(policy, repository, request=github_issue_types): + try: + types = request(repository) + actual = { + issue_type["name"]: { + "color": issue_type["color"], + "description": issue_type.get("description") or "", + } + for issue_type in types + if issue_type.get("isEnabled") + } + except (KeyError, TypeError, RuntimeError) as error: + return CheckResult(TYPES_CHECK, "failed", f"GitHub request failed: {error}") + + return evaluate_style_check( + TYPES_CHECK, "types", policy, actual, format_color=lambda color: color, check_unexpected=False + ) def evaluate_checks( @@ -183,13 +256,16 @@ def evaluate_checks( skipped=(), request=github_repository, labels_request=github_labels, + types_request=github_issue_types, ): skipped = set(skipped) if unknown := skipped - set(checks): names = ", ".join(sorted(unknown)) raise ValueError(f"Unknown skipped GitHub config checks: {names}") - active_fields = [field for field in checks if field != LABELS_CHECK and field not in skipped] + active_fields = [ + field for field in checks if field not in (LABELS_CHECK, TYPES_CHECK) and field not in skipped + ] payload = None request_error = None if active_fields: @@ -208,6 +284,10 @@ def evaluate_checks( results.append(evaluate_label_check(expected, repository, labels_request)) continue + if field == TYPES_CHECK: + results.append(evaluate_type_check(expected, repository, types_request)) + continue + if request_error: results.append(CheckResult(field, "failed", f"GitHub request failed: {request_error}")) continue diff --git a/README.md b/README.md index fb1fe3f..e84fa66 100644 --- a/README.md +++ b/README.md @@ -61,10 +61,17 @@ jobs: -Baseline checks the repository settings and labels against +Baseline checks the repository settings, labels, and issue types against [`config/github.yml`](config/github.yml). This includes squash-only merging, -automatic branch deletion, auto-merge, and the canonical label set and colors. -Release Please labels are allowed but optional. +automatic branch deletion, auto-merge, and the canonical label and issue type +sets and colors. Release Please labels are allowed but optional, as is the +Epic issue type. + +Issue types are set at the organization level, not per repository, so unlike +labels the check only verifies that the required and optional types are +present and correctly defined. Other types in the organization are ignored, +since an organization may use issue types for repositories outside Baseline's +policy. The caller grants both permissions because a reusable workflow can reduce its caller's `GITHUB_TOKEN` permissions, but cannot elevate them. diff --git a/config/github.yml b/config/github.yml index 59d770d..b2e35e1 100644 --- a/config/github.yml +++ b/config/github.yml @@ -6,42 +6,64 @@ config: squashMergeAllowed: true squashMergeCommitTitle: PR_TITLE squashMergeCommitMessage: BLANK -labels: - required: - ":triangular_flag_on_post:": - color: FFEFEF - description: Temporary fast-track flag - deps: - color: 0366d6 - description: Pull requests that update a dependency file - "priority: P1": - color: fef2c0 - description: Active tier, do this first - "priority: P2": - color: fef2c0 - description: Second-priority candidate - "priority: P3": - color: fef2c0 - description: Low-pressure, review later - "size: L": - color: bfd4f2 - description: One or two days - "size: M": - color: bfd4f2 - description: A few hours of work - "size: S": - color: bfd4f2 - description: Less than an hour - "size: XL": - color: bfd4f2 - description: Multiple days of work - "size: XS": - color: bfd4f2 - description: Minutes of work - optional: - "autorelease: pending": - color: fbca04 - description: Pending release - "autorelease: tagged": - color: fbca04 - description: Tagged release +issues: + labels: + required: + ":triangular_flag_on_post:": + color: FFEFEF + description: Temporary fast-track flag + deps: + color: 0366d6 + description: Pull requests that update a dependency file + "priority: P1": + color: fef2c0 + description: Active tier, do this first + "priority: P2": + color: fef2c0 + description: Second-priority candidate + "priority: P3": + color: fef2c0 + description: Low-pressure, review later + "size: L": + color: bfd4f2 + description: One or two days + "size: M": + color: bfd4f2 + description: A few hours of work + "size: S": + color: bfd4f2 + description: Less than an hour + "size: XL": + color: bfd4f2 + description: Multiple days of work + "size: XS": + color: bfd4f2 + description: Minutes of work + optional: + "autorelease: pending": + color: fbca04 + description: Pending release + "autorelease: tagged": + color: fbca04 + description: Tagged release + types: + required: + Task: + color: BLUE + description: A specific piece of work + Bug: + color: RED + description: An unexpected problem or behavior + Feature: + color: GREEN + description: A request, idea, or new functionality + optional: + Epic: + color: PURPLE + description: A large goal tracked via sub-issues + Research: + color: GREEN + description: An inquiry to reduce uncertainty + Proposal: + color: GRAY + description: "A proposal for changes to another issue, created as its native sub-issue (see #11)" diff --git a/test/test_github_config.py b/test/test_github_config.py index 82d2720..ef37b87 100644 --- a/test/test_github_config.py +++ b/test/test_github_config.py @@ -35,6 +35,7 @@ def test_loads_repository_config(self): "squashMergeCommitMessage", "squashMergeCommitTitle", "labels", + "types", }, ) self.assertIn("deps", checks["labels"]["required"]) @@ -43,6 +44,9 @@ def test_loads_repository_config(self): checks["labels"]["required"]["deps"]["description"], "Pull requests that update a dependency file", ) + self.assertIn("Task", checks["types"]["required"]) + self.assertIn("Epic", checks["types"]["optional"]) + self.assertEqual(checks["types"]["required"]["Task"]["color"], "BLUE") def test_shared_workflow_grants_permissions_for_repository_and_label_checks(self): with open(BASELINE_ROOT / ".github" / "workflows" / "github-shared.yml") as workflow_file: @@ -68,12 +72,20 @@ def test_rejects_malformed_yaml(self): with self.assertRaisesRegex(ValueError, "must be valid YAML"): CHECK_GITHUB_CONFIG.load_config(config.name) + def test_rejects_missing_issues_mapping(self): + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as config: + config.write("config:\n hasWikiEnabled: false\n") + config.flush() + + with self.assertRaisesRegex(ValueError, "must contain an issues mapping"): + CHECK_GITHUB_CONFIG.load_config(config.name) + def test_rejects_invalid_label_color(self): with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as config: config.write( "config:\n hasWikiEnabled: false\n" - "labels:\n required:\n deps:\n" - " color: blue\n description: Dependency updates\n" + "issues:\n labels:\n required:\n deps:\n" + " color: blue\n description: Dependency updates\n" ) config.flush() @@ -84,13 +96,70 @@ def test_rejects_missing_label_description(self): with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as config: config.write( "config:\n hasWikiEnabled: false\n" - "labels:\n required:\n deps:\n color: 0366d6\n" + "issues:\n labels:\n required:\n deps:\n color: 0366d6\n" ) config.flush() with self.assertRaisesRegex(ValueError, "color and description"): CHECK_GITHUB_CONFIG.load_config(config.name) + def test_rejects_missing_types_mapping(self): + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as config: + config.write( + "config:\n hasWikiEnabled: false\n" + "issues:\n labels:\n required:\n deps:\n" + " color: 0366d6\n description: Dependency updates\n" + ) + config.flush() + + with self.assertRaisesRegex(ValueError, "must contain an issues.types mapping"): + CHECK_GITHUB_CONFIG.load_config(config.name) + + def test_rejects_invalid_type_color(self): + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as config: + config.write( + "config:\n hasWikiEnabled: false\n" + "issues:\n" + " labels:\n required:\n deps:\n" + " color: 0366d6\n description: Dependency updates\n" + " types:\n required:\n Task:\n" + " color: blue\n description: A specific piece of work\n" + ) + config.flush() + + with self.assertRaisesRegex(ValueError, "one of"): + CHECK_GITHUB_CONFIG.load_config(config.name) + + def test_rejects_missing_type_description(self): + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as config: + config.write( + "config:\n hasWikiEnabled: false\n" + "issues:\n" + " labels:\n required:\n deps:\n" + " color: 0366d6\n description: Dependency updates\n" + " types:\n required:\n Task:\n color: BLUE\n" + ) + config.flush() + + with self.assertRaisesRegex(ValueError, "color and description"): + CHECK_GITHUB_CONFIG.load_config(config.name) + + def test_rejects_type_that_is_both_required_and_optional(self): + with tempfile.NamedTemporaryFile(mode="w", suffix=".yml") as config: + config.write( + "config:\n hasWikiEnabled: false\n" + "issues:\n" + " labels:\n required:\n deps:\n" + " color: 0366d6\n description: Dependency updates\n" + " types:\n" + " required:\n Task:\n color: BLUE\n description: A specific piece of work\n" + " optional:\n Task:\n color: BLUE\n description: A specific piece of work\n" + ) + config.flush() + + with self.assertRaisesRegex(ValueError, "cannot be both required and optional"): + CHECK_GITHUB_CONFIG.load_config(config.name) + def test_parses_json_skip(self): self.assertEqual(CHECK_GITHUB_CONFIG.parse_json_list('["hasWikiEnabled"]'), {"hasWikiEnabled"}) @@ -221,6 +290,109 @@ def test_skips_label_policy_without_requesting_labels(self): self.assertEqual(results[0].status, "skipped") + def test_type_policy_accepts_required_and_present_optional_types(self): + policy = { + "required": {"Task": {"color": "BLUE", "description": "A specific piece of work"}}, + "optional": {"Epic": {"color": "PURPLE", "description": "A goal split into sub-issues"}}, + } + result = CHECK_GITHUB_CONFIG.evaluate_type_check( + policy, + "owner/repo", + request=lambda _repository: [ + {"name": "Task", "color": "BLUE", "description": "A specific piece of work", "isEnabled": True}, + { + "name": "Epic", + "color": "PURPLE", + "description": "A goal split into sub-issues", + "isEnabled": True, + }, + ], + ) + + self.assertEqual(result.status, "passed") + + def test_type_policy_accepts_absent_optional_types(self): + policy = { + "required": {"Task": {"color": "BLUE", "description": "A specific piece of work"}}, + "optional": {"Epic": {"color": "PURPLE", "description": "A goal split into sub-issues"}}, + } + result = CHECK_GITHUB_CONFIG.evaluate_type_check( + policy, + "owner/repo", + request=lambda _repository: [ + {"name": "Task", "color": "BLUE", "description": "A specific piece of work", "isEnabled": True} + ], + ) + + self.assertEqual(result.status, "passed") + + def test_type_policy_ignores_disabled_types(self): + policy = { + "required": {"Task": {"color": "BLUE", "description": "A specific piece of work"}}, + "optional": {}, + } + result = CHECK_GITHUB_CONFIG.evaluate_type_check( + policy, + "owner/repo", + request=lambda _repository: [ + {"name": "Task", "color": "BLUE", "description": "A specific piece of work", "isEnabled": True}, + {"name": "Idea", "color": "ORANGE", "description": "A product idea", "isEnabled": False}, + ], + ) + + self.assertEqual(result.status, "passed") + + def test_type_policy_reports_all_differences(self): + policy = { + "required": { + "Task": {"color": "BLUE", "description": "A specific piece of work"}, + "Bug": {"color": "RED", "description": "An unexpected problem or behavior"}, + }, + "optional": {"Epic": {"color": "PURPLE", "description": "A goal split into sub-issues"}}, + } + result = CHECK_GITHUB_CONFIG.evaluate_type_check( + policy, + "owner/repo", + request=lambda _repository: [ + {"name": "Task", "color": "GREEN", "description": None, "isEnabled": True}, + {"name": "Research", "color": "GREEN", "description": "An inquiry", "isEnabled": True}, + ], + ) + + self.assertEqual(result.status, "failed") + self.assertIn("missing: Bug", result.message) + self.assertNotIn("unexpected", result.message) + self.assertIn("Task expected BLUE, got GREEN", result.message) + self.assertIn('Task expected "A specific piece of work", got ""', result.message) + + def test_type_policy_ignores_types_outside_the_policy(self): + policy = { + "required": {"Task": {"color": "BLUE", "description": "A specific piece of work"}}, + "optional": {}, + } + result = CHECK_GITHUB_CONFIG.evaluate_type_check( + policy, + "owner/repo", + request=lambda _repository: [ + {"name": "Task", "color": "BLUE", "description": "A specific piece of work", "isEnabled": True}, + {"name": "Book", "color": "YELLOW", "description": "A book to read", "isEnabled": True}, + {"name": "Guitar", "color": "YELLOW", "description": "A song to learn", "isEnabled": True}, + ], + ) + + self.assertEqual(result.status, "passed") + + def test_skips_type_policy_without_requesting_types(self): + results = CHECK_GITHUB_CONFIG.evaluate_checks( + {"types": {"required": {}, "optional": {}}}, + "owner/repo", + {"types"}, + request=lambda _repository, _fields: {}, + types_request=lambda _repository: self.fail("skipped check made an API request"), + ) + + self.assertEqual(results[0].status, "skipped") + def test_formats_expected_values_as_json(self): self.assertEqual(CHECK_GITHUB_CONFIG.format_value(False), "false") self.assertEqual(json.loads(CHECK_GITHUB_CONFIG.format_value({"enabled": True})), {"enabled": True})