From 81125fcffeeb7a337e34a1b84562b2f036a84eb8 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Fri, 15 Sep 2023 06:46:44 +0200 Subject: [PATCH 1/8] feat: adding support for propagating annotations and labels --- conformance/k8s_utils.py | 44 +++++++++++++++++++++++++++++++++------- conformance/tests.py | 27 ++++++++++++++++++++++++ src/handlers.py | 8 +++++++- src/kubernetes_utils.py | 39 +++++++++++++++++++++++++++-------- 4 files changed, 101 insertions(+), 17 deletions(-) diff --git a/conformance/k8s_utils.py b/conformance/k8s_utils.py index a8ee998..802a9c2 100644 --- a/conformance/k8s_utils.py +++ b/conformance/k8s_utils.py @@ -1,11 +1,18 @@ import time -from typing import Dict, Optional, List, Callable, Any +from typing import Dict, Optional, List, Callable, Mapping, Any from kubernetes import client, config from kubernetes.client import V1Secret, CoreV1Api, CustomObjectsApi from kubernetes.client.rest import ApiException from time import sleep +def is_subset(_set: Mapping[str, str], _subset: Mapping[str, str]) -> bool: + for key, item in _subset.items(): + if _set.get(key, None) != item: + return False + return True + + def wait_for_pod_ready_with_events(pod_selector: dict, namespace: str, timeout_seconds: int = 300): """ Wait for a pod to be ready in the specified namespace and print all events. @@ -52,6 +59,7 @@ class ClusterSecretManager: def __init__(self, custom_objects_api: CustomObjectsApi, api_instance: CoreV1Api): self.custom_objects_api: CustomObjectsApi = custom_objects_api self.api_instance: CoreV1Api = api_instance + # immutable after self.retry_attempts = 3 self.retry_delay = 5 @@ -167,18 +175,23 @@ def get_kubernetes_secret(self, name: str, namespace: str) -> Optional[V1Secret] raise e def validate_namespace_secrets( - self, name: str, + self, + name: str, data: Dict[str, str], - namespaces: Optional[List[str]] = None + namespaces: Optional[List[str]] = None, + labels: Optional[Dict[str, str]] = None, + annotations: Optional[Dict[str, str]] = None, ) -> bool: """ Parameters ---------- - name - data + name: str + data: Dict[str, str] namespaces: Optional[List[str]] If None, it means the secret should be present in ALL namespaces + annotations: Optional[Dict[str, str]] + labels: Optional[Dict[str, str]] Returns ------- @@ -199,16 +212,33 @@ def validate(): if secret is None or secret.data != data: return False + if annotations is not None and not is_subset(secret.metadata.annotations, annotations): + return False + + if labels is not None and not is_subset(secret.metadata.labels, labels): + return False + return True return self.retry(validate) def retry(self, f: Callable[[], bool]) -> bool: - while self.retry_attempts > 0: + """ + Utility function + Parameters + ---------- + f + + Returns + ------- + + """ + retry = self.retry_attempts + while retry > 0: if f(): return True sleep(self.retry_delay) - self.retry_attempts -= 1 + retry -= 1 return False def cleanup(self): diff --git a/conformance/tests.py b/conformance/tests.py index 87ae3f9..bd39883 100644 --- a/conformance/tests.py +++ b/conformance/tests.py @@ -268,6 +268,33 @@ def test_value_from_with_keys_cluster_secret(self): msg=f'Cluster secret should take the data from the {secret_name} secret but only the keys specified.' ) + def test_simple_cluster_secret_with_annotation(self): + name = "simple-cluster-secret" + username_data = "MTIzNDU2Cg==" + annotations = { + 'custom-annotation': 'example', + } + cluster_secret_manager = ClusterSecretManager( + custom_objects_api=custom_objects_api, + api_instance=api_instance + ) + + cluster_secret_manager.create_cluster_secret( + name=name, + namespace=USER_NAMESPACES[0], + data={"username": username_data}, + annotations=annotations, + ) + + # We expect the secret to be in ALL namespaces + self.assertTrue( + cluster_secret_manager.validate_namespace_secrets( + name=name, + data={"username": username_data}, + annotations=annotations + ) + ) + if __name__ == '__main__': unittest.main() diff --git a/src/handlers.py b/src/handlers.py index f962d5c..1c362c1 100644 --- a/src/handlers.py +++ b/src/handlers.py @@ -110,6 +110,7 @@ def on_field_data( old: Dict[str, str], new: Dict[str, str], body: Dict[str, Any], + meta: kopf.Meta, name: str, logger: logging.Logger, **_, @@ -130,7 +131,12 @@ def on_field_data( api_version='v1', data=new, kind='Secret', - metadata=create_secret_metadata(name=name, namespace=ns), + metadata=create_secret_metadata( + name=name, + namespace=ns, + annotations=meta.annotations, + labels=meta.labels, + ), type=secret_type, ) # Ensuring the secret still exist. diff --git a/src/kubernetes_utils.py b/src/kubernetes_utils.py index 518b8d4..30a2d0b 100644 --- a/src/kubernetes_utils.py +++ b/src/kubernetes_utils.py @@ -1,6 +1,6 @@ import logging from datetime import datetime -from typing import Optional, Dict, Any, List +from typing import Optional, Dict, Any, List, Mapping import re import kopf @@ -169,7 +169,10 @@ def sync_secret( if 'name' not in body['metadata']: raise kopf.TemporaryError('Property name is missing in metadata.') - sec_name = body['metadata']['name'] + cs_metadata: Dict[str, Any] = body.get('metadata') + sec_name = cs_metadata.get('name') + annotations = cs_metadata.get('annotations', None) + labels = cs_metadata.get('labels', None) if 'data' not in body: raise kopf.TemporaryError('Property data is missing.') @@ -203,7 +206,12 @@ def sync_secret( secret_type = body.get('type', 'Opaque') body = V1Secret() - body.metadata = create_secret_metadata(name=sec_name, namespace=namespace) + body.metadata = create_secret_metadata( + name=sec_name, + namespace=namespace, + annotations=annotations, + labels=labels, + ) body.type = secret_type body.data = data logger.info(f'cloning secret in namespace {namespace}') @@ -254,7 +262,12 @@ def sync_secret( logger.debug(f'Kube exception {e}') -def create_secret_metadata(name: str, namespace: str) -> V1ObjectMeta: +def create_secret_metadata( + name: str, + namespace: str, + annotations: Optional[Mapping[str, str]] = None, + labels: Optional[Mapping[str, str]] = None, +) -> V1ObjectMeta: """Create Kubernetes metadata objects. Parameters @@ -263,20 +276,28 @@ def create_secret_metadata(name: str, namespace: str) -> V1ObjectMeta: The name of the Kubernetes secret. namespace: str The namespace where the secret will be place. + labels: Optional[Dict[str, str]] + The secret labels. + annotations: Optional[Dict[str, str]] + The secrets annotations. Returns ------- V1ObjectMeta Kubernetes metadata object with ClusterSecret annotations. """ + + _annotations = { + CREATE_BY_ANNOTATION: 'ClusterSecrets', + VERSION_ANNOTATION: get_version(), + LAST_SYNC_ANNOTATION: datetime.now().isoformat(), + }.update(annotations or {}) + return V1ObjectMeta( name=name, namespace=namespace, - annotations={ - CREATE_BY_ANNOTATION: 'ClusterSecrets', - VERSION_ANNOTATION: get_version(), - LAST_SYNC_ANNOTATION: datetime.now().isoformat(), - }, + annotations=_annotations, + labels=labels, ) From eabbbbfd71ea3ac387155b3f33c129dad148ba9e Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Fri, 15 Sep 2023 14:49:19 +0200 Subject: [PATCH 2/8] fix: tests badly configured --- conformance/k8s_utils.py | 29 +++++++++++++++++++---------- conformance/tests.py | 2 +- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/conformance/k8s_utils.py b/conformance/k8s_utils.py index 802a9c2..43eda3b 100644 --- a/conformance/k8s_utils.py +++ b/conformance/k8s_utils.py @@ -199,7 +199,7 @@ def validate_namespace_secrets( """ all_namespaces = [item.metadata.name for item in self.api_instance.list_namespace().items] - def validate(): + def validate() -> Optional[str]: for namespace in all_namespaces: secret = self.get_kubernetes_secret(name=name, namespace=namespace) @@ -207,22 +207,25 @@ def validate(): if namespaces is not None and namespace not in namespaces: if secret is None: continue - return False + return f'' - if secret is None or secret.data != data: - return False + if secret is None: + return f'secret {name} is none in namespace {namespace}.' + + if secret.data != data: + return f'secret {name} data mismatch in namespace {namespace}.' if annotations is not None and not is_subset(secret.metadata.annotations, annotations): - return False + return f'secret {name} annotations mismatch in namespace {namespace}.' if labels is not None and not is_subset(secret.metadata.labels, labels): - return False + return f'secret {name} labels mismatch in namespace {namespace}.' - return True + return None return self.retry(validate) - def retry(self, f: Callable[[], bool]) -> bool: + def retry(self, f: Callable[[], Optional[str]]) -> bool: """ Utility function Parameters @@ -233,12 +236,18 @@ def retry(self, f: Callable[[], bool]) -> bool: ------- """ - retry = self.retry_attempts + retry: int = self.retry_attempts + err: Optional[str] = None + while retry > 0: - if f(): + err = f() + if err is None: return True sleep(self.retry_delay) retry -= 1 + + if err is not None: + print(f"Retry attempts exhausted. Last error: {err}") return False def cleanup(self): diff --git a/conformance/tests.py b/conformance/tests.py index bd39883..9ef193b 100644 --- a/conformance/tests.py +++ b/conformance/tests.py @@ -269,7 +269,7 @@ def test_value_from_with_keys_cluster_secret(self): ) def test_simple_cluster_secret_with_annotation(self): - name = "simple-cluster-secret" + name = "simple-cluster-secret-annotation" username_data = "MTIzNDU2Cg==" annotations = { 'custom-annotation': 'example', From c0318ed34c32e16296689b59d70c6b759352da14 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 18 Sep 2023 16:48:57 +0200 Subject: [PATCH 3/8] empty commit From af45688fe4b447e7360de5b4fc0b086d2cfcbed3 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 18 Sep 2023 16:58:28 +0200 Subject: [PATCH 4/8] fix: handlers using default on dict --- conformance/k8s_utils.py | 5 ++++- src/handlers.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/conformance/k8s_utils.py b/conformance/k8s_utils.py index 43eda3b..e35b00b 100644 --- a/conformance/k8s_utils.py +++ b/conformance/k8s_utils.py @@ -6,7 +6,10 @@ from time import sleep -def is_subset(_set: Mapping[str, str], _subset: Mapping[str, str]) -> bool: +def is_subset(_set: Optional[Mapping[str, str]], _subset: Optional[Mapping[str, str]]) -> bool: + if _set is None: + return _subset is None + for key, item in _subset.items(): if _set.get(key, None) != item: return False diff --git a/src/handlers.py b/src/handlers.py index 1c362c1..c6665af 100644 --- a/src/handlers.py +++ b/src/handlers.py @@ -123,7 +123,7 @@ def on_field_data( logger.debug(f'Updating Object body == {body}') syncedns = body.get('status', {}).get('create_fn', {}).get('syncedns', []) - secret_type = body.get('type', default='Opaque') + secret_type = body.get('type', 'Opaque') for ns in syncedns: logger.info(f'Re Syncing secret {name} in ns {ns}') From 2fa7f1b4b0cea81397c04a5122158d27269b0768 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 18 Sep 2023 18:00:20 +0200 Subject: [PATCH 5/8] bugs fixes: bad startup, error in create_secret_metadata and fixing getting default key --- src/consts.py | 2 ++ src/handlers.py | 3 ++- src/kubernetes_utils.py | 10 ++++++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/consts.py b/src/consts.py index d93de5e..b304735 100644 --- a/src/consts.py +++ b/src/consts.py @@ -5,3 +5,5 @@ CREATE_BY_ANNOTATION = 'clustersecret.io/created-by' LAST_SYNC_ANNOTATION = 'clustersecret.io/last-sync' VERSION_ANNOTATION = 'clustersecret.io/version' + +BLACK_LISTED_ANNOTATIONS = ["kopf.zalando.org", "kubectl.kubernetes.io"] \ No newline at end of file diff --git a/src/handlers.py b/src/handlers.py index c6665af..ec1906b 100644 --- a/src/handlers.py +++ b/src/handlers.py @@ -129,7 +129,7 @@ def on_field_data( logger.info(f'Re Syncing secret {name} in ns {ns}') body = client.V1Secret( api_version='v1', - data=new, + data=dict(new), kind='Secret', metadata=create_secret_metadata( name=name, @@ -243,5 +243,6 @@ async def startup_fn(logger: logging.Logger, **_): name=metadata.get('name'), namespace=metadata.get('namespace'), data=item.get('data'), + synced_namespace=item.get('status', {}).get('create_fn', {}).get('syncedns', []), ) ) diff --git a/src/kubernetes_utils.py b/src/kubernetes_utils.py index 30a2d0b..14e7177 100644 --- a/src/kubernetes_utils.py +++ b/src/kubernetes_utils.py @@ -7,7 +7,7 @@ from kubernetes.client import CoreV1Api, CustomObjectsApi, exceptions, V1ObjectMeta, rest, V1Secret from os_utils import get_replace_existing, get_version -from consts import CREATE_BY_ANNOTATION, LAST_SYNC_ANNOTATION, VERSION_ANNOTATION +from consts import CREATE_BY_ANNOTATION, LAST_SYNC_ANNOTATION, VERSION_ANNOTATION, BLACK_LISTED_ANNOTATIONS def patch_clustersecret_status( @@ -215,6 +215,7 @@ def sync_secret( body.type = secret_type body.data = data logger.info(f'cloning secret in namespace {namespace}') + logger.debug(f'V1Secret= {body}') try: # Get metadata from secrets (if exist) @@ -291,7 +292,12 @@ def create_secret_metadata( CREATE_BY_ANNOTATION: 'ClusterSecrets', VERSION_ANNOTATION: get_version(), LAST_SYNC_ANNOTATION: datetime.now().isoformat(), - }.update(annotations or {}) + } + _annotations.update(annotations or {}) + + # Remove potential useless / dangerous annotations + _annotations = {key: value for key, value in _annotations.items() if + not any(key.startswith(prefix) for prefix in BLACK_LISTED_ANNOTATIONS)} return V1ObjectMeta( name=name, From c012cb0f3cd262671dfbe4c69451517266a177b1 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 18 Sep 2023 18:59:12 +0200 Subject: [PATCH 6/8] feat: global improvements on annotations and labels --- .../templates/role-cluster-rbac.yaml | 1 + src/consts.py | 3 + src/handlers.py | 7 +- src/kubernetes_utils.py | 71 ++++++++++--------- yaml/00_rbac.yaml | 2 +- 5 files changed, 47 insertions(+), 37 deletions(-) diff --git a/charts/cluster-secret/templates/role-cluster-rbac.yaml b/charts/cluster-secret/templates/role-cluster-rbac.yaml index c1b534d..3a76d29 100644 --- a/charts/cluster-secret/templates/role-cluster-rbac.yaml +++ b/charts/cluster-secret/templates/role-cluster-rbac.yaml @@ -42,6 +42,7 @@ rules: - list - watch - patch + - get - apiGroups: - "" resources: diff --git a/src/consts.py b/src/consts.py index b304735..755cdf5 100644 --- a/src/consts.py +++ b/src/consts.py @@ -3,7 +3,10 @@ """ CREATE_BY_ANNOTATION = 'clustersecret.io/created-by' +CREATE_BY_AUTHOR = 'ClusterSecrets' LAST_SYNC_ANNOTATION = 'clustersecret.io/last-sync' VERSION_ANNOTATION = 'clustersecret.io/version' +CLUSTER_SECRET_LABEL = "clustersecret.io" + BLACK_LISTED_ANNOTATIONS = ["kopf.zalando.org", "kubectl.kubernetes.io"] \ No newline at end of file diff --git a/src/handlers.py b/src/handlers.py index ec1906b..d511cbb 100644 --- a/src/handlers.py +++ b/src/handlers.py @@ -129,16 +129,17 @@ def on_field_data( logger.info(f'Re Syncing secret {name} in ns {ns}') body = client.V1Secret( api_version='v1', - data=dict(new), + data={str(key): str(value) for key, value in new.items()}, kind='Secret', metadata=create_secret_metadata( name=name, namespace=ns, - annotations=meta.annotations, - labels=meta.labels, + annotations={str(key): str(value) for key, value in meta.annotations.items()}, + labels={str(key): str(value) for key, value in meta.labels.items()}, ), type=secret_type, ) + logger.debug(f'body: {body}') # Ensuring the secret still exist. if secret_exists(logger=logger, name=name, namespace=ns, v1=v1): response = v1.replace_namespaced_secret(name=name, namespace=ns, body=body) diff --git a/src/kubernetes_utils.py b/src/kubernetes_utils.py index 14e7177..779c6b3 100644 --- a/src/kubernetes_utils.py +++ b/src/kubernetes_utils.py @@ -7,15 +7,16 @@ from kubernetes.client import CoreV1Api, CustomObjectsApi, exceptions, V1ObjectMeta, rest, V1Secret from os_utils import get_replace_existing, get_version -from consts import CREATE_BY_ANNOTATION, LAST_SYNC_ANNOTATION, VERSION_ANNOTATION, BLACK_LISTED_ANNOTATIONS +from consts import CREATE_BY_ANNOTATION, LAST_SYNC_ANNOTATION, VERSION_ANNOTATION, BLACK_LISTED_ANNOTATIONS, \ + CREATE_BY_AUTHOR, CLUSTER_SECRET_LABEL def patch_clustersecret_status( - logger: logging.Logger, - namespace: str, - name: str, - new_status, - custom_objects_api: CustomObjectsApi, + logger: logging.Logger, + namespace: str, + name: str, + new_status, + custom_objects_api: CustomObjectsApi, ): """Patch the status of a given clustersecret object """ @@ -48,9 +49,9 @@ def patch_clustersecret_status( def get_ns_list( - logger: logging.Logger, - body: Dict[str, Any], - v1: CoreV1Api, + logger: logging.Logger, + body: Dict[str, Any], + v1: CoreV1Api, ) -> List[str]: """Returns a list of namespaces where the secret should be matched """ @@ -83,10 +84,10 @@ def get_ns_list( def read_data_secret( - logger: logging.Logger, - name: str, - namespace: str, - v1: CoreV1Api, + logger: logging.Logger, + name: str, + namespace: str, + v1: CoreV1Api, ) -> Dict[str, str]: """Gets the data from the 'name' secret in namespace """ @@ -107,10 +108,10 @@ def read_data_secret( def delete_secret( - logger: logging.Logger, - namespace: str, - name: str, - v1: CoreV1Api, + logger: logging.Logger, + namespace: str, + name: str, + v1: CoreV1Api, ): """Deletes a given secret from a given namespace """ @@ -126,10 +127,10 @@ def delete_secret( def secret_exists( - logger: logging.Logger, - name: str, - namespace: str, - v1: CoreV1Api, + logger: logging.Logger, + name: str, + namespace: str, + v1: CoreV1Api, ): return secret_metadata( logger=logger, @@ -140,10 +141,10 @@ def secret_exists( def secret_metadata( - logger: logging.Logger, - name: str, - namespace: str, - v1: CoreV1Api, + logger: logging.Logger, + name: str, + namespace: str, + v1: CoreV1Api, ) -> Optional[V1ObjectMeta]: try: secret = v1.read_namespaced_secret(name, namespace) @@ -156,10 +157,10 @@ def secret_metadata( def sync_secret( - logger: logging.Logger, - namespace: str, - body: Dict[str, Any], - v1: CoreV1Api, + logger: logging.Logger, + namespace: str, + body: Dict[str, Any], + v1: CoreV1Api, ): """Creates a given secret on a given namespace """ @@ -288,8 +289,13 @@ def create_secret_metadata( Kubernetes metadata object with ClusterSecret annotations. """ + _labels = { + CLUSTER_SECRET_LABEL: 'true' + } + _labels.update(labels or {}) + _annotations = { - CREATE_BY_ANNOTATION: 'ClusterSecrets', + CREATE_BY_ANNOTATION: CREATE_BY_AUTHOR, VERSION_ANNOTATION: get_version(), LAST_SYNC_ANNOTATION: datetime.now().isoformat(), } @@ -297,13 +303,13 @@ def create_secret_metadata( # Remove potential useless / dangerous annotations _annotations = {key: value for key, value in _annotations.items() if - not any(key.startswith(prefix) for prefix in BLACK_LISTED_ANNOTATIONS)} + not any(key.startswith(prefix) for prefix in BLACK_LISTED_ANNOTATIONS)} return V1ObjectMeta( name=name, namespace=namespace, annotations=_annotations, - labels=labels, + labels=_labels, ) @@ -340,4 +346,3 @@ def get_custom_objects_by_kind( except rest.ApiException as e: # Properly handle API exceptions raise rest.ApiException(f'Error while retrieving custom objects: {e}') - diff --git a/yaml/00_rbac.yaml b/yaml/00_rbac.yaml index 45abfa5..1df6754 100644 --- a/yaml/00_rbac.yaml +++ b/yaml/00_rbac.yaml @@ -33,7 +33,7 @@ rules: # Application: read-only access for watching cluster-wide. - apiGroups: [clustersecret.io] resources: [clustersecrets] - verbs: [list, watch, patch] + verbs: [list, watch, patch, get] # Watch namespaces - apiGroups: [""] From 4339b98c14af1fd2d5419bdfc844be63f7e61347 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 18 Sep 2023 19:00:38 +0200 Subject: [PATCH 7/8] fix: minor version boost --- charts/cluster-secret/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/cluster-secret/Chart.yaml b/charts/cluster-secret/Chart.yaml index 390f1f8..92f3639 100755 --- a/charts/cluster-secret/Chart.yaml +++ b/charts/cluster-secret/Chart.yaml @@ -3,7 +3,7 @@ name: cluster-secret description: ClusterSecret Operator kubeVersion: '>= 1.16.0-0' type: application -version: 0.2.1 +version: 0.2.2 icon: https://clustersecret.io/assets/csninjasmall.png sources: - https://github.com/zakkg3/ClusterSecret From ba0212fab20f3f450194a9c8fa70e5fd30da2dd0 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 18 Sep 2023 19:02:17 +0200 Subject: [PATCH 8/8] fix: annoying helm linter --- charts/cluster-secret/values.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/cluster-secret/values.yaml b/charts/cluster-secret/values.yaml index f2bfffd..19cc493 100644 --- a/charts/cluster-secret/values.yaml +++ b/charts/cluster-secret/values.yaml @@ -5,7 +5,7 @@ clustersecret: tag: 0.0.10 # use tag-alt for ARM and other alternative builds - read the readme for more information # If Clustersecret is about to create a secret and then it founds it exists: - # Default is to ignore it. (to not loose any unintentional data) + # Default is to ignore it. (to not loose any unintentional data) # It can also reeplace it. Just uncommenting next line. - #replace_existing: 'true' + # replace_existing: 'true' kubernetesClusterDomain: cluster.local