Skip to content

Commit

Permalink
Chef - Don't use metadata for hash generation in naming convention
Browse files Browse the repository at this point in the history
  • Loading branch information
cpagravel committed Mar 29, 2023
1 parent 6e37dc3 commit 626dc32
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 79 deletions.
2 changes: 1 addition & 1 deletion examples/chef/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pw_python_package("chef") {

inputs = [
"sample_app_util/test_files/sample_zap_file.zap",
"sample_app_util/test_files/sample_zap_file_hashmeta.yaml",
"sample_app_util/test_files/sample_zap_file_meta.yaml",
]

sources = [
Expand Down
2 changes: 1 addition & 1 deletion examples/chef/chef.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def bundle(platform: str, device_name: str) -> None:
dest_item = os.path.join(_CD_STAGING_DIR, matter_file)
shutil.copy(src_item, dest_item)
flush_print(f"Generating metadata for {device_name}")
metadata_file = zap_file_parser.generate_hash_metadata_file(zap_file)
metadata_file = zap_file_parser.generate_metadata_file(zap_file)
metadata_dest = os.path.join(_CD_STAGING_DIR,
os.path.basename(metadata_file))
shutil.copy(metadata_file, metadata_dest)
Expand Down
2 changes: 1 addition & 1 deletion examples/chef/dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ RUN apt-get update && \
COPY out/${DEVICE_NAME} /chef_device
COPY out/${DEVICE_NAME}.map /chef_device.map
COPY out/${DEVICE_NAME}.matter /chef_device.matter
COPY out/${DEVICE_NAME}_hashmeta.yaml /chef_device_hashmeta.yaml
COPY out/${DEVICE_NAME}_meta.yaml /chef_device_meta.yaml

ENV DBUS_SYSTEM_BUS_ADDRESS="unix:path=/var/run/dbus/system_bus_socket"

Expand Down
27 changes: 11 additions & 16 deletions examples/chef/sample_app_util/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ should rarely happen.

Sample apps should be named by concatenating the name of all endpoints in the
order of their index. Endpoint names are separated by underscores (`_`) and a 10
character hash[^hash_note] of the sample app metadata is appended to the end.
character hash[^hash_note] which is randomly generated.

The naming convention tool should be used for every newly created app. The name
should only need to be updated if the endpoints of the app are changed.

Valid sample app names conform to the following format:

Expand Down Expand Up @@ -62,16 +65,12 @@ bridgednode_temperaturesensor_onofflight_onoffpluginunit_MI9DSdkH8H

[^hash_note]:

The 10 character hash is a base64 encoding of the md5 hash generated by
digesting the JSON string encoding of the metadata information. The code for
generating the hash can be found in `generate_hash` in
[zap_file_parser](zap_file_parser.py) There are some notable details
here: 1) The full base64 encoded hash is 16 characters, but only 10 are
used. This still gives us a sufficiently low probability of collision (~1.2
x 10^-8). 2) `_` and `-` are replaced in the base64 encoding because they
have other uses in the naming. 3) Platform specific information is omitted
from the hash. E.g. the networking_commissioning cluster is excluded. This
is to make the hashes platform agnostic.
The 10 character hash used to be generated by consuming the JSON encoded
metadata. This is no longer the case. It was found that the hash encoding
only served to differentiate one app from another (the initial goal) and
there was little value in tying the generated hash to the config data
because there wasn't a use case for decoding it as long as the config file
was packaged with the generated app.

### Sample App Build Naming Convention

Expand Down Expand Up @@ -139,13 +138,9 @@ The metadata files have a structure as follows:
```

For an example, see
[sample_zap_file_hashmeta.yaml](test_files/sample_zap_file_hashmeta.yaml) which
[sample_zap_file_meta.yaml](test_files/sample_zap_file_meta.yaml) which
was generated from [sample_zap_file.zap](test_files/sample_zap_file.zap).

Note that it is more readable in `yaml` format. Since hashes are generated from
the metadata info, additional conventions are needed to ensure consistency for
the metadata structure.

The following conventions are used:

- All lists are sorted alphabetically.
Expand Down
9 changes: 4 additions & 5 deletions examples/chef/sample_app_util/sample_app_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ def zap_cmd_handler(args: argparse.Namespace) -> None:
output_path = os.path.join(dirpath, f"{name}.zap")
shutil.move(zap_file_path, output_path)
print(f"Renamed from: {zap_file_path} to {output_path}")
elif args.generate_hash_metadata:
created_file = zap_file_parser.generate_hash_metadata_file(zap_file_path)
elif args.generate_metadata:
created_file = zap_file_parser.generate_metadata_file(zap_file_path)
print(f"Created {created_file}")


Expand All @@ -53,10 +53,9 @@ def zap_cmd_handler(args: argparse.Namespace) -> None:
)

zap_cmd_parser.add_argument(
"--generate-hash-metadata", action="store_true",
"--generate-metadata", action="store_true",
help=(
"Generate the hash metadata file which provide information about what was included in "
"the hash digest.")
"Generate the metadata file which provides summary information about the app.")
)

zap_cmd_group.add_argument(
Expand Down
22 changes: 13 additions & 9 deletions examples/chef/sample_app_util/test_zap_file_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import shutil
import tempfile
import unittest
from unittest import mock
import uuid

import zap_file_parser

Expand All @@ -20,20 +22,21 @@

_HERE = os.path.abspath(os.path.dirname(__file__))
_TEST_FILE = os.path.join(_HERE, "test_files", "sample_zap_file.zap")
_TEST_METADATA = os.path.join(_HERE, "test_files", "sample_zap_file_hashmeta.yaml")
_TEST_METADATA = os.path.join(_HERE, "test_files", "sample_zap_file_meta.yaml")


class TestZapFileParser(unittest.TestCase):
"""Testcases for zap_file_parser.py."""

def test_generate_hash(self):
"""Tests generate_hash function."""
hash_string = zap_file_parser.generate_hash(_TEST_FILE)
self.assertEqual(hash_string, "Xir1gEfjij", "Hash is incorrectly generated.")
with mock.patch.object(uuid, "uuid4", return_value="Xir1gEfjij"):
hash_string = zap_file_parser.generate_hash()
self.assertEqual(hash_string, "Xir1gEfjij", "Hash is incorrectly generated.")

def test_generate_metadata(self):
"""Tests generate_metadata."""
generated_metadata = zap_file_parser.generate_hash_metadata(_TEST_FILE)
generated_metadata = zap_file_parser.generate_metadata(_TEST_FILE)
with open(_TEST_METADATA) as f:
expected_metadata = yaml.load(f.read(), Loader=yaml.FullLoader)
self.assertEqual(
Expand All @@ -43,10 +46,10 @@ def test_generate_metadata_file(self):
"""Tests generate_metadata_file."""
with tempfile.TemporaryDirectory(dir=os.path.dirname(_HERE)) as dir:
zap_file = os.path.join(dir, "test_file.zap")
hashmeta_file = os.path.join(dir, "test_file_hashmeta.yaml")
meta_file = os.path.join(dir, "test_file_meta.yaml")
shutil.copy(_TEST_FILE, zap_file)
zap_file_parser.generate_hash_metadata_file(zap_file)
with open(hashmeta_file) as f:
zap_file_parser.generate_metadata_file(zap_file)
with open(meta_file) as f:
generated_metadata = yaml.load(f.read(), Loader=yaml.FullLoader)
with open(_TEST_METADATA) as f:
expected_metadata = yaml.load(f.read(), Loader=yaml.FullLoader)
Expand All @@ -55,8 +58,9 @@ def test_generate_metadata_file(self):

def test_generate_name(self):
"""Tests generate_name."""
name = zap_file_parser.generate_name(_TEST_FILE)
self.assertEqual(name, "rootnode_dimmablelight_Xir1gEfjij", "Name incorrectly generated.")
with mock.patch.object(uuid, "uuid4", return_value="Xir1gEfjij"):
name = zap_file_parser.generate_name(_TEST_FILE)
self.assertEqual(name, "rootnode_dimmablelight_Xir1gEfjij", "Name incorrectly generated.")


if __name__ == "__main__":
Expand Down
54 changes: 9 additions & 45 deletions examples/chef/sample_app_util/zap_file_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@
available.
- Add support for .matter files.
"""
import base64
import copy
import hashlib
import json
import os
import re
from typing import Dict, List, Optional, Sequence, TypedDict, Union
import uuid

try:
import yaml
Expand Down Expand Up @@ -64,19 +63,6 @@ class EndpointType(TypedDict):
server_clusters: Dict[str, ClusterType]


def _b64encode(input_data: bytes) -> bytes:
"""Returns urlsafe base64 encoded with padding removed."""
return base64.urlsafe_b64encode(input_data).strip(b"=")


def _b64decode(input_data: bytes) -> bytes:
"""Returns urlsafe base64 decoded with padding added."""
# "=" is padding character that doesn't carry info.
# Adding 2x "=" will handle all edge cases where there may be
# incorrect number of bytes.
return base64.urlsafe_b64decode(input_data + b"==")


def _convert_metadata_name(name: str, code: Union[int, str]) -> str:
"""Converts a name for use in a metadata file - CamelCaseName/ID."""
# Preserve camel case if it's already there
Expand Down Expand Up @@ -181,35 +167,13 @@ def _get_id(name):
return name.split("/")[-1]


def generate_hash(zap_file_path: str) -> str:
def generate_hash() -> str:
"""Generates a hash for a zap file.
Args:
zap_file_path: Path to the zap file.
Returns:
MD5 hash of the metadata generated from the zap file.
This is converted to base64 and then the first 10 characters are used.
A 10 character alphanumeric hash.
"""
parsed = generate_hash_metadata(zap_file_path)
# Use json.dumps to produce a consistent output for the object passed into it.
digestible_content = _convert_metadata_to_hashable_digest(parsed)
md5_hash = hashlib.md5(digestible_content.encode("utf-8")).digest()
output = str(_b64encode(md5_hash), encoding="utf-8")[:10]
# Replace "-" and "_" with "a" and "b".
# The reason for doing this is to allow the generated name to be parsed by splitting on "_".
# Replacing "-" makes the name easier to parse visually.
# This increases likelihood of hash collisions, but minimally so. See module docstring.
return output.replace("-", "a").replace("_", "b")


def generate_hash_metadata(zap_file_path: str) -> List[Dict[str, EndpointType]]:
"""Generates metadata for hash digest consumption."""
return generate_metadata(
zap_file_path=zap_file_path,
attribute_allow_list=_ATTRIBUTE_ALLOW_LIST,
include_commands=False,
include_platform_specific_info=False)
return str(uuid.uuid4())[-10:]


def generate_metadata(
Expand Down Expand Up @@ -320,21 +284,21 @@ def generate_metadata(
return return_obj


def generate_hash_metadata_file(zap_file_path: str) -> str:
"""Generates hash metadata file for a zap file input.
def generate_metadata_file(zap_file_path: str) -> str:
"""Generates metadata file for a zap file.
The purpose of this file is to inform the user what data was included in the hash digest.
Args:
zap_file_path: Path to the zap file to parse for generating the metadata file.
"""
parsed = generate_hash_metadata(zap_file_path)
parsed = generate_metadata(zap_file_path)
output = yaml.dump(parsed, indent=4, sort_keys=True)

dirname, filename = os.path.split(zap_file_path)

filename = os.path.splitext(filename)[0]
output_file_path = os.path.join(dirname, f"{filename}_hashmeta.yaml")
output_file_path = os.path.join(dirname, f"{filename}_meta.yaml")
with open(output_file_path, "w") as f:
f.write(output)
return output_file_path
Expand All @@ -354,5 +318,5 @@ def generate_name(zap_file_path: str) -> str:
for endpoint in parsed:
name = next(iter(endpoint))
names.append(_convert_filename(name))
hash_string = generate_hash(zap_file_path)
hash_string = generate_hash()
return "_".join(names) + f"_{hash_string}"
2 changes: 1 addition & 1 deletion examples/chef/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ zip_safe = False
[options.package_data]
sample_app_util =
test_files/sample_zap_file.zap
test_files/sample_zap_file_hashmeta.yaml
test_files/sample_zap_file_meta.yaml

[options.entry_points]
console_scripts = chef = chef:main

0 comments on commit 626dc32

Please sign in to comment.