Provide Kubernetes resource validation to k8s module (#43352)

* Provide Kubernetes resource validation to k8s module

Use kubernetes-validate to validate Kubernetes resource
definitions against the published schema

* Additional tests for kubernetes-validate

* Improve k8s error messages on exceptions

Parse the response body for the message rather than returning
a JSON blob

If we've validated and there are warnings, return those too - they
can be more helpful

```
"msg": "Failed to patch object: {\"kind\":\"Status\",\"apiVersion\":\"v1\",\"metadata\":{},
       \"status\":\"Failure\",\"message\":\"[pos 334]: json: decNum: got first char 'h'\",\"code\":500}\n",
```
vs
```
"msg": "Failed to patch object: [pos 334]: json: decNum: got first char 'h'\nresource
        validation error at spec.replicas: 'hello' is not of type u'integer'",
```

* Update versions used

In particular openshift/origin:3.9.0

* Add changelog for k8s validate change

changelogs/fragments/k8s_validate.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - k8s - add validate parameter to k8s module to allow resources to be validated against their specification

lib/ansible/module_utils/k8s/raw.py

@@ -20,6 +20,7 @@ from __future__ import absolute_import, division, print_function
 
 import copy
 from datetime import datetime
+from distutils.version import LooseVersion
 import time
 import sys
 
@@ -31,14 +32,28 @@ from ansible.module_utils.common.dict_transformations import dict_merge
 
 try:
     import yaml
-    from openshift.dynamic.exceptions import DynamicApiError, NotFoundError, ConflictError, ForbiddenError
+    from openshift.dynamic.exceptions import DynamicApiError, NotFoundError, ConflictError, ForbiddenError, KubernetesValidateMissing
 except ImportError:
     # Exceptions handled in common
     pass
 
+try:
+    import kubernetes_validate
+    HAS_KUBERNETES_VALIDATE = True
+except ImportError:
+    HAS_KUBERNETES_VALIDATE = False
+
 
 class KubernetesRawModule(KubernetesAnsibleModule):
 
+    @property
+    def validate_spec(self):
+        return dict(
+            fail_on_error=dict(type='bool'),
+            version=dict(),
+            strict=dict(type='bool', default=True)
+        )
+
     @property
     def argspec(self):
         argument_spec = copy.deepcopy(COMMON_ARG_SPEC)
@@ -46,6 +61,7 @@ class KubernetesRawModule(KubernetesAnsibleModule):
         argument_spec['merge_type'] = dict(type='list', choices=['json', 'merge', 'strategic-merge'])
         argument_spec['wait'] = dict(type='bool', default=False)
         argument_spec['wait_timeout'] = dict(type='int', default=120)
+        argument_spec['validate'] = dict(type='dict', default=None, options=self.validate_spec)
         return argument_spec
 
     def __init__(self, *args, **kwargs):
@@ -59,12 +75,17 @@ class KubernetesRawModule(KubernetesAnsibleModule):
                                          mutually_exclusive=mutually_exclusive,
                                          supports_check_mode=True,
                                          **kwargs)
-
+        self.kind = self.params.get('kind')
-        self.kind = self.params.pop('kind')
+        self.api_version = self.params.get('api_version')
-        self.api_version = self.params.pop('api_version')
+        self.name = self.params.get('name')
-        self.name = self.params.pop('name')
+        self.namespace = self.params.get('namespace')
-        self.namespace = self.params.pop('namespace')
+        resource_definition = self.params.get('resource_definition')
-        resource_definition = self.params.pop('resource_definition')
+        if self.params['validate']:
+            if LooseVersion(self.openshift_version) < LooseVersion("0.8.0"):
+                self.fail_json(msg="openshift >= 0.8.0 is required for validate")
+        if self.params['merge_type']:
+            if LooseVersion(self.openshift_version) < LooseVersion("0.6.2"):
+                self.fail_json(msg="openshift >= 0.6.2 is required for merge_type")
         if resource_definition:
             if isinstance(resource_definition, string_types):
                 try:
@@ -101,7 +122,11 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             api_version = definition.get('apiVersion', self.api_version)
             resource = self.find_resource(search_kind, api_version, fail=True)
             definition = self.set_defaults(resource, definition)
+            self.warnings = []
+            if self.params['validate'] is not None:
+                self.warnings = self.validate(definition)
             result = self.perform_action(resource, definition)
+            result['warnings'] = self.warnings
             changed = changed or result['changed']
             results.append(result)
 
@@ -115,6 +140,17 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             }
         })
 
+    def validate(self, resource):
+        try:
+            warnings, errors = self.client.validate(resource, self.params['validate'].get('version'), self.params['validate'].get('strict'))
+        except KubernetesValidateMissing:
+            self.fail_json(msg="kubernetes-validate python library is required to validate resources")
+
+        if errors and self.params['validate']['fail_on_error']:
+            self.fail_json(msg="\n".join(errors))
+        else:
+            return warnings + errors
+
     def set_defaults(self, resource, definition):
         definition['kind'] = resource.kind
         definition['apiVersion'] = resource.group_version
@@ -198,8 +234,10 @@ class KubernetesRawModule(KubernetesAnsibleModule):
                                   if the resource you are creating does not directly create a resource of the same kind.".format(name))
                         return result
                     except DynamicApiError as exc:
-                        self.fail_json(msg="Failed to create object: {0}".format(exc.body),
+                        msg = "Failed to create object: {0}".format(exc.body)
-                                       error=exc.status, status=exc.status, reason=exc.reason, definition=definition)
+                        if self.warnings:
+                            msg += "\n" + "\n    ".join(self.warnings)
+                        self.fail_json(msg=msg, error=exc.status, status=exc.status, reason=exc.reason)
                 success = True
                 result['result'] = k8s_obj
                 if wait:
@@ -220,8 +258,11 @@ class KubernetesRawModule(KubernetesAnsibleModule):
                     try:
                         k8s_obj = resource.replace(definition, name=name, namespace=namespace).to_dict()
                     except DynamicApiError as exc:
-                        self.fail_json(msg="Failed to replace object: {0}".format(exc.body),
+                        msg = "Failed to replace object: {0}".format(exc.body)
-                                       error=exc.status, status=exc.status, reason=exc.reason)
+                        if self.warnings:
+                            msg += "\n" + "\n    ".join(self.warnings)
+                        self.fail_json(msg=msg, error=exc.status, status=exc.status, reason=exc.reason)
+                match, diffs = self.diff_objects(existing.to_dict(), k8s_obj)
                 success = True
                 result['result'] = k8s_obj
                 if wait:
@@ -238,13 +279,9 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             if self.check_mode:
                 k8s_obj = dict_merge(existing.to_dict(), definition)
             else:
-                from distutils.version import LooseVersion
                 if LooseVersion(self.openshift_version) < LooseVersion("0.6.2"):
-                    if self.params['merge_type']:
+                    k8s_obj, error = self.patch_resource(resource, definition, existing, name,
-                        self.fail_json(msg="openshift >= 0.6.2 is required for merge_type")
+                                                         namespace)
-                    else:
-                        k8s_obj, error = self.patch_resource(resource, definition, existing, name,
-                                                             namespace)
                 else:
                     for merge_type in self.params['merge_type'] or ['strategic-merge', 'merge']:
                         k8s_obj, error = self.patch_resource(resource, definition, existing, name,
@@ -278,8 +315,10 @@ class KubernetesRawModule(KubernetesAnsibleModule):
             error = {}
             return k8s_obj, {}
         except DynamicApiError as exc:
-            error = dict(msg="Failed to patch object: {0}".format(exc.body),
+            msg = "Failed to patch object: {0}".format(exc.body)
-                         error=exc.status, status=exc.status, reason=exc.reason)
+            if self.warnings:
+                msg += "\n" + "\n    ".join(self.warnings)
+            error = dict(msg=msg, error=exc.status, status=exc.status, reason=exc.reason, warnings=self.warnings)
             return None, error
 
     def create_project_request(self, definition):

lib/ansible/modules/clustering/k8s/k8s.py

@@ -73,6 +73,22 @@ options:
     - How long in seconds to wait for the resource to end up in the desired state. Ignored if C(wait) is not set.
     default: 120
     version_added: "2.8"
+  validate:
+    description:
+      - how (if at all) to validate the resource definition against the kubernetes schema.
+        Requires the kubernetes-validate python module
+    suboptions:
+      fail_on_error:
+        description: whether to fail on validation errors.
+        required: yes
+        type: bool
+      version:
+        description: version of Kubernetes to validate against. defaults to Kubernetes server version
+      strict:
+        description: whether to fail when passing unexpected properties
+        default: no
+        type: bool
+    version_added: "2.8"
 
 requirements:
   - "python >= 2.7"
@@ -141,6 +157,21 @@ EXAMPLES = '''
   k8s:
     state: present
     definition: "{{ lookup('template', '/testing/deployment.yml') }}"
+
+- name: fail on validation errors
+  k8s:
+    state: present
+    definition: "{{ lookup('template', '/testing/deployment.yml') }}"
+    validate:
+      fail_on_error: yes
+
+- name: warn on validation errors, check for unexpected properties
+  k8s:
+    state: present
+    definition: "{{ lookup('template', '/testing/deployment.yml') }}"
+    validate:
+      fail_on_error: no
+      strict: yes
 '''
 
 RETURN = '''

test/integration/targets/k8s/playbooks/older_openshift_fail.yml

@@ -0,0 +1,61 @@
+- hosts: localhost
+  connection: local
+  gather_facts: no
+  vars:
+    ansible_python_interpreter: "{{ ansible_playbook_python }}"
+    recreate_crd_default_merge_expectation: recreate_crd is failed
+    playbook_namespace: ansible-test-k8s-older-openshift
+
+  tasks:
+    - python_requirements_facts:
+        dependencies:
+          - openshift==0.6.0
+          - kubernetes==6.0.0
+
+    # append_hash
+    - name: use append_hash with ConfigMap
+      k8s:
+        definition:
+          metadata:
+            name: config-map-test
+            namespace: "{{ playbook_namespace }}"
+          apiVersion: v1
+          kind: ConfigMap
+          data:
+            hello: world
+        append_hash: yes
+      ignore_errors: yes
+      register: k8s_append_hash
+
+    - name: assert that append_hash fails gracefully
+      assert:
+        that:
+          - k8s_append_hash is failed
+          - "k8s_append_hash.msg == 'openshift >= 0.7.FIXME is required for append_hash'"
+
+    # merge_type
+    - include_role:
+        name: k8s
+        tasks_from: crd
+
+    # validate
+    - name: attempt to use validate with older openshift
+      k8s:
+        definition:
+          metadata:
+            name: config-map-test
+            namespace: "{{ playbook_namespace }}"
+          apiVersion: v1
+          kind: ConfigMap
+          data:
+            hello: world
+        validate:
+          fail_on_error: yes
+      ignore_errors: yes
+      register: k8s_validate
+
+    - name: assert that validate fails gracefully
+      assert:
+        that:
+          - k8s_validate is failed
+          - "k8s_validate.msg == 'openshift >= 0.7.FIXME is required for validate'"

test/integration/targets/k8s/playbooks/roles/k8s/files/kuard-extra-property.yml

@@ -0,0 +1,21 @@
+apiVersion: extensions/v1beta1
+kind: Deployment
+metadata:
+  labels:
+    app: kuard
+  name: kuard
+  namespace: default
+spec:
+  replicas: 3
+  selector:
+    matchLabels:
+      app: kuard
+    unwanted: value
+  template:
+    metadata:
+      labels:
+        app: kuard
+    spec:
+      containers:
+      - image: gcr.io/kuar-demo/kuard-amd64:1
+        name: kuard

test/integration/targets/k8s/playbooks/roles/k8s/files/kuard-invalid-type.yml

@@ -0,0 +1,20 @@
+apiVersion: extensions/v1beta1
+kind: Deployment
+metadata:
+  labels:
+    app: kuard
+  name: kuard
+  namespace: default
+spec:
+  replicas: hello
+  selector:
+    matchLabels:
+      app: kuard
+  template:
+    metadata:
+      labels:
+        app: kuard
+    spec:
+      containers:
+      - image: gcr.io/kuar-demo/kuard-amd64:1
+        name: kuard

test/integration/targets/k8s/playbooks/roles/k8s/tasks/validate_installed.yml

@@ -0,0 +1,117 @@
+- block:
+    - name: Create a namespace
+      k8s:
+        name: "{{ playbook_namespace }}"
+        kind: namespace
+
+    - name: incredibly simple ConfigMap
+      k8s:
+        definition:
+          apiVersion: v1
+          kind: ConfigMap
+          metadata:
+            name: hello
+            namespace: "{{ playbook_namespace }}"
+        validate:
+          fail_on_error: yes
+      register: k8s_with_validate
+
+    - name: assert that k8s_with_validate succeeds
+      assert:
+        that:
+          - k8s_with_validate is successful
+
+    - name: extra property does not fail without strict
+      k8s:
+        src: "{{ role_path }}/files/kuard-extra-property.yml"
+        namespace: "{{ playbook_namespace }}"
+        validate:
+          fail_on_error: yes
+          strict: no
+
+    - name: extra property fails with strict
+      k8s:
+        src: "{{ role_path }}/files/kuard-extra-property.yml"
+        namespace: "{{ playbook_namespace }}"
+        validate:
+          fail_on_error: yes
+          strict: yes
+      ignore_errors: yes
+      register: extra_property
+
+    - name: check that extra property fails with strict
+      assert:
+        that:
+          - extra_property is failed
+
+    - name: invalid type fails at validation stage
+      k8s:
+        src: "{{ role_path }}/files/kuard-invalid-type.yml"
+        namespace: "{{ playbook_namespace }}"
+        validate:
+          fail_on_error: yes
+          strict: no
+      ignore_errors: yes
+      register: invalid_type
+
+    - name: check that invalid type fails
+      assert:
+        that:
+          - invalid_type is failed
+
+    - name: invalid type fails with warnings when fail_on_error is False
+      k8s:
+        src: "{{ role_path }}/files/kuard-invalid-type.yml"
+        namespace: "{{ playbook_namespace }}"
+        validate:
+          fail_on_error: no
+          strict: no
+      ignore_errors: yes
+      register: invalid_type_no_fail
+
+    - name: check that invalid type fails
+      assert:
+        that:
+          - invalid_type_no_fail is failed
+
+    - name: setup custom resource definition
+      k8s:
+        src: "{{ role_path }}/files/setup-crd.yml"
+
+    - name: add custom resource definition
+      k8s:
+        src: "{{ role_path }}/files/crd-resource.yml"
+        namespace: "{{ playbook_namespace }}"
+        validate:
+          fail_on_error: yes
+          strict: yes
+      register: unknown_kind
+
+    - name: check that unknown kind warns
+      assert:
+        that:
+          - unknown_kind is successful
+          - "'warnings' in unknown_kind"
+
+  always:
+    - name: remove custom resource
+      k8s:
+        definition: "{{ lookup('file', role_path + '/files/crd-resource.yml') }}"
+        namespace: "{{ playbook_namespace }}"
+        state: absent
+      ignore_errors: yes
+
+    - name: remove custom resource definitions
+      k8s:
+        definition: "{{ lookup('file', role_path + '/files/setup-crd.yml') }}"
+        state: absent
+
+    - name: Delete namespace
+      k8s:
+        state: absent
+        definition:
+          - kind: Namespace
+            apiVersion: v1
+            metadata:
+              name: "{{ playbook_namespace }}"
+      ignore_errors: yes

test/integration/targets/k8s/playbooks/validate_installed.yml

@@ -0,0 +1,9 @@
+- hosts: localhost
+  connection: local
+  vars:
+    playbook_namespace: ansible-test-k8s-validate
+
+  tasks:
+  - include_role:
+      name: k8s
+      tasks_from: validate_installed

test/integration/targets/k8s/playbooks/validate_not_installed.yml

@@ -0,0 +1,21 @@
+- hosts: localhost
+  connection: local
+
+  tasks:
+  - k8s:
+      definition:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: hello
+          namespace: default
+      validate:
+        fail_on_error: yes
+    ignore_errors: yes
+    register: k8s_no_validate
+
+  - name: assert that k8s_no_validate fails gracefully
+    assert:
+      that:
+        - k8s_no_validate is failed
+        - "k8s_no_validate.msg == 'kubernetes-validate python library is required to validate resources'"

test/integration/targets/k8s/runme.sh

@@ -12,14 +12,26 @@ MYTMPDIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
 # but for the python3 tests we need virtualenv to use python3
 PYTHON=${ANSIBLE_TEST_PYTHON_INTERPRETER:-python}
 
+# Test graceful failure for missing kubernetes-validate
+virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/openshift-validate-not-installed"
+source "${MYTMPDIR}/openshift-validate-not-installed/bin/activate"
+$PYTHON -m pip install openshift==0.8.1
+ansible-playbook -v playbooks/validate_not_installed.yml "$@"
+
+# Test validate with kubernetes-validate
+virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/openshift-validate-installed"
+source "${MYTMPDIR}/openshift-validate-installed/bin/activate"
+$PYTHON -m pip install openshift==0.8.1 kubernetes-validate==1.12.0
+ansible-playbook -v playbooks/validate_installed.yml "$@"
+
 # Test graceful failure for older versions of openshift
 virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/openshift-0.6.0"
 source "${MYTMPDIR}/openshift-0.6.0/bin/activate"
-$PYTHON -m pip install 'openshift==0.6.0' 'kubernetes==6.0.0'
+$PYTHON -m pip install openshift==0.6.0 kubernetes==6.0.0
 ansible-playbook -v playbooks/merge_type_fail.yml "$@"
 
 # Run full test suite
 virtualenv --system-site-packages --python "${PYTHON}" "${MYTMPDIR}/openshift-recent"
 source "${MYTMPDIR}/openshift-recent/bin/activate"
-$PYTHON -m pip install 'openshift==0.7.2'
+$PYTHON -m pip install openshift==0.8.1
 ansible-playbook -v playbooks/full_test.yml "$@"

test/runner/lib/cloud/openshift.py

@@ -44,7 +44,7 @@ class OpenShiftCloudProvider(CloudProvider):
         super(OpenShiftCloudProvider, self).__init__(args, config_extension='.kubeconfig')
 
         # The image must be pinned to a specific version to guarantee CI passes with the version used.
-        self.image = 'openshift/origin:v3.7.1'
+        self.image = 'openshift/origin:v3.9.0'
         self.container_name = ''
 
     def filter(self, targets, exclude):