Skip to content

Commit

Permalink
Skip empty labels in label creation
Browse files Browse the repository at this point in the history
This can happen if a Dockerfile contains a LABEL statement without a key value. In this case, we simply ignore the label and don't add it to our table, as it has no defined key.
  • Loading branch information
Joseph Schorr committed Jan 16, 2020
1 parent 345bc6a commit d97a1f6
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 5 deletions.
2 changes: 1 addition & 1 deletion data/model/oci/label.py
Expand Up @@ -65,7 +65,7 @@ def create_manifest_label(

# Note that we don't prevent invalid label names coming from the manifest to be stored, as Docker
# does not currently prevent them from being put into said manifests.
if not validate_label_key(key) and source_type_name != "manifest":
if source_type_name != "manifest" and not validate_label_key(key):
raise InvalidLabelKeyException("Key `%s` is invalid or reserved" % key)

# Find the matching media type. If none specified, we infer.
Expand Down
7 changes: 7 additions & 0 deletions data/model/oci/manifest.py
Expand Up @@ -310,9 +310,16 @@ def _create_manifest(
create_temporary_tag_if_necessary(manifest, temp_tag_expiration_sec)

# Define the labels for the manifest (if any).
# TODO: Once the old data model is gone, turn this into a batch operation and make the label
# application to the manifest occur under the transaction.
labels = manifest_interface_instance.get_manifest_labels(retriever)
if labels:
for key, value in labels.iteritems():
# NOTE: There can technically be empty label keys via Dockerfile's. We ignore any
# such `labels`, as they don't really mean anything.
if not key:
continue

media_type = "application/json" if is_json(value) else "text/plain"
create_manifest_label(manifest, key, value, "manifest", media_type)

Expand Down
5 changes: 5 additions & 0 deletions data/registry_model/registry_pre_oci_model.py
Expand Up @@ -203,6 +203,11 @@ def create_manifest_and_retarget_tag(
for key, value in manifest_interface_instance.layers[
-1
].v1_metadata.labels.iteritems():
# NOTE: There can technically be empty label keys via Dockerfile's. We ignore any
# such `labels`, as they don't really mean anything.
if not key:
continue

media_type = "application/json" if is_json(value) else "text/plain"
add_label(key, value, "manifest", media_type)
has_labels = True
Expand Down
11 changes: 7 additions & 4 deletions test/registry/registry_tests.py
Expand Up @@ -979,6 +979,8 @@ def test_wrong_image_order(legacy_pusher, liveserver_session, app_reloader):
[("somejson", '{"some": "json"}', "application/json"), ("plain", "", "text/plain")],
# JSON-esque (but not valid JSON) labels.
[("foo", "[hello world]", "text/plain"), ("bar", "{wassup?!}", "text/plain")],
# Empty key label.
[("", "somevalue", "text/plain")],
],
)
def test_labels(labels, manifest_protocol, liveserver_session, api_caller, app_reloader):
Expand All @@ -1004,14 +1006,15 @@ def test_labels(labels, manifest_protocol, liveserver_session, api_caller, app_r

data = api_caller.get("/api/v1/repository/devtable/newrepo/manifest/%s/labels" % digest).json()
labels_found = data["labels"]
assert len(labels_found) == len(labels)
assert len(labels_found) == len([label for label in labels if label[0]])

labels_found_map = {l["key"]: l for l in labels_found}
assert set(images[0].config["Labels"].keys()) == set(labels_found_map.keys())
assert set([k for k in images[0].config["Labels"].keys() if k]) == set(labels_found_map.keys())

for key, _, media_type in labels:
assert labels_found_map[key]["source_type"] == "manifest"
assert labels_found_map[key]["media_type"] == media_type
if key:
assert labels_found_map[key]["source_type"] == "manifest"
assert labels_found_map[key]["media_type"] == media_type


@pytest.mark.parametrize(
Expand Down

0 comments on commit d97a1f6

Please sign in to comment.