From 6853ed9a5a10487ec63f2b582024bf736731f285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Bregu=C5=82a?= Date: Sat, 31 Oct 2020 17:55:09 +0100 Subject: [PATCH 1/2] All k8s resources should have global labels --- .../scheduler/scheduler-serviceaccount.yaml | 6 ++--- chart/templates/secrets/redis-secrets.yaml | 3 +++ .../webserver/webserver-serviceaccount.yaml | 10 ++++----- .../workers/worker-serviceaccount.yaml | 12 +++++----- chart/tests/test_basic_helm_chart.py | 22 ++++++++++++++++++- 5 files changed, 36 insertions(+), 17 deletions(-) diff --git a/chart/templates/scheduler/scheduler-serviceaccount.yaml b/chart/templates/scheduler/scheduler-serviceaccount.yaml index c5e97f10371064..2d61c768f68ea3 100644 --- a/chart/templates/scheduler/scheduler-serviceaccount.yaml +++ b/chart/templates/scheduler/scheduler-serviceaccount.yaml @@ -28,13 +28,13 @@ metadata: release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- with .Values.labels }} + {{ toYaml . | nindent 4 }} + {{- end }} {{- with .Values.scheduler.serviceAccountAnnotations }} annotations: {{- range $key, $value := . }} {{- printf "%s: %s" $key (tpl $value $ | quote) | nindent 4 }} {{- end }} {{- end }} -{{- with .Values.labels }} -{{ toYaml . | indent 4 }} -{{- end }} {{- end }} diff --git a/chart/templates/secrets/redis-secrets.yaml b/chart/templates/secrets/redis-secrets.yaml index 7c9fe26ecf9e3b..958474d2324031 100644 --- a/chart/templates/secrets/redis-secrets.yaml +++ b/chart/templates/secrets/redis-secrets.yaml @@ -51,6 +51,9 @@ metadata: release: {{ .Release.Name }} chart: {{ .Chart.Name }} heritage: {{ .Release.Service }} + {{- with .Values.labels }} + {{ toYaml . | nindent 4 }} + {{- end }} annotations: "helm.sh/hook": "pre-install" "helm.sh/hook-delete-policy": "before-hook-creation" diff --git a/chart/templates/webserver/webserver-serviceaccount.yaml b/chart/templates/webserver/webserver-serviceaccount.yaml index ba99cea9ef1932..e42d767c1e28bf 100644 --- a/chart/templates/webserver/webserver-serviceaccount.yaml +++ b/chart/templates/webserver/webserver-serviceaccount.yaml @@ -27,12 +27,10 @@ metadata: release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- with .Values.labels }} + {{ toYaml . | nindent 4 }} + {{- end }} {{- with .Values.webserver.serviceAccountAnnotations }} annotations: - {{- range $key, $value := . }} - {{- printf "%s: %s" $key (tpl $value $ | quote) | nindent 4 }} - {{- end }} + {{ toYaml . | nindent 4 }} {{- end }} -{{- with .Values.labels }} -{{ toYaml . | indent 4 }} -{{- end }} diff --git a/chart/templates/workers/worker-serviceaccount.yaml b/chart/templates/workers/worker-serviceaccount.yaml index 3f2df95d4220b7..d7ed9270092c82 100644 --- a/chart/templates/workers/worker-serviceaccount.yaml +++ b/chart/templates/workers/worker-serviceaccount.yaml @@ -28,13 +28,11 @@ metadata: release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} - {{- with .Values.workers.serviceAccountAnnotations }} + {{- with .Values.labels }} + {{ toYaml . | nindent 4 }} + {{- end }} + {{- with .Values.workers.serviceAccountAnnotations}} annotations: - {{- range $key, $value := . }} - {{- printf "%s: %s" $key (tpl $value $ | quote) | nindent 4 }} - {{- end }} + {{ toYaml . | nindent 4 }} {{- end }} - {{- with .Values.labels }} -{{ toYaml . | indent 4 }} -{{- end }} {{- end }} diff --git a/chart/tests/test_basic_helm_chart.py b/chart/tests/test_basic_helm_chart.py index dcf20cf195ae8a..e45435e7ae1700 100644 --- a/chart/tests/test_basic_helm_chart.py +++ b/chart/tests/test_basic_helm_chart.py @@ -17,6 +17,8 @@ import unittest +import jmespath + from tests.helm_template_generator import render_chart OBJECT_COUNT_IN_BASIC_DEPLOYMENT = 22 @@ -24,7 +26,15 @@ class TestBaseChartTest(unittest.TestCase): def test_basic_deployments(self): - k8s_objects = render_chart("TEST-BASIC", {"chart": {'metadata': 'AA'}}) + k8s_objects = render_chart( + "TEST-BASIC", + values={ + "chart": { + 'metadata': 'AA', + }, + 'labels': {"TEST-LABEL": "TEST-VALUE"}, + }, + ) list_of_kind_names_tuples = [ (k8s_object['kind'], k8s_object['metadata']['name']) for k8s_object in k8s_objects ] @@ -56,6 +66,16 @@ def test_basic_deployments(self): ], ) self.assertEqual(OBJECT_COUNT_IN_BASIC_DEPLOYMENT, len(k8s_objects)) + for k8s_object in k8s_objects: + labels = jmespath.search('metadata.labels', k8s_object) or {} + if 'postgresql' in labels.get('chart'): + continue + k8s_name = k8s_object['kind'] + ":" + k8s_object['metadata']['name'] + self.assertEqual( + 'TEST-VALUE', + labels.get("TEST-LABEL"), + f"Missing label TEST-LABEL on {k8s_name}. Current labels: {labels}", + ) def test_basic_deployment_without_default_users(self): k8s_objects = render_chart("TEST-BASIC", {"webserver": {'defaultUser': {'enabled': False}}}) From aaff26b1fac3269da3ea467b755d4acdbdd5c79d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Bregu=C5=82a?= Date: Sat, 31 Oct 2020 18:04:25 +0100 Subject: [PATCH 2/2] All k8s object must comply with JSON Schema --- .../pod-template-file.kubernetes-helm-yaml | 2 +- chart/templates/redis/redis-statefulset.yaml | 4 +- .../webserver/webserver-ingress.yaml | 6 +-- .../webserver/webserver-service.yaml | 2 +- chart/tests/helm_template_generator.py | 37 +++++++++++++++++++ 5 files changed, 44 insertions(+), 7 deletions(-) diff --git a/chart/files/pod-template-file.kubernetes-helm-yaml b/chart/files/pod-template-file.kubernetes-helm-yaml index 5e75090cc84f78..8647060fa5e8ce 100644 --- a/chart/files/pod-template-file.kubernetes-helm-yaml +++ b/chart/files/pod-template-file.kubernetes-helm-yaml @@ -40,7 +40,7 @@ spec: volumeMounts: - mountPath: {{ template "airflow_logs" . }} name: airflow-logs -{{- if .Values.dags.gitSync.sshKeySecret }} +{{- if .Values.dags.gitSync.knownHosts }} - mountPath: /etc/git-secret/known_hosts name: {{ .Values.dags.gitSync.knownHosts }} subPath: known_hosts diff --git a/chart/templates/redis/redis-statefulset.yaml b/chart/templates/redis/redis-statefulset.yaml index 6df78b493589b3..ca47d97ffbbb12 100644 --- a/chart/templates/redis/redis-statefulset.yaml +++ b/chart/templates/redis/redis-statefulset.yaml @@ -48,10 +48,10 @@ spec: {{- with .Values.labels }} {{ toYaml . | indent 8 }} {{- end }} + {{- if .Values.redis.safeToEvict }} annotations: - {{- if .Values.redis.safeToEvict }} cluster-autoscaler.kubernetes.io/safe-to-evict: "true" - {{- end }} + {{- end }} spec: nodeSelector: {{ toYaml .Values.nodeSelector | indent 8 }} diff --git a/chart/templates/webserver/webserver-ingress.yaml b/chart/templates/webserver/webserver-ingress.yaml index 919ecc31450349..c249cb29d7c483 100644 --- a/chart/templates/webserver/webserver-ingress.yaml +++ b/chart/templates/webserver/webserver-ingress.yaml @@ -29,10 +29,10 @@ metadata: release: {{ .Release.Name }} chart: "{{ .Chart.Name }}-{{ .Chart.Version }}" heritage: {{ .Release.Service }} + {{- with .Values.ingress.web.annotations }} annotations: - {{ range $key, $value := .Values.ingress.web.annotations }} - {{ $key }}: {{ $value | quote }} - {{- end }} + {{ toYaml . | indent 4 }} + {{- end }} spec: {{- if .Values.ingress.web.tls.enabled }} tls: diff --git a/chart/templates/webserver/webserver-service.yaml b/chart/templates/webserver/webserver-service.yaml index 878a5d3771ccc9..feae23d5c68b5e 100644 --- a/chart/templates/webserver/webserver-service.yaml +++ b/chart/templates/webserver/webserver-service.yaml @@ -31,8 +31,8 @@ metadata: {{- with .Values.labels }} {{ toYaml . | indent 4 }} {{- end }} - annotations: {{- with .Values.webserver.service.annotations }} + annotations: {{- toYaml . | nindent 4 }} {{- end }} spec: diff --git a/chart/tests/helm_template_generator.py b/chart/tests/helm_template_generator.py index dccbb1ed1d93a3..ba870edac83507 100644 --- a/chart/tests/helm_template_generator.py +++ b/chart/tests/helm_template_generator.py @@ -17,13 +17,48 @@ import subprocess import sys +from functools import lru_cache from tempfile import NamedTemporaryFile +import jmespath +import jsonschema +import requests import yaml from kubernetes.client.api_client import ApiClient api_client = ApiClient() +BASE_URL_SPEC = "https://raw.githubusercontent.com/instrumenta/kubernetes-json-schema/master/v1.12.9" + + +@lru_cache(maxsize=None) +def create_validator(api_version, kind): + api_version = api_version.lower() + kind = kind.lower() + + if '/' in api_version: + ext, _, api_version = api_version.partition("/") + ext = ext.split(".")[0] + url = f'{BASE_URL_SPEC}/{kind}-{ext}-{api_version}.json' + else: + url = f'{BASE_URL_SPEC}/{kind}-{api_version}.json' + request = requests.get(url) + request.raise_for_status() + schema = request.json() + jsonschema.Draft7Validator.check_schema(schema) + validator = jsonschema.Draft7Validator(schema) + return validator + + +def validate_k8s_object(instance): + # Skip PostgresSQL chart + chart = jmespath.search("metadata.labels.chart", instance) + if chart and 'postgresql' in chart: + return + + validate = create_validator(instance.get("apiVersion"), instance.get("kind")) + validate.validate(instance) + def render_chart(name="RELEASE-NAME", values=None, show_only=None): """ @@ -41,6 +76,8 @@ def render_chart(name="RELEASE-NAME", values=None, show_only=None): templates = subprocess.check_output(command) k8s_objects = yaml.load_all(templates) k8s_objects = [k8s_object for k8s_object in k8s_objects if k8s_object] # type: ignore + for k8s_object in k8s_objects: + validate_k8s_object(k8s_object) return k8s_objects