Draft
Conversation
Previous image parsing and dependency tree generation failed to properly compare hashes resulting in duplicate digest values being stored and duplicate image names being generated.
This change ensures that images references in the form of 'image.tar:tag' will fallback to the previous manual parsing code, rather than attempt to parse the string as an OCI image reference.
15af853 to
72c36b0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Centralizes all OCI image reference parsing in
lib/image-reference.tsand uses it everywhere the plugin interprets image reference strings (e.g.nginx:latest,gcr.io/project/nginx:1.23@sha256:...). This removes duplicate regexes and ad-hoc parsing, and keeps behavior consistent across scan, pull, metadata, and dependency tree.Where should the reviewer start?
lib/image-reference.ts: Single source of truth for image string parsing. Derived from CNCF regex.test/lib/image-reference.spec.ts: Tests that define the intended parsing behavior. Copies pre-existing validation tests fromtests/lib/utils.spec.tsand adds new tests to validate the component parsing.Callers:
lib/scan.ts: Uses the new validation andappendLatestTagIfMissinglogic.lib/analyzer/image-inspector.ts:extractImageDetailsrefactor (replaced helpers with one parser call + mapping).lib/extractor/image.ts:ImageNameconstructor refactor to useparseImageReferenceand map to existing shape.lib/extractor/oci-distribution-metadata.ts:constructOCIDisributionMetadatausesparseImageReference(imageName)instead of@swimlane/docker-reference’sparseAll.lib/dependency-tree/index.ts: AddednameAndVersionFromTargetImagehelper tobuildTreewhich usesparseImageReferencebefore falling back to previous logic.How should this be manually tested?
TODO
Any background context you want to provide?
ImageName Creation
The previous ImageName constructor attempted to deduplicate digests if a digest was included in the image reference string. However, this comparison always fails, meaning that if the image included a pinned digest it would always be added as
digests.other. This then results in two duplicate names being created by thegetAllNamesfunction. This bug was captured in some of our snapshot tests causing these tests to fail after the bug was corrected. The snapshots have been updated to reflect the corrected behavior.Dependency Tree Generation
The
buildTreefunction used "ad-hoc" parsing to extract an "imageName" and "version" from the image reference string that are used as the image identifiers in analysis. This PR adds a helper to perform this parsing with a fallback to the previous logic. Tests have been added intest/lib/dependency-tree/index.spec.tsto ensure the fallback did not impact the previous functionality which is reached for archive image references. These tests also make the parsing results more explicit and could be a focus for further improvement.Removed:
lib/utils.ts: this was a duplicate source of image string validation.@swimlane/docker-referenceremoved as a dependency, another duplicate sourcce of image string parsing.What are the relevant tickets?
TODO
Screenshots
N/A
Additional questions
None