Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image pinning is broken #389

Open
aboukhal opened this issue Feb 10, 2025 · 1 comment
Open

Image pinning is broken #389

aboukhal opened this issue Feb 10, 2025 · 1 comment

Comments

@aboukhal
Copy link

aboukhal commented Feb 10, 2025

The helm chart supports image pinning and it's part of the default values.yaml.

However, doing the same with the Operator breaks the update checker.

To reproduce:

  1. Set spec.version: 10.4.2 for the Mattermost resource; observe that it starts up fine
  2. Set spec.version: 10.4.2@sha256:f655fae92b8126e7d7f5dcbb796a8bbe32883f0af9cd20841c1be035e241052f
  3. Observe that the update checker fails to start up because the Operator sets an invalid image of mattermost/[email protected]@sha256:f655fae92b8126e7d7f5dcbb796a8bbe32883f0af9cd20841c1be035e241052f

The first @ should actually be a colon.

@aboukhal
Copy link
Author

I found the offending lines of code:
https://github.com/mattermost/mattermost-operator/blob/master/apis/mattermost/v1alpha1/clusterinstallation_utils.go#L195-L197

The operator supports either the Tag or the Digest. This is not how dependency update tools like Renovate-Bot work, however. They set both in order to set the version to be used and to be able to compare the version number with available version numbers to create Pull Requests. The Digest is set to make sure that moving tags won't surprise us without being tested and from a security standpoint.

It would be awesome if the operator could be adjusted to support the scenario where the version contains the tag and the digest. A simple fix would be to change this line:

if strings.Contains(d.Version, "sha256:") {

with this:

if strings.HasPrefix(d.Version, "sha256:") {

Adding an @ as opposed to a : is only necessary if the version number only contains the digest. As soon as it contains something ahead of the digest, It's safe to assume that it's both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant