Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Commit

Permalink
Limit persistence annotations to 63 chars
Browse files Browse the repository at this point in the history
  • Loading branch information
nolar committed Apr 9, 2020
1 parent 698171a commit 4761ec9
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 11 deletions.
37 changes: 26 additions & 11 deletions kopf/storage/progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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):
"""
Expand Down
96 changes: 96 additions & 0 deletions tests/persistence/test_annotations_hashing.py
Original file line number Diff line number Diff line change
@@ -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}

0 comments on commit 4761ec9

Please sign in to comment.