From b26afc35181f673807f32a1efd67ea8c97af238f Mon Sep 17 00:00:00 2001 From: Mike Graves Date: Thu, 14 Jan 2021 11:05:11 -0500 Subject: [PATCH] Fix Secret check_mode (#343) When adding a Secret and using stringData, check_mode will always show changes. An existing resource fetched from Kubernetes will have the stringData already base64 encoded and merged into the data attribute. This change performs the base64 encoding and merging with the provided definition to more accurately represent the current state of the cluster. This change only affects check_mode. When making any changes to the cluster the stringData is passed along as provided in the definition. Closes #282. --- .../fragments/343-secret-check-mode.yaml | 2 + molecule/default/tasks/apply.yml | 75 +++++++++++++++++++ plugins/module_utils/common.py | 20 +++-- tests/unit/module_utils/test_common.py | 39 ++++++++++ 4 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/343-secret-check-mode.yaml create mode 100644 tests/unit/module_utils/test_common.py diff --git a/changelogs/fragments/343-secret-check-mode.yaml b/changelogs/fragments/343-secret-check-mode.yaml new file mode 100644 index 00000000..6351a888 --- /dev/null +++ b/changelogs/fragments/343-secret-check-mode.yaml @@ -0,0 +1,2 @@ +bugfixes: + - k8s - fix check_mode always showing changes when using stringData on Secrets (https://github.com/ansible-collections/community.kubernetes/issues/282). diff --git a/molecule/default/tasks/apply.yml b/molecule/default/tasks/apply.yml index 2f579755..291a35d6 100644 --- a/molecule/default/tasks/apply.yml +++ b/molecule/default/tasks/apply.yml @@ -761,6 +761,81 @@ that: - deploy_after_serviceaccount_removal is failed + - name: Add a secret + k8s: + definition: + apiVersion: v1 + kind: Secret + metadata: + name: apply-secret + namespace: "{{ apply_namespace }}" + type: Opaque + stringData: + foo: bar + register: k8s_secret + + - name: Check secret was created + assert: + that: + - k8s_secret is changed + - k8s_secret.result.data.foo + + - name: Add same secret + k8s: + definition: + apiVersion: v1 + kind: Secret + metadata: + name: apply-secret + namespace: "{{ apply_namespace }}" + type: Opaque + stringData: + foo: bar + register: k8s_secret + + - name: Check nothing changed + assert: + that: + - k8s_secret is not changed + + - name: Add same secret with check mode on + k8s: + definition: + apiVersion: v1 + kind: Secret + metadata: + name: apply-secret + namespace: "{{ apply_namespace }}" + type: Opaque + stringData: + foo: bar + check_mode: yes + register: k8s_secret + + - name: Check nothing changed + assert: + that: + - k8s_secret is not changed + + - name: Add same secret with check mode on using data + k8s: + definition: + apiVersion: v1 + kind: Secret + metadata: + name: apply-secret + namespace: "{{ apply_namespace }}" + type: Opaque + data: + foo: YmFy + check_mode: yes + register: k8s_secret + + - name: Check nothing changed + assert: + that: + - k8s_secret is not changed + always: - name: Remove namespace k8s: diff --git a/plugins/module_utils/common.py b/plugins/module_utils/common.py index 788c9df8..2e2bc6ed 100644 --- a/plugins/module_utils/common.py +++ b/plugins/module_utils/common.py @@ -18,6 +18,7 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +import base64 import time import os import traceback @@ -28,7 +29,7 @@ from ansible.module_utils.basic import AnsibleModule, missing_required_lib from ansible.module_utils.six import iteritems, string_types -from ansible.module_utils._text import to_native +from ansible.module_utils._text import to_native, to_bytes, to_text from ansible.module_utils.common.dict_transformations import dict_merge from ansible.module_utils.parsing.convert_bool import boolean @@ -690,7 +691,7 @@ def perform_action(self, resource, definition): else: if self.apply: if self.check_mode: - ignored, patch = apply_object(resource, definition) + ignored, patch = apply_object(resource, _encode_stringdata(definition)) if existing: k8s_obj = dict_merge(existing.to_dict(), patch) else: @@ -721,7 +722,7 @@ def perform_action(self, resource, definition): if not existing: if self.check_mode: - k8s_obj = definition + k8s_obj = _encode_stringdata(definition) else: try: k8s_obj = resource.create(definition, namespace=namespace).to_dict() @@ -757,7 +758,7 @@ def perform_action(self, resource, definition): if existing and force: if self.check_mode: - k8s_obj = definition + k8s_obj = _encode_stringdata(definition) else: try: k8s_obj = resource.replace(definition, name=name, namespace=namespace, append_hash=self.append_hash).to_dict() @@ -781,7 +782,7 @@ def perform_action(self, resource, definition): # Differences exist between the existing obj and requested params if self.check_mode: - k8s_obj = dict_merge(existing.to_dict(), definition) + k8s_obj = dict_merge(existing.to_dict(), _encode_stringdata(definition)) else: if LooseVersion(self.openshift_version) < LooseVersion("0.6.2"): k8s_obj, error = self.patch_resource(resource, definition, existing, name, @@ -858,3 +859,12 @@ def __init__(self, *args, **kwargs): self.warn("class KubernetesAnsibleModule is deprecated" " and will be removed in 2.0.0. Please use K8sAnsibleMixin instead.") + + +def _encode_stringdata(definition): + if definition['kind'] == 'Secret' and 'stringData' in definition: + for k, v in definition['stringData'].items(): + encoded = base64.b64encode(to_bytes(v)) + definition.setdefault('data', {})[k] = to_text(encoded) + del definition['stringData'] + return definition diff --git a/tests/unit/module_utils/test_common.py b/tests/unit/module_utils/test_common.py new file mode 100644 index 00000000..5eb4621b --- /dev/null +++ b/tests/unit/module_utils/test_common.py @@ -0,0 +1,39 @@ +# -*- coding: utf-8 -*- +# Copyright: (c) 2021, Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + + +from ansible_collections.community.kubernetes.plugins.module_utils.common import ( + _encode_stringdata, +) + + +def test_encode_stringdata_modifies_definition(): + definition = { + "apiVersion": "v1", + "kind": "Secret", + "type": "Opaque", + "stringData": { + "mydata": "ansiβle" + } + } + res = _encode_stringdata(definition) + assert "stringData" not in res + assert res["data"]["mydata"] == "YW5zac6ybGU=" + + +def test_encode_stringdata_does_not_modify_data(): + definition = { + "apiVersion": "v1", + "kind": "Secret", + "type": "Opaque", + "data": { + "mydata": "Zm9vYmFy" + } + } + res = _encode_stringdata(definition) + assert res["data"]["mydata"] == "Zm9vYmFy"