Remove --branch arg

Background: The `--branch` arg was used to create the config PR branch _AND_ to locate the PR branch in case of `create-preview` and `delete-preview`. This is confusing for the user and can potentially lead to branch collisions in the config repo. Solution: - Randomly create branch names for the config repo (if `--create_pr` is given) or directly push to the master branch. - In case of `create-preview`, get the PR branch name through the API via `--pr_id` - In case of `delete-preview` we still need to know the branch name. For this reason, this is the only command which still has the arg `--branch`.
baloise · Mar 19, 2020 · 5baad8e · 5baad8e
1 parent d45bce0
commit 5baad8e
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+- The `--branch` arg of the `delete-preview` command now only refers to the app repo branch. The config repo changes are pushed to `master` by default or to a randomly created branch name if `--create_pr` is given.
+
+### Removed
+- The `--branch` arg was removed for commands `deploy` and `create-preview`. For `create-preview` the app repo branch is detected via the `--pr_id`. All changes in the config repo are pushed to `master` by default, or to a randomly created branch name if `--create_pr` is given.
 
 ## [2.1.0] - 2020-02-09
 ### Added

diff --git a/docs/commands/create-preview.md b/docs/commands/create-preview.md
@@ -70,21 +70,18 @@ gitopscli create-preview \
   --organisation "my-team" \
   --repository-name "app-xy" \
   --pr-id 4711 \
-  --branch "my-pr-branch" \
   --create-pr \
   --auto-merge
 ```
 
-!!! warning "The `--branch` parameter is currently used to locate the PR branch in the *app repository* as well as to create the change branch in the *deployment config repository*. This may lead to branch collisions in the config repository. Hence, the current behavior may be subject to change in the near future. [(Issue #40)](https://github.com/baloise-incubator/gitopscli/issues/40)"
-
 ## Usage
 ```
 usage: gitopscli create-preview [-h] --username USERNAME --password PASSWORD
                                 [--git-user GIT_USER] [--git-email GIT_EMAIL]
                                 --organisation ORGANISATION --repository-name
                                 REPOSITORY_NAME [--git-provider GIT_PROVIDER]
                                 [--git-provider-url GIT_PROVIDER_URL]
-                                [--branch BRANCH] [--create-pr [CREATE_PR]]
+                                [--create-pr [CREATE_PR]]
                                 [--auto-merge [AUTO_MERGE]] --pr-id PR_ID
                                 [--parent-id PARENT_ID] [-v [VERBOSE]]
 
@@ -104,10 +101,8 @@ optional arguments:
   --git-provider-url GIT_PROVIDER_URL
                         Git provider base API URL (e.g.
                         https://bitbucket.example.tld)
-  --branch BRANCH       Branch to push the changes to
   --create-pr [CREATE_PR]
-                        Creates a Pull Request (only when --branch is not
-                        master/default branch)
+                        Creates a Pull Request
   --auto-merge [AUTO_MERGE]
                         Automatically merge the created PR (only valid with
                         --create-pr)

diff --git a/docs/commands/delete-preview.md b/docs/commands/delete-preview.md
@@ -24,8 +24,8 @@ usage: gitopscli delete-preview [-h] --username USERNAME --password PASSWORD
                                 [--git-user GIT_USER] [--git-email GIT_EMAIL]
                                 --organisation ORGANISATION --repository-name
                                 REPOSITORY_NAME [--git-provider GIT_PROVIDER]
-                                [--git-provider-url GIT_PROVIDER_URL]
-                                [--branch BRANCH] [--create-pr [CREATE_PR]]
+                                [--git-provider-url GIT_PROVIDER_URL] --branch
+                                BRANCH [--create-pr [CREATE_PR]]
                                 [--auto-merge [AUTO_MERGE]] [-v [VERBOSE]]
 
 optional arguments:
@@ -44,10 +44,9 @@ optional arguments:
   --git-provider-url GIT_PROVIDER_URL
                         Git provider base API URL (e.g.
                         https://bitbucket.example.tld)
-  --branch BRANCH       Branch to push the changes to
+  --branch BRANCH       The branch for which the preview was created for
   --create-pr [CREATE_PR]
-                        Creates a Pull Request (only when --branch is not
-                        master/default branch)
+                        Creates a Pull Request
   --auto-merge [AUTO_MERGE]
                         Automatically merge the created PR (only valid with
                         --create-pr)

diff --git a/docs/commands/deploy.md b/docs/commands/deploy.md
@@ -27,8 +27,7 @@ gitopscli deploy \
   --organisation "deployment" \
   --repository-name "incubator-non-prod" \
   --file "example/values.yaml" \
-  --values "{frontend.tag: 1.1.0, backend.tag: 1.1.0}" \
-  --branch "master"
+  --values "{frontend.tag: 1.1.0, backend.tag: 1.1.0}"
 ```
 
 ### Number Of Commits
@@ -66,8 +65,6 @@ Date:   Thu Mar 12 15:30:00 2020 +0100
 
 In some cases you might want to create a pull request for your updates. You can achieve this by adding `--create-pr` to the command. The pull request can be left open or merged directly with `--auto-merge`.
 
-!!! info "We recommend creating a random branch name for automatically created pull requests to prevent collisions."
-
 ```bash
 gitopscli deploy \
   --git-provider-url https://bitbucket.baloise.dev \
@@ -79,7 +76,6 @@ gitopscli deploy \
   --repository-name "incubator-non-prod" \
   --file "example/values.yaml" \
   --values "{frontend.tag: 1.1.0, backend.tag: 1.1.0}" \
-  --branch deploy-$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 5 | head -n 1) \
   --create-pr \
   --auto-merge
 ```
@@ -98,8 +94,8 @@ usage: gitopscli deploy [-h] --file FILE --values VALUES
                         --repository-name REPOSITORY_NAME
                         [--git-provider GIT_PROVIDER]
                         [--git-provider-url GIT_PROVIDER_URL]
-                        [--branch BRANCH] [--create-pr [CREATE_PR]]
-                        [--auto-merge [AUTO_MERGE]] [-v [VERBOSE]]
+                        [--create-pr [CREATE_PR]] [--auto-merge [AUTO_MERGE]]
+                        [-v [VERBOSE]]
 
 optional arguments:
   -h, --help            show this help message and exit
@@ -122,10 +118,8 @@ optional arguments:
   --git-provider-url GIT_PROVIDER_URL
                         Git provider base API URL (e.g.
                         https://bitbucket.example.tld)
-  --branch BRANCH       Branch to push the changes to
   --create-pr [CREATE_PR]
-                        Creates a Pull Request (only when --branch is not
-                        master/default branch)
+                        Creates a Pull Request
   --auto-merge [AUTO_MERGE]
                         Automatically merge the created PR (only valid with
                         --create-pr)

diff --git a/gitopscli/cliparser.py b/gitopscli/cliparser.py
@@ -76,6 +76,9 @@ def __add_create_preview_command_parser(subparsers):
 def __add_delete_preview_command_parser(subparsers):
     add_delete_preview_p = subparsers.add_parser("delete-preview", help="Delete a preview environment")
     __add_git_parser_args(add_delete_preview_p)
+    add_delete_preview_p.add_argument(
+        "--branch", help="The branch for which the preview was created for", required=True
+    )
     __add_branch_pr_parser_args(add_delete_preview_p)
     __add_verbose_parser(add_delete_preview_p)
 
@@ -93,14 +96,8 @@ def __add_git_parser_args(deploy_p, api_only=False):
 
 
 def __add_branch_pr_parser_args(deploy_p):
-    deploy_p.add_argument("--branch", help="Branch to push the changes to", default="master")
     deploy_p.add_argument(
-        "--create-pr",
-        help="Creates a Pull Request (only when --branch is not master/default branch)",
-        type=__str2bool,
-        nargs="?",
-        const=True,
-        default=False,
+        "--create-pr", help="Creates a Pull Request", type=__str2bool, nargs="?", const=True, default=False,
     )
     deploy_p.add_argument(
         "--auto-merge",

diff --git a/gitopscli/commands/create_preview.py b/gitopscli/commands/create_preview.py
@@ -14,7 +14,6 @@ def create_preview_command(
     command,
     pr_id,
     parent_id,
-    branch,
     username,
     password,
     git_user,
@@ -44,10 +43,12 @@ def create_preview_command(
             apps_tmp_dir,
         )
 
-        apps_git.checkout(branch)
-        logging.info("App repo branch %s checkout successful", branch)
-        shortened_branch_hash = hashlib.sha256(branch.encode("utf-8")).hexdigest()[:8]
-        logging.info("Hashed branch %s to hash: %s", branch, shortened_branch_hash)
+        pr_branch = apps_git.get_pull_request_branch(pr_id)
+
+        apps_git.checkout(pr_branch)
+        logging.info("App repo PR branch %s checkout successful", pr_branch)
+        shortened_branch_hash = hashlib.sha256(pr_branch.encode("utf-8")).hexdigest()[:8]
+        logging.info("Hashed branch %s to hash: %s", pr_branch, shortened_branch_hash)
         gitops_config = GitOpsConfig(apps_git.get_full_file_path(".gitops.config.yaml"))
         logging.info("Read GitOpsConfig: %s", gitops_config)
 
@@ -64,8 +65,12 @@ def create_preview_command(
         )
         root_git.checkout("master")
         logging.info("Config repo branch master checkout successful")
-        root_git.new_branch(branch)
-        logging.info("Created branch %s in config repo", branch)
+
+        config_branch = f"gitopscli-create-preview-{str(uuid.uuid4())[:8]}" if create_pr else "master"
+        if create_pr:
+            root_git.new_branch(config_branch)
+            logging.info("Created branch %s in config repo", config_branch)
+
         preview_template_folder_name = ".preview-templates/" + gitops_config.application_name
         logging.info("Using the preview template folder: %s", preview_template_folder_name)
         new_preview_folder_name = gitops_config.application_name + "-" + shortened_branch_hash + "-preview"
@@ -74,7 +79,11 @@ def create_preview_command(
         logging.info("Is preview env already existing for branch? %s", branch_preview_env_already_exist)
         if not branch_preview_env_already_exist:
             __create_new_preview_env(
-                branch, new_preview_folder_name, preview_template_folder_name, root_git, gitops_config.application_name
+                config_branch,
+                new_preview_folder_name,
+                preview_template_folder_name,
+                root_git,
+                gitops_config.application_name,
             )
         new_image_tag = apps_git.get_last_commit_hash()
         logging.info("Using image tag from last app repo commit: %s", new_image_tag)
@@ -94,27 +103,27 @@ def create_preview_command(
         if not value_replaced:
             __no_deployment_needed(apps_git, new_image_tag, parent_id, pr_id)
             return
-        root_git.commit(f"Upated preview environment for {gitops_config.application_name} and branch {branch}.")
-        root_git.push(branch)
-        logging.info("Pushed branch %s", branch)
+        root_git.commit(f"Upated preview environment for {gitops_config.application_name} and branch {pr_branch}.")
+        root_git.push(config_branch)
+        logging.info("Pushed branch %s", config_branch)
         pr_comment_text = f"""
-New Preview Environment for {gitops_config.application_name} and branch {branch} created successfully. Access it here: 
+New Preview Environment for {gitops_config.application_name} and branch {pr_branch} created successfully. Access it here: 
 https://{route_host}
 """
         if branch_preview_env_already_exist:
             pr_comment_text = f"""
-Preview Environment for {gitops_config.application_name} and branch {branch} updated successfully. Access it here: 
+Preview Environment for {gitops_config.application_name} and branch {pr_branch} updated successfully. Access it here: 
 https://{route_host}
 """
         logging.info("Creating PullRequest comment for pr with id %s and content: %s", pr_id, pr_comment_text)
         apps_git.add_pull_request_comment(pr_id, pr_comment_text, parent_id)
     finally:
         shutil.rmtree(apps_tmp_dir, ignore_errors=True)
         shutil.rmtree(root_tmp_dir, ignore_errors=True)
-    if create_pr and branch != "master":
-        pull_request = __create_pullrequest(branch, gitops_config, root_git)
+    if create_pr:
+        pull_request = __create_pullrequest(config_branch, gitops_config, root_git)
         if auto_merge:
-            __merge_pullrequest(branch, pull_request, root_git)
+            __merge_pullrequest(config_branch, pull_request, root_git)
 
 
 def __replace_value(

diff --git a/gitopscli/commands/delete_preview.py b/gitopscli/commands/delete_preview.py
@@ -59,8 +59,11 @@ def delete_preview_command(
         )
         root_git.checkout("master")
         logging.info("Config repo branch master checkout successful")
-        root_git.new_branch(branch)
-        logging.info("Created branch %s in config repo", branch)
+
+        config_branch = f"gitopscli-delete-preview-{str(uuid.uuid4())[:8]}" if create_pr else "master"
+
+        root_git.new_branch(config_branch)
+        logging.info("Created branch %s in config repo", config_branch)
         shortened_branch_hash = hashlib.sha256(branch.encode("utf-8")).hexdigest()[:8]
         logging.info("Hashed branch %s to hash: %s", branch, shortened_branch_hash)
         preview_folder_name = gitops_config.application_name + "-" + shortened_branch_hash + "-preview"
@@ -74,16 +77,16 @@ def delete_preview_command(
         root_git.commit(
             f"Deleted preview environment for application: {gitops_config.application_name} and branch: {branch}."
         )
-        root_git.push(branch)
-        logging.info("Pushed branch %s", branch)
+        root_git.push(config_branch)
+        logging.info("Pushed branch %s", config_branch)
 
     finally:
         shutil.rmtree(apps_tmp_dir, ignore_errors=True)
         shutil.rmtree(root_tmp_dir, ignore_errors=True)
-    if create_pr and branch != "master":
-        pull_request = __create_pullrequest(branch, gitops_config, root_git)
+    if create_pr:
+        pull_request = __create_pullrequest(config_branch, gitops_config, root_git)
         if auto_merge:
-            __merge_pullrequest(branch, pull_request, root_git)
+            __merge_pullrequest(config_branch, pull_request, root_git)
 
 
 def __create_pullrequest(branch, gitops_config, root_git):

diff --git a/gitopscli/commands/deploy.py b/gitopscli/commands/deploy.py
@@ -12,7 +12,6 @@ def deploy_command(
     command,
     file,
     values,
-    branch,
     username,
     password,
     git_user,
@@ -46,22 +45,24 @@ def deploy_command(
         git.checkout("master")
         logging.info("Master checkout successful")
 
-        if branch != "master":
-            git.new_branch(branch)
-            logging.info("Created branch %s", branch)
+        config_branch = f"gitopscli-deploy-{str(uuid.uuid4())[:8]}" if create_pr else "master"
+
+        if create_pr:
+            git.new_branch(config_branch)
+            logging.info("Created branch %s", config_branch)
 
         updated_values = __update_values(git, file, values, single_commit)
         if not updated_values:
             logging.info("All values already up-to-date. I'm done here")
             return
 
-        git.push(branch)
-        logging.info("Pushed branch %s", branch)
+        git.push(config_branch)
+        logging.info("Pushed branch %s", config_branch)
     finally:
         shutil.rmtree(tmp_dir, ignore_errors=True)
 
-    if create_pr and branch != "master":
-        __create_pr(git, branch, file, updated_values, auto_merge)
+    if create_pr:
+        __create_pr(git, config_branch, file, updated_values, auto_merge)
 
 
 def __update_values(git, file, values, single_commit):

diff --git a/gitopscli/git/abstract_git_util.py b/gitopscli/git/abstract_git_util.py
@@ -99,3 +99,7 @@ def add_pull_request_comment(self, pr_id, text, parent_id):
     @abstractmethod
     def delete_branch(self, branch):
         pass
+
+    @abstractmethod
+    def get_pull_request_branch(self, pr_id):
+        pass
diff --git a/gitopscli/git/bitbucket_git_util.py b/gitopscli/git/bitbucket_git_util.py
@@ -80,3 +80,9 @@ def delete_branch(self, branch):
         )
         if result and "errors" in result:
             raise GitOpsException(result["errors"][0]["message"])
+
+    def get_pull_request_branch(self, pr_id):
+        pull_request = self._bitbucket.get_pullrequest(self._organisation, self._repository_name, pr_id)
+        if "errors" in pull_request:
+            raise GitOpsException(pull_request["errors"][0]["message"])
+        return pull_request["fromRef"]["displayId"]
diff --git a/gitopscli/git/github_git_util.py b/gitopscli/git/github_git_util.py
@@ -26,11 +26,7 @@ def merge_pull_request(self, pull_request):
         pull_request.merge()
 
     def add_pull_request_comment(self, pr_id, text, parent_id=None):
-        repo = self.__get_repo()
-        try:
-            pull_request = repo.get_pull(pr_id)
-        except UnknownObjectException as ex:
-            raise GitOpsException(f"Pull request with ID '{pr_id}' does not exist.") from ex
+        pull_request = self.__get_pull_request(pr_id)
         pr_comment = pull_request.create_issue_comment(text)
         return pr_comment
 
@@ -42,6 +38,17 @@ def delete_branch(self, branch):
             raise GitOpsException(f"Branch '{branch}' does not exist.") from ex
         git_ref.delete()
 
+    def get_pull_request_branch(self, pr_id):
+        pull_request = self.__get_pull_request(pr_id)
+        return pull_request.head.ref
+
+    def __get_pull_request(self, pr_id):
+        repo = self.__get_repo()
+        try:
+            return repo.get_pull(pr_id)
+        except UnknownObjectException as ex:
+            raise GitOpsException(f"Pull request with ID '{pr_id}' does not exist.") from ex
+
     def __get_repo(self):
         try:
             return self._github.get_repo(f"{self._organisation}/{self._repository_name}")