Community merge conflicts (#113)

* add function that retrieves the file contents * add tests for retrieving content of a file * remove unnecessary None alternative * make repository client available to reconcile * add content from default branch to content change * add merge conflict module * add tests for content module * add module that checks actions * add check for any content conflicts * add reconcile test with merge conflicts * add content merging * separate action tests into multiple modules * add github token to e2e tests * move requirements to requirements.txt files * add support for the case where the file was not found on the branch * add tests for no base * add support for retrieving the file from a particular branch * add base branch input * add tests for that file is retrieved from the correct branch * add integration test for merge conflict * update docs * aggregate clients into a tuple * add update e2e test * move writing GitGHub output to separate module
canonical · Apr 4, 2023 · 69bfd6d · 69bfd6d
1 parent 046a6c0
commit 69bfd6d
Show file tree

Hide file tree

Showing 43 changed files with 2,895 additions and 1,193 deletions.
diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml
@@ -5,6 +5,7 @@ on:
 
 jobs:
   e2e-tests-reconcile:
+    permissions: write-all
     runs-on: ubuntu-22.04
     steps:
       # Each job has to have this configuration because secrets can be passed through the output of
@@ -56,6 +57,7 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
           dry_run: true
       - name: Show pages
         run: echo '${{ steps.e2e-test-draft.outputs.urls_with_actions }}'
@@ -71,6 +73,7 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
       - name: Show pages
         run: echo '${{ steps.e2e-test-create.outputs.urls_with_actions }}'
       - name: Check create
@@ -80,6 +83,28 @@ jobs:
         run: |
           echo "docs: ${{ steps.e2e-test-create.outputs.index_url }}" >> metadata.yaml
           cat metadata.yaml
+      - name: Prepare for update test
+        id: e2e-test-prepare-update
+        run: |
+          # Update the contents of one of the files, do it first to get around content conflicts
+          echo -n more content >> docs/doc.md
+          GITHUB_TOKEN='${{ secrets.GITHUB_TOKEN }}'
+          REPO='${{ github.repository }}'
+          PYTHONPATH=$(pwd) python3 prepare_check_cleanup/reconcile.py --action prepare-update --action-kwargs "{\"github_token\": \"$GITHUB_TOKEN\", \"repo\": \"$REPO\", \"filename\": \"docs/doc.md\"}" '${{ steps.e2e-test-create.outputs.urls_with_actions }}' '${{ steps.configuration.outputs.discourse }}'
+      - name: Update self test
+        id: e2e-test-update
+        uses: ./
+        with:
+          discourse_host: discourse.charmhub.io
+          discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
+          discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
+          base_branch: ${{ steps.e2e-test-prepare-update.outputs.update_branch }}
+      - name: Show pages
+        run: echo '${{ steps.e2e-test-update.outputs.urls_with_actions }}'
+      - name: Check update
+        run: |
+          PYTHONPATH=$(pwd) python3 prepare_check_cleanup/reconcile.py --action check-update --action-kwargs '{"expected_url_results": ["success", "success", "success"]}' '${{ steps.e2e-test-create.outputs.urls_with_actions }}' '${{ steps.configuration.outputs.discourse }}'
       - name: Delete the alternate doc with delete_topics disabled
         run: rm docs/alternate_doc.md
       - name: Delete topics disabled self test
@@ -90,6 +115,8 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
+          base_branch: ${{ steps.e2e-test-prepare-update.outputs.update_branch }}
       - name: Show pages
         run: echo '${{ steps.e2e-test-delete-topics.outputs.urls_with_actions }}'
       - name: Check delete topics disabled
@@ -104,6 +131,8 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
+          base_branch: ${{ steps.e2e-test-prepare-update.outputs.update_branch }}
       - name: Show pages
         run: echo '${{ steps.e2e-test-delete.outputs.urls_with_actions }}'
       - name: Check delete topics enabled
@@ -112,7 +141,9 @@ jobs:
       - name: Clean up
         if: always()
         run: |
-          PYTHONPATH=$(pwd) python3 prepare_check_cleanup/reconcile.py --action cleanup '${{ steps.e2e-test-create.outputs.urls_with_actions }}' '${{ steps.configuration.outputs.discourse }}'
+          GITHUB_TOKEN='${{ secrets.GITHUB_TOKEN }}'
+          REPO='${{ github.repository }}'
+          PYTHONPATH=$(pwd) python3 prepare_check_cleanup/reconcile.py --action cleanup --action-kwargs "{\"github_token\": \"$GITHUB_TOKEN\", \"repo\": \"$REPO\"}" '${{ steps.e2e-test-create.outputs.urls_with_actions }}' '${{ steps.configuration.outputs.discourse }}'
   e2e-tests-migration:
     permissions: write-all
     runs-on: ubuntu-22.04

diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml
@@ -8,4 +8,4 @@ jobs:
     uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main
     secrets: inherit
     with:
-      modules: '["discourse", "reconcile", "migrate"]'
+      modules: '["discourse", "reconcile", "conflict", "migrate"]'
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -18,5 +18,5 @@ jobs:
       - name: Check for any code improvements
         run: |
           PYTHON_FILES=$(find . -name '*.py')
-          pyupgrade --py310-plus $PYTHON_FILES
+          pyupgrade --py311-plus $PYTHON_FILES
           refurb $PYTHON_FILES
diff --git a/README.md b/README.md
@@ -55,6 +55,7 @@ charmhub.
               discourse_host: discourse.charmhub.io
               discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
               discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+              github_token: ${{ secrets.GITHUB_TOKEN }}
           - name: Show index page
             run: echo '${{ steps.publishDocumentation.outputs.index_url }}'
     ```
@@ -101,12 +102,12 @@ charmhub.
               github_token: ${{ secrets.GITHUB_TOKEN }}
     ```
 
-    a branch name with `upload-charm-docs/migrate` will be created and a pull 
-    request named `[upload-charm-docs] Migrate charm docs` will be created 
-    towards the working branch the workflow was triggered with. 
-    In order to ensure that the branches can be created successfully, please 
-    make sure that there are no existing branches clashing with the name above.
-    Please note that `dry_run` parameter has no effect on migrate mode.
+    a branch name with `upload-charm-docs/migrate` will be created and a pull
+    request named `[upload-charm-docs] Migrate charm docs` will be created
+    targeting the default branch of the repository. In order to ensure that the
+    branches can be created successfully, please make sure that there are no
+    existing branches clashing with the name above. Please note that the
+    `dry_run` input has no effect on migrate mode.
 
 The action will now compare the discourse topics with the files and directories
 under the `docs` directory and make any changes based on differences.
@@ -117,3 +118,19 @@ Additional recommended steps:
   publish a new version of the charm and documentation.
 * Add the action in dry run mode on publishes to `edge` to see what changes to
   the documentation will be made once you publish to `stable`.
+
+## Discourse Documentation Edits
+
+To ensure that contributions to the documentation on discourse are not
+overridden, the action compares the content that was last pushed to discourse
+with the current documentation on discourse and any proposed changes. If there
+are changes both on discourse and in the repository, the action will attempt to
+merge the content using essentially `git merge`. If that fails, the action will
+prompt you to resolve those conflicts by editing the documentation on discourse
+and on the repository. Be sure to explain the reasoning for the changes on
+discourse.
+
+Note that `git merge` will not make any changes on your branch. The content that
+was last pushed to discourse is determined by getting the content from a given
+file from the default branch of the repository. A different branch can be
+configured using the `base_branch` input.
diff --git a/action.yaml b/action.yaml
@@ -40,6 +40,12 @@ inputs:
       Required if running in migration mode.
     required: false
     type: string
+  base_branch:
+    description: |
+      The name of the branch that contains the documentation after it has been uploaded to
+      discourse. Defaults to the default branch of the repository.
+    required: false
+    type: string
 outputs:
   urls_with_actions:
     description: |

diff --git a/dev-requirements.txt b/dev-requirements.txt
@@ -1,2 +1,6 @@
+ops
+pytest-operator
 factory_boy>=3.2,<3.3
+pytest-asyncio>=0.21,<0.22
+pytest>=7.2,<7.3
 juju==3.0.4
diff --git a/main.py b/main.py
@@ -36,6 +36,7 @@ def _parse_env_vars() -> types_.UserInputs:
     delete_topics = os.getenv("INPUT_DELETE_TOPICS") == "true"
     dry_run = os.getenv("INPUT_DRY_RUN") == "true"
     github_access_token = os.getenv("INPUT_GITHUB_TOKEN")
+    base_branch = os.getenv("INPUT_BASE_BRANCH")
 
     return types_.UserInputs(
         discourse=types_.UserInputsDiscourse(
@@ -47,6 +48,7 @@ def _parse_env_vars() -> types_.UserInputs:
         delete_pages=delete_topics,
         dry_run=dry_run,
         github_access_token=github_access_token,
+        base_branch=base_branch,
     )
 
 

diff --git a/prepare_check_cleanup/migration.py b/prepare_check_cleanup/migration.py
@@ -6,7 +6,6 @@
 import argparse
 import json
 import logging
-import os
 import sys
 from contextlib import suppress
 from enum import Enum
@@ -17,16 +16,16 @@
 from github.PullRequest import PullRequest
 from github.Repository import Repository
 
-from prepare_check_cleanup import exit_
+from prepare_check_cleanup import exit_, output
+from src.constants import (
+    DOCUMENTATION_FOLDER_NAME,
+    DOCUMENTATION_INDEX_FILENAME,
+    NAVIGATION_TABLE_START,
+)
 from src.discourse import create_discourse
 from src.exceptions import DiscourseError
-from src.index import DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_INDEX_FILENAME
-from src.pull_request import (
-    ACTIONS_PULL_REQUEST_TITLE,
-    DEFAULT_BRANCH_NAME,
-    create_repository_client,
-)
-from src.reconcile import NAVIGATION_TABLE_START
+from src.pull_request import DEFAULT_BRANCH_NAME
+from src.repository import ACTIONS_PULL_REQUEST_TITLE, create_repository_client
 
 
 class Action(str, Enum):
@@ -78,8 +77,7 @@ def main() -> None:
         case Action.CHECK_PULL_REQUEST.value:
             exit_.with_result(check_pull_request(**action_kwargs))
         case Action.CLEANUP.value:
-            cleanup(**action_kwargs)
-            sys.exit(0)
+            exit_.with_result(cleanup(**action_kwargs))
         case _:
             raise NotImplementedError(f"{args.action} has not been implemented")
 
@@ -110,14 +108,8 @@ def prepare(index_filename: str, page_filename: str, discourse_config: dict[str,
 
     topics = {"index": index_url, "page": page_url}
 
-    github_output = os.getenv("GITHUB_OUTPUT")
-    assert github_output, (
-        "the GITHUB_OUTPUT environment variable is empty or defined, "
-        "is this running in a GitHub workflow?"
-    )
-    output_file = Path(github_output)
     topics_output = json.dumps(topics, separators=(",", ":")).replace('"', '\\"')
-    output_file.write_text(f"topics={topics_output}\nindex_url={index_url}\n", encoding="utf-8")
+    output.write(f"topics={topics_output}\nindex_url={index_url}\n")
 
 
 def _create_repository_client(github_access_token: str) -> Repository:
@@ -258,21 +250,27 @@ def check_pull_request(github_access_token: str) -> bool:
 
 def cleanup(
     topics: dict[str, str], github_access_token: str, discourse_config: dict[str, str]
-) -> None:
+) -> bool:
     """Clean up testing artifacts on GitHub and Discourse.
 
     Args:
         topics: The discourse topics created for the migration.
         github_access_token: The secret required for interactions with GitHub.
         discourse_config: Details required to communicate with discourse.
+
+    Returns:
+        Whether the cleanup succeeded.
     """
+    result = True
+
     # Delete discourse topics
     discourse = create_discourse(**discourse_config)
     try:
         for topic_url in topics.values():
             discourse.delete_topic(url=topic_url)
     except DiscourseError as exc:
         logging.exception("cleanup failed for discourse, %s", exc)
+        result = False
 
     github_repo = _create_repository_client(github_access_token=github_access_token)
     # Delete the migration PR
@@ -282,13 +280,17 @@ def cleanup(
             migration_pull_request.edit(state="closed")
     except GithubException as exc:
         logging.exception("cleanup failed for migration pull request, %s", exc)
+        result = False
     # Delete the migration branch
     try:
         migration_branch = _get_migration_branch(github_repo=github_repo)
         if migration_branch:
             migration_branch.delete()
     except GithubException as exc:
         logging.exception("cleanup failed for migration branch, %s", exc)
+        result = False
+
+    return result
 
 
 if __name__ == "__main__":

diff --git a/prepare_check_cleanup/output.py b/prepare_check_cleanup/output.py
@@ -0,0 +1,22 @@
+# Copyright 2023 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Functions that support writing output."""
+
+import os
+from pathlib import Path
+
+
+def write(text: str) -> None:
+    """Write text to the GitHub output file.
+
+    Args:
+        text: The content to write to the GitHub output.
+    """
+    github_output = os.getenv("GITHUB_OUTPUT")
+    assert github_output, (
+        "the GITHUB_OUTPUT environment variable is empty or defined, "
+        "is this running in a GitHub workflow?"
+    )
+    output_file = Path(github_output)
+    output_file.write_text(text, encoding="utf-8")