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

Skip invalid docker image references in helm charts #6595

Closed
wants to merge 1 commit into from

Conversation

lyoung-confluent
Copy link
Contributor

It is possible to include variables/references in a helm chart, ex:

image:
  repository: foo/bar
  tag: "${bar.foo}"

Currently, the docker parser will extract this and construct the docker reference as the string foo/bar:${bar.foo}.

Because this reference contains invalid characters ({, }, $) for a docker tag, the regex match against IMAGE_SPEC will (correctly) result in nil:

lib/dependabot/docker/file_parser.rb:172:in `block (2 levels) in workfile_file_dependencies': undefined method `named_captures' for nil:NilClass (NoMethodError)

By adding a next unless check we can skip over references like this without crashing allowing the rest of the file/directory to be upgraded.

@lyoung-confluent lyoung-confluent requested a review from a team as a code owner February 4, 2023 20:48
Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

Nice find!

Can you add a test?

@jeffwidman jeffwidman added the needs-tests Pull requests that need tests before we can review them label Feb 9, 2023
Copy link
Contributor

github-actions bot commented Feb 9, 2025

👋 This pull request has been marked as stale because it has been open for 2 years with no activity. You can comment on the PR to hold stalebot off for a while, or do nothing. If you do nothing, this pull request will be closed eventually by the stalebot. Please see CONTRIBUTING.md for more policy details.

@markhallen
Copy link
Contributor

Thank you for the contribution @lyoung-confluent. We really appreciate your effort in finding the exact cause of this problem.

It seems that a recent change addressed this issue and you can see the updated code here:

images.each do |string|
# TODO: Support Docker references and path references
details = string.match(IMAGE_SPEC)&.named_captures
next if details.nil?
details["registry"] = nil if details["registry"] == "docker.io"
version = version_from(details)
next unless version
dependency_set << build_dependency(file, details, version)
end

I'm going to close this PR. Please let me know if the problem is resolved for you or if you continue to have issues. Thanks

@markhallen markhallen closed this Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-tests Pull requests that need tests before we can review them
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants