Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/hubcast/clients/github/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,12 @@ async def set_check_status(
url = f"/repos/{self.repo_owner}/{self.repo_name}/check-runs/{existing_check['id']}"
await gh.patch(url, data=payload)

async def get_repo_config(self) -> str | None:
async def get_repo_config(self, ref: str | None = None) -> str | None:
"""Get the contents of the repo's hubcast config file.

Args:
ref: Where to read the file from. Defaults to the repo's default branch.
"""
gh_token = await self.auth.authenticate_installation(
self.repo_owner, self.repo_name
)
Expand All @@ -132,6 +137,8 @@ async def get_repo_config(self) -> str | None:

# get the contents of the repository hubcast.yml file
url = f"/repos/{self.repo_owner}/{self.repo_name}/contents/{self.repo_config_path}"
if ref is not None:
url = f"{url}?ref={ref}"
# get raw contents rather than base64 encoded text
try:
return await gh.getitem(url, accept="application/vnd.github.raw")
Expand All @@ -142,6 +149,19 @@ async def get_repo_config(self) -> str | None:
# all others are unhandled
raise

async def get_pr_files(self, pr_number: int) -> list[str]:
"""Return the files changed in a PR."""
gh_token = await self.auth.authenticate_installation(
self.repo_owner, self.repo_name
)

async with aiohttp.ClientSession() as session:
gh = gh_aiohttp.GitHubAPI(session, self.requester, oauth_token=gh_token)

url = f"/repos/{self.repo_owner}/{self.repo_name}/pulls/{pr_number}/files"
files = await gh.getitem(url)
return [f["filename"] for f in files]

async def get_pr(self, id: int) -> dict[str, Any]:
"""Return individual PR data."""
gh_token = await self.auth.authenticate_installation(
Expand Down
5 changes: 5 additions & 0 deletions src/hubcast/web/github/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
"Hubcast could not parse `hubcast.yml`. "
f"Fix the configuration file and retry. See the [user guide]({CONFIG_DOCS_URL}) for details."
)
CONFIG_VALID_TITLE = "Hubcast config file is valid"
CONFIG_VALID_SUMMARY = (
"Hubcast has validated `hubcast.yml`. "
"These changes will take effect once merged into the default branch."
)


def help_message(bot_caller: str) -> str:
Expand Down
77 changes: 71 additions & 6 deletions src/hubcast/web/github/routes.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import logging
import re
from collections.abc import Awaitable, Callable
from collections.abc import Awaitable, Callable, Collection
from typing import Any

from aiohttp.client_exceptions import ClientResponseError
Expand All @@ -14,6 +14,8 @@
from hubcast.exceptions import HubcastError, RepoConfigError, WebhookPermissionError
from hubcast.logging import update_log_context
from hubcast.web.github.messages import (
CONFIG_VALID_SUMMARY,
CONFIG_VALID_TITLE,
DEACTIVATED_ACCOUNT_MARKER,
DEACTIVATED_ACCOUNT_MSG,
HOOK_DECLINED_MSG,
Expand All @@ -31,7 +33,11 @@
WEBHOOK_PERMISSION_DENIED_TITLE,
help_message,
)
from hubcast.web.github.utils import changed_files_from_push, get_repo_config
from hubcast.web.github.utils import (
changed_files_from_push,
get_repo_config,
parse_repo_config,
)

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -60,7 +66,7 @@ async def dispatch(self, event: sansio.Event, *args: Any, **kwargs: Any) -> None
# this avoids overwriting errors if a normal pipeline succeeds, and provides
# a default for situations where there is no default check name set
# this check won't linger because resolving issues requires a new commit to be pushed
ERROR_CHECK_NAME = "hubcast-error"
ERROR_CHECK_NAME = "hubcast-config"

NULL_SHA = "0" * 40

Expand Down Expand Up @@ -282,15 +288,25 @@ async def sync_branch(
# only refresh config when a default branch push touches .github/hubcast.yml
default_branch = event.data["repository"]["default_branch"]
is_default_branch = sync_ref == f"refs/heads/{default_branch}"
config_changed = gh.repo_config_path in changed_files_from_push(event.data)
changed_files = changed_files_from_push(event.data)
config_changed = gh.repo_config_path in changed_files
try:
repo_config = await get_repo_config(
gh, src_fullname, refresh=is_default_branch and config_changed
)
except RepoConfigError as exc:
await report_config_error(gh, want_sha, exc)
# only report the default branch config's error when this push isn't trying to fix it
if is_default_branch or not config_changed:
await report_config_error(gh, want_sha, exc)
# if the config has changes and not on default branch, validate the new config
if not is_default_branch:
await validate_config_change(gh, changed_files, want_sha)
return

# validate the changes when the default branch config doesn't have issues
if not is_default_branch:
await validate_config_change(gh, changed_files, want_sha)

dest_fullname = repo_config.dest_fullname
dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git"
head_commit = event.data.get("head_commit")
Expand Down Expand Up @@ -387,6 +403,46 @@ async def remove_branch(
# -----------------------------------


async def validate_config_change(
gh: GitHubClient, changed_files: Collection[str], head_sha: str
) -> None:
"""
Validate the Hubcast repo config at head_sha if changed_files touches it,
reporting feedback via a GH check.

This is meant to supersede previously reported config errors on the default branch.
"""
if gh.repo_config_path not in changed_files:
return

# config was deleted in this change
config = await gh.get_repo_config(ref=head_sha)
if config is None:
return

try:
parse_repo_config(config)
except RepoConfigError as exc:
exc.log(log)
await gh.set_check_status(
head_sha,
ERROR_CHECK_NAME,
"failure",
title=exc.title,
summary=exc.summary,
)
return

# report success if validation passes for the PR's config
await gh.set_check_status(
head_sha,
ERROR_CHECK_NAME,
"success",
title=CONFIG_VALID_TITLE,
summary=CONFIG_VALID_SUMMARY,
)


async def sync_pr(
pull_request: dict[str, Any],
gh: GitHubClient,
Expand Down Expand Up @@ -418,13 +474,22 @@ async def sync_pr(
)
return

changed_files = await gh.get_pr_files(pull_request["number"])
config_changed = gh.repo_config_path in changed_files

# get the repository configuration from .github/hubcast.yml
try:
repo_config = await get_repo_config(gh, base_fullname)
except RepoConfigError as exc:
await report_config_error(gh, want_sha, exc)
# only report the default branch config's error when this push isn't trying to fix it
if not config_changed:
await report_config_error(gh, want_sha, exc)
await validate_config_change(gh, changed_files, want_sha)
return

# validate the changes when the default branch config doesn't have issues
await validate_config_change(gh, changed_files, want_sha)

if not repo_config.sync_drafts and pull_request["draft"]:
if repo_config.sync_drafts_msg:
await gh.set_check_status(
Expand Down
81 changes: 62 additions & 19 deletions src/hubcast/web/github/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
from typing import Any

import yaml
import yaml.reader
from cachetools import TTLCache
from pydantic import ValidationError

from hubcast.clients.github import GitHubClient
from hubcast.exceptions import HubcastError, RepoConfigError
Expand All @@ -28,6 +30,65 @@ def changed_files_from_push(payload: dict[str, Any]) -> set[str]:
}


def _format_yaml_error(exc: yaml.YAMLError) -> str:
"""Render YAML parse errors with bullet points rather than pyyaml's default mess."""
if isinstance(exc, yaml.reader.ReaderError):
# encoding/control-character issues
return f"- {exc.reason} (position {exc.position})"

if not isinstance(exc, yaml.MarkedYAMLError):
# the only other error safe_load raises (needed to resolve type issues)
raise TypeError(f"Unexpected YAML error type: {type(exc)!r}")

problem = f"{exc.context}; {exc.problem}" if exc.context else exc.problem
mark = exc.problem_mark
line = (
f"- {problem} (line {mark.line + 1}, column {mark.column + 1})"
if mark
else f"- {problem}"
)

snippet = mark.get_snippet() if mark else None
return f"{line}\n\n```\n{snippet}\n```" if snippet else line


def _format_validation_error(exc: ValidationError) -> str:
"""Render pydantic validation errors as a per-field bullet list."""
lines = []
for err in exc.errors(include_url=False, include_input=False):
loc = ".".join(str(p) for p in err["loc"])
# pydantic prefixes messages from raised errors
msg = err["msg"].removeprefix("Value error, ")
lines.append(f"- `{loc}`: {msg}" if loc else f"- {msg}")
return "\n".join(lines)


def parse_repo_config(raw_config: str) -> RepoConfig:
"""Parse YAML as a RepoConfig.

Raises RepoConfigError for invalid YAML or schema validation issues.
"""
try:
config_yaml = yaml.safe_load(raw_config)
except yaml.YAMLError as e:
raise RepoConfigError(
"Invalid YAML in repo config",
title=CONFIG_INVALID_TITLE,
summary=f"{CONFIG_INVALID_SUMMARY}\n\n---\n\n{_format_yaml_error(e)}",
error=str(e),
)

try:
return RepoConfig.model_validate(config_yaml)
except ValidationError as e:
raise RepoConfigError(
"Invalid repo config",
title=CONFIG_INVALID_TITLE,
summary=f"{CONFIG_INVALID_SUMMARY}\n\n---\n\n{_format_validation_error(e)}",
error=str(e),
)


async def get_repo_config(
gh: GitHubClient, fullname: str, refresh: bool = False
) -> RepoConfig:
Expand Down Expand Up @@ -66,25 +127,7 @@ async def get_repo_config(
raise HubcastError("Repo config file not found", log_level="INFO")

# parse and validate YAML
try:
config_yaml = yaml.safe_load(fetched_config)
except yaml.YAMLError as e:
raise RepoConfigError(
"Invalid YAML in repo config",
title=CONFIG_INVALID_TITLE,
summary=CONFIG_INVALID_SUMMARY,
error=str(e),
)

try:
config = RepoConfig.model_validate(config_yaml)
except ValueError as e:
raise RepoConfigError(
"Invalid repo config",
title=CONFIG_INVALID_TITLE,
summary=CONFIG_INVALID_SUMMARY,
error=str(e),
)
config = parse_repo_config(fetched_config)

config_cache[fullname] = config
log.info("Repo config fetched from source forge")
Expand Down
Loading
Loading