diff --git a/README.md b/README.md index 62baff9..59cd1b0 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ A GitHub Action that monitors and updates RHDH plugins in your RHDH GitOps repos ## Features - šŸ”„ **Automatic Plugin Updates**: Monitors RHDH plugin versions from the GitHub Container Registry -- šŸŽÆ **Flexible PR Strategies**: Create `separate` PRs per plugin or a single `joint` PR for all updates +- šŸŽÆ **Flexible PR Strategies**: Create `separate` PRs per plugin, a single `joint` PR for all updates, or `grouped` PRs for matching frontend/backend packages - šŸ”’ **Safe Updates**: Preserves YAML formatting and validates plugin definitions - šŸš€ **GitOps Integration**: Works seamlessly with GitOps-based RHDH deployments - āš™ļø **Configurable**: Customize PR limits, config paths, and update strategies @@ -74,6 +74,23 @@ Create a single pull request containing all plugin updates: update-pr-strategy: "joint" ``` +### Grouped PRs for Frontend and Backend Updates + +Changing the `update-pr-strategy` field from `separate` to `grouped` results in creating one PR for an exact frontend/backend package pair, such as `pluginabc` and `pluginabc-backend`, when both have updates in the same run and use the same tag prefix. Each package keeps its own target version; the versions can differ. Packages without a matching update get individual PRs. + +```yaml +- name: Update RHDH plugins + uses: thepetk/rhdh-plugin-gitops-updater@v1 + with: + config-path: "charts/rhdh/values.yaml" + github-token: ${{ secrets.GITHUB_TOKEN }} + update-pr-strategy: "grouped" +``` + +Grouping removes only a terminal `-backend` from the full OCI package name and collects updates with the same resulting name and tag channel. Namespaces are preserved; backend modules and packages in different tag channels remain separate. An existing individual update branch is skipped before grouping, so a partner that arrives later gets its own PR without duplicating the original update. Existing PRs are not refreshed; newer target versions can create new PRs, following the separate strategy. + +The `pr-creation-limit` counts newly created PRs per run, and a pair counts as one. Existing branches and failed creation attempts do not consume the limit. PR creation failures are logged and other groups continue, matching the separate strategy. Grouped mode completes discovery before creating PRs: a lookup exception aborts the run without creating any grouped PRs, while a package with no available versions is skipped normally. + ### Custom Configuration Location If your dynamic plugins configuration is at a different YAML path: @@ -137,8 +154,8 @@ Specify custom tag prefixes to filter plugin versions: | `config-path` | Path to the dynamic plugins config YAML file (e.g., `charts/rhdh/values.yaml`) | Yes | - | | `config-location` | Location of the dynamic plugins config inside the YAML file | No | `redhat-developer-hub.dynamicPlugins.plugins` | | `github-token` | GitHub token for API access and PR creation | Yes | - | -| `update-pr-strategy` | PR creation strategy: `separate` or `joint` | No | `separate` | -| `pr-creation-limit` | Maximum number of PRs to create (0 for unlimited, only applies with `separate` strategy) | No | `0` | +| `update-pr-strategy` | PR creation strategy: `separate`, `joint`, or `grouped` | No | `separate` | +| `pr-creation-limit` | Maximum new PRs per run (0 for unlimited, applies to `separate` and `grouped`; a pair counts as one PR) | No | `0` | | `tag-prefixes` | Tag prefixes to consider when checking for plugin updates (newline-separated list) | No | `next__` | | `base-branch` | Base branch for pull requests (overrides the branch the workflow runs on) | No | `main` | | `verbose` | Enable verbose logging (0 = normal, 1 = debug) | No | `0` | @@ -151,6 +168,7 @@ Specify custom tag prefixes to filter plugin versions: 4. **PR Creation**: Creates pull requests based on your chosen strategy: - **Separate**: One PR per plugin update - **Joint**: Single PR with all updates + - **Grouped**: One PR per matching frontend/backend pair in the same tag channel; unmatched updates get individual PRs 5. **Formatting Preservation**: Updates are made via string replacement to preserve your YAML formatting ## Supported Plugin Format diff --git a/action.yml b/action.yml index 0407d68..b8c07e7 100644 --- a/action.yml +++ b/action.yml @@ -16,11 +16,11 @@ inputs: description: 'GitHub token for API access and PR creation' required: true update-pr-strategy: - description: 'PR creation strategy: "separate" for individual PRs per plugin, "joint" for one PR with all updates' + description: 'PR creation strategy: "separate" per plugin, "joint" for all updates, "grouped" for matching frontend/backend packages in the same tag channel' required: false default: 'separate' pr-creation-limit: - description: 'Maximum number of PRs to create (0 for unlimited, only applies when update-pr-strategy is "separate")' + description: 'Maximum new PRs per run (0 for unlimited, applies to "separate" and "grouped"; a grouped pair counts as one PR)' required: false default: '0' verbose: diff --git a/main.py b/main.py index 7ec7e41..fb17dfc 100644 --- a/main.py +++ b/main.py @@ -1,4 +1,6 @@ #!/usr/bin/env python3 +import hashlib +import json import sys from src.constants import ( @@ -19,7 +21,152 @@ RHDHPluginUpdaterConfig, ) from src.updater import RHDHPluginConfigUpdater -from src.utils import build_version_string, rhdh_plugin_needs_update +from src.utils import ( + build_version_string, + group_plugin_updates, + rhdh_plugin_needs_update, +) + + +def _single_plugin_pr_info( + plugin_name: "str", current_version: "str", latest_version: "str" +) -> "tuple[str, str, str]": + values = { + "plugin_name": plugin_name, + "current_version": current_version, + "latest_version": latest_version, + } + return ( + RHDHPluginUpdaterConfig.GH_PR_BRANCH_NAME_BASE.format(**values), + RHDHPluginUpdaterConfig.GH_PR_TITLE_BASE.format(**values), + RHDHPluginUpdaterConfig.GH_PR_BODY_BASE.format(**values), + ) + + +def _plugin_update_details(updates: "list[RHDHPluginUpdate]") -> "str": + body = "" + for update in updates: + plugin = update.rhdh_plugin + current = build_version_string( + plugin.current_version, plugin.current_second_version + ) + latest = build_version_string(update.new_version, update.new_second_version) + body += f"- **{plugin.plugin_name}**: `{current}` ā†’ `{latest}`\n" + return body + ( + "\nšŸ¤– Generated with [RHDH Plugin GitOps Updater]" + "(https://github.com/thepetk/rhdh-plugin-gitops-updater)\n" + ) + + +def _grouped_update_hash( + updates: "list[RHDHPluginUpdate]", base_branch: "str", config_path: "str" +) -> "str": + identities: "list[dict[str, str]]" = [ + { + "package_name": update.rhdh_plugin.package_name, + "plugin_name": update.rhdh_plugin.plugin_name, + "tag_prefix": update.rhdh_plugin.current_tag_prefix or "", + "new_version": str(update.new_version), + "new_second_version": ( + str(update.new_second_version) if update.new_second_version else "" + ), + } + for update in updates + ] + identity = { + "updates": sorted(identities, key=lambda item: tuple(item.values())), + "base_branch": base_branch, + "config_path": config_path, + } + encoded_identity = json.dumps( + identity, sort_keys=True, separators=(",", ":") + ).encode("utf-8") + return hashlib.sha256(encoded_identity).hexdigest()[:10] + + +def _create_grouped_pull_requests( + gh_api_client: "GithubAPIClient", + rhdh_config_updater: "RHDHPluginConfigUpdater", + plugin_updates: "list[RHDHPluginUpdate]", + trimmed_file_path: "str", +) -> "int": + repo = gh_api_client.client.get_repo(GITHUB_REPOSITORY) + eligible_updates: "list[RHDHPluginUpdate]" = [] + for update in plugin_updates: + plugin = update.rhdh_plugin + latest_version_string = build_version_string( + update.new_version, update.new_second_version + ) + separate_branch_name = RHDHPluginUpdaterConfig.GH_PR_BRANCH_NAME_BASE.format( + plugin_name=plugin.plugin_name, + latest_version=latest_version_string, + ) + # Only a confirmed 404 means absent here: lookup errors must not duplicate + # an existing individual update in a new group. + if not gh_api_client._branch_exists(repo, separate_branch_name, strict=True): + eligible_updates.append(update) + + groups = group_plugin_updates(eligible_updates) + prs_created = 0 + for group in groups: + if PR_CREATION_LIMIT > 0 and prs_created >= PR_CREATION_LIMIT: + logger.warning( + f"reached the PR creation limit of {PR_CREATION_LIMIT}, stopping..." + ) + break + + try: + updated_yaml = rhdh_config_updater.bulk_update_rhdh_plugins(group) + + if len(group) == 1: + update = group[0] + plugin = update.rhdh_plugin + current_version_string = build_version_string( + plugin.current_version, plugin.current_second_version + ) + latest_version_string = build_version_string( + update.new_version, update.new_second_version + ) + branch_name, pr_title, pr_body = _single_plugin_pr_info( + plugin.plugin_name, current_version_string, latest_version_string + ) + else: + package_name = group[0].rhdh_plugin.package_name + family_name = package_name.removesuffix("-backend").rsplit("/", 1)[-1] + update_hash = _grouped_update_hash(group, GITHUB_REF, trimmed_file_path) + branch_name = ( + RHDHPluginUpdaterConfig.GH_GROUPED_PR_BRANCH_NAME_BASE.format( + plugin_name=family_name, + update_hash=update_hash, + ) + ) + pr_title = RHDHPluginUpdaterConfig.GH_GROUPED_PR_TITLE_BASE.format( + plugin_name=family_name, + plugin_updates_count=len(group), + ) + pr_body = ( + "## Grouped Plugin Update\n\n" + f"This PR updates {len(group)} related RHDH plugin packages " + f"for `{family_name}`:\n\n" + ) + pr_body += _plugin_update_details(group) + + pr_url = gh_api_client.create_pull_request( + repo_full_name=GITHUB_REPOSITORY, + file_path=trimmed_file_path, + new_content=updated_yaml, + branch_name=branch_name, + pr_title=pr_title, + pr_body=pr_body, + base_branch=GITHUB_REF, + ) + prs_created += 1 + logger.info(f"āœ“ Created grouped PR: {pr_url}") + + except GithubPRFailedException as e: + logger.warning(f"āœ— Failed to create grouped PR: {e}") + + return prs_created def main(): @@ -41,7 +188,7 @@ def main(): logger.info(f"found {len(rhdh_plugins)} RHDH plugins to check for updates") - # list to cache all updates in case of joint strategy + # list to cache updates for joint and grouped strategies plugin_updates: "list[RHDHPluginUpdate]" = [] prs_created = 0 @@ -88,8 +235,11 @@ def main(): f"{latest_version_string} (current: {current_version_string})" ) - if UPDATE_PR_STRATEGY == GithubPullRequestStrategy.JOINT: - logger.debug("caching plugin update for joint PR...") + if UPDATE_PR_STRATEGY in ( + GithubPullRequestStrategy.JOINT, + GithubPullRequestStrategy.GROUPED, + ): + logger.debug("caching plugin update for deferred PR creation...") plugin_updates.append( RHDHPluginUpdate( rhdh_plugin=plugin, @@ -109,24 +259,17 @@ def main(): updated_yaml = rhdh_config_updater.update_rhdh_plugin( plugin, latest_version, latest_second_version ) + branch_name, pr_title, pr_body = _single_plugin_pr_info( + plugin.plugin_name, current_version_string, latest_version_string + ) pr_url = gh_api_client.create_pull_request( repo_full_name=GITHUB_REPOSITORY, file_path=trimmed_file_path, new_content=updated_yaml, - branch_name=RHDHPluginUpdaterConfig.GH_PR_BRANCH_NAME_BASE.format( - plugin_name=plugin.plugin_name, - latest_version=latest_version_string, - ), - pr_title=RHDHPluginUpdaterConfig.GH_PR_TITLE_BASE.format( - plugin_name=plugin.plugin_name, - latest_version=latest_version_string, - ), - pr_body=RHDHPluginUpdaterConfig.GH_PR_BODY_BASE.format( - plugin_name=plugin.plugin_name, - current_version=current_version_string, - latest_version=latest_version_string, - ), + branch_name=branch_name, + pr_title=pr_title, + pr_body=pr_body, base_branch=GITHUB_REF, ) logger.info(f"āœ“ Created PR: {pr_url}") @@ -135,7 +278,16 @@ def main(): except GithubPRFailedException as e: logger.warning(f"āœ— Failed to create PR for {plugin.plugin_name}: {e}") - if plugin_updates: + if UPDATE_PR_STRATEGY == GithubPullRequestStrategy.GROUPED and plugin_updates: + logger.info(f"creating grouped PRs for {len(plugin_updates)} plugin updates...") + prs_created += _create_grouped_pull_requests( + gh_api_client, + rhdh_config_updater, + plugin_updates, + trimmed_file_path, + ) + + elif plugin_updates: logger.info(f"creating joint PR for {len(plugin_updates)} plugin updates...") try: updated_yaml = rhdh_config_updater.bulk_update_rhdh_plugins(plugin_updates) @@ -143,24 +295,7 @@ def main(): pr_body = RHDHPluginUpdaterConfig.GH_BULK_PR_BODY_BASE.format( plugin_updates_count=len(plugin_updates) ) - for update in plugin_updates: - update_current = build_version_string( - update.rhdh_plugin.current_version, - update.rhdh_plugin.current_second_version, - ) - update_new = build_version_string( - update.new_version, update.new_second_version - ) - pr_body += ( - f"- **{update.rhdh_plugin.plugin_name}**: " - f"`{update_current}` ā†’ " - f"`{update_new}`\n" - ) - - pr_body += ( - "\nšŸ¤– Generated with [RHDH Plugin GitOps Updater]" - "(https://github.com/thepetk/rhdh-plugin-gitops-updater)\n" - ) + pr_body += _plugin_update_details(plugin_updates) pr_url = gh_api_client.create_pull_request( repo_full_name=GITHUB_REPOSITORY, diff --git a/src/constants.py b/src/constants.py index f7241e2..892c9cb 100644 --- a/src/constants.py +++ b/src/constants.py @@ -34,10 +34,10 @@ GITHUB_TOKEN = os.getenv("GITHUB_TOKEN", "") # UPDATE_PR_STRATEGY: is the strategy to use when creating -# the github pull requests. It can be either "separate" or "joint". +# the github pull requests. It can be "separate", "joint", or "grouped". UPDATE_PR_STRATEGY = os.getenv("UPDATE_PR_STRATEGY", "separate") -# PR_CREATION_LIMIT: is the maximum number of pull requests to create. +# PR_CREATION_LIMIT: maximum new PRs per run for separate or grouped strategies. PR_CREATION_LIMIT = int(os.getenv("PR_CREATION_LIMIT", "0")) # GITHUB_REPOSITORY: is the Github repository where PRs will be diff --git a/src/github_api_client.py b/src/github_api_client.py index 0e9b262..e75c590 100644 --- a/src/github_api_client.py +++ b/src/github_api_client.py @@ -2,7 +2,7 @@ from urllib.parse import quote import requests -from github import Auth, Github +from github import Auth, Github, GithubException from github.ContentFile import ContentFile from github.Repository import Repository from requests import Response @@ -163,7 +163,9 @@ def fetch_package( package_name, raw_versions, tag_prefix_filter ) - def _branch_exists(self, repo: "Repository", branch_name: "str") -> "bool": + def _branch_exists( + self, repo: "Repository", branch_name: "str", *, strict: "bool" = False + ) -> "bool": """ checks if a branch exists in the given repository """ @@ -171,7 +173,11 @@ def _branch_exists(self, repo: "Repository", branch_name: "str") -> "bool": repo.get_git_ref(f"heads/{branch_name}") logger.debug(f"branch {branch_name} already exists") return True - except Exception: + except Exception as error: + if strict and ( + not isinstance(error, GithubException) or error.status != 404 + ): + raise logger.debug(f"branch {branch_name} does not exist, will create it") return False @@ -209,8 +215,11 @@ def create_pull_request( base_sha = base_ref.object.sha if self._branch_exists(repo, branch_name): - # skip for separate pr strategy - if UPDATE_PR_STRATEGY == GithubPullRequestStrategy.SEPARATE: + # skip for separate and grouped PR strategies + if UPDATE_PR_STRATEGY in ( + GithubPullRequestStrategy.SEPARATE, + GithubPullRequestStrategy.GROUPED, + ): logger.debug(f"checking for existing open PR for branch {branch_name}") raise GithubPRFailedException(f"Branch {branch_name} already exists") diff --git a/src/types.py b/src/types.py index ed81278..eec5253 100644 --- a/src/types.py +++ b/src/types.py @@ -8,6 +8,7 @@ class GithubPullRequestStrategy: SEPARATE = "separate" JOINT = "joint" + GROUPED = "grouped" @dataclass @@ -68,6 +69,11 @@ class RHDHPluginUpdaterConfig: "chore(`rhdh-plugin-gitops-updater`) Update `{plugin_name}` " "to version `{latest_version}`" ) + GH_GROUPED_PR_BRANCH_NAME_BASE = "update-grouped-{plugin_name}-{update_hash}" + GH_GROUPED_PR_TITLE_BASE = ( + "chore(`rhdh-plugin-gitops-updater`) Update `{plugin_name}` " + "({plugin_updates_count} packages)" + ) GH_PR_BODY_BASE = """## Plugin Update **Plugin**: `{plugin_name}` diff --git a/src/utils.py b/src/utils.py index 632da50..93f223d 100644 --- a/src/utils.py +++ b/src/utils.py @@ -4,7 +4,7 @@ from packaging.version import Version from src.constants import logger -from src.types import RHDHPluginUpdaterConfig +from src.types import RHDHPluginUpdate, RHDHPluginUpdaterConfig def get_plugins_list_from_dict( @@ -111,3 +111,18 @@ def match_tag_prefix(tag: "str") -> "str | None": if tag.startswith(prefix): return prefix return None + + +def group_plugin_updates( + updates: "list[RHDHPluginUpdate]", +) -> "list[list[RHDHPluginUpdate]]": + """Group by full package name without a terminal -backend, and tag channel.""" + groups: dict[tuple[str, str | None], list[RHDHPluginUpdate]] = {} + for update in updates: + plugin = update.rhdh_plugin + key = ( + plugin.package_name.removesuffix("-backend"), + plugin.current_tag_prefix, + ) + groups.setdefault(key, []).append(update) + return list(groups.values()) diff --git a/tests/test_github_api_client.py b/tests/test_github_api_client.py index e436e0f..caa3cd9 100644 --- a/tests/test_github_api_client.py +++ b/tests/test_github_api_client.py @@ -3,6 +3,7 @@ import pytest import requests +from github import GithubException from github.ContentFile import ContentFile from packaging.version import Version @@ -436,22 +437,47 @@ class TestBranchExists: handles all tests for _branch_exists method. """ - def test_branch_exists_true(self, github_client: "GithubAPIClient") -> "None": - mock_repo = Mock() - mock_repo.get_git_ref.return_value = Mock() - - result = github_client._branch_exists(mock_repo, "test-branch") - - assert result is True - mock_repo.get_git_ref.assert_called_once_with("heads/test-branch") + @pytest.mark.parametrize( + ("strict", "error", "expected"), + [ + (False, None, True), + (True, None, True), + (False, Exception("Branch not found"), False), + (True, GithubException(404, {"message": "Not Found"}), False), + ], + ) + def test_branch_exists( + self, + github_client: "GithubAPIClient", + strict: "bool", + error: "Exception | None", + expected: "bool", + ) -> "None": + repo = Mock() + repo.get_git_ref.side_effect = error - def test_branch_exists_false(self, github_client: "GithubAPIClient") -> "None": - mock_repo = Mock() - mock_repo.get_git_ref.side_effect = Exception("Branch not found") + assert ( + github_client._branch_exists(repo, "test-branch", strict=strict) is expected + ) + repo.get_git_ref.assert_called_once_with("heads/test-branch") + + @pytest.mark.parametrize( + "error", + [ + GithubException(500, {"message": "Server Error"}), + requests.Timeout("GitHub API timed out"), + ], + ) + def test_strict_branch_lookup_propagates_errors( + self, github_client: "GithubAPIClient", error: "Exception" + ) -> "None": + repo = Mock() + repo.get_git_ref.side_effect = error - result = github_client._branch_exists(mock_repo, "test-branch") + with pytest.raises(type(error)) as exc_info: + github_client._branch_exists(repo, "test-branch", strict=True) - assert result is False + assert exc_info.value is error class TestHandleNewEndline: @@ -505,12 +531,11 @@ class TestCreatePullRequest: handles all tests for create_pull_request method. """ - @patch( - "src.github_api_client.UPDATE_PR_STRATEGY", GithubPullRequestStrategy.SEPARATE - ) + @pytest.mark.parametrize("strategy", ["separate", "grouped"]) def test_create_pr_success_new_branch( - self, github_client: "GithubAPIClient" + self, github_client: "GithubAPIClient", monkeypatch: "Any", strategy: "str" ) -> "None": + monkeypatch.setattr("src.github_api_client.UPDATE_PR_STRATEGY", strategy) # Mock repository and related objects mock_repo = Mock() mock_base_ref = Mock() @@ -544,16 +569,17 @@ def test_create_pr_success_new_branch( ) assert result == "https://github.com/owner/repo/pull/1" - mock_repo.create_git_ref.assert_called_once() + mock_repo.create_git_ref.assert_called_once_with( + "refs/heads/update-plugin", "base_sha_123" + ) mock_repo.update_file.assert_called_once() mock_repo.create_pull.assert_called_once() - @patch( - "src.github_api_client.UPDATE_PR_STRATEGY", GithubPullRequestStrategy.SEPARATE - ) - def test_create_pr_branch_exists_separate_strategy( - self, github_client: "GithubAPIClient" + @pytest.mark.parametrize("strategy", ["separate", "grouped"]) + def test_create_pr_branch_exists_separate_or_grouped_strategy( + self, github_client: "GithubAPIClient", monkeypatch: "Any", strategy: "str" ) -> "None": + monkeypatch.setattr("src.github_api_client.UPDATE_PR_STRATEGY", strategy) mock_repo = Mock() mock_base_ref = Mock() mock_base_ref.object.sha = "base_sha_123" @@ -575,6 +601,11 @@ def test_create_pr_branch_exists_separate_strategy( assert "already exists" in str(exc_info.value) + mock_repo.get_pulls.assert_not_called() + mock_repo.create_git_ref.assert_not_called() + mock_repo.update_file.assert_not_called() + mock_repo.create_pull.assert_not_called() + @patch("src.github_api_client.UPDATE_PR_STRATEGY", GithubPullRequestStrategy.JOINT) def test_create_pr_branch_exists_with_open_pr( self, github_client: "GithubAPIClient" diff --git a/tests/test_main.py b/tests/test_main.py index 5d3bdf0..6e62403 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,17 +1,94 @@ +from dataclasses import replace +from functools import partial from typing import Any from unittest.mock import Mock, patch import pytest +from github import GithubException from packaging.version import Version -from main import main +from main import _grouped_update_hash, main from src.exceptions import GithubPRFailedException +from src.github_api_client import GithubAPIClient from src.types import ( GithubPullRequestStrategy, RHDHPlugin, RHDHPluginPackage, RHDHPluginPackageVersion, + RHDHPluginUpdate, + RHDHPluginUpdaterConfig, ) +from src.updater import RHDHPluginConfigUpdater + + +def _configure_grouped_main( + monkeypatch: "Any", + items: "list[tuple[RHDHPlugin, RHDHPluginPackage]]", + pr_creation_limit: "int" = 0, + config_path: "str" = "dynamic-plugins.yaml", +) -> "tuple[Mock, Mock]": + monkeypatch.setattr("main.GITHUB_REPOSITORY", "owner/repo") + monkeypatch.setattr("main.GITHUB_TOKEN", "test_token") + monkeypatch.setattr("main.GITHUB_REF", "main") + monkeypatch.setattr("main.DYNAMIC_PLUGINS_CONFIG_YAML_FILE_PATH", config_path) + monkeypatch.setattr("main.UPDATE_PR_STRATEGY", "grouped") + monkeypatch.setattr("main.PR_CREATION_LIMIT", pr_creation_limit) + + loader = Mock() + loader.load_rhdh_plugins.return_value = [plugin for plugin, _ in items] + monkeypatch.setattr("main.RHDHPluginsConfigLoader", Mock(return_value=loader)) + + api = Mock() + api._branch_exists.return_value = False + api.fetch_package.side_effect = [package for _, package in items] + api.create_pull_request.return_value = "https://github.com/owner/repo/pull/1" + monkeypatch.setattr("main.GithubAPIClient", Mock(return_value=api)) + + updater = Mock() + updater.bulk_update_rhdh_plugins.return_value = "updated yaml content" + monkeypatch.setattr("main.RHDHPluginConfigUpdater", Mock(return_value=updater)) + return api, updater + + +def _plugin_and_package( + package_name: "str", + plugin_name: "str", + current_version: "str", + new_version: "str", + tag_prefix: "str | None" = None, +) -> "tuple[RHDHPlugin, RHDHPluginPackage]": + current, _, current_second = current_version.partition("__") + target, _, target_second = new_version.partition("__") + plugin = RHDHPlugin( + package_name=package_name, + current_version=Version(current), + current_second_version=Version(current_second) if current_second else None, + plugin_name=plugin_name, + disabled=False, + current_tag_prefix=tag_prefix, + ) + package = RHDHPluginPackage( + name=package_name, + versions=[ + RHDHPluginPackageVersion( + name=f"{plugin_name}-latest", + version=Version(target), + second_version=Version(target_second) if target_second else None, + created_at="2024-01-20T10:00:00Z", + ) + ], + ) + return plugin, package + + +@pytest.fixture +def plugin_pair() -> "list[tuple[RHDHPlugin, RHDHPluginPackage]]": + return [ + _plugin_and_package("scope/pluginabc", "pluginabc", "1.0.0", "1.1.0"), + _plugin_and_package( + "scope/pluginabc-backend", "pluginabc-backend", "2.0.0", "2.1.0" + ), + ] class TestMain: @@ -47,15 +124,19 @@ def test_main_no_plugins_found( mock_loader.load_rhdh_plugins.assert_called_once() + @pytest.mark.parametrize("strategy", ["separate", "grouped"]) @patch("main.GITHUB_REPOSITORY", "owner/repo") @patch("main.GITHUB_TOKEN", "test_token") - @patch("main.UPDATE_PR_STRATEGY", GithubPullRequestStrategy.SEPARATE) @patch("main.PR_CREATION_LIMIT", 0) @patch("main.GithubAPIClient") @patch("main.RHDHPluginsConfigLoader") @patch("main.RHDHPluginConfigUpdater") def test_main_with_plugins_up_to_date( - self, mock_updater_class: "Any", mock_loader_class: "Any", mock_api_class: "Any" + self, + mock_updater_class: "Any", + mock_loader_class: "Any", + mock_api_class: "Any", + strategy: "str", ) -> "None": mock_plugin = RHDHPlugin( package_name="test-package", @@ -82,7 +163,8 @@ def test_main_with_plugins_up_to_date( mock_api.fetch_package.return_value = mock_package mock_api_class.return_value = mock_api - main() + with patch("main.UPDATE_PR_STRATEGY", strategy): + main() mock_loader.load_rhdh_plugins.assert_called_once() mock_api.fetch_package.assert_called_once_with( @@ -91,6 +173,8 @@ def test_main_with_plugins_up_to_date( # no pr should be created mock_api.create_pull_request.assert_not_called() + mock_api._branch_exists.assert_not_called() + @patch("main.GITHUB_REPOSITORY", "owner/repo") @patch("main.GITHUB_TOKEN", "test_token") @patch("main.GITHUB_REF", "main") @@ -597,3 +681,205 @@ def fetch_package_side_effect( mock_api.fetch_package.assert_any_call( "test-package-2", tag_prefix_filter="stable__" ) + + def test_main_grouped_pair_and_unrelated_update_are_isolated( + self, monkeypatch: "Any", tmp_path: "Any" + ) -> "None": + package_root = "rhdh-plugin-export-overlays/" + versions = [ + ("pluginabc", "1.45.3__1.2.3", "1.45.3__1.2.4", "bs_"), + ("pluginabc-backend", "1.44.0__0.9.0", "1.46.0__1.2.5", "bs_"), + ("pluginxyz", "9.0.0", "9.0.1", "next__"), + ] + items = [ + _plugin_and_package(package_root + name, name, current, target, prefix) + for name, current, target, prefix in versions + ] + original = "plugins:\n # preserve this comment\n" + "".join( + f" - package: oci://ghcr.io/redhat-developer/{package_root}" + f"{name}:{prefix}{current}\n" + for name, current, _, prefix in versions + ) + config_path = tmp_path / "plugins.yaml" + config_path.write_text(original) + api, _ = _configure_grouped_main( + monkeypatch, + items, + config_path=str(config_path), + ) + monkeypatch.setattr( + RHDHPluginUpdaterConfig, "GH_PACKAGE_TAG_PREFIX", ["next__", "bs_"] + ) + monkeypatch.setattr( + "main.RHDHPluginConfigUpdater", + lambda: RHDHPluginConfigUpdater(config_path=str(config_path)), + ) + + main() + + calls = [call.kwargs for call in api.create_pull_request.call_args_list] + assert len(calls) == 2 + pair, singleton = calls + expected_pair = original + for name, current, target, prefix in versions[:2]: + expected_pair = expected_pair.replace( + f"{name}:{prefix}{current}", f"{name}:{prefix}{target}" + ) + assert f"**{name}**: `{current}` ā†’ `{target}`" in pair["pr_body"] + assert pair["new_content"] == expected_pair + assert pair["branch_name"].startswith("update-grouped-pluginabc-") + assert "pluginabc" in pair["pr_title"] + assert "2 packages" in pair["pr_title"] + assert "pluginxyz" not in pair["pr_body"] + assert singleton["new_content"] == original.replace( + "pluginxyz:next__9.0.0", "pluginxyz:next__9.0.1" + ) + assert singleton["branch_name"] == "update-pluginxyz-9.0.1" + assert singleton["pr_title"] == RHDHPluginUpdaterConfig.GH_PR_TITLE_BASE.format( + plugin_name="pluginxyz", latest_version="9.0.1" + ) + assert singleton["pr_body"] == RHDHPluginUpdaterConfig.GH_PR_BODY_BASE.format( + plugin_name="pluginxyz", current_version="9.0.0", latest_version="9.0.1" + ) + assert config_path.read_text() == original + + @pytest.mark.parametrize("existing_index", [0, 1]) + def test_main_grouped_does_not_readd_existing_separate_update( + self, + monkeypatch: "Any", + plugin_pair: "Any", + existing_index: "int", + ) -> "None": + api, updater = _configure_grouped_main(monkeypatch, plugin_pair) + existing_plugin, existing_package = plugin_pair[existing_index] + remaining_plugin, remaining_package = plugin_pair[1 - existing_index] + existing_branch = RHDHPluginUpdaterConfig.GH_PR_BRANCH_NAME_BASE.format( + plugin_name=existing_plugin.plugin_name, + latest_version=existing_package.versions[0].version, + ) + api._branch_exists.side_effect = ( + lambda _repo, branch, *, strict: branch == existing_branch + ) + + main() + + updater.bulk_update_rhdh_plugins.assert_called_once() + remaining = updater.bulk_update_rhdh_plugins.call_args.args[0] + assert [update.rhdh_plugin for update in remaining] == [remaining_plugin] + assert api.create_pull_request.call_count == 1 + assert api.create_pull_request.call_args.kwargs["branch_name"] == ( + f"update-{remaining_plugin.plugin_name}-{remaining_package.versions[0].version}" + ) + + def test_main_grouped_branch_lookup_failure_creates_no_prs( + self, monkeypatch: "Any", plugin_pair: "Any", github_client: "GithubAPIClient" + ) -> "None": + api, updater = _configure_grouped_main(monkeypatch, plugin_pair) + github_client.fetch_package = api.fetch_package + github_client.create_pull_request = api.create_pull_request + repo = Mock() + repo.get_git_ref.side_effect = GithubException( + 500, {"message": "lookup failed"} + ) + github_client.client.get_repo.return_value = repo + monkeypatch.setattr("main.GithubAPIClient", Mock(return_value=github_client)) + + with pytest.raises(GithubException, match="lookup failed"): + main() + + api.create_pull_request.assert_not_called() + updater.bulk_update_rhdh_plugins.assert_not_called() + + @pytest.mark.parametrize("creation_fails", [False, True]) + def test_main_grouped_limit_counts_successful_prs_and_keeps_pairs_together( + self, monkeypatch: "Any", plugin_pair: "Any", creation_fails: "bool" + ) -> "None": + unrelated = _plugin_and_package( + "scope/pluginxyz", "pluginxyz", "4.0.0", "4.1.0" + ) + api, updater = _configure_grouped_main( + monkeypatch, [*plugin_pair, unrelated], pr_creation_limit=1 + ) + if creation_fails: + api.create_pull_request.side_effect = [ + GithubPRFailedException("temporary failure"), + "https://github.com/owner/repo/pull/2", + ] + + main() + + expected_attempts = 2 if creation_fails else 1 + assert api.create_pull_request.call_count == expected_attempts + assert updater.bulk_update_rhdh_plugins.call_count == expected_attempts + updates = updater.bulk_update_rhdh_plugins.call_args_list[0].args[0] + assert [update.rhdh_plugin for update in updates] == [ + plugin for plugin, _ in plugin_pair + ] + if creation_fails: + assert api.create_pull_request.call_args.kwargs["branch_name"] == ( + "update-pluginxyz-4.1.0" + ) + + def test_main_grouped_discovery_failure_creates_no_prs( + self, monkeypatch: "Any", plugin_pair: "Any" + ) -> "None": + api, updater = _configure_grouped_main(monkeypatch, plugin_pair) + api.fetch_package.side_effect = [ + plugin_pair[0][1], + RuntimeError("package lookup failed"), + ] + + with pytest.raises(RuntimeError, match="package lookup failed"): + main() + + api.create_pull_request.assert_not_called() + api._branch_exists.assert_not_called() + updater.bulk_update_rhdh_plugins.assert_not_called() + + def test_main_grouped_skips_packages_without_versions( + self, monkeypatch: "Any", plugin_pair: "Any" + ) -> "None": + plugin_pair[0][1].versions = [] + api, updater = _configure_grouped_main(monkeypatch, plugin_pair) + + main() + + assert api.create_pull_request.call_count == 1 + updates = updater.bulk_update_rhdh_plugins.call_args.args[0] + assert [update.rhdh_plugin for update in updates] == [plugin_pair[1][0]] + + def test_grouped_hash_tracks_update_identity_and_ignores_input_order( + self, plugin_pair: "Any" + ) -> "None": + updates = [ + RHDHPluginUpdate(plugin, package.versions[0].version) + for plugin, package in plugin_pair + ] + fingerprint = partial( + _grouped_update_hash, base_branch="main", config_path="dynamic-plugins.yaml" + ) + original = fingerprint(updates) + assert fingerprint(updates[::-1]) == original + assert fingerprint(updates, config_path="overlays/plugins.yaml") != original + assert fingerprint(updates, base_branch="release-1.0") != original + + frontend, backend = updates + fingerprints = {original} + for changed in [ + replace(frontend, new_version=Version("1.2.0")), + replace(frontend, new_second_version=Version("5.1.0")), + replace(frontend, new_second_version=Version("5.2.0")), + replace( + frontend, + rhdh_plugin=replace(frontend.rhdh_plugin, current_tag_prefix="bs_"), + ), + ]: + fingerprints.add(fingerprint([changed, backend])) + assert len(fingerprints) == 5 + repeated_package = replace( + frontend, + rhdh_plugin=replace(frontend.rhdh_plugin, plugin_name="another-alias"), + ) + repeated_updates = [*updates, repeated_package] + assert fingerprint(repeated_updates) != original + assert fingerprint(repeated_updates[::-1]) == fingerprint(repeated_updates) diff --git a/tests/test_utils.py b/tests/test_utils.py index 6b3b50f..80084f8 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -3,10 +3,12 @@ import pytest from packaging.version import Version +from src.types import RHDHPlugin, RHDHPluginUpdate from src.utils import ( build_version_string, compare_versions, get_plugins_list_from_dict, + group_plugin_updates, parse_dual_version, rhdh_plugin_needs_update, ) @@ -308,3 +310,82 @@ def test_dual_version(self) -> "None": def test_none_second_version(self) -> "None": result = build_version_string(Version("1.0.0"), None) assert result == "1.0.0" + + +class TestGroupPluginUpdates: + """Groups updates by package family and tag channel.""" + + @staticmethod + def update( + package_name: str, + *, + plugin_name: str | None = None, + prefix: str = "next__", + version: str = "1.0.0", + ) -> RHDHPluginUpdate: + return RHDHPluginUpdate( + rhdh_plugin=RHDHPlugin( + package_name=package_name, + current_version=Version("0.1.0"), + plugin_name=plugin_name or package_name.rsplit("/", 1)[-1], + disabled=False, + current_tag_prefix=prefix, + ), + new_version=Version(version), + ) + + def test_keeps_backend_only_and_non_exact_namespaces_or_suffixes_separate( + self, + ) -> "None": + updates = [ + self.update("scope/orphan-backend"), + self.update("scope/plugin"), + self.update("other/plugin-backend"), + self.update("scope/plugin-backend-module"), + self.update("scope/plugin-extra"), + self.update("scope/plugin-backend-extra"), + ] + + groups = group_plugin_updates(updates) + + assert groups == [[update] for update in updates] + + def test_groups_exact_families_and_channels_in_first_seen_order(self) -> "None": + assistant = self.update( + "scope/intelligent-assistant", + plugin_name="frontend-alias", + prefix="bs_", + version="2.0.0", + ) + assistant_backend = self.update( + "scope/intelligent-assistant-backend", + plugin_name="backend-alias", + prefix="bs_", + version="3.0.0", + ) + orchestrator = self.update("scope/orchestrator", prefix="next__") + orchestrator_backend = self.update("scope/orchestrator-backend", prefix="bs_") + topology = self.update("scope/topology", prefix="bs_") + + groups = group_plugin_updates( + [assistant, orchestrator, topology, assistant_backend, orchestrator_backend] + ) + + assert groups == [ + [assistant, assistant_backend], + [orchestrator], + [topology], + [orchestrator_backend], + ] + + def test_groups_in_reverse_input_order_and_preserves_duplicate_records( + self, + ) -> "None": + frontend = self.update("scope/plugin") + backend = self.update("scope/plugin-backend") + updates = [backend, frontend, backend, frontend] + + groups = group_plugin_updates(updates) + + assert groups == [[backend, frontend, backend, frontend]] + assert [update for group in groups for update in group] == updates