Add optional support for helm diff (#355)

There are some cases where the existing module has difficulty
determining if an upgrade would result in changes. This can particularly
be a problem when changes are made to a local chart.

This adds optional support for helm diff. If the plugin is present it
will be used. Otherwise, the default implementation will be used and a
warning will be issued. One caveat: helm diff does not currently support
using a repo url, so the default implementation will be used in this
case, as well.

Closes: #248

changelogs/fragments/355-helm-diff.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+  - helm - add optional support for helm diff (https://github.com/ansible-collections/community.kubernetes/issues/248).

molecule/default/roles/helm/defaults/main.yml

@@ -16,3 +16,4 @@ chart_test_repo: "https://kubernetes.github.io/ingress-nginx"
 chart_test_git_repo: "http://github.com/helm/charts.git"
 chart_test_values:
   revisionHistoryLimit: 0
+  myValue: "changed"

molecule/default/roles/helm/files/appversionless-chart-v2/Chart.yaml

@@ -0,0 +1,5 @@
+apiVersion: v2
+name: appversionless-chart
+description: A chart used in molecule tests
+type: application
+version: 0.2.0

molecule/default/roles/helm/files/appversionless-chart-v2/templates/configmap.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-chart-configmap
+data:
+  myValue: {{ default "test" .Values.myValue }}
+  myOtherValue: {{ default "foo" .Values.myOtherValue }}

molecule/default/roles/helm/files/appversionless-chart/templates/configmap.yaml

@@ -0,0 +1,6 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-chart-configmap
+data:
+  myValue: {{ default "test" .Values.myValue }}

molecule/default/roles/helm/files/test-chart-v2/Chart.yaml

@@ -0,0 +1,6 @@
+apiVersion: v2
+name: test-chart
+description: A chart used in molecule tests
+type: application
+version: 0.2.0
+appVersion: "default"

molecule/default/roles/helm/files/test-chart-v2/templates/configmap.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-chart-configmap
+data:
+  myValue: {{ default "test" .Values.myValue }}
+  myOtherValue: {{ default "foo" .Values.myOtherValue }}

molecule/default/roles/helm/files/test-chart/templates/configmap.yaml

@@ -0,0 +1,6 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: test-chart-configmap
+data:
+  myValue: {{ default "test" .Values.myValue }}

molecule/default/roles/helm/tasks/run_test.yml

@@ -30,6 +30,9 @@
 - name: Test helm plugin
   include_tasks: tests_helm_plugin.yml
 
+- name: Test helm diff
+  include_tasks: tests_helm_diff.yml
+
 - name: Clean helm install
   file:
     path: "{{ item }}"

molecule/default/roles/helm/tasks/tests_chart/from_local_path.yml

@@ -26,8 +26,9 @@
 - name: Test appVersion idempotence
   vars:
     chart_test: "test-chart"
+    chart_test_upgrade: "test-chart-v2"
     chart_test_version: "0.1.0"
-    chart_test_version_upgrade: "0.1.0"
+    chart_test_version_upgrade: "0.2.0"
     chart_test_app_version: "v1"
     chart_test_upgrade_app_version: "v2"
   block:
@@ -36,6 +37,11 @@
         src: "{{ chart_test }}"
         dest: "/tmp/helm_test_appversion/test-chart/"
 
+    - name: Copy test chart v2
+      copy:
+        src: "{{ chart_test_upgrade }}"
+        dest: "/tmp/helm_test_appversion/test-chart/"
+
     # create package with appVersion v1
     - name: "Package chart into archive with appVersion {{ chart_test_app_version }}"
       command: "{{ helm_binary }} package --app-version {{ chart_test_app_version }} /tmp/helm_test_appversion/test-chart/{{ chart_test }}"
@@ -47,41 +53,47 @@
 
     # create package with appVersion v2
     - name: "Package chart into archive with appVersion {{ chart_test_upgrade_app_version }}"
-      command: "{{ helm_binary }} package --app-version {{ chart_test_upgrade_app_version }} /tmp/helm_test_appversion/test-chart/{{ chart_test }}"
+      command: "{{ helm_binary }} package --app-version {{ chart_test_upgrade_app_version }} /tmp/helm_test_appversion/test-chart/{{ chart_test_upgrade }}"
     - name: "Move appVersion {{ chart_test_upgrade_app_version }} chart archive"
       copy:
         remote_src: true
-        src: "test-chart-{{ chart_test_version }}.tgz"
-        dest: "/tmp/helm_test_appversion/test-chart/{{ chart_test }}-{{ chart_test_upgrade_app_version }}-{{ chart_test_version }}.tgz"
+        src: "test-chart-{{ chart_test_version_upgrade }}.tgz"
+        dest: "/tmp/helm_test_appversion/test-chart/{{ chart_test }}-{{ chart_test_upgrade_app_version }}-{{ chart_test_version_upgrade }}.tgz"
 
     - name: Install Chart from local path
       include_tasks: "../tests_chart.yml"
       vars:
         source: local_path
         chart_source: "/tmp/helm_test_appversion/test-chart/{{ chart_test }}-{{ chart_test_app_version }}-{{ chart_test_version }}.tgz"
-        chart_source_upgrade: "/tmp/helm_test_appversion/test-chart/{{ chart_test }}-{{ chart_test_upgrade_app_version }}-{{ chart_test_version }}.tgz"
+        chart_source_upgrade: "/tmp/helm_test_appversion/test-chart/{{ chart_test }}-{{ chart_test_upgrade_app_version }}-{{ chart_test_version_upgrade }}.tgz"
 
 - name: Test appVersion handling when null
   vars:
     chart_test: "appversionless-chart"
+    chart_test_upgrade: "appversionless-chart-v2"
     chart_test_version: "0.1.0"
-    chart_test_version_upgrade: "0.1.0"
+    chart_test_version_upgrade: "0.2.0"
   block:
     - name: Copy test chart
       copy:
         src: "{{ chart_test }}"
         dest: "/tmp/helm_test_appversion/test-null/"
 
+    - name: Copy test chart v2
+      copy:
+        src: "{{ chart_test_upgrade }}"
+        dest: "/tmp/helm_test_appversion/test-null/"
+
     # create package with appVersion v1
     - name: "Package chart into archive with appVersion v1"
-      command: "{{ helm_binary }} package --app-version v1 /tmp/helm_test_appversion/test-null/{{ chart_test }}"
+      command: "{{ helm_binary }} package --app-version v1 /tmp/helm_test_appversion/test-null/{{ chart_test_upgrade }}"
 
     - name: Install Chart from local path
       include_tasks: "../tests_chart.yml"
       vars:
         source: local_path
         chart_source: "/tmp/helm_test_appversion/test-null/{{ chart_test }}/"
-        chart_source_upgrade: "{{ chart_test }}-{{ chart_test_version }}.tgz"
+        chart_source_upgrade: "{{ chart_test }}-{{ chart_test_version_upgrade }}.tgz"
 
 - name: Remove clone repos
   file:

molecule/default/roles/helm/tasks/tests_helm_diff.yml

@@ -0,0 +1,153 @@
+---
+- name: Test helm diff functionality
+  vars:
+    test_chart_ref: "/tmp/test-chart"
+
+  block:
+    - name: Install helm diff
+      helm_plugin:
+        namespace: "{{ helm_namespace }}"
+        state: present
+        plugin_path: https://github.com/databus23/helm-diff
+
+    - name: Copy test chart
+      copy:
+        src: "test-chart/"
+        dest: "{{ test_chart_ref }}"
+
+    - name: Install local chart
+      helm:
+        binary_path: "{{ helm_binary }}"
+        name: test-chart
+        namespace: "{{ helm_namespace }}"
+        chart_ref: "{{ test_chart_ref }}"
+        create_namespace: yes
+      register: install
+
+    - assert:
+        that:
+          - install is changed
+
+    - name: Modify local chart
+      blockinfile:
+        create: yes
+        path: "{{ test_chart_ref }}/templates/anothermap.yaml"
+        block: !unsafe |
+          apiVersion: v1
+          kind: ConfigMap
+          metadata:
+            name: test-chart-another-configmap
+          data:
+            foo: {{ .Values.foo | default "bar" }}
+
+    - name: Upgrade local chart with modifications
+      helm:
+        binary_path: "{{ helm_binary }}"
+        name: test-chart
+        namespace: "{{ helm_namespace }}"
+        chart_ref: "{{ test_chart_ref }}"
+      register: install
+
+    - assert:
+        that:
+          - install is changed
+
+    - name: Upgrade modified local chart idempotency check
+      helm:
+        binary_path: "{{ helm_binary }}"
+        name: test-chart
+        namespace: "{{ helm_namespace }}"
+        chart_ref: "{{ test_chart_ref }}"
+      register: install
+
+    - assert:
+        that:
+          - install is not changed
+
+    - name: Modify values
+      blockinfile:
+        create: yes
+        path: "{{ test_chart_ref }}/values.yml"
+        block: |
+          ---
+          foo: baz
+
+    - name: Upgrade with values file
+      helm:
+        binary_path: "{{ helm_binary }}"
+        name: test-chart
+        namespace: "{{ helm_namespace }}"
+        chart_ref: "{{ test_chart_ref }}"
+        values_files:
+          - "{{ test_chart_ref }}/values.yml"
+      register: install
+
+    - assert:
+        that:
+          - install is changed
+
+    - name: Upgrade with values file idempotency check
+      helm:
+        binary_path: "{{ helm_binary }}"
+        name: test-chart
+        namespace: "{{ helm_namespace }}"
+        chart_ref: "{{ test_chart_ref }}"
+        values_files:
+          - "{{ test_chart_ref }}/values.yml"
+      register: install
+
+    - assert:
+        that:
+          - install is not changed
+
+    - name: Upgrade with values
+      helm:
+        binary_path: "{{ helm_binary }}"
+        name: test-chart
+        namespace: "{{ helm_namespace }}"
+        chart_ref: "{{ test_chart_ref }}"
+        values:
+          foo: gaz
+      register: install
+
+    - assert:
+        that:
+          - install is changed
+
+    - name: Upgrade with values idempotency check
+      helm:
+        binary_path: "{{ helm_binary }}"
+        name: test-chart
+        namespace: "{{ helm_namespace }}"
+        chart_ref: "{{ test_chart_ref }}"
+        values:
+          foo: gaz
+      register: install
+
+    - assert:
+        that:
+          - install is not changed
+
+  always:
+    - name: Remove chart directory
+      file:
+        path: "{{ test_chart_ref }}"
+        state: absent
+      ignore_errors: yes
+
+    - name: Uninstall helm diff
+      helm_plugin:
+        namespace: "{{ helm_namespace }}"
+        state: absent
+        plugin_name: diff
+      ignore_errors: yes
+
+    - name: Remove helm namespace
+      k8s:
+        api_version: v1
+        kind: Namespace
+        name: "{{ helm_namespace }}"
+        state: absent
+        wait: yes
+        wait_timeout: 180
+      ignore_errors: yes

plugins/modules/helm.py

@@ -399,6 +399,70 @@ def load_values_files(values_files):
     return values
 
 
+def has_plugin(command, plugin):
+    """
+    Check if helm plugin is installed.
+    """
+
+    cmd = command + " plugin list"
+    rc, out, err = run_helm(module, cmd)
+    for line in out.splitlines():
+        if line.startswith("NAME"):
+            continue
+        name, _rest = line.split("\t", 1)
+        if name == plugin:
+            return True
+    return False
+
+
+def helmdiff_check(module, helm_cmd, release_name, chart_ref, release_values,
+                   values_files=None, chart_version=None, replace=False):
+    """
+    Use helm diff to determine if a release would change by upgrading a chart.
+    """
+    cmd = helm_cmd + " diff upgrade"
+    cmd += " " + release_name
+    cmd += " " + chart_ref
+
+    if chart_version is not None:
+        cmd += " " + "--version=" + chart_version
+    if not replace:
+        cmd += " " + "--reset-values"
+
+    if release_values != {}:
+        fd, path = tempfile.mkstemp(suffix='.yml')
+        with open(path, 'w') as yaml_file:
+            yaml.dump(release_values, yaml_file, default_flow_style=False)
+        cmd += " -f=" + path
+
+    if values_files:
+        for values_file in values_files:
+            cmd += " -f=" + values_file
+
+    rc, out, err = run_helm(module, cmd)
+    return len(out.strip()) > 0
+
+
+def default_check(release_status, chart_info, values=None, values_files=None):
+    """
+    Use default check to determine if release would change by upgrading a chart.
+    """
+    # the 'appVersion' specification is optional in a chart
+    chart_app_version = chart_info.get('appVersion', None)
+    released_app_version = release_status.get('app_version', None)
+
+    # when deployed without an 'appVersion' chart value the 'helm list' command will return the entry `app_version: ""`
+    appversion_is_same = (chart_app_version == released_app_version) or (chart_app_version is None and released_app_version == "")
+
+    if values_files:
+        values_match = release_status['values'] == load_values_files(values_files)
+    else:
+        values_match = release_status['values'] == values
+    return not values_match \
+        or (chart_info['name'] + '-' + chart_info['version']) != release_status["chart"] \
+        or not appversion_is_same
+
+
 def main():
     global module
     module = AnsibleModule(
@@ -507,21 +571,16 @@ def main():
             changed = True
 
         else:
-            # the 'appVersion' specification is optional in a chart
-            chart_app_version = chart_info.get('appVersion', None)
-            released_app_version = release_status.get('app_version', None)
 
-            # when deployed without an 'appVersion' chart value the 'helm list' command will return the entry `app_version: ""`
-            appversion_is_same = (chart_app_version == released_app_version) or (chart_app_version is None and released_app_version == "")
-
-            if values_files:
-                values_match = release_status['values'] == load_values_files(values_files)
+            if has_plugin(helm_cmd_common, "diff") and not chart_repo_url:
+                would_change = helmdiff_check(module, helm_cmd_common, release_name, chart_ref,
+                                              release_values, values_files, chart_version, replace)
             else:
-                values_match = release_status['values'] == release_values
+                module.warn("The default idempotency check can fail to report changes in certain cases. "
+                            "Install helm diff for better results.")
+                would_change = default_check(release_status, chart_info, release_values, values_files)
 
-            if force or not values_match \
-                    or (chart_info['name'] + '-' + chart_info['version']) != release_status["chart"] \
-                    or not appversion_is_same:
+            if force or would_change:
                 helm_cmd = deploy(helm_cmd, release_name, release_values, chart_ref, wait, wait_timeout,
                                   disable_hook, force, values_files=values_files, atomic=atomic,
                                   create_namespace=create_namespace, replace=replace)

tests/sanity/ignore-2.10.txt

@@ -4,3 +4,7 @@ plugins/modules/k8s_scale.py validate-modules:parameter-type-not-in-doc
 plugins/modules/k8s_scale.py validate-modules:return-syntax-error
 plugins/modules/k8s_service.py validate-modules:return-syntax-error
 plugins/modules/k8s_service.py validate-modules:parameter-type-not-in-doc
+molecule/default/roles/helm/files/appversionless-chart-v2/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/appversionless-chart/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/test-chart-v2/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/test-chart/templates/configmap.yaml yamllint!skip

tests/sanity/ignore-2.11.txt

@@ -4,3 +4,7 @@ plugins/modules/k8s_scale.py validate-modules:parameter-type-not-in-doc
 plugins/modules/k8s_scale.py validate-modules:return-syntax-error
 plugins/modules/k8s_service.py validate-modules:return-syntax-error
 plugins/modules/k8s_service.py validate-modules:parameter-type-not-in-doc
+molecule/default/roles/helm/files/appversionless-chart-v2/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/appversionless-chart/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/test-chart-v2/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/test-chart/templates/configmap.yaml yamllint!skip

tests/sanity/ignore-2.9.txt

@@ -1,3 +1,7 @@
 plugins/modules/k8s.py validate-modules:parameter-type-not-in-doc
 plugins/modules/k8s_scale.py validate-modules:parameter-type-not-in-doc
 plugins/modules/k8s_service.py validate-modules:parameter-type-not-in-doc
+molecule/default/roles/helm/files/appversionless-chart-v2/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/appversionless-chart/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/test-chart-v2/templates/configmap.yaml yamllint!skip
+molecule/default/roles/helm/files/test-chart/templates/configmap.yaml yamllint!skip