From 91b666c75ff57704650bf68a9c0f962e2be0c067 Mon Sep 17 00:00:00 2001
From: myii <myii@users.noreply.github.com>
Date: Sun, 10 Jun 2018 10:18:20 +0100
Subject: [PATCH] Improve limitations related to `git.latest` in
 `salt.formulas` (#239)

* Ensure options for formulas `git.latest` are also based on defaults

* Prevent erroneous remote tracking branch modification by `salt.formulas` #238

* Allow multiple envs to use the same gitdir for `salt.formulas`

* Ensure `gitdir` original branch is checked out after formulas `git.latest`

* Fix erroneous comment re: merging in `salt` pillar
---
 pillar.example      |  3 +++
 salt/defaults.yaml  |  3 +++
 salt/formulas.jinja | 11 ++++++++++-
 salt/formulas.sls   | 43 +++++++++++++++++++++++++++++++++++++------
 salt/map.jinja      |  9 ++++++++-
 5 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/pillar.example b/pillar.example
index 9a1c169..716f458 100644
--- a/pillar.example
+++ b/pillar.example
@@ -351,6 +351,9 @@ salt_formulas:
     user: root
     group: root
     mode: 755
+  # Explicitly checkout the original branch for repos after the
+  # git.latest states have been processed (False by default)
+  checkout_orig_branch: False
   # List of formulas to enable in each environment
   list:
     base:
diff --git a/salt/defaults.yaml b/salt/defaults.yaml
index ae984d4..3795032 100644
--- a/salt/defaults.yaml
+++ b/salt/defaults.yaml
@@ -53,3 +53,6 @@ salt:
       providers: salt://salt/files/cloud.providers.d
       profiles: salt://salt/files/cloud.profiles.d
       maps: salt://salt/files/cloud.maps.d
+
+salt_formulas:
+  checkout_orig_branch: False
diff --git a/salt/formulas.jinja b/salt/formulas.jinja
index 2a466a2..a501c04 100644
--- a/salt/formulas.jinja
+++ b/salt/formulas.jinja
@@ -2,7 +2,9 @@
     'baseurl': 'https://github.com/saltstack-formulas',
     'basedir': '/srv/formulas',
     'update': False,
-    'options': {},
+    'options': {
+      'branch': 'master',
+    },
    }
 %}
 {% set formulas = salt['pillar.get']('salt_formulas:list', {}) %}
@@ -14,6 +16,13 @@
 {{ value|yaml }}
 {%- endmacro -%}
 
+{%- macro formulas_opts_for_git_latest(env) -%}
+{%- set options = defaults['options'] or {} -%}
+{%- do options.update(salt['pillar.get']('salt_formulas:git_opts:default:options') or {}) -%}
+{%- do options.update(salt['pillar.get']('salt_formulas:git_opts:{0}:options'.format(env)) or {}) -%}
+{{ options|yaml }}
+{%- endmacro -%}
+
 {%- macro formulas_roots(env) -%}
 {%- set value = [] -%}
 {%- for dir in formulas.get(env, []) -%}
diff --git a/salt/formulas.sls b/salt/formulas.sls
index b433bb5..ca1e5d3 100644
--- a/salt/formulas.sls
+++ b/salt/formulas.sls
@@ -1,7 +1,10 @@
-{% set processed_gitdirs = [] %}
+{% set processed_gitdirs = {} %}
+{% set processed_gitdir_envs = [] %}
 {% set processed_basedirs = [] %}
 
+{% from "salt/map.jinja" import formulas_settings with context %}
 {% from "salt/formulas.jinja" import formulas_git_opt with context %}
+{% from "salt/formulas.jinja" import formulas_opts_for_git_latest with context %}
 
 # Loop over all formulas listed in pillar data
 {% for env, entries in salt['pillar.get']('salt_formulas:list', {}).items() %}
@@ -11,6 +14,15 @@
 {% set gitdir = '{0}/{1}'.format(basedir, entry) %}
 {% set update = formulas_git_opt(env, 'update')|load_yaml %}
 
+{% if formulas_settings.checkout_orig_branch %}
+{% if not salt['file.directory_exists']('{0}/{1}'.format(gitdir, '.git')) %}
+{% set gitdir_branch = '' %}
+{% else %}
+{% set gitdir_branch = salt['git.current_branch'](gitdir) %}
+{% endif %}
+{% do processed_gitdirs.update({gitdir:gitdir_branch}) %}
+{% endif %}
+
 # Setup the directory hosting the Git repository
 {% if basedir not in processed_basedirs %}
 {% do processed_basedirs.append(basedir) %}
@@ -23,11 +35,13 @@
 {% endif %}
 
 # Setup the formula Git repository
-{% if gitdir not in processed_gitdirs %}
-{% do processed_gitdirs.append(gitdir) %}
-{% set options = formulas_git_opt(env, 'options')|load_yaml %}
+{% set gitdir_env = '{0}_{1}'.format(gitdir, env) %}
+{% if gitdir_env not in processed_gitdir_envs %}
+{% do processed_gitdir_envs.append(gitdir_env) %}
+{% set options = formulas_opts_for_git_latest(env)|load_yaml %}
 {% set baseurl = formulas_git_opt(env, 'baseurl')|load_yaml %}
-{{ gitdir }}:
+
+{{ gitdir_env }}:
   git.latest:
     - name: {{ baseurl }}/{{ entry }}.git
     - target: {{ gitdir }}
@@ -40,6 +54,23 @@
     - unless: test -e {{ gitdir }}
     {%- endif %}
 {% endif %}
+{% endfor %}
+{% endfor %}
 
+{% if formulas_settings.checkout_orig_branch %}
+# For each directory processed, explicitly checkout the original branch before
+# the `git.latest` state ran
+{% for gitdir, original_branch in processed_gitdirs.items() %}
+{% if original_branch %}
+{% set gitdir_user = salt['file.get_user'](gitdir) %}
+checkout_original_branch_for_{{ gitdir }}:
+  module.run:
+    - name: git.checkout
+    - order: last
+    - cwd: {{ gitdir }}
+    - rev: {{ original_branch }}
+    - user: {{ gitdir_user }}
+    - unless: test "$(cd {{ gitdir }}; git rev-parse --abbrev-ref HEAD)" = "{{ original_branch }}"
+{% endif %}
 {% endfor %}
-{% endfor %}
+{% endif %}
diff --git a/salt/map.jinja b/salt/map.jinja
index 243571c..2ce570e 100644
--- a/salt/map.jinja
+++ b/salt/map.jinja
@@ -218,9 +218,16 @@ that differ from whats in defaults.yaml
 {## Merge the flavor_map to the default settings ##}
 {% do deep_merge(default_settings.salt,os_family_map) %}
 
-{## Merge in salt:lookup pillar ##}
+{## Merge in salt pillar ##}
 {% set salt_settings = salt['pillar.get'](
     'salt',
     default=default_settings.salt,
     merge=True)
 %}
+
+{## Merge in salt_formulas pillar ##}
+{% set formulas_settings = salt['pillar.get'](
+    'salt_formulas',
+    default=default_settings.salt_formulas,
+    merge=True)
+%}