charts/osm: use image digest for default images

[shashankram]

Default images corresponding to the latest released
image should use the image digest instead of tags
to avoid supply chain attacks, which tags are vulernable
to due to being mutatble. Image digests are immutable.

This change updates the way images are published during
a release and also the release workflow as noted in the
release guide. The change does the following:

1. Uses image digests for the control plane images instead
   of tags when the digests are specified. The digests must
   always be specified in release branches, while the code
   in the main branch can continue to use tags for better
   compatibility between the charts and images. As a result
   of this change, all images in the main branch will leverage
   the latest-main tag instead of release tags. This is
   necessary because the CLI/charts are not backward compatible
   with the previous release and thus unusable. The image refs
   in the main branch will thus use the latest-main tag.

2. Updates the release workflow to include a pre-release step
   to publish the control plane images for the release prior
   to the final release being cut. This is necessary because
   the image digests for the released images published to the
   container registry must be included in the charts and CLI
   as a part of the final release step. The release step will
   not rebuild the control plane images, but instead encode them
   into the charts/CLI and upload the release binaries.

3. Updates the release guide to reflect the new release process.

Part of #3715

Testing done:

• Local demo run

When tags are not specified:

        "initContainerImage": "openservicemesh/init@sha256:3717f5054f835ccefc751c6fe5d4b2824038dc49514e811bf867093206ec2ce1",
          image: "openservicemesh/osm-crds@"
          image: "openservicemesh/osm-bootstrap@"
          image: "openservicemesh/osm-controller@sha256:f77659d771d82c8f053bf008fd513574bde5021ea344289ffd47de863fac4461"
          image: "openservicemesh/osm-injector@sha256:c3ded5e1cd4b02474aac573a7437c8e8a91ada556e05060701af4be3982c02b2"
          image: "openservicemesh/osm-crds@"

When tags are specified:

        "initContainerImage": "openservicemesh/init:foo",
          image: "openservicemesh/osm-crds:foo"
          image: "openservicemesh/osm-bootstrap:foo"
          image: "openservicemesh/osm-controller:foo"
          image: "openservicemesh/osm-injector:foo"
          image: "openservicemesh/osm-crds:foo"

Affected area:

Functional Area
Install	[X]

Please answer the following questions with yes/no.

1. Does this change contain code from or inspired by another project? no

   • Did you notify the maintainers and provide attribution?

2. Is this a breaking change? no

[codecov-commenter]

Codecov Report

Merging #4108 (b5b4571) into main (6e9464d) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4108      +/-   ##
==========================================
- Coverage   69.17%   69.12%   -0.05%     
==========================================
  Files         205      205              
  Lines       11240    11240              
==========================================
- Hits         7775     7770       -5     
- Misses       3413     3418       +5     
  Partials       52       52              

Flag	Coverage Δ
unittests	69.12% <ø> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/crdconversion/crdconversion.go	69.56% <0.00%> (-5.44%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e9464d...b5b4571. Read the comment docs.

[nojnhuh]

Left a couple nits and questions. Overall looks good.

[nojnhuh]

Let's change the pull policy to Always by default if latest-main images are used by default.

[shashankram]

I didn't change this just because the demo scripts sets this to Always. We have to be careful in not setting this to Always for release versions, so it makes sense to default it to IfNotPresent.

[draychev]

@nojnhuh, what's the reason behind your request to change to Always? It seems to me that IfNotPresent is the safest bet for production environments (so Kubernetes does not do any extra unnecessary work) and Always perhaps for development.

[nojnhuh]

I didn't change this just because the demo scripts sets this to Always. We have to be careful in not setting this to Always for release versions, so it makes sense to default it to IfNotPresent.

I suppose we could effectively get this behavior "for free" if the latest-main images were instead named latest and take advantage of the default behavior by blanking the pull policy. Not sure how many people are currently relying on latest images though.

what's the reason behind your request to change to Always?

latest-main is a floating tag whose meaning changes frequently. If I were running a cluster and installed the chart using latest-main images pulled IfNotPresent, then installing a new version of the chart some time later without changing the tag or pull policy will still use the old images since images with the latest-main tag already exist on the node. The only way to get the new images to pull would be to manually delete them from the node. If the images were pulled Always, then each install of OSM would get the newest images automatically. I find those kinds of issues where I'm not running the versions of images that I think I am particularly frustrating.

It seems to me that IfNotPresent is the safest bet for production environments (so Kubernetes does not do any extra unnecessary work) and Always perhaps for development.

Isn't that exactly the distinction we're making here with the tags, where the main branch is set up for development and the release branches are set up for production?

[shashankram]

@nojnhuh I don't mind changing the PullPolicy to Always for the main branch, but we have to make it a part of the release checklist to ensure the PullPolicy is always set to IfNotPresent for released images.
I don't think this should be a major concern for this PR in any way, given that we can easily override it.

[nojnhuh]

Suggested change

Once the pre-release Git tag has been pushed, wait for the Pre-release Github workflow to complete. Upon workflow completion, retrieve the image digests for the given release. The image digests are uploaded as a part of the Pre-release workflow. Note that multiple image digest files will be present in the uploaded artificat, one per image tag, e.g. v0.4.0 and latest. Use the release version specific image digest file to extract the image digests to be used as the default for the control plane images, i.e. osm_image_digest_v0.4.0.txt instead of osm_image_digest_latest.txt if the release version if v0.4.0.

Once the pre-release Git tag has been pushed, wait for the Pre-release Github workflow to complete. Upon workflow completion, retrieve the image digests for the given release. The image digests are uploaded as a part of the Pre-release workflow. Note that multiple image digest files will be present in the uploaded artifact, one per image tag, e.g. v0.4.0 and latest. Use the release version specific image digest file to extract the image digests to be used as the default for the control plane images, i.e. osm_image_digest_v0.4.0.txt instead of osm_image_digest_latest.txt if the release version is v0.4.0.

[nojnhuh]

For context, Shashank and I do think this flow of build images, grab digests, update references to the digests , do the rest of the release could all be automated in a single release CI workflow in the future. Having some manual intervention is the easiest thing to do right now to unblock the rest of these changes and allows us the chance to work around any issues we may run into with this process.

[shashankram]

While I think it would be ideal to have a single workflow, I don't think it's possible because there are code changes necessary once the image digests are available. We might need to auto-commit a PR to achieve that or manipulate tags in a hacky way. I don't have any ideas to coalesce these 2 steps right now.

[draychev]

For readability - I wonder if instead of OpenServiceMesh.image.digest.osmBootstrap this should be OpenServiceMesh.image.osmBootstrap.digest (i.e. swap the positions of digest and osmBootstrap)
This mainly because I see that the comment to this value is osm-boostrap's image digest (first name of image, then digest)

[shashankram]

That works, I felt it's easier to keep image.digest as the top-level key and use the image names as child keys, so we have:

image
  digest:
     foo: ..
     bar: ...
     baz: ...

instead of:

image:
  foo:
    digest: ...
  bar:
    digest: ...
  baz:
    digest: ...

[nojnhuh]

I'm not sure it makes much of a difference now, but if we decide later to encode more things for each image, like if we make it possible to pull different images from different repos and with different tags, then it definitely makes sense to reorganize the values to image.<name>.digest. But reorganizing the values like that would be a breaking change to the Helm chart which we should do our best to avoid.

[shashankram]

For simplicity, I don't think we should be allowing mixing tags for different images, that's an unnecessary complexity I wouldn't consider at the moment. We have been using the image tags property as before, and given that only the digests are different it makes sense to keep this change as is, else it would also make sense to move the tags, pullPolicy etc. per image which I think we should avoid.

[draychev]

LGTM
Thank you!