Fix digest-pinned image references being truncated by FromString#3146
Fix digest-pinned image references being truncated by FromString#3146carloscastrojumo wants to merge 2 commits into
Conversation
|
FromString split the last image path component on every colon and kept only the first two parts, so a reference like registry/name:tag@sha256:digest lost the digest hex and was rendered as the invalid registry/name:tag@sha256, and pods failed with InvalidImageName. Split the digest off before parsing the tag, carry it through Image and ToString, and make overrides of the image identity replace any previous digest instead of carrying it over. Regression introduced in DataDog#2605.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b6c913d78
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| splitImg := strings.Split(stringImage, "/") | ||
| // A digest contains ":" itself (e.g. "@sha256:abc..."), so it must be split off before | ||
| // the name is parsed for a tag | ||
| imageRef, digest, _ := strings.Cut(stringImage, "@") |
There was a problem hiding this comment.
This split fixes FromString, but digest-pinned overrides are now rendered successfully while the version-gating path still parses them with common.GetAgentVersionFromImage (internal/controller/datadogagent/common/utils.go:118) -> utils.GetTagFromImageName (pkg/utils/utils.go:20), which splits on the last colon. For an override like agent:7.64.0@sha256:..., feature gates such as service discovery (internal/controller/datadogagent/feature/servicediscovery/feature.go:81) see the digest hex instead of 7.64.0; because those checks use an “unknown means latest” fallback, they can enable features for an old pinned Agent. Please strip the digest in the shared version parser as well.
Useful? React with 👍 / 👎.
carloscastrojumo
force-pushed
the
fix-image-digest-parsing
branch
from
8b6c913 to
f5fc14a
Compare
GetTagFromImageName split on the last colon, so a digest-pinned image such as agent:7.64.0@sha256:... returned the digest hex instead of 7.64.0. Version-gated features (e.g. service discovery auto-enable) then compared against the hex and fell back to "latest", which could enable features for an older pinned Agent. Strip the digest before splitting, mirroring the FromString fix.
|
Thanks, this is affecting me as well. Looking forward to the release of this fix. |
2 participants
What does this PR do?
Fixes parsing of digest-pinned image references in
pkg/images. Since v1.26.0,FromStringsplits the last image path component on every colon and keeps only the first two parts, so an override such asloses the digest hex and is rendered into pods as
eu.gcr.io/datadoghq/cluster-agent:7.80.0@sha256, which is not a valid image reference. The cluster agent, cluster checks runners, and any rescheduled node agent pods then fail withInvalidImageName.This change splits the digest off before the tag is parsed, carries it through
ImageandToString, and makes an override of the image identity (name or tag) replace any previous digest instead of carrying a stale one over. Digest-only references (registry/name@sha256:...) now parse correctly too.Motivation
We pin agent images by digest. Upgrading the operator from 1.23.1 to 1.27.1 broke every cluster that applied the upgrade: the regression came in with the tag suffix parsing rewrite in #2605 (first released in v1.26.0). Before that, the parser split on the first colon only, so tag plus digest references survived. Backports to v1.27 and v1.28 would be appreciated.
Additional Notes
None.
Minimum Agent Versions
No minimum Agent or Cluster Agent version changes.
Describe your test plan
Added unit tests covering digest parsing in
Test_FromString,Test_ToString,Test_OverrideAgentImage(including the production repro and digest clearing on tag override) andTest_AssembleImage.go test ./pkg/images/...go test ./internal/controller/datadogagent/override/... ./internal/controller/datadogagent/global/...Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel