Skip to content

Commit

Permalink
fix(RHTAPBUGS-756): set digest in repo when creating an image in Pyxis (
Browse files Browse the repository at this point in the history
#58)

Based on the mediaType returned by `skopeo inspect --raw`,
the create_container_image will determine if the image
is single arch or multi arch and based on that
it will populate either manifest_schema2_digest
or manifest_list_digest in the embedded
ContainerImageRepo object when creating a new
image in Pyxis.

This also removes the docker_image_digest and
image_id fields when creating the image. This is to align
with metaxor which does not populate these either.

This also affects the mechanism to find out if the image
already exists - we can no longer rely on the digest
field in the ContainerImage object, instead we have
to check the right field in the embedded repository.

Note that currently we do not really support multi arch
images in RHTAP and other changes will likely be needed
to fully support them.

Signed-off-by: Martin Malina <[email protected]>
  • Loading branch information
mmalina authored Sep 14, 2023
1 parent 06643f9 commit b198ed7
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 13 deletions.
34 changes: 31 additions & 3 deletions pyxis/create_container_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@

LOGGER = logging.getLogger("create_container_image")

# Media types that are used for multi arch images
MANIFEST_LIST_TYPES = [
"application/vnd.oci.image.index.v1+json",
"application/vnd.docker.distribution.manifest.list.v2+json",
]


def setup_argparser() -> Any: # pragma: no cover
"""Setup argument parser
Expand Down Expand Up @@ -42,6 +48,12 @@ def setup_argparser() -> Any: # pragma: no cover
help="Should the `latest` tag of the ContainerImage be overwritten?",
required=True,
)
parser.add_argument(
"--media-type",
help="The mediaType string returned by `skopeo inspect --raw`. "
"Used to determine if it's a single arch or multiarch image.",
required=True,
)
parser.add_argument("--verbose", action="store_true", help="Verbose output")
return parser

Expand All @@ -52,9 +64,10 @@ def image_already_exists(args, digest: str) -> bool:
:return: True if one exists, else false
"""
digest_field = get_digest_field(args.media_type)

# quote is needed to urlparse the quotation marks
filter_str = quote(f'docker_image_digest=="{digest}";' f"not(deleted==true)")
filter_str = quote(f'repositories.{digest_field}=="{digest}";not(deleted==true)')

check_url = urljoin(args.pyxis_url, f"v1/images?page_size=1&filter={filter_str}")

Expand Down Expand Up @@ -131,8 +144,6 @@ def create_container_image(args, parsed_data: Dict[str, Any]):
}
],
"certified": json.loads(args.certified.lower()),
"docker_image_digest": docker_image_digest,
"image_id": docker_image_digest,
"architecture": parsed_data["architecture"],
"parsed_data": parsed_data,
}
Expand All @@ -145,6 +156,9 @@ def create_container_image(args, parsed_data: Dict[str, Any]):
}
)

digest_field = get_digest_field(args.media_type)
container_image_payload["repositories"][0][digest_field] = docker_image_digest

rsp = pyxis.post(upload_url, container_image_payload).json()

# Make sure container metadata was successfully added to Pyxis
Expand All @@ -154,6 +168,20 @@ def create_container_image(args, parsed_data: Dict[str, Any]):
raise Exception("Image metadata was not successfully added to Pyxis.")


def get_digest_field(media_type: str) -> str:
"""This will return one of the two possible digest fields
to use in the repository object which is embedded in the
ContainerImage object.
manifest_schema2_digest is used for single arch images,
manifest_list_digest is used for multi arch images
"""
if media_type in MANIFEST_LIST_TYPES:
return "manifest_list_digest"
else:
return "manifest_schema2_digest"


def main(): # pragma: no cover
"""Main func"""

Expand Down
64 changes: 54 additions & 10 deletions pyxis/test_create_container_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,67 @@
image_already_exists,
create_container_image,
prepare_parsed_data,
get_digest_field,
)


mock_pyxis_url = "https://catalog.redhat.com/api/containers/"


@patch("create_container_image.get_digest_field")
@patch("create_container_image.pyxis.get")
def test_image_already_exists(mock_get: MagicMock):
def test_image_already_exists__image_does_exist(mock_get, mock_get_digest_field: MagicMock):
# Arrange
mock_rsp = MagicMock()
mock_get.return_value = mock_rsp

args = MagicMock()
args.pyxis_url = mock_pyxis_url
digest = "some_digest"
mock_get_digest_field.return_value = "digest_field"

# Image already exists
mock_rsp.json.return_value = {"data": [{"_id": 0}]}

# Act
exists = image_already_exists(args, digest)

# Assert
assert exists
mock_get.assert_called_with(
mock_get_digest_field.assert_called_once_with(args.media_type)
mock_get.assert_called_once_with(
mock_pyxis_url
+ "v1/images?page_size=1&filter="
+ "docker_image_digest%3D%3D%22some_digest%22%3Bnot%28deleted%3D%3Dtrue%29"
+ "repositories.digest_field%3D%3D%22some_digest%22%3Bnot%28deleted%3D%3Dtrue%29"
)


@patch("create_container_image.get_digest_field")
@patch("create_container_image.pyxis.get")
def test_image_already_exists__image_does_not_exist(
mock_get, mock_get_digest_field: MagicMock
):
# Arrange
mock_rsp = MagicMock()
mock_get.return_value = mock_rsp
args = MagicMock()
args.pyxis_url = mock_pyxis_url
digest = "some_digest"
mock_get_digest_field.return_value = "digest_field"

# Image doesn't exist
mock_rsp.json.return_value = {"data": []}

# Act
exists = image_already_exists(args, digest)

# Assert
assert not exists


@patch("create_container_image.get_digest_field")
@patch("create_container_image.pyxis.post")
@patch("create_container_image.datetime")
def test_create_container_image(mock_datetime: MagicMock, mock_post: MagicMock):
def test_create_container_image(mock_datetime, mock_post, mock_get_digest_field: MagicMock):
# Mock an _id in the response for logger check
mock_post.return_value.json.return_value = {"_id": 0}

Expand All @@ -57,6 +77,7 @@ def test_create_container_image(mock_datetime: MagicMock, mock_post: MagicMock):
args.pyxis_url = mock_pyxis_url
args.tag = "some_version"
args.certified = "false"
mock_get_digest_field.return_value = "digest_field"

# Act
create_container_image(
Expand All @@ -80,20 +101,23 @@ def test_create_container_image(mock_datetime: MagicMock, mock_post: MagicMock):
"name": "some_version",
}
],
"digest_field": "some_digest",
}
],
"certified": False,
"docker_image_digest": "some_digest",
"image_id": "some_digest",
"architecture": "ok",
"parsed_data": {"architecture": "ok"},
},
)
mock_get_digest_field.assert_called_once_with(args.media_type)


@patch("create_container_image.get_digest_field")
@patch("create_container_image.pyxis.post")
@patch("create_container_image.datetime")
def test_create_container_image_latest(mock_datetime: MagicMock, mock_post: MagicMock):
def test_create_container_image_latest(
mock_datetime, mock_post, mock_get_digest_field: MagicMock
):
# Mock an _id in the response for logger check
mock_post.return_value.json.return_value = {"_id": 0}

Expand All @@ -105,6 +129,7 @@ def test_create_container_image_latest(mock_datetime: MagicMock, mock_post: Magi
args.tag = "some_version"
args.certified = "false"
args.is_latest = "true"
mock_get_digest_field.return_value = "digest_field"

# Act
create_container_image(
Expand Down Expand Up @@ -136,11 +161,10 @@ def test_create_container_image_latest(mock_datetime: MagicMock, mock_post: Magi
"name": "latest",
},
],
"digest_field": "some_digest",
}
],
"certified": False,
"docker_image_digest": "some_digest",
"image_id": "some_digest",
"architecture": "ok",
"parsed_data": {"architecture": "ok"},
},
Expand Down Expand Up @@ -196,3 +220,23 @@ def test_prepare_parsed_data():
"layers": ["1", "2"],
"name": "quay.io/hacbs-release/release-service-utils",
}


def test_get_digest_field():
"""This will test that the common mediaType strings are translated to the correct
digest field to be used in the image.repository object"""
assert (
get_digest_field("application/vnd.docker.distribution.manifest.v2+json")
== "manifest_schema2_digest"
)
assert (
get_digest_field("application/vnd.oci.image.manifest.v1+json")
== "manifest_schema2_digest"
)
assert (
get_digest_field("application/vnd.docker.distribution.manifest.list.v2+json")
== "manifest_list_digest"
)
assert (
get_digest_field("application/vnd.oci.image.index.v1+json") == "manifest_list_digest"
)

0 comments on commit b198ed7

Please sign in to comment.