-
Notifications
You must be signed in to change notification settings - Fork 1.3k
docker: add support for org.opencontainers.image.version and org.opencontainers.image.revision
#13855
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?
Conversation
aa3a074 to
37b1bcc
Compare
37b1bcc to
9a359c5
Compare
org.opencontainers.image.version and org.opencontainers.image.revision
org.opencontainers.image.version and org.opencontainers.image.revisiondocker: add support for org.opencontainers.image.version and org.opencontainers.image.revision
939b164 to
591a979
Compare
| requirement: nil, | ||
| groups: [], | ||
| source: { registry: "ghcr.io", | ||
| digest: "sha256:389a5a9a5457ed237b05d623ddc31a42fa97811051dcd02d7ca4ad46bd3edd3e" } |
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.
The only change I applied to this test is to remove the sha256 prefix as the spec was misleading. Our source parser drops sha256 currently
f66c0ff to
ccd2792
Compare
|
|
||
| Dependabot.logger.info "Building source with branch '#{image_version}' and commit '#{revision}'" | ||
|
|
||
| Dependabot::Source.new( |
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.
I am not sure if there is any easier way to do this other than to change how Source.from_url works which may cause undesired side-effects. I am open for feedback/ideas 👀
3d2cc65 to
a911de9
Compare
a911de9 to
ffee40b
Compare
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.
Pull request overview
This PR extends Docker metadata extraction to support digest-only image references by utilizing OCI annotations org.opencontainers.image.version and org.opencontainers.image.revision. When an image is referenced by digest without a tag, the implementation now attempts to extract version information from these OCI metadata fields to construct a Dependabot::Source object with branch and commit information.
Key changes:
- Added logic to handle digest-only image references using OCI metadata annotations
- Extracted
image_detailsmethod to centralize Docker image inspection logic - Implemented
build_source_from_image_versionto construct Source objects from OCI version/revision labels
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
docker/lib/dependabot/docker/metadata_finder.rb |
Refactored look_up_source to support digest-only images; extracted image_details method for image inspection; added build_source_from_image_version to build Source from OCI annotations |
docker/spec/dependabot/docker/metadata_finder_spec.rb |
Added test coverage for digest-only images with OCI version annotations, digest-only images without proper annotations, and fixed digest format consistency (removed "sha256:" prefix) |
|
@yeikel , There are copilot suggestions. Will it be possible if you go over them? You can either apply the suggested fix or resolve with a feedback. |
5e4befe to
8421b3e
Compare
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| groups: [], | ||
| source: { registry: "ghcr.io", | ||
| digest: "sha256:389a5a9a5457ed237b05d623ddc31a42fa97811051dcd02d7ca4ad46bd3edd3e" } | ||
| digest: "389a5a9a5457ed237b05d623ddc31a42fa97811051dcd02d7ca4ad46bd3edd3e" } |
Copilot
AI
Dec 31, 2025
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.
The digest format is being corrected to remove the "sha256:" algorithm prefix (changed from the old format with prefix to just the hash). This is a breaking change to the test data format that could affect other code relying on this structure. Ensure that all code paths that read or write digest values consistently use the hash-only format without the algorithm prefix.
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.
This is not a breaking change because the tests are still working as is. The test data was just misleading and never using this. The nature of the misleading test data confused me while creating other tests
| # This is a safeguard for a first iteration. We may adjust this later based on user feedback. | ||
| tag_like = image_version&.start_with?("v") | ||
|
|
||
| return unless tag_like || revision |
Copilot
AI
Dec 31, 2025
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.
The code supports digest-only images with just a revision (commit) when no version is present (line 82: return unless tag_like || revision). However, there is no test coverage for this scenario. Consider adding a test case for an image that has org.opencontainers.image.revision but not org.opencontainers.image.version to ensure this path works correctly.
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.
I added the spec with a digest but no tag or revision data to cover this
| # If both tag and digest are present, use the digest as docker ignores the tag when a digest is present | ||
| if digest |
Copilot
AI
Dec 31, 2025
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.
The code handles the case where both tag and digest are present (line 53-54: "If both tag and digest are present, use the digest as docker ignores the tag when a digest is present"). However, there is no test coverage for this scenario. Consider adding a test case with both tag and digest specified to verify the digest takes precedence.
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.
I added the spec with a docker image with both tag and sha that has an OCI source annotation to cover this scenario
| # We only want to build a source if the version looks like a tag (starts with "v") | ||
| # This is a safeguard for a first iteration. We may adjust this later based on user feedback. | ||
| tag_like = image_version&.start_with?("v") |
Copilot
AI
Dec 31, 2025
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.
The heuristic for determining if a version is "tag-like" only checks if it starts with "v" (line 80). This is overly restrictive and will miss many valid semantic versions that don't use the "v" prefix (e.g., "1.2.3", "2024.01"). Consider using a more robust check, such as matching against semantic versioning patterns or at least including versions that start with digits.
| # We only want to build a source if the version looks like a tag (starts with "v") | |
| # This is a safeguard for a first iteration. We may adjust this later based on user feedback. | |
| tag_like = image_version&.start_with?("v") | |
| # We only want to build a source if the version looks like a tag (starts with "v" or a digit) | |
| # This is a safeguard for a first iteration. We may adjust this later based on user feedback. | |
| tag_like = | |
| if image_version | |
| image_version.start_with?("v") || image_version.match?(/\A\d/) | |
| else | |
| false | |
| end |
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.
This is a safeguard for a first iteration. We may adjust this later based on user feedback. I'd like to avoid being overly ambitions because it may cause incorrect behavior such as the Ubuntu example documented in the specs
0a9feab to
37785e8
Compare
@kbukum1 I applied the feedback for the latest round of Copilot's feedback Thanks! |
…sion and org.opencontainers.image.revision Extends metadata extraction to handle digest-only image references, where no tag is present. This new logic will run when one of the following additional OCI metadata fields are present along with the digest: - `org.opencontainers.image.version`: Packaged software version. This can sometimes be a tag - `org.opencontainers.image.revision`: Source control revision identifier for the packaged software
37785e8 to
cd071ad
Compare
What are you trying to accomplish?
Extends metadata extraction to handle digest-only image references, where no tag is present.
This new logic will run when one of the following additional OCI metadata fields are present along with the digest:
From the OCI docs:
org.opencontainers.image.version: Packaged software version. This can sometimes be a tagorg.opencontainers.image.revision: Source control revision identifier for the packaged softwareHow will you know you've accomplished your goal?
Checklist