#1821 Resolve container tags to digests
Merged 10 months ago by lsedlar. Opened a year 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

11 months ago

rebased onto 252044a

11 months ago

After discussion, no additional actions are needed for me.

Looks good to me. :thumbsup:

Pull-Request has been merged by lsedlar

10 months ago