#1821 Resolve container tags to digests
Merged 23 days ago by lsedlar. Opened 2 months ago by lsedlar.

file modified
+41 -29
@@ -265,6 +265,28 @@ 

              if error.validator in ("anyOf", "oneOf"):

                  for suberror in error.context:

                      errors.append("    Possible reason: %s" % suberror.message)

+ 

+     # Resolve container tags in extra_files

+     tag_resolver = util.ContainerTagResolver(offline=offline)

+     if config.get("extra_files"):

+         for _, arch_dict in config["extra_files"]:

+             for value in arch_dict.values():

+                 if isinstance(value, dict):

+                     _resolve_container_tag(value, tag_resolver)

+                 elif isinstance(value, list):

+                     for subinstance in value:

+                         _resolve_container_tag(subinstance, tag_resolver)

+     if config.get("extra_isos"):

+         for cfgs in config["extra_isos"].values():

+             if not isinstance(cfgs, list):

+                 cfgs = [cfgs]

+             for cfg in cfgs:

+                 if isinstance(cfg.get("extra_files"), dict):

+                     _resolve_container_tag(cfg["extra_files"], tag_resolver)

+                 elif isinstance(cfg.get("extra_files"), list):

+                     for c in cfg["extra_files"]:

+                         _resolve_container_tag(c, tag_resolver)

+ 

      return (errors + _validate_requires(schema, config, CONFIG_DEPS), warnings)

  

  
@@ -533,6 +555,18 @@ 

              "str_or_scm_dict": {

                  "anyOf": [{"type": "string"}, {"$ref": "#/definitions/scm_dict"}]

              },

+             "extra_file": {

+                 "type": "object",

+                 "properties": {

+                     "scm": {"type": "string"},

+                     "repo": {"type": "string"},

+                     "branch": {"$ref": "#/definitions/optional_string"},

+                     "file": {"$ref": "#/definitions/strings"},

+                     "dir": {"$ref": "#/definitions/strings"},

+                     "target": {"type": "string"},

+                 },

+                 "additionalProperties": False,

+             },

              "repo_dict": {

                  "type": "object",

                  "properties": {
@@ -935,20 +969,7 @@ 

                              "properties": {

                                  "include_variants": {"$ref": "#/definitions/strings"},

                                  "extra_files": _one_or_list(

-                                     {

-                                         "type": "object",

-                                         "properties": {

-                                             "scm": {"type": "string"},

-                                             "repo": {"type": "string"},

-                                             "branch": {

-                                                 "$ref": "#/definitions/optional_string"

-                                             },

-                                             "file": {"$ref": "#/definitions/strings"},

-                                             "dir": {"$ref": "#/definitions/strings"},

-                                             "target": {"type": "string"},

-                                         },

-                                         "additionalProperties": False,

-                                     }

+                                     {"$ref": "#/definitions/extra_file"}

                                  ),

                                  "filename": {"type": "string"},

                                  "volid": {"$ref": "#/definitions/strings"},
@@ -1471,21 +1492,7 @@ 

                  "additionalProperties": False,

              },

              "extra_files": _variant_arch_mapping(

-                 {

-                     "type": "array",

-                     "items": {

-                         "type": "object",

-                         "properties": {

-                             "scm": {"type": "string"},

-                             "repo": {"type": "string"},

-                             "branch": {"$ref": "#/definitions/optional_string"},

-                             "file": {"$ref": "#/definitions/strings"},

-                             "dir": {"type": "string"},

-                             "target": {"type": "string"},

-                         },

-                         "additionalProperties": False,

-                     },

-                 }

+                 {"type": "array", "items": {"$ref": "#/definitions/extra_file"}}

              ),

              "gather_lookaside_repos": _variant_arch_mapping(

                  {"$ref": "#/definitions/strings"}
@@ -1611,3 +1618,8 @@ 

  

  def _get_default_gather_backend():

      return "dnf"

+ 

+ 

+ def _resolve_container_tag(instance, tag_resolver):

+     if instance.get("scm") == "container-image":

+         instance["repo"] = tag_resolver(instance["repo"])

file modified
+39
@@ -250,6 +250,45 @@ 

          return self.cache[key]

  

  

+ class ContainerTagResolver(object):

+     """

+     A caching resolver for container image urls that replaces tags with digests.

+     """

+ 

+     def __init__(self, offline=False):

+         self.offline = offline

+         self.cache = {}

+ 

+     def __call__(self, url):

+         if self.offline:

+             # We're offline, nothing to do

+             return url

+         if re.match(".*@sha256:[a-z0.9]+", url):

+             # We already have a digest

+             return url

+         if url not in self.cache:

+             self.cache[url] = self._resolve(url)

+         return self.cache[url]

+ 

+     def _resolve(self, url):

+         m = re.match("^.+(:.+)$", url)

+         if not m:

+             raise RuntimeError("Failed to find tag name")

+         tag = m.group(1)

+ 

+         data = _skopeo_inspect(url)

+         digest = data["Digest"]

+         return url.replace(tag, f"@{digest}")

+ 

+ 

+ def _skopeo_inspect(url):

+     """Wrapper for running `skopeo inspect {url}` and parsing the output."""

+     cp = subprocess.run(

+         ["skopeo", "inspect", url], stdout=subprocess.PIPE, check=True, encoding="utf-8"

+     )

+     return json.loads(cp.stdout)

+ 

+ 

  # format: {arch|*: [data]}

  def get_arch_data(conf, var_name, arch):

      result = []

file modified
+38
@@ -251,6 +251,44 @@ 

          self.assertEqual(mock_resolve.call_args_list, [mock.call(url, None)])

  

  

+ class TestContainerTagResolver(unittest.TestCase):

+     @mock.patch("pungi.util._skopeo_inspect")

+     def test_offline(self, inspect):

+         resolver = util.ContainerTagResolver(offline=True)

+         url = "docker://example.com/repo:latest"

+         assert url == resolver(url)

+         assert inspect.mock_calls == []

+ 

+     @mock.patch("pungi.util._skopeo_inspect")

+     def test_already_digest(self, inspect):

+         resolver = util.ContainerTagResolver()

+         url = "docker://example.com/repo@sha256:abcdef0123456789"

+         assert url == resolver(url)

+         assert inspect.mock_calls == []

+ 

+     @mock.patch("pungi.util._skopeo_inspect")

+     def test_simple(self, inspect):

+         url = "docker://example.com/repo"

+         digest = "sha256:abcdef"

+         orig_url = f"{url}:latest"

+         inspect.return_value = {"Digest": digest}

+         resolver = util.ContainerTagResolver()

+         assert f"{url}@{digest}" == resolver(orig_url)

+         assert inspect.mock_calls == [mock.call(orig_url)]

+ 

+     @mock.patch("pungi.util._skopeo_inspect")

+     def test_caching(self, inspect):

+         url = "docker://example.com/repo"

+         digest = "sha256:abcdef"

+         orig_url = f"{url}:latest"

+         inspect.return_value = {"Digest": digest}

+         resolver = util.ContainerTagResolver()

+         assert f"{url}@{digest}" == resolver(orig_url)

+         assert f"{url}@{digest}" == resolver(orig_url)

+         assert f"{url}@{digest}" == resolver(orig_url)

+         assert inspect.mock_calls == [mock.call(orig_url)]

+ 

+ 

  class TestGetVariantData(unittest.TestCase):

      def test_get_simple(self):

          conf = {"foo": {"^Client$": 1}}

When the compose is configured to include any container image, it just followed the provided URL. This is not particularly reproducible. If the image spec contains a tag, it may point to different images at different time.

This commit adds a step to validating the configuration that will query the registry and replace the tag with a digest.

This makes it more reproducible, and also fixes a problem where changing container image would not stop ISO reuse. There's still a chance of non-container file changing and not forcing the reuse, but that is not very common.

I'm not very happy about how the resolving is hooked into the config validation. But I did not find a much better way to do it yet.

rebased onto 6a9c855

2 months ago

rebased onto 252044a

a month ago

After discussion, no additional actions are needed for me.

Looks good to me. :thumbsup:

Pull-Request has been merged by lsedlar

23 days ago