-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix DockerImageName compatibility check when digest is present #11629
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
base: main
Are you sure you want to change the base?
Changes from all commits
6c576a5
6da85e7
a63de7c
3651ae9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,12 +79,26 @@ class Sha256Versioning implements Versioning { | |
|
|
||
| private final String hash; | ||
|
|
||
| private final String tag; | ||
|
|
||
| Sha256Versioning(String hash) { | ||
| this(hash, null); | ||
| } | ||
|
|
||
| Sha256Versioning(String hash, String tag) { | ||
| this.hash = hash; | ||
| this.tag = tag; | ||
| } | ||
|
Comment on lines
+88
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
After introducing the optional Useful? React with 👍 / 👎. |
||
|
|
||
| String getTag() { | ||
| return tag; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isValid() { | ||
| if (tag != null && !tag.matches(TagVersioning.TAG_REGEX)) { | ||
| return false; | ||
| } | ||
| return hash.matches(HASH_REGEX); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the tag has a higher priority than the digest? If yes, I think that is not a good idea, because version tags are mutable and can suddenly point to another build. In contrast, digests are immutable and will always point to the exact same build. In light of recent supply chain attacks, preferring digests over tags is of particular importance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @burneyy , great point on supply chain security — and you're right that tags are mutable while digests are immutable.
However, getVersionPart() returning the tag is intentional here. It's used exclusively for version-based compatibility checks in modules like KafkaContainer, ElasticsearchContainer, Neo4jContainer, etc., where a human-readable version string (e.g. 7.3.0) is needed for version comparisons.
The actual image pulling in RemoteDockerImage already prefers the digest:
String pullTag = imageName.getDigest() != null ? imageName.getDigest() : imageName.getVersionPart();
So when both tag and digest are present, the image is always pulled by its immutable digest — the tag is never used for pulling. The security concern is therefore already handled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @konstantinosGkilas,
I see, apologies for the noise then and thanks for the explanation! Happy to hear that the digest is preferred over the version tag when performing the actual pulling already. Thanks for taking care of this issue - much appreciated 🙏