Merge pull request #346 from nolar/limited-annotations

Limit persistence annotations to 63 chars

kopf/storage/progress.py

@@ -39,10 +39,11 @@
 All timestamps are strings in ISO8601 format in UTC (no explicit ``Z`` suffix).
 """
 import abc
+import base64
 import copy
-import datetime
+import hashlib
 import json
-from typing import Optional, Collection, Mapping, Dict, Any, cast
+from typing import Optional, Collection, Mapping, Dict, Union, Any, cast
 
 from typing_extensions import TypedDict
 
@@ -176,8 +177,7 @@ def fetch(
             key: handlers.HandlerId,
             body: bodies.Body,
     ) -> Optional[ProgressRecord]:
-        safe_key = key.replace('/', '.')
-        full_key = f'{self.prefix}/{safe_key}' if self.prefix else safe_key
+        full_key = self.make_key(key)
         value = body.metadata.annotations.get(full_key, None)
         content = json.loads(value) if value is not None else None
         return cast(Optional[ProgressRecord], content)
@@ -190,8 +190,7 @@ def store(
             body: bodies.Body,
             patch: patches.Patch,
     ) -> None:
-        safe_key = key.replace('/', '.')
-        full_key = f'{self.prefix}/{safe_key}' if self.prefix else safe_key
+        full_key = self.make_key(key)
         clean_data = {key: val for key, val in record.items() if self.verbose or val is not None}
         patch.meta.annotations[full_key] = json.dumps(clean_data)
 
@@ -202,8 +201,7 @@ def purge(
             body: bodies.Body,
             patch: patches.Patch,
     ) -> None:
-        safe_key = key.replace('/', '.')
-        full_key = f'{self.prefix}/{safe_key}' if self.prefix else safe_key
+        full_key = self.make_key(key)
         if full_key in body.metadata.annotations or full_key in patch.meta.annotations:
             patch.meta.annotations[full_key] = None
 
@@ -214,9 +212,7 @@ def touch(
             patch: patches.Patch,
             value: Optional[str],
     ) -> None:
-        key = self.touch_key
-        safe_key = key.replace('/', '.')
-        full_key = f'{self.prefix}/{safe_key}' if self.prefix else safe_key
+        full_key = self.make_key(self.touch_key)
         if body.meta.annotations.get(full_key, None) != value:  # also covers absent-vs-None cases.
             patch.meta.annotations[full_key] = value
 
@@ -228,6 +224,25 @@ def clear(self, *, essence: bodies.BodyEssence) -> bodies.BodyEssence:
                 del annotations[name]
         return essence
 
+    def make_key(self, key: Union[str, handlers.HandlerId], max_length: int = 63) -> str:
+
+        # K8s has a limitation on the allowed charsets in annotation/label keys.
+        # https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set
+        safe_key = key.replace('/', '.')
+
+        # K8s has a limitation of 63 chars per annotation/label key.
+        # Force it to 63 chars by replacing the tail with a consistent hash (with full alphabet).
+        prefix = f'{self.prefix}/' if self.prefix else ''
+        if len(safe_key) <= max_length - len(prefix):
+            suffix = ''
+        else:
+            digest = hashlib.blake2b(safe_key.encode('utf-8'), digest_size=4).digest()
+            alnums = base64.b64encode(digest, altchars=b'-.').replace(b'=', b'-').decode('ascii')
+            suffix = f'-{alnums}'
+
+        full_key = f'{prefix}{safe_key[:max_length - len(prefix) - len(suffix)]}{suffix}'
+        return full_key
+
 
 class StatusProgressStorage(ProgressStorage):
     """

tests/persistence/test_annotations_hashing.py

@@ -0,0 +1,96 @@
+import json
+
+import pytest
+
+from kopf.structs.bodies import Body
+from kopf.structs.handlers import HandlerId
+from kopf.structs.patches import Patch
+from kopf.storage.progress import ProgressRecord, AnnotationsProgressStorage, SmartProgressStorage
+
+ANNOTATIONS_POPULATING_STORAGES = [AnnotationsProgressStorage, SmartProgressStorage]
+
+CONTENT_DATA = ProgressRecord(
+    started='2020-01-01T00:00:00',
+    stopped='2020-12-31T23:59:59',
+    delayed='3000-01-01T00:00:00',
+    retries=0,
+    success=False,
+    failure=False,
+    message=None,
+)
+
+CONTENT_JSON = json.dumps(CONTENT_DATA)  # the same serialisation for all environments
+
+
+keys = pytest.mark.parametrize('prefix, provided_key, expected_key', [
+
+    # For character replacements (only those that happen in our own ids, not all of them).
+    ['my-operator.example.com', 'a_b.c-d/e', 'my-operator.example.com/a_b.c-d.e'],
+    [None, 'a_b.c-d/e', 'a_b.c-d.e'],
+
+    # For length cutting. Hint: the prefix length is 23, the remaining space is 63 - 23 - 1 = 39.
+    # The suffix itself (if appended) takes 9, so it is 30 left. The same math for no prefix.
+    ['my-operator.example.com', 'x', 'my-operator.example.com/x'],
+    ['my-operator.example.com', 'x' * 39, 'my-operator.example.com/' + 'x' * 39],
+    ['my-operator.example.com', 'x' * 40, 'my-operator.example.com/' + 'x' * 30 + '-tEokcg--'],
+    ['my-operator.example.com', 'y' * 40, 'my-operator.example.com/' + 'y' * 30 + '-VZlvhw--'],
+    ['my-operator.example.com', 'z' * 40, 'my-operator.example.com/' + 'z' * 30 + '-LlPQyA--'],
+    [None, 'x', 'x'],
+    [None, 'x' * 63, 'x' * 63],
+    [None, 'x' * 64, 'x' * 54 + '-SItAqA--'],
+    [None, 'y' * 64, 'y' * 54 + '-0d251g--'],
+    [None, 'z' * 64, 'z' * 54 + '-E7wvIA--'],
+
+    # For special chars in base64 encoding ("+" and "/"), which are not compatible with K8s.
+    # The numbers are found empirically so that both "/" and "+" are found in the base64'ed digest.
+    ['my-operator.example.com', 'fn' * 323, 'my-operator.example.com/' + 'fn' * 15 + '-Az-r.g--'],
+    [None, 'fn' * 323, 'fn' * 27 + '-Az-r.g--'],
+
+])
+
+
+@keys
+def test_key_hashing(prefix, provided_key, expected_key):
+    storage = AnnotationsProgressStorage(prefix=prefix)
+    returned_key = storage.make_key(provided_key)
+    assert returned_key == expected_key
+
+
+@keys
+@pytest.mark.parametrize('cls', ANNOTATIONS_POPULATING_STORAGES)
+def test_keys_hashed_on_fetching(cls, prefix, provided_key, expected_key):
+    storage = cls(prefix=prefix)
+    body = Body({'metadata': {'annotations': {expected_key: CONTENT_JSON}}})
+    record = storage.fetch(body=body, key=HandlerId(provided_key))
+    assert record is not None
+    assert record == CONTENT_DATA
+
+
+@keys
+@pytest.mark.parametrize('cls', ANNOTATIONS_POPULATING_STORAGES)
+def test_keys_normalized_on_storing(cls, prefix, provided_key, expected_key):
+    storage = cls(prefix=prefix)
+    patch = Patch()
+    body = Body({'metadata': {'annotations': {expected_key: 'null'}}})
+    storage.store(body=body, patch=patch, key=HandlerId(provided_key), record=CONTENT_DATA)
+    assert set(patch.metadata.annotations) == {expected_key}
+
+
+@keys
+@pytest.mark.parametrize('cls', ANNOTATIONS_POPULATING_STORAGES)
+def test_keys_normalized_on_purging(cls, prefix, provided_key, expected_key):
+    storage = cls(prefix=prefix)
+    patch = Patch()
+    body = Body({'metadata': {'annotations': {expected_key: 'null'}}})
+    storage.purge(body=body, patch=patch, key=HandlerId(provided_key))
+    assert set(patch.metadata.annotations) == {expected_key}
+
+
+@keys
+@pytest.mark.parametrize('cls', ANNOTATIONS_POPULATING_STORAGES)
+def test_keys_normalized_on_touching(cls, prefix, provided_key, expected_key):
+    storage = cls(prefix=prefix, touch_key=provided_key)
+    patch = Patch()
+    body = Body({})
+    storage.touch(body=body, patch=patch, value='irrelevant')
+    assert set(patch.metadata.annotations) == {expected_key}