Skip to content
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

feat(dockerfile): add support for escape chars and ARG instructions #15751

Merged
merged 12 commits into from Jun 6, 2022

Conversation

Churro
Copy link
Collaborator

@Churro Churro commented May 27, 2022

Changes

This rework of the dockerfile extractor causes image parts in ARG values (referenced in FROM statements) to be assembled to one string value. The code in this PR enables auto updates for images that occur as a whole within an ARG value (example here) or split into multiple ARG values (example here).

  • Added support for parsing and evaluating ARG name/value definitions that are referenced in FROM instructions
    • As their sequence matters, parsing was changed to a line-wise approach. This is favorable also to prevent false negative matches that currently occur due the position-independent RegEx search, e.g.,:
      COPY --chown=root --from=image11 / ./ # image11 is detected as stageName, although defined only afterwards
      FROM nginx as image11
  • Added support for ` as escape character for multi-line instructions in Dockerfile
  • Slightly simplified FROM and COPY Regex patterns

No existing tests were changed/rewritten, therefore I don't expect any breaking changes.

Known Limitations

 ARG FROM_REGISTRY=ghcr.io
 ARG FROM_IMAGE=${FROM_REGISTRY:+$FROM_REGISTRY/owner/sample-image}
 FROM ${FROM_IMAGE:-scratch}

Context

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Test repositories:

Churro and others added 2 commits May 30, 2022 14:24
Co-authored-by: StinkyLord <42116482+PhilipAbed@users.noreply.github.com>
Copy link
Collaborator

@PhilipAbed PhilipAbed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but i'm not 100% familiar with docker files and this flow so others should review too.

@rarkins rarkins requested a review from zharinov June 2, 2022 08:53
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving based on the quality of code, extensiveness of tests, and the fact that existing tests were left as-is, hopefully indicating backwards compatibility

Copy link
Collaborator

@zharinov zharinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, it's the last set of changes before resorting to tokenizer/parser

@rarkins
Copy link
Collaborator

rarkins commented Jun 2, 2022

@Churro thank you for this amazingly good first-time contribution, both with quality of code as well as extensive testing. I will leave to @viceice to merge once he is available to confirm he has no further comments.

@rarkins rarkins changed the title feat(manager/dockerfile): add support for escape chars and ARG instructions feat(dockerfile): add support for escape chars and ARG instructions Jun 6, 2022
@rarkins rarkins enabled auto-merge (squash) June 6, 2022 05:35
@rarkins rarkins merged commit 44c67da into renovatebot:main Jun 6, 2022
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 32.75.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants