bitnami/charts-syncer

Ensure new image reference works

Opened this issue · 2 comments

Great tool and great demo @tompizmor!

A question that came to my mind during the demo is how do we guarantee that a chart that has been tested in TAC keeps working after we do the transformation (change image references) in this tool?

Do we do any checks before or after doing the transformation? If we don't, would it make sense to compare the container image(s) SHA from the image(s) referenced in the chart and the ones in the local target repo? I do not know how expensive would this be but could be an additional flag (i.e --integrity check in the tool if we are worried about it.

Alternatively and since that ^^ approach might be an overkill (and hard to do because of credentials and so on), we could ensure that they have the same sha if we start crafting our Helm charts pointing to the sha not to a tag no?

instead of:

  registry: gcr.io
  repository: sys-2b0109it/demo/bitnami/apache
  tag: 2.4.43-centos-7-r29

Have this in the values.yaml

  registry: gcr.io
  repository: sys-2b0109it/demo/bitnami/apache
  tag: sha256:a82a3c82cb19e7f037877006c2691371f2c1c895d0b9a641c0ae338a29aef901

I know that tags such as 2.4.43-centos-7-r29 are not meant to be rolling releases but using a sha makes sure that no overrides have happened even if by mistake.

wdyt? cc/ @jotadrilo

I have a similar feeling. I think that using digest or rolling tags actually hides an inner issue: We don't validate the image references that we include in the charts. This is a major issue that could make typos to have big consequences in the user experience.

We have several problems:

  1. Assume that the source tag pattern (in the source chart) matches the destination tag pattern (in the new chart).
  2. Assume that the image references are correct. We don't run any validation nor check. This is a simplification so we don't include the container registry acknowledge in this initial version.
  3. Assume that the image integrity is correct once performed the modification (what this issue is about).

I think that we can't ensure (1). Tags are string references and we can ensure that the image with a given tag in the source is the same image with the same tag in the destination if their digests match (3). However, I think we don't need to use sha256:deadbeef notation to achieve it. I would check if the container library retrieves the digest of an arbitrary image.

This is slightly tied to (2), where we don't run any validation regarding the provided container registry nor the images being added to the chart.

To sum up:

  1. We should validate the provided container registry and chart before move on.
  2. We should validate the included image references before publishing a chart.

Should we keep this issue for (2) while opening a different issue for (1). In the end, we need to include the protos for a container registry. This is something I have mentioned in the container registry syncer PR.

Some notes after talking to @jotadrilo offline.

We are talking about two different things:

1 - Ensure that the syncer is properly configured.
2 - Ensure that the synced chart is pointing to an image that we have built and run tests against, in other words, that we certified.

This issue is about 2, which in my opinion is the most important one since otherwise we are breaking the chain of trust of the asset at this point.

Even though I mentioned to make this tool container image aware, I know that's difficult and maybe unnecessary. But I think there is a simplest way of achieving this goal, to make sure that our charts values.yaml files point to the digest of the images not to their tags (see original description of the issue), by doing that, this syncer will not need to know anything about container registries.

We could argue that our tags are immutable but that's not enough. What if the user re-pushes the image in their internal repo? What if we do it by mistake? At that point we can not claim that chart from being certified.

By having the digest encoded in the chart we could guarantee that the integrity of the images have not being changed during the "copy" process.

@jotadrilo mentioned to me though that our charts might not support setting the digest instead of a tag so if we decide to go through with this solution an effort upstream by the content team will be required cc/ @javsalgar