-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Workaround hash tagger incorrect parsed image ref #9866
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?
Workaround hash tagger incorrect parsed image ref #9866
Conversation
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.
Summary of Changes
Hello @reingart, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request provides a crucial workaround for a parsing anomaly observed with Buildkit, where FROM instructions utilizing variable expansions (like $BASE) are incorrectly resolved to "image:latest". This misinterpretation previously caused failures in Skaffold's input digest tagger, particularly for multi-stage builds or those with inter-artifact dependencies. The implemented change ensures that Skaffold can correctly process these Dockerfiles, maintaining the integrity of image tagging and build processes without requiring a fix within Buildkit itself.
Highlights
- Buildkit Parser Workaround: Implemented a workaround in Skaffold's image parsing logic to ignore "image:latest" values returned by Buildkit when a FROM instruction uses a variable expansion (e.g., FROM $BASE). This prevents parseOnbuild from failing due to an incorrect image reference.
- Fix for Input Digest Tagger: Addresses an issue where the input digest tagger would fail for images with dependencies on other Skaffold build artifacts, specifically when Buildkit incorrectly parsed FROM instructions with variable expansions.
- Example Update: The simple-artifact-dependency example has been updated to demonstrate the problem and verify the fix, including modifications to its Dockerfile and skaffold.yaml to enable input digest tagging.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces a workaround for an issue where the buildkit parser incorrectly returns 'image:latest' for a FROM instruction with a variable expansion. The fix is to ignore this specific value, similar to how empty image names are handled, which prevents the input digest tagger from failing. The changes are minimal and well-targeted, and the inclusion of an updated example to reproduce and verify the fix is a great addition. My only suggestion is to avoid using a magic string in the condition.
1dacdb9
to
b31e5cb
Compare
Fixes: #9830
Description
Buildkit AST parser library seems to return 'image:latest' when a FROM has a variable expansion template instead of the actual image name, for example,
$BASE
in the following stanza:This breaks input digest tagger for images that have a dependency to other skaffold build artifacts, as
parseOnbuild
later fails to pull the incorrect/inexistent image.Note that empty images in the from were being ignored, but no this incorrect value.
So, as a workaround, this condition is extended to include and ignore incorrect 'image:latest' values.
Actual digest seems not affected, changes in parent base dockerfiles are still computed for the digest via the transitive dependencies.
A more complete fix would be to inject the proper resolved image refs in
expandBuildArgs
(so correct image is downloaded and ONBUILD instructions can be properly parsed), but this will need to solve the issue in buildkit parser to return the template.Also, probably it will require having pre-computed the previous image tags dependencies.
Full error message:
User facing changes
Example for simple artifact has been updated to show/reproduce, check in tests & confirm fix for issue #9830
Before (note that taggers failed and image is not found, being rebuilt unnecessarily):
After (note tagger works ok):